Clippy strictness linting

.github/workflows/rust.yml

@@ -3,10 +3,10 @@ name: Build
 on:
   push:
     branches:
-      - '*'
+      - "*"
   pull_request:
     branches:
-      - '*'
+      - "*"
 
 env:
   CARGO_TERM_COLOR: always
@@ -20,15 +20,15 @@ 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

Cargo.toml

@@ -36,6 +36,28 @@ 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"
+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"

benches/benches.rs

@@ -1,3 +1,5 @@
+#![allow(clippy::restriction)]
+
 #[macro_use]
 extern crate criterion;
 

ci-tests.sh

@@ -3,16 +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

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<Vec<u8>> {
-    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)
 }

src/builder.rs

@@ -1,4 +1,4 @@
-use std::fmt::Debug;
+use core::fmt::{self, Debug};
 
 #[cfg(feature = "hfs")]
 use crate::params::HandshakeModifier;
@@ -19,13 +19,15 @@ 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<u8>,
     /// The public asymmetric key
     pub public:  Vec<u8>,
 }
 
+#[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<Self, Error> {
-        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<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)?;
-        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,15 +262,15 @@ 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() {
             if let Some(key) = *psk {
                 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<dyn std::error::Error>>;
@@ -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)

src/cipherstate.rs

@@ -68,15 +68,15 @@ impl CipherState {
     }
 
     pub fn encrypt(&mut self, plaintext: &[u8], out: &mut [u8]) -> Result<usize, Error> {
-        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<usize, Error> {
-        self.decrypt_ad(&[0u8; 0], ciphertext, out)
+        self.decrypt_ad(&[0_u8; 0], ciphertext, out)
     }
 
     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<dyn Cipher>) {
+    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);
+}