clippy: further cleanups for optional features

ci-tests.sh

@@ -6,6 +6,7 @@ COMMON_FEATURES="p256 xchachapoly vector-tests"
 
 set -x
 cargo check --benches --tests --examples
+cargo clippy --features "$COMMON_FEATURES"
 cargo test $TARGET --no-default-features
 # Custom set of crypto without std
 cargo test $TARGET --no-default-features --features "default-resolver use-curve25519 use-blake2 use-chacha20poly1305"

src/builder.rs

@@ -298,12 +298,14 @@ impl<'builder> Builder<'builder> {
 
     #[cfg(not(feature = "hfs"))]
     #[allow(clippy::unnecessary_wraps)]
+    #[allow(clippy::needless_pass_by_value)]
     fn resolve_kem(_: Box<dyn CryptoResolver>, _: &mut HandshakeState) -> Result<(), Error> {
         // HFS is disabled, return nothing
         Ok(())
     }
 
     #[cfg(feature = "hfs")]
+    #[allow(clippy::needless_pass_by_value)]
     fn resolve_kem(
         resolver: Box<dyn CryptoResolver>,
         hs: &mut HandshakeState,

src/handshakestate.rs

@@ -432,7 +432,7 @@ impl HandshakeState {
                     self.symmetricstate.decrypt_and_mix_hash(&ptr[..read_len], ciphertext)?;
                     let mut kem_output_buf = [0; MAXKEMSSLEN];
                     let kem_output = &mut kem_output_buf[..kem.shared_secret_len()];
-                    kem.decapsulate(ciphertext, kem_output).map_err(|_| Error::Kem)?;
+                    kem.decapsulate(ciphertext, kem_output)?;
                     self.symmetricstate.mix_key(&kem_output[..kem.shared_secret_len()]);
                     ptr = &ptr[read_len..];
                 },

src/params/mod.rs

@@ -193,7 +193,7 @@ impl NoiseParams {
 
     #[cfg(feature = "hfs")]
     /// Construct a new NoiseParams via specifying enums directly.
-    pub fn new(
+    #[must_use] pub fn new(
         name: String,
         base: BaseChoice,
         handshake: HandshakeChoice,
@@ -235,17 +235,14 @@ impl FromStr for NoiseParams {
             split.next().ok_or(PatternProblem::TooFewParameters)?.parse()?,
             split
                 .peek()
-                .ok_or(PatternProblem::TooFewParameters)?
-                .splitn(2, '+')
+                .ok_or(PatternProblem::TooFewParameters)?.split('+')
                 .nth(0)
                 .ok_or(PatternProblem::TooFewParameters)?
                 .parse()?,
             split
                 .next()
-                .ok_or(PatternProblem::TooFewParameters)?
-                .splitn(2, '+')
-                .nth(1)
-                .map(|s| s.parse())
+                .ok_or(PatternProblem::TooFewParameters)?.split_once('+').map(|x| x.1)
+                .map(str::parse)
                 .transpose()?,
             split.next().ok_or(PatternProblem::TooFewParameters)?.parse()?,
             split.next().ok_or(PatternProblem::TooFewParameters)?.parse()?,

src/params/patterns.rs

@@ -102,11 +102,8 @@ pub(crate) enum Token {
 
 #[cfg(feature = "hfs")]
 impl Token {
-    fn is_dh(&self) -> bool {
-        match *self {
-            Dh(_) => true,
-            _ => false,
-        }
+    fn is_dh(self) -> bool {
+        matches!(self, Dh(_))
     }
 }
 
@@ -250,6 +247,7 @@ impl HandshakeChoice {
 
     /// Whether the handshake choice includes the hfs modifier.
     #[cfg(feature = "hfs")]
+    #[must_use]
     pub fn is_hfs(&self) -> bool {
         self.modifiers.list.contains(&HandshakeModifier::Hfs)
     }
@@ -526,7 +524,7 @@ impl<'a> TryFrom<&'a HandshakeChoice> for HandshakeTokens {
 /// if `handshake` is invalid because of this. Otherwise it will return `()`.
 fn check_hfs_and_oneway_conflict(handshake: &HandshakeChoice) -> Result<(), Error> {
     if handshake.is_hfs() && handshake.pattern.is_oneway() {
-        return Err(PatternProblem::UnsupportedModifier.into());
+        Err(PatternProblem::UnsupportedModifier.into())
     } else {
         Ok(())
     }
@@ -560,9 +558,9 @@ fn apply_hfs_modifier(patterns: &mut Patterns) {
 
     // Add the e1 token
     let mut e1_insert_idx = None;
-    for msg in patterns.2.iter_mut() {
+    for msg in &mut patterns.2 {
         if let Some(e_idx) = msg.iter().position(|x| *x == Token::E) {
-            if let Some(dh_idx) = msg.iter().position(|x| x.is_dh()) {
+            if let Some(dh_idx) = msg.iter().copied().position(Token::is_dh) {
                 e1_insert_idx = Some(dh_idx + 1);
             } else {
                 e1_insert_idx = Some(e_idx + 1);
@@ -575,12 +573,12 @@ fn apply_hfs_modifier(patterns: &mut Patterns) {
     }
 
     // Add the ekem1 token
-    let mut ee_insert_idx = None;
-    for msg in patterns.2.iter_mut() {
+    let mut ekem1_insert_idx = None;
+    for msg in &mut patterns.2 {
         if let Some(ee_idx) = msg.iter().position(|x| *x == Token::Dh(Ee)) {
-            ee_insert_idx = Some(ee_idx + 1)
+            ekem1_insert_idx = Some(ee_idx + 1);
         }
-        if let Some(idx) = ee_insert_idx {
+        if let Some(idx) = ekem1_insert_idx {
             msg.insert(idx, Token::Ekem1);
             break;
         }
@@ -589,7 +587,7 @@ fn apply_hfs_modifier(patterns: &mut Patterns) {
     // This should not be possible, because the caller verified that the
     // HandshakePattern is not one-way.
     assert!(
-        !(e1_insert_idx.is_some() ^ ee_insert_idx.is_some()),
+        !(e1_insert_idx.is_some() ^ ekem1_insert_idx.is_some()),
         "handshake messages contain one of the ['e1', 'ekem1'] tokens, but not the other",
     );
 }

src/resolvers/default.rs

@@ -580,54 +580,46 @@ impl Kem for Kyber1024 {
         "Kyber1024"
     }
 
-    /// The length in bytes of a public key for this primitive.
     fn pub_len(&self) -> usize {
         kyber1024::public_key_bytes()
     }
 
-    /// The length in bytes the Kem cipherthext for this primitive.
     fn ciphertext_len(&self) -> usize {
         kyber1024::ciphertext_bytes()
     }
 
-    /// Shared secret length in bytes that this Kem encapsulates.
     fn shared_secret_len(&self) -> usize {
         kyber1024::shared_secret_bytes()
     }
 
-    /// Generate a new private key.
     fn generate(&mut self, _rng: &mut dyn Random) {
         // PQClean uses their own random generator
         let (pk, sk) = kyber1024::keypair();
         self.pubkey = pk;
         self.privkey = sk;
     }
 
-    /// Get the public key.
     fn pubkey(&self) -> &[u8] {
         self.pubkey.as_bytes()
     }
 
-    /// Generate a shared secret and encapsulate it using this Kem.
-    #[must_use]
     fn encapsulate(
         &self,
         pubkey: &[u8],
         shared_secret_out: &mut [u8],
         ciphertext_out: &mut [u8],
-    ) -> Result<(usize, usize), ()> {
-        let pubkey = kyber1024::PublicKey::from_bytes(pubkey).map_err(|_| ())?;
-        let (shared_secret, ciphertext) = kyber1024::encapsulate(&pubkey);
+    ) -> Result<(usize, usize), Error> {
+        let public_key = kyber1024::PublicKey::from_bytes(pubkey).map_err(|_| Error::Kem)?;
+        let (shared_secret, ciphertext) = kyber1024::encapsulate(&public_key);
         shared_secret_out.copy_from_slice(shared_secret.as_bytes());
         ciphertext_out.copy_from_slice(ciphertext.as_bytes());
         Ok((shared_secret.as_bytes().len(), ciphertext.as_bytes().len()))
     }
 
-    /// Decapsulate a ciphertext producing a shared secret.
-    #[must_use]
-    fn decapsulate(&self, ciphertext: &[u8], shared_secret_out: &mut [u8]) -> Result<usize, ()> {
-        let ciphertext = kyber1024::Ciphertext::from_bytes(ciphertext).map_err(|_| ())?;
-        let shared_secret = kyber1024::decapsulate(&ciphertext, &self.privkey);
+    fn decapsulate(&self, ciphertext: &[u8], shared_secret_out: &mut [u8]) -> Result<usize, Error> {
+        let kyber_ciphertext =
+            kyber1024::Ciphertext::from_bytes(ciphertext).map_err(|_| Error::Kem)?;
+        let shared_secret = kyber1024::decapsulate(&kyber_ciphertext, &self.privkey);
         shared_secret_out.copy_from_slice(shared_secret.as_bytes());
         Ok(shared_secret.as_bytes().len())
     }
@@ -934,7 +926,7 @@ mod tests {
     #[test]
     #[cfg(feature = "use-pqcrypto-kyber1024")]
     fn test_kyber1024() {
-        let mut rng = OsRng::default();
+        let mut rng = OsRng;
         let mut kem_1 = Kyber1024::default();
         let kem_2 = Kyber1024::default();
 
@@ -957,7 +949,7 @@ mod tests {
     #[test]
     #[cfg(feature = "use-pqcrypto-kyber1024")]
     fn test_kyber1024_fail() {
-        let mut rng = OsRng::default();
+        let mut rng = OsRng;
         let mut kem_1 = Kyber1024::default();
         let kem_2 = Kyber1024::default();
 

src/types.rs

@@ -183,15 +183,21 @@ pub trait Kem: Send + Sync {
     fn pubkey(&self) -> &[u8];
 
     /// Generate a shared secret and encapsulate it using this Kem.
-    #[must_use]
+    ///
+    /// # Errors
+    /// Returns `Error::Kem` if the public key is invalid.
+    #[must_use = "returned value includes needed length values for output slices"]
     fn encapsulate(
         &self,
         pubkey: &[u8],
         shared_secret_out: &mut [u8],
         ciphertext_out: &mut [u8],
-    ) -> Result<(usize, usize), ()>;
+    ) -> Result<(usize, usize), Error>;
 
     /// Decapsulate a ciphertext producing a shared secret.
-    #[must_use]
-    fn decapsulate(&self, ciphertext: &[u8], shared_secret_out: &mut [u8]) -> Result<usize, ()>;
+    ///
+    /// # Errors
+    /// Returns `Error::Kem` if the ciphertext is invalid.
+    #[must_use = "returned value includes needed length value for output slice"]
+    fn decapsulate(&self, ciphertext: &[u8], shared_secret_out: &mut [u8]) -> Result<usize, Error>;
 }

tests/general.rs

@@ -272,8 +272,8 @@ fn test_NNhfs_sanity_session() -> TestResult {
     let mut h_i = Builder::new(params.clone()).build_initiator()?;
     let mut h_r = Builder::new(params).build_responder()?;
 
-    let mut buffer_msg = [0u8; 4096];
-    let mut buffer_out = [0u8; 4096];
+    let mut buffer_msg = [0_u8; 4096];
+    let mut buffer_out = [0_u8; 4096];
     let len = h_i.write_message(b"abc", &mut buffer_msg)?;
     h_r.read_message(&buffer_msg[..len], &mut buffer_out)?;