Skip to content

Commit

Permalink
cargo clippy pedantic fixes
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
mcginty committed Jan 26, 2024
1 parent 357fbac commit da0ba4d
Show file tree
Hide file tree
Showing 17 changed files with 171 additions and 119 deletions.
12 changes: 6 additions & 6 deletions benches/benches.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
extern crate criterion;

use criterion::{Criterion, Throughput};
use snow::{params::*, *};
use snow::{params::NoiseParams, Builder};

const MSG_SIZE: usize = 4096;

Expand All @@ -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| {
Expand Down Expand Up @@ -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| {
Expand All @@ -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();

Expand Down Expand Up @@ -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();
})
});
});
}

Expand All @@ -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();
})
});
});
}

Expand Down
2 changes: 1 addition & 1 deletion build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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\"");
Expand Down
2 changes: 1 addition & 1 deletion examples/simple.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
44 changes: 35 additions & 9 deletions src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -105,11 +106,17 @@ impl<'builder> Builder<'builder> {
}

/// Create a Builder with a custom crypto resolver.
#[must_use]
pub fn with_resolver(params: NoiseParams, resolver: BoxedCryptoResolver) -> Self {
Builder { params, resolver, s: None, e_fixed: None, rs: None, plog: None, psks: [None; 10] }
}

/// Specify a PSK (only used with `NoisePSK` base parameter)
///
/// # Errors
/// * `InitError(InitStage::ValidatePskPosition)` if the location is a number larger than
/// allowed.
/// * `InitError(InitStage::ParameterOverwrite)` if this method has been called previously.
pub fn psk(mut self, location: u8, key: &'builder [u8; PSKLEN]) -> Result<Self, Error> {
let location = location as usize;
if location >= MAX_PSKS {
Expand All @@ -125,6 +132,9 @@ impl<'builder> Builder<'builder> {
/// Your static private key (can be generated with [`generate_keypair()`]).
///
/// [`generate_keypair()`]: #method.generate_keypair
///
/// # Errors
/// * `InitError(InitStage::ParameterOverwrite)` if this method has been called previously.
pub fn local_private_key(mut self, key: &'builder [u8]) -> Result<Self, Error> {
if self.s.is_some() {
Err(InitStage::ParameterOverwrite.into())
Expand All @@ -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
Expand All @@ -143,6 +154,9 @@ impl<'builder> Builder<'builder> {
/// Arbitrary data to be hashed in to the handshake hash value.
///
/// This may only be set once
///
/// # Errors
/// * `InitError(InitStage::ParameterOverwrite)` if this method has been called previously.
pub fn prologue(mut self, key: &'builder [u8]) -> Result<Self, Error> {
if self.plog.is_some() {
Err(InitStage::ParameterOverwrite.into())
Expand All @@ -153,6 +167,9 @@ impl<'builder> Builder<'builder> {
}

/// The responder's static public key.
///
/// # Errors
/// * `InitError(InitStage::ParameterOverwrite)` if this method has been called previously.
pub fn remote_public_key(mut self, pub_key: &'builder [u8]) -> Result<Self, Error> {
if self.rs.is_some() {
Err(InitStage::ParameterOverwrite.into())
Expand All @@ -164,6 +181,10 @@ impl<'builder> Builder<'builder> {

// TODO: performance issue w/ creating a new RNG and DH instance per call.
/// Generate a new asymmetric keypair (for use as a static key).
///
/// # Errors
/// * `InitError(InitStage::GetRngImpl)` if the RNG implementation failed to resolve.
/// * `InitError(InitStage::GetDhImpl)` if the DH implementation failed to resolve.
pub fn generate_keypair(&self) -> Result<Keypair, Error> {
let mut rng = self.resolver.resolve_rng().ok_or(InitStage::GetRngImpl)?;
let mut dh = self.resolver.resolve_dh(&self.params.dh).ok_or(InitStage::GetDhImpl)?;
Expand All @@ -178,11 +199,19 @@ impl<'builder> Builder<'builder> {
}

/// Build a [`HandshakeState`] for the side who will initiate the handshake (send the first message)
///
/// # Errors
/// * `InitError(InitStage::GetRngImpl)` if the RNG implementation failed to resolve.
/// * `InitError(InitStage::GetDhImpl)` if the DH implementation failed to resolve.
pub fn build_initiator(self) -> Result<HandshakeState, Error> {
self.build(true)
}

/// Build a [`HandshakeState`] for the side who will be responder (receive the first message)
///
/// # Errors
/// An `InitError(InitStage)` variant will be returned for various issues in the building of a
/// usable `HandshakeState`. See `InitStage` for further details.
pub fn build_responder(self) -> Result<HandshakeState, Error> {
self.build(false)
}
Expand Down Expand Up @@ -256,7 +285,7 @@ impl<'builder> Builder<'builder> {
re,
initiator,
self.params,
psks,
&psks,
self.plog.unwrap_or(&[]),
cipherstates,
)?;
Expand All @@ -265,6 +294,7 @@ impl<'builder> Builder<'builder> {
}

#[cfg(not(feature = "hfs"))]
#[allow(clippy::unnecessary_wraps)]
fn resolve_kem(_: Box<dyn CryptoResolver>, _: &mut HandshakeState) -> Result<(), Error> {
// HFS is disabled, return nothing
Ok(())
Expand Down Expand Up @@ -316,9 +346,7 @@ mod tests {
let params: ::std::result::Result<NoiseParams, _> =
"Noise_NK_25519_ChaChaPoly_BLAH256".parse();

if params.is_ok() {
panic!("NoiseParams should have failed");
}
assert!(params.is_err(), "NoiseParams should have failed");
}

#[test]
Expand All @@ -328,20 +356,18 @@ mod tests {
.local_private_key(&[0u8; 32])?
.build_initiator(); // missing remote key, should result in Err

if noise.is_ok() {
panic!("builder should have failed on build");
}
assert!(noise.is_err(), "builder should have failed on build");
Ok(())
}

#[test]
fn test_builder_param_overwrite() -> TestResult {
fn build_builder<'a>() -> Result<Builder<'a>, Error> {
Ok(Builder::new("Noise_NNpsk0_25519_ChaChaPoly_SHA256".parse()?)
Builder::new("Noise_NNpsk0_25519_ChaChaPoly_SHA256".parse()?)
.prologue(&[2u8; 10])?
.psk(0, &[0u8; 32])?
.local_private_key(&[0u8; 32])?
.remote_public_key(&[1u8; 32])?)
.remote_public_key(&[1u8; 32])
}

assert_eq!(
Expand Down
18 changes: 9 additions & 9 deletions src/cipherstate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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]) {
Expand Down Expand Up @@ -208,18 +208,18 @@ impl From<CipherStates> for StatelessCipherStates {

impl StatelessCipherStates {
pub fn rekey_initiator(&mut self) {
self.0.rekey()
self.0.rekey();
}

pub fn rekey_initiator_manually(&mut self, key: &[u8; CIPHERKEYLEN]) {
self.0.rekey_manually(key)
self.0.rekey_manually(key);
}

pub fn rekey_responder(&mut self) {
self.1.rekey()
self.1.rekey();
}

pub fn rekey_responder_manually(&mut self, key: &[u8; CIPHERKEYLEN]) {
self.1.rekey_manually(key)
self.1.rekey_manually(key);
}
}
8 changes: 4 additions & 4 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,14 +164,14 @@ impl From<StateProblem> for Error {
impl fmt::Display for Error {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Error::Pattern(reason) => write!(f, "pattern error: {:?}", reason),
Error::Pattern(reason) => write!(f, "pattern error: {reason:?}"),
Error::Init(reason) => {
write!(f, "initialization error: {:?}", reason)
write!(f, "initialization error: {reason:?}")
},
Error::Prereq(reason) => {
write!(f, "prerequisite error: {:?}", reason)
write!(f, "prerequisite error: {reason:?}")
},
Error::State(reason) => write!(f, "state error: {:?}", reason),
Error::State(reason) => write!(f, "state error: {reason:?}"),
Error::Input => write!(f, "input error"),
Error::Dh => write!(f, "diffie-hellman error"),
Error::Decrypt => write!(f, "decrypt error"),
Expand Down
Loading

0 comments on commit da0ba4d

Please sign in to comment.