pkcs5: add generate_* method prefix; support TryCryptoRng

Adds a `generate_*` prefix to all methods of `pbes2::Parameters` that
accept an `rng` parameter and generate a random IV/salt, and changes the
argument from `CryptoRng` to `TryCryptoRng`, propagating errors rather
than using `expect`, which gets rid of a bunch of panic lint
suppression.

Also extracts constants to `ScryptParams` and `Pbkdf2Params`, and
renames the constructor for the latter to `Pbkdf2Params::hmac_sha256`.

Author: tarcieri

cms/tests/builder.rs

@@ -697,7 +697,7 @@ fn test_create_password_recipient_info() {
             Aes128CbcPwriEncryptor {
                 challenge_password,
                 key_encryption_iv,
-                key_derivation_params: pkcs5::pbes2::Pbkdf2Params::hmac_with_sha256(
+                key_derivation_params: pkcs5::pbes2::Pbkdf2Params::hmac_sha256(
                     60_000, // use >=600_000 in real world applications
                     b"salz",
                 )

pkcs5/src/error.rs

@@ -22,10 +22,14 @@ pub enum Error {
     /// Encryption Failed
     EncryptFailed,
 
-    /// Pbes1 support is limited to parsing; encryption/decryption is not supported (won't fix)
+    /// PBES1 support is limited to parsing; encryption/decryption is not supported (won't fix)
     #[cfg(feature = "pbes2")]
     NoPbes1CryptSupport,
 
+    /// Random number generation failure.
+    #[cfg(feature = "rand_core")]
+    Rng,
+
     /// Algorithm is not supported
     ///
     /// This may be due to a disabled crate feature
@@ -50,6 +54,8 @@ impl fmt::Display for Error {
             Error::NoPbes1CryptSupport => {
                 f.write_str("PKCS#5 encryption/decryption unsupported for PBES1 (won't fix)")
             }
+            #[cfg(feature = "rand_core")]
+            Error::Rng => f.write_str("random number generation failure"),
             Error::UnsupportedAlgorithm { oid } => {
                 write!(f, "PKCS#5 algorithm {oid} is unsupported")
             }

pkcs5/src/pbes2.rs

@@ -19,7 +19,7 @@ use der::{
 };
 
 #[cfg(feature = "rand_core")]
-use rand_core::CryptoRng;
+use rand_core::TryCryptoRng;
 
 #[cfg(all(feature = "alloc", feature = "pbes2"))]
 use alloc::vec::Vec;
@@ -89,11 +89,14 @@ impl Parameters {
     /// Generate PBES2 parameters using the recommended algorithm settings and
     /// a randomly generated salt and IV.
     ///
-    /// This is currently an alias for [`Parameters::scrypt`]. See that method
+    /// This is currently an alias for [`Parameters::generate_scrypt`]. See that method
     /// for more information.
+    ///
+    /// # Errors
+    /// Returns [`Error::Rng`] in the event the random number generator `R` fails.
     #[cfg(all(feature = "pbes2", feature = "rand_core"))]
-    pub fn recommended<R: CryptoRng>(rng: &mut R) -> Self {
-        Self::scrypt(rng)
+    pub fn generate_recommended<R: TryCryptoRng>(rng: &mut R) -> Result<Self> {
+        Self::generate_scrypt(rng)
     }
 
     /// Generate PBES2 parameters using PBKDF2 as the password hashing
@@ -103,29 +106,31 @@ impl Parameters {
     ///
     /// This will use AES-256-CBC as the encryption algorithm and SHA-256 as
     /// the hash function for PBKDF2.
+    ///
+    /// # Errors
+    /// Returns [`Error::Rng`] in the event the random number generator `R` fails.
     #[cfg(feature = "rand_core")]
-    #[allow(clippy::missing_panics_doc, reason = "params should be valid")]
-    pub fn pbkdf2<R: CryptoRng>(rng: &mut R) -> Self {
+    pub fn generate_pbkdf2<R: TryCryptoRng>(rng: &mut R) -> Result<Self> {
         let mut iv = [0u8; Self::DEFAULT_IV_LEN];
-        rng.fill_bytes(&mut iv);
+        rng.try_fill_bytes(&mut iv).map_err(|_| Error::Rng)?;
 
         let mut salt = [0u8; Self::DEFAULT_SALT_LEN];
-        rng.fill_bytes(&mut salt);
+        rng.try_fill_bytes(&mut salt).map_err(|_| Error::Rng)?;
 
-        Self::pbkdf2_sha256_aes256cbc(600_000, &salt, iv).expect("invalid PBKDF2 parameters")
+        Self::generate_pbkdf2_sha256_aes256cbc(Pbkdf2Params::DEFAULT_SHA256_ITERATIONS, &salt, iv)
     }
 
     /// Initialize PBES2 parameters using PBKDF2-SHA256 as the password-based
     /// key derivation function and AES-128-CBC as the symmetric cipher.
     ///
     /// # Errors
-    /// Propagates errors from [`Pbkdf2Params::hmac_with_sha256`].
-    pub fn pbkdf2_sha256_aes128cbc(
+    /// Propagates errors from [`Pbkdf2Params::hmac_sha256`].
+    pub fn generate_pbkdf2_sha256_aes128cbc(
         pbkdf2_iterations: u32,
         pbkdf2_salt: &[u8],
         aes_iv: [u8; AES_BLOCK_SIZE],
     ) -> Result<Self> {
-        let kdf = Pbkdf2Params::hmac_with_sha256(pbkdf2_iterations, pbkdf2_salt)?.into();
+        let kdf = Pbkdf2Params::hmac_sha256(pbkdf2_iterations, pbkdf2_salt)?.into();
         let encryption = EncryptionScheme::Aes128Cbc { iv: aes_iv };
         Ok(Self { kdf, encryption })
     }
@@ -134,13 +139,13 @@ impl Parameters {
     /// key derivation function and AES-256-CBC as the symmetric cipher.
     ///
     /// # Errors
-    /// Propagates errors from [`Pbkdf2Params::hmac_with_sha256`].
-    pub fn pbkdf2_sha256_aes256cbc(
+    /// Propagates errors from [`Pbkdf2Params::hmac_sha256`].
+    pub fn generate_pbkdf2_sha256_aes256cbc(
         pbkdf2_iterations: u32,
         pbkdf2_salt: &[u8],
         aes_iv: [u8; AES_BLOCK_SIZE],
     ) -> Result<Self> {
-        let kdf = Pbkdf2Params::hmac_with_sha256(pbkdf2_iterations, pbkdf2_salt)?.into();
+        let kdf = Pbkdf2Params::hmac_sha256(pbkdf2_iterations, pbkdf2_salt)?.into();
         let encryption = EncryptionScheme::Aes256Cbc { iv: aes_iv };
         Ok(Self { kdf, encryption })
     }
@@ -161,20 +166,26 @@ impl Parameters {
     /// - salt length: 16
     ///
     /// [RustCrypto/formats#1205]: https://github.com/RustCrypto/formats/issues/1205
+    ///
+    /// # Errors
+    /// Returns [`Error::Rng`] in the event the random number generator `R` fails.
     #[cfg(all(feature = "pbes2", feature = "rand_core"))]
     #[cfg(feature = "rand_core")]
-    #[allow(clippy::missing_panics_doc, reason = "params should be valid")]
-    pub fn scrypt<R: CryptoRng>(rng: &mut R) -> Self {
+    pub fn generate_scrypt<R: TryCryptoRng>(rng: &mut R) -> Result<Self> {
         let mut iv = [0u8; Self::DEFAULT_IV_LEN];
-        rng.fill_bytes(&mut iv);
+        rng.try_fill_bytes(&mut iv).map_err(|_| Error::Rng)?;
 
         let mut salt = [0u8; Self::DEFAULT_SALT_LEN];
-        rng.fill_bytes(&mut salt);
+        rng.try_fill_bytes(&mut salt).map_err(|_| Error::Rng)?;
+
+        let params = scrypt::Params::new(
+            ScryptParams::DEFAULT_LOG_N,
+            ScryptParams::DEFAULT_R,
+            ScryptParams::DEFAULT_P,
+        )
+        .map_err(|_| Error::AlgorithmParametersInvalid { oid: SCRYPT_OID })?;
 
-        scrypt::Params::new(14, 8, 1)
-            .ok()
-            .and_then(|params| Self::scrypt_aes256cbc(params, &salt, iv).ok())
-            .expect("invalid scrypt parameters")
+        Self::generate_scrypt_aes256cbc(params, &salt, iv)
     }
 
     /// Initialize PBES2 parameters using scrypt as the password-based
@@ -187,7 +198,7 @@ impl Parameters {
     /// Propagates errors from [`ScryptParams::from_params_and_salt`].
     // TODO(tarcieri): encapsulate `scrypt::Params`?
     #[cfg(feature = "pbes2")]
-    pub fn scrypt_aes128cbc(
+    pub fn generate_scrypt_aes128cbc(
         params: scrypt::Params,
         salt: &[u8],
         aes_iv: [u8; AES_BLOCK_SIZE],
@@ -210,7 +221,7 @@ impl Parameters {
     /// Propagates errors from [`ScryptParams::from_params_and_salt`].
     // TODO(tarcieri): encapsulate `scrypt::Params`?
     #[cfg(feature = "pbes2")]
-    pub fn scrypt_aes256cbc(
+    pub fn generate_scrypt_aes256cbc(
         params: scrypt::Params,
         salt: &[u8],
         aes_iv: [u8; AES_BLOCK_SIZE],

pkcs5/src/pbes2/kdf.rs

@@ -213,14 +213,18 @@ impl Pbkdf2Params {
     /// and [RFC 8018, §A.2](https://datatracker.ietf.org/doc/html/rfc8018#appendix-A.2)
     pub const MAX_ITERATION_COUNT: u32 = 100_000_000;
 
+    /// OWASP recommended number of iterations for PBKDF2-HMAC-SHA256.
+    #[cfg(all(feature = "pbes2", feature = "rand_core"))]
+    pub(super) const DEFAULT_SHA256_ITERATIONS: u32 = 600_000;
+
     const INVALID_ERR: Error = Error::AlgorithmParametersInvalid { oid: PBKDF2_OID };
 
-    /// Initialize PBKDF2-SHA256 with the given iteration count and salt.
+    /// Initialize PBKDF2-HMAC-SHA256 with the given iteration count and salt.
     ///
     /// # Errors
     /// Returns [`Error::AlgorithmParametersInvalid`] if `iteration_count` exceeds
     /// [`Pbkdf2Params::MAX_ITERATION_COUNT`] or `salt` exceeds [`Salt::MAX_LEN`].
-    pub fn hmac_with_sha256(iteration_count: u32, salt: &[u8]) -> Result<Self> {
+    pub fn hmac_sha256(iteration_count: u32, salt: &[u8]) -> Result<Self> {
         if iteration_count > Self::MAX_ITERATION_COUNT {
             return Err(Self::INVALID_ERR);
         }
@@ -412,6 +416,15 @@ pub struct ScryptParams {
 }
 
 impl ScryptParams {
+    // NOTE: scrypt parameters are deliberately chosen to retain compatibility with OpenSSL v3.
+    // See RustCrypto/formats#1205 for more information.
+    #[cfg(all(feature = "pbes2", feature = "rand_core"))]
+    pub(super) const DEFAULT_LOG_N: u8 = 14;
+    #[cfg(all(feature = "pbes2", feature = "rand_core"))]
+    pub(super) const DEFAULT_R: u32 = 8;
+    #[cfg(all(feature = "pbes2", feature = "rand_core"))]
+    pub(super) const DEFAULT_P: u32 = 1;
+
     #[cfg(feature = "pbes2")]
     const INVALID_ERR: Error = Error::AlgorithmParametersInvalid { oid: SCRYPT_OID };
 

pkcs8/src/encrypted_private_key_info.rs

@@ -69,7 +69,7 @@ where
         password: impl AsRef<[u8]>,
         doc: &[u8],
     ) -> Result<SecretDocument> {
-        let pbes2_params = pbes2::Parameters::recommended(rng);
+        let pbes2_params = pbes2::Parameters::generate_recommended(rng)?;
         EncryptedPrivateKeyInfoOwned::encrypt_with(pbes2_params, password, doc)
     }
 

pkcs8/tests/encrypted_private_key.rs

@@ -170,7 +170,7 @@ fn decrypt_ed25519_der_encpriv_aes256_scrypt() {
 #[cfg(feature = "encryption")]
 #[test]
 fn encrypt_ed25519_der_encpriv_aes256_pbkdf2_sha256() {
-    let pbes2_params = pkcs5::pbes2::Parameters::pbkdf2_sha256_aes256cbc(
+    let pbes2_params = pkcs5::pbes2::Parameters::generate_pbkdf2_sha256_aes256cbc(
         2048,
         &hex!("79d982e70df91a88"),
         hex!("b2d02d78b2efd9dff694cf8e0af40925"),
@@ -191,7 +191,7 @@ fn encrypt_ed25519_der_encpriv_aes256_pbkdf2_sha256() {
 #[cfg(feature = "encryption")]
 #[test]
 fn encrypt_ed25519_der_encpriv_aes256_scrypt() {
-    let scrypt_params = pkcs5::pbes2::Parameters::scrypt_aes256cbc(
+    let scrypt_params = pkcs5::pbes2::Parameters::generate_scrypt_aes256cbc(
         pkcs5::scrypt::Params::new(15, 8, 1).unwrap(),
         &hex!("E6211E2348AD69E0"),
         hex!("9BD0A6251F2254F9FD5963887C27CF01"),