Skip to content

Commit

Permalink
feat!: update Argon2 parameters (#5140)
Browse files Browse the repository at this point in the history
Description
---
Updates `Argon2` parameters.

Closes [issue 5139](#5139).

Motivation and Context
---
A recent [update](OWASP/CheatSheetSeries#1073) to the [OWASP recommendations](https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html#argon2id) for `Argon2` password-based key derivation means the codebase is out of date.

This PR updates all `Argon2` parameters to meet this standard. While there are no particularly concerning risks to users with the older standard, it's a matter of good practice to keep these updated where feasible.

Note that this PR does not introduce any kind of key migration, so this change is...


How Has This Been Tested?
---
Existing tests pass.


BREAKING CHANGE: Renders all previous `Argon2`-derived keys invalid.
  • Loading branch information
AaronFeickert committed Jan 30, 2023
1 parent 73d105c commit 4c4a056
Show file tree
Hide file tree
Showing 6 changed files with 11 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ pub fn create_salted_hashed_password(password: &[u8]) -> argon2::password_hash::
// Use the recommended OWASP parameters, which are not the default:
// https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html#argon2id
let params = argon2::Params::new(
37 * 1024, // m-cost: 37 MiB, converted to KiB
46 * 1024, // m-cost: 46 MiB, converted to KiB
1, // t-cost
1, // p-cost
None, // output length: default
Expand Down
2 changes: 1 addition & 1 deletion base_layer/key_manager/src/cipher_seed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ impl CipherSeed {
// We use the recommended OWASP parameters for this:
// https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html#argon2id
let params = argon2::Params::new(
37 * 1024, // m-cost should be 37 Mib = 37 * 1024 Kib
46 * 1024, // m-cost should be 46 MiB = 46 * 1024 KiB
1, // t-cost
1, // p-cost
Some(CIPHER_SEED_ENCRYPTION_KEY_BYTES + CIPHER_SEED_MAC_KEY_BYTES),
Expand Down
6 changes: 3 additions & 3 deletions base_layer/key_manager/src/wasm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,8 +181,8 @@ mod test {
#[wasm_bindgen_test]
fn it_creates_key_manager_from() {
let bytes = [
1, 34, 207, 175, 242, 162, 209, 98, 199, 251, 212, 88, 214, 61, 84, 199, 115, 189, 159, 168, 6, 137, 216,
235, 137, 235, 26, 192, 38, 195, 217, 218, 53,
1, 99, 74, 224, 171, 168, 58, 26, 131, 253, 184, 89, 101, 253, 223, 238, 246, 10, 42, 130, 236, 100, 142,
184, 173, 225, 165, 207, 8, 119, 159, 45, 231,
];
let seed = CipherSeed::from_enciphered_bytes(&bytes, None).unwrap();
let seed = JsValue::from_serde(&seed).unwrap();
Expand All @@ -194,7 +194,7 @@ mod test {
let next_key = response.key_manager.next_key().unwrap();
assert_eq!(
next_key.k.to_hex(),
"7220010f6eb7b1a5429c3e29f3186190312a824cb6551c0c0c4640ecc676da0e".to_string()
"a3c3ea5da2c23049191a184f92f621356311e0d0ed24a073e6a6514a917c1300".to_string()
)
}

Expand Down
4 changes: 2 additions & 2 deletions base_layer/wallet/src/storage/sqlite_db/wallet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,7 @@ fn get_cipher_for_db_encryption(

// These are the parameters for the passphrase hash
let params_passphrase = argon2::Params::new(
37 * 1024, // m-cost: 37 MiB, converted to KiB
46 * 1024, // m-cost: 46 MiB, converted to KiB
1, // t-cost
1, // p-cost
None, // output length: default is fine for this use
Expand Down Expand Up @@ -547,7 +547,7 @@ fn get_cipher_for_db_encryption(
// Use the recommended OWASP parameters, which are not the default:
// https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html#argon2id
let params_encryption = argon2::Params::new(
37 * 1024, // m-cost: 37 MiB, converted to KiB
46 * 1024, // m-cost: 46 MiB, converted to KiB
1, // t-cost
1, // p-cost
Some(size_of::<Key>()), // output length: ChaCha20-Poly1305 key size
Expand Down
13 changes: 2 additions & 11 deletions base_layer/wallet/tests/output_manager_service_tests/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,18 +176,9 @@ async fn setup_output_manager_service<T: OutputManagerBackend + 'static, U: KeyM
wallet_connectivity_mock.set_base_node_wallet_rpc_client(connect_rpc_client(&mut connection).await);
}

// To create a new seed word sequence, uncomment below
// let seed = CipherSeed::new();
// use tari_key_manager::mnemonic::MnemonicLanguage;
// let mnemonic_seq = seed
// .to_mnemonic(MnemonicLanguage::English, None)
// .expect("Couldn't convert CipherSeed to Mnemonic");
// println!("{:?}", mnemonic_seq);

let words = [
"scan", "train", "success", "hover", "prepare", "donor", "upgrade", "attitude", "debate", "emotion", "myself",
"ladder", "display", "athlete", "welcome", "artist", "home", "punch", "sense", "park", "midnight", "quantum",
"bright", "carbon",
"scan", "announce", "neither", "belt", "grace", "arch", "sting", "butter", "run", "frost", "debris", "slide",
"glory", "nature", "asthma", "fame", "during", "silly", "panda", "picnic", "run", "small", "engage", "pride",
];
let seed_words = SeedWords::new(words.iter().map(|s| Hidden::hide(s.to_string())).collect::<Vec<_>>());

Expand Down
13 changes: 2 additions & 11 deletions base_layer/wallet/tests/wallet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -798,18 +798,9 @@ async fn test_recovery_birthday() {
let factories = CryptoFactories::default();
let shutdown = Shutdown::new();

// To create a new seed word sequence, uncomment below
// let seed = CipherSeed::new();
// use tari_key_manager::mnemonic::MnemonicLanguage;
// let mnemonic_seq = seed
// .to_mnemonic(MnemonicLanguage::Spanish, None)
// .expect("Couldn't convert CipherSeed to Mnemonic");
// println!("{:?}", mnemonic_seq);

let vec_words: Vec<Hidden<String>> = [
"octubre", "rinon", "ameno", "rigido", "verbo", "dosis", "ocaso", "fallo", "tez", "ladron", "entrar", "pedal",
"fortuna", "ahogo", "llanto", "mascara", "intuir", "buey", "cubrir", "anillo", "cajon", "entrar", "clase",
"latir",
"scan", "announce", "neither", "belt", "grace", "arch", "sting", "butter", "run", "frost", "debris", "slide",
"glory", "nature", "asthma", "fame", "during", "silly", "panda", "picnic", "run", "small", "engage", "pride",
]
.iter()
.map(|w| Hidden::hide(w.to_string()))
Expand Down

0 comments on commit 4c4a056

Please sign in to comment.