Skip to content

Commit

Permalink
Encrypt work factor (#477)
Browse files Browse the repository at this point in the history
* public access to encrypt_work_factor added

* fmt + clippy

* changes added

* enable 'zeroize/alloc' feature to enable

* allow clippy

* comment removed
  • Loading branch information
semenov-vladyslav committed Jun 8, 2023
1 parent 7a2e9dd commit 988a9d1
Show file tree
Hide file tree
Showing 10 changed files with 45 additions and 10 deletions.
5 changes: 5 additions & 0 deletions .changes/snapshot_encrypt_work_factor.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"stronghold-engine": patch
---

Added snapshot encryption work factor public access. It should only be used in tests to decrease snapshot encryption/decryption times. It must not be used in production as low values of work factor might lead to secrets/seeds leakage.
10 changes: 6 additions & 4 deletions client/src/procedures/primitives.rs
Original file line number Diff line number Diff line change
Expand Up @@ -500,16 +500,18 @@ impl DeriveSecret<1> for Slip10Derive {
Slip10DeriveInput::Key(_) => {
let r = &*guards[0].borrow();
if r.len() != 64 {
return Err(FatalProcedureError::from("bad slip10 extended secret key size".to_owned()));
return Err(FatalProcedureError::from(
"bad slip10 extended secret key size".to_owned(),
));
}
let mut ext_bytes = Zeroizing::new([0_u8; 65]);
ext_bytes.as_mut()[1..].copy_from_slice(&r);
ext_bytes.as_mut()[1..].copy_from_slice(r);
match self.curve {
Curve::Ed25519 => slip10::Slip10::<ed25519::SecretKey>::try_from_extended_bytes(&*ext_bytes)
Curve::Ed25519 => slip10::Slip10::<ed25519::SecretKey>::try_from_extended_bytes(&ext_bytes)
.and_then(|parent| parent.derive(&self.chain))
.map(|dk| (Zeroizing::new(dk.extended_bytes()[1..].into()), *dk.chain_code())),
Curve::Secp256k1 => {
slip10::Slip10::<secp256k1_ecdsa::SecretKey>::try_from_extended_bytes(&*ext_bytes)
slip10::Slip10::<secp256k1_ecdsa::SecretKey>::try_from_extended_bytes(&ext_bytes)
.and_then(|parent| parent.derive(&self.chain))
.map(|dk| (Zeroizing::new((dk.extended_bytes()[1..]).into()), *dk.chain_code()))
}
Expand Down
7 changes: 7 additions & 0 deletions client/src/tests/interface_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ fn test_stronghold_purge_client() {

#[test]
fn purge_client() {
engine::snapshot::try_set_encrypt_work_factor(0).unwrap();
// This test will create a client, write secret data into the vault, commit
// the state into a snapshot. Then purge the client, commit the purged state
// and reload the client, with an empty state
Expand Down Expand Up @@ -254,6 +255,7 @@ fn purge_client() {

#[test]
fn write_client_to_snapshot() {
engine::snapshot::try_set_encrypt_work_factor(0).unwrap();
let stronghold = Stronghold::default();

let snapshot_path = {
Expand Down Expand Up @@ -298,6 +300,7 @@ fn write_client_to_snapshot() {

#[test]
fn test_load_client_from_snapshot() {
engine::snapshot::try_set_encrypt_work_factor(0).unwrap();
let client_path = fixed_random_bytes(1024);
let vault_path = fixed_random_bytes(1024);
let record_path = fixed_random_bytes(1024);
Expand Down Expand Up @@ -346,6 +349,7 @@ fn test_load_client_from_snapshot() {

#[test]
fn test_load_multiple_clients_from_snapshot() {
engine::snapshot::try_set_encrypt_work_factor(0).unwrap();
let number_of_clients = 10;
let client_path_vec: Vec<Vec<u8>> = (0..number_of_clients).map(|_| fixed_random_bytes(256)).collect();
let mut clients = vec![];
Expand Down Expand Up @@ -449,6 +453,7 @@ fn test_create_snapshot_file_in_custom_directory() {

#[test]
fn test_clear_stronghold_state() {
engine::snapshot::try_set_encrypt_work_factor(0).unwrap();
// pre-requisites
let client_path = "my-awesome-client-path";
let vault_path = b"vault_path".to_vec();
Expand Down Expand Up @@ -581,6 +586,7 @@ fn test_keyprovider_hashed_passphrase_blake2b() {

#[test]
fn test_stronghold_with_key_location_for_snapshot() {
engine::snapshot::try_set_encrypt_work_factor(0).unwrap();
let client_path = "my-awesome-client-path";
let vault_path = b"vault_path".to_vec();
let record_path = b"record_path".to_vec();
Expand Down Expand Up @@ -636,6 +642,7 @@ fn test_stronghold_with_key_location_for_snapshot() {

#[test]
fn test_load_unload_client() {
engine::snapshot::try_set_encrypt_work_factor(0).unwrap();
let stronghold = Stronghold::default();
let client_path = "my-awesome-client-path";
let client = stronghold.create_client(client_path).expect("Failed to create client");
Expand Down
2 changes: 1 addition & 1 deletion client/src/tests/procedure_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ async fn usecase_secp256k1_slip10_derive_key() -> Result<(), Box<dyn std::error:
let chain_code = client.execute_procedure(slip10_derive).unwrap();

let secp256k1_ecdsa_pk = PublicKey {
private_key: key.clone(),
private_key: key,
ty: KeyType::Secp256k1Ecdsa,
};
let pk = client.execute_procedure(secp256k1_ecdsa_pk).unwrap();
Expand Down
1 change: 1 addition & 0 deletions client/tests/test_multithreaded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ fn test_stronghold_multithreaded_safety() {
// - Check that all the secrets that were generated concurrently before are present in the saved state
#[test]
fn test_full_stronghold_access_multithreaded() {
engine::snapshot::try_set_encrypt_work_factor(0).unwrap();
const NB_INPUTS: usize = 100;
let pool = ThreadPool::new(NB_THREADS);

Expand Down
2 changes: 1 addition & 1 deletion derive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ pub fn permissions(input: proc_macro::TokenStream) -> proc_macro::TokenStream {
}
}
Data::Struct(_) | Data::Union(_) => {
let ident = derive_input.ident.clone();
let ident = derive_input.ident;
quote! {
impl VariantPermission for #ident {
fn permission(&self) -> PermissionValue {
Expand Down
2 changes: 1 addition & 1 deletion engine/runtime/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ name = "runtime"
[dependencies]
libc = { version = "0.2" }
log = { version = "0.4.17" }
zeroize = { version = "1.5.7", default-features = false, features = [ "zeroize_derive" ] }
zeroize = { version = "1.5.7", default-features = false, features = [ "alloc", "zeroize_derive" ] }
libsodium-sys = { version = "0.2" }
serde = { version = "1.0", features = [ "derive" ] }
random = { version = "0.8.4", package = "rand" }
Expand Down
1 change: 1 addition & 0 deletions engine/runtime/src/boxed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,7 @@ mod test {
boxed.lock();
}

#[allow(clippy::redundant_clone)]
#[test]
fn test_eq() {
let boxed_a = Boxed::<u8>::random(1);
Expand Down
1 change: 1 addition & 0 deletions engine/runtime/src/memories/buffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,7 @@ mod tests {
assert_eq!(borrow, clone);
}

#[allow(clippy::redundant_clone)]
#[test]
fn buffer_comparisons() {
let guard = Buffer::<u8>::from(&mut [1, 2, 3][..]);
Expand Down
24 changes: 21 additions & 3 deletions engine/src/snapshot/logic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use std::{
fs::{rename, File, OpenOptions},
io::{Read, Write},
path::Path,
sync::atomic::{AtomicU8, Ordering},
};

use crypto::{keys::age, utils::rand};
Expand Down Expand Up @@ -71,6 +72,25 @@ pub enum WriteError {
IncorrectWorkFactor,
}

// `ENCRYPT_WORK_FACTOR` exposes public access to the default work_factor used in snapshot encryption. Small values of
// work_factor used together with a weak password will result in insecure snapshot protection and potential leakage of
// all secrets, including seed and private keys. Too large values will result in significantly slow snapshot
// encryption/decryption.
//
// The public access is exposed as a workaround so that encryption/decryption time can be controllably low during
// testing. The work_factor must not be modified in production.
static ENCRYPT_WORK_FACTOR: AtomicU8 = AtomicU8::new(age::RECOMMENDED_MINIMUM_ENCRYPT_WORK_FACTOR);

pub fn get_encrypt_work_factor() -> u8 {
ENCRYPT_WORK_FACTOR.load(Ordering::Relaxed)
}

pub fn try_set_encrypt_work_factor(work_factor: u8) -> Result<(), WriteError> {
let _ = age::WorkFactor::try_from(work_factor).map_err(|_| WriteError::IncorrectWorkFactor)?;
ENCRYPT_WORK_FACTOR.store(work_factor, Ordering::Relaxed);
Ok(())
}

/// Encrypt snapshot content with key using work factor recommended for password-based (weak) keys.
///
/// # Security
Expand All @@ -85,9 +105,7 @@ pub enum WriteError {
/// It is safe to use with strong keys, although computing resources may be wasted.
/// In this case it is recommended to use `encrypt_content_with_work_factor` with small/zero work factor.
pub fn encrypt_content<O: Write>(plain: &[u8], output: &mut O, key: &Key) -> Result<(), WriteError> {
let work_factor = age::RECOMMENDED_MINIMUM_ENCRYPT_WORK_FACTOR;
// `work_factor = 1` is intentionally just for development.
// let work_factor = 1;
let work_factor = get_encrypt_work_factor();
encrypt_content_with_work_factor(plain, output, key, work_factor)
}

Expand Down

0 comments on commit 988a9d1

Please sign in to comment.