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/rsa2kpkcs.rs deleted file mode 100644 index 9c4ab6b6e47..00000000000 --- a/src/mechanisms/rsa2kpkcs.rs +++ /dev/null @@ -1,260 +0,0 @@ -use rsa::{ - RsaPrivateKey, - RsaPublicKey, - PublicKey, - pkcs8::{EncodePrivateKey, DecodePrivateKey, EncodePublicKey} -}; - -use crate::api::*; -// use crate::config::*; -// use crate::debug; -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 -{ - #[inline(never)] - fn derive_key(keystore: &mut impl Keystore, request: &request::DeriveKey) - -> Result - { - // Retrieve private key - let base_key_id = &request.base_key; - - // std::println!("Loading key: {:?}", base_key_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 = 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 - 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")] -impl DeserializeKey for super::Rsa2kPkcs -{ - #[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); - } - - 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 - 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::Secret, key::Kind::Rsa2k, - private_key_der.as_ref())?; - - Ok(reply::DeserializeKey { - key: private_key_id, - }) - } -} - -#[cfg(feature = "rsa2k-pkcs")] -impl GenerateKey for super::Rsa2kPkcs -{ - #[inline(never)] - fn generate_key(keystore: &mut impl Keystore, request: &request::GenerateKey) - -> Result - { - // We want an RSA 2K key - let bits = 2048; - - 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 priv_key_id = keystore.store_key( - request.attributes.persistence, - key::Secrecy::Secret, - key::Info::from(key::Kind::Rsa2k).with_local_flag(), - priv_key_der.as_ref())?; - - // return handle - Ok(reply::GenerateKey { - key: priv_key_id, - }) - } -} - -#[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; - - // 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: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)?; - serialized_key - } - - _ => { return Err(Error::InternalError); } - }; - - Ok(reply::SerializeKey { serialized_key }) - } -} - -#[cfg(feature = "rsa2k-pkcs")] -impl Exists for super::Rsa2kPkcs -{ - #[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::Rsa2k), &key_id); - Ok(reply::Exists { exists }) - } -} - -#[cfg(feature = "rsa2k-pkcs")] -impl Sign for super::Rsa2kPkcs -{ - #[inline(never)] - fn sign(keystore: &mut impl Keystore, request: &request::Sign) - -> Result - { - // First, get the key - let key_id = request.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 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. - // 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: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 - .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); - } - - // TODO:alt3r-3go: 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 = DecodePrivateKey::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"))] -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/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)); }) }