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 25, 2024
1 parent 752c071 commit c381619
Show file tree
Hide file tree
Showing 16 changed files with 127 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
18 changes: 9 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,6 +106,7 @@ 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] }
}
Expand Down Expand Up @@ -135,6 +137,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 Down Expand Up @@ -256,7 +259,7 @@ impl<'builder> Builder<'builder> {
re,
initiator,
self.params,
psks,
&psks,
self.plog.unwrap_or(&[]),
cipherstates,
)?;
Expand All @@ -265,6 +268,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 +320,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 +330,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
41 changes: 20 additions & 21 deletions src/handshakestate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<HandshakeState, Error> {
Expand Down Expand Up @@ -140,7 +140,7 @@ impl HandshakeState {
re,
initiator,
params,
psks,
psks: *psks,
#[cfg(feature = "hfs")]
kem: None,
#[cfg(feature = "hfs")]
Expand All @@ -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),
Expand Down Expand Up @@ -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()
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
},
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
},
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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)
}

Expand Down Expand Up @@ -476,28 +470,33 @@ 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()])
}

/// 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
}
Expand Down
Loading

0 comments on commit c381619

Please sign in to comment.