Skip to content

Commit

Permalink
fix: wallet database encryption does not bind to field keys tari-proj…
Browse files Browse the repository at this point in the history
…ect#4137 (tari-project#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
---
tari-project#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
  • Loading branch information
agubarev committed Aug 3, 2022
1 parent a059b99 commit 32184b5
Show file tree
Hide file tree
Showing 8 changed files with 299 additions and 66 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
}
}
Expand Down
108 changes: 73 additions & 35 deletions base_layer/wallet/src/storage/sqlite_db/wallet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
};

Expand Down Expand Up @@ -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)?;
},
}
Expand All @@ -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)?
},
};
Expand Down Expand Up @@ -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)?;
},
}
Expand All @@ -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()))?
},
Expand Down Expand Up @@ -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);

Expand All @@ -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
Expand All @@ -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)?;
}

Expand Down Expand Up @@ -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)?;
Expand All @@ -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()))?;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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(())
}
}
Expand Down
Loading

0 comments on commit 32184b5

Please sign in to comment.