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

feat: refactor key-related field operations to be atomic #5178

Merged
merged 3 commits into from
Feb 14, 2023
Merged

feat: refactor key-related field operations to be atomic #5178

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
82ddca4
Refactor key-related database field operations
AaronFeickert Feb 13, 2023
3a1ebf8
Formatting and lints
AaronFeickert Feb 13, 2023
280f98e
Rename struct
AaronFeickert Feb 14, 2023
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
200 changes: 121 additions & 79 deletions base_layer/wallet/src/storage/sqlite_db/wallet.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,82 @@ impl Argon2Parameters {
}
}

/// A structure to hold key-related database field data, to make atomic operations cleaner
pub struct DatabaseKeyFields {
secondary_key_version: u8, // the encryption parameter version
secondary_key_salt: String, // the high-entropy salt used to derive the secondary key
encrypted_main_key: Vec<u8>, // the main key, encrypted with the secondary key
}
impl DatabaseKeyFields {
/// 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 encrypted_main_key: Option<String> = None;

// Read all fields atomically
connection
.transaction::<_, Error, _>(|| {
secondary_key_version = WalletSettingSql::get(&DbKey::SecondaryKeyVersion, connection)
.map_err(|_| Error::RollbackTransaction)?;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This mapping is not actually needed. Any Err will rollback the transaction. The docs on diesel.rs don't reflect this in v1.4, but have been updated in the 2.0 docs on docs.rs. In either case, I checked the source and it rolls back on Err.

So it's better to just return the error with ? and have the original error find their way to the logs

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I had tried this initially, but the compiler complained that the internal error couldn't be converted to a diesel error.

secondary_key_salt = WalletSettingSql::get(&DbKey::SecondaryKeySalt, connection)
.map_err(|_| Error::RollbackTransaction)?;
encrypted_main_key = WalletSettingSql::get(&DbKey::EncryptedMainKey, connection)
.map_err(|_| Error::RollbackTransaction)?;

Ok(())
})
.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) {
// It's fine if none of the fields are set
(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)) => {
let secondary_key_version = u8::from_str(&secondary_key_version)
.map_err(|e| WalletStorageError::BadEncryptionVersion(e.to_string()))?;
let encrypted_main_key =
from_hex(&encrypted_main_key).map_err(|e| WalletStorageError::ConversionError(e.to_string()))?;

Ok(Some(DatabaseKeyFields {
secondary_key_version,
secondary_key_salt,
encrypted_main_key,
}))
},

// If only some fields are present, there is an invalid state
_ => Err(WalletStorageError::UnexpectedResult(
"Not all key data is present in the database".into(),
)),
}
}

/// Encode and write field data to the database atomically
pub fn write(&self, connection: &SqliteConnection) -> Result<(), WalletStorageError> {
// Because the encoding can't fail, just do it inside the write transaction
connection
.transaction::<_, Error, _>(|| {
WalletSettingSql::new(DbKey::SecondaryKeyVersion, self.secondary_key_version.to_string())
.set(connection)
.map_err(|_| Error::RollbackTransaction)?;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same comment here

WalletSettingSql::new(DbKey::SecondaryKeySalt, self.secondary_key_salt.to_string())
.set(connection)
.map_err(|_| Error::RollbackTransaction)?;
WalletSettingSql::new(DbKey::EncryptedMainKey, self.encrypted_main_key.to_hex())
.set(connection)
.map_err(|_| Error::RollbackTransaction)?;

Ok(())
})
.map_err(|_| WalletStorageError::UnexpectedResult("Unable to write key fields into database".into()))?;

Ok(())
}
}

/// A Sqlite backend for the Output Manager Service. The Backend is accessed via a connection pool to the Sqlite file.
#[derive(Clone)]
pub struct WalletSqliteDatabase {
Expand Down Expand Up @@ -446,60 +522,45 @@ impl WalletBackend for WalletSqliteDatabase {
fn change_passphrase(&self, existing: &SafePassword, new: &SafePassword) -> Result<(), WalletStorageError> {
let conn = self.database_connection.get_pooled_connection()?;

let secondary_key_version = WalletSettingSql::get(&DbKey::SecondaryKeyVersion, &conn)?;
let secondary_key_salt = WalletSettingSql::get(&DbKey::SecondaryKeySalt, &conn)?;
let encrypted_main_key = WalletSettingSql::get(&DbKey::EncryptedMainKey, &conn)?;

// If any of these aren't present, something went wrong internally, so abort
match (secondary_key_version, secondary_key_salt, encrypted_main_key) {
(Some(secondary_key_version), Some(secondary_key_salt), Some(encrypted_main_key)) => {
// Get the existing key-related data so we can decrypt the main key
match DatabaseKeyFields::read(&conn) {
// Key-related data was present and valid
Ok(Some(data)) => {
// Use the given version if it is valid
let version = u8::from_str(&secondary_key_version)
.map_err(|e| WalletStorageError::BadEncryptionVersion(e.to_string()))?;
let argon2_params = Argon2Parameters::from_version(Some(version))?;
let argon2_params = Argon2Parameters::from_version(Some(data.secondary_key_version))?;

// Derive a secondary key from the existing passphrase and salt
let secondary_key = derive_secondary_key(existing, argon2_params.clone(), &secondary_key_salt)?;
let secondary_key = derive_secondary_key(existing, argon2_params.clone(), &data.secondary_key_salt)?;

// Attempt to decrypt the encrypted main key
let main_key = decrypt_main_key(&secondary_key, &encrypted_main_key, argon2_params.id)?;
let main_key = decrypt_main_key(&secondary_key, &data.encrypted_main_key, argon2_params.id)?;

// Now use the most recent version
let new_argon2_params = Argon2Parameters::from_version(None)?;

// Derive a new secondary key from the new passphrase and a fresh salt
let new_secondary_key_salt = SaltString::generate(&mut OsRng);
let new_secondary_key =
derive_secondary_key(new, new_argon2_params.clone(), &new_secondary_key_salt.to_string())?;
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)?;

// 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 secondary key version, secondary key salt, and encrypted main key
conn.transaction::<_, Error, _>(|| {
// If any operation fails, trigger a rollback
WalletSettingSql::new(DbKey::SecondaryKeyVersion, new_argon2_params.id.to_string())
.set(&conn)
.map_err(|_| Error::RollbackTransaction)?;
WalletSettingSql::new(DbKey::SecondaryKeySalt, new_secondary_key_salt.to_string())
.set(&conn)
.map_err(|_| Error::RollbackTransaction)?;
WalletSettingSql::new(DbKey::EncryptedMainKey, new_encrypted_main_key.to_hex())
.set(&conn)
.map_err(|_| Error::RollbackTransaction)?;

Ok(())
})
.map_err(|_| {
WalletStorageError::UnexpectedResult("Unable to update database for password change".into())
})?;
// Store the new key-related fields
DatabaseKeyFields {
secondary_key_version: new_argon2_params.id,
secondary_key_salt: new_secondary_key_salt,
encrypted_main_key: new_encrypted_main_key,
}
.write(&conn)?;
},

// If any key-related is not present, this is an invalid state
_ => {
return Err(WalletStorageError::UnexpectedResult(
"Not enough data provided to decrypt encrypted main key".into(),
));
"Unable to get valid key-related data from database".into(),
))
},
}
};

Ok(())
}
Expand Down Expand Up @@ -541,7 +602,7 @@ fn encrypt_main_key(
/// Decrypt the main database key using the secondary key
fn decrypt_main_key(
secondary_key: &WalletSecondaryEncryptionKey,
encrypted_main_key: &str,
encrypted_main_key: &[u8],
version: u8,
) -> Result<WalletMainEncryptionKey, WalletStorageError> {
// Set up the authenticated data
Expand All @@ -552,12 +613,8 @@ fn decrypt_main_key(
let cipher = XChaCha20Poly1305::new(Key::from_slice(secondary_key.reveal()));

Ok(WalletMainEncryptionKey::from(
decrypt_bytes_integral_nonce(
&cipher,
aad,
&from_hex(encrypted_main_key).map_err(|e| WalletStorageError::ConversionError(e.to_string()))?,
)
.map_err(|_| WalletStorageError::InvalidPassphrase)?,
decrypt_bytes_integral_nonce(&cipher, aad, encrypted_main_key)
.map_err(|_| WalletStorageError::InvalidPassphrase)?,
))
}

Expand All @@ -568,14 +625,10 @@ fn get_db_cipher(
) -> Result<XChaCha20Poly1305, WalletStorageError> {
let conn = database_connection.get_pooled_connection()?;

// Fetch the database fields used for encryption, if they exist
let secondary_key_version = WalletSettingSql::get(&DbKey::SecondaryKeyVersion, &conn)?;
let secondary_key_salt = WalletSettingSql::get(&DbKey::SecondaryKeySalt, &conn)?;
let encrypted_main_key = WalletSettingSql::get(&DbKey::EncryptedMainKey, &conn)?;

let main_key = match (secondary_key_version, secondary_key_salt, encrypted_main_key) {
// Either set up a new main key, or decrypt it using existing data
let main_key = match DatabaseKeyFields::read(&conn) {
// Encryption is not set up yet
(None, None, None) => {
Ok(None) => {
// Generate a high-entropy main key
let mut main_key = WalletMainEncryptionKey::from(vec![0u8; size_of::<Key>()]);
let mut rng = OsRng;
Expand All @@ -585,51 +638,40 @@ fn get_db_cipher(
let argon2_params = Argon2Parameters::from_version(None)?;

// Derive the secondary key from the user's passphrase and a high-entropy salt
let secondary_key_salt = SaltString::generate(&mut rng);
let secondary_key =
derive_secondary_key(passphrase, argon2_params.clone(), &secondary_key_salt.to_string())?;
let secondary_key_salt = SaltString::generate(&mut rng).to_string();
let secondary_key = derive_secondary_key(passphrase, argon2_params.clone(), &secondary_key_salt)?;

// 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 secondary key version, secondary key salt, and encrypted main key
conn.transaction::<_, Error, _>(|| {
// If any operation fails, trigger a rollback
WalletSettingSql::new(DbKey::SecondaryKeyVersion, argon2_params.id.to_string())
.set(&conn)
.map_err(|_| Error::RollbackTransaction)?;
WalletSettingSql::new(DbKey::SecondaryKeySalt, secondary_key_salt.to_string())
.set(&conn)
.map_err(|_| Error::RollbackTransaction)?;
WalletSettingSql::new(DbKey::EncryptedMainKey, encrypted_main_key.to_hex())
.set(&conn)
.map_err(|_| Error::RollbackTransaction)?;

Ok(())
})
.map_err(|_| WalletStorageError::UnexpectedResult("Unable to update database for new password".into()))?;
// Store the key-related fields
DatabaseKeyFields {
secondary_key_version: argon2_params.id,
secondary_key_salt,
encrypted_main_key,
}
.write(&conn)?;

// Return the unencrypted main key
main_key
},

// Encryption has already been set up
(Some(secondary_key_version), Some(secondary_key_salt), Some(encrypted_main_key)) => {
Ok(Some(data)) => {
// Use the given version if it is valid
let version = u8::from_str(&secondary_key_version)
.map_err(|e| WalletStorageError::BadEncryptionVersion(e.to_string()))?;
let argon2_params = Argon2Parameters::from_version(Some(version))?;
let argon2_params = Argon2Parameters::from_version(Some(data.secondary_key_version))?;

// Derive the secondary key from the user's passphrase and salt
let secondary_key = derive_secondary_key(passphrase, argon2_params, &secondary_key_salt)?;
let secondary_key = derive_secondary_key(passphrase, argon2_params, &data.secondary_key_salt)?;

// Attempt to decrypt and return the encrypted main key
decrypt_main_key(&secondary_key, &encrypted_main_key, version)?
decrypt_main_key(&secondary_key, &data.encrypted_main_key, data.secondary_key_version)?
},
// We don't have all the data required for encryption
_ => {
error!(target: LOG_TARGET, "Not enough data provided to set up encryption");

// We couldn't get valid key-related data
Err(_) => {
return Err(WalletStorageError::UnexpectedResult(
"Not enough data provided to set up encryption".into(),
"Unable to parse key fields from database".into(),
));
},
};
Expand Down