diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 3ea508c..b570aa2 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -101,7 +101,7 @@ jobs: cargo build -vv $OPTS --features "$FEATURES" cargo test -vv $OPTS --features "$FEATURES" - - uses: actions/upload-artifact@v3 + - uses: actions/upload-artifact@v4 if: failure() with: name: Build logs ${{ matrix.name }} diff --git a/src/error.rs b/src/error.rs index f28670c..49353cb 100644 --- a/src/error.rs +++ b/src/error.rs @@ -204,6 +204,13 @@ impl From> for Error { } } +use std::array::TryFromSliceError; +impl From for Error { + fn from(error: TryFromSliceError) -> Error { + Error::other_error(error) + } +} + #[macro_export] macro_rules! some_or_err { ($action:expr) => { diff --git a/src/storage/nssdb/ci.rs b/src/storage/nssdb/ci.rs index 2885dd0..02930dd 100644 --- a/src/storage/nssdb/ci.rs +++ b/src/storage/nssdb/ci.rs @@ -1,6 +1,9 @@ // Copyright 2024 Simo Sorce // See LICENSE.txt file for terms +use std::collections::BTreeMap; +use std::sync::{RwLock, RwLockReadGuard}; + use crate::attribute::CkAttrs; use crate::error::Result; use crate::interface::*; @@ -12,6 +15,130 @@ use crate::token::TokenFacilities; use crate::CSPRNG; use crate::{sizeof, void_ptr}; +use zeroize::Zeroize; + +const SHA256_LEN: usize = 32; +const MAX_KEY_CACHE_SIZE: usize = 128; + +enum KeyOp { + Encryption, + Signature, +} + +#[derive(Debug)] +struct LockedKey<'a> { + id: [u8; SHA256_LEN], + l: RwLockReadGuard<'a, BTreeMap<[u8; SHA256_LEN], Object>>, +} + +impl LockedKey<'_> { + fn get_id(&self) -> [u8; SHA256_LEN] { + self.id + } + + pub fn get_key<'a>(&'a self) -> Option<&'a Object> { + self.l.get(&self.id) + } +} + +#[derive(Debug)] +pub struct KeysWithCaching { + enckey: Option>, + cache: RwLock>, +} + +impl Default for KeysWithCaching { + fn default() -> KeysWithCaching { + KeysWithCaching { + enckey: None, + cache: RwLock::new(BTreeMap::new()), + } + } +} + +impl Drop for KeysWithCaching { + fn drop(&mut self) { + if let Some(ref mut key) = &mut self.enckey { + key.zeroize(); + } + } +} + +impl KeysWithCaching { + pub fn available(&self) -> bool { + self.enckey.is_some() + } + + fn get_key(&self) -> Result<&[u8]> { + match &self.enckey { + Some(ref key) => Ok(key.as_slice()), + None => Err(CKR_USER_NOT_LOGGED_IN)?, + } + } + + pub fn check_key(&self, check: &[u8]) -> bool { + match &self.enckey { + Some(ref key) => key.as_slice() == check, + None => false, + } + } + + pub fn set_key(&mut self, key: Vec) { + if let Some(ref mut oldkey) = &mut self.enckey { + oldkey.zeroize(); + } + self.enckey = Some(key); + } + + pub fn unset_key(&mut self) { + if let Some(ref mut key) = &mut self.enckey { + key.zeroize(); + self.enckey = None; + } + } + + fn get_cached_key(&self, id: &[u8; SHA256_LEN]) -> Option { + if self.enckey.is_none() { + /* access to the cache is available only if enckey is set. + * When unset it means the user logged off and no + * keys should be available */ + return None; + } + let read_lock = match self.cache.read() { + Ok(r) => r, + Err(_) => return None, + }; + Some(LockedKey { + id: *id, + l: read_lock, + }) + } + + fn set_cached_key(&self, id: &[u8; SHA256_LEN], key: Object) -> Result<()> { + match self.cache.write() { + Ok(mut w) => { + if w.len() > MAX_KEY_CACHE_SIZE { + let _ = w.pop_last(); + } + let _ = w.insert(*id, key); + } + Err(_) => return Err(CKR_CANT_LOCK)?, + }; + Ok(()) + } + + fn invalidate_cached_key(&self, lk: LockedKey) { + let id = lk.get_id(); + drop(lk); + match self.cache.write() { + Ok(mut w) => { + let _ = w.remove(&id); + } + Err(_) => (), + } + } +} + pub const NSS_MP_PBE_ITERATION_COUNT: usize = 10000; #[derive( @@ -125,9 +252,72 @@ fn aes_cbc_encrypt( Ok((iv[2..].to_vec(), encdata)) } +fn derive_key_internal<'a>( + facilities: &TokenFacilities, + keys: &'a KeysWithCaching, + params: &PBKDF2Params, + operation: KeyOp, +) -> Result> { + let keyid: [u8; 32] = params.salt.try_into()?; + + /* First check if we have this key in cache */ + match keys.get_cached_key(&keyid) { + Some(lk) => match lk.get_key() { + Some(_) => return Ok(lk), + None => (), + }, + None => (), + } + + /* if not compute it */ + let ck_class: CK_OBJECT_CLASS = CKO_SECRET_KEY; + let ck_key_type: CK_KEY_TYPE = match operation { + KeyOp::Encryption => CKK_AES, + KeyOp::Signature => CKK_GENERIC_SECRET, + }; + let ck_key_len: CK_ULONG = match params.key_length { + Some(l) => CK_ULONG::try_from(l)?, + None => return Err(CKR_MECHANISM_PARAM_INVALID)?, + }; + let ck_true: CK_BBOOL = CK_TRUE; + let mut key_template = CkAttrs::with_capacity(5); + key_template.add_ulong(CKA_CLASS, &ck_class); + key_template.add_ulong(CKA_KEY_TYPE, &ck_key_type); + key_template.add_ulong(CKA_VALUE_LEN, &ck_key_len); + match operation { + KeyOp::Encryption => { + key_template.add_bool(CKA_DECRYPT, &ck_true); + key_template.add_bool(CKA_ENCRYPT, &ck_true); + } + KeyOp::Signature => { + key_template.add_bool(CKA_SIGN, &ck_true); + key_template.add_bool(CKA_VERIFY, &ck_true); + } + } + + let key = pbkdf2_derive( + facilities, + params, + keys.get_key()?, + key_template.as_slice(), + )?; + + /* and store in cache for later use */ + keys.set_cached_key(&keyid, key)?; + match keys.get_cached_key(&keyid) { + Some(lk) => match lk.get_key() { + Some(_) => return Ok(lk), + None => (), + }, + None => (), + } + + return Err(CKR_GENERAL_ERROR)?; +} + pub fn decrypt_data( facilities: &TokenFacilities, - secret: &[u8], + keys: &KeysWithCaching, data: &[u8], ) -> Result> { let info = match asn1::parse_single::(data) { @@ -140,43 +330,36 @@ pub fn decrypt_data( _ => return Err(CKR_MECHANISM_INVALID)?, }; - let key = match &pbes2.key_derivation_func.params { + let lockedkey = match &pbes2.key_derivation_func.params { AlgorithmParameters::Pbkdf2(ref params) => { - let ck_class: CK_OBJECT_CLASS = CKO_SECRET_KEY; - let ck_key_type: CK_KEY_TYPE = CKK_AES; - let ck_key_len: CK_ULONG = match params.key_length { - Some(l) => CK_ULONG::try_from(l)?, - None => return Err(CKR_MECHANISM_PARAM_INVALID)?, - }; - let ck_true: CK_BBOOL = CK_TRUE; - let mut key_template = CkAttrs::with_capacity(5); - key_template.add_ulong(CKA_CLASS, &ck_class); - key_template.add_ulong(CKA_KEY_TYPE, &ck_key_type); - key_template.add_ulong(CKA_VALUE_LEN, &ck_key_len); - key_template.add_bool(CKA_DECRYPT, &ck_true); - key_template.add_bool(CKA_ENCRYPT, &ck_true); - - pbkdf2_derive(facilities, params, secret, key_template.as_slice())? + derive_key_internal(facilities, keys, params, KeyOp::Encryption)? } _ => return Err(CKR_MECHANISM_INVALID)?, }; - match &pbes2.encryption_scheme.params { + + let key = match lockedkey.get_key() { + Some(k) => k, + None => return Err(CKR_GENERAL_ERROR)?, + }; + + let result = match &pbes2.encryption_scheme.params { BrokenAlgorithmParameters::Aes128Cbc(ref iv) => { - aes_cbc_decrypt(facilities, &key, iv, info.enc_or_sig_data) + aes_cbc_decrypt(facilities, key, iv, info.enc_or_sig_data) } BrokenAlgorithmParameters::Aes256Cbc(ref iv) => { - aes_cbc_decrypt(facilities, &key, iv, info.enc_or_sig_data) + aes_cbc_decrypt(facilities, key, iv, info.enc_or_sig_data) } _ => Err(CKR_MECHANISM_INVALID)?, + }; + if result.is_err() { + keys.invalidate_cached_key(lockedkey) } + result } -/* SHA2-256 length */ -const SHA256_LEN: usize = 32; - -pub fn encrypt_data<'a>( +pub fn encrypt_data( facilities: &TokenFacilities, - secret: &'a [u8], + keys: &KeysWithCaching, iterations: usize, data: &[u8], ) -> Result> { @@ -191,25 +374,25 @@ pub fn encrypt_data<'a>( prf: Box::new(HMAC_SHA_256_ALG), }; - /* compute key */ - let ck_class: CK_OBJECT_CLASS = CKO_SECRET_KEY; - let ck_key_type: CK_KEY_TYPE = CKK_AES; - let ck_key_len = CK_ULONG::try_from(SHA256_LEN)?; - let ck_true: CK_BBOOL = CK_TRUE; - let mut key_template = CkAttrs::with_capacity(5); - key_template.add_ulong(CKA_CLASS, &ck_class); - key_template.add_ulong(CKA_KEY_TYPE, &ck_key_type); - key_template.add_ulong(CKA_VALUE_LEN, &ck_key_len); - key_template.add_bool(CKA_DECRYPT, &ck_true); - key_template.add_bool(CKA_ENCRYPT, &ck_true); - - let key = pbkdf2_derive( + let lockedkey = derive_key_internal( facilities, + keys, &pbkdf2_params, - secret, - key_template.as_slice(), + KeyOp::Encryption, )?; - let (iv, enc_data) = aes_cbc_encrypt(facilities, &key, data)?; + + let key = match lockedkey.get_key() { + Some(k) => k, + None => return Err(CKR_GENERAL_ERROR)?, + }; + + let (iv, enc_data) = match aes_cbc_encrypt(facilities, key, data) { + Ok(x) => x, + Err(e) => { + keys.invalidate_cached_key(lockedkey); + return Err(e); + } + }; let enc_params = BrokenAlgorithmIdentifier { oid: asn1::DefinedByMarker::marker(), @@ -263,7 +446,7 @@ fn hmac_verify( pub fn check_signature( facilities: &TokenFacilities, - secret: &[u8], + keys: &KeysWithCaching, attribute: &[u8], signature: &[u8], nssobjid: u32, @@ -279,39 +462,34 @@ pub fn check_signature( _ => return Err(CKR_MECHANISM_INVALID)?, }; - let key = match &pbmac1.key_derivation_func.params { + let lockedkey = match &pbmac1.key_derivation_func.params { AlgorithmParameters::Pbkdf2(ref params) => { - let ck_class: CK_OBJECT_CLASS = CKO_SECRET_KEY; - let ck_key_type: CK_KEY_TYPE = CKK_GENERIC_SECRET; - let ck_key_len: CK_ULONG = match params.key_length { - Some(l) => CK_ULONG::try_from(l)?, - None => return Err(CKR_MECHANISM_PARAM_INVALID)?, - }; - let ck_true: CK_BBOOL = CK_TRUE; - let mut key_template = CkAttrs::with_capacity(5); - key_template.add_ulong(CKA_CLASS, &ck_class); - key_template.add_ulong(CKA_KEY_TYPE, &ck_key_type); - key_template.add_ulong(CKA_VALUE_LEN, &ck_key_len); - key_template.add_bool(CKA_SIGN, &ck_true); - key_template.add_bool(CKA_VERIFY, &ck_true); - - pbkdf2_derive(facilities, params, secret, key_template.as_slice())? + derive_key_internal(facilities, keys, params, KeyOp::Signature)? } _ => return Err(CKR_MECHANISM_INVALID)?, }; - match &pbmac1.message_auth_scheme.params { + let key = match lockedkey.get_key() { + Some(k) => k, + None => return Err(CKR_GENERAL_ERROR)?, + }; + + let result = match &pbmac1.message_auth_scheme.params { AlgorithmParameters::HmacWithSha256(None) => hmac_verify( facilities, CKM_SHA256_HMAC_GENERAL, - &key, + key, nssobjid, sdbtype, attribute, info.enc_or_sig_data, ), _ => Err(CKR_MECHANISM_INVALID)?, + }; + if result.is_err() { + keys.invalidate_cached_key(lockedkey); } + result } pub fn enckey_derive( @@ -358,42 +536,41 @@ fn hmac_sign( pub fn make_signature( facilities: &TokenFacilities, - secret: &[u8], + keys: &KeysWithCaching, attribute: &[u8], nssobjid: u32, sdbtype: u32, - iterations: u64, + iterations: usize, ) -> Result> { let mut salt: [u8; SHA256_LEN] = [0u8; SHA256_LEN]; CSPRNG.with(|rng| rng.borrow_mut().generate_random(&mut salt))?; let pbkdf2_params = PBKDF2Params { salt: &salt, - iteration_count: iterations, + iteration_count: u64::try_from(iterations)?, key_length: Some(u64::try_from(SHA256_LEN)?), prf: Box::new(HMAC_SHA_256_ALG), }; - /* compute key */ - let ck_class: CK_OBJECT_CLASS = CKO_SECRET_KEY; - let ck_key_type: CK_KEY_TYPE = CKK_GENERIC_SECRET; - let ck_key_len = CK_ULONG::try_from(SHA256_LEN)?; - let ck_true: CK_BBOOL = CK_TRUE; - let mut key_template = CkAttrs::with_capacity(5); - key_template.add_ulong(CKA_CLASS, &ck_class); - key_template.add_ulong(CKA_KEY_TYPE, &ck_key_type); - key_template.add_ulong(CKA_VALUE_LEN, &ck_key_len); - key_template.add_bool(CKA_SIGN, &ck_true); - key_template.add_bool(CKA_VERIFY, &ck_true); - - let key = pbkdf2_derive( + let lockedkey = derive_key_internal( facilities, + keys, &pbkdf2_params, - secret, - key_template.as_slice(), + KeyOp::Signature, )?; - let sig = hmac_sign(facilities, &key, nssobjid, sdbtype, attribute)?; + let key = match lockedkey.get_key() { + Some(k) => k, + None => return Err(CKR_GENERAL_ERROR)?, + }; + + let sig = match hmac_sign(facilities, key, nssobjid, sdbtype, attribute) { + Ok(x) => x, + Err(e) => { + keys.invalidate_cached_key(lockedkey); + return Err(e); + } + }; let info = NSSEncryptedDataInfo { algorithm: Box::new(BrokenAlgorithmIdentifier { diff --git a/src/storage/nssdb/mod.rs b/src/storage/nssdb/mod.rs index 63993d9..ea2e8d8 100644 --- a/src/storage/nssdb/mod.rs +++ b/src/storage/nssdb/mod.rs @@ -120,15 +120,7 @@ struct NSSSearchQuery { pub struct NSSStorage { config: NSSConfig, conn: Arc>, - enckey: Option>, -} - -impl Drop for NSSStorage { - fn drop(&mut self) { - if let Some(ref mut key) = &mut self.enckey { - key.zeroize(); - } - } + keys: KeysWithCaching, } impl NSSStorage { @@ -823,7 +815,7 @@ impl Storage for NSSStorage { _facilities: &TokenFacilities, ) -> Result { self.initialize()?; - self.enckey = None; + self.keys = KeysWithCaching::default(); self.get_token_info() } @@ -864,15 +856,14 @@ impl Storage for NSSStorage { } let mut obj = self.fetch_by_nssid(&table, nssobjid, attrs.as_slice())?; - if let Some(ref enckey) = self.enckey { + if self.keys.available() { if table == NSS_PRIVATE_TABLE { for typ in NSS_SENSITIVE_ATTRIBUTES { let encval = match obj.get_attr(typ) { Some(attr) => attr.get_value(), None => continue, }; - let plain = - decrypt_data(facilities, enckey.as_slice(), encval)?; + let plain = decrypt_data(facilities, &self.keys, encval)?; obj.set_attr(Attribute::from_bytes(typ, plain))?; } } @@ -891,7 +882,7 @@ impl Storage for NSSStorage { let signature = self.fetch_signature(dbtype, nssobjid, typ)?; check_signature( facilities, - enckey.as_slice(), + &self.keys, value.as_slice(), signature.as_slice(), nssobjid, @@ -923,10 +914,9 @@ impl Storage for NSSStorage { _ => return Err(CKR_ATTRIBUTE_VALUE_INVALID)?, }; - let enckey = match self.enckey { - Some(ref e) => e, - None => return Err(CKR_USER_NOT_LOGGED_IN)?, - }; + if !self.keys.available() { + return Err(CKR_USER_NOT_LOGGED_IN)?; + } if table == NSS_PRIVATE_TABLE { for typ in NSS_SENSITIVE_ATTRIBUTES { @@ -938,7 +928,7 @@ impl Storage for NSSStorage { }; let encval = encrypt_data( facilities, - enckey.as_slice(), + &self.keys, NSS_MP_PBE_ITERATION_COUNT, plain.as_slice(), )?; @@ -967,11 +957,11 @@ impl Storage for NSSStorage { }; let sig = make_signature( facilities, - enckey.as_slice(), + &self.keys, value.as_slice(), nssobjid, u32::try_from(typ)?, - u64::try_from(NSS_MP_PBE_ITERATION_COUNT)?, + NSS_MP_PBE_ITERATION_COUNT, )?; Self::store_signature( &mut tx, @@ -1004,10 +994,9 @@ impl Storage for NSSStorage { }; let (table, nssobjid) = nss_id_parse(nssid)?; - let enckey = match self.enckey { - Some(ref e) => e, - None => return Err(CKR_USER_NOT_LOGGED_IN)?, - }; + if !self.keys.available() { + return Err(CKR_USER_NOT_LOGGED_IN)?; + } let mut attrs = CkAttrs::from(template); @@ -1020,7 +1009,7 @@ impl Storage for NSSStorage { let plain = a.to_buf()?; let encval = encrypt_data( facilities, - enckey.as_slice(), + &self.keys, NSS_MP_PBE_ITERATION_COUNT, plain.as_slice(), )?; @@ -1044,11 +1033,11 @@ impl Storage for NSSStorage { let value = a.to_buf()?; let sig = make_signature( facilities, - enckey.as_slice(), + &self.keys, value.as_slice(), nssobjid, u32::try_from(typ)?, - u64::try_from(NSS_MP_PBE_ITERATION_COUNT)?, + NSS_MP_PBE_ITERATION_COUNT, )?; Self::store_signature( &mut tx, @@ -1141,35 +1130,44 @@ impl Storage for NSSStorage { * key through pbkdf2 and then decrypting the entry according * to the chosen algorithm. The data is a structured ASN.1 * structure that includes in formation about which algorithm - * to use for the decryption. */ + * to use for the decryption. + * NOTE: to allow key caching we set the key unchecked, and + * then remove it on failure or if only a check was requested */ let (salt, data) = self.fetch_password()?; - let mut enckey = enckey_derive(facilities, pin, salt.as_slice())?; - let plain = - decrypt_data(facilities, enckey.as_slice(), data.as_slice())?; - if plain.as_slice() != NSS_PASS_CHECK { - return Err(CKR_PIN_INCORRECT)?; + let enckey = enckey_derive(facilities, pin, salt.as_slice())?; + let originally_set = self.keys.available(); + if originally_set && self.keys.check_key(enckey.as_slice()) { + return Ok(()); + } + self.keys.set_key(enckey); + let check = match decrypt_data(facilities, &self.keys, data.as_slice()) + { + Ok(plain) => { + if plain.as_slice() == NSS_PASS_CHECK { + Ok(()) + } else { + Err(CKR_PIN_INCORRECT)? + } + } + Err(e) => Err(e), + }; + if check.is_err() { + /* unconditionally remove the key on failure */ + self.keys.unset_key(); + return check; } /* NSS does not support any error counter for authentication attempts */ *flag = 0; - if !check_only { - self.enckey = Some(enckey); - } else { - enckey.zeroize(); + if check_only && !originally_set { + self.keys.unset_key(); } Ok(()) } fn unauth_user(&mut self, _user_type: CK_USER_TYPE) -> Result<()> { - match self.enckey { - Some(ref mut key) => { - key.zeroize(); - self.enckey = None; - } - None => (), - } - Ok(()) + Ok(self.keys.unset_key()) } fn set_user_pin( @@ -1188,7 +1186,9 @@ impl Storage for NSSStorage { let mut salt: [u8; NSS_PIN_SALT_LEN] = [0u8; NSS_PIN_SALT_LEN]; CSPRNG.with(|rng| rng.borrow_mut().generate_random(&mut salt))?; - let newenckey = enckey_derive(facilities, pin, &salt)?; + let enckey = enckey_derive(facilities, pin, &salt)?; + let mut newkeys = KeysWithCaching::default(); + newkeys.set_key(enckey); let iterations = match pin.len() { 0 => 1, @@ -1197,15 +1197,16 @@ impl Storage for NSSStorage { NSS_MP_PBE_ITERATION_COUNT } }; - let mut encdata = encrypt_data( - facilities, - newenckey.as_slice(), - iterations, - NSS_PASS_CHECK, - )?; + let mut encdata = + encrypt_data(facilities, &newkeys, iterations, NSS_PASS_CHECK)?; /* FIXME: need to re-encode all encrypted/integrity protected attributes */ + /* now that the pin has changed all cached keys are invalid, replace the lot */ + self.keys = newkeys; + /* changing the pin does not leave the token logged in */ + self.keys.unset_key(); + let result = self.save_password(&salt, encdata.as_slice()); encdata.zeroize(); result @@ -1259,7 +1260,7 @@ impl StorageDBInfo for NSSDBInfo { Ok(Box::new(NSSStorage { config: config, conn: conn, - enckey: None, + keys: KeysWithCaching::default(), })) } diff --git a/src/tests/nssdb.rs b/src/tests/nssdb.rs index 9e880e5..8400967 100644 --- a/src/tests/nssdb.rs +++ b/src/tests/nssdb.rs @@ -290,3 +290,56 @@ fn test_nssdb_init_token_params() { testtokn.finalize(); } + +#[test] +#[parallel] +fn test_nssdb_key_cache() { + let name = String::from("test_nssdb_key_cache"); + let datadir = "testdata/nssdbdir"; + let destdir = format!("{}/{}", TESTDIR, name); + + let dbargs = format!("configDir={}", destdir); + let dbtype = "nssdb"; + + /* allocates a unique slotid to use in the tests */ + let mut testtokn = + TestToken::new_type(String::from(dbtype), String::from(""), name); + + /* Do this after TestToken::new() otherwise the data + * is wiped away by the initialization code */ + std::fs::create_dir_all(destdir.clone()).unwrap(); + assert!(std::fs::copy( + format!("{}/cert9.db", datadir), + format!("{}/cert9.db", destdir), + ) + .is_ok()); + assert!(std::fs::copy( + format!("{}/key4.db", datadir), + format!("{}/key4.db", destdir), + ) + .is_ok()); + assert!(std::fs::copy( + format!("{}/pkcs11.txt", datadir), + format!("{}/pkcs11.txt", destdir), + ) + .is_ok()); + + /* pre-populate conf so we get the correct slot number assigned */ + let mut slot = config::Slot::with_db(dbtype, Some(dbargs.clone())); + slot.slot = u32::try_from(testtokn.get_slot()).unwrap(); + let ret = add_slot(slot); + assert_eq!(ret, CKR_OK); + + let mut args = TestToken::make_init_args(Some(dbargs.clone())); + let args_ptr = &mut args as *mut CK_C_INITIALIZE_ARGS; + let ret = fn_initialize(args_ptr as *mut std::ffi::c_void); + assert_eq!(ret, CKR_OK); + + let _ = testtokn.get_session(false); + for _ in 0..1000 { + testtokn.login(); + testtokn.logout(); + } + + testtokn.finalize(); +}