cargo clippy pedantic fixes

This is in anticipation of adding better automated warnings for bugs like the medium-severity logic flaw that might have been caught by more aggressive linting.
mcginty · Jan 25, 2024 · 2c63224 · 2c63224
1 parent 752c071
commit 2c63224
Show file tree

Hide file tree

Showing 17 changed files with 171 additions and 119 deletions.
diff --git a/benches/benches.rs b/benches/benches.rs
@@ -2,7 +2,7 @@
 extern crate criterion;
 
 use criterion::{Criterion, Throughput};
-use snow::{params::*, *};
+use snow::{params::NoiseParams, Builder};
 
 const MSG_SIZE: usize = 4096;
 
@@ -14,7 +14,7 @@ fn benchmarks(c: &mut Criterion) {
             Builder::new("Noise_NN_25519_ChaChaPoly_SHA256".parse().unwrap())
                 .build_initiator()
                 .unwrap();
-        })
+        });
     });
 
     builder_group.bench_function("withkey", |b| {
@@ -54,7 +54,7 @@ fn benchmarks(c: &mut Criterion) {
             h_i.read_message(&buffer_msg[..len], &mut buffer_out).unwrap();
             let len = h_i.write_message(&[0u8; 0], &mut buffer_msg).unwrap();
             h_r.read_message(&buffer_msg[..len], &mut buffer_out).unwrap();
-        })
+        });
     });
 
     handshake_group.bench_function("nn", |b| {
@@ -71,7 +71,7 @@ fn benchmarks(c: &mut Criterion) {
             h_r.read_message(&buffer_msg[..len], &mut buffer_out).unwrap();
             let len = h_r.write_message(&[0u8; 0], &mut buffer_msg).unwrap();
             h_i.read_message(&buffer_msg[..len], &mut buffer_out).unwrap();
-        })
+        });
     });
     handshake_group.finish();
 
@@ -99,7 +99,7 @@ fn benchmarks(c: &mut Criterion) {
             b.iter(move || {
                 let len = h_i.write_message(&buffer_msg[..MSG_SIZE], &mut buffer_out).unwrap();
                 let _ = h_r.read_message(&buffer_out[..len], &mut buffer_msg).unwrap();
-            })
+            });
         });
     }
 
@@ -124,7 +124,7 @@ fn benchmarks(c: &mut Criterion) {
         b.iter(move || {
             let len = h_i.write_message(&buffer_msg[..MSG_SIZE], &mut buffer_out).unwrap();
             let _ = h_r.read_message(&buffer_out[..len], &mut buffer_msg).unwrap();
-        })
+        });
     });
 }
 

diff --git a/build.rs b/build.rs
@@ -6,7 +6,7 @@ fn main() {
         println!(
             "cargo:warning=Use of the sodiumoxide backend is deprecated, as it is no longer \
              maintained; please consider switching to another resolver."
-        )
+        );
     }
     if version_meta().unwrap().channel == Channel::Nightly {
         println!("cargo:rustc-cfg=feature=\"nightly\"");

diff --git a/examples/simple.rs b/examples/simple.rs
@@ -24,7 +24,7 @@ lazy_static! {
 #[cfg(any(feature = "default-resolver", feature = "ring-accelerated"))]
 fn main() {
     let server_mode =
-        std::env::args().next_back().map(|arg| arg == "-s" || arg == "--server").unwrap_or(true);
+        std::env::args().next_back().map_or(true, |arg| arg == "-s" || arg == "--server");
 
     if server_mode {
         run_server();

diff --git a/src/builder.rs b/src/builder.rs
@@ -76,6 +76,7 @@ impl<'builder> Builder<'builder> {
         feature = "default-resolver",
         not(any(feature = "ring-accelerated", feature = "libsodium-accelerated"))
     ))]
+    #[must_use]
     pub fn new(params: NoiseParams) -> Self {
         use crate::resolvers::DefaultResolver;
 
@@ -105,11 +106,17 @@ impl<'builder> Builder<'builder> {
     }
 
     /// Create a Builder with a custom crypto resolver.
+    #[must_use]
     pub fn with_resolver(params: NoiseParams, resolver: BoxedCryptoResolver) -> Self {
         Builder { params, resolver, s: None, e_fixed: None, rs: None, plog: None, psks: [None; 10] }
     }
 
     /// Specify a PSK (only used with `NoisePSK` base parameter)
+    ///
+    /// # Errors
+    /// * `InitError(InitStage::ValidatePskPosition)` if the location is a number larger than
+    ///   allowed.
+    /// * `InitError(InitStage::ParameterOverwrite)` if this method has been called previously.
     pub fn psk(mut self, location: u8, key: &'builder [u8; PSKLEN]) -> Result<Self, Error> {
         let location = location as usize;
         if location >= MAX_PSKS {
@@ -125,6 +132,9 @@ impl<'builder> Builder<'builder> {
     /// Your static private key (can be generated with [`generate_keypair()`]).
     ///
     /// [`generate_keypair()`]: #method.generate_keypair
+    ///
+    /// # Errors
+    /// * `InitError(InitStage::ParameterOverwrite)` if this method has been called previously.
     pub fn local_private_key(mut self, key: &'builder [u8]) -> Result<Self, Error> {
         if self.s.is_some() {
             Err(InitStage::ParameterOverwrite.into())
@@ -135,6 +145,7 @@ impl<'builder> Builder<'builder> {
     }
 
     #[doc(hidden)]
+    #[must_use]
     pub fn fixed_ephemeral_key_for_testing_only(mut self, key: &'builder [u8]) -> Self {
         self.e_fixed = Some(key);
         self
@@ -143,6 +154,9 @@ impl<'builder> Builder<'builder> {
     /// Arbitrary data to be hashed in to the handshake hash value.
     ///
     /// This may only be set once
+    ///
+    /// # Errors
+    /// * `InitError(InitStage::ParameterOverwrite)` if this method has been called previously.
     pub fn prologue(mut self, key: &'builder [u8]) -> Result<Self, Error> {
         if self.plog.is_some() {
             Err(InitStage::ParameterOverwrite.into())
@@ -153,6 +167,9 @@ impl<'builder> Builder<'builder> {
     }
 
     /// The responder's static public key.
+    ///
+    /// # Errors
+    /// * `InitError(InitStage::ParameterOverwrite)` if this method has been called previously.
     pub fn remote_public_key(mut self, pub_key: &'builder [u8]) -> Result<Self, Error> {
         if self.rs.is_some() {
             Err(InitStage::ParameterOverwrite.into())
@@ -164,6 +181,10 @@ impl<'builder> Builder<'builder> {
 
     // TODO: performance issue w/ creating a new RNG and DH instance per call.
     /// Generate a new asymmetric keypair (for use as a static key).
+    ///
+    /// # Errors
+    /// * `InitError(InitStage::GetRngImpl)` if the RNG implementation failed to resolve.
+    /// * `InitError(InitStage::GetDhImpl)` if the DH implementation failed to resolve.
     pub fn generate_keypair(&self) -> Result<Keypair, Error> {
         let mut rng = self.resolver.resolve_rng().ok_or(InitStage::GetRngImpl)?;
         let mut dh = self.resolver.resolve_dh(&self.params.dh).ok_or(InitStage::GetDhImpl)?;
@@ -178,11 +199,19 @@ impl<'builder> Builder<'builder> {
     }
 
     /// Build a [`HandshakeState`] for the side who will initiate the handshake (send the first message)
+    ///
+    /// # Errors
+    /// * `InitError(InitStage::GetRngImpl)` if the RNG implementation failed to resolve.
+    /// * `InitError(InitStage::GetDhImpl)` if the DH implementation failed to resolve.
     pub fn build_initiator(self) -> Result<HandshakeState, Error> {
         self.build(true)
     }
 
     /// Build a [`HandshakeState`] for the side who will be responder (receive the first message)
+    ///
+    /// # Errors
+    /// An `InitError(InitStage)` variant will be returned for various issues in the building of a
+    /// usable `HandshakeState`. See `InitStage` for further details.
     pub fn build_responder(self) -> Result<HandshakeState, Error> {
         self.build(false)
     }
@@ -256,7 +285,7 @@ impl<'builder> Builder<'builder> {
             re,
             initiator,
             self.params,
-            psks,
+            &psks,
             self.plog.unwrap_or(&[]),
             cipherstates,
         )?;
@@ -265,6 +294,7 @@ impl<'builder> Builder<'builder> {
     }
 
     #[cfg(not(feature = "hfs"))]
+    #[allow(clippy::unnecessary_wraps)]
     fn resolve_kem(_: Box<dyn CryptoResolver>, _: &mut HandshakeState) -> Result<(), Error> {
         // HFS is disabled, return nothing
         Ok(())
@@ -316,9 +346,7 @@ mod tests {
         let params: ::std::result::Result<NoiseParams, _> =
             "Noise_NK_25519_ChaChaPoly_BLAH256".parse();
 
-        if params.is_ok() {
-            panic!("NoiseParams should have failed");
-        }
+        assert!(params.is_err(), "NoiseParams should have failed");
     }
 
     #[test]
@@ -328,20 +356,18 @@ mod tests {
             .local_private_key(&[0u8; 32])?
             .build_initiator(); // missing remote key, should result in Err
 
-        if noise.is_ok() {
-            panic!("builder should have failed on build");
-        }
+        assert!(noise.is_err(), "builder should have failed on build");
         Ok(())
     }
 
     #[test]
     fn test_builder_param_overwrite() -> TestResult {
         fn build_builder<'a>() -> Result<Builder<'a>, Error> {
-            Ok(Builder::new("Noise_NNpsk0_25519_ChaChaPoly_SHA256".parse()?)
+            Builder::new("Noise_NNpsk0_25519_ChaChaPoly_SHA256".parse()?)
                 .prologue(&[2u8; 10])?
                 .psk(0, &[0u8; 32])?
                 .local_private_key(&[0u8; 32])?
-                .remote_public_key(&[1u8; 32])?)
+                .remote_public_key(&[1u8; 32])
         }
 
         assert_eq!(

diff --git a/src/cipherstate.rs b/src/cipherstate.rs
@@ -104,19 +104,19 @@ impl CipherStates {
     }
 
     pub fn rekey_initiator(&mut self) {
-        self.0.rekey()
+        self.0.rekey();
     }
 
     pub fn rekey_initiator_manually(&mut self, key: &[u8; CIPHERKEYLEN]) {
-        self.0.rekey_manually(key)
+        self.0.rekey_manually(key);
     }
 
     pub fn rekey_responder(&mut self) {
-        self.1.rekey()
+        self.1.rekey();
     }
 
     pub fn rekey_responder_manually(&mut self, key: &[u8; CIPHERKEYLEN]) {
-        self.1.rekey_manually(key)
+        self.1.rekey_manually(key);
     }
 }
 
@@ -171,7 +171,7 @@ impl StatelessCipherState {
     }
 
     pub fn rekey(&mut self) {
-        self.cipher.rekey()
+        self.cipher.rekey();
     }
 
     pub fn rekey_manually(&mut self, key: &[u8; CIPHERKEYLEN]) {
@@ -208,18 +208,18 @@ impl From<CipherStates> for StatelessCipherStates {
 
 impl StatelessCipherStates {
     pub fn rekey_initiator(&mut self) {
-        self.0.rekey()
+        self.0.rekey();
     }
 
     pub fn rekey_initiator_manually(&mut self, key: &[u8; CIPHERKEYLEN]) {
-        self.0.rekey_manually(key)
+        self.0.rekey_manually(key);
     }
 
     pub fn rekey_responder(&mut self) {
-        self.1.rekey()
+        self.1.rekey();
     }
 
     pub fn rekey_responder_manually(&mut self, key: &[u8; CIPHERKEYLEN]) {
-        self.1.rekey_manually(key)
+        self.1.rekey_manually(key);
     }
 }
diff --git a/src/error.rs b/src/error.rs
@@ -164,14 +164,14 @@ impl From<StateProblem> for Error {
 impl fmt::Display for Error {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
-            Error::Pattern(reason) => write!(f, "pattern error: {:?}", reason),
+            Error::Pattern(reason) => write!(f, "pattern error: {reason:?}"),
             Error::Init(reason) => {
-                write!(f, "initialization error: {:?}", reason)
+                write!(f, "initialization error: {reason:?}")
             },
             Error::Prereq(reason) => {
-                write!(f, "prerequisite error: {:?}", reason)
+                write!(f, "prerequisite error: {reason:?}")
             },
-            Error::State(reason) => write!(f, "state error: {:?}", reason),
+            Error::State(reason) => write!(f, "state error: {reason:?}"),
             Error::Input => write!(f, "input error"),
             Error::Dh => write!(f, "diffie-hellman error"),
             Error::Decrypt => write!(f, "decrypt error"),