From 5bbe379ed0d90406da7c58c514d3e0e90467a84e Mon Sep 17 00:00:00 2001 From: Tony Arcieri Date: Wed, 22 Apr 2026 09:35:47 -0600 Subject: [PATCH] pkcs5: enable workspace-level lint config Applies the workspace-level `clippy` and other lints added in #2231 for the `spki` crate, then fixes the lint failures. --- pkcs5/Cargo.toml | 4 +++- pkcs5/src/lib.rs | 48 ++++++++++++++++++++++--------------- pkcs5/src/pbes1.rs | 8 +++++-- pkcs5/src/pbes2.rs | 36 +++++++++++++++++++++++++++- pkcs5/src/pbes2/kdf.rs | 31 +++++++++++++++++------- pkcs5/src/pbes2/kdf/salt.rs | 22 ++++++++--------- 6 files changed, 106 insertions(+), 43 deletions(-) diff --git a/pkcs5/Cargo.toml b/pkcs5/Cargo.toml index 3dacf9805..43e0fe8f1 100644 --- a/pkcs5/Cargo.toml +++ b/pkcs5/Cargo.toml @@ -40,6 +40,8 @@ des-insecure = ["dep:des", "pbes2"] pbes2 = ["dep:aes", "dep:cbc", "dep:pbkdf2", "dep:scrypt", "dep:sha2"] sha1-insecure = ["dep:sha1", "pbes2"] +[lints] +workspace = true + [package.metadata.docs.rs] -all-features = true rustdoc-args = ["--cfg", "docsrs"] diff --git a/pkcs5/src/lib.rs b/pkcs5/src/lib.rs index 97cdbc2d4..56ca6c246 100644 --- a/pkcs5/src/lib.rs +++ b/pkcs5/src/lib.rs @@ -6,14 +6,6 @@ html_favicon_url = "https://raw.githubusercontent.com/RustCrypto/media/6ee8e381/logo.svg" )] #![forbid(unsafe_code)] -#![warn( - clippy::mod_module_files, - clippy::unwrap_used, - missing_docs, - rust_2018_idioms, - unused_lifetimes, - unused_qualifications -)] //! # Usage //! @@ -65,8 +57,13 @@ pub enum EncryptionScheme { } impl EncryptionScheme { - /// Attempt to decrypt the given ciphertext, allocating and returning a - /// byte vector containing the plaintext. + /// Attempt to decrypt the given ciphertext, allocating and returning a byte vector containing + /// the plaintext. + /// + /// # Errors + /// Returns an error if the algorithm specified in this scheme's parameters is unsupported + /// (e.g. PBES1 is completely unsupported), or if the ciphertext is malformed (e.g. not a + /// multiple of a block mode's padding). #[cfg(all(feature = "alloc", feature = "pbes2"))] pub fn decrypt(&self, password: impl AsRef<[u8]>, ciphertext: &[u8]) -> Result> { match self { @@ -75,12 +72,13 @@ impl EncryptionScheme { } } - /// Attempt to decrypt the given ciphertext in-place using a key derived - /// from the provided password and this scheme's parameters. + /// Attempt to decrypt the given ciphertext in-place using a key derived from the provided + /// password and this scheme's parameters. /// - /// Returns an error if the algorithm specified in this scheme's parameters - /// is unsupported, or if the ciphertext is malformed (e.g. not a multiple - /// of a block mode's padding) + /// # Errors + /// Returns an error if the algorithm specified in this scheme's parameters is unsupported + /// (e.g. PBES1 is completely unsupported), or if the ciphertext is malformed (e.g. not a + /// multiple of a block mode's padding). #[cfg(feature = "pbes2")] pub fn decrypt_in_place<'a>( &self, @@ -93,8 +91,12 @@ impl EncryptionScheme { } } - /// Encrypt the given plaintext, allocating and returning a vector - /// containing the ciphertext. + /// Encrypt the given plaintext, allocating and returning a vector containing the ciphertext. + /// + /// # Errors + /// - For PBES1, simply returns [`Error::NoPbes1CryptSupport`] unconditionally. + /// - Returns [`Error::UnsupportedAlgorithm`] if support for the requested algorithm has not + /// been enabled in this crate's features. #[cfg(all(feature = "alloc", feature = "pbes2"))] pub fn encrypt(&self, password: impl AsRef<[u8]>, plaintext: &[u8]) -> Result> { match self { @@ -103,8 +105,13 @@ impl EncryptionScheme { } } - /// Encrypt the given ciphertext in-place using a key derived from the - /// provided password and this scheme's parameters. + /// Encrypt the given ciphertext in-place using a key derived from the provided password and + /// this scheme's parameters. + /// + /// # Errors + /// - For PBES1, simply returns [`Error::NoPbes1CryptSupport`] unconditionally. + /// - Returns [`Error::UnsupportedAlgorithm`] if support for the requested algorithm has not + /// been enabled in this crate's features. #[cfg(feature = "pbes2")] pub fn encrypt_in_place<'a>( &self, @@ -119,6 +126,7 @@ impl EncryptionScheme { } /// Get the [`ObjectIdentifier`] (a.k.a OID) for this algorithm. + #[must_use] pub fn oid(&self) -> ObjectIdentifier { match self { Self::Pbes1(params) => params.oid(), @@ -127,6 +135,7 @@ impl EncryptionScheme { } /// Get [`pbes1::Parameters`] if it is the selected algorithm. + #[must_use] pub fn pbes1(&self) -> Option<&pbes1::Algorithm> { match self { Self::Pbes1(alg) => Some(alg), @@ -135,6 +144,7 @@ impl EncryptionScheme { } /// Get [`pbes2::Parameters`] if it is the selected algorithm. + #[must_use] pub fn pbes2(&self) -> Option<&pbes2::Parameters> { match self { Self::Pbes2(params) => Some(params), diff --git a/pkcs5/src/pbes1.rs b/pkcs5/src/pbes1.rs index e754df61e..af099a5f9 100644 --- a/pkcs5/src/pbes1.rs +++ b/pkcs5/src/pbes1.rs @@ -50,7 +50,7 @@ pub const SALT_LENGTH: usize = 8; /// ``` /// /// [RFC 8018 Appendix A.C]: https://datatracker.ietf.org/doc/html/rfc8018#appendix-C -#[derive(Clone, Debug, Eq, PartialEq)] +#[derive(Clone, Copy, Debug, Eq, PartialEq)] pub struct Algorithm { /// Encryption scheme. pub encryption: EncryptionScheme, @@ -61,6 +61,7 @@ pub struct Algorithm { impl Algorithm { /// Get the [`ObjectIdentifier`] (a.k.a OID) for this algorithm. + #[must_use] pub fn oid(&self) -> ObjectIdentifier { self.encryption.oid() } @@ -117,7 +118,7 @@ impl<'a> TryFrom> for Algorithm { /// ``` /// /// [RFC 8018 Appendix A.3]: https://tools.ietf.org/html/rfc8018#appendix-A.3 -#[derive(Clone, Debug, Eq, PartialEq)] +#[derive(Clone, Copy, Debug, Eq, PartialEq)] pub struct Parameters { /// Salt value pub salt: [u8; SALT_LENGTH], @@ -206,6 +207,7 @@ impl TryFrom for EncryptionScheme { impl EncryptionScheme { /// Get the [`SymmetricCipher`] to be used. + #[must_use] pub fn cipher(self) -> SymmetricCipher { match self { Self::PbeWithMd2AndDesCbc => SymmetricCipher::DesCbc, @@ -218,6 +220,7 @@ impl EncryptionScheme { } /// Get the [`DigestAlgorithm`] to be used. + #[must_use] pub fn digest(self) -> DigestAlgorithm { match self { Self::PbeWithMd2AndDesCbc => DigestAlgorithm::Md2, @@ -230,6 +233,7 @@ impl EncryptionScheme { } /// Get the [`ObjectIdentifier`] (a.k.a OID) for this algorithm. + #[must_use] pub fn oid(self) -> ObjectIdentifier { match self { Self::PbeWithMd2AndDesCbc => PBE_WITH_MD2_AND_DES_CBC_OID, diff --git a/pkcs5/src/pbes2.rs b/pkcs5/src/pbes2.rs index 6a18f98ac..04f5dab65 100644 --- a/pkcs5/src/pbes2.rs +++ b/pkcs5/src/pbes2.rs @@ -104,6 +104,7 @@ impl Parameters { /// This will use AES-256-CBC as the encryption algorithm and SHA-256 as /// the hash function for PBKDF2. #[cfg(feature = "rand_core")] + #[allow(clippy::missing_panics_doc, reason = "params should be valid")] pub fn pbkdf2(rng: &mut R) -> Self { let mut iv = [0u8; Self::DEFAULT_IV_LEN]; rng.fill_bytes(&mut iv); @@ -116,6 +117,9 @@ impl Parameters { /// 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( pbkdf2_iterations: u32, pbkdf2_salt: &[u8], @@ -128,6 +132,9 @@ impl Parameters { /// Initialize PBES2 parameters using PBKDF2-SHA256 as the password-based /// key derivation function and AES-256-CBC as the symmetric cipher. + /// + /// # Errors + /// Propagates errors from [`Pbkdf2Params::hmac_with_sha256`]. pub fn pbkdf2_sha256_aes256cbc( pbkdf2_iterations: u32, pbkdf2_salt: &[u8], @@ -155,6 +162,8 @@ impl Parameters { /// /// [RustCrypto/formats#1205]: https://github.com/RustCrypto/formats/issues/1205 #[cfg(all(feature = "pbes2", feature = "rand_core"))] + #[cfg(feature = "rand_core")] + #[allow(clippy::missing_panics_doc, reason = "params should be valid")] pub fn scrypt(rng: &mut R) -> Self { let mut iv = [0u8; Self::DEFAULT_IV_LEN]; rng.fill_bytes(&mut iv); @@ -173,6 +182,9 @@ impl Parameters { /// /// For more information on scrypt parameters, see documentation for the /// [`scrypt::Params`] struct. + /// + /// # Errors + /// Propagates errors from [`ScryptParams::from_params_and_salt`]. // TODO(tarcieri): encapsulate `scrypt::Params`? #[cfg(feature = "pbes2")] pub fn scrypt_aes128cbc( @@ -193,6 +205,9 @@ impl Parameters { /// /// When in doubt, use `Default::default()` as the [`scrypt::Params`]. /// This also avoids the need to import the type from the `scrypt` crate. + /// + /// # Errors + /// Propagates errors from [`ScryptParams::from_params_and_salt`]. // TODO(tarcieri): encapsulate `scrypt::Params`? #[cfg(feature = "pbes2")] pub fn scrypt_aes256cbc( @@ -207,6 +222,10 @@ impl Parameters { /// Attempt to decrypt the given ciphertext, allocating and returning a /// byte vector containing the plaintext. + /// + /// # Errors + /// Returns [`Error::UnsupportedAlgorithm`] if support for the requested algorithm has not been + /// enabled in this crate's features. #[cfg(all(feature = "alloc", feature = "pbes2"))] pub fn decrypt(&self, password: impl AsRef<[u8]>, ciphertext: &[u8]) -> Result> { let mut buffer = ciphertext.to_vec(); @@ -220,7 +239,11 @@ impl Parameters { /// /// Returns an error if the algorithm specified in this scheme's parameters /// is unsupported, or if the ciphertext is malformed (e.g. not a multiple - /// of a block mode's padding) + /// of a block mode's padding). + /// + /// # Errors + /// Returns [`Error::UnsupportedAlgorithm`] if support for the requested algorithm has not been + /// enabled in this crate's features. #[cfg(feature = "pbes2")] pub fn decrypt_in_place<'a>( &self, @@ -232,6 +255,10 @@ impl Parameters { /// Encrypt the given plaintext, allocating and returning a vector /// containing the ciphertext. + /// + /// # Errors + /// Returns [`Error::UnsupportedAlgorithm`] if support for the requested algorithm has not been + /// enabled in this crate's features. #[cfg(all(feature = "alloc", feature = "pbes2"))] pub fn encrypt(&self, password: impl AsRef<[u8]>, plaintext: &[u8]) -> Result> { // TODO(tarcieri): support non-AES ciphers? @@ -250,6 +277,10 @@ impl Parameters { /// Encrypt the given plaintext in-place using a key derived from the /// provided password and this scheme's parameters, writing the ciphertext /// into the same buffer. + /// + /// # Errors + /// Returns [`Error::UnsupportedAlgorithm`] if support for the requested algorithm has not been + /// enabled in this crate's features. #[cfg(feature = "pbes2")] pub fn encrypt_in_place<'a>( &self, @@ -338,6 +369,7 @@ pub enum EncryptionScheme { impl EncryptionScheme { /// Get the size of a key used by this algorithm in bytes. + #[must_use] pub fn key_size(&self) -> usize { match self { Self::Aes128Cbc { .. } => 16, @@ -351,6 +383,7 @@ impl EncryptionScheme { } /// Get the [`ObjectIdentifier`] (a.k.a OID) for this algorithm. + #[must_use] pub fn oid(&self) -> ObjectIdentifier { match self { Self::Aes128Cbc { .. } => AES_128_CBC_OID, @@ -366,6 +399,7 @@ impl EncryptionScheme { /// Convenience function to turn the OID (see [`oid`](Self::oid)) /// of this [`EncryptionScheme`] into error case /// [`Error::AlgorithmParametersInvalid`] + #[must_use] pub fn to_alg_params_invalid(&self) -> Error { Error::AlgorithmParametersInvalid { oid: self.oid() } } diff --git a/pkcs5/src/pbes2/kdf.rs b/pkcs5/src/pbes2/kdf.rs index 01434c862..d7b508289 100644 --- a/pkcs5/src/pbes2/kdf.rs +++ b/pkcs5/src/pbes2/kdf.rs @@ -11,9 +11,6 @@ use der::{ asn1::{AnyRef, ObjectIdentifier}, }; -#[cfg(feature = "pbes2")] -use core::mem::size_of; - /// Password-Based Key Derivation Function (PBKDF2) OID. pub const PBKDF2_OID: ObjectIdentifier = ObjectIdentifier::new_unwrap("1.2.840.113549.1.5.12"); @@ -58,6 +55,7 @@ pub enum Kdf { impl Kdf { /// Get derived key length in bytes, if defined. // TODO(tarcieri): rename to `key_size` to match `EncryptionScheme::key_size`? + #[must_use] pub fn key_length(&self) -> Option { match self { Self::Pbkdf2(params) => params.key_length, @@ -66,6 +64,7 @@ impl Kdf { } /// Get the [`ObjectIdentifier`] (a.k.a OID) for this algorithm. + #[must_use] pub fn oid(&self) -> ObjectIdentifier { match self { Self::Pbkdf2(_) => PBKDF2_OID, @@ -74,6 +73,7 @@ impl Kdf { } /// Get [`Pbkdf2Params`] if it is the selected algorithm. + #[must_use] pub fn pbkdf2(&self) -> Option<&Pbkdf2Params> { match self { Self::Pbkdf2(params) => Some(params), @@ -82,6 +82,7 @@ impl Kdf { } /// Get [`ScryptParams`] if it is the selected algorithm. + #[must_use] pub fn scrypt(&self) -> Option<&ScryptParams> { match self { Self::Scrypt(params) => Some(params), @@ -90,17 +91,20 @@ impl Kdf { } /// Is the selected KDF PBKDF2? + #[must_use] pub fn is_pbkdf2(&self) -> bool { self.pbkdf2().is_some() } /// Is the selected KDF scrypt? + #[must_use] pub fn is_scrypt(&self) -> bool { self.scrypt().is_some() } /// Convenience function to turn the OID (see [`oid`](Self::oid)) /// of this [`Kdf`] into error case [`Error::AlgorithmParametersInvalid`] + #[must_use] pub fn to_alg_params_invalid(&self) -> Error { Error::AlgorithmParametersInvalid { oid: self.oid() } } @@ -181,7 +185,7 @@ impl TryFrom> for Kdf { /// ``` /// /// [RFC 8018 Appendix A.2]: https://tools.ietf.org/html/rfc8018#appendix-A.2 -#[derive(Clone, Debug, Eq, PartialEq)] +#[derive(Clone, Copy, Debug, Eq, PartialEq)] pub struct Pbkdf2Params { /// PBKDF2 salt // TODO(tarcieri): support `CHOICE` with `otherSource` @@ -211,7 +215,11 @@ impl Pbkdf2Params { const INVALID_ERR: Error = Error::AlgorithmParametersInvalid { oid: PBKDF2_OID }; - /// Initialize PBKDF2-SHA256 with the given iteration count and salt + /// Initialize PBKDF2-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 { if iteration_count > Self::MAX_ITERATION_COUNT { return Err(Self::INVALID_ERR); @@ -302,6 +310,7 @@ pub enum Pbkdf2Prf { impl Pbkdf2Prf { /// Get the [`ObjectIdentifier`] (a.k.a OID) for this algorithm. + #[must_use] pub fn oid(self) -> ObjectIdentifier { match self { Self::HmacWithSha1 => HMAC_WITH_SHA1_OID, @@ -384,7 +393,7 @@ impl Encode for Pbkdf2Prf { /// ``` /// /// [RFC 7914 Section 7.1]: https://datatracker.ietf.org/doc/html/rfc7914#section-7.1 -#[derive(Clone, Debug, Eq, PartialEq)] +#[derive(Clone, Copy, Debug, Eq, PartialEq)] pub struct ScryptParams { /// scrypt salt pub salt: Salt, @@ -408,6 +417,9 @@ impl ScryptParams { /// Get the [`ScryptParams`] for the provided upstream [`scrypt::Params`] /// and a provided salt string. + /// + /// # Errors + /// Returns [`Error::AlgorithmParametersInvalid`] if the parameters are invalid. // TODO(tarcieri): encapsulate `scrypt::Params`? #[cfg(feature = "pbes2")] pub fn from_params_and_salt(params: scrypt::Params, salt: &[u8]) -> Result { @@ -479,11 +491,12 @@ impl TryFrom for scrypt::Params { impl TryFrom<&ScryptParams> for scrypt::Params { type Error = Error; + #[allow(clippy::unwrap_in_result, reason = "invariant should hold")] fn try_from(params: &ScryptParams) -> Result { - let n = params.cost_parameter; - // Compute log2 and verify its correctness - let log_n = ((8 * size_of::() as u32) - n.leading_zeros() - 1) as u8; + let n = params.cost_parameter; + let log_n = + u8::try_from(ScryptCost::BITS - n.leading_zeros() - 1).expect("should always fit"); if 1 << log_n != n { return Err(ScryptParams::INVALID_ERR); diff --git a/pkcs5/src/pbes2/kdf/salt.rs b/pkcs5/src/pbes2/kdf/salt.rs index 7d2beadd2..a1e2a0531 100644 --- a/pkcs5/src/pbes2/kdf/salt.rs +++ b/pkcs5/src/pbes2/kdf/salt.rs @@ -1,11 +1,11 @@ -//! Salt storage buffer which works on heapless `no_std` targets. -// TODO(tarcieri): use `ArrayVec` when it's available in `core`. +//! Stack-allocated salt storage buffer which works on `no_alloc` targets. +// TODO(tarcieri): replace this with an `ArrayVec`-like type? use core::fmt; use der::{DecodeValue, EncodeValue, Error, FixedTag, Header, Length, Reader, Result, Tag, Writer}; /// Salt as used by the PBES2 KDF. -#[derive(Clone, Eq, PartialEq)] +#[derive(Clone, Copy, Eq, PartialEq)] pub struct Salt { inner: [u8; Self::MAX_LEN], length: Length, @@ -16,12 +16,12 @@ impl Salt { pub const MAX_LEN: usize = 32; /// Create a new salt from the given byte slice. + /// + /// # Errors + /// Returns [`Error`] in the event the length of `slice` exceeds [`Salt::MAX_LEN`]. pub fn new(slice: impl AsRef<[u8]>) -> Result { let slice = slice.as_ref(); - - if slice.len() > Self::MAX_LEN { - return Err(Self::TAG.length_error().into()); - } + let length = Length::new(u32::try_from(slice.len()).map_err(|_| Self::TAG.length_error())?); let mut inner = [0u8; Self::MAX_LEN]; let mut i = 0; @@ -31,19 +31,19 @@ impl Salt { i += 1; } - Ok(Self { - inner, - length: Length::new(slice.len() as u32), - }) + Ok(Self { inner, length }) } /// Borrow the salt data as a byte slice. + #[must_use] + #[allow(clippy::missing_panics_doc, reason = "invariant should hold")] pub fn as_bytes(&self) -> &[u8] { let length = usize::try_from(self.length).expect("should be less than Self::MAX_LEN"); &self.inner[..length] } /// Get the length of the salt data. + #[must_use] pub fn len(&self) -> Length { self.length }