Merge branch 'main' into fix-hugues-email

e2e_tests/tests/per_provider/normal_tests/asym_encryption.rs

@@ -213,7 +213,7 @@ fn asym_encrypt_wrong_algorithm() {
         return;
     }
 
-    let _key_id = client
+    client
         .generate_rsa_encryption_keys_rsaoaep_sha256(key_name.clone())
         .unwrap();
     let status = client
@@ -271,6 +271,34 @@ fn asym_encrypt_decrypt_rsa_pkcs_different_keys() {
         .unwrap_err();
 }
 
+// Test is disabled for PKCS11 provider since SoftHSMv2 does not
+// properly notify users of invalid padding.
+// See https://github.com/opendnssec/SoftHSMv2/issues/678
+#[cfg(not(feature = "pkcs11-provider"))]
+#[test]
+fn asym_decrypt_wrong_padding() {
+    let key_name = auto_test_keyname!();
+    let mut client = TestClient::new();
+
+    if !client.is_operation_supported(Opcode::PsaAsymmetricEncrypt)
+        || !client.is_operation_supported(Opcode::PsaAsymmetricDecrypt)
+    {
+        return;
+    }
+
+    client
+        .generate_rsa_encryption_keys_rsapkcs1v15crypt(key_name.clone())
+        .unwrap();
+    let mut ciphertext = client
+        .asymmetric_encrypt_message_with_rsapkcs1v15(key_name.clone(), PLAINTEXT_MESSAGE.to_vec())
+        .unwrap();
+    ciphertext[20] ^= 0x1;
+    let res = client
+        .asymmetric_decrypt_message_with_rsapkcs1v15(key_name, ciphertext)
+        .unwrap_err();
+    assert_eq!(res, ResponseStatus::PsaErrorInvalidPadding);
+}
+
 #[test]
 fn asym_encrypt_verify_decrypt_with_rsa_crate() {
     let key_name = auto_test_keyname!();

e2e_tests/tests/per_provider/normal_tests/export_key.rs

@@ -244,12 +244,12 @@ fn check_export_rsa_not_possible() {
         },
     };
 
-    let _generated_key = client
+    client
         .generate_key(key_name.clone(), key_attributes)
         .unwrap();
 
-    let _exported_key = client.export_key(key_name).unwrap_err();
-    assert_eq!(_exported_key, ResponseStatus::PsaErrorNotPermitted);
+    let exported_key = client.export_key(key_name).unwrap_err();
+    assert_eq!(exported_key, ResponseStatus::PsaErrorNotPermitted);
 }
 
 #[test]
@@ -277,12 +277,12 @@ fn check_export_ecc_not_possible() {
         },
     };
 
-    let _generated_key = client
+    client
         .generate_key(key_name.clone(), key_attributes)
         .unwrap();
 
-    let _exported_key = client.export_key(key_name).unwrap_err();
-    assert_eq!(_exported_key, ResponseStatus::PsaErrorNotPermitted);
+    let exported_key = client.export_key(key_name).unwrap_err();
+    assert_eq!(exported_key, ResponseStatus::PsaErrorNotPermitted);
 }
 
 #[cfg(not(feature = "cryptoauthlib-provider"))]

src/key_info_managers/mod.rs

@@ -352,7 +352,7 @@ impl KeyInfoManagerClient {
         let mut clients = Vec::new();
         for key_identity in key_identities {
             if !clients.contains(&key_identity.application) {
-                let _ = clients.push(key_identity.application.clone());
+                clients.push(key_identity.application.clone());
             }
         }
 

src/providers/mbed_crypto/key_management.rs

@@ -21,7 +21,7 @@ pub fn create_key_id(max_current_id: &AtomicU32) -> Result<key::psa_key_id_t> {
     if new_key_id > key::PSA_KEY_ID_USER_MAX {
         // If storing key failed and no other keys were created in the mean time, it is safe to
         // decrement the key counter.
-        let _ = max_current_id.store(key::PSA_KEY_ID_USER_MAX, Relaxed);
+        max_current_id.store(key::PSA_KEY_ID_USER_MAX, Relaxed);
         error!(
             "PSA max key ID limit of {} reached",
             key::PSA_KEY_ID_USER_MAX
@@ -226,7 +226,7 @@ impl Provider {
         );
 
         let key_id = self.key_info_store.get_key_id(&key_identity)?;
-        let _ = self.key_info_store.remove_key_info(&key_identity)?;
+        self.key_info_store.remove_key_info(&key_identity)?;
 
         let _guard = self
             .key_handle_mutex

src/providers/pkcs11/asym_encryption.rs

@@ -5,9 +5,11 @@ use super::KeyPairType;
 use super::Provider;
 use crate::authenticators::ApplicationIdentity;
 use crate::key_info_managers::KeyIdentity;
+use cryptoki::error::Error;
+use cryptoki::error::RvError;
 use cryptoki::mechanism::Mechanism;
 use log::{info, trace};
-use parsec_interface::operations::psa_algorithm::Algorithm;
+use parsec_interface::operations::psa_algorithm::{Algorithm, AsymmetricEncryption};
 use parsec_interface::operations::{psa_asymmetric_decrypt, psa_asymmetric_encrypt};
 use parsec_interface::requests::{ResponseStatus, Result};
 use std::convert::TryFrom;
@@ -70,7 +72,25 @@ impl Provider {
         Ok(psa_asymmetric_decrypt::Result {
             plaintext: session
                 .decrypt(&mech, key, &op.ciphertext)
-                .map_err(to_response_status)?
+                .map_err(|e| {
+                    // If the algorithm is RSA with PKCS#1 v1.5 padding and we get CKR_ENCRYPTED_DATA_INVALID back,
+                    // it means the padding has been deemed invalid and we should let the caller know
+                    // about that. This allows clients to mitigate attacks that leverage padding
+                    // oracles a la Bleichenbacher.
+                    // See https://cryptosense.com/blog/why-pkcs1v1-5-encryption-should-be-put-out-of-our-misery
+                    // for more details.
+                    if let Algorithm::AsymmetricEncryption(AsymmetricEncryption::RsaPkcs1v15Crypt) =
+                        key_attributes.policy.permitted_algorithms
+                    {
+                        match e {
+                            Error::Pkcs11(RvError::EncryptedDataInvalid) => {
+                                return ResponseStatus::PsaErrorInvalidPadding
+                            }
+                            _ => (),
+                        }
+                    }
+                    to_response_status(e)
+                })?
                 .into(),
         })
     }

src/providers/tpm/asym_encryption.rs

@@ -3,10 +3,12 @@
 use super::{utils, Provider};
 use crate::authenticators::ApplicationIdentity;
 use crate::key_info_managers::KeyIdentity;
+use parsec_interface::operations::psa_algorithm::{Algorithm, AsymmetricEncryption};
 use parsec_interface::operations::{psa_asymmetric_decrypt, psa_asymmetric_encrypt};
-use parsec_interface::requests::Result;
+use parsec_interface::requests::{ResponseStatus, Result};
 use std::convert::TryInto;
 use std::ops::Deref;
+use tss_esapi::{constants::Tss2ResponseCodeKind, Error};
 
 impl Provider {
     pub(super) fn psa_asymmetric_encrypt_internal(
@@ -114,6 +116,22 @@ impl Provider {
                 plaintext: plaintext.value().to_vec().into(),
             }),
             Err(tss_error) => {
+                // If the algorithm is RSA with PKCS#1 v1.5 padding and we get TPM_RC_VALUE back,
+                // it means the padding has been deemed invalid and we should let the caller know
+                // about that. This allows clients to mitigate attacks that leverage padding
+                // oracles a la Bleichenbacher.
+                // See https://cryptosense.com/blog/why-pkcs1v1-5-encryption-should-be-put-out-of-our-misery
+                // for more details.
+                if let Algorithm::AsymmetricEncryption(AsymmetricEncryption::RsaPkcs1v15Crypt) =
+                    key_attributes.policy.permitted_algorithms
+                {
+                    if let Error::Tss2Error(e) = tss_error {
+                        if Some(Tss2ResponseCodeKind::Value) == e.kind() {
+                            format_error!("Wrong plaintext padding", tss_error);
+                            return Err(ResponseStatus::PsaErrorInvalidPadding);
+                        }
+                    }
+                }
                 let error = utils::to_response_status(tss_error);
                 format_error!("Encryption failed", tss_error);
                 Err(error)

src/providers/tpm/key_management.rs

@@ -55,7 +55,7 @@ impl Provider {
 
                     // Grab key attributes and replace legacy entry with new one
                     let attributes = self.key_info_store.get_key_attributes(key_identity)?;
-                    let _ = self.key_info_store.replace_key_info(
+                    self.key_info_store.replace_key_info(
                         key_identity.clone(),
                         &password_ctx,
                         attributes,
@@ -195,7 +195,7 @@ impl Provider {
             key_name,
         );
 
-        let _ = self.key_info_store.remove_key_info(&key_identity)?;
+        self.key_info_store.remove_key_info(&key_identity)?;
 
         Ok(psa_destroy_key::Result {})
     }

src/providers/trusted_service/context/key_management.rs

@@ -30,7 +30,7 @@ impl Context {
                 }),
             }),
         };
-        let _ = self.send_request(&generate_req)?;
+        self.send_request(&generate_req)?;
 
         Ok(())
     }
@@ -70,7 +70,7 @@ impl Context {
             }),
             data,
         };
-        let _ = self.send_request(&import_req)?;
+        self.send_request(&import_req)?;
 
         Ok(())
     }

src/providers/trusted_service/key_management.rs

@@ -21,7 +21,7 @@ pub fn create_key_id(max_current_id: &AtomicU32) -> Result<u32> {
     if new_key_id > PSA_KEY_ID_USER_MAX {
         // If storing key failed and no other keys were created in the mean time, it is safe to
         // decrement the key counter.
-        let _ = max_current_id.store(PSA_KEY_ID_USER_MAX, Relaxed);
+        max_current_id.store(PSA_KEY_ID_USER_MAX, Relaxed);
         error!("PSA max key ID limit of {} reached", PSA_KEY_ID_USER_MAX);
         return Err(ResponseStatus::PsaErrorInsufficientMemory);
     }
@@ -193,7 +193,7 @@ impl Provider {
             key_name,
         );
         let key_id = self.key_info_store.get_key_id(&key_identity)?;
-        let _ = self.key_info_store.remove_key_info(&key_identity)?;
+        self.key_info_store.remove_key_info(&key_identity)?;
 
         match self.context.destroy_key(key_id) {
             Ok(()) => Ok(psa_destroy_key::Result {}),

src/utils/service_builder.rs

@@ -276,7 +276,7 @@ fn build_providers(
                 return Err(Error::new(ErrorKind::Other, "failed to create provider").into());
             }
         };
-        let _ = providers.push((provider_id, provider));
+        providers.push((provider_id, provider));
     }
 
     Ok(providers)