From ff24073c4a6539d80b356c5717c7a7575126cfcb Mon Sep 17 00:00:00 2001 From: alt3r 3go Date: Mon, 1 Nov 2021 16:53:54 +0100 Subject: [PATCH 01/10] WIP: RSA lib integration - part 1 (very first steps, incomplete) Signed-off-by: alt3r 3go --- Cargo.toml | 8 ++ src/config.rs | 11 +- src/key.rs | 7 ++ src/mechanisms.rs | 3 + src/mechanisms/rsa2k.rs | 239 ++++++++++++++++++++++++++++++++++++++++ src/types.rs | 4 + 6 files changed, 271 insertions(+), 1 deletion(-) create mode 100644 src/mechanisms/rsa2k.rs diff --git a/Cargo.toml b/Cargo.toml index 25befb1095b..dc2cee99810 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -36,6 +36,8 @@ hmac = "0.11" sha-1 = { version = "0.9", default-features = false, optional = true } sha2 = { version = "0.9", default-features = false } +rsa = { version = "0.5.0", optional = true } + # ours cosey = "0.3" delog = "0.1.0" @@ -90,6 +92,9 @@ default-mechanisms = [ "tdes", "totp", "trng", + "rsa2k", + "rsa3k", + "rsa4k" ] aes256-cbc = [] chacha8-poly1305 = [] @@ -104,6 +109,9 @@ sha256 = [] tdes = ["des"] totp = ["sha-1"] trng = ["sha-1"] +rsa2k = ["rsa"] +rsa3k = ["rsa"] +rsa4k = ["rsa"] clients-1 = [] clients-2 = [] diff --git a/src/config.rs b/src/config.rs index 29e452aa654..05e8ea0d801 100644 --- a/src/config.rs +++ b/src/config.rs @@ -13,7 +13,7 @@ pub type MAX_OBJECT_HANDLES = consts::U16; pub type MAX_LABEL_LENGTH = consts::U256; pub const MAX_MEDIUM_DATA_LENGTH: usize = 256; pub type MAX_PATH_LENGTH = consts::U256; -pub const MAX_KEY_MATERIAL_LENGTH: usize = 128; +//pub const MAX_KEY_MATERIAL_LENGTH: usize = 128; // must be above + 4 pub const MAX_SERIALIZED_KEY_LENGTH: usize = 132; cfg_if::cfg_if! { @@ -44,8 +44,17 @@ cfg_if::cfg_if! { } } pub const MAX_SHORT_DATA_LENGTH: usize = 128; + +//TODO: Do we want better granularity here? +#[cfg(any(feature = "rsa2k", feature = "rsa3k", feature = "rsa4k"))] +pub const MAX_SIGNATURE_LENGTH: usize = 512; +pub const MAX_KEY_MATERIAL_LENGTH: usize = 2*512; //TODO: Assuming (e/d, N) for now +#[cfg(not(any(feature = "rsa2k", feature = "rsa3k", feature = "rsa4k")))] pub const MAX_SIGNATURE_LENGTH: usize = 72; +pub const MAX_KEY_MATERIAL_LENGTH: usize = 128; + pub const MAX_USER_ATTRIBUTE_LENGTH: usize = 256; + pub const USER_ATTRIBUTE_NUMBER: u8 = 37; diff --git a/src/key.rs b/src/key.rs index ce74889773d..cb6e1fef7de 100644 --- a/src/key.rs +++ b/src/key.rs @@ -63,6 +63,7 @@ pub enum Kind { Ed255, P256, X255, + Rsa2k, } bitflags::bitflags! { @@ -136,6 +137,9 @@ impl Kind { Kind::Ed255 => 4, Kind::P256 => 5, Kind::X255 => 6, + Kind::Rsa2k => 0x7, + // Kind::Rsa3k => 0xE0, + // Kind::Rsa4k => 0xE1, } } @@ -147,6 +151,9 @@ impl Kind { 4 => Self::Ed255, 5 => Self::P256, 6 => Self::X255, + + 0x7 => Self::Rsa2k, + _ => return Err(Error::InvalidSerializedKey), }) } diff --git a/src/mechanisms.rs b/src/mechanisms.rs index f55392a4260..d2fce289996 100644 --- a/src/mechanisms.rs +++ b/src/mechanisms.rs @@ -46,6 +46,9 @@ pub struct P256 {} pub struct P256Prehashed {} mod p256; +pub struct Rsa2k {} +mod rsa2k; + pub struct Sha256 {} mod sha256; diff --git a/src/mechanisms/rsa2k.rs b/src/mechanisms/rsa2k.rs new file mode 100644 index 00000000000..8754c5205b5 --- /dev/null +++ b/src/mechanisms/rsa2k.rs @@ -0,0 +1,239 @@ +use core::convert::{TryFrom, TryInto}; + +use crate::api::*; +// use crate::config::*; +// use crate::debug; +use crate::error::Error; +use crate::service::*; +use crate::types::*; + +#[inline(never)] +fn load_public_key(keystore: &mut impl Keystore, key_id: &KeyId) + -> Result { + + let public_bytes: [u8; 256] = keystore + .load_key(key::Secrecy::Public, Some(key::Kind::Rsa2k), &key_id)? + .material.as_slice() + .try_into() + .map_err(|_| Error::InternalError)?; + + let public_key = salty::signature::PublicKey::try_from(&public_bytes).map_err(|_| Error::InternalError)?; + + Ok(public_key) +} + + #[inline(never)] +fn load_keypair(keystore: &mut impl Keystore, key_id: &KeyId) + -> Result { + + let seed: [u8; 32] = keystore + .load_key(key::Secrecy::Secret, Some(key::Kind::Ed255), &key_id)? + .material.as_slice() + .try_into() + .map_err(|_| Error::InternalError)?; + + let keypair = salty::signature::Keypair::from(&seed); + // hprintln!("seed: {:?}", &seed).ok(); + Ok(keypair) +} + +#[cfg(feature = "ed255")] +impl DeriveKey for super::Ed255 +{ + #[inline(never)] + fn derive_key(keystore: &mut impl Keystore, request: &request::DeriveKey) + -> Result + { + let base_id = &request.base_key; + let keypair = load_keypair(keystore, base_id)?; + + let public_id = keystore.store_key( + request.attributes.persistence, + key::Secrecy::Public, key::Kind::Ed255, + keypair.public.as_bytes())?; + + Ok(reply::DeriveKey { + key: public_id, + }) + } +} + +#[cfg(feature = "ed255")] +impl DeserializeKey for super::Ed255 +{ + #[inline(never)] + fn deserialize_key(keystore: &mut impl Keystore, request: &request::DeserializeKey) + -> Result + { + // - mechanism: Mechanism + // - serialized_key: Message + // - attributes: StorageAttributes + + if request.format != KeySerialization::Raw { + return Err(Error::InternalError); + } + + if request.serialized_key.len() != 32 { + return Err(Error::InvalidSerializedKey); + } + + let serialized_key: [u8; 32] = request.serialized_key[..32].try_into().unwrap(); + let public_key = salty::signature::PublicKey::try_from(&serialized_key) + .map_err(|_| Error::InvalidSerializedKey)?; + + let public_id = keystore.store_key( + request.attributes.persistence, + key::Secrecy::Public, key::Kind::Ed255, + public_key.as_bytes())?; + + Ok(reply::DeserializeKey { + key: public_id + }) + } +} + +#[cfg(feature = "ed255")] +impl GenerateKey for super::Ed255 +{ + #[inline(never)] + fn generate_key(keystore: &mut impl Keystore, request: &request::GenerateKey) + -> Result + { + let mut seed = [0u8; 32]; + keystore.rng().fill_bytes(&mut seed); + + // let keypair = salty::signature::Keypair::from(&seed); + // #[cfg(all(test, feature = "verbose-tests"))] + // println!("ed255 keypair with public key = {:?}", &keypair.public); + + // store keys + let key_id = keystore.store_key( + request.attributes.persistence, + key::Secrecy::Secret, + key::Info::from(key::Kind::Ed255).with_local_flag(), + &seed)?; + + // return handle + Ok(reply::GenerateKey { key: key_id }) + } +} + +#[cfg(feature = "ed255")] +impl SerializeKey for super::Ed255 +{ + #[inline(never)] + fn serialize_key(keystore: &mut impl Keystore, request: &request::SerializeKey) + -> Result + { + let key_id = request.key; + let public_key = load_public_key(keystore, &key_id)?; + + let serialized_key = match request.format { + KeySerialization::Cose => { + let cose_pk = cosey::Ed25519PublicKey { + // x: Bytes::from_slice(public_key.x_coordinate()).unwrap(), + // x: Bytes::from_slice(&buf).unwrap(), + x: Bytes::from_slice(public_key.as_bytes()).unwrap(), + }; + crate::cbor_serialize_bytes(&cose_pk).map_err(|_| Error::CborError)? + } + + KeySerialization::Raw => { + let mut serialized_key = Message::new(); + serialized_key.extend_from_slice(public_key.as_bytes()).map_err(|_| Error::InternalError)?; + // serialized_key.extend_from_slice(&buf).map_err(|_| Error::InternalError)?; + serialized_key + } + + _ => { return Err(Error::InternalError); } + }; + + Ok(reply::SerializeKey { serialized_key }) + } +} + +#[cfg(feature = "ed255")] +impl Exists for super::Ed255 +{ + #[inline(never)] + fn exists(keystore: &mut impl Keystore, request: &request::Exists) + -> Result + { + let key_id = request.key; + + let exists = keystore.exists_key(key::Secrecy::Secret, Some(key::Kind::Ed255), &key_id); + Ok(reply::Exists { exists }) + } +} + +#[cfg(feature = "ed255")] +impl Sign for super::Ed255 +{ + #[inline(never)] + fn sign(keystore: &mut impl Keystore, request: &request::Sign) + -> Result + { + // Not so nice, expands to + // `trussed::/home/nicolas/projects/solo-bee/components/trussed/src/mechanisms/ed255.rs:151 + // Ed255::Sign`, i.e. VEERY long + // debug!("trussed::{}:{} Ed255::Sign", file!(), line!()).ok(); + // debug!("trussed: Ed255::Sign").ok(); + // if let SignatureSerialization::Raw = request.format { + // } else { + // return Err(Error::InvalidSerializationFormat); + // } + + let key_id = request.key; + + let keypair = load_keypair(keystore, &key_id)?; + + let native_signature = keypair.sign(&request.message); + let our_signature = Signature::from_slice(&native_signature.to_bytes()).unwrap(); + + // hprintln!("Ed255 signature:").ok(); + // hprintln!("msg: {:?}", &request.message).ok(); + // hprintln!("pk: {:?}", &keypair.public.as_bytes()).ok(); + // hprintln!("sig: {:?}", &our_signature).ok(); + + // return signature + Ok(reply::Sign { signature: our_signature }) + } +} + +#[cfg(feature = "ed255")] +impl Verify for super::Ed255 +{ + #[inline(never)] + fn verify(keystore: &mut impl Keystore, request: &request::Verify) + -> Result + { + if let SignatureSerialization::Raw = request.format { + } else { + return Err(Error::InvalidSerializationFormat); + } + + if request.signature.len() != salty::constants::SIGNATURE_SERIALIZED_LENGTH { + return Err(Error::WrongSignatureLength); + } + + let key_id = request.key; + let public_key = load_public_key(keystore, &key_id)?; + + let mut signature_array = [0u8; salty::constants::SIGNATURE_SERIALIZED_LENGTH]; + signature_array.copy_from_slice(request.signature.as_ref()); + let salty_signature = salty::signature::Signature::from(&signature_array); + + Ok(reply::Verify { valid: + public_key.verify(&request.message, &salty_signature).is_ok() + }) + } +} + +#[cfg(not(feature = "ed255"))] +impl DeriveKey for super::Ed255 {} +#[cfg(not(feature = "ed255"))] +impl GenerateKey for super::Ed255 {} +#[cfg(not(feature = "ed255"))] +impl Sign for super::Ed255 {} +#[cfg(not(feature = "ed255"))] +impl Verify for super::Ed255 {} diff --git a/src/types.rs b/src/types.rs index 965a048d90e..23eea6c684a 100644 --- a/src/types.rs +++ b/src/types.rs @@ -508,6 +508,10 @@ pub enum Mechanism { Totp, Trng, X255, + //TODO: Do we want to distinguish PKCS_v1.5 vs PSS/OAEP right here? + Rsa2k, + Rsa3k, + Rsa4k, } pub type LongData = Bytes; From 20df3c7f205b787862a6e881c15225bfc1ee6678 Mon Sep 17 00:00:00 2001 From: alt3r 3go Date: Sun, 21 Nov 2021 18:38:22 +0100 Subject: [PATCH 02/10] WIP: RSA lib integration - part 2 (more plumbing, still incomplete) Signed-off-by: alt3r 3go --- Cargo.toml | 5 +- src/config.rs | 13 ++-- src/key.rs | 6 +- src/mechanisms.rs | 4 +- src/mechanisms/{rsa2k.rs => rsa2kpkcs.rs} | 77 +++++++++++------------ src/service.rs | 7 +++ src/store/keystore.rs | 4 ++ src/types.rs | 3 +- 8 files changed, 64 insertions(+), 55 deletions(-) rename src/mechanisms/{rsa2k.rs => rsa2kpkcs.rs} (77%) diff --git a/Cargo.toml b/Cargo.toml index dc2cee99810..57445b52123 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -35,7 +35,6 @@ des = { version = "0.7", optional = true } hmac = "0.11" sha-1 = { version = "0.9", default-features = false, optional = true } sha2 = { version = "0.9", default-features = false } - rsa = { version = "0.5.0", optional = true } # ours @@ -92,7 +91,7 @@ default-mechanisms = [ "tdes", "totp", "trng", - "rsa2k", + "rsa2k-pkcs", "rsa3k", "rsa4k" ] @@ -109,7 +108,7 @@ sha256 = [] tdes = ["des"] totp = ["sha-1"] trng = ["sha-1"] -rsa2k = ["rsa"] +rsa2k-pkcs = ["rsa"] rsa3k = ["rsa"] rsa4k = ["rsa"] diff --git a/src/config.rs b/src/config.rs index 05e8ea0d801..fb6ec020649 100644 --- a/src/config.rs +++ b/src/config.rs @@ -45,14 +45,19 @@ cfg_if::cfg_if! { } pub const MAX_SHORT_DATA_LENGTH: usize = 128; -//TODO: Do we want better granularity here? -#[cfg(any(feature = "rsa2k", feature = "rsa3k", feature = "rsa4k"))] +//TODO: Do we want better keylength granularity here? +#[cfg(any(feature = "rsa2k-pkcs", feature = "rsa3k", feature = "rsa4k"))] pub const MAX_SIGNATURE_LENGTH: usize = 512; -pub const MAX_KEY_MATERIAL_LENGTH: usize = 2*512; //TODO: Assuming (e/d, N) for now -#[cfg(not(any(feature = "rsa2k", feature = "rsa3k", feature = "rsa4k")))] +#[cfg(any(feature = "rsa2k-pkcs", feature = "rsa3k", feature = "rsa4k"))] +pub const MAX_KEY_MATERIAL_LENGTH: usize = 2*512; //TODO: Assuming plain (, N) for now + +#[cfg(not(any(feature = "rsa2k-pkcs", feature = "rsa3k", feature = "rsa4k")))] pub const MAX_SIGNATURE_LENGTH: usize = 72; +#[cfg(not(any(feature = "rsa2k-pkcs", feature = "rsa3k", feature = "rsa4k")))] pub const MAX_KEY_MATERIAL_LENGTH: usize = 128; +// must be MAX_KEY_MATERIAL_LENGTH + 4 +pub const MAX_SERIALIZED_KEY_LENGTH: usize = MAX_KEY_MATERIAL_LENGTH + 4; pub const MAX_USER_ATTRIBUTE_LENGTH: usize = 256; diff --git a/src/key.rs b/src/key.rs index cb6e1fef7de..1d94adaf618 100644 --- a/src/key.rs +++ b/src/key.rs @@ -138,8 +138,8 @@ impl Kind { Kind::P256 => 5, Kind::X255 => 6, Kind::Rsa2k => 0x7, - // Kind::Rsa3k => 0xE0, - // Kind::Rsa4k => 0xE1, + //Kind::Rsa3k => 0xE0, + //Kind::Rsa4k => 0xE1, } } @@ -153,6 +153,8 @@ impl Kind { 6 => Self::X255, 0x7 => Self::Rsa2k, + //0xE0 => Kind::Rsa3k, + //0xE1 => Kind::Rsa4k, _ => return Err(Error::InvalidSerializedKey), }) diff --git a/src/mechanisms.rs b/src/mechanisms.rs index d2fce289996..2461811d928 100644 --- a/src/mechanisms.rs +++ b/src/mechanisms.rs @@ -46,8 +46,8 @@ pub struct P256 {} pub struct P256Prehashed {} mod p256; -pub struct Rsa2k {} -mod rsa2k; +pub struct Rsa2kPkcs {} +mod rsa2kpkcs; pub struct Sha256 {} mod sha256; diff --git a/src/mechanisms/rsa2k.rs b/src/mechanisms/rsa2kpkcs.rs similarity index 77% rename from src/mechanisms/rsa2k.rs rename to src/mechanisms/rsa2kpkcs.rs index 8754c5205b5..95c56115b7d 100644 --- a/src/mechanisms/rsa2k.rs +++ b/src/mechanisms/rsa2kpkcs.rs @@ -9,25 +9,27 @@ use crate::types::*; #[inline(never)] fn load_public_key(keystore: &mut impl Keystore, key_id: &KeyId) - -> Result { + -> Result { - let public_bytes: [u8; 256] = keystore + //TODO: The key size should better be defined somewhere instead of hardcoding + let public_bytes: [u8; 512] = keystore .load_key(key::Secrecy::Public, Some(key::Kind::Rsa2k), &key_id)? .material.as_slice() .try_into() .map_err(|_| Error::InternalError)?; - let public_key = salty::signature::PublicKey::try_from(&public_bytes).map_err(|_| Error::InternalError)?; + // let public_key = salty::signature::PublicKey::try_from(&public_bytes).map_err(|_| Error::InternalError)?; + let public_key = rsa::PublicKey::try_from(&public_bytes).map_err(|_| Error::InternalError)?; Ok(public_key) } - #[inline(never)] +#[inline(never)] fn load_keypair(keystore: &mut impl Keystore, key_id: &KeyId) -> Result { let seed: [u8; 32] = keystore - .load_key(key::Secrecy::Secret, Some(key::Kind::Ed255), &key_id)? + .load_key(key::Secrecy::Secret, Some(key::Kind::Rsa2k), &key_id)? .material.as_slice() .try_into() .map_err(|_| Error::InternalError)?; @@ -37,8 +39,8 @@ fn load_keypair(keystore: &mut impl Keystore, key_id: &KeyId) Ok(keypair) } -#[cfg(feature = "ed255")] -impl DeriveKey for super::Ed255 +#[cfg(feature = "rsa2k-pkcs")] +impl DeriveKey for super::Rsa2kPkcs { #[inline(never)] fn derive_key(keystore: &mut impl Keystore, request: &request::DeriveKey) @@ -49,7 +51,7 @@ impl DeriveKey for super::Ed255 let public_id = keystore.store_key( request.attributes.persistence, - key::Secrecy::Public, key::Kind::Ed255, + key::Secrecy::Public, key::Kind::Rsa2k, keypair.public.as_bytes())?; Ok(reply::DeriveKey { @@ -58,8 +60,8 @@ impl DeriveKey for super::Ed255 } } -#[cfg(feature = "ed255")] -impl DeserializeKey for super::Ed255 +#[cfg(feature = "rsa2k-pkcs")] +impl DeserializeKey for super::Rsa2kPkcs { #[inline(never)] fn deserialize_key(keystore: &mut impl Keystore, request: &request::DeserializeKey) @@ -83,7 +85,7 @@ impl DeserializeKey for super::Ed255 let public_id = keystore.store_key( request.attributes.persistence, - key::Secrecy::Public, key::Kind::Ed255, + key::Secrecy::Public, key::Kind::Rsa2k, public_key.as_bytes())?; Ok(reply::DeserializeKey { @@ -92,8 +94,8 @@ impl DeserializeKey for super::Ed255 } } -#[cfg(feature = "ed255")] -impl GenerateKey for super::Ed255 +#[cfg(feature = "rsa2k-pkcs")] +impl GenerateKey for super::Rsa2kPkcs { #[inline(never)] fn generate_key(keystore: &mut impl Keystore, request: &request::GenerateKey) @@ -104,13 +106,13 @@ impl GenerateKey for super::Ed255 // let keypair = salty::signature::Keypair::from(&seed); // #[cfg(all(test, feature = "verbose-tests"))] - // println!("ed255 keypair with public key = {:?}", &keypair.public); + // println!("rsa2k-pkcs keypair with public key = {:?}", &keypair.public); // store keys let key_id = keystore.store_key( request.attributes.persistence, key::Secrecy::Secret, - key::Info::from(key::Kind::Ed255).with_local_flag(), + key::Info::from(key::Kind::Rsa2k).with_local_flag(), &seed)?; // return handle @@ -118,8 +120,8 @@ impl GenerateKey for super::Ed255 } } -#[cfg(feature = "ed255")] -impl SerializeKey for super::Ed255 +#[cfg(feature = "rsa2k-pkcs")] +impl SerializeKey for super::Rsa2kPkcs { #[inline(never)] fn serialize_key(keystore: &mut impl Keystore, request: &request::SerializeKey) @@ -152,8 +154,8 @@ impl SerializeKey for super::Ed255 } } -#[cfg(feature = "ed255")] -impl Exists for super::Ed255 +#[cfg(feature = "rsa2k-pkcs")] +impl Exists for super::Rsa2kPkcs { #[inline(never)] fn exists(keystore: &mut impl Keystore, request: &request::Exists) @@ -161,27 +163,18 @@ impl Exists for super::Ed255 { let key_id = request.key; - let exists = keystore.exists_key(key::Secrecy::Secret, Some(key::Kind::Ed255), &key_id); + let exists = keystore.exists_key(key::Secrecy::Secret, Some(key::Kind::Rsa2k), &key_id); Ok(reply::Exists { exists }) } } -#[cfg(feature = "ed255")] -impl Sign for super::Ed255 +#[cfg(feature = "rsa2k-pkcs")] +impl Sign for super::Rsa2kPkcs { #[inline(never)] fn sign(keystore: &mut impl Keystore, request: &request::Sign) -> Result { - // Not so nice, expands to - // `trussed::/home/nicolas/projects/solo-bee/components/trussed/src/mechanisms/ed255.rs:151 - // Ed255::Sign`, i.e. VEERY long - // debug!("trussed::{}:{} Ed255::Sign", file!(), line!()).ok(); - // debug!("trussed: Ed255::Sign").ok(); - // if let SignatureSerialization::Raw = request.format { - // } else { - // return Err(Error::InvalidSerializationFormat); - // } let key_id = request.key; @@ -190,7 +183,7 @@ impl Sign for super::Ed255 let native_signature = keypair.sign(&request.message); let our_signature = Signature::from_slice(&native_signature.to_bytes()).unwrap(); - // hprintln!("Ed255 signature:").ok(); + // hprintln!("RSA2K-PKCS_v1.5 signature:").ok(); // hprintln!("msg: {:?}", &request.message).ok(); // hprintln!("pk: {:?}", &keypair.public.as_bytes()).ok(); // hprintln!("sig: {:?}", &our_signature).ok(); @@ -200,8 +193,8 @@ impl Sign for super::Ed255 } } -#[cfg(feature = "ed255")] -impl Verify for super::Ed255 +#[cfg(feature = "rsa2k-pkcs")] +impl Verify for super::Rsa2kPkcs { #[inline(never)] fn verify(keystore: &mut impl Keystore, request: &request::Verify) @@ -229,11 +222,11 @@ impl Verify for super::Ed255 } } -#[cfg(not(feature = "ed255"))] -impl DeriveKey for super::Ed255 {} -#[cfg(not(feature = "ed255"))] -impl GenerateKey for super::Ed255 {} -#[cfg(not(feature = "ed255"))] -impl Sign for super::Ed255 {} -#[cfg(not(feature = "ed255"))] -impl Verify for super::Ed255 {} +#[cfg(not(feature = "rsa2k-pkcs"))] +impl DeriveKey for super::Rsa2kPkcs {} +#[cfg(not(feature = "rsa2k-pkcs"))] +impl GenerateKey for super::Rsa2kPkcs {} +#[cfg(not(feature = "rsa2k-pkcs"))] +impl Sign for super::Rsa2kPkcs {} +#[cfg(not(feature = "rsa2k-pkcs"))] +impl Verify for super::Rsa2kPkcs {} diff --git a/src/service.rs b/src/service.rs index 21a6d0c0721..80fd554078c 100644 --- a/src/service.rs +++ b/src/service.rs @@ -173,6 +173,7 @@ impl ServiceResources

{ Mechanism::P256 => mechanisms::P256::derive_key(keystore, request), Mechanism::Sha256 => mechanisms::Sha256::derive_key(keystore, request), Mechanism::X255 => mechanisms::X255::derive_key(keystore, request), + Mechanism::Rsa2kPkcs => mechanisms::Rsa2kPkcs::derive_key(keystore, request), _ => Err(Error::MechanismNotAvailable), }.map(Reply::DeriveKey) @@ -184,6 +185,7 @@ impl ServiceResources

{ Mechanism::Ed255 => mechanisms::Ed255::deserialize_key(keystore, request), Mechanism::P256 => mechanisms::P256::deserialize_key(keystore, request), Mechanism::X255 => mechanisms::X255::deserialize_key(keystore, request), + Mechanism::Rsa2kPkcs => mechanisms::Rsa2kPkcs::deserialize_key(keystore, request), _ => Err(Error::MechanismNotAvailable), }.map(Reply::DeserializeKey) @@ -217,6 +219,7 @@ impl ServiceResources

{ Mechanism::P256 => mechanisms::P256::exists(keystore, request), Mechanism::Totp => mechanisms::Totp::exists(keystore, request), Mechanism::X255 => mechanisms::X255::exists(keystore, request), + Mechanism::Rsa2kPkcs => mechanisms::Rsa2kPkcs::exists(keystore, request), _ => Err(Error::MechanismNotAvailable), }.map(Reply::Exists) @@ -228,6 +231,7 @@ impl ServiceResources

{ Mechanism::Ed255 => mechanisms::Ed255::generate_key(keystore, request), Mechanism::P256 => mechanisms::P256::generate_key(keystore, request), Mechanism::X255 => mechanisms::X255::generate_key(keystore, request), + Mechanism::Rsa2kPkcs => mechanisms::Rsa2kPkcs::generate_key(keystore, request), _ => Err(Error::MechanismNotAvailable), }.map(Reply::GenerateKey) }, @@ -431,6 +435,7 @@ impl ServiceResources

{ Mechanism::Ed255 => mechanisms::Ed255::serialize_key(keystore, request), Mechanism::P256 => mechanisms::P256::serialize_key(keystore, request), Mechanism::X255 => mechanisms::X255::serialize_key(keystore, request), + Mechanism::Rsa2kPkcs => mechanisms::Rsa2kPkcs::serialize_key(keystore, request), _ => Err(Error::MechanismNotAvailable), }.map(Reply::SerializeKey) @@ -447,6 +452,7 @@ impl ServiceResources

{ Mechanism::P256 => mechanisms::P256::sign(keystore, request), Mechanism::P256Prehashed => mechanisms::P256Prehashed::sign(keystore, request), Mechanism::Totp => mechanisms::Totp::sign(keystore, request), + Mechanism::Rsa2kPkcs => mechanisms::Rsa2kPkcs::sign(keystore, request), _ => Err(Error::MechanismNotAvailable), }.map(Reply::Sign) @@ -471,6 +477,7 @@ impl ServiceResources

{ Mechanism::Ed255 => mechanisms::Ed255::verify(keystore, request), Mechanism::P256 => mechanisms::P256::verify(keystore, request), + Mechanism::Rsa2kPkcs => mechanisms::Rsa2kPkcs::verify(keystore, request), _ => Err(Error::MechanismNotAvailable), }.map(Reply::Verify) diff --git a/src/store/keystore.rs b/src/store/keystore.rs index abec9a88423..d26e55ed8c8 100644 --- a/src/store/keystore.rs +++ b/src/store/keystore.rs @@ -146,7 +146,11 @@ impl Keystore for ClientKeystore

{ let location = self.location(secrecy, id).ok_or(Error::NoSuchKey)?; + //TODO: This should better be defined in some way, instead of hardcoding + #[cfg(not(feature = "rsa2k-pkcs"))] let bytes: Bytes<128> = store::read(self.store, location, &path)?; + #[cfg(feature = "rsa2k-pkcs")] + let bytes: Bytes<512> = store::read(self.store, location, &path)?; let key = key::Key::try_deserialize(&bytes)?; diff --git a/src/types.rs b/src/types.rs index 23eea6c684a..4d28e71821b 100644 --- a/src/types.rs +++ b/src/types.rs @@ -508,8 +508,7 @@ pub enum Mechanism { Totp, Trng, X255, - //TODO: Do we want to distinguish PKCS_v1.5 vs PSS/OAEP right here? - Rsa2k, + Rsa2kPkcs, Rsa3k, Rsa4k, } From b01039eb3d469ee5f904d14b6d86054d6105e83a Mon Sep 17 00:00:00 2001 From: alt3r 3go Date: Sun, 28 Nov 2021 15:18:15 +0100 Subject: [PATCH 03/10] WIP: RSA lib integration - part 3 (RSA2K GenerateKey+test) Signed-off-by: alt3r 3go --- src/client/mechanisms.rs | 11 ++ src/config.rs | 2 +- src/mechanisms/rsa2kpkcs.rs | 234 ++++++++++++++++++------------------ tests/rsa2kpkcs.rs | 20 +++ 4 files changed, 151 insertions(+), 116 deletions(-) create mode 100644 tests/rsa2kpkcs.rs diff --git a/src/client/mechanisms.rs b/src/client/mechanisms.rs index 630391286a1..140febd2d4b 100644 --- a/src/client/mechanisms.rs +++ b/src/client/mechanisms.rs @@ -245,6 +245,17 @@ pub trait P256: CryptoClient { } } +#[cfg(feature = "rsa2k-pkcs")] +impl Rsa2kPkcs for ClientImplementation {} + +pub trait Rsa2kPkcs: CryptoClient { + fn generate_rsa2kpkcs_private_key(&mut self, persistence: Location) + -> ClientResult<'_, reply::GenerateKey, Self> + { + self.generate_key(Mechanism::Rsa2kPkcs, StorageAttributes::new().set_persistence(persistence)) + } +} + #[cfg(feature = "sha256")] impl Sha256 for ClientImplementation {} diff --git a/src/config.rs b/src/config.rs index fb6ec020649..02afced013b 100644 --- a/src/config.rs +++ b/src/config.rs @@ -49,7 +49,7 @@ pub const MAX_SHORT_DATA_LENGTH: usize = 128; #[cfg(any(feature = "rsa2k-pkcs", feature = "rsa3k", feature = "rsa4k"))] pub const MAX_SIGNATURE_LENGTH: usize = 512; #[cfg(any(feature = "rsa2k-pkcs", feature = "rsa3k", feature = "rsa4k"))] -pub const MAX_KEY_MATERIAL_LENGTH: usize = 2*512; //TODO: Assuming plain (, N) for now +pub const MAX_KEY_MATERIAL_LENGTH: usize = 2*609; //TODO: We use PKCS#8 DER format, this value found empirically for 2K keys. Need to generalize. #[cfg(not(any(feature = "rsa2k-pkcs", feature = "rsa3k", feature = "rsa4k")))] pub const MAX_SIGNATURE_LENGTH: usize = 72; diff --git a/src/mechanisms/rsa2kpkcs.rs b/src/mechanisms/rsa2kpkcs.rs index 95c56115b7d..68e2977b8b0 100644 --- a/src/mechanisms/rsa2kpkcs.rs +++ b/src/mechanisms/rsa2kpkcs.rs @@ -1,5 +1,7 @@ use core::convert::{TryFrom, TryInto}; +use rsa::{RsaPrivateKey, pkcs8::ToPrivateKey}; + use crate::api::*; // use crate::config::*; // use crate::debug; @@ -7,57 +9,57 @@ use crate::error::Error; use crate::service::*; use crate::types::*; -#[inline(never)] -fn load_public_key(keystore: &mut impl Keystore, key_id: &KeyId) - -> Result { +// #[inline(never)] +// fn load_public_key(keystore: &mut impl Keystore, key_id: &KeyId) +// -> Result { - //TODO: The key size should better be defined somewhere instead of hardcoding - let public_bytes: [u8; 512] = keystore - .load_key(key::Secrecy::Public, Some(key::Kind::Rsa2k), &key_id)? - .material.as_slice() - .try_into() - .map_err(|_| Error::InternalError)?; +// //TODO: The key size should better be defined somewhere instead of hardcoding +// let public_bytes: [u8; 512] = keystore +// .load_key(key::Secrecy::Public, Some(key::Kind::Rsa2k), &key_id)? +// .material.as_slice() +// .try_into() +// .map_err(|_| Error::InternalError)?; - // let public_key = salty::signature::PublicKey::try_from(&public_bytes).map_err(|_| Error::InternalError)?; - let public_key = rsa::PublicKey::try_from(&public_bytes).map_err(|_| Error::InternalError)?; +// // let public_key = salty::signature::PublicKey::try_from(&public_bytes).map_err(|_| Error::InternalError)?; +// let public_key = rsa::RsaPublicKey::try_from(&public_bytes).map_err(|_| Error::InternalError)?; - Ok(public_key) -} +// Ok(public_key) +// } -#[inline(never)] -fn load_keypair(keystore: &mut impl Keystore, key_id: &KeyId) - -> Result { +// #[inline(never)] +// fn load_keypair(keystore: &mut impl Keystore, key_id: &KeyId) +// -> Result { - let seed: [u8; 32] = keystore - .load_key(key::Secrecy::Secret, Some(key::Kind::Rsa2k), &key_id)? - .material.as_slice() - .try_into() - .map_err(|_| Error::InternalError)?; +// let seed: [u8; 32] = keystore +// .load_key(key::Secrecy::Secret, Some(key::Kind::Rsa2k), &key_id)? +// .material.as_slice() +// .try_into() +// .map_err(|_| Error::InternalError)?; - let keypair = salty::signature::Keypair::from(&seed); - // hprintln!("seed: {:?}", &seed).ok(); - Ok(keypair) -} +// let keypair = salty::signature::Keypair::from(&seed); +// // hprintln!("seed: {:?}", &seed).ok(); +// Ok(keypair) +// } #[cfg(feature = "rsa2k-pkcs")] impl DeriveKey for super::Rsa2kPkcs { - #[inline(never)] - fn derive_key(keystore: &mut impl Keystore, request: &request::DeriveKey) - -> Result - { - let base_id = &request.base_key; - let keypair = load_keypair(keystore, base_id)?; - - let public_id = keystore.store_key( - request.attributes.persistence, - key::Secrecy::Public, key::Kind::Rsa2k, - keypair.public.as_bytes())?; - - Ok(reply::DeriveKey { - key: public_id, - }) - } + // #[inline(never)] + // fn derive_key(keystore: &mut impl Keystore, request: &request::DeriveKey) + // -> Result + // { + // let base_id = &request.base_key; + // let keypair = load_keypair(keystore, base_id)?; + + // let public_id = keystore.store_key( + // request.attributes.persistence, + // key::Secrecy::Public, key::Kind::Rsa2k, + // keypair.public.as_bytes())?; + + // Ok(reply::DeriveKey { + // key: public_id, + // }) + // } } #[cfg(feature = "rsa2k-pkcs")] @@ -101,19 +103,21 @@ impl GenerateKey for super::Rsa2kPkcs fn generate_key(keystore: &mut impl Keystore, request: &request::GenerateKey) -> Result { - let mut seed = [0u8; 32]; - keystore.rng().fill_bytes(&mut seed); + // We want an RSA 2K key + let bits = 2048; + + let private_key = RsaPrivateKey::new(keystore.rng(), bits).expect("Failed to generate the RSA 2K private key"); + let pk_der = ToPrivateKey::to_pkcs8_der(&private_key).expect("Failed to serialize the RSA 2K private key to PKCS#8 DER"); - // let keypair = salty::signature::Keypair::from(&seed); // #[cfg(all(test, feature = "verbose-tests"))] - // println!("rsa2k-pkcs keypair with public key = {:?}", &keypair.public); + // std::println!("rsa2k-pkcs private key = {:?}", &private_key); - // store keys + // store the key let key_id = keystore.store_key( request.attributes.persistence, key::Secrecy::Secret, key::Info::from(key::Kind::Rsa2k).with_local_flag(), - &seed)?; + pk_der.as_ref())?; // return handle Ok(reply::GenerateKey { key: key_id }) @@ -123,35 +127,35 @@ impl GenerateKey for super::Rsa2kPkcs #[cfg(feature = "rsa2k-pkcs")] impl SerializeKey for super::Rsa2kPkcs { - #[inline(never)] - fn serialize_key(keystore: &mut impl Keystore, request: &request::SerializeKey) - -> Result - { - let key_id = request.key; - let public_key = load_public_key(keystore, &key_id)?; - - let serialized_key = match request.format { - KeySerialization::Cose => { - let cose_pk = cosey::Ed25519PublicKey { - // x: Bytes::from_slice(public_key.x_coordinate()).unwrap(), - // x: Bytes::from_slice(&buf).unwrap(), - x: Bytes::from_slice(public_key.as_bytes()).unwrap(), - }; - crate::cbor_serialize_bytes(&cose_pk).map_err(|_| Error::CborError)? - } - - KeySerialization::Raw => { - let mut serialized_key = Message::new(); - serialized_key.extend_from_slice(public_key.as_bytes()).map_err(|_| Error::InternalError)?; - // serialized_key.extend_from_slice(&buf).map_err(|_| Error::InternalError)?; - serialized_key - } - - _ => { return Err(Error::InternalError); } - }; - - Ok(reply::SerializeKey { serialized_key }) - } + // #[inline(never)] + // fn serialize_key(keystore: &mut impl Keystore, request: &request::SerializeKey) + // -> Result + // { + // let key_id = request.key; + // let public_key = load_public_key(keystore, &key_id)?; + + // let serialized_key = match request.format { + // KeySerialization::Cose => { + // let cose_pk = cosey::Ed25519PublicKey { + // // x: Bytes::from_slice(public_key.x_coordinate()).unwrap(), + // // x: Bytes::from_slice(&buf).unwrap(), + // x: Bytes::from_slice(public_key.as_bytes()).unwrap(), + // }; + // crate::cbor_serialize_bytes(&cose_pk).map_err(|_| Error::CborError)? + // } + + // KeySerialization::Raw => { + // let mut serialized_key = Message::new(); + // serialized_key.extend_from_slice(public_key.as_bytes()).map_err(|_| Error::InternalError)?; + // // serialized_key.extend_from_slice(&buf).map_err(|_| Error::InternalError)?; + // serialized_key + // } + + // _ => { return Err(Error::InternalError); } + // }; + + // Ok(reply::SerializeKey { serialized_key }) + // } } #[cfg(feature = "rsa2k-pkcs")] @@ -171,55 +175,55 @@ impl Exists for super::Rsa2kPkcs #[cfg(feature = "rsa2k-pkcs")] impl Sign for super::Rsa2kPkcs { - #[inline(never)] - fn sign(keystore: &mut impl Keystore, request: &request::Sign) - -> Result - { + // #[inline(never)] + // fn sign(keystore: &mut impl Keystore, request: &request::Sign) + // -> Result + // { - let key_id = request.key; + // let key_id = request.key; - let keypair = load_keypair(keystore, &key_id)?; + // let keypair = load_keypair(keystore, &key_id)?; - let native_signature = keypair.sign(&request.message); - let our_signature = Signature::from_slice(&native_signature.to_bytes()).unwrap(); + // let native_signature = keypair.sign(&request.message); + // let our_signature = Signature::from_slice(&native_signature.to_bytes()).unwrap(); - // hprintln!("RSA2K-PKCS_v1.5 signature:").ok(); - // hprintln!("msg: {:?}", &request.message).ok(); - // hprintln!("pk: {:?}", &keypair.public.as_bytes()).ok(); - // hprintln!("sig: {:?}", &our_signature).ok(); + // // hprintln!("RSA2K-PKCS_v1.5 signature:").ok(); + // // hprintln!("msg: {:?}", &request.message).ok(); + // // hprintln!("pk: {:?}", &keypair.public.as_bytes()).ok(); + // // hprintln!("sig: {:?}", &our_signature).ok(); - // return signature - Ok(reply::Sign { signature: our_signature }) - } + // // return signature + // Ok(reply::Sign { signature: our_signature }) + // } } #[cfg(feature = "rsa2k-pkcs")] impl Verify for super::Rsa2kPkcs { - #[inline(never)] - fn verify(keystore: &mut impl Keystore, request: &request::Verify) - -> Result - { - if let SignatureSerialization::Raw = request.format { - } else { - return Err(Error::InvalidSerializationFormat); - } - - if request.signature.len() != salty::constants::SIGNATURE_SERIALIZED_LENGTH { - return Err(Error::WrongSignatureLength); - } - - let key_id = request.key; - let public_key = load_public_key(keystore, &key_id)?; - - let mut signature_array = [0u8; salty::constants::SIGNATURE_SERIALIZED_LENGTH]; - signature_array.copy_from_slice(request.signature.as_ref()); - let salty_signature = salty::signature::Signature::from(&signature_array); - - Ok(reply::Verify { valid: - public_key.verify(&request.message, &salty_signature).is_ok() - }) - } + // #[inline(never)] + // fn verify(keystore: &mut impl Keystore, request: &request::Verify) + // -> Result + // { + // if let SignatureSerialization::Raw = request.format { + // } else { + // return Err(Error::InvalidSerializationFormat); + // } + + // if request.signature.len() != salty::constants::SIGNATURE_SERIALIZED_LENGTH { + // return Err(Error::WrongSignatureLength); + // } + + // let key_id = request.key; + // let public_key = load_public_key(keystore, &key_id)?; + + // let mut signature_array = [0u8; salty::constants::SIGNATURE_SERIALIZED_LENGTH]; + // signature_array.copy_from_slice(request.signature.as_ref()); + // let salty_signature = salty::signature::Signature::from(&signature_array); + + // Ok(reply::Verify { valid: + // public_key.verify(&request.message, &salty_signature).is_ok() + // }) + // } } #[cfg(not(feature = "rsa2k-pkcs"))] diff --git a/tests/rsa2kpkcs.rs b/tests/rsa2kpkcs.rs new file mode 100644 index 00000000000..99f0a123fdc --- /dev/null +++ b/tests/rsa2kpkcs.rs @@ -0,0 +1,20 @@ +use generic_array::typenum::assert_type; +use trussed::client::mechanisms::Rsa2kPkcs; +use trussed::syscall; +use trussed::types::KeyId; + +mod client; + +use trussed::types::Location::*; + + +#[test] +fn rsa2kpkcs_generate_key() { + client::get(|client| { + let sk = syscall!(client.generate_rsa2kpkcs_private_key(Internal)).key; + + // This assumes we don't really get they key with ID 0 + // TODO: make sure the above always holds + assert_ne!(sk, KeyId::from_special(0)); + }) +} From edce7ab7ab474ff4bae7cd772aa1258f83bf9024 Mon Sep 17 00:00:00 2001 From: alt3r 3go Date: Sun, 12 Dec 2021 15:20:01 +0100 Subject: [PATCH 04/10] WIP: RSA lib integration - part 4 (DeriveKey+test) Signed-off-by: alt3r 3go --- src/client/mechanisms.rs | 6 ++++ src/config.rs | 3 +- src/mechanisms/rsa2kpkcs.rs | 71 ++++++++++++++++++++++++++----------- src/store/keystore.rs | 6 ++-- tests/rsa2kpkcs.rs | 20 +++++++++-- 5 files changed, 80 insertions(+), 26 deletions(-) diff --git a/src/client/mechanisms.rs b/src/client/mechanisms.rs index 140febd2d4b..865f5f21963 100644 --- a/src/client/mechanisms.rs +++ b/src/client/mechanisms.rs @@ -254,6 +254,12 @@ pub trait Rsa2kPkcs: CryptoClient { { self.generate_key(Mechanism::Rsa2kPkcs, StorageAttributes::new().set_persistence(persistence)) } + + fn derive_rsa2kpkcs_public_key(&mut self, shared_key: KeyId, persistence: Location) + -> ClientResult<'_, reply::DeriveKey, Self> + { + self.derive_key(Mechanism::Rsa2kPkcs, shared_key, None, StorageAttributes::new().set_persistence(persistence)) + } } #[cfg(feature = "sha256")] diff --git a/src/config.rs b/src/config.rs index 02afced013b..5d712361958 100644 --- a/src/config.rs +++ b/src/config.rs @@ -49,7 +49,8 @@ pub const MAX_SHORT_DATA_LENGTH: usize = 128; #[cfg(any(feature = "rsa2k-pkcs", feature = "rsa3k", feature = "rsa4k"))] pub const MAX_SIGNATURE_LENGTH: usize = 512; #[cfg(any(feature = "rsa2k-pkcs", feature = "rsa3k", feature = "rsa4k"))] -pub const MAX_KEY_MATERIAL_LENGTH: usize = 2*609; //TODO: We use PKCS#8 DER format, this value found empirically for 2K keys. Need to generalize. +//TODO: We use PKCS#8 DER format, this value was found empirically for 2K keys. Need to generalize. +pub const MAX_KEY_MATERIAL_LENGTH: usize = 1217; #[cfg(not(any(feature = "rsa2k-pkcs", feature = "rsa3k", feature = "rsa4k")))] pub const MAX_SIGNATURE_LENGTH: usize = 72; diff --git a/src/mechanisms/rsa2kpkcs.rs b/src/mechanisms/rsa2kpkcs.rs index 68e2977b8b0..4bca7ba8ec4 100644 --- a/src/mechanisms/rsa2kpkcs.rs +++ b/src/mechanisms/rsa2kpkcs.rs @@ -1,6 +1,10 @@ use core::convert::{TryFrom, TryInto}; -use rsa::{RsaPrivateKey, pkcs8::ToPrivateKey}; +use rsa::{ + RsaPrivateKey, + RsaPublicKey, + pkcs8::{FromPrivateKey, ToPrivateKey, ToPublicKey}, +}; use crate::api::*; // use crate::config::*; @@ -44,22 +48,40 @@ use crate::types::*; #[cfg(feature = "rsa2k-pkcs")] impl DeriveKey for super::Rsa2kPkcs { - // #[inline(never)] - // fn derive_key(keystore: &mut impl Keystore, request: &request::DeriveKey) - // -> Result - // { - // let base_id = &request.base_key; - // let keypair = load_keypair(keystore, base_id)?; + #[inline(never)] + fn derive_key(keystore: &mut impl Keystore, request: &request::DeriveKey) + -> Result + { + // Retrieve private key + let base_key_id = &request.base_key; - // let public_id = keystore.store_key( - // request.attributes.persistence, - // key::Secrecy::Public, key::Kind::Rsa2k, - // keypair.public.as_bytes())?; + // std::println!("Loading key: {:?}", base_key_id); - // Ok(reply::DeriveKey { - // key: public_id, - // }) - // } + let priv_key_der = keystore.load_key(key::Secrecy::Secret, Some(key::Kind::Rsa2k), base_key_id) + .expect("Failed to load an RSA 2K private key with the given ID") + .material; + + // std::println!("Loaded key material: {}", delog::hex_str!(&priv_key_der)); + // std::println!("Key material length is {}", priv_key_der.len()); + + let priv_key = FromPrivateKey::from_pkcs8_der(&priv_key_der) + .expect("Failed to deserialize an RSA 2K private key from PKCS#8 DER"); + + // Derive and store public key + let pub_key_der = RsaPublicKey::from(&priv_key) + .to_public_key_der() + .expect("Failed to derive an RSA 2K public key or to serialize it to PKCS#8 DER"); + + let pub_key_id = keystore.store_key( + request.attributes.persistence, + key::Secrecy::Public, key::Kind::Rsa2k, + pub_key_der.as_ref())?; + + // Send a reply + Ok(reply::DeriveKey { + key: pub_key_id, + }) + } } #[cfg(feature = "rsa2k-pkcs")] @@ -106,21 +128,30 @@ impl GenerateKey for super::Rsa2kPkcs // We want an RSA 2K key let bits = 2048; - let private_key = RsaPrivateKey::new(keystore.rng(), bits).expect("Failed to generate the RSA 2K private key"); - let pk_der = ToPrivateKey::to_pkcs8_der(&private_key).expect("Failed to serialize the RSA 2K private key to PKCS#8 DER"); + let priv_key = RsaPrivateKey::new(keystore.rng(), bits) + .expect("Failed to generate an RSA 2K private key"); + + // std::println!("Stored key material before DER: {:#?}", priv_key); + let priv_key_der = priv_key.to_pkcs8_der() + .expect("Failed to serialize an RSA 2K private key to PKCS#8 DER"); + + // std::println!("Stored key material after DER: {}", delog::hex_str!(&priv_key_der)); + // std::println!("Key material length is {}", priv_key_der.as_ref().len()); // #[cfg(all(test, feature = "verbose-tests"))] // std::println!("rsa2k-pkcs private key = {:?}", &private_key); // store the key - let key_id = keystore.store_key( + let priv_key_id = keystore.store_key( request.attributes.persistence, key::Secrecy::Secret, key::Info::from(key::Kind::Rsa2k).with_local_flag(), - pk_der.as_ref())?; + priv_key_der.as_ref())?; // return handle - Ok(reply::GenerateKey { key: key_id }) + Ok(reply::GenerateKey { + key: priv_key_id, + }) } } diff --git a/src/store/keystore.rs b/src/store/keystore.rs index d26e55ed8c8..84cc7a8ee7a 100644 --- a/src/store/keystore.rs +++ b/src/store/keystore.rs @@ -8,6 +8,7 @@ use crate::{ Platform, store::{self, Store as _}, types::{KeyId, Location}, + config::MAX_SERIALIZED_KEY_LENGTH, }; @@ -146,11 +147,12 @@ impl Keystore for ClientKeystore

{ let location = self.location(secrecy, id).ok_or(Error::NoSuchKey)?; - //TODO: This should better be defined in some way, instead of hardcoding + //TODO: This should better be defined in some way, instead of hardcoding. + // I've tried referring to MAX_SERIALIZED_KEY_LENGTH, is this a good idea? #[cfg(not(feature = "rsa2k-pkcs"))] let bytes: Bytes<128> = store::read(self.store, location, &path)?; #[cfg(feature = "rsa2k-pkcs")] - let bytes: Bytes<512> = store::read(self.store, location, &path)?; + let bytes: Bytes = store::read(self.store, location, &path)?; let key = key::Key::try_deserialize(&bytes)?; diff --git a/tests/rsa2kpkcs.rs b/tests/rsa2kpkcs.rs index 99f0a123fdc..5f3f8add8c4 100644 --- a/tests/rsa2kpkcs.rs +++ b/tests/rsa2kpkcs.rs @@ -1,4 +1,3 @@ -use generic_array::typenum::assert_type; use trussed::client::mechanisms::Rsa2kPkcs; use trussed::syscall; use trussed::types::KeyId; @@ -7,14 +6,29 @@ mod client; use trussed::types::Location::*; +// TODO: Looks like the test infra is not supposed to be used with several tests like below - +// right now it randomly fails with SIGSERV on either of the two, when run together, +// but never when run separately. Need to investigate and fix. #[test] fn rsa2kpkcs_generate_key() { client::get(|client| { let sk = syscall!(client.generate_rsa2kpkcs_private_key(Internal)).key; - // This assumes we don't really get they key with ID 0 - // TODO: make sure the above always holds + // This assumes we don't ever get a key with ID 0 + // TODO: make sure the above always holds or find a better way to check for success assert_ne!(sk, KeyId::from_special(0)); }) } + +#[test] +fn rsa2kpkcs_derive_key() { + client::get(|client| { + let sk = syscall!(client.generate_rsa2kpkcs_private_key(Internal)).key; + let pk = syscall!(client.derive_rsa2kpkcs_public_key(sk, Volatile)).key; + + // This assumes we don't ever get a key with ID 0 + // TODO: make sure the above always holds or find a better way to check for success + assert_ne!(pk, KeyId::from_special(0)); + }) +} \ No newline at end of file From 0bdeb28cc0b6756407cccfee8f0e06a34b1ff54c Mon Sep 17 00:00:00 2001 From: alt3r 3go Date: Sun, 19 Dec 2021 20:03:54 +0100 Subject: [PATCH 05/10] WIP: RSA lib integration - part 5 (Exists/{De}SerializeKey+tests) Signed-off-by: alt3r 3go --- src/client/mechanisms.rs | 12 +++++++ src/config.rs | 12 +++++-- src/mechanisms/rsa2kpkcs.rs | 71 +++++++++++++++++-------------------- tests/rsa2kpkcs.rs | 41 ++++++++++++++++++++- 4 files changed, 94 insertions(+), 42 deletions(-) diff --git a/src/client/mechanisms.rs b/src/client/mechanisms.rs index 865f5f21963..d60285a197c 100644 --- a/src/client/mechanisms.rs +++ b/src/client/mechanisms.rs @@ -260,6 +260,18 @@ pub trait Rsa2kPkcs: CryptoClient { { self.derive_key(Mechanism::Rsa2kPkcs, shared_key, None, StorageAttributes::new().set_persistence(persistence)) } + + fn serialize_rsa2kpkcs_key(&mut self, key: KeyId, format: KeySerialization) + -> ClientResult<'_, reply::SerializeKey, Self> + { + self.serialize_key(Mechanism::Rsa2kPkcs, key, format) + } + + fn deserialize_rsa2kpkcs_key<'c>(&'c mut self, serialized_key: &[u8], format: KeySerialization, attributes: StorageAttributes) + -> ClientResult<'c, reply::DeserializeKey, Self> + { + self.deserialize_key(Mechanism::Rsa2kPkcs, serialized_key, format, attributes) + } } #[cfg(feature = "sha256")] diff --git a/src/config.rs b/src/config.rs index 5d712361958..17fab04fff8 100644 --- a/src/config.rs +++ b/src/config.rs @@ -8,7 +8,6 @@ use littlefs2::consts; pub type MAX_APPLICATION_NAME_LENGTH = consts::U256; pub const MAX_LONG_DATA_LENGTH: usize = 1024; -pub const MAX_MESSAGE_LENGTH: usize = 1024; pub type MAX_OBJECT_HANDLES = consts::U16; pub type MAX_LABEL_LENGTH = consts::U256; pub const MAX_MEDIUM_DATA_LENGTH: usize = 256; @@ -45,17 +44,24 @@ cfg_if::cfg_if! { } pub const MAX_SHORT_DATA_LENGTH: usize = 128; -//TODO: Do we want better keylength granularity here? +// TODO: Do we want better keylength granularity here? #[cfg(any(feature = "rsa2k-pkcs", feature = "rsa3k", feature = "rsa4k"))] pub const MAX_SIGNATURE_LENGTH: usize = 512; #[cfg(any(feature = "rsa2k-pkcs", feature = "rsa3k", feature = "rsa4k"))] -//TODO: We use PKCS#8 DER format, this value was found empirically for 2K keys. Need to generalize. +// TODO: We use PKCS#8 DER format, this value was found empirically for 2K keys. Need to generalize. pub const MAX_KEY_MATERIAL_LENGTH: usize = 1217; +#[cfg(any(feature = "rsa2k-pkcs", feature = "rsa3k", feature = "rsa4k"))] +// TODO: This is due to the fact that KEY_MATERIAL_LENGTH is now bigger than MESSAGE_LENGTH. +// Double-check this is okay. +pub const MAX_MESSAGE_LENGTH: usize = MAX_KEY_MATERIAL_LENGTH; + #[cfg(not(any(feature = "rsa2k-pkcs", feature = "rsa3k", feature = "rsa4k")))] pub const MAX_SIGNATURE_LENGTH: usize = 72; #[cfg(not(any(feature = "rsa2k-pkcs", feature = "rsa3k", feature = "rsa4k")))] pub const MAX_KEY_MATERIAL_LENGTH: usize = 128; +#[cfg(not(any(feature = "rsa2k-pkcs", feature = "rsa3k", feature = "rsa4k")))] +pub const MAX_MESSAGE_LENGTH: usize = 1024; // must be MAX_KEY_MATERIAL_LENGTH + 4 pub const MAX_SERIALIZED_KEY_LENGTH: usize = MAX_KEY_MATERIAL_LENGTH + 4; diff --git a/src/mechanisms/rsa2kpkcs.rs b/src/mechanisms/rsa2kpkcs.rs index 4bca7ba8ec4..8175b5caed9 100644 --- a/src/mechanisms/rsa2kpkcs.rs +++ b/src/mechanisms/rsa2kpkcs.rs @@ -99,21 +99,20 @@ impl DeserializeKey for super::Rsa2kPkcs return Err(Error::InternalError); } - if request.serialized_key.len() != 32 { - return Err(Error::InvalidSerializedKey); - } - - let serialized_key: [u8; 32] = request.serialized_key[..32].try_into().unwrap(); - let public_key = salty::signature::PublicKey::try_from(&serialized_key) + let private_key: RsaPrivateKey = FromPrivateKey::from_pkcs8_der(&request.serialized_key) .map_err(|_| Error::InvalidSerializedKey)?; - let public_id = keystore.store_key( + // We store our keys in PKCS#8 DER format as well + let private_key_der = private_key.to_pkcs8_der() + .expect("Failed to serialize an RSA 2K private key to PKCS#8 DER"); + + let private_key_id = keystore.store_key( request.attributes.persistence, - key::Secrecy::Public, key::Kind::Rsa2k, - public_key.as_bytes())?; + key::Secrecy::Secret, key::Kind::Rsa2k, + private_key_der.as_ref())?; Ok(reply::DeserializeKey { - key: public_id + key: private_key_id, }) } } @@ -158,35 +157,31 @@ impl GenerateKey for super::Rsa2kPkcs #[cfg(feature = "rsa2k-pkcs")] impl SerializeKey for super::Rsa2kPkcs { - // #[inline(never)] - // fn serialize_key(keystore: &mut impl Keystore, request: &request::SerializeKey) - // -> Result - // { - // let key_id = request.key; - // let public_key = load_public_key(keystore, &key_id)?; + #[inline(never)] + fn serialize_key(keystore: &mut impl Keystore, request: &request::SerializeKey) + -> Result + { + let key_id = request.key; - // let serialized_key = match request.format { - // KeySerialization::Cose => { - // let cose_pk = cosey::Ed25519PublicKey { - // // x: Bytes::from_slice(public_key.x_coordinate()).unwrap(), - // // x: Bytes::from_slice(&buf).unwrap(), - // x: Bytes::from_slice(public_key.as_bytes()).unwrap(), - // }; - // crate::cbor_serialize_bytes(&cose_pk).map_err(|_| Error::CborError)? - // } - - // KeySerialization::Raw => { - // let mut serialized_key = Message::new(); - // serialized_key.extend_from_slice(public_key.as_bytes()).map_err(|_| Error::InternalError)?; - // // serialized_key.extend_from_slice(&buf).map_err(|_| Error::InternalError)?; - // serialized_key - // } - - // _ => { return Err(Error::InternalError); } - // }; - - // Ok(reply::SerializeKey { serialized_key }) - // } + // We rely on the fact that we store the keys in the PKCS#8 DER format already + let priv_key_der = keystore.load_key(key::Secrecy::Secret, Some(key::Kind::Rsa2k), &key_id) + .expect("Failed to load an RSA 2K private key with the given ID") + .material; + + let serialized_key = match request.format { + // TODO: There are "Der" and "Asn1Der" commented out in KeySerialization enum, + // should those be used instead? + KeySerialization::Raw => { + let mut serialized_key = Message::new(); + serialized_key.extend_from_slice(&priv_key_der).map_err(|_| Error::InternalError)?; + serialized_key + } + + _ => { return Err(Error::InternalError); } + }; + + Ok(reply::SerializeKey { serialized_key }) + } } #[cfg(feature = "rsa2k-pkcs")] diff --git a/tests/rsa2kpkcs.rs b/tests/rsa2kpkcs.rs index 5f3f8add8c4..98b233f9887 100644 --- a/tests/rsa2kpkcs.rs +++ b/tests/rsa2kpkcs.rs @@ -1,10 +1,13 @@ +use trussed::client::CryptoClient; use trussed::client::mechanisms::Rsa2kPkcs; use trussed::syscall; use trussed::types::KeyId; mod client; +use trussed::types::KeySerialization; use trussed::types::Location::*; +use trussed::types::StorageAttributes; // TODO: Looks like the test infra is not supposed to be used with several tests like below - // right now it randomly fails with SIGSERV on either of the two, when run together, @@ -31,4 +34,40 @@ fn rsa2kpkcs_derive_key() { // TODO: make sure the above always holds or find a better way to check for success assert_ne!(pk, KeyId::from_special(0)); }) -} \ No newline at end of file +} + +#[test] +fn rsa2kpkcs_exists_key() { + client::get(|client| { + let sk = syscall!(client.generate_rsa2kpkcs_private_key(Internal)).key; + let key_exists = syscall!(client.exists(trussed::types::Mechanism::Rsa2kPkcs, sk)).exists; + + assert!(key_exists); + }) +} + +#[test] +fn rsa2kpkcs_serialize_key() { + client::get(|client| { + let sk = syscall!(client.generate_rsa2kpkcs_private_key(Internal)).key; + + let serialized_key = syscall!(client.serialize_rsa2kpkcs_key(sk, KeySerialization::Raw)).serialized_key; + + assert!(!serialized_key.is_empty()); + }) +} + +#[test] +fn rsa2kpkcs_deserialize_key() { + client::get(|client| { + let sk = syscall!(client.generate_rsa2kpkcs_private_key(Internal)).key; + let serialized_key = syscall!(client.serialize_rsa2kpkcs_key(sk, KeySerialization::Raw)).serialized_key; + let location = StorageAttributes { persistence: Volatile }; + + let deserialized_key_id = syscall!(client.deserialize_rsa2kpkcs_key(&serialized_key, KeySerialization::Raw, location)).key; + + // This assumes we don't ever get a key with ID 0 + // TODO: make sure the above always holds or find a better way to check for success + assert_ne!(deserialized_key_id, KeyId::from_special(0)); + }) +} From 497bf9d8dc7fd3adcf7b07a822a002b92e9b279e Mon Sep 17 00:00:00 2001 From: alt3r 3go Date: Sun, 23 Jan 2022 19:04:10 +0100 Subject: [PATCH 06/10] WIP: RSA lib integration - part 6 (Sign/Verify+tests) Signed-off-by: alt3r 3go --- src/client/mechanisms.rs | 12 ++++ src/mechanisms/rsa2kpkcs.rs | 114 ++++++++++++++++++++++-------------- tests/rsa2kpkcs.rs | 20 +++++++ 3 files changed, 102 insertions(+), 44 deletions(-) diff --git a/src/client/mechanisms.rs b/src/client/mechanisms.rs index d60285a197c..06baa60a7b8 100644 --- a/src/client/mechanisms.rs +++ b/src/client/mechanisms.rs @@ -272,6 +272,18 @@ pub trait Rsa2kPkcs: CryptoClient { { self.deserialize_key(Mechanism::Rsa2kPkcs, serialized_key, format, attributes) } + + fn sign_rsa2kpkcs<'c>(&'c mut self, key: KeyId, message: &[u8]) + -> ClientResult<'c, reply::Sign, Self> + { + self.sign(Mechanism::Rsa2kPkcs, key, message, SignatureSerialization::Raw) + } + + fn verify_rsa2kpkcs<'c>(&'c mut self, key: KeyId, message: &[u8], signature: &[u8]) + -> ClientResult<'c, reply::Verify, Self> + { + self.verify(Mechanism::Rsa2kPkcs, key, message, signature, SignatureSerialization::Raw) + } } #[cfg(feature = "sha256")] diff --git a/src/mechanisms/rsa2kpkcs.rs b/src/mechanisms/rsa2kpkcs.rs index 8175b5caed9..d1d216c13c6 100644 --- a/src/mechanisms/rsa2kpkcs.rs +++ b/src/mechanisms/rsa2kpkcs.rs @@ -1,9 +1,8 @@ -use core::convert::{TryFrom, TryInto}; - use rsa::{ RsaPrivateKey, RsaPublicKey, - pkcs8::{FromPrivateKey, ToPrivateKey, ToPublicKey}, + PublicKey, + pkcs8::{FromPrivateKey, ToPrivateKey, ToPublicKey} }; use crate::api::*; @@ -201,55 +200,82 @@ impl Exists for super::Rsa2kPkcs #[cfg(feature = "rsa2k-pkcs")] impl Sign for super::Rsa2kPkcs { - // #[inline(never)] - // fn sign(keystore: &mut impl Keystore, request: &request::Sign) - // -> Result - // { - - // let key_id = request.key; - - // let keypair = load_keypair(keystore, &key_id)?; + #[inline(never)] + fn sign(keystore: &mut impl Keystore, request: &request::Sign) + -> Result + { + // First, get the key + let key_id = request.key; - // let native_signature = keypair.sign(&request.message); - // let our_signature = Signature::from_slice(&native_signature.to_bytes()).unwrap(); + // We rely on the fact that we store the keys in the PKCS#8 DER format already + let priv_key_der = keystore.load_key(key::Secrecy::Secret, Some(key::Kind::Rsa2k), &key_id) + .expect("Failed to load an RSA 2K private key with the given ID") + .material; - // // hprintln!("RSA2K-PKCS_v1.5 signature:").ok(); - // // hprintln!("msg: {:?}", &request.message).ok(); - // // hprintln!("pk: {:?}", &keypair.public.as_bytes()).ok(); - // // hprintln!("sig: {:?}", &our_signature).ok(); + let priv_key: RsaPrivateKey = FromPrivateKey::from_pkcs8_der(&priv_key_der) + .expect("Failed to deserialize an RSA 2K private key from PKCS#8 DER"); - // // return signature - // Ok(reply::Sign { signature: our_signature }) - // } + // RSA lib takes in a hash value to sign, not raw data. + // TODO: Do we assume we get digest into this function, or we calculate it ourselves? + // use sha2::digest::Digest; + // let digest_to_sign: [u8; 32] = sha2::Sha256::digest(&request.message).into(); + + // TODO: there's also .sign_blinded(), which is supposed to protect the private key from timing side channels, + // but requires an RNG instance - decide if we want to always use it. + use rsa::padding::PaddingScheme; + use rsa::hash::Hash; + let native_signature = priv_key + .sign(PaddingScheme::new_pkcs1v15_sign(Some(Hash::SHA2_256)), &request.message) + .unwrap(); + let our_signature = Signature::from_slice(&native_signature).unwrap(); + + // std::println!("RSA2K-PKCS_v1.5 signature:"); + // std::println!("msg: {:?}", &request.message); + // std::println!("pk: {:?}", &priv_key); + // std::println!("sig: {:?}", &our_signature); + + // return signature + Ok(reply::Sign { signature: our_signature }) + } } #[cfg(feature = "rsa2k-pkcs")] impl Verify for super::Rsa2kPkcs { - // #[inline(never)] - // fn verify(keystore: &mut impl Keystore, request: &request::Verify) - // -> Result - // { - // if let SignatureSerialization::Raw = request.format { - // } else { - // return Err(Error::InvalidSerializationFormat); - // } - - // if request.signature.len() != salty::constants::SIGNATURE_SERIALIZED_LENGTH { - // return Err(Error::WrongSignatureLength); - // } - - // let key_id = request.key; - // let public_key = load_public_key(keystore, &key_id)?; - - // let mut signature_array = [0u8; salty::constants::SIGNATURE_SERIALIZED_LENGTH]; - // signature_array.copy_from_slice(request.signature.as_ref()); - // let salty_signature = salty::signature::Signature::from(&signature_array); - - // Ok(reply::Verify { valid: - // public_key.verify(&request.message, &salty_signature).is_ok() - // }) - // } + #[inline(never)] + fn verify(keystore: &mut impl Keystore, request: &request::Verify) + -> Result + { + if let SignatureSerialization::Raw = request.format { + } else { + return Err(Error::InvalidSerializationFormat); + } + + // TODO: This must not be a hardcoded magic number, need to generalize + if request.signature.len() != 256 { + return Err(Error::WrongSignatureLength); + } + + let key_id = request.key; + + let priv_key_der = keystore.load_key(key::Secrecy::Secret, Some(key::Kind::Rsa2k), &key_id) + .expect("Failed to load an RSA 2K private key with the given ID") + .material; + + let priv_key = FromPrivateKey::from_pkcs8_der(&priv_key_der) + .expect("Failed to deserialize an RSA 2K private key from PKCS#8 DER"); + + // Get the public key + let pub_key = RsaPublicKey::from(&priv_key); + + use rsa::padding::PaddingScheme; + use rsa::hash::Hash; + let verification_ok = pub_key + .verify(PaddingScheme::new_pkcs1v15_sign(Some(Hash::SHA2_256)), &request.message, &request.signature) + .is_ok(); + + Ok(reply::Verify { valid: verification_ok }) + } } #[cfg(not(feature = "rsa2k-pkcs"))] diff --git a/tests/rsa2kpkcs.rs b/tests/rsa2kpkcs.rs index 98b233f9887..7640f1d66fc 100644 --- a/tests/rsa2kpkcs.rs +++ b/tests/rsa2kpkcs.rs @@ -71,3 +71,23 @@ fn rsa2kpkcs_deserialize_key() { assert_ne!(deserialized_key_id, KeyId::from_special(0)); }) } + +#[test] +fn rsa2kpkcs_sign_verify() { + client::get(|client| { + let sk = syscall!(client.generate_rsa2kpkcs_private_key(Volatile)).key; + + let message = [1u8, 2u8, 3u8]; + use sha2::digest::Digest; + let digest_to_sign = sha2::Sha256::digest(&message); + let signature = syscall!(client.sign_rsa2kpkcs(sk, &digest_to_sign)).signature; + + // println!("Message: {:?}", &message); + // println!("Digest: {:?}", &digest_to_sign); + // println!("Signature (len={}): {:?}", signature.len(), &signature); + + let verify_ok = syscall!(client.verify_rsa2kpkcs(sk, &digest_to_sign, &signature)).valid; + + assert!(signature.len() == 256 && verify_ok); + }) +} From b72b80e8724c226e4334c8eeb1176830a034cc91 Mon Sep 17 00:00:00 2001 From: alt3r 3go Date: Mon, 11 Jul 2022 12:59:30 +0200 Subject: [PATCH 07/10] WIP: remove unneeded code, mark my TODOs for simpler discovery Signed-off-by: alt3r 3go --- src/config.rs | 8 +++---- src/mechanisms/rsa2kpkcs.rs | 44 +++++-------------------------------- src/store/keystore.rs | 4 ++-- tests/rsa2kpkcs.rs | 12 +++++----- 4 files changed, 18 insertions(+), 50 deletions(-) diff --git a/src/config.rs b/src/config.rs index 17fab04fff8..950af8f047c 100644 --- a/src/config.rs +++ b/src/config.rs @@ -44,15 +44,15 @@ cfg_if::cfg_if! { } pub const MAX_SHORT_DATA_LENGTH: usize = 128; -// TODO: Do we want better keylength granularity here? +// TODO:alt3r-3go: Do we want better keylength granularity here? #[cfg(any(feature = "rsa2k-pkcs", feature = "rsa3k", feature = "rsa4k"))] pub const MAX_SIGNATURE_LENGTH: usize = 512; #[cfg(any(feature = "rsa2k-pkcs", feature = "rsa3k", feature = "rsa4k"))] -// TODO: We use PKCS#8 DER format, this value was found empirically for 2K keys. Need to generalize. +// TODO:alt3r-3go: We use PKCS#8 DER format, this value was found empirically for 2K keys. Need to generalize. pub const MAX_KEY_MATERIAL_LENGTH: usize = 1217; #[cfg(any(feature = "rsa2k-pkcs", feature = "rsa3k", feature = "rsa4k"))] -// TODO: This is due to the fact that KEY_MATERIAL_LENGTH is now bigger than MESSAGE_LENGTH. -// Double-check this is okay. +// TODO:alt3r-3go: This is due to the fact that KEY_MATERIAL_LENGTH is now bigger than MESSAGE_LENGTH. +// Double-check this is okay. pub const MAX_MESSAGE_LENGTH: usize = MAX_KEY_MATERIAL_LENGTH; diff --git a/src/mechanisms/rsa2kpkcs.rs b/src/mechanisms/rsa2kpkcs.rs index d1d216c13c6..8ee765382c9 100644 --- a/src/mechanisms/rsa2kpkcs.rs +++ b/src/mechanisms/rsa2kpkcs.rs @@ -12,38 +12,6 @@ use crate::error::Error; use crate::service::*; use crate::types::*; -// #[inline(never)] -// fn load_public_key(keystore: &mut impl Keystore, key_id: &KeyId) -// -> Result { - -// //TODO: The key size should better be defined somewhere instead of hardcoding -// let public_bytes: [u8; 512] = keystore -// .load_key(key::Secrecy::Public, Some(key::Kind::Rsa2k), &key_id)? -// .material.as_slice() -// .try_into() -// .map_err(|_| Error::InternalError)?; - -// // let public_key = salty::signature::PublicKey::try_from(&public_bytes).map_err(|_| Error::InternalError)?; -// let public_key = rsa::RsaPublicKey::try_from(&public_bytes).map_err(|_| Error::InternalError)?; - -// Ok(public_key) -// } - -// #[inline(never)] -// fn load_keypair(keystore: &mut impl Keystore, key_id: &KeyId) -// -> Result { - -// let seed: [u8; 32] = keystore -// .load_key(key::Secrecy::Secret, Some(key::Kind::Rsa2k), &key_id)? -// .material.as_slice() -// .try_into() -// .map_err(|_| Error::InternalError)?; - -// let keypair = salty::signature::Keypair::from(&seed); -// // hprintln!("seed: {:?}", &seed).ok(); -// Ok(keypair) -// } - #[cfg(feature = "rsa2k-pkcs")] impl DeriveKey for super::Rsa2kPkcs { @@ -168,8 +136,8 @@ impl SerializeKey for super::Rsa2kPkcs .material; let serialized_key = match request.format { - // TODO: There are "Der" and "Asn1Der" commented out in KeySerialization enum, - // should those be used instead? + // TODO:alt3r-3go: There are "Der" and "Asn1Der" commented out in KeySerialization enum, + // should those be used instead? KeySerialization::Raw => { let mut serialized_key = Message::new(); serialized_key.extend_from_slice(&priv_key_der).map_err(|_| Error::InternalError)?; @@ -216,12 +184,12 @@ impl Sign for super::Rsa2kPkcs .expect("Failed to deserialize an RSA 2K private key from PKCS#8 DER"); // RSA lib takes in a hash value to sign, not raw data. - // TODO: Do we assume we get digest into this function, or we calculate it ourselves? + // TODO:alt3r-3go: Do we assume we get digest into this function, or we calculate it ourselves? // use sha2::digest::Digest; // let digest_to_sign: [u8; 32] = sha2::Sha256::digest(&request.message).into(); - // TODO: there's also .sign_blinded(), which is supposed to protect the private key from timing side channels, - // but requires an RNG instance - decide if we want to always use it. + // TODO:alt3r-3go: There's also .sign_blinded(), which is supposed to protect the private key from timing side channels, + // but requires an RNG instance - decide if we want to always use it. use rsa::padding::PaddingScheme; use rsa::hash::Hash; let native_signature = priv_key @@ -251,7 +219,7 @@ impl Verify for super::Rsa2kPkcs return Err(Error::InvalidSerializationFormat); } - // TODO: This must not be a hardcoded magic number, need to generalize + // TODO:alt3r-3go: This must not be a hardcoded magic number, need to generalize if request.signature.len() != 256 { return Err(Error::WrongSignatureLength); } diff --git a/src/store/keystore.rs b/src/store/keystore.rs index 84cc7a8ee7a..020baa6465e 100644 --- a/src/store/keystore.rs +++ b/src/store/keystore.rs @@ -147,8 +147,8 @@ impl Keystore for ClientKeystore

{ let location = self.location(secrecy, id).ok_or(Error::NoSuchKey)?; - //TODO: This should better be defined in some way, instead of hardcoding. - // I've tried referring to MAX_SERIALIZED_KEY_LENGTH, is this a good idea? + //TODO:alt3r-3go: This should better be defined in some way, instead of hardcoding. + // I've tried referring to MAX_SERIALIZED_KEY_LENGTH, is this a good idea? #[cfg(not(feature = "rsa2k-pkcs"))] let bytes: Bytes<128> = store::read(self.store, location, &path)?; #[cfg(feature = "rsa2k-pkcs")] diff --git a/tests/rsa2kpkcs.rs b/tests/rsa2kpkcs.rs index 7640f1d66fc..e7b96fc7d21 100644 --- a/tests/rsa2kpkcs.rs +++ b/tests/rsa2kpkcs.rs @@ -9,9 +9,9 @@ use trussed::types::KeySerialization; use trussed::types::Location::*; use trussed::types::StorageAttributes; -// TODO: Looks like the test infra is not supposed to be used with several tests like below - -// right now it randomly fails with SIGSERV on either of the two, when run together, -// but never when run separately. Need to investigate and fix. +// TODO:alt3r-3go: Looks like the test infra is not supposed to be used with several tests like below - +// right now it randomly fails with SIGSERV on either of the two, when run together, +// but never when run separately. Need to investigate and fix. #[test] fn rsa2kpkcs_generate_key() { @@ -19,7 +19,7 @@ fn rsa2kpkcs_generate_key() { let sk = syscall!(client.generate_rsa2kpkcs_private_key(Internal)).key; // This assumes we don't ever get a key with ID 0 - // TODO: make sure the above always holds or find a better way to check for success + // TODO:alt3r-3go: make sure the above always holds or find a better way to check for success assert_ne!(sk, KeyId::from_special(0)); }) } @@ -31,7 +31,7 @@ fn rsa2kpkcs_derive_key() { let pk = syscall!(client.derive_rsa2kpkcs_public_key(sk, Volatile)).key; // This assumes we don't ever get a key with ID 0 - // TODO: make sure the above always holds or find a better way to check for success + // TODO:alt3r-3go: make sure the above always holds or find a better way to check for success assert_ne!(pk, KeyId::from_special(0)); }) } @@ -67,7 +67,7 @@ fn rsa2kpkcs_deserialize_key() { let deserialized_key_id = syscall!(client.deserialize_rsa2kpkcs_key(&serialized_key, KeySerialization::Raw, location)).key; // This assumes we don't ever get a key with ID 0 - // TODO: make sure the above always holds or find a better way to check for success + // TODO:alt3r-3go: make sure the above always holds or find a better way to check for success assert_ne!(deserialized_key_id, KeyId::from_special(0)); }) } From 7bc9a6913f47b9e8d9e5173135c85e4c08ceb177 Mon Sep 17 00:00:00 2001 From: alt3r 3go Date: Sun, 17 Jul 2022 13:30:03 +0200 Subject: [PATCH 08/10] WIP: Rebase to current trussed/main, cleanup Signed-off-by: alt3r 3go --- src/config.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/config.rs b/src/config.rs index 950af8f047c..a2bace35121 100644 --- a/src/config.rs +++ b/src/config.rs @@ -14,7 +14,7 @@ pub const MAX_MEDIUM_DATA_LENGTH: usize = 256; pub type MAX_PATH_LENGTH = consts::U256; //pub const MAX_KEY_MATERIAL_LENGTH: usize = 128; // must be above + 4 -pub const MAX_SERIALIZED_KEY_LENGTH: usize = 132; +//pub const MAX_SERIALIZED_KEY_LENGTH: usize = 132; cfg_if::cfg_if! { if #[cfg(feature = "clients-12")] { pub type MAX_SERVICE_CLIENTS = consts::U12; From c360dced0f1dba951d2a29ee489d5de9e899e7cd Mon Sep 17 00:00:00 2001 From: alt3r 3go Date: Sun, 17 Jul 2022 13:59:07 +0200 Subject: [PATCH 09/10] WIP: update RSA traits after updating to 0.6.0 Signed-off-by: alt3r 3go --- Cargo.toml | 2 +- src/mechanisms/rsa2kpkcs.rs | 14 +++++++++----- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 57445b52123..7db5cb76657 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -35,7 +35,7 @@ des = { version = "0.7", optional = true } hmac = "0.11" sha-1 = { version = "0.9", default-features = false, optional = true } sha2 = { version = "0.9", default-features = false } -rsa = { version = "0.5.0", optional = true } +rsa = { version = "0.6.0", optional = true } # ours cosey = "0.3" diff --git a/src/mechanisms/rsa2kpkcs.rs b/src/mechanisms/rsa2kpkcs.rs index 8ee765382c9..9c4ab6b6e47 100644 --- a/src/mechanisms/rsa2kpkcs.rs +++ b/src/mechanisms/rsa2kpkcs.rs @@ -2,7 +2,7 @@ use rsa::{ RsaPrivateKey, RsaPublicKey, PublicKey, - pkcs8::{FromPrivateKey, ToPrivateKey, ToPublicKey} + pkcs8::{EncodePrivateKey, DecodePrivateKey, EncodePublicKey} }; use crate::api::*; @@ -12,6 +12,10 @@ use crate::error::Error; use crate::service::*; use crate::types::*; +//TODO:alt3r-3go: sign() and verify() are the only two methods that are actually different between -pkcs and -pss. +// Moreover, the key::Kind::Rsa2K could also probably be parametrized, instead of having a dedicated kind +// for each. Overall this means the class structure can probably be simplified - need to decide. + #[cfg(feature = "rsa2k-pkcs")] impl DeriveKey for super::Rsa2kPkcs { @@ -31,7 +35,7 @@ impl DeriveKey for super::Rsa2kPkcs // std::println!("Loaded key material: {}", delog::hex_str!(&priv_key_der)); // std::println!("Key material length is {}", priv_key_der.len()); - let priv_key = FromPrivateKey::from_pkcs8_der(&priv_key_der) + let priv_key = DecodePrivateKey::from_pkcs8_der(&priv_key_der) .expect("Failed to deserialize an RSA 2K private key from PKCS#8 DER"); // Derive and store public key @@ -66,7 +70,7 @@ impl DeserializeKey for super::Rsa2kPkcs return Err(Error::InternalError); } - let private_key: RsaPrivateKey = FromPrivateKey::from_pkcs8_der(&request.serialized_key) + let private_key: RsaPrivateKey = DecodePrivateKey::from_pkcs8_der(&request.serialized_key) .map_err(|_| Error::InvalidSerializedKey)?; // We store our keys in PKCS#8 DER format as well @@ -180,7 +184,7 @@ impl Sign for super::Rsa2kPkcs .expect("Failed to load an RSA 2K private key with the given ID") .material; - let priv_key: RsaPrivateKey = FromPrivateKey::from_pkcs8_der(&priv_key_der) + let priv_key: RsaPrivateKey = DecodePrivateKey::from_pkcs8_der(&priv_key_der) .expect("Failed to deserialize an RSA 2K private key from PKCS#8 DER"); // RSA lib takes in a hash value to sign, not raw data. @@ -230,7 +234,7 @@ impl Verify for super::Rsa2kPkcs .expect("Failed to load an RSA 2K private key with the given ID") .material; - let priv_key = FromPrivateKey::from_pkcs8_der(&priv_key_der) + let priv_key = DecodePrivateKey::from_pkcs8_der(&priv_key_der) .expect("Failed to deserialize an RSA 2K private key from PKCS#8 DER"); // Get the public key From c7cca842ae89e6d6c70f44038a54b460ecd844aa Mon Sep 17 00:00:00 2001 From: alt3r 3go Date: Sun, 31 Jul 2022 15:57:39 +0200 Subject: [PATCH 10/10] WIP: Address review comments - 1 In a separate commit for better visibility. Removed addressed TODOs, reworded those that should stay for later (magic numbers), rebased to the latest "main". Signed-off-by: alt3r 3go --- Cargo.toml | 6 ++-- src/client/mechanisms.rs | 2 +- src/config.rs | 18 +++++------ src/mechanisms.rs | 3 +- src/mechanisms/{rsa2kpkcs.rs => rsa2k.rs} | 39 +++++++++-------------- src/store/keystore.rs | 6 ++-- src/types.rs | 7 ++-- tests/rsa2kpkcs.rs | 7 +--- 8 files changed, 36 insertions(+), 52 deletions(-) rename src/mechanisms/{rsa2kpkcs.rs => rsa2k.rs} (84%) diff --git a/Cargo.toml b/Cargo.toml index 7db5cb76657..51f12a1b92f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -91,9 +91,7 @@ default-mechanisms = [ "tdes", "totp", "trng", - "rsa2k-pkcs", - "rsa3k", - "rsa4k" + "rsa2k" ] aes256-cbc = [] chacha8-poly1305 = [] @@ -108,7 +106,7 @@ sha256 = [] tdes = ["des"] totp = ["sha-1"] trng = ["sha-1"] -rsa2k-pkcs = ["rsa"] +rsa2k = ["rsa"] rsa3k = ["rsa"] rsa4k = ["rsa"] diff --git a/src/client/mechanisms.rs b/src/client/mechanisms.rs index 06baa60a7b8..cc9d835877a 100644 --- a/src/client/mechanisms.rs +++ b/src/client/mechanisms.rs @@ -245,7 +245,7 @@ pub trait P256: CryptoClient { } } -#[cfg(feature = "rsa2k-pkcs")] +#[cfg(feature = "rsa2k")] impl Rsa2kPkcs for ClientImplementation {} pub trait Rsa2kPkcs: CryptoClient { diff --git a/src/config.rs b/src/config.rs index a2bace35121..e26bc5b79f4 100644 --- a/src/config.rs +++ b/src/config.rs @@ -44,23 +44,21 @@ cfg_if::cfg_if! { } pub const MAX_SHORT_DATA_LENGTH: usize = 128; -// TODO:alt3r-3go: Do we want better keylength granularity here? -#[cfg(any(feature = "rsa2k-pkcs", feature = "rsa3k", feature = "rsa4k"))] +#[cfg(any(feature = "rsa2k", feature = "rsa3k", feature = "rsa4k"))] pub const MAX_SIGNATURE_LENGTH: usize = 512; -#[cfg(any(feature = "rsa2k-pkcs", feature = "rsa3k", feature = "rsa4k"))] -// TODO:alt3r-3go: We use PKCS#8 DER format, this value was found empirically for 2K keys. Need to generalize. +#[cfg(any(feature = "rsa2k", feature = "rsa3k", feature = "rsa4k"))] +// TODO: We use PKCS#8 DER format, this value was found empirically for 2K keys. Need to generalize. pub const MAX_KEY_MATERIAL_LENGTH: usize = 1217; -#[cfg(any(feature = "rsa2k-pkcs", feature = "rsa3k", feature = "rsa4k"))] -// TODO:alt3r-3go: This is due to the fact that KEY_MATERIAL_LENGTH is now bigger than MESSAGE_LENGTH. -// Double-check this is okay. +#[cfg(any(feature = "rsa2k", feature = "rsa3k", feature = "rsa4k"))] +// This is due to the fact that KEY_MATERIAL_LENGTH is bigger than MESSAGE_LENGTH for RSA. pub const MAX_MESSAGE_LENGTH: usize = MAX_KEY_MATERIAL_LENGTH; -#[cfg(not(any(feature = "rsa2k-pkcs", feature = "rsa3k", feature = "rsa4k")))] +#[cfg(not(any(feature = "rsa2k", feature = "rsa3k", feature = "rsa4k")))] pub const MAX_SIGNATURE_LENGTH: usize = 72; -#[cfg(not(any(feature = "rsa2k-pkcs", feature = "rsa3k", feature = "rsa4k")))] +#[cfg(not(any(feature = "rsa2k", feature = "rsa3k", feature = "rsa4k")))] pub const MAX_KEY_MATERIAL_LENGTH: usize = 128; -#[cfg(not(any(feature = "rsa2k-pkcs", feature = "rsa3k", feature = "rsa4k")))] +#[cfg(not(any(feature = "rsa2k", feature = "rsa3k", feature = "rsa4k")))] pub const MAX_MESSAGE_LENGTH: usize = 1024; // must be MAX_KEY_MATERIAL_LENGTH + 4 diff --git a/src/mechanisms.rs b/src/mechanisms.rs index 2461811d928..9f38d7352a1 100644 --- a/src/mechanisms.rs +++ b/src/mechanisms.rs @@ -47,7 +47,8 @@ pub struct P256Prehashed {} mod p256; pub struct Rsa2kPkcs {} -mod rsa2kpkcs; +// Later on we'll add: "pub struct Rsa2kPss {}" and so on +mod rsa2k; pub struct Sha256 {} mod sha256; diff --git a/src/mechanisms/rsa2kpkcs.rs b/src/mechanisms/rsa2k.rs similarity index 84% rename from src/mechanisms/rsa2kpkcs.rs rename to src/mechanisms/rsa2k.rs index 9c4ab6b6e47..6f3e8838f87 100644 --- a/src/mechanisms/rsa2kpkcs.rs +++ b/src/mechanisms/rsa2k.rs @@ -12,11 +12,7 @@ use crate::error::Error; use crate::service::*; use crate::types::*; -//TODO:alt3r-3go: sign() and verify() are the only two methods that are actually different between -pkcs and -pss. -// Moreover, the key::Kind::Rsa2K could also probably be parametrized, instead of having a dedicated kind -// for each. Overall this means the class structure can probably be simplified - need to decide. - -#[cfg(feature = "rsa2k-pkcs")] +#[cfg(feature = "rsa2k")] impl DeriveKey for super::Rsa2kPkcs { #[inline(never)] @@ -55,7 +51,7 @@ impl DeriveKey for super::Rsa2kPkcs } } -#[cfg(feature = "rsa2k-pkcs")] +#[cfg(feature = "rsa2k")] impl DeserializeKey for super::Rsa2kPkcs { #[inline(never)] @@ -73,7 +69,7 @@ impl DeserializeKey for super::Rsa2kPkcs let private_key: RsaPrivateKey = DecodePrivateKey::from_pkcs8_der(&request.serialized_key) .map_err(|_| Error::InvalidSerializedKey)?; - // We store our keys in PKCS#8 DER format as well + // We store our keys in PKCS#8 DER format let private_key_der = private_key.to_pkcs8_der() .expect("Failed to serialize an RSA 2K private key to PKCS#8 DER"); @@ -88,7 +84,7 @@ impl DeserializeKey for super::Rsa2kPkcs } } -#[cfg(feature = "rsa2k-pkcs")] +#[cfg(feature = "rsa2k")] impl GenerateKey for super::Rsa2kPkcs { #[inline(never)] @@ -125,7 +121,7 @@ impl GenerateKey for super::Rsa2kPkcs } } -#[cfg(feature = "rsa2k-pkcs")] +#[cfg(feature = "rsa2k")] impl SerializeKey for super::Rsa2kPkcs { #[inline(never)] @@ -140,8 +136,6 @@ impl SerializeKey for super::Rsa2kPkcs .material; let serialized_key = match request.format { - // TODO:alt3r-3go: There are "Der" and "Asn1Der" commented out in KeySerialization enum, - // should those be used instead? KeySerialization::Raw => { let mut serialized_key = Message::new(); serialized_key.extend_from_slice(&priv_key_der).map_err(|_| Error::InternalError)?; @@ -155,7 +149,7 @@ impl SerializeKey for super::Rsa2kPkcs } } -#[cfg(feature = "rsa2k-pkcs")] +#[cfg(feature = "rsa2k")] impl Exists for super::Rsa2kPkcs { #[inline(never)] @@ -169,7 +163,7 @@ impl Exists for super::Rsa2kPkcs } } -#[cfg(feature = "rsa2k-pkcs")] +#[cfg(feature = "rsa2k")] impl Sign for super::Rsa2kPkcs { #[inline(never)] @@ -188,12 +182,9 @@ impl Sign for super::Rsa2kPkcs .expect("Failed to deserialize an RSA 2K private key from PKCS#8 DER"); // RSA lib takes in a hash value to sign, not raw data. - // TODO:alt3r-3go: Do we assume we get digest into this function, or we calculate it ourselves? - // use sha2::digest::Digest; - // let digest_to_sign: [u8; 32] = sha2::Sha256::digest(&request.message).into(); + // We assume we get digest into this function, too. - // TODO:alt3r-3go: There's also .sign_blinded(), which is supposed to protect the private key from timing side channels, - // but requires an RNG instance - decide if we want to always use it. + // TODO: Consider using .sign_blinded(), which is supposed to protect the private key from timing side channels use rsa::padding::PaddingScheme; use rsa::hash::Hash; let native_signature = priv_key @@ -211,7 +202,7 @@ impl Sign for super::Rsa2kPkcs } } -#[cfg(feature = "rsa2k-pkcs")] +#[cfg(feature = "rsa2k")] impl Verify for super::Rsa2kPkcs { #[inline(never)] @@ -223,7 +214,7 @@ impl Verify for super::Rsa2kPkcs return Err(Error::InvalidSerializationFormat); } - // TODO:alt3r-3go: This must not be a hardcoded magic number, need to generalize + // TODO: This must not be a hardcoded magic number, convert when a common mechanism is available if request.signature.len() != 256 { return Err(Error::WrongSignatureLength); } @@ -250,11 +241,11 @@ impl Verify for super::Rsa2kPkcs } } -#[cfg(not(feature = "rsa2k-pkcs"))] +#[cfg(not(feature = "rsa2k"))] impl DeriveKey for super::Rsa2kPkcs {} -#[cfg(not(feature = "rsa2k-pkcs"))] +#[cfg(not(feature = "rsa2k"))] impl GenerateKey for super::Rsa2kPkcs {} -#[cfg(not(feature = "rsa2k-pkcs"))] +#[cfg(not(feature = "rsa2k"))] impl Sign for super::Rsa2kPkcs {} -#[cfg(not(feature = "rsa2k-pkcs"))] +#[cfg(not(feature = "rsa2k"))] impl Verify for super::Rsa2kPkcs {} diff --git a/src/store/keystore.rs b/src/store/keystore.rs index 020baa6465e..f82113a4546 100644 --- a/src/store/keystore.rs +++ b/src/store/keystore.rs @@ -147,11 +147,9 @@ impl Keystore for ClientKeystore

{ let location = self.location(secrecy, id).ok_or(Error::NoSuchKey)?; - //TODO:alt3r-3go: This should better be defined in some way, instead of hardcoding. - // I've tried referring to MAX_SERIALIZED_KEY_LENGTH, is this a good idea? - #[cfg(not(feature = "rsa2k-pkcs"))] + #[cfg(not(feature = "rsa2k"))] let bytes: Bytes<128> = store::read(self.store, location, &path)?; - #[cfg(feature = "rsa2k-pkcs")] + #[cfg(feature = "rsa2k")] let bytes: Bytes = store::read(self.store, location, &path)?; let key = key::Key::try_deserialize(&bytes)?; diff --git a/src/types.rs b/src/types.rs index 4d28e71821b..ace1d31d354 100644 --- a/src/types.rs +++ b/src/types.rs @@ -509,8 +509,11 @@ pub enum Mechanism { Trng, X255, Rsa2kPkcs, - Rsa3k, - Rsa4k, + Rsa2kPss, + Rsa3kPkcs, + Rsa3kPss, + Rsa4kPkcs, + Rsa4kPss, } pub type LongData = Bytes; diff --git a/tests/rsa2kpkcs.rs b/tests/rsa2kpkcs.rs index e7b96fc7d21..f4a621d556c 100644 --- a/tests/rsa2kpkcs.rs +++ b/tests/rsa2kpkcs.rs @@ -9,9 +9,7 @@ use trussed::types::KeySerialization; use trussed::types::Location::*; use trussed::types::StorageAttributes; -// TODO:alt3r-3go: Looks like the test infra is not supposed to be used with several tests like below - -// right now it randomly fails with SIGSERV on either of the two, when run together, -// but never when run separately. Need to investigate and fix. +// Tests below can be run on a PC using the "virt" feature #[test] fn rsa2kpkcs_generate_key() { @@ -19,7 +17,6 @@ fn rsa2kpkcs_generate_key() { let sk = syscall!(client.generate_rsa2kpkcs_private_key(Internal)).key; // This assumes we don't ever get a key with ID 0 - // TODO:alt3r-3go: make sure the above always holds or find a better way to check for success assert_ne!(sk, KeyId::from_special(0)); }) } @@ -31,7 +28,6 @@ fn rsa2kpkcs_derive_key() { let pk = syscall!(client.derive_rsa2kpkcs_public_key(sk, Volatile)).key; // This assumes we don't ever get a key with ID 0 - // TODO:alt3r-3go: make sure the above always holds or find a better way to check for success assert_ne!(pk, KeyId::from_special(0)); }) } @@ -67,7 +63,6 @@ fn rsa2kpkcs_deserialize_key() { let deserialized_key_id = syscall!(client.deserialize_rsa2kpkcs_key(&serialized_key, KeySerialization::Raw, location)).key; // This assumes we don't ever get a key with ID 0 - // TODO:alt3r-3go: make sure the above always holds or find a better way to check for success assert_ne!(deserialized_key_id, KeyId::from_special(0)); }) }