fix: wallet database encryption does not bind to field keys #4137 (#4340

)

Description
---
Added `source_key` to `encrypt_bytes_integral_nonce()` and `decrypt_bytes_integral_nonce()` which are used to encrypt and decrypt values in the storage backend. Also, encrypted values are now suffixed with a MAC.

Motivation and Context
---
#4137
Wallet database field key-value entries are secured in place: AES-GCM is used to encrypt values, with keys left in the clear. However, the value encryption does not bind this operation to the field key. An attacker could replace these values with other encrypted values taken from elsewhere in the database (or otherwise encrypted using the same AES-GCM key) without detection.

One mitigation is to use the field key as associated data passed to the encryption and decryption operations.

How Has This Been Tested?
---
unit test

base_layer/wallet/src/key_manager_service/storage/sqlite_db/key_manager_state.rs

@@ -151,24 +151,38 @@ pub struct KeyManagerStateUpdateSql {
 }
 
 impl Encryptable<Aes256Gcm> for KeyManagerStateSql {
+    fn domain(&self, field_name: &'static str) -> Vec<u8> {
+        [Self::KEY_MANAGER, self.branch_seed.as_bytes(), field_name.as_bytes()]
+            .concat()
+            .to_vec()
+    }
+
     fn encrypt(&mut self, cipher: &Aes256Gcm) -> Result<(), String> {
-        let encrypted_index = encrypt_bytes_integral_nonce(cipher, self.primary_key_index.clone())?;
-        self.primary_key_index = encrypted_index;
+        self.primary_key_index =
+            encrypt_bytes_integral_nonce(cipher, self.domain("primary_key_index"), self.primary_key_index.clone())?;
+
         Ok(())
     }
 
     fn decrypt(&mut self, cipher: &Aes256Gcm) -> Result<(), String> {
-        let decrypted_index = decrypt_bytes_integral_nonce(cipher, self.primary_key_index.clone())?;
-        self.primary_key_index = decrypted_index;
+        self.primary_key_index =
+            decrypt_bytes_integral_nonce(cipher, self.domain("primary_key_index"), self.primary_key_index.clone())?;
 
         Ok(())
     }
 }
 
 impl Encryptable<Aes256Gcm> for NewKeyManagerStateSql {
+    fn domain(&self, field_name: &'static str) -> Vec<u8> {
+        [Self::KEY_MANAGER, self.branch_seed.as_bytes(), field_name.as_bytes()]
+            .concat()
+            .to_vec()
+    }
+
     fn encrypt(&mut self, cipher: &Aes256Gcm) -> Result<(), String> {
-        let encrypted_index = encrypt_bytes_integral_nonce(cipher, self.primary_key_index.clone())?;
-        self.primary_key_index = encrypted_index;
+        self.primary_key_index =
+            encrypt_bytes_integral_nonce(cipher, self.domain("primary_key_index"), self.primary_key_index.clone())?;
+
         Ok(())
     }
 

base_layer/wallet/src/output_manager_service/storage/sqlite_db/mod.rs

@@ -1456,13 +1456,23 @@ impl From<KnownOneSidedPaymentScript> for KnownOneSidedPaymentScriptSql {
 }
 
 impl Encryptable<Aes256Gcm> for KnownOneSidedPaymentScriptSql {
+    fn domain(&self, field_name: &'static str) -> Vec<u8> {
+        [
+            Self::KNOWN_ONESIDED_PAYMENT_SCRIPT,
+            self.script_hash.as_slice(),
+            field_name.as_bytes(),
+        ]
+        .concat()
+        .to_vec()
+    }
+
     fn encrypt(&mut self, cipher: &Aes256Gcm) -> Result<(), String> {
-        self.private_key = encrypt_bytes_integral_nonce(cipher, self.private_key.clone())?;
+        self.private_key = encrypt_bytes_integral_nonce(cipher, self.domain("private_key"), self.private_key.clone())?;
         Ok(())
     }
 
     fn decrypt(&mut self, cipher: &Aes256Gcm) -> Result<(), String> {
-        self.private_key = decrypt_bytes_integral_nonce(cipher, self.private_key.clone())?;
+        self.private_key = decrypt_bytes_integral_nonce(cipher, self.domain("private_key"), self.private_key.clone())?;
         Ok(())
     }
 }

base_layer/wallet/src/output_manager_service/storage/sqlite_db/new_output_sql.rs

@@ -121,15 +121,36 @@ impl NewOutputSql {
 }
 
 impl Encryptable<Aes256Gcm> for NewOutputSql {
+    fn domain(&self, field_name: &'static str) -> Vec<u8> {
+        // WARNING: using `OUTPUT` for both NewOutputSql and OutputSql due to later transition without re-encryption
+        [Self::OUTPUT, self.script.as_slice(), field_name.as_bytes()]
+            .concat()
+            .to_vec()
+    }
+
     fn encrypt(&mut self, cipher: &Aes256Gcm) -> Result<(), String> {
-        self.spending_key = encrypt_bytes_integral_nonce(cipher, self.spending_key.clone())?;
-        self.script_private_key = encrypt_bytes_integral_nonce(cipher, self.script_private_key.clone())?;
+        self.spending_key =
+            encrypt_bytes_integral_nonce(cipher, self.domain("spending_key"), self.spending_key.clone())?;
+
+        self.script_private_key = encrypt_bytes_integral_nonce(
+            cipher,
+            self.domain("script_private_key"),
+            self.script_private_key.clone(),
+        )?;
+
         Ok(())
     }
 
     fn decrypt(&mut self, cipher: &Aes256Gcm) -> Result<(), String> {
-        self.spending_key = decrypt_bytes_integral_nonce(cipher, self.spending_key.clone())?;
-        self.script_private_key = decrypt_bytes_integral_nonce(cipher, self.script_private_key.clone())?;
+        self.spending_key =
+            decrypt_bytes_integral_nonce(cipher, self.domain("spending_key"), self.spending_key.clone())?;
+
+        self.script_private_key = decrypt_bytes_integral_nonce(
+            cipher,
+            self.domain("script_private_key"),
+            self.script_private_key.clone(),
+        )?;
+
         Ok(())
     }
 }

base_layer/wallet/src/output_manager_service/storage/sqlite_db/output_sql.rs

@@ -746,15 +746,36 @@ impl TryFrom<OutputSql> for DbUnblindedOutput {
 }
 
 impl Encryptable<Aes256Gcm> for OutputSql {
+    fn domain(&self, field_name: &'static str) -> Vec<u8> {
+        // WARNING: using `OUTPUT` for both NewOutputSql and OutputSql due to later transition without re-encryption
+        [Self::OUTPUT, self.script.as_slice(), field_name.as_bytes()]
+            .concat()
+            .to_vec()
+    }
+
     fn encrypt(&mut self, cipher: &Aes256Gcm) -> Result<(), String> {
-        self.spending_key = encrypt_bytes_integral_nonce(cipher, self.spending_key.clone())?;
-        self.script_private_key = encrypt_bytes_integral_nonce(cipher, self.script_private_key.clone())?;
+        self.spending_key =
+            encrypt_bytes_integral_nonce(cipher, self.domain("spending_key"), self.spending_key.clone())?;
+
+        self.script_private_key = encrypt_bytes_integral_nonce(
+            cipher,
+            self.domain("script_private_key"),
+            self.script_private_key.clone(),
+        )?;
+
         Ok(())
     }
 
     fn decrypt(&mut self, cipher: &Aes256Gcm) -> Result<(), String> {
-        self.spending_key = decrypt_bytes_integral_nonce(cipher, self.spending_key.clone())?;
-        self.script_private_key = decrypt_bytes_integral_nonce(cipher, self.script_private_key.clone())?;
+        self.spending_key =
+            decrypt_bytes_integral_nonce(cipher, self.domain("spending_key"), self.spending_key.clone())?;
+
+        self.script_private_key = decrypt_bytes_integral_nonce(
+            cipher,
+            self.domain("script_private_key"),
+            self.script_private_key.clone(),
+        )?;
+
         Ok(())
     }
 }

base_layer/wallet/src/storage/sqlite_db/wallet.rs

@@ -25,11 +25,7 @@ use std::{
     sync::{Arc, RwLock},
 };
 
-use aes_gcm::{
-    aead::{generic_array::GenericArray, Aead},
-    Aes256Gcm,
-    NewAead,
-};
+use aes_gcm::{aead::generic_array::GenericArray, Aes256Gcm, NewAead};
 use argon2::{
     password_hash::{rand_core::OsRng, PasswordHash, PasswordHasher, PasswordVerifier, SaltString},
     Argon2,
@@ -58,7 +54,13 @@ use crate::{
         sqlite_db::scanned_blocks::ScannedBlockSql,
         sqlite_utilities::wallet_db_connection::WalletDbConnection,
     },
-    util::encryption::{decrypt_bytes_integral_nonce, encrypt_bytes_integral_nonce, Encryptable, AES_NONCE_BYTES},
+    util::encryption::{
+        decrypt_bytes_integral_nonce,
+        encrypt_bytes_integral_nonce,
+        Encryptable,
+        AES_MAC_BYTES,
+        AES_NONCE_BYTES,
+    },
     utxo_scanner_service::service::ScannedBlock,
 };
 
@@ -95,8 +97,9 @@ impl WalletSqliteDatabase {
             },
             Some(cipher) => {
                 let seed_bytes = seed.encipher(None)?;
-                let ciphertext_integral_nonce = encrypt_bytes_integral_nonce(cipher, seed_bytes)
-                    .map_err(|e| WalletStorageError::AeadError(format!("Encryption Error:{}", e)))?;
+                let ciphertext_integral_nonce =
+                    encrypt_bytes_integral_nonce(cipher, b"wallet_setting_master_seed".to_vec(), seed_bytes)
+                        .map_err(|e| WalletStorageError::AeadError(format!("Encryption Error:{}", e)))?;
                 WalletSettingSql::new(DbKey::MasterSeed.to_string(), ciphertext_integral_nonce.to_hex()).set(conn)?;
             },
         }
@@ -110,8 +113,12 @@ impl WalletSqliteDatabase {
             let seed = match cipher.as_ref() {
                 None => CipherSeed::from_enciphered_bytes(&from_hex(seed_str.as_str())?, None)?,
                 Some(cipher) => {
-                    let decrypted_key_bytes = decrypt_bytes_integral_nonce(cipher, from_hex(seed_str.as_str())?)
-                        .map_err(|e| WalletStorageError::AeadError(format!("Decryption Error:{}", e)))?;
+                    let decrypted_key_bytes = decrypt_bytes_integral_nonce(
+                        cipher,
+                        b"wallet_setting_master_seed".to_vec(),
+                        from_hex(seed_str.as_str())?,
+                    )
+                    .map_err(|e| WalletStorageError::AeadError(format!("Decryption Error:{}", e)))?;
                     CipherSeed::from_enciphered_bytes(&decrypted_key_bytes, None)?
                 },
             };
@@ -172,8 +179,10 @@ impl WalletSqliteDatabase {
             },
             Some(cipher) => {
                 let bytes = bincode::serialize(&tor).map_err(|e| WalletStorageError::ConversionError(e.to_string()))?;
-                let ciphertext_integral_nonce = encrypt_bytes_integral_nonce(cipher, bytes)
-                    .map_err(|e| WalletStorageError::AeadError(format!("Encryption Error:{}", e)))?;
+                let ciphertext_integral_nonce =
+                    encrypt_bytes_integral_nonce(cipher, b"wallet_setting_tor_id".to_vec(), bytes)
+                        .map_err(|e| WalletStorageError::AeadError(format!("Encryption Error:{}", e)))?;
+
                 WalletSettingSql::new(DbKey::TorId.to_string(), ciphertext_integral_nonce.to_hex()).set(conn)?;
             },
         }
@@ -189,8 +198,10 @@ impl WalletSqliteDatabase {
                     TorIdentity::from_json(&key_str).map_err(|e| WalletStorageError::ConversionError(e.to_string()))?
                 },
                 Some(cipher) => {
-                    let decrypted_key_bytes = decrypt_bytes_integral_nonce(cipher, from_hex(&key_str)?)
-                        .map_err(|e| WalletStorageError::AeadError(format!("Decryption Error:{}", e)))?;
+                    let decrypted_key_bytes =
+                        decrypt_bytes_integral_nonce(cipher, b"wallet_setting_tor_id".to_vec(), from_hex(&key_str)?)
+                            .map_err(|e| WalletStorageError::AeadError(format!("Decryption Error:{}", e)))?;
+
                     bincode::deserialize(&decrypted_key_bytes)
                         .map_err(|e| WalletStorageError::ConversionError(e.to_string()))?
                 },
@@ -415,6 +426,7 @@ impl WalletBackend for WalletSqliteDatabase {
             .map_err(|e| WalletStorageError::AeadError(e.to_string()))?
             .hash
             .ok_or_else(|| WalletStorageError::AeadError("Problem generating encryption key hash".to_string()))?;
+
         let key = GenericArray::from_slice(derived_encryption_key.as_bytes());
         let cipher = Aes256Gcm::new(key);
 
@@ -425,11 +437,14 @@ impl WalletBackend for WalletSqliteDatabase {
             None => return Err(WalletStorageError::ValueNotFound(DbKey::MasterSeed)),
             Some(sk) => sk,
         };
+
         let master_seed_bytes = from_hex(master_seed_str.as_str())?;
+
         // Sanity check that the decrypted bytes are a valid CipherSeed
         let _master_seed = CipherSeed::from_enciphered_bytes(&master_seed_bytes, None)?;
-        let ciphertext_integral_nonce = encrypt_bytes_integral_nonce(&cipher, master_seed_bytes)
-            .map_err(|e| WalletStorageError::AeadError(format!("Encryption Error:{}", e)))?;
+        let ciphertext_integral_nonce =
+            encrypt_bytes_integral_nonce(&cipher, b"wallet_setting_master_seed".to_vec(), master_seed_bytes)
+                .map_err(|e| WalletStorageError::AeadError(format!("Encryption Error:{}", e)))?;
         WalletSettingSql::new(DbKey::MasterSeed.to_string(), ciphertext_integral_nonce.to_hex()).set(&conn)?;
 
         // Encrypt all the client values
@@ -445,8 +460,9 @@ impl WalletBackend for WalletSqliteDatabase {
         if let Some(v) = tor_id {
             let tor = TorIdentity::from_json(&v).map_err(|e| WalletStorageError::ConversionError(e.to_string()))?;
             let bytes = bincode::serialize(&tor).map_err(|e| WalletStorageError::ConversionError(e.to_string()))?;
-            let ciphertext_integral_nonce = encrypt_bytes_integral_nonce(&cipher, bytes)
-                .map_err(|e| WalletStorageError::AeadError(format!("Encryption Error:{}", e)))?;
+            let ciphertext_integral_nonce =
+                encrypt_bytes_integral_nonce(&cipher, b"wallet_setting_tor_id".to_vec(), bytes)
+                    .map_err(|e| WalletStorageError::AeadError(format!("Encryption Error:{}", e)))?;
             WalletSettingSql::new(DbKey::TorId.to_string(), ciphertext_integral_nonce.to_hex()).set(&conn)?;
         }
 
@@ -479,8 +495,13 @@ impl WalletBackend for WalletSqliteDatabase {
             Some(sk) => sk,
         };
 
-        let master_seed_bytes = decrypt_bytes_integral_nonce(&cipher, from_hex(master_seed_str.as_str())?)
-            .map_err(|e| WalletStorageError::AeadError(format!("Decryption Error:{}", e)))?;
+        let master_seed_bytes = decrypt_bytes_integral_nonce(
+            &cipher,
+            b"wallet_setting_master_seed".to_vec(),
+            from_hex(master_seed_str.as_str())?,
+        )
+        .map_err(|e| WalletStorageError::AeadError(format!("Decryption Error:{}", e)))?;
+
         // Sanity check that the decrypted bytes are a valid CipherSeed
         let _master_seed = CipherSeed::from_enciphered_bytes(&master_seed_bytes, None)?;
         WalletSettingSql::new(DbKey::MasterSeed.to_string(), master_seed_bytes.to_hex()).set(&conn)?;
@@ -499,10 +520,13 @@ impl WalletBackend for WalletSqliteDatabase {
         // remove tor id encryption if present
         let key_str = WalletSettingSql::get(DbKey::TorId.to_string(), &conn)?;
         if let Some(v) = key_str {
-            let decrypted_key_bytes = decrypt_bytes_integral_nonce(&cipher, from_hex(v.as_str())?)
-                .map_err(|e| WalletStorageError::AeadError(format!("Decryption Error:{}", e)))?;
+            let decrypted_key_bytes =
+                decrypt_bytes_integral_nonce(&cipher, b"wallet_setting_tor_id".to_vec(), from_hex(v.as_str())?)
+                    .map_err(|e| WalletStorageError::AeadError(format!("Decryption Error:{}", e)))?;
+
             let tor_id: TorIdentity = bincode::deserialize(&decrypted_key_bytes)
                 .map_err(|e| WalletStorageError::ConversionError(e.to_string()))?;
+
             let tor_string = tor_id
                 .to_json()
                 .map_err(|e| WalletStorageError::ConversionError(e.to_string()))?;
@@ -582,6 +606,7 @@ fn check_db_encryption_status(
             let argon2 = Argon2::default();
             let stored_hash =
                 PasswordHash::new(&db_passphrase_hash).map_err(|e| WalletStorageError::AeadError(e.to_string()))?;
+
             if let Err(e) = argon2.verify_password(passphrase.reveal(), &stored_hash) {
                 error!(target: LOG_TARGET, "Incorrect passphrase ({})", e);
                 return Err(WalletStorageError::InvalidPassphrase);
@@ -623,18 +648,19 @@ fn check_db_encryption_status(
             Err(_) => {
                 // This means the secret key was encrypted. Try decrypt
                 if let Some(cipher_inner) = cipher.clone() {
-                    let mut sk_bytes: Vec<u8> = from_hex(sk.as_str())?;
-                    if sk_bytes.len() < AES_NONCE_BYTES {
+                    let sk_bytes: Vec<u8> = from_hex(sk.as_str())?;
+
+                    if sk_bytes.len() < AES_NONCE_BYTES + AES_MAC_BYTES {
                         return Err(WalletStorageError::MissingNonce);
                     }
-                    // This leaves the nonce in sk_bytes
-                    let data = sk_bytes.split_off(AES_NONCE_BYTES);
-                    let nonce = GenericArray::from_slice(sk_bytes.as_slice());
 
-                    let decrypted_key = cipher_inner.decrypt(nonce, data.as_ref()).map_err(|e| {
-                        error!(target: LOG_TARGET, "Incorrect passphrase ({})", e);
-                        WalletStorageError::InvalidPassphrase
-                    })?;
+                    let decrypted_key =
+                        decrypt_bytes_integral_nonce(&cipher_inner, b"wallet_setting_master_seed".to_vec(), sk_bytes)
+                            .map_err(|e| {
+                            error!(target: LOG_TARGET, "Incorrect passphrase ({})", e);
+                            WalletStorageError::InvalidPassphrase
+                        })?;
+
                     let _cipher_seed = CipherSeed::from_enciphered_bytes(&decrypted_key, None).map_err(|_| {
                         error!(
                             target: LOG_TARGET,
@@ -749,20 +775,32 @@ impl ClientKeyValueSql {
 }
 
 impl Encryptable<Aes256Gcm> for ClientKeyValueSql {
+    fn domain(&self, field_name: &'static str) -> Vec<u8> {
+        [Self::CLIENT_KEY_VALUE, self.key.as_bytes(), field_name.as_bytes()]
+            .concat()
+            .to_vec()
+    }
+
     #[allow(unused_assignments)]
     fn encrypt(&mut self, cipher: &Aes256Gcm) -> Result<(), String> {
-        let encrypted_value = encrypt_bytes_integral_nonce(cipher, self.value.as_bytes().to_vec())?;
-        self.value = encrypted_value.to_hex();
+        self.value =
+            encrypt_bytes_integral_nonce(cipher, self.domain("value"), self.value.as_bytes().to_vec())?.to_hex();
+
         Ok(())
     }
 
     #[allow(unused_assignments)]
     fn decrypt(&mut self, cipher: &Aes256Gcm) -> Result<(), String> {
-        let decrypted_value =
-            decrypt_bytes_integral_nonce(cipher, from_hex(self.value.as_str()).map_err(|e| e.to_string())?)?;
+        let decrypted_value = decrypt_bytes_integral_nonce(
+            cipher,
+            self.domain("value"),
+            from_hex(self.value.as_str()).map_err(|e| e.to_string())?,
+        )?;
+
         self.value = from_utf8(decrypted_value.as_slice())
             .map_err(|e| e.to_string())?
             .to_string();
+
         Ok(())
     }
 }