Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid key prompts when resetting crypto #4586

Merged
merged 3 commits into from
Dec 17, 2024
Merged

Avoid key prompts when resetting crypto #4586

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2c8d122
Avoid key prompts when resetting crypto
dbkr Dec 13, 2024
88b2cc4
Merge branch 'develop' into dbkr/improve_backup_key_storage
dbkr Dec 16, 2024
f9caf58
Fix doc
dbkr Dec 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/crypto-api/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ export interface CryptoApi {

/**
* Bootstrap the secret storage by creating a new secret storage key, add it in the secret storage and
* store the cross signing keys in the secret storage.
* store the cross signing and room key backup key in the secret storage.
*
* - Generate a new key {@link GeneratedSecretStorageKey} with `createSecretStorageKey`.
* Only if `setupNewSecretStorage` is set or if there is no AES key in the secret storage
Expand Down
13 changes: 1 addition & 12 deletions src/rust-crypto/rust-crypto.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -863,12 +863,6 @@ export class RustCrypto extends TypedEventEmitter<RustCryptoEvents, CryptoEventH
return;
}

const activeBackupVersion = await this.backupManager.getActiveBackupVersion();
if (!activeBackupVersion || activeBackupVersion !== keyBackupInfo.version) {
logger.info("Not saving backup key to secret storage: backup keys do not match active backup version");
return;
}

const backupKeys: RustSdkCryptoJs.BackupKeys = await this.olmMachine.getBackupKeys();
if (!backupKeys.decryptionKey) {
logger.info("Not saving backup key to secret storage: no backup key");
Expand All @@ -880,14 +874,9 @@ export class RustCrypto extends TypedEventEmitter<RustCryptoEvents, CryptoEventH
return;
}

const backupKeyFromStorage = await this.secretStorage.get("m.megolm_backup.v1");
const backupKeyBase64 = backupKeys.decryptionKey.toBase64();

// The backup version that the key corresponds to isn't saved in 4S so if it's different, we must assume
// it's stale and overwrite.
if (backupKeyFromStorage !== backupKeyBase64) {
await this.secretStorage.store("m.megolm_backup.v1", backupKeyBase64);
}
await this.secretStorage.store("m.megolm_backup.v1", backupKeyBase64);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation on this method says "... and we have secret storage active ..." - is that correct?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mmm, doesn't look like there's a way to determine if we have 4s set up, afaics, so I guess I'll just remove that bit.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is this.secretStorageHasAESKey() which is what resetKeyBackup does, but given this is only called from bootstrapSecretStorage, I don't think there's any need to call it.

}

/**
Expand Down
Loading