From 7cb143e970cddfb68189cd8d6b8271519e13b3ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Thu, 11 Nov 2021 16:00:35 +0100 Subject: [PATCH] crypto: Don't create a new salt when we just want to rederive a recovery key --- .../crypto/keysbackup/RustKeyBackupService.kt | 51 ++++++++++++++-- rust-sdk/src/backup_recovery_key.rs | 61 +++++++++++-------- rust-sdk/src/machine.rs | 6 +- rust-sdk/src/olm.udl | 4 +- 4 files changed, 90 insertions(+), 32 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/keysbackup/RustKeyBackupService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/keysbackup/RustKeyBackupService.kt index 26d44bfc38..e1c46b48be 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/keysbackup/RustKeyBackupService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/keysbackup/RustKeyBackupService.kt @@ -120,7 +120,7 @@ internal class RustKeyBackupService @Inject constructor( runCatching { withContext(coroutineDispatchers.crypto) { val key = if (password != null) { - BackupRecoveryKey.fromPassphrase(password) + BackupRecoveryKey.newFromPassphrase(password) } else { BackupRecoveryKey() } @@ -397,9 +397,15 @@ internal class RustKeyBackupService @Inject constructor( callback: MatrixCallback) { cryptoCoroutineScope.launch { try { - val key = BackupRecoveryKey.fromPassphrase(password) - checkRecoveryKey(key, keysBackupVersion) - trustKeysBackupVersion(keysBackupVersion, true, callback) + val key = recoveryKeyFromPassword(password, keysBackupVersion) + + if (key == null) { + Timber.w("trustKeysBackupVersionWithPassphrase: Key backup is missing required data") + callback.onFailure(IllegalArgumentException("Missing element")) + } else { + checkRecoveryKey(key, keysBackupVersion) + trustKeysBackupVersion(keysBackupVersion, true, callback) + } } catch (exception: Throwable) { callback.onFailure(exception) } @@ -589,7 +595,15 @@ internal class RustKeyBackupService @Inject constructor( cryptoCoroutineScope.launch(coroutineDispatchers.main) { runCatching { val recoveryKey = withContext(coroutineDispatchers.crypto) { - BackupRecoveryKey.fromPassphrase(password) + val key = recoveryKeyFromPassword(password, keysBackupVersion) + + if (key == null) { + Timber.w("trustKeysBackupVersionWithPassphrase: Key backup is missing required data") + + throw IllegalArgumentException("Missing element") + } else { + key + } } restoreBackup(keysBackupVersion, recoveryKey, roomId, sessionId, stepProgressListener) @@ -749,6 +763,33 @@ internal class RustKeyBackupService @Inject constructor( return SavedKeyBackupKeyInfo(info.recoveryKey, info.backupVersion) } + /** + * Compute the recovery key from a password and key backup version. + * + * @param password the password. + * @param keysBackupData the backup and its auth data. + * + * @return the recovery key if successful, null in other cases + */ + @WorkerThread + private fun recoveryKeyFromPassword(password: String, keysBackupData: KeysVersionResult): BackupRecoveryKey? { + val authData = getMegolmBackupAuthData(keysBackupData) + + if (authData == null) { + Timber.w("recoveryKeyFromPassword: invalid parameter") + return null + } + + if (authData.privateKeySalt.isNullOrBlank() + || authData.privateKeyIterations == null) { + Timber.w("recoveryKeyFromPassword: Salt and/or iterations not found in key backup auth data") + + return null + } + + return BackupRecoveryKey.fromPassphrase(password, authData.privateKeySalt, authData.privateKeyIterations) + } + /** * Extract MegolmBackupAuthData data from a backup version. * diff --git a/rust-sdk/src/backup_recovery_key.rs b/rust-sdk/src/backup_recovery_key.rs index 6ddf6d05ff..3eb82a30c5 100644 --- a/rust-sdk/src/backup_recovery_key.rs +++ b/rust-sdk/src/backup_recovery_key.rs @@ -5,47 +5,48 @@ use sha2::Sha512; use std::{collections::HashMap, iter}; use thiserror::Error; -use matrix_sdk_crypto::backups::{RecoveryKey, OlmPkDecryptionError}; +use matrix_sdk_crypto::backups::{OlmPkDecryptionError, RecoveryKey}; -/// TODO +/// The private part of the backup key, the one used for recovery. pub struct BackupRecoveryKey { pub(crate) inner: RecoveryKey, passphrase_info: Option, } -/// TODO +/// Error type for the decryption of backed up room keys. #[derive(Debug, Error)] pub enum PkDecryptionError { - /// TODO + /// An internal libolm error happened during decryption. #[error("Error decryption a PkMessage {0}")] Olm(#[from] OlmPkDecryptionError), } -/// TODO +/// Struct containing info about the way the backup key got derived from a +/// passphrase. #[derive(Debug, Clone)] pub struct PassphraseInfo { - /// TODO + /// The salt that was used during key derivation. pub private_key_salt: String, - /// TODO + /// The number of PBKDF rounds that were used for key derivation. pub private_key_iterations: i32, } -/// TODO +/// The public part of the backup key. pub struct BackupKey { - /// TODO + /// The actuall base64 encoded public key. pub public_key: String, - /// TODO + /// Signatures that have signed our backup key. pub signatures: HashMap>, - /// TODO + /// The passphrase info, if the key was derived from one. pub passphrase_info: Option, } impl BackupRecoveryKey { const KEY_SIZE: usize = 32; const SALT_SIZE: usize = 32; - const PBKDF_ROUNDS: u32 = 500_000; + const PBKDF_ROUNDS: i32 = 500_000; - /// TODO + /// Create a new random [`BackupRecoveryKey`]. pub fn new() -> Self { Self { inner: RecoveryKey::new() @@ -54,26 +55,26 @@ impl BackupRecoveryKey { } } - /// TODO + /// Try to create a [`BackupRecoveryKey`] from a base 64 encoded string. pub fn from_base64(key: String) -> Self { Self { + // TODO remove the unwrap inner: RecoveryKey::from_base64(&key).unwrap(), passphrase_info: None, } } - /// TODO + /// Try to create a [`BackupRecoveryKey`] from a base 58 encoded string. pub fn from_base58(key: String) -> Self { Self { + // TODO remove the unwrap inner: RecoveryKey::from_base58(&key).unwrap(), passphrase_info: None, } } - /// TODO - pub fn from_passphrase(passphrase: String) -> Self { - let mut key = [0u8; Self::KEY_SIZE]; - + /// Create a new [`BackupRecoveryKey`] from the given passphrase. + pub fn new_from_passphrase(passphrase: String) -> Self { let mut rng = thread_rng(); let salt: String = iter::repeat(()) .map(|()| rng.sample(Alphanumeric)) @@ -81,10 +82,18 @@ impl BackupRecoveryKey { .take(Self::SALT_SIZE) .collect(); + Self::from_passphrase(passphrase, salt, Self::PBKDF_ROUNDS) + } + + /// Restore a [`BackupRecoveryKey`] from the given passphrase. + pub fn from_passphrase(passphrase: String, salt: String, rounds: i32) -> Self { + let mut key = [0u8; Self::KEY_SIZE]; + let rounds = rounds as u32; + pbkdf2::>( passphrase.as_bytes(), salt.as_bytes(), - Self::PBKDF_ROUNDS, + rounds, &mut key, ); @@ -92,12 +101,12 @@ impl BackupRecoveryKey { inner: RecoveryKey::from_bytes(key), passphrase_info: Some(PassphraseInfo { private_key_salt: salt, - private_key_iterations: Self::PBKDF_ROUNDS as i32, + private_key_iterations: rounds as i32, }), } } - /// TODO + /// Get the public part of the backup key. pub fn public_key(&self) -> BackupKey { let public_key = self.inner.public_key(); @@ -113,22 +122,24 @@ impl BackupRecoveryKey { .collect(); BackupKey { - public_key: public_key.encoded_key(), + public_key: public_key.to_base64(), signatures, passphrase_info: self.passphrase_info.clone(), } } - /// TODO + /// Convert the recovery key to a base 58 encoded string. pub fn to_base58(&self) -> String { self.inner.to_base58() } - /// TODO + /// Convert the recovery key to a base 64 encoded string. pub fn to_base64(&self) -> String { self.inner.to_base64() } + /// Try to decrypt a message that was encrypted using the public part of the + /// backup key. pub fn decrypt( &self, ephemeral_key: String, diff --git a/rust-sdk/src/machine.rs b/rust-sdk/src/machine.rs index 272d675779..f8cc596ef7 100644 --- a/rust-sdk/src/machine.rs +++ b/rust-sdk/src/machine.rs @@ -1311,7 +1311,11 @@ impl OlmMachine { key: Option, version: Option, ) -> Result<(), CryptoStoreError> { - let key = key.map(|k| RecoveryKey::from_base64(&k)).transpose().ok().flatten(); + let key = key + .map(|k| RecoveryKey::from_base64(&k)) + .transpose() + .ok() + .flatten(); Ok(self .runtime .block_on(self.inner.backup_machine().save_recovery_key(key, version))?) diff --git a/rust-sdk/src/olm.udl b/rust-sdk/src/olm.udl index 0006dc4ae5..41d77a3aca 100644 --- a/rust-sdk/src/olm.udl +++ b/rust-sdk/src/olm.udl @@ -400,7 +400,9 @@ interface BackupRecoveryKey { [Name=from_base64] constructor(string key); [Name=from_passphrase] - constructor(string key); + constructor(string passphrase, string salt, i32 rounds); + [Name=new_from_passphrase] + constructor(string passphrase); [Name=from_base58] constructor(string key); string to_base58();