Skip to content

Commit

Permalink
Database secondary key commitment
Browse files Browse the repository at this point in the history
  • Loading branch information
AaronFeickert committed Feb 17, 2023
1 parent 799bae5 commit 701dc14
Show file tree
Hide file tree
Showing 2 changed files with 98 additions and 6 deletions.
4 changes: 4 additions & 0 deletions base_layer/wallet/src/storage/database.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ pub enum DbKey {
EncryptedMainKey, // the database encryption key, itself encrypted with the secondary key
SecondaryKeySalt, // the salt used (with the user's passphrase) to derive the secondary key
SecondaryKeyVersion, // the parameter version for the secondary key, which determines how it is derived
SecondaryKeyHash, // a hash of the secondary key, used for AEAD key commitment
WalletBirthday,
}

Expand All @@ -90,6 +91,7 @@ impl DbKey {
DbKey::EncryptedMainKey => "EncryptedMainKey".to_string(),
DbKey::SecondaryKeySalt => "SecondaryKeySalt".to_string(),
DbKey::SecondaryKeyVersion => "SecondaryKeyVersion".to_string(),
DbKey::SecondaryKeyHash => "SecondaryKeyHash".to_string(),
DbKey::WalletBirthday => "WalletBirthday".to_string(),
DbKey::CommsIdentitySignature => "CommsIdentitySignature".to_string(),
}
Expand All @@ -108,6 +110,7 @@ pub enum DbValue {
EncryptedMainKey(String),
SecondaryKeySalt(String),
SecondaryKeyVersion(String),
SecondaryKeyHash(String),
WalletBirthday(String),
}

Expand Down Expand Up @@ -348,6 +351,7 @@ impl Display for DbValue {
DbValue::EncryptedMainKey(k) => f.write_str(&format!("EncryptedMainKey: {:?}", k)),
DbValue::SecondaryKeySalt(s) => f.write_str(&format!("SecondaryKeySalt: {}", s)),
DbValue::SecondaryKeyVersion(v) => f.write_str(&format!("SecondaryKeyVersion: {}", v)),
DbValue::SecondaryKeyHash(h) => f.write_str(&format!("SecondaryKeyHash: {}", h)),
DbValue::WalletBirthday(b) => f.write_str(&format!("WalletBirthday: {}", b)),
DbValue::CommsIdentitySignature(_) => f.write_str("CommsIdentitySignature"),
}
Expand Down
100 changes: 94 additions & 6 deletions base_layer/wallet/src/storage/sqlite_db/wallet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ use tari_comms::{
peer_manager::{IdentitySignature, PeerFeatures},
tor::TorIdentity,
};
use tari_crypto::{hash::blake2::Blake256, hash_domain, hashing::DomainSeparatedHasher};
use tari_key_manager::cipher_seed::CipherSeed;
use tari_utilities::{
hex::{from_hex, Hex},
Expand Down Expand Up @@ -76,6 +77,14 @@ hidden_type!(WalletSecondaryEncryptionKey, SafeArray<u8, { size_of::<Key>() }>);
// Authenticated data prefix for main key encryption; append the encryption version later
const MAIN_KEY_AAD_PREFIX: &str = "wallet_main_key_encryption_v";

// Hash domain for secondary key commitment
// This allows us to augment the main key encryption AEAD with key commitment
hash_domain!(
SecondaryKeyHashDomain,
"com.tari.tari_project.base_layer.wallet.secondary_key_commitment",
0
);

/// A structure to hold `Argon2` parameter versions, which may change over time and must be supported
#[derive(Clone)]
pub struct Argon2Parameters {
Expand Down Expand Up @@ -108,13 +117,15 @@ impl Argon2Parameters {
pub struct DatabaseEncryptionFields {
secondary_key_version: u8, // the encryption parameter version
secondary_key_salt: String, // the high-entropy salt used to derive the secondary key
secondary_key_hash: Vec<u8>, // a hash of the secondary key, used for AEAD key commitment
encrypted_main_key: Vec<u8>, // the main key, encrypted with the secondary key
}
impl DatabaseEncryptionFields {
/// Read and parse field data from the database atomically
pub fn read(connection: &SqliteConnection) -> Result<Option<Self>, WalletStorageError> {
let mut secondary_key_version: Option<String> = None;
let mut secondary_key_salt: Option<String> = None;
let mut secondary_key_hash: Option<String> = None;
let mut encrypted_main_key: Option<String> = None;

// Read all fields atomically
Expand All @@ -124,6 +135,8 @@ impl DatabaseEncryptionFields {
.map_err(|_| Error::RollbackTransaction)?;
secondary_key_salt = WalletSettingSql::get(&DbKey::SecondaryKeySalt, connection)
.map_err(|_| Error::RollbackTransaction)?;
secondary_key_hash = WalletSettingSql::get(&DbKey::SecondaryKeyHash, connection)
.map_err(|_| Error::RollbackTransaction)?;
encrypted_main_key = WalletSettingSql::get(&DbKey::EncryptedMainKey, connection)
.map_err(|_| Error::RollbackTransaction)?;

Expand All @@ -132,20 +145,33 @@ impl DatabaseEncryptionFields {
.map_err(|_| WalletStorageError::UnexpectedResult("Unable to read key fields from database".into()))?;

// Parse the fields
match (secondary_key_version, secondary_key_salt, encrypted_main_key) {
match (
secondary_key_version,
secondary_key_salt,
secondary_key_hash,
encrypted_main_key,
) {
// It's fine if none of the fields are set
(None, None, None) => Ok(None),
(None, None, None, None) => Ok(None),

// If all of the fields are set, they must be parsed as valid
(Some(secondary_key_version), Some(secondary_key_salt), Some(encrypted_main_key)) => {
(
Some(secondary_key_version),
Some(secondary_key_salt),
Some(secondary_key_hash),
Some(encrypted_main_key),
) => {
let secondary_key_version = u8::from_str(&secondary_key_version)
.map_err(|e| WalletStorageError::BadEncryptionVersion(e.to_string()))?;
let secondary_key_hash =
from_hex(&secondary_key_hash).map_err(|e| WalletStorageError::ConversionError(e.to_string()))?;
let encrypted_main_key =
from_hex(&encrypted_main_key).map_err(|e| WalletStorageError::ConversionError(e.to_string()))?;

Ok(Some(DatabaseEncryptionFields {
secondary_key_version,
secondary_key_salt,
secondary_key_hash,
encrypted_main_key,
}))
},
Expand All @@ -168,6 +194,9 @@ impl DatabaseEncryptionFields {
WalletSettingSql::new(DbKey::SecondaryKeySalt, self.secondary_key_salt.to_string())
.set(connection)
.map_err(|_| Error::RollbackTransaction)?;
WalletSettingSql::new(DbKey::SecondaryKeyHash, self.secondary_key_hash.to_hex())
.set(connection)
.map_err(|_| Error::RollbackTransaction)?;
WalletSettingSql::new(DbKey::EncryptedMainKey, self.encrypted_main_key.to_hex())
.set(connection)
.map_err(|_| Error::RollbackTransaction)?;
Expand Down Expand Up @@ -409,8 +438,9 @@ impl WalletSqliteDatabase {
DbKey::CommsAddress |
DbKey::BaseNodeChainMetadata |
DbKey::EncryptedMainKey |
DbKey::SecondaryKeySalt |
DbKey::SecondaryKeyVersion |
DbKey::SecondaryKeySalt |
DbKey::SecondaryKeyHash |
DbKey::WalletBirthday |
DbKey::CommsIdentitySignature => {
return Err(WalletStorageError::OperationNotSupported);
Expand Down Expand Up @@ -455,8 +485,9 @@ impl WalletBackend for WalletSqliteDatabase {
DbKey::CommsFeatures => self.get_comms_features(&conn)?.map(DbValue::CommsFeatures),
DbKey::BaseNodeChainMetadata => self.get_chain_metadata(&conn)?.map(DbValue::BaseNodeChainMetadata),
DbKey::EncryptedMainKey => WalletSettingSql::get(key, &conn)?.map(DbValue::EncryptedMainKey),
DbKey::SecondaryKeySalt => WalletSettingSql::get(key, &conn)?.map(DbValue::SecondaryKeySalt),
DbKey::SecondaryKeyVersion => WalletSettingSql::get(key, &conn)?.map(DbValue::SecondaryKeyVersion),
DbKey::SecondaryKeySalt => WalletSettingSql::get(key, &conn)?.map(DbValue::SecondaryKeySalt),
DbKey::SecondaryKeyHash => WalletSettingSql::get(key, &conn)?.map(DbValue::SecondaryKeyHash),
DbKey::WalletBirthday => WalletSettingSql::get(key, &conn)?.map(DbValue::WalletBirthday),
DbKey::CommsIdentitySignature => WalletSettingSql::get(key, &conn)?
.and_then(|s| from_hex(&s).ok())
Expand Down Expand Up @@ -542,13 +573,21 @@ impl WalletBackend for WalletSqliteDatabase {
let new_secondary_key_salt = SaltString::generate(&mut OsRng).to_string();
let new_secondary_key = derive_secondary_key(new, new_argon2_params.clone(), &new_secondary_key_salt)?;

// Hash the secondary key to commit to it
let new_secondary_key_hash = DomainSeparatedHasher::<Blake256, SecondaryKeyHashDomain>::new()
.chain(new_secondary_key.reveal())
.finalize()
.as_ref()
.to_vec();

// Encrypt the main key with the new secondary key
let new_encrypted_main_key = encrypt_main_key(&new_secondary_key, &main_key, new_argon2_params.id)?;

// Store the new key-related fields
DatabaseEncryptionFields {
secondary_key_version: new_argon2_params.id,
secondary_key_salt: new_secondary_key_salt,
secondary_key_hash: new_secondary_key_hash,
encrypted_main_key: new_encrypted_main_key,
}
.write(&conn)?;
Expand Down Expand Up @@ -641,13 +680,21 @@ fn get_db_cipher(
let secondary_key_salt = SaltString::generate(&mut rng).to_string();
let secondary_key = derive_secondary_key(passphrase, argon2_params.clone(), &secondary_key_salt)?;

// Hash the secondary key to commit to it
let secondary_key_hash = DomainSeparatedHasher::<Blake256, SecondaryKeyHashDomain>::new()
.chain(secondary_key.reveal())
.finalize()
.as_ref()
.to_vec();

// Use the secondary key to encrypt the main key
let encrypted_main_key = encrypt_main_key(&secondary_key, &main_key, argon2_params.id)?;

// Store the key-related fields
DatabaseEncryptionFields {
secondary_key_version: argon2_params.id,
secondary_key_salt,
secondary_key_hash,
encrypted_main_key,
}
.write(&conn)?;
Expand All @@ -664,6 +711,18 @@ fn get_db_cipher(
// Derive the secondary key from the user's passphrase and salt
let secondary_key = derive_secondary_key(passphrase, argon2_params, &data.secondary_key_salt)?;

// Check the secondary key against the existing hash
let secondary_key_hash = DomainSeparatedHasher::<Blake256, SecondaryKeyHashDomain>::new()
.chain(secondary_key.reveal())
.finalize()
.as_ref()
.to_vec();
if secondary_key_hash != data.secondary_key_hash {
return Err(WalletStorageError::AeadError(
"Secondary key hash does not match".into(),
));
}

// Attempt to decrypt and return the encrypted main key
decrypt_main_key(&secondary_key, &data.encrypted_main_key, data.secondary_key_version)?
},
Expand Down Expand Up @@ -809,7 +868,11 @@ impl Encryptable<XChaCha20Poly1305> for ClientKeyValueSql {
mod test {
use tari_key_manager::cipher_seed::CipherSeed;
use tari_test_utils::random::string;
use tari_utilities::{hex::from_hex, ByteArray, SafePassword};
use tari_utilities::{
hex::{from_hex, Hex},
ByteArray,
SafePassword,
};
use tempfile::tempdir;

use crate::{
Expand Down Expand Up @@ -865,6 +928,31 @@ mod test {
assert!(WalletSqliteDatabase::new(connection, "new passphrase".to_string().into()).is_ok());
}

#[test]
#[allow(unused_must_use)]
fn test_malleated_secondary_key_hash() {
// Set up a database
let db_name = format!("{}.sqlite3", string(8).as_str());
let db_tempdir = tempdir().unwrap();
let db_folder = db_tempdir.path().to_str().unwrap().to_string();
let db_path = format!("{}/{}", db_folder, db_name);
let connection = run_migration_and_create_sqlite_connection(db_path, 16).unwrap();

// Encrypt with a passphrase
WalletSqliteDatabase::new(connection.clone(), "passphrase".to_string().into()).unwrap();

// Loading the wallet should succeed
assert!(WalletSqliteDatabase::new(connection.clone(), "passphrase".to_string().into()).is_ok());

// Manipulate the secondary key hash; this is a (poor) proxy for an AEAD attack
let evil_secondary_key_hash = vec![0u8; 32];
WalletSettingSql::new(DbKey::SecondaryKeyHash, evil_secondary_key_hash.to_hex())
.set(&connection.get_pooled_connection().unwrap());

// Loading the wallet should fail
assert!(WalletSqliteDatabase::new(connection, "passphrase".to_string().into()).is_err());
}

#[test]
fn test_encryption_is_forced() {
let db_name = format!("{}.sqlite3", string(8).as_str());
Expand Down

0 comments on commit 701dc14

Please sign in to comment.