From 51def8d8851d80cf6438018bbe697a398292c31e Mon Sep 17 00:00:00 2001 From: Jake McGinty Date: Thu, 25 Jan 2024 15:08:11 -0800 Subject: [PATCH 1/7] large-scale clippy strictness increase --- Cargo.toml | 23 ++++++++ benches/benches.rs | 2 + examples/simple.rs | 8 +-- src/builder.rs | 27 +++++---- src/cipherstate.rs | 4 +- src/error.rs | 50 ++++++++++++++++- src/handshakestate.rs | 8 +-- src/{params/mod.rs => params.rs} | 18 +++++- src/params/patterns.rs | 9 ++- src/{resolvers/mod.rs => resolvers.rs} | 0 src/resolvers/default.rs | 78 +++++++++++++------------- src/stateless_transportstate.rs | 2 +- src/symmetricstate.rs | 22 ++++---- src/transportstate.rs | 2 +- src/types.rs | 27 +++++---- src/utils.rs | 2 +- tests/general.rs | 37 ++++++------ tests/vectors.rs | 2 + 18 files changed, 211 insertions(+), 110 deletions(-) rename src/{params/mod.rs => params.rs} (95%) rename src/{resolvers/mod.rs => resolvers.rs} (100%) diff --git a/Cargo.toml b/Cargo.toml index 46060dcb..316b373e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -36,6 +36,29 @@ harness = false [badges] travis-ci = { repository = "mcginty/snow", branch = "master" } +[lints.rust] +unsafe_code = "forbid" + +[lints.clippy] +pedantic = "warn" +unseparated_literal_suffix = "warn" +mod_module_files = "warn" +std_instead_of_core = "warn" +exhaustive_structs = "warn" +missing_trait_methods = "warn" +missing_inline_in_public_item = "warn" +absolute_paths = "warn" +as_conversions = "warn" +shadow_reuse = "warn" +# TODO(mcginty): warn once rewrite of slice code happens +indexing_slicing = "allow" +missing_asserts_for_indexing = "warn" +missing_assert_message = "warn" +string_slice = "warn" +arithmetic_side_effects = "allow" +exhaustive_enums = "warn" +wildcard_enum_match_arm = "warn" + [dependencies] rand_core = { version = "0.6", features = ["std", "getrandom"] } subtle = "2.4" diff --git a/benches/benches.rs b/benches/benches.rs index c1dfae4d..e8d447b2 100644 --- a/benches/benches.rs +++ b/benches/benches.rs @@ -1,3 +1,5 @@ +#![allow(clippy::restriction)] + #[macro_use] extern crate criterion; diff --git a/examples/simple.rs b/examples/simple.rs index 3eff1b3f..2e490587 100644 --- a/examples/simple.rs +++ b/examples/simple.rs @@ -36,7 +36,7 @@ fn main() { #[cfg(any(feature = "default-resolver", feature = "ring-accelerated"))] fn run_server() { - let mut buf = vec![0u8; 65535]; + let mut buf = vec![0_u8; 65535]; // Initialize our responder using a builder. let builder = Builder::new(PARAMS.clone()); @@ -75,7 +75,7 @@ fn run_server() { #[cfg(any(feature = "default-resolver", feature = "ring-accelerated"))] fn run_client() { - let mut buf = vec![0u8; 65535]; + let mut buf = vec![0_u8; 65535]; // Initialize our initiator using a builder. let builder = Builder::new(PARAMS.clone()); @@ -116,10 +116,10 @@ fn run_client() { /// Hyper-basic stream transport receiver. 16-bit BE size followed by payload. fn recv(stream: &mut TcpStream) -> io::Result> { - let mut msg_len_buf = [0u8; 2]; + let mut msg_len_buf = [0_u8; 2]; stream.read_exact(&mut msg_len_buf)?; let msg_len = usize::from(u16::from_be_bytes(msg_len_buf)); - let mut msg = vec![0u8; msg_len]; + let mut msg = vec![0_u8; msg_len]; stream.read_exact(&mut msg[..])?; Ok(msg) } diff --git a/src/builder.rs b/src/builder.rs index 07f8aafa..06e44bcc 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -1,4 +1,4 @@ -use std::fmt::Debug; +use core::fmt::{self, Debug}; #[cfg(feature = "hfs")] use crate::params::HandshakeModifier; @@ -19,6 +19,7 @@ const MAX_PSKS: usize = 10; /// A keypair object returned by [`Builder::generate_keypair()`] /// /// [`generate_keypair()`]: #method.generate_keypair +#[allow(clippy::exhaustive_structs)] pub struct Keypair { /// The private asymmetric key pub private: Vec, @@ -26,6 +27,7 @@ pub struct Keypair { pub public: Vec, } +#[allow(clippy::missing_trait_methods)] impl PartialEq for Keypair { fn eq(&self, other: &Keypair) -> bool { let priv_eq = self.private.ct_eq(&other.private); @@ -65,7 +67,7 @@ pub struct Builder<'builder> { } impl<'builder> Debug for Builder<'builder> { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("Builder").field("params", &self.params.name).finish_non_exhaustive() } } @@ -118,13 +120,13 @@ impl<'builder> Builder<'builder> { /// 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 { + let index = usize::from(location); + if index >= MAX_PSKS { Err(InitStage::ValidatePskPosition.into()) - } else if self.psks[location].is_some() { + } else if self.psks[index].is_some() { Err(InitStage::ParameterOverwrite.into()) } else { - self.psks[location] = Some(key); + self.psks[index] = Some(key); Ok(self) } } @@ -188,8 +190,8 @@ impl<'builder> Builder<'builder> { 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)?; - let mut private = vec![0u8; dh.priv_len()]; - let mut public = vec![0u8; dh.pub_len()]; + let mut private = vec![0_u8; dh.priv_len()]; + let mut public = vec![0_u8; dh.pub_len()]; dh.generate(&mut *rng); private.copy_from_slice(dh.privkey()); @@ -251,7 +253,7 @@ impl<'builder> Builder<'builder> { } let e = Toggle::off(e_dh); - let mut rs_buf = [0u8; MAXDHLEN]; + let mut rs_buf = [0_u8; MAXDHLEN]; let rs = match self.rs { Some(v) => { rs_buf[..v.len()].copy_from_slice(v); @@ -260,7 +262,7 @@ impl<'builder> Builder<'builder> { None => Toggle::off(rs_buf), }; - let re = Toggle::off([0u8; MAXDHLEN]); + let re = Toggle::off([0_u8; MAXDHLEN]); let mut psks = [None::<[u8; PSKLEN]>; 10]; for (i, psk) in self.psks.iter().enumerate() { @@ -268,7 +270,7 @@ impl<'builder> Builder<'builder> { if key.len() != PSKLEN { return Err(InitStage::ValidatePskLengths.into()); } - let mut k = [0u8; PSKLEN]; + let mut k = [0_u8; PSKLEN]; k.copy_from_slice(key); psks[i] = Some(k); } @@ -319,6 +321,7 @@ impl<'builder> Builder<'builder> { #[cfg(test)] #[cfg(any(feature = "default-resolver", feature = "ring-accelerated"))] +#[allow(clippy::restriction)] mod tests { use super::*; type TestResult = Result<(), Box>; @@ -374,7 +377,7 @@ mod tests { build_builder()?.prologue(&[1u8; 10]).unwrap_err(), Error::Init(InitStage::ParameterOverwrite) ); - assert!(build_builder()?.psk(1, &[1u8; 32]).is_ok()); + build_builder()?.psk(1, &[1u8; 32]).unwrap(); assert_eq!( build_builder()?.psk(0, &[1u8; 32]).unwrap_err(), Error::Init(InitStage::ParameterOverwrite) diff --git a/src/cipherstate.rs b/src/cipherstate.rs index fa29dc45..19864db1 100644 --- a/src/cipherstate.rs +++ b/src/cipherstate.rs @@ -68,11 +68,11 @@ impl CipherState { } pub fn encrypt(&mut self, plaintext: &[u8], out: &mut [u8]) -> Result { - self.encrypt_ad(&[0u8; 0], plaintext, out) + self.encrypt_ad(&[0_u8; 0], plaintext, out) } pub fn decrypt(&mut self, ciphertext: &[u8], out: &mut [u8]) -> Result { - self.decrypt_ad(&[0u8; 0], ciphertext, out) + self.decrypt_ad(&[0_u8; 0], ciphertext, out) } pub fn rekey(&mut self) { diff --git a/src/error.rs b/src/error.rs index 8734ec4f..768cab0e 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,3 +1,4 @@ +#![allow(clippy::absolute_paths)] //! All error types used by Snow operations. use std::fmt; @@ -47,6 +48,7 @@ pub enum Error { /// The various stages of initialization used to help identify /// the specific cause of an `Init` error. #[derive(Debug, PartialEq)] +#[non_exhaustive] pub enum PatternProblem { /// Caused by a pattern string that is too short and malformed (e.g. `Noise_NN_25519`). TooFewParameters, @@ -77,6 +79,15 @@ pub enum PatternProblem { /// Invalid KEM type. /// Check that there are no typos and that any feature flags you might need are toggled UnsupportedKemType, + /// Invalid string (non-ASCII) + NonAscii, +} +#[allow(clippy::missing_trait_methods)] +impl std::error::Error for PatternProblem {} +impl fmt::Display for PatternProblem { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{self:?}") + } } impl From for Error { @@ -88,6 +99,7 @@ impl From for Error { /// The various stages of initialization used to help identify /// the specific cause of an `Init` error. #[derive(Debug, PartialEq)] +#[non_exhaustive] pub enum InitStage { /// Provided and received key lengths were not equal. ValidateKeyLengths, @@ -112,6 +124,13 @@ pub enum InitStage { /// they can introduce subtle security issues. ParameterOverwrite, } +#[allow(clippy::missing_trait_methods)] +impl std::error::Error for InitStage {} +impl fmt::Display for InitStage { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{self:?}") + } +} impl From for Error { fn from(reason: InitStage) -> Self { @@ -121,12 +140,20 @@ impl From for Error { /// A prerequisite that may be missing. #[derive(Debug, PartialEq)] +#[non_exhaustive] pub enum Prerequisite { /// A local private key wasn't provided when it was needed by the selected pattern. LocalPrivateKey, /// A remote public key wasn't provided when it was needed by the selected pattern. RemotePublicKey, } +#[allow(clippy::missing_trait_methods)] +impl std::error::Error for Prerequisite {} +impl fmt::Display for Prerequisite { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{self:?}") + } +} impl From for Error { fn from(reason: Prerequisite) -> Self { @@ -136,6 +163,7 @@ impl From for Error { /// Specific errors in the state machine. #[derive(Debug, PartialEq)] +#[non_exhaustive] pub enum StateProblem { /// Missing key material in the internal handshake state. MissingKeyMaterial, @@ -154,6 +182,13 @@ pub enum StateProblem { /// The nonce counter attempted to go higher than (2^64) - 1 Exhausted, } +#[allow(clippy::missing_trait_methods)] +impl std::error::Error for StateProblem {} +impl fmt::Display for StateProblem { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{self:?}") + } +} impl From for Error { fn from(reason: StateProblem) -> Self { @@ -181,4 +216,17 @@ impl fmt::Display for Error { } } -impl std::error::Error for Error {} +#[allow(clippy::missing_trait_methods)] +impl std::error::Error for Error { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + Error::Pattern(reason) => Some(reason), + Error::Init(reason) => Some(reason), + Error::Prereq(reason) => Some(reason), + Error::State(reason) => Some(reason), + Error::Input | Error::Dh | Error::Decrypt => None, + #[cfg(feature = "hfs")] + Error::Kem => None, + } + } +} diff --git a/src/handshakestate.rs b/src/handshakestate.rs index 671b55dc..08bde035 100644 --- a/src/handshakestate.rs +++ b/src/handshakestate.rs @@ -15,7 +15,7 @@ use crate::{ types::{Dh, Hash, Random}, utils::Toggle, }; -use std::{ +use core::{ convert::{TryFrom, TryInto}, fmt, }; @@ -49,6 +49,7 @@ pub struct HandshakeState { impl HandshakeState { #[allow(clippy::too_many_arguments)] + #[allow(clippy::wildcard_enum_match_arm)] pub(crate) fn new( rng: Box, cipherstate: CipherState, @@ -161,7 +162,7 @@ impl HandshakeState { } fn dh(&self, token: DhToken) -> Result<[u8; MAXDHLEN], Error> { - let mut dh_out = [0u8; MAXDHLEN]; + let mut dh_out = [0_u8; MAXDHLEN]; let (dh, key) = match (token, self.is_initiator()) { (DhToken::Ee, _) => (&self.e, &self.re), (DhToken::Ss, _) => (&self.s, &self.rs), @@ -252,7 +253,6 @@ impl HandshakeState { } else if byte_index + self.s.pub_len() > message.len() { return Err(Error::Input); } - byte_index += self .symmetricstate .encrypt_and_mix_hash(self.s.pubkey(), &mut message[byte_index..])?; @@ -457,7 +457,7 @@ impl HandshakeState { return Err(Error::Input); } - let mut new_psk = [0u8; PSKLEN]; + let mut new_psk = [0_u8; PSKLEN]; new_psk.copy_from_slice(key); self.psks[location] = Some(new_psk); diff --git a/src/params/mod.rs b/src/params.rs similarity index 95% rename from src/params/mod.rs rename to src/params.rs index f8d7463d..803eb7ea 100644 --- a/src/params/mod.rs +++ b/src/params.rs @@ -5,7 +5,7 @@ //! patterns/names) use crate::error::{Error, PatternProblem}; -use std::str::FromStr; +use core::str::FromStr; mod patterns; pub use self::patterns::{ @@ -17,6 +17,7 @@ pub(crate) use self::patterns::{DhToken, HandshakeTokens, MessagePatterns, Token /// I recommend you choose `Noise`. #[derive(PartialEq, Copy, Clone, Debug)] +#[allow(clippy::exhaustive_enums)] pub enum BaseChoice { /// Ole' faithful. Noise, @@ -36,6 +37,7 @@ impl FromStr for BaseChoice { /// Which Diffie-Hellman primitive to use. One of `25519` or `448`, per the spec. #[derive(PartialEq, Copy, Clone, Debug)] +#[non_exhaustive] pub enum DHChoice { /// The Curve25519 ellpitic curve. Curve25519, @@ -58,6 +60,7 @@ impl FromStr for DHChoice { /// One of `ChaChaPoly` or `AESGCM`, per the spec. #[derive(PartialEq, Copy, Clone, Debug)] +#[non_exhaustive] pub enum CipherChoice { /// The ChaCha20Poly1305 AEAD. ChaChaPoly, @@ -87,6 +90,7 @@ impl FromStr for CipherChoice { /// One of the supported SHA-family or BLAKE-family hash choices, per the spec. #[derive(PartialEq, Copy, Clone, Debug)] +#[non_exhaustive] pub enum HashChoice { /// The SHA-256 hash function. SHA256, @@ -117,6 +121,7 @@ impl FromStr for HashChoice { /// One of the supported Kems provided for unstable HFS extension. #[cfg(feature = "hfs")] #[derive(PartialEq, Copy, Clone, Debug)] +#[non_exhaustive] pub enum KemChoice { /// The 1024-bit Kyber variant. Kyber1024, @@ -150,6 +155,7 @@ impl FromStr for KemChoice { /// ``` #[derive(PartialEq, Clone, Debug)] #[allow(clippy::module_name_repetitions)] +#[non_exhaustive] pub struct NoiseParams { /// The full pattern string. pub name: String, @@ -203,6 +209,9 @@ impl FromStr for NoiseParams { #[cfg(not(feature = "hfs"))] fn from_str(s: &str) -> Result { + if !s.is_ascii() { + return Err(Error::Pattern(PatternProblem::NonAscii)); + } let mut split = s.split('_'); let params = NoiseParams::new( s.to_owned(), @@ -220,6 +229,9 @@ impl FromStr for NoiseParams { #[cfg(feature = "hfs")] fn from_str(s: &str) -> Result { + if !s.is_ascii() { + return Err(Error::Pattern(PatternProblem::NonAscii)); + } let mut split = s.split('_').peekable(); let p = NoiseParams::new( s.to_owned(), @@ -257,7 +269,7 @@ impl FromStr for NoiseParams { #[cfg(test)] mod tests { use super::*; - use std::convert::TryFrom; + use core::convert::TryFrom; #[test] fn test_simple_handshake() { @@ -311,7 +323,7 @@ mod tests { #[test] fn test_duplicate_psk_mod() { - assert!("Noise_XXfallback+psk1_25519_AESGCM_SHA256".parse::().is_ok()); + "Noise_XXfallback+psk1_25519_AESGCM_SHA256".parse::().unwrap(); assert_eq!( Error::Pattern(PatternProblem::DuplicateModifier), "Noise_XXfallback+fallback_25519_AESGCM_SHA256".parse::().unwrap_err() diff --git a/src/params/patterns.rs b/src/params/patterns.rs index ffff2414..1ace5252 100644 --- a/src/params/patterns.rs +++ b/src/params/patterns.rs @@ -1,7 +1,7 @@ #![allow(clippy::enum_glob_use)] use crate::error::{Error, PatternProblem}; -use std::{convert::TryFrom, str::FromStr}; +use core::{convert::TryFrom, str::FromStr}; /// A small helper macro that behaves similar to the `vec![]` standard macro, /// except it allocates a bit extra to avoid resizing. @@ -34,6 +34,7 @@ macro_rules! pattern_enum { /// [Handshake Pattern](https://noiseprotocol.org/noise.html#handshake-patterns) /// section. #[allow(missing_docs)] + #[allow(clippy::exhaustive_enums)] #[derive(Copy, Clone, PartialEq, Debug)] pub enum $name { $($variant),*, @@ -160,6 +161,7 @@ impl HandshakePattern { /// A modifier applied to the base pattern as defined in the Noise spec. #[derive(Copy, Clone, PartialEq, Debug)] +#[non_exhaustive] pub enum HandshakeModifier { /// Insert a PSK to mix at the associated position Psk(u8), @@ -175,6 +177,7 @@ pub enum HandshakeModifier { impl FromStr for HandshakeModifier { type Err = Error; + #[allow(clippy::string_slice)] fn from_str(s: &str) -> Result { match s { s if s.starts_with("psk") => { @@ -219,6 +222,7 @@ impl FromStr for HandshakeModifierList { /// The pattern/modifier combination choice (no primitives specified) /// for a full noise protocol definition. #[derive(Clone, PartialEq, Debug)] +#[non_exhaustive] pub struct HandshakeChoice { /// The base pattern itself pub pattern: HandshakePattern, @@ -252,6 +256,7 @@ impl HandshakeChoice { } /// Parse and split a base `HandshakePattern` from its optional modifiers + #[allow(clippy::string_slice)] 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) { @@ -533,7 +538,7 @@ fn check_hfs_and_oneway_conflict(handshake: &HandshakeChoice) -> Result<(), Erro fn apply_psk_modifier(patterns: &mut Patterns, n: u8) -> Result<(), Error> { let tokens = patterns .2 - .get_mut((n as usize).saturating_sub(1)) + .get_mut((usize::from(n)).saturating_sub(1)) .ok_or(Error::Pattern(PatternProblem::InvalidPsk))?; if n == 0 { tokens.insert(0, Token::Psk(n)); diff --git a/src/resolvers/mod.rs b/src/resolvers.rs similarity index 100% rename from src/resolvers/mod.rs rename to src/resolvers.rs diff --git a/src/resolvers/default.rs b/src/resolvers/default.rs index 93ed5b6a..bf8a3754 100644 --- a/src/resolvers/default.rs +++ b/src/resolvers/default.rs @@ -145,14 +145,14 @@ impl Dh for Dh25519 { } fn set(&mut self, privkey: &[u8]) { - let mut bytes = [0u8; CIPHERKEYLEN]; + let mut bytes = [0_u8; CIPHERKEYLEN]; copy_slices!(privkey, bytes); self.privkey = bytes; self.derive_pubkey(); } fn generate(&mut self, rng: &mut dyn Random) { - let mut bytes = [0u8; CIPHERKEYLEN]; + let mut bytes = [0_u8; CIPHERKEYLEN]; rng.fill_bytes(&mut bytes); self.privkey = bytes; self.derive_pubkey(); @@ -167,7 +167,7 @@ impl Dh for Dh25519 { } fn dh(&self, pubkey: &[u8], out: &mut [u8]) -> Result<(), Error> { - let mut pubkey_owned = [0u8; CIPHERKEYLEN]; + let mut pubkey_owned = [0_u8; CIPHERKEYLEN]; copy_slices!(&pubkey[..32], pubkey_owned); let result = MontgomeryPoint(pubkey_owned).mul_clamped(self.privkey).to_bytes(); copy_slices!(result, out); @@ -187,7 +187,7 @@ impl Cipher for CipherAesGcm { fn encrypt(&self, nonce: u64, authtext: &[u8], plaintext: &[u8], out: &mut [u8]) -> usize { let aead = aes_gcm::Aes256Gcm::new(&self.key.into()); - let mut nonce_bytes = [0u8; 12]; + let mut nonce_bytes = [0_u8; 12]; copy_slices!(nonce.to_be_bytes(), &mut nonce_bytes[4..]); copy_slices!(plaintext, out); @@ -210,7 +210,7 @@ impl Cipher for CipherAesGcm { ) -> Result { let aead = aes_gcm::Aes256Gcm::new(&self.key.into()); - let mut nonce_bytes = [0u8; 12]; + let mut nonce_bytes = [0_u8; 12]; copy_slices!(nonce.to_be_bytes(), &mut nonce_bytes[4..]); let message_len = ciphertext.len() - TAGLEN; @@ -238,7 +238,7 @@ impl Cipher for CipherChaChaPoly { } fn encrypt(&self, nonce: u64, authtext: &[u8], plaintext: &[u8], out: &mut [u8]) -> usize { - let mut nonce_bytes = [0u8; 12]; + let mut nonce_bytes = [0_u8; 12]; copy_slices!(nonce.to_le_bytes(), &mut nonce_bytes[4..]); copy_slices!(plaintext, out); @@ -259,7 +259,7 @@ impl Cipher for CipherChaChaPoly { ciphertext: &[u8], out: &mut [u8], ) -> Result { - let mut nonce_bytes = [0u8; 12]; + let mut nonce_bytes = [0_u8; 12]; copy_slices!(nonce.to_le_bytes(), &mut nonce_bytes[4..]); let message_len = ciphertext.len() - TAGLEN; @@ -527,7 +527,7 @@ mod tests { #[test] fn test_sha256() { - let mut output = [0u8; 32]; + let mut output = [0_u8; 32]; let mut hasher = HashSHA256::default(); hasher.input(b"abc"); hasher.result(&mut output); @@ -544,7 +544,7 @@ mod tests { "dddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd", ) .unwrap(); - let mut output1 = [0u8; 32]; + let mut output1 = [0_u8; 32]; let mut hasher = HashSHA256::default(); hasher.hmac(&key, &data, &mut output1); assert!( @@ -552,7 +552,7 @@ mod tests { == "773ea91e36800e46854db8ebd09181a72959098b3ef8c122d9635514ced565fe" ); - let mut output2 = [0u8; 64]; + let mut output2 = [0_u8; 64]; let mut hasher = HashSHA512::default(); hasher.hmac(&key, &data, &mut output2); assert!( @@ -567,7 +567,7 @@ mod tests { #[test] fn test_blake2b() { // BLAKE2b test - draft-saarinen-blake2-06 - let mut output = [0u8; 64]; + let mut output = [0_u8; 64]; let mut hasher = HashBLAKE2b::default(); hasher.input(b"abc"); hasher.result(&mut output); @@ -583,7 +583,7 @@ mod tests { #[test] fn test_blake2s() { // BLAKE2s test - draft-saarinen-blake2-06 - let mut output = [0u8; 32]; + let mut output = [0_u8; 32]; let mut hasher = HashBLAKE2s::default(); hasher.input(b"abc"); hasher.result(&mut output); @@ -605,7 +605,7 @@ mod tests { let public = Vec::::from_hex("e6db6867583030db3594c1a424b15f7c726624ec26b3353b10a903a6d0ab1c4c") .unwrap(); - let mut output = [0u8; 32]; + let mut output = [0_u8; 32]; keypair.dh(&public, &mut output).unwrap(); assert_eq!( hex::encode(output), @@ -617,27 +617,27 @@ mod tests { fn test_aesgcm() { // AES256-GCM tests - gcm-spec.pdf // Test Case 13 - let key = [0u8; 32]; - let nonce = 0u64; - let plaintext = [0u8; 0]; - let authtext = [0u8; 0]; - let mut ciphertext = [0u8; 16]; + let key = [0_u8; 32]; + let nonce = 0_u64; + let plaintext = [0_u8; 0]; + let authtext = [0_u8; 0]; + let mut ciphertext = [0_u8; 16]; 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 resulttext = [0_u8; 1]; let mut cipher2 = CipherAesGcm::default(); cipher2.set(&key); cipher2.decrypt(nonce, &authtext, &ciphertext, &mut resulttext).unwrap(); assert!(resulttext[0] == 0); ciphertext[0] ^= 1; - assert!(cipher2.decrypt(nonce, &authtext, &ciphertext, &mut resulttext).is_err()); + cipher2.decrypt(nonce, &authtext, &ciphertext, &mut resulttext).unwrap_err(); // Test Case 14 - let plaintext2 = [0u8; 16]; - let mut ciphertext2 = [0u8; 32]; + let plaintext2 = [0_u8; 16]; + let mut ciphertext2 = [0_u8; 32]; let mut cipher3 = CipherAesGcm::default(); cipher3.set(&key); cipher3.encrypt(nonce, &authtext, &plaintext2, &mut ciphertext2); @@ -646,49 +646,49 @@ mod tests { == "cea7403d4d606b6e074ec5d3baf39d18d0d1c8a799996bf0265b98b5d48ab919" ); - let mut resulttext2 = [1u8; 16]; + let mut resulttext2 = [1_u8; 16]; let mut cipher4 = CipherAesGcm::default(); cipher4.set(&key); cipher4.decrypt(nonce, &authtext, &ciphertext2, &mut resulttext2).unwrap(); assert!(plaintext2 == resulttext2); ciphertext2[0] ^= 1; - assert!(cipher4.decrypt(nonce, &authtext, &ciphertext2, &mut resulttext2).is_err()); + cipher4.decrypt(nonce, &authtext, &ciphertext2, &mut resulttext2).unwrap_err(); } #[test] fn test_chachapoly_empty() { //ChaChaPoly round-trip test, empty plaintext - let key = [0u8; 32]; - let nonce = 0u64; - let plaintext = [0u8; 0]; - let authtext = [0u8; 0]; - let mut ciphertext = [0u8; 16]; + let key = [0_u8; 32]; + let nonce = 0_u64; + let plaintext = [0_u8; 0]; + let authtext = [0_u8; 0]; + let mut ciphertext = [0_u8; 16]; let mut cipher1 = CipherChaChaPoly::default(); cipher1.set(&key); cipher1.encrypt(nonce, &authtext, &plaintext, &mut ciphertext); - let mut resulttext = [0u8; 1]; + let mut resulttext = [0_u8; 1]; let mut cipher2 = CipherChaChaPoly::default(); cipher2.set(&key); cipher2.decrypt(nonce, &authtext, &ciphertext, &mut resulttext).unwrap(); assert!(resulttext[0] == 0); ciphertext[0] ^= 1; - assert!(cipher2.decrypt(nonce, &authtext, &ciphertext, &mut resulttext).is_err()); + cipher2.decrypt(nonce, &authtext, &ciphertext, &mut resulttext).unwrap_err(); } #[test] fn test_chachapoly_nonempty() { //ChaChaPoly round-trip test, non-empty plaintext - let key = [0u8; 32]; - let nonce = 0u64; - let plaintext = [0x34u8; 117]; - let authtext = [0u8; 0]; - let mut ciphertext = [0u8; 133]; + let key = [0_u8; 32]; + let nonce = 0_u64; + let plaintext = [0x34_u8; 117]; + let authtext = [0_u8; 0]; + let mut ciphertext = [0_u8; 133]; let mut cipher1 = CipherChaChaPoly::default(); cipher1.set(&key); cipher1.encrypt(nonce, &authtext, &plaintext, &mut ciphertext); - let mut resulttext = [0u8; 117]; + let mut resulttext = [0_u8; 117]; let mut cipher2 = CipherChaChaPoly::default(); cipher2.set(&key); cipher2.decrypt(nonce, &authtext, &ciphertext, &mut resulttext).unwrap(); @@ -746,8 +746,8 @@ mod tests { .unwrap(); let tag = Vec::::from_hex("eead9d67890cbb22392336fea1851f38").unwrap(); let authtext = Vec::::from_hex("f33388860000000000004e91").unwrap(); - let mut combined_text = [0u8; 1024]; - let mut out = [0u8; 1024]; + let mut combined_text = [0_u8; 1024]; + let mut out = [0_u8; 1024]; copy_slices!(&ciphertext, &mut combined_text); copy_slices!(&tag[0..TAGLEN], &mut combined_text[ciphertext.len()..]); diff --git a/src/stateless_transportstate.rs b/src/stateless_transportstate.rs index 904ab4a5..1fd52014 100644 --- a/src/stateless_transportstate.rs +++ b/src/stateless_transportstate.rs @@ -6,7 +6,7 @@ use crate::{ params::HandshakePattern, utils::Toggle, }; -use std::{convert::TryFrom, fmt}; +use core::{convert::TryFrom, fmt}; /// A state machine encompassing the transport phase of a Noise session, using the two /// `CipherState`s (for sending and receiving) that were spawned from the `SymmetricState`'s diff --git a/src/symmetricstate.rs b/src/symmetricstate.rs index 7114cd30..226bec13 100644 --- a/src/symmetricstate.rs +++ b/src/symmetricstate.rs @@ -6,7 +6,7 @@ use crate::{ }; #[derive(Copy, Clone)] -pub(crate) struct SymmetricStateData { +pub(in crate) struct SymmetricStateData { h: [u8; MAXHASHLEN], ck: [u8; MAXHASHLEN], has_key: bool, @@ -15,14 +15,14 @@ pub(crate) struct SymmetricStateData { impl Default for SymmetricStateData { fn default() -> Self { SymmetricStateData { - h: [0u8; MAXHASHLEN], - ck: [0u8; MAXHASHLEN], + h: [0_u8; MAXHASHLEN], + ck: [0_u8; MAXHASHLEN], has_key: false, } } } -pub(crate) struct SymmetricState { +pub(in crate) struct SymmetricState { cipherstate: CipherState, hasher: Box, inner: SymmetricStateData, @@ -47,7 +47,7 @@ impl SymmetricState { pub fn mix_key(&mut self, data: &[u8]) { let hash_len = self.hasher.hash_len(); - let mut hkdf_output = ([0u8; MAXHASHLEN], [0u8; MAXHASHLEN]); + let mut hkdf_output = ([0_u8; MAXHASHLEN], [0_u8; MAXHASHLEN]); self.hasher.hkdf( &self.inner.ck[..hash_len], data, @@ -58,7 +58,7 @@ impl SymmetricState { ); // TODO(mcginty): use `split_array_ref` once stable to avoid memory inefficiency - let mut cipher_key = [0u8; CIPHERKEYLEN]; + let mut cipher_key = [0_u8; CIPHERKEYLEN]; cipher_key.copy_from_slice(&hkdf_output.1[..CIPHERKEYLEN]); self.inner.ck = hkdf_output.0; @@ -76,7 +76,7 @@ impl SymmetricState { pub fn mix_key_and_hash(&mut self, data: &[u8]) { let hash_len = self.hasher.hash_len(); - let mut hkdf_output = ([0u8; MAXHASHLEN], [0u8; MAXHASHLEN], [0u8; MAXHASHLEN]); + let mut hkdf_output = ([0_u8; MAXHASHLEN], [0_u8; MAXHASHLEN], [0_u8; MAXHASHLEN]); self.hasher.hkdf( &self.inner.ck[..hash_len], data, @@ -89,7 +89,7 @@ impl SymmetricState { self.mix_hash(&hkdf_output.1[..hash_len]); // TODO(mcginty): use `split_array_ref` once stable to avoid memory inefficiency - let mut cipher_key = [0u8; CIPHERKEYLEN]; + let mut cipher_key = [0_u8; CIPHERKEYLEN]; cipher_key.copy_from_slice(&hkdf_output.2[..CIPHERKEYLEN]); self.cipherstate.set(&cipher_key, 0); } @@ -131,11 +131,11 @@ impl SymmetricState { } pub fn split(&mut self, child1: &mut CipherState, child2: &mut CipherState) { - let mut hkdf_output = ([0u8; MAXHASHLEN], [0u8; MAXHASHLEN]); + let mut hkdf_output = ([0_u8; MAXHASHLEN], [0_u8; MAXHASHLEN]); self.split_raw(&mut hkdf_output.0, &mut hkdf_output.1); // TODO(mcginty): use `split_array_ref` once stable to avoid memory inefficiency - let mut cipher_keys = ([0u8; CIPHERKEYLEN], [0u8; CIPHERKEYLEN]); + let mut cipher_keys = ([0_u8; CIPHERKEYLEN], [0_u8; CIPHERKEYLEN]); cipher_keys.0.copy_from_slice(&hkdf_output.0[..CIPHERKEYLEN]); cipher_keys.1.copy_from_slice(&hkdf_output.1[..CIPHERKEYLEN]); child1.set(&cipher_keys.0, 0); @@ -144,7 +144,7 @@ impl SymmetricState { pub fn split_raw(&mut self, out1: &mut [u8], out2: &mut [u8]) { let hash_len = self.hasher.hash_len(); - self.hasher.hkdf(&self.inner.ck[..hash_len], &[0u8; 0], 2, out1, out2, &mut []); + self.hasher.hkdf(&self.inner.ck[..hash_len], &[0_u8; 0], 2, out1, out2, &mut []); } pub(crate) fn checkpoint(&mut self) -> SymmetricStateData { diff --git a/src/transportstate.rs b/src/transportstate.rs index b927b37f..47e79435 100644 --- a/src/transportstate.rs +++ b/src/transportstate.rs @@ -6,7 +6,7 @@ use crate::{ params::HandshakePattern, utils::Toggle, }; -use std::{convert::TryFrom, fmt}; +use core::{convert::TryFrom, fmt}; /// A state machine encompassing the transport phase of a Noise session, using the two /// `CipherState`s (for sending and receiving) that were spawned from the `SymmetricState`'s diff --git a/src/types.rs b/src/types.rs index e98c2583..a038c075 100644 --- a/src/types.rs +++ b/src/types.rs @@ -67,10 +67,14 @@ pub trait Cipher: Send + Sync { fn rekey(&mut self) { let mut ciphertext = [0; CIPHERKEYLEN + TAGLEN]; let ciphertext_len = self.encrypt(u64::MAX, &[], &[0; CIPHERKEYLEN], &mut ciphertext); - assert_eq!(ciphertext_len, ciphertext.len()); + assert_eq!( + ciphertext_len, + ciphertext.len(), + "returned ciphertext length not expected size" + ); // TODO(mcginty): use `split_array_ref` once stable to avoid memory inefficiency - let mut key = [0u8; CIPHERKEYLEN]; + let mut key = [0_u8; CIPHERKEYLEN]; key.copy_from_slice(&ciphertext[..CIPHERKEYLEN]); self.set(&key); @@ -101,19 +105,20 @@ pub trait Hash: Send + Sync { /// /// NOTE: This method clobbers the existing internal state fn hmac(&mut self, key: &[u8], data: &[u8], out: &mut [u8]) { - assert!(key.len() <= self.block_len()); + let key_len = key.len(); let block_len = self.block_len(); let hash_len = self.hash_len(); - let mut ipad = [0x36u8; MAXBLOCKLEN]; - let mut opad = [0x5cu8; MAXBLOCKLEN]; - for count in 0..key.len() { + assert!(key.len() <= block_len, "key and block lengths differ"); + let mut ipad = [0x36_u8; MAXBLOCKLEN]; + let mut opad = [0x5c_u8; MAXBLOCKLEN]; + for count in 0..key_len { ipad[count] ^= key[count]; opad[count] ^= key[count]; } self.reset(); self.input(&ipad[..block_len]); self.input(data); - let mut inner_output = [0u8; MAXHASHLEN]; + let mut inner_output = [0_u8; MAXHASHLEN]; self.result(&mut inner_output); self.reset(); self.input(&opad[..block_len]); @@ -134,14 +139,14 @@ pub trait Hash: Send + Sync { out3: &mut [u8], ) { let hash_len = self.hash_len(); - let mut temp_key = [0u8; MAXHASHLEN]; + let mut temp_key = [0_u8; MAXHASHLEN]; self.hmac(chaining_key, input_key_material, &mut temp_key); - self.hmac(&temp_key, &[1u8], out1); + self.hmac(&temp_key, &[1_u8], out1); if outputs == 1 { return; } - let mut in2 = [0u8; MAXHASHLEN + 1]; + let mut in2 = [0_u8; MAXHASHLEN + 1]; copy_slices!(out1[0..hash_len], &mut in2); in2[hash_len] = 2; self.hmac(&temp_key, &in2[..=hash_len], out2); @@ -149,7 +154,7 @@ pub trait Hash: Send + Sync { return; } - let mut in3 = [0u8; MAXHASHLEN + 1]; + let mut in3 = [0_u8; MAXHASHLEN + 1]; copy_slices!(out2[0..hash_len], &mut in3); in3[hash_len] = 3; self.hmac(&temp_key, &in3[..=hash_len], out3); diff --git a/src/utils.rs b/src/utils.rs index 633bea94..4e6efb47 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -1,4 +1,4 @@ -use std::ops::{Deref, DerefMut}; +use core::ops::{Deref, DerefMut}; /// Toggle is similar to Option, except that even in the Off/"None" case, there is still /// an owned allocated inner object. This is useful for holding onto pre-allocated objects diff --git a/tests/general.rs b/tests/general.rs index d97662bb..b3058b22 100644 --- a/tests/general.rs +++ b/tests/general.rs @@ -1,5 +1,6 @@ #![cfg(any(feature = "default-resolver", feature = "ring-accelerated"))] #![allow(clippy::needless_range_loop)] +#![allow(clippy::restriction)] #![allow(non_snake_case)] use hex::FromHex; @@ -417,7 +418,7 @@ fn test_rekey() -> TestResult { // rekey outgoing on initiator h_i.rekey_outgoing(); let len = h_i.write_message(b"hack the planet", &mut buffer_msg)?; - assert!(h_r.read_message(&buffer_msg[..len], &mut buffer_out).is_err()); + h_r.read_message(&buffer_msg[..len], &mut buffer_out).unwrap_err(); h_r.set_receiving_nonce(h_i.sending_nonce()); // rekey incoming on responder @@ -429,7 +430,7 @@ fn test_rekey() -> TestResult { // rekey outgoing on responder h_r.rekey_outgoing(); let len = h_r.write_message(b"hack the planet", &mut buffer_msg)?; - assert!(h_i.read_message(&buffer_msg[..len], &mut buffer_out).is_err()); + h_i.read_message(&buffer_msg[..len], &mut buffer_out).unwrap_err(); h_i.set_receiving_nonce(h_r.sending_nonce()); // rekey incoming on initiator @@ -468,7 +469,7 @@ fn test_rekey_manually() -> TestResult { // The message *should* have failed to read, so we also force nonce re-sync. h_i.rekey_manually(Some(&[1u8; 32]), None); let len = h_i.write_message(b"hack the planet", &mut buffer_msg)?; - assert!(h_r.read_message(&buffer_msg[..len], &mut buffer_out).is_err()); + h_r.read_message(&buffer_msg[..len], &mut buffer_out).unwrap_err(); h_r.set_receiving_nonce(h_i.sending_nonce()); // rekey responder-side responder key to K1, expecting a successful decryption. @@ -483,7 +484,7 @@ fn test_rekey_manually() -> TestResult { // The message *should* have failed to read, so we also force nonce re-sync. h_r.rekey_manually(None, Some(&[1u8; 32])); let len = h_r.write_message(b"hack the planet", &mut buffer_msg)?; - assert!(h_i.read_message(&buffer_msg[..len], &mut buffer_out).is_err()); + h_i.read_message(&buffer_msg[..len], &mut buffer_out).unwrap_err(); h_i.set_receiving_nonce(h_r.sending_nonce()); // rekey intiator-side responder key to K1, expecting a successful decryption. @@ -500,7 +501,7 @@ fn test_handshake_message_exceeds_max_len() -> TestResult { let mut h_i = Builder::new(params).build_initiator()?; let mut buffer_out = [0u8; 65535 * 2]; - assert!(h_i.write_message(&[0u8; 65530], &mut buffer_out).is_err()); + h_i.write_message(&[0u8; 65530], &mut buffer_out).unwrap_err(); Ok(()) } @@ -510,7 +511,7 @@ fn test_handshake_message_undersized_output_buffer() -> TestResult { let mut h_i = Builder::new(params).build_initiator()?; let mut buffer_out = [0u8; 200]; - assert!(h_i.write_message(&[0u8; 400], &mut buffer_out).is_err()); + h_i.write_message(&[0u8; 400], &mut buffer_out).unwrap_err(); Ok(()) } @@ -552,7 +553,7 @@ fn test_transport_message_exceeds_max_len() -> TestResult { let mut buffer_out = [0u8; 65535 * 2]; noise.write_message(&[0u8; 0], &mut buffer_out)?; let mut noise = noise.into_transport_mode()?; - assert!(noise.write_message(&[0u8; 65534], &mut buffer_out).is_err()); + noise.write_message(&[0u8; 65534], &mut buffer_out).unwrap_err(); Ok(()) } @@ -564,7 +565,7 @@ fn test_transport_message_undersized_output_buffer() -> TestResult { let mut buffer_out = [0u8; 200]; noise.write_message(&[0u8; 0], &mut buffer_out)?; let mut noise = noise.into_transport_mode()?; - assert!(noise.write_message(&[0u8; 300], &mut buffer_out).is_err()); + noise.write_message(&[0u8; 300], &mut buffer_out).unwrap_err(); Ok(()) } @@ -576,7 +577,7 @@ fn test_oneway_initiator_enforcements() -> TestResult { let mut buffer_out = [0u8; 1024]; noise.write_message(&[0u8; 0], &mut buffer_out)?; let mut noise = noise.into_transport_mode()?; - assert!(noise.read_message(&[0u8; 1024], &mut buffer_out).is_err()); + noise.read_message(&[0u8; 1024], &mut buffer_out).unwrap_err(); Ok(()) } @@ -596,8 +597,8 @@ fn test_oneway_responder_enforcements() -> TestResult { let mut init = init.into_transport_mode()?; let mut resp = resp.into_transport_mode()?; - assert!(init.read_message(&[0u8; 1024], &mut buffer_init).is_err()); - assert!(resp.write_message(&[0u8; 1024], &mut buffer_resp).is_err()); + init.read_message(&[0u8; 1024], &mut buffer_init).unwrap_err(); + resp.write_message(&[0u8; 1024], &mut buffer_resp).unwrap_err(); Ok(()) } @@ -612,7 +613,7 @@ fn test_buffer_issues() -> TestResult { let len = h_i.write_message(b"abc", &mut buffer_msg)?; let res = h_r.read_message(&buffer_msg[..len], &mut buffer_out); - assert!(res.is_err()); + res.unwrap_err(); Ok(()) } @@ -636,17 +637,17 @@ fn test_read_buffer_issues() -> TestResult { let len = h_i.write_message(b"abc", &mut buffer_msg)?; let res = h_r.read_message(&buffer_msg[..len], &mut buffer_out); - assert!(res.is_ok()); + res.unwrap(); let len = h_r.write_message(b"abc", &mut buffer_msg)?; let res = h_i.read_message(&buffer_msg[..len], &mut buffer_out); - assert!(res.is_ok()); + res.unwrap(); let _len = h_i.write_message(b"abc", &mut buffer_msg)?; let res = h_r.read_message(&buffer_msg[..2], &mut buffer_out); - assert!(res.is_err()); + res.unwrap_err(); Ok(()) } @@ -676,7 +677,7 @@ fn test_buffer_issues_encrypted_handshake() -> TestResult { let len = h_i.write_message(b"abc", &mut buffer_msg)?; let res = h_r.read_message(&buffer_msg[..len], &mut buffer_out); - assert!(res.is_err()); + res.unwrap_err(); Ok(()) } @@ -947,12 +948,12 @@ fn test_stateful_nonce_increment_behavior() -> TestResult { corrupted[0] = corrupted[0].wrapping_add(1); // This should result in an error, but should not change any internal state - assert!(h_r.read_message(&corrupted, &mut buffer_out).is_err()); + h_r.read_message(&corrupted, &mut buffer_out).unwrap_err(); // This should now succeed as the nonce counter should have remained the same h_r.read_message(&buffer_msg[..len], &mut buffer_out)?; // This should now fail again as the nonce counter should have incremented - assert!(h_r.read_message(&buffer_msg[..len], &mut buffer_out).is_err()); + h_r.read_message(&buffer_msg[..len], &mut buffer_out).unwrap_err(); Ok(()) } diff --git a/tests/vectors.rs b/tests/vectors.rs index 932d146f..21c285b0 100644 --- a/tests/vectors.rs +++ b/tests/vectors.rs @@ -1,4 +1,6 @@ #![cfg(feature = "vector-tests")] +#![allow(clippy::restriction)] + #[macro_use] extern crate serde_derive; From eeab5403ce47200d39b5e850f280e21e6765ef11 Mon Sep 17 00:00:00 2001 From: Jake McGinty Date: Mon, 29 Jan 2024 21:01:28 -0800 Subject: [PATCH 2/7] move rekey, hkdf, hmac functions outside of trait --- src/cipherstate.rs | 18 ++++++++- src/handshakestate.rs | 6 +-- src/params/patterns.rs | 1 + src/resolvers/default.rs | 20 ++++++---- src/symmetricstate.rs | 73 ++++++++++++++++++++++++++++++++--- src/types.rs | 82 +--------------------------------------- tests/general.rs | 2 +- 7 files changed, 101 insertions(+), 101 deletions(-) diff --git a/src/cipherstate.rs b/src/cipherstate.rs index 19864db1..cdd19909 100644 --- a/src/cipherstate.rs +++ b/src/cipherstate.rs @@ -76,7 +76,7 @@ impl CipherState { } pub fn rekey(&mut self) { - self.cipher.rekey(); + rekey(&mut self.cipher); } pub fn rekey_manually(&mut self, key: &[u8; CIPHERKEYLEN]) { @@ -171,7 +171,7 @@ impl StatelessCipherState { } pub fn rekey(&mut self) { - self.cipher.rekey(); + rekey(&mut self.cipher); } pub fn rekey_manually(&mut self, key: &[u8; CIPHERKEYLEN]) { @@ -223,3 +223,17 @@ impl StatelessCipherStates { self.1.rekey_manually(key); } } + +/// Rekey according to Section 4.2 of the Noise Specification, with a default +/// implementation guaranteed to be secure for all ciphers. +pub(crate) fn rekey(cipher: &mut Box) { + let mut ciphertext = [0; CIPHERKEYLEN + TAGLEN]; + let ciphertext_len = cipher.encrypt(u64::MAX, &[], &[0; CIPHERKEYLEN], &mut ciphertext); + assert_eq!(ciphertext_len, ciphertext.len(), "returned ciphertext length not expected size"); + + // TODO(mcginty): use `split_array_ref` once stable to avoid memory inefficiency + let mut key = [0_u8; CIPHERKEYLEN]; + key.copy_from_slice(&ciphertext[..CIPHERKEYLEN]); + + cipher.set(&key); +} diff --git a/src/handshakestate.rs b/src/handshakestate.rs index 08bde035..c54c0e0c 100644 --- a/src/handshakestate.rs +++ b/src/handshakestate.rs @@ -172,7 +172,7 @@ impl HandshakeState { if !(dh.is_on() && key.is_on()) { return Err(StateProblem::MissingKeyMaterial.into()); } - dh.dh(&**key, &mut dh_out)?; + dh.dh(&key[..dh.pub_len()], &mut dh_out)?; Ok(dh_out) } @@ -257,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[usize::from(n)] { Some(psk) => { self.symmetricstate.mix_key_and_hash(&psk); }, @@ -391,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[usize::from(n)] { Some(psk) => { self.symmetricstate.mix_key_and_hash(&psk); }, diff --git a/src/params/patterns.rs b/src/params/patterns.rs index 1ace5252..68d944c6 100644 --- a/src/params/patterns.rs +++ b/src/params/patterns.rs @@ -193,6 +193,7 @@ impl FromStr for HandshakeModifier { /// Handshake modifiers that will be used during key exchange handshake. #[derive(Clone, PartialEq, Debug)] +#[non_exhaustive] pub struct HandshakeModifierList { /// List of parsed modifiers. pub list: Vec, diff --git a/src/resolvers/default.rs b/src/resolvers/default.rs index bf8a3754..6fa01420 100644 --- a/src/resolvers/default.rs +++ b/src/resolvers/default.rs @@ -27,6 +27,7 @@ use crate::{ /// pure-Rust (or nearly pure-Rust) implementations. #[allow(clippy::module_name_repetitions)] #[derive(Default)] +#[non_exhaustive] pub struct DefaultResolver; impl CryptoResolver for DefaultResolver { @@ -167,6 +168,7 @@ impl Dh for Dh25519 { } fn dh(&self, pubkey: &[u8], out: &mut [u8]) -> Result<(), Error> { + assert!(pubkey.len() == 32, "pubkey expected to be 32 bytes"); let mut pubkey_owned = [0_u8; CIPHERKEYLEN]; copy_slices!(&pubkey[..32], pubkey_owned); let result = MontgomeryPoint(pubkey_owned).mul_clamped(self.privkey).to_bytes(); @@ -522,13 +524,15 @@ impl Kem for Kyber1024 { #[cfg(test)] mod tests { + use crate::symmetricstate::hmac; + use super::*; use hex::FromHex; #[test] fn test_sha256() { let mut output = [0_u8; 32]; - let mut hasher = HashSHA256::default(); + let mut hasher = Box::new(HashSHA256::default()); hasher.input(b"abc"); hasher.result(&mut output); assert!( @@ -545,16 +549,16 @@ mod tests { ) .unwrap(); let mut output1 = [0_u8; 32]; - let mut hasher = HashSHA256::default(); - hasher.hmac(&key, &data, &mut output1); + let mut hasher: Box = Box::new(HashSHA256::default()); + hmac(&mut hasher, &key, &data, &mut output1); assert!( hex::encode(output1) == "773ea91e36800e46854db8ebd09181a72959098b3ef8c122d9635514ced565fe" ); let mut output2 = [0_u8; 64]; - let mut hasher = HashSHA512::default(); - hasher.hmac(&key, &data, &mut output2); + let mut hasher: Box = Box::new(HashSHA512::default()); + hmac(&mut hasher, &key, &data, &mut output2); assert!( hex::encode(output2) == "fa73b0089d56a284efb0f0756c890be9\ @@ -568,7 +572,7 @@ mod tests { fn test_blake2b() { // BLAKE2b test - draft-saarinen-blake2-06 let mut output = [0_u8; 64]; - let mut hasher = HashBLAKE2b::default(); + let mut hasher: Box = Box::new(HashBLAKE2b::default()); hasher.input(b"abc"); hasher.result(&mut output); assert!( @@ -584,7 +588,7 @@ mod tests { fn test_blake2s() { // BLAKE2s test - draft-saarinen-blake2-06 let mut output = [0_u8; 32]; - let mut hasher = HashBLAKE2s::default(); + let mut hasher: Box = Box::new(HashBLAKE2s::default()); hasher.input(b"abc"); hasher.result(&mut output); assert!( @@ -597,7 +601,7 @@ mod tests { #[test] fn test_curve25519() { // Curve25519 test - draft-curves-10 - let mut keypair = Dh25519::default(); + let mut keypair = Box::new(Dh25519::default()); let scalar = Vec::::from_hex("a546e36bf0527c9d3b16154b82465edd62144c0ac1fc5a18506a2244ba449ac4") .unwrap(); diff --git a/src/symmetricstate.rs b/src/symmetricstate.rs index 226bec13..00a96bef 100644 --- a/src/symmetricstate.rs +++ b/src/symmetricstate.rs @@ -1,12 +1,12 @@ use crate::{ cipherstate::CipherState, - constants::{CIPHERKEYLEN, MAXHASHLEN}, + constants::{CIPHERKEYLEN, MAXBLOCKLEN, MAXHASHLEN}, error::Error, types::Hash, }; #[derive(Copy, Clone)] -pub(in crate) struct SymmetricStateData { +pub(crate) struct SymmetricStateData { h: [u8; MAXHASHLEN], ck: [u8; MAXHASHLEN], has_key: bool, @@ -22,7 +22,7 @@ impl Default for SymmetricStateData { } } -pub(in crate) struct SymmetricState { +pub(crate) struct SymmetricState { cipherstate: CipherState, hasher: Box, inner: SymmetricStateData, @@ -48,7 +48,8 @@ impl SymmetricState { pub fn mix_key(&mut self, data: &[u8]) { let hash_len = self.hasher.hash_len(); let mut hkdf_output = ([0_u8; MAXHASHLEN], [0_u8; MAXHASHLEN]); - self.hasher.hkdf( + hkdf( + &mut self.hasher, &self.inner.ck[..hash_len], data, 2, @@ -77,7 +78,8 @@ impl SymmetricState { pub fn mix_key_and_hash(&mut self, data: &[u8]) { let hash_len = self.hasher.hash_len(); let mut hkdf_output = ([0_u8; MAXHASHLEN], [0_u8; MAXHASHLEN], [0_u8; MAXHASHLEN]); - self.hasher.hkdf( + hkdf( + &mut self.hasher, &self.inner.ck[..hash_len], data, 3, @@ -144,7 +146,7 @@ impl SymmetricState { pub fn split_raw(&mut self, out1: &mut [u8], out2: &mut [u8]) { let hash_len = self.hasher.hash_len(); - self.hasher.hkdf(&self.inner.ck[..hash_len], &[0_u8; 0], 2, out1, out2, &mut []); + hkdf(&mut self.hasher, &self.inner.ck[..hash_len], &[0_u8; 0], 2, out1, out2, &mut []); } pub(crate) fn checkpoint(&mut self) -> SymmetricStateData { @@ -160,3 +162,62 @@ impl SymmetricState { &self.inner.h[..hash_len] } } + +/// Calculate HMAC, as specified in the Noise spec. +/// +/// NOTE: This method clobbers the existing internal state +pub(crate) fn hmac(hasher: &mut Box, key: &[u8], data: &[u8], out: &mut [u8]) { + let key_len = key.len(); + let block_len = hasher.block_len(); + let hash_len = hasher.hash_len(); + assert!(key.len() <= block_len, "key and block lengths differ"); + let mut ipad = [0x36_u8; MAXBLOCKLEN]; + let mut opad = [0x5c_u8; MAXBLOCKLEN]; + for count in 0..key_len { + ipad[count] ^= key[count]; + opad[count] ^= key[count]; + } + hasher.reset(); + hasher.input(&ipad[..block_len]); + hasher.input(data); + let mut inner_output = [0_u8; MAXHASHLEN]; + hasher.result(&mut inner_output); + hasher.reset(); + hasher.input(&opad[..block_len]); + hasher.input(&inner_output[..hash_len]); + hasher.result(out); +} + +/// Derive keys as specified in the Noise spec. +/// +/// NOTE: This method clobbers the existing internal state +pub(crate) fn hkdf( + hasher: &mut Box, + chaining_key: &[u8], + input_key_material: &[u8], + outputs: usize, + out1: &mut [u8], + out2: &mut [u8], + out3: &mut [u8], +) { + let hash_len = hasher.hash_len(); + let mut temp_key = [0_u8; MAXHASHLEN]; + hmac(hasher, chaining_key, input_key_material, &mut temp_key); + hmac(hasher, &temp_key, &[1_u8], out1); + if outputs == 1 { + return; + } + + let mut in2 = [0_u8; MAXHASHLEN + 1]; + copy_slices!(out1[0..hash_len], &mut in2); + in2[hash_len] = 2; + hmac(hasher, &temp_key, &in2[..=hash_len], out2); + if outputs == 2 { + return; + } + + let mut in3 = [0_u8; MAXHASHLEN + 1]; + copy_slices!(out2[0..hash_len], &mut in3); + in3[hash_len] = 3; + hmac(hasher, &temp_key, &in3[..=hash_len], out3); +} diff --git a/src/types.rs b/src/types.rs index a038c075..faa7b654 100644 --- a/src/types.rs +++ b/src/types.rs @@ -1,9 +1,6 @@ //! The traits for cryptographic implementations that can be used by Noise. -use crate::{ - constants::{CIPHERKEYLEN, MAXBLOCKLEN, MAXHASHLEN, TAGLEN}, - Error, -}; +use crate::{constants::CIPHERKEYLEN, Error}; use rand_core::{CryptoRng, RngCore}; /// CSPRNG operations @@ -61,24 +58,6 @@ pub trait Cipher: Send + Sync { ciphertext: &[u8], out: &mut [u8], ) -> Result; - - /// Rekey according to Section 4.2 of the Noise Specification, with a default - /// implementation guaranteed to be secure for all ciphers. - fn rekey(&mut self) { - let mut ciphertext = [0; CIPHERKEYLEN + TAGLEN]; - let ciphertext_len = self.encrypt(u64::MAX, &[], &[0; CIPHERKEYLEN], &mut ciphertext); - assert_eq!( - ciphertext_len, - ciphertext.len(), - "returned ciphertext length not expected size" - ); - - // TODO(mcginty): use `split_array_ref` once stable to avoid memory inefficiency - let mut key = [0_u8; CIPHERKEYLEN]; - key.copy_from_slice(&ciphertext[..CIPHERKEYLEN]); - - self.set(&key); - } } /// Hashing operations @@ -100,65 +79,6 @@ pub trait Hash: Send + Sync { /// Get the resulting hash fn result(&mut self, out: &mut [u8]); - - /// Calculate HMAC, as specified in the Noise spec. - /// - /// NOTE: This method clobbers the existing internal state - fn hmac(&mut self, key: &[u8], data: &[u8], out: &mut [u8]) { - let key_len = key.len(); - let block_len = self.block_len(); - let hash_len = self.hash_len(); - assert!(key.len() <= block_len, "key and block lengths differ"); - let mut ipad = [0x36_u8; MAXBLOCKLEN]; - let mut opad = [0x5c_u8; MAXBLOCKLEN]; - for count in 0..key_len { - ipad[count] ^= key[count]; - opad[count] ^= key[count]; - } - self.reset(); - self.input(&ipad[..block_len]); - self.input(data); - let mut inner_output = [0_u8; MAXHASHLEN]; - self.result(&mut inner_output); - self.reset(); - self.input(&opad[..block_len]); - self.input(&inner_output[..hash_len]); - self.result(out); - } - - /// Derive keys as specified in the Noise spec. - /// - /// NOTE: This method clobbers the existing internal state - fn hkdf( - &mut self, - chaining_key: &[u8], - input_key_material: &[u8], - outputs: usize, - out1: &mut [u8], - out2: &mut [u8], - out3: &mut [u8], - ) { - let hash_len = self.hash_len(); - let mut temp_key = [0_u8; MAXHASHLEN]; - self.hmac(chaining_key, input_key_material, &mut temp_key); - self.hmac(&temp_key, &[1_u8], out1); - if outputs == 1 { - return; - } - - let mut in2 = [0_u8; MAXHASHLEN + 1]; - copy_slices!(out1[0..hash_len], &mut in2); - in2[hash_len] = 2; - self.hmac(&temp_key, &in2[..=hash_len], out2); - if outputs == 2 { - return; - } - - let mut in3 = [0_u8; MAXHASHLEN + 1]; - copy_slices!(out2[0..hash_len], &mut in3); - in3[hash_len] = 3; - self.hmac(&temp_key, &in3[..=hash_len], out3); - } } /// Kem operations. diff --git a/tests/general.rs b/tests/general.rs index b3058b22..4bd7a981 100644 --- a/tests/general.rs +++ b/tests/general.rs @@ -60,7 +60,7 @@ struct TestResolver { #[allow(unused)] impl TestResolver { pub fn new(next_byte: u8) -> Self { - TestResolver { next_byte, parent: DefaultResolver } + TestResolver { next_byte, parent: DefaultResolver::default() } } pub fn next_byte(&mut self, next_byte: u8) { From c2e45378d92142c6badb51d74c4b03108702792f Mon Sep 17 00:00:00 2001 From: Jake McGinty Date: Mon, 29 Jan 2024 22:00:31 -0800 Subject: [PATCH 3/7] more clippy tweaks --- Cargo.toml | 1 - src/symmetricstate.rs | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 316b373e..857f7967 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -46,7 +46,6 @@ mod_module_files = "warn" std_instead_of_core = "warn" exhaustive_structs = "warn" missing_trait_methods = "warn" -missing_inline_in_public_item = "warn" absolute_paths = "warn" as_conversions = "warn" shadow_reuse = "warn" diff --git a/src/symmetricstate.rs b/src/symmetricstate.rs index 00a96bef..eeb6ff45 100644 --- a/src/symmetricstate.rs +++ b/src/symmetricstate.rs @@ -166,6 +166,7 @@ impl SymmetricState { /// Calculate HMAC, as specified in the Noise spec. /// /// NOTE: This method clobbers the existing internal state +#[allow(clippy::similar_names)] pub(crate) fn hmac(hasher: &mut Box, key: &[u8], data: &[u8], out: &mut [u8]) { let key_len = key.len(); let block_len = hasher.block_len(); From 15c7eb1df179ed514aa81f4f7c8a452074527bb1 Mon Sep 17 00:00:00 2001 From: Jake McGinty Date: Tue, 30 Jan 2024 22:09:32 -0800 Subject: [PATCH 4/7] make rust-analyzer happy --- src/resolvers/default.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/resolvers/default.rs b/src/resolvers/default.rs index 6fa01420..db9514ed 100644 --- a/src/resolvers/default.rs +++ b/src/resolvers/default.rs @@ -335,7 +335,7 @@ impl Cipher for CipherXChaChaPoly { impl Default for HashSHA256 { fn default() -> HashSHA256 { - HashSHA256 { hasher: Sha256::new() } + HashSHA256 { hasher: Sha256::default() } } } @@ -353,7 +353,7 @@ impl Hash for HashSHA256 { } fn reset(&mut self) { - self.hasher = Sha256::new(); + self.hasher = Sha256::default(); } fn input(&mut self, data: &[u8]) { @@ -368,7 +368,7 @@ impl Hash for HashSHA256 { impl Default for HashSHA512 { fn default() -> HashSHA512 { - HashSHA512 { hasher: Sha512::new() } + HashSHA512 { hasher: Sha512::default() } } } @@ -386,7 +386,7 @@ impl Hash for HashSHA512 { } fn reset(&mut self) { - self.hasher = Sha512::new(); + self.hasher = Sha512::default(); } fn input(&mut self, data: &[u8]) { From 6d8c2c58bb38d3cea437488d14475a5e3dd60200 Mon Sep 17 00:00:00 2001 From: Jake McGinty Date: Fri, 9 Feb 2024 18:10:16 -0500 Subject: [PATCH 5/7] add clippy to github actions --- .github/workflows/rust.yml | 30 ++++++++++++++++-------------- ci-tests.sh | 1 + src/resolvers.rs | 3 ++- 3 files changed, 19 insertions(+), 15 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index f797860b..41b21457 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -3,10 +3,10 @@ name: Build on: push: branches: - - '*' + - "*" pull_request: branches: - - '*' + - "*" env: CARGO_TERM_COLOR: always @@ -20,15 +20,17 @@ jobs: os: [ubuntu-latest, macos-latest, windows-latest] steps: - - uses: actions/checkout@v2 - - name: Install LLVM and Clang - if: startsWith(matrix.os, 'windows') - uses: KyleMayes/install-llvm-action@v1 - with: - version: "11.0" - directory: ${{ runner.temp }}/llvm - - name: Set LIBCLANG_PATH - if: startsWith(matrix.os, 'windows') - run: echo "LIBCLANG_PATH=$((gcm clang).source -replace "clang.exe")" >> $env:GITHUB_ENV - - name: Run tests - run: bash ./ci-tests.sh + - uses: actions/checkout@v2 + - name: Install LLVM and Clang + if: startsWith(matrix.os, 'windows') + uses: KyleMayes/install-llvm-action@v1 + with: + version: "11.0" + directory: ${{ runner.temp }}/llvm + - name: Set LIBCLANG_PATH + if: startsWith(matrix.os, 'windows') + run: echo "LIBCLANG_PATH=$((gcm clang).source -replace "clang.exe")" >> $env:GITHUB_ENV + - name: Run tests + run: bash ./ci-tests.sh + - name: Run Clippy + run: cargo clippy --all-targets --all-features diff --git a/ci-tests.sh b/ci-tests.sh index a49562d6..110829a8 100755 --- a/ci-tests.sh +++ b/ci-tests.sh @@ -16,3 +16,4 @@ if ! rustc -vV | grep 'host: .*windows' &> /dev/null; then fi cargo test $TARGET --features "libsodium-resolver $COMMON_FEATURES" cargo test $TARGET --features "libsodium-accelerated $COMMON_FEATURES" + diff --git a/src/resolvers.rs b/src/resolvers.rs index e6c15dd5..53764c0b 100644 --- a/src/resolvers.rs +++ b/src/resolvers.rs @@ -60,7 +60,8 @@ pub struct FallbackResolver { impl FallbackResolver { /// Create a new `FallbackResolver` that holds the primary and secondary resolver. - #[must_use] pub fn new(preferred: BoxedCryptoResolver, fallback: BoxedCryptoResolver) -> Self { + #[must_use] + pub fn new(preferred: BoxedCryptoResolver, fallback: BoxedCryptoResolver) -> Self { Self { preferred, fallback } } } From 90abe6cdd3e1c487f184febd096382151059c66b Mon Sep 17 00:00:00 2001 From: Jake McGinty Date: Fri, 9 Feb 2024 19:11:59 -0500 Subject: [PATCH 6/7] refactor ci-tests.sh for clippy --- ci-tests.sh | 35 +++++++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/ci-tests.sh b/ci-tests.sh index 110829a8..0cf1ca02 100755 --- a/ci-tests.sh +++ b/ci-tests.sh @@ -3,17 +3,32 @@ set -e TARGET="$([ -n "$1" ] && echo "--target $1" || echo "")" COMMON_FEATURES="xchachapoly vector-tests" +COMMON_ARGS="--color=always" -set -x -cargo check --benches -cargo test $TARGET --no-default-features -cargo test $TARGET --features "$COMMON_FEATURES" -cargo test $TARGET --features "ring-resolver $COMMON_FEATURES" -cargo test $TARGET --features "ring-accelerated $COMMON_FEATURES" +FEATURE_SETS=( + "" # common features only + "ring-resolver" + "ring-accelerated" + "libsodium-resolver" + "libsodium-accelerated" +) if ! rustc -vV | grep 'host: .*windows' &> /dev/null; then - cargo test $TARGET --features "hfs pqclean_kyber1024 $COMMON_FEATURES" - cargo test $TARGET --features "ring-resolver hfs pqclean_kyber1024 $COMMON_FEATURES" + FEATURE_SETS+=("hfs pqclean_kyber1024") + FEATURE_SETS+=("ring-resolver hfs pqclean_kyber1024") fi -cargo test $TARGET --features "libsodium-resolver $COMMON_FEATURES" -cargo test $TARGET --features "libsodium-accelerated $COMMON_FEATURES" +cmd() { + echo -e "\033[34m=>\033[m "$@"" + output="$("$@" 2>&1)" || (echo "$output" && exit 1) +} + +cmd cargo check --benches + +cmd cargo test $COMMON_ARGS $TARGET --no-default-features +cmd cargo clippy $COMMON_ARGS --no-default-features + +for feature_set in ${FEATURE_SETS[@]}; do + features="$COMMON_FEATURES $feature_set" + cmd cargo test $COMMON_ARGS $TARGET --features "$features" + cmd cargo clippy $COMMON_ARGS --features "$features" -- -D warnings +done From bad5f2076a15e54f21becade38fc15b0d41102c8 Mon Sep 17 00:00:00 2001 From: Jake McGinty Date: Fri, 9 Feb 2024 19:12:13 -0500 Subject: [PATCH 7/7] remove specific clippy github action --- .github/workflows/rust.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 41b21457..766b012d 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -32,5 +32,3 @@ jobs: run: echo "LIBCLANG_PATH=$((gcm clang).source -replace "clang.exe")" >> $env:GITHUB_ENV - name: Run tests run: bash ./ci-tests.sh - - name: Run Clippy - run: cargo clippy --all-targets --all-features