From 2c63224f3327858e10c9235c3031b67508918ae2 Mon Sep 17 00:00:00 2001 From: Jake McGinty Date: Thu, 25 Jan 2024 14:33:57 -0800 Subject: [PATCH] `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. --- benches/benches.rs | 12 ++++---- build.rs | 2 +- examples/simple.rs | 2 +- src/builder.rs | 44 +++++++++++++++++++++++------ src/cipherstate.rs | 18 ++++++------ src/error.rs | 8 +++--- src/handshakestate.rs | 49 +++++++++++++++++++-------------- src/params/mod.rs | 9 ++++-- src/params/patterns.rs | 26 +++++++++++------ src/resolvers/default.rs | 49 +++++++++++++++++---------------- src/resolvers/libsodium.rs | 1 + src/resolvers/mod.rs | 10 +++---- src/resolvers/ring.rs | 1 + src/stateless_transportstate.rs | 16 +++++------ src/transportstate.rs | 20 +++++++------- src/types.rs | 6 ++++ tests/general.rs | 17 +++++------- 17 files changed, 171 insertions(+), 119 deletions(-) diff --git a/benches/benches.rs b/benches/benches.rs index aaca6376..c1dfae4d 100644 --- 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 index a9a6c565..b77c058b 100644 --- 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 index d965432f..3eff1b3f 100644 --- 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 index bd2af82c..07f8aafa 100644 --- 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 { 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 { 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 { 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 { 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 { 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 { 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 { 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, _: &mut HandshakeState) -> Result<(), Error> { // HFS is disabled, return nothing Ok(()) @@ -316,9 +346,7 @@ mod tests { let params: ::std::result::Result = "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, 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 index 550dbe76..fa29dc45 100644 --- 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 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 index 634aef16..8734ec4f 100644 --- a/src/error.rs +++ b/src/error.rs @@ -164,14 +164,14 @@ impl From 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"), diff --git a/src/handshakestate.rs b/src/handshakestate.rs index 8e7185e8..671b55dc 100644 --- a/src/handshakestate.rs +++ b/src/handshakestate.rs @@ -60,7 +60,7 @@ impl HandshakeState { re: Toggle<[u8; MAXDHLEN]>, initiator: bool, params: NoiseParams, - psks: [Option<[u8; PSKLEN]>; 10], + psks: &[Option<[u8; PSKLEN]>; 10], prologue: &[u8], cipherstates: CipherStates, ) -> Result { @@ -140,7 +140,7 @@ impl HandshakeState { re, initiator, params, - psks, + psks: *psks, #[cfg(feature = "hfs")] kem: None, #[cfg(feature = "hfs")] @@ -160,7 +160,7 @@ impl HandshakeState { self.kem = Some(kem); } - fn dh(&self, token: &DhToken) -> Result<[u8; MAXDHLEN], Error> { + fn dh(&self, token: DhToken) -> Result<[u8; MAXDHLEN], Error> { let mut dh_out = [0u8; MAXDHLEN]; let (dh, key) = match (token, self.is_initiator()) { (DhToken::Ee, _) => (&self.e, &self.re), @@ -190,6 +190,7 @@ impl HandshakeState { /// /// assert!(session.was_write_payload_encrypted()); /// ``` + #[must_use] pub fn was_write_payload_encrypted(&self) -> bool { self.symmetricstate.has_key() } @@ -226,8 +227,8 @@ impl HandshakeState { } let mut byte_index = 0; - for token in self.message_patterns[self.pattern_position].iter() { - match token { + for token in &self.message_patterns[self.pattern_position] { + match *token { Token::E => { if byte_index + self.e.pub_len() > message.len() { return Err(Error::Input); @@ -256,7 +257,7 @@ impl HandshakeState { .symmetricstate .encrypt_and_mix_hash(self.s.pubkey(), &mut message[byte_index..])?; }, - Token::Psk(n) => match self.psks[*n as usize] { + Token::Psk(n) => match self.psks[n as usize] { Some(psk) => { self.symmetricstate.mix_key_and_hash(&psk); }, @@ -357,8 +358,8 @@ impl HandshakeState { let dh_len = self.dh_len(); let mut ptr = message; - for token in self.message_patterns[self.pattern_position].iter() { - match token { + for token in &self.message_patterns[self.pattern_position] { + match *token { Token::E => { if ptr.len() < dh_len { return Err(Error::Input); @@ -390,7 +391,7 @@ impl HandshakeState { self.symmetricstate.decrypt_and_mix_hash(data, &mut self.rs[..dh_len])?; self.rs.enable(); }, - Token::Psk(n) => match self.psks[*n as usize] { + Token::Psk(n) => match self.psks[n as usize] { Some(psk) => { self.symmetricstate.mix_key_and_hash(&psk); }, @@ -405,11 +406,8 @@ impl HandshakeState { #[cfg(feature = "hfs")] Token::E1 => { let kem = self.kem.as_ref().ok_or(Error::Kem)?; - let read_len = if self.symmetricstate.has_key() { - kem.pub_len() + TAGLEN - } else { - kem.pub_len() - }; + let read_len = + kem.pub_len() + if self.symmetricstate.has_key() { TAGLEN } else { 0 }; if ptr.len() < read_len { return Err(Error::Input); } @@ -422,11 +420,8 @@ impl HandshakeState { #[cfg(feature = "hfs")] Token::Ekem1 => { let kem = self.kem.as_ref().unwrap(); - let read_len = if self.symmetricstate.has_key() { - kem.ciphertext_len() + TAGLEN - } else { - kem.ciphertext_len() - }; + let read_len = kem.ciphertext_len() + + if self.symmetricstate.has_key() { TAGLEN } else { 0 }; if ptr.len() < read_len { return Err(Error::Input); } @@ -446,8 +441,7 @@ impl HandshakeState { if last { self.symmetricstate.split(&mut self.cipherstates.0, &mut self.cipherstates.1); } - let payload_len = - if self.symmetricstate.has_key() { ptr.len() - TAGLEN } else { ptr.len() }; + let payload_len = ptr.len() - if self.symmetricstate.has_key() { TAGLEN } else { 0 }; Ok(payload_len) } @@ -476,6 +470,7 @@ impl HandshakeState { /// doesn't necessitate a remote static key, *or* if the remote /// static key is not yet known (as can be the case in the `XX` /// pattern, for example). + #[must_use] pub fn get_remote_static(&self) -> Option<&[u8]> { self.rs.get().map(|rs| &rs[..self.dh_len()]) } @@ -483,21 +478,25 @@ impl HandshakeState { /// Get the handshake hash. /// /// Returns a slice of length `Hasher.hash_len()` (i.e. HASHLEN for the chosen Hash function). + #[must_use] pub fn get_handshake_hash(&self) -> &[u8] { self.symmetricstate.handshake_hash() } /// Check if this session was started with the "initiator" role. + #[must_use] pub fn is_initiator(&self) -> bool { self.initiator } /// Check if the handshake is finished and `into_transport_mode()` can now be called. + #[must_use] pub fn is_handshake_finished(&self) -> bool { self.pattern_position == self.message_patterns.len() } /// Check whether it is our turn to send in the handshake state machine + #[must_use] pub fn is_my_turn(&self) -> bool { self.my_turn } @@ -514,11 +513,19 @@ impl HandshakeState { } /// Convert this `HandshakeState` into a `TransportState` with an internally stored nonce. + /// + /// # Errors + /// A `State(StateProblem)` variant will be returned for various issues in the building of a + /// usable `TransportState`. See `StateProblem` for further details. pub fn into_transport_mode(self) -> Result { self.try_into() } /// Convert this `HandshakeState` into a `StatelessTransportState` without an internally stored nonce. + /// + /// # Errors + /// A `State(StateProblem)` variant will be returned for various issues in the building of a + /// usable `StatelessTransportState`. See `StateProblem` for further details. pub fn into_stateless_transport_mode(self) -> Result { self.try_into() } diff --git a/src/params/mod.rs b/src/params/mod.rs index 3521665f..f8d7463d 100644 --- a/src/params/mod.rs +++ b/src/params/mod.rs @@ -1,3 +1,6 @@ +#![allow(clippy::match_on_vec_items)] +#![allow(clippy::enum_glob_use)] + //! All structures related to Noise parameter definitions (cryptographic primitive choices, protocol //! patterns/names) @@ -146,6 +149,7 @@ impl FromStr for KemChoice { /// let params: NoiseParams = "Noise_XX_25519_AESGCM_SHA256".parse().unwrap(); /// ``` #[derive(PartialEq, Clone, Debug)] +#[allow(clippy::module_name_repetitions)] pub struct NoiseParams { /// The full pattern string. pub name: String, @@ -166,7 +170,8 @@ pub struct NoiseParams { impl NoiseParams { #[cfg(not(feature = "hfs"))] - /// Construct a new NoiseParams via specifying enums directly. + /// Construct a new `NoiseParams` via specifying enums directly. + #[must_use] pub fn new( name: String, base: BaseChoice, @@ -300,7 +305,7 @@ mod tests { let mods = p.handshake.modifiers.list; match (mods[0], mods[1], mods[2]) { (Psk(0), Psk(1), Psk(2)) => {}, - _ => panic!("modifiers weren't as expected! actual: {:?}", mods), + _ => panic!("modifiers weren't as expected! actual: {mods:?}"), } } diff --git a/src/params/patterns.rs b/src/params/patterns.rs index b5457840..ffff2414 100644 --- a/src/params/patterns.rs +++ b/src/params/patterns.rs @@ -1,3 +1,5 @@ +#![allow(clippy::enum_glob_use)] + use crate::error::{Error, PatternProblem}; use std::{convert::TryFrom, str::FromStr}; @@ -53,7 +55,7 @@ macro_rules! pattern_enum { impl $name { /// The equivalent of the `ToString` trait, but for `&'static str`. - pub fn as_str(self) -> &'static str { + #[must_use] pub fn as_str(self) -> &'static str { use self::$name::*; match self { $( @@ -71,7 +73,7 @@ macro_rules! pattern_enum { /// The tokens which describe patterns involving DH calculations. /// -/// See: https://noiseprotocol.org/noise.html#handshake-patterns +/// See: #[derive(Copy, Clone, PartialEq, Debug)] pub(crate) enum DhToken { Ee, @@ -82,7 +84,7 @@ pub(crate) enum DhToken { /// The tokens which describe message patterns. /// -/// See: https://noiseprotocol.org/noise.html#handshake-patterns +/// See: #[derive(Copy, Clone, PartialEq, Debug)] pub(crate) enum Token { E, @@ -124,11 +126,13 @@ impl HandshakePattern { /// If the protocol is one-way only /// /// See: + #[must_use] pub fn is_oneway(self) -> bool { matches!(self, N | X | K) } /// Whether this pattern requires a long-term static key. + #[must_use] pub fn needs_local_static_key(self, initiator: bool) -> bool { if initiator { !matches!(self, N | NN | NK | NX | NK1 | NX1) @@ -139,7 +143,7 @@ impl HandshakePattern { /// Whether this pattern demands a remote public key pre-message. #[rustfmt::skip] - pub fn need_known_remote_pubkey(self, initiator: bool) -> bool { + #[must_use] pub fn need_known_remote_pubkey(self, initiator: bool) -> bool { if initiator { matches!( self, @@ -204,9 +208,8 @@ impl FromStr for HandshakeModifierList { let modifier: HandshakeModifier = modifier_name.parse()?; if modifiers.contains(&modifier) { return Err(Error::Pattern(PatternProblem::DuplicateModifier)); - } else { - modifiers.push(modifier); } + modifiers.push(modifier); } Ok(HandshakeModifierList { list: modifiers }) } @@ -226,6 +229,7 @@ pub struct HandshakeChoice { impl HandshakeChoice { /// Whether the handshake choice includes one or more PSK modifiers. + #[must_use] pub fn is_psk(&self) -> bool { for modifier in &self.modifiers.list { if let HandshakeModifier::Psk(_) = *modifier { @@ -236,6 +240,7 @@ impl HandshakeChoice { } /// Whether the handshake choice includes the fallback modifier. + #[must_use] pub fn is_fallback(&self) -> bool { self.modifiers.list.contains(&HandshakeModifier::Fallback) } @@ -246,7 +251,7 @@ impl HandshakeChoice { self.modifiers.list.contains(&HandshakeModifier::Hfs) } - /// Parse and split a base HandshakePattern from its optional modifiers + /// Parse and split a base `HandshakePattern` from its optional modifiers fn parse_pattern_and_modifier(s: &str) -> Result<(HandshakePattern, &str), Error> { for i in (1..=4).rev() { if s.len() > i - 1 && s.is_char_boundary(i) { @@ -276,7 +281,7 @@ pub(crate) type MessagePatterns = Vec>; /// The defined token patterns for a given handshake. /// -/// See: https://noiseprotocol.org/noise.html#handshake-patterns +/// See: #[derive(Debug)] pub(crate) struct HandshakeTokens { pub premsg_pattern_i: PremessagePatterns, @@ -291,7 +296,10 @@ type Patterns = (PremessagePatterns, PremessagePatterns, MessagePatterns); impl<'a> TryFrom<&'a HandshakeChoice> for HandshakeTokens { type Error = Error; + // We're going to ignore the clippy warnings here about this function being too long because + // it's essentially a lookup table and not problematic complex logic. #[allow(clippy::cognitive_complexity)] + #[allow(clippy::too_many_lines)] fn try_from(handshake: &'a HandshakeChoice) -> Result { // Hfs cannot be combined with one-way handshake patterns #[cfg(feature = "hfs")] @@ -491,7 +499,7 @@ impl<'a> TryFrom<&'a HandshakeChoice> for HandshakeTokens { ), }; - for modifier in handshake.modifiers.list.iter() { + for modifier in &handshake.modifiers.list { match modifier { HandshakeModifier::Psk(n) => apply_psk_modifier(&mut patterns, *n)?, #[cfg(feature = "hfs")] diff --git a/src/resolvers/default.rs b/src/resolvers/default.rs index 23475db9..93ed5b6a 100644 --- a/src/resolvers/default.rs +++ b/src/resolvers/default.rs @@ -25,6 +25,7 @@ use crate::{ /// The default resolver provided by snow. This resolver is designed to /// support as many of the Noise spec primitives as possible with /// pure-Rust (or nearly pure-Rust) implementations. +#[allow(clippy::module_name_repetitions)] #[derive(Default)] pub struct DefaultResolver; @@ -36,7 +37,7 @@ impl CryptoResolver for DefaultResolver { fn resolve_dh(&self, choice: &DHChoice) -> Option> { match *choice { DHChoice::Curve25519 => Some(Box::::default()), - _ => None, + DHChoice::Curve448 => None, } } @@ -79,7 +80,7 @@ struct CipherAesGcm { key: [u8; CIPHERKEYLEN], } -/// Wraps `chacha20_poly1305_aead`'s ChaCha20Poly1305 implementation. +/// Wraps `chacha20_poly1305_aead`'s `ChaCha20Poly1305` implementation. #[derive(Default)] struct CipherChaChaPoly { key: [u8; CIPHERKEYLEN], @@ -180,7 +181,7 @@ impl Cipher for CipherAesGcm { } fn set(&mut self, key: &[u8; CIPHERKEYLEN]) { - copy_slices!(key, &mut self.key) + copy_slices!(key, &mut self.key); } fn encrypt(&self, nonce: u64, authtext: &[u8], plaintext: &[u8], out: &mut [u8]) -> usize { @@ -222,7 +223,7 @@ impl Cipher for CipherAesGcm { &mut out[..message_len], ciphertext[message_len..].into(), ) - .map(|_| message_len) + .map(|()| message_len) .map_err(|_| Error::Decrypt) } } @@ -359,7 +360,7 @@ impl Hash for HashSHA256 { fn result(&mut self, out: &mut [u8]) { let hash = self.hasher.finalize_reset(); - copy_slices!(hash.as_slice(), out) + copy_slices!(hash.as_slice(), out); } } @@ -392,7 +393,7 @@ impl Hash for HashSHA512 { fn result(&mut self, out: &mut [u8]) { let hash = self.hasher.finalize_reset(); - copy_slices!(hash.as_slice(), out) + copy_slices!(hash.as_slice(), out); } } @@ -527,7 +528,7 @@ mod tests { #[test] fn test_sha256() { let mut output = [0u8; 32]; - let mut hasher: HashSHA256 = Default::default(); + let mut hasher = HashSHA256::default(); hasher.input(b"abc"); hasher.result(&mut output); assert!( @@ -544,7 +545,7 @@ mod tests { ) .unwrap(); let mut output1 = [0u8; 32]; - let mut hasher: HashSHA256 = Default::default(); + let mut hasher = HashSHA256::default(); hasher.hmac(&key, &data, &mut output1); assert!( hex::encode(output1) @@ -552,7 +553,7 @@ mod tests { ); let mut output2 = [0u8; 64]; - let mut hasher: HashSHA512 = Default::default(); + let mut hasher = HashSHA512::default(); hasher.hmac(&key, &data, &mut output2); assert!( hex::encode(output2) @@ -567,7 +568,7 @@ mod tests { fn test_blake2b() { // BLAKE2b test - draft-saarinen-blake2-06 let mut output = [0u8; 64]; - let mut hasher: HashBLAKE2b = Default::default(); + let mut hasher = HashBLAKE2b::default(); hasher.input(b"abc"); hasher.result(&mut output); assert!( @@ -583,7 +584,7 @@ mod tests { fn test_blake2s() { // BLAKE2s test - draft-saarinen-blake2-06 let mut output = [0u8; 32]; - let mut hasher: HashBLAKE2s = Default::default(); + let mut hasher = HashBLAKE2s::default(); hasher.input(b"abc"); hasher.result(&mut output); assert!( @@ -596,7 +597,7 @@ mod tests { #[test] fn test_curve25519() { // Curve25519 test - draft-curves-10 - let mut keypair: Dh25519 = Default::default(); + let mut keypair = Dh25519::default(); let scalar = Vec::::from_hex("a546e36bf0527c9d3b16154b82465edd62144c0ac1fc5a18506a2244ba449ac4") .unwrap(); @@ -621,13 +622,13 @@ mod tests { let plaintext = [0u8; 0]; let authtext = [0u8; 0]; let mut ciphertext = [0u8; 16]; - let mut cipher1: CipherAesGcm = Default::default(); + let mut cipher1 = CipherAesGcm::default(); cipher1.set(&key); cipher1.encrypt(nonce, &authtext, &plaintext, &mut ciphertext); assert!(hex::encode(ciphertext) == "530f8afbc74536b9a963b4f1c4cb738b"); let mut resulttext = [0u8; 1]; - let mut cipher2: CipherAesGcm = Default::default(); + let mut cipher2 = CipherAesGcm::default(); cipher2.set(&key); cipher2.decrypt(nonce, &authtext, &ciphertext, &mut resulttext).unwrap(); assert!(resulttext[0] == 0); @@ -637,7 +638,7 @@ mod tests { // Test Case 14 let plaintext2 = [0u8; 16]; let mut ciphertext2 = [0u8; 32]; - let mut cipher3: CipherAesGcm = Default::default(); + let mut cipher3 = CipherAesGcm::default(); cipher3.set(&key); cipher3.encrypt(nonce, &authtext, &plaintext2, &mut ciphertext2); assert!( @@ -646,7 +647,7 @@ mod tests { ); let mut resulttext2 = [1u8; 16]; - let mut cipher4: CipherAesGcm = Default::default(); + let mut cipher4 = CipherAesGcm::default(); cipher4.set(&key); cipher4.decrypt(nonce, &authtext, &ciphertext2, &mut resulttext2).unwrap(); assert!(plaintext2 == resulttext2); @@ -662,12 +663,12 @@ mod tests { let plaintext = [0u8; 0]; let authtext = [0u8; 0]; let mut ciphertext = [0u8; 16]; - let mut cipher1: CipherChaChaPoly = Default::default(); + let mut cipher1 = CipherChaChaPoly::default(); cipher1.set(&key); cipher1.encrypt(nonce, &authtext, &plaintext, &mut ciphertext); let mut resulttext = [0u8; 1]; - let mut cipher2: CipherChaChaPoly = Default::default(); + let mut cipher2 = CipherChaChaPoly::default(); cipher2.set(&key); cipher2.decrypt(nonce, &authtext, &ciphertext, &mut resulttext).unwrap(); assert!(resulttext[0] == 0); @@ -683,12 +684,12 @@ mod tests { let plaintext = [0x34u8; 117]; let authtext = [0u8; 0]; let mut ciphertext = [0u8; 133]; - let mut cipher1: CipherChaChaPoly = Default::default(); + let mut cipher1 = CipherChaChaPoly::default(); cipher1.set(&key); cipher1.encrypt(nonce, &authtext, &plaintext, &mut ciphertext); let mut resulttext = [0u8; 117]; - let mut cipher2: CipherChaChaPoly = Default::default(); + let mut cipher2 = CipherChaChaPoly::default(); cipher2.set(&key); cipher2.decrypt(nonce, &authtext, &ciphertext, &mut resulttext).unwrap(); assert!(hex::encode(resulttext) == hex::encode(plaintext)); @@ -703,12 +704,12 @@ mod tests { let plaintext = [0x34u8; 117]; let authtext = [0u8; 0]; let mut ciphertext = [0u8; 133]; - let mut cipher1: CipherXChaChaPoly = Default::default(); + let mut cipher1 = CipherXChaChaPoly::default(); cipher1.set(&key); cipher1.encrypt(nonce, &authtext, &plaintext, &mut ciphertext); let mut resulttext = [0u8; 117]; - let mut cipher2: CipherXChaChaPoly = Default::default(); + let mut cipher2 = CipherXChaChaPoly::default(); cipher2.set(&key); cipher2.decrypt(nonce, &authtext, &ciphertext, &mut resulttext).unwrap(); assert!(hex::encode(resulttext.to_vec()) == hex::encode(plaintext.to_vec())); @@ -722,7 +723,7 @@ mod tests { 473917c1402b80099dca5cbc207075c0", ) .unwrap(); - let nonce = 0x0807060504030201u64; + let nonce = 0x0807_0605_0403_0201_u64; let ciphertext = Vec::::from_hex( "64a0861575861af460f062c79be643bd\ 5e805cfd345cf389f108670ac76c8cb2\ @@ -750,7 +751,7 @@ mod tests { copy_slices!(&ciphertext, &mut combined_text); copy_slices!(&tag[0..TAGLEN], &mut combined_text[ciphertext.len()..]); - let mut cipher: CipherChaChaPoly = Default::default(); + let mut cipher = CipherChaChaPoly::default(); cipher.set(&key); cipher .decrypt( diff --git a/src/resolvers/libsodium.rs b/src/resolvers/libsodium.rs index 39c01fc7..b0c0117b 100644 --- a/src/resolvers/libsodium.rs +++ b/src/resolvers/libsodium.rs @@ -19,6 +19,7 @@ use sodiumoxide::crypto::{ /// A resolver that uses [libsodium](https://github.com/jedisct1/libsodium) /// via [sodiumoxide](https://crates.io/crates/sodiumoxide). +#[allow(clippy::module_name_repetitions)] #[derive(Default)] pub struct SodiumResolver; diff --git a/src/resolvers/mod.rs b/src/resolvers/mod.rs index 3fcbe26e..e6c15dd5 100644 --- a/src/resolvers/mod.rs +++ b/src/resolvers/mod.rs @@ -26,7 +26,7 @@ pub use self::libsodium::SodiumResolver; #[cfg(feature = "ring-resolver")] pub use self::ring::RingResolver; -/// Boxed CryptoResolver +/// Boxed `CryptoResolver` pub type BoxedCryptoResolver = Box; /// An object that resolves the providers of Noise crypto choices @@ -34,13 +34,13 @@ pub trait CryptoResolver { /// Provide an implementation of the Random trait or None if none available. fn resolve_rng(&self) -> Option>; - /// Provide an implementation of the Dh trait for the given DHChoice or None if unavailable. + /// Provide an implementation of the Dh trait for the given `DHChoice` or None if unavailable. fn resolve_dh(&self, choice: &DHChoice) -> Option>; - /// Provide an implementation of the Hash trait for the given HashChoice or None if unavailable. + /// Provide an implementation of the Hash trait for the given `HashChoice` or None if unavailable. fn resolve_hash(&self, choice: &HashChoice) -> Option>; - /// Provide an implementation of the Cipher trait for the given CipherChoice or None if unavailable. + /// Provide an implementation of the Cipher trait for the given `CipherChoice` or None if unavailable. fn resolve_cipher(&self, choice: &CipherChoice) -> Option>; /// Provide an implementation of the Kem trait for the given KemChoice or None if unavailable @@ -60,7 +60,7 @@ pub struct FallbackResolver { impl FallbackResolver { /// Create a new `FallbackResolver` that holds the primary and secondary resolver. - pub fn new(preferred: BoxedCryptoResolver, fallback: BoxedCryptoResolver) -> Self { + #[must_use] pub fn new(preferred: BoxedCryptoResolver, fallback: BoxedCryptoResolver) -> Self { Self { preferred, fallback } } } diff --git a/src/resolvers/ring.rs b/src/resolvers/ring.rs index 85172eba..83180fbb 100644 --- a/src/resolvers/ring.rs +++ b/src/resolvers/ring.rs @@ -13,6 +13,7 @@ use ring::{ /// A resolver that chooses [ring](https://github.com/briansmith/ring)-backed /// primitives when available. +#[allow(clippy::module_name_repetitions)] #[derive(Default)] pub struct RingResolver; diff --git a/src/stateless_transportstate.rs b/src/stateless_transportstate.rs index fd771fa7..7eeb0a88 100644 --- a/src/stateless_transportstate.rs +++ b/src/stateless_transportstate.rs @@ -40,7 +40,7 @@ impl StatelessTransportState { /// doesn't necessitate a remote static key, *or* if the remote /// static key is not yet known (as can be the case in the `XX` /// pattern, for example). - pub fn get_remote_static(&self) -> Option<&[u8]> { + #[must_use] pub fn get_remote_static(&self) -> Option<&[u8]> { self.rs.get().map(|rs| &rs[..self.dh_len]) } @@ -98,9 +98,9 @@ impl StatelessTransportState { /// of the Noise Specification. pub fn rekey_outgoing(&mut self) { if self.initiator { - self.cipherstates.rekey_initiator() + self.cipherstates.rekey_initiator(); } else { - self.cipherstates.rekey_responder() + self.cipherstates.rekey_responder(); } } @@ -110,9 +110,9 @@ impl StatelessTransportState { /// of the Noise Specification. pub fn rekey_incoming(&mut self) { if self.initiator { - self.cipherstates.rekey_responder() + self.cipherstates.rekey_responder(); } else { - self.cipherstates.rekey_initiator() + self.cipherstates.rekey_initiator(); } } @@ -132,16 +132,16 @@ impl StatelessTransportState { /// Set a new key for the initiator-egress symmetric cipher. pub fn rekey_initiator_manually(&mut self, key: &[u8; CIPHERKEYLEN]) { - self.cipherstates.rekey_initiator_manually(key) + self.cipherstates.rekey_initiator_manually(key); } /// Set a new key for the responder-egress symmetric cipher. pub fn rekey_responder_manually(&mut self, key: &[u8; CIPHERKEYLEN]) { - self.cipherstates.rekey_responder_manually(key) + self.cipherstates.rekey_responder_manually(key); } /// Check if this session was started with the "initiator" role. - pub fn is_initiator(&self) -> bool { + #[must_use] pub fn is_initiator(&self) -> bool { self.initiator } } diff --git a/src/transportstate.rs b/src/transportstate.rs index 9183627b..b5ab4033 100644 --- a/src/transportstate.rs +++ b/src/transportstate.rs @@ -40,7 +40,7 @@ impl TransportState { /// doesn't necessitate a remote static key, *or* if the remote /// static key is not yet known (as can be the case in the `XX` /// pattern, for example). - pub fn get_remote_static(&self) -> Option<&[u8]> { + #[must_use] pub fn get_remote_static(&self) -> Option<&[u8]> { self.rs.get().map(|rs| &rs[..self.dh_len]) } @@ -91,9 +91,9 @@ impl TransportState { /// of the Noise Specification. pub fn rekey_outgoing(&mut self) { if self.initiator { - self.cipherstates.rekey_initiator() + self.cipherstates.rekey_initiator(); } else { - self.cipherstates.rekey_responder() + self.cipherstates.rekey_responder(); } } @@ -103,9 +103,9 @@ impl TransportState { /// of the Noise Specification. pub fn rekey_incoming(&mut self) { if self.initiator { - self.cipherstates.rekey_responder() + self.cipherstates.rekey_responder(); } else { - self.cipherstates.rekey_initiator() + self.cipherstates.rekey_initiator(); } } @@ -125,12 +125,12 @@ impl TransportState { /// Set a new key for the initiator-egress symmetric cipher. pub fn rekey_initiator_manually(&mut self, key: &[u8; CIPHERKEYLEN]) { - self.cipherstates.rekey_initiator_manually(key) + self.cipherstates.rekey_initiator_manually(key); } /// Set a new key for the responder-egress symmetric cipher. pub fn rekey_responder_manually(&mut self, key: &[u8; CIPHERKEYLEN]) { - self.cipherstates.rekey_responder_manually(key) + self.cipherstates.rekey_responder_manually(key); } /// Set the forthcoming *inbound* nonce value. Useful for using noise on lossy transports. @@ -147,7 +147,7 @@ impl TransportState { /// # Errors /// /// Will result in `Error::State` if not in transport mode. - pub fn receiving_nonce(&self) -> u64 { + #[must_use] pub fn receiving_nonce(&self) -> u64 { if self.initiator { self.cipherstates.1.nonce() } else { @@ -160,7 +160,7 @@ impl TransportState { /// # Errors /// /// Will result in `Error::State` if not in transport mode. - pub fn sending_nonce(&self) -> u64 { + #[must_use] pub fn sending_nonce(&self) -> u64 { if self.initiator { self.cipherstates.0.nonce() } else { @@ -169,7 +169,7 @@ impl TransportState { } /// Check if this session was started with the "initiator" role. - pub fn is_initiator(&self) -> bool { + #[must_use] pub fn is_initiator(&self) -> bool { self.initiator } } diff --git a/src/types.rs b/src/types.rs index 091f1bf1..e98c2583 100644 --- a/src/types.rs +++ b/src/types.rs @@ -33,6 +33,9 @@ pub trait Dh: Send + Sync { fn privkey(&self) -> &[u8]; /// Calculate a Diffie-Hellman exchange. + /// + /// # Errors + /// Returns `Error::Dh` in the event that the Diffie-Hellman failed. fn dh(&self, pubkey: &[u8], out: &mut [u8]) -> Result<(), Error>; } @@ -48,6 +51,9 @@ pub trait Cipher: Send + Sync { fn encrypt(&self, nonce: u64, authtext: &[u8], plaintext: &[u8], out: &mut [u8]) -> usize; /// Decrypt (with associated data) a given ciphertext. + /// + /// # Errors + /// Returns `Error::Decrypt` in the event that the decryption failed. fn decrypt( &self, nonce: u64, diff --git a/tests/general.rs b/tests/general.rs index d9520a9d..57e428de 100644 --- a/tests/general.rs +++ b/tests/general.rs @@ -18,6 +18,7 @@ type TestResult = Result<(), Box>; struct CountingRng(u64); impl RngCore for CountingRng { + #[allow(clippy::cast_possible_truncation)] fn next_u32(&mut self) -> u32 { self.next_u64() as u32 } @@ -28,7 +29,7 @@ impl RngCore for CountingRng { } fn fill_bytes(&mut self, dest: &mut [u8]) { - impls::fill_bytes_via_next(self, dest) + impls::fill_bytes_via_next(self, dest); } fn try_fill_bytes(&mut self, dest: &mut [u8]) -> Result<(), rand_core::Error> { @@ -40,6 +41,7 @@ impl RngCore for CountingRng { impl CryptoRng for CountingRng {} impl Random for CountingRng {} +#[allow(clippy::cast_possible_truncation)] fn get_inc_key(start: u8) -> [u8; 32] { let mut k = [0u8; 32]; for i in 0..32 { @@ -67,7 +69,7 @@ impl TestResolver { impl CryptoResolver for TestResolver { fn resolve_rng(&self) -> Option> { - let rng = CountingRng(self.next_byte as u64); + let rng = CountingRng(u64::from(self.next_byte)); Some(Box::new(rng)) } @@ -94,14 +96,10 @@ fn test_protocol_name() -> TestResult { assert_eq!(protocol_spec.hash, HashChoice::Blake2s); let protocol_spec: Result = "Noise_NK_25519_ChaChaPoly_FAKE2X".parse(); - if protocol_spec.is_ok() { - panic!("invalid protocol was parsed inaccurately"); - } + assert!(protocol_spec.is_err(), "invalid protocol was parsed inaccurately"); let protocol_spec: Result = "Noise_NK_25519_ChaChaPoly".parse(); - if protocol_spec.is_ok() { - panic!("invalid protocol was parsed inaccurately"); - } + assert!(protocol_spec.is_err(), "invalid protocol was parsed inaccurately"); Ok(()) } @@ -653,7 +651,7 @@ fn test_buffer_issues_encrypted_handshake() -> TestResult { } #[test] -fn test_send_trait() -> TestResult { +fn test_send_trait() { use std::{sync::mpsc::channel, thread}; let (tx, rx) = channel(); @@ -664,7 +662,6 @@ fn test_send_trait() -> TestResult { tx.send(session).unwrap(); }); let _session = rx.recv().expect("failed to receive noise session"); - Ok(()) } #[test]