Fix ctap1 clippy warning (#517)

And improve clippy workflows.
google · Jul 22, 2022 · 9bb1a2f · 9bb1a2f
1 parent 168de29
commit 9bb1a2f
Show file tree

Hide file tree

Showing 8 changed files with 68 additions and 66 deletions.
diff --git a/.github/workflows/cargo_clippy.yml b/.github/workflows/cargo_clippy.yml
@@ -26,5 +26,9 @@ jobs:
         with:
           token: ${{ secrets.GITHUB_TOKEN }}
           args: --all-targets --features std
-      - name: Deny Clippy warnings
+      - name: Deny Clippy warnings (std)
         run: cargo clippy --all-targets --features std -- -A clippy::new_without_default -D warnings
+      - name: Deny Clippy warnings (all)
+        run: cargo clippy --all-targets --features std,with_ctap1,ed25519,vendor_hid -- -A clippy::new_without_default -D warnings
+      - name: Deny Clippy warnings (all, nfc)
+        run: cargo clippy --all-targets --features std,with_ctap1,with_nfc,ed25519,vendor_hid -- -A clippy::new_without_default -D warnings
diff --git a/run_desktop_tests.sh b/run_desktop_tests.sh
@@ -38,8 +38,8 @@ cd ..
 
 echo "Running Clippy lints..."
 cargo clippy --all-targets --features std -- -A clippy::new_without_default -D warnings
-cargo clippy --all-targets --features std,with_nfc -- -A clippy::new_without_default -D warnings
-cargo clippy --all-targets --features std,vendor_hid -- -A clippy::new_without_default -D warnings
+cargo clippy --all-targets --features std,with_ctap1,ed25519,vendor_hid -- -A clippy::new_without_default -D warnings
+cargo clippy --all-targets --features std,with_ctap1,with_nfc,ed25519,vendor_hid -- -A clippy::new_without_default -D warnings
 
 echo "Building sha256sum tool..."
 cargo build --manifest-path third_party/tock/tools/sha256sum/Cargo.toml

diff --git a/src/ctap/command.rs b/src/ctap/command.rs
@@ -1103,7 +1103,7 @@ mod test {
                 data: vec![0xFF; 0x100],
                 hash: vec![0x44; 32],
                 signature: Some(CoseSignature {
-                    algorithm: SignatureAlgorithm::ES256,
+                    algorithm: SignatureAlgorithm::Es256,
                     bytes: [0x55; 64],
                 }),
             })

diff --git a/src/ctap/credential_id.rs b/src/ctap/credential_id.rs
@@ -288,19 +288,19 @@ mod test {
 
     #[test]
     fn test_encrypt_decrypt_ecdsa_credential() {
-        test_encrypt_decrypt_credential(SignatureAlgorithm::ES256);
+        test_encrypt_decrypt_credential(SignatureAlgorithm::Es256);
     }
 
     #[test]
     #[cfg(feature = "ed25519")]
     fn test_encrypt_decrypt_ed25519_credential() {
-        test_encrypt_decrypt_credential(SignatureAlgorithm::EDDSA);
+        test_encrypt_decrypt_credential(SignatureAlgorithm::Eddsa);
     }
 
     #[test]
     fn test_encrypt_decrypt_bad_version() {
         let mut env = TestEnv::new();
-        let private_key = PrivateKey::new(&mut env, SignatureAlgorithm::ES256);
+        let private_key = PrivateKey::new(&mut env, SignatureAlgorithm::Es256);
 
         let rp_id_hash = [0x55; 32];
         let mut encrypted_id =
@@ -337,13 +337,13 @@ mod test {
 
     #[test]
     fn test_ecdsa_encrypt_decrypt_bad_hmac() {
-        test_encrypt_decrypt_bad_hmac(SignatureAlgorithm::ES256);
+        test_encrypt_decrypt_bad_hmac(SignatureAlgorithm::Es256);
     }
 
     #[test]
     #[cfg(feature = "ed25519")]
     fn test_ed25519_encrypt_decrypt_bad_hmac() {
-        test_encrypt_decrypt_bad_hmac(SignatureAlgorithm::EDDSA);
+        test_encrypt_decrypt_bad_hmac(SignatureAlgorithm::Eddsa);
     }
 
     fn test_decrypt_credential_missing_blocks(signature_algorithm: SignatureAlgorithm) {
@@ -369,13 +369,13 @@ mod test {
 
     #[test]
     fn test_ecdsa_decrypt_credential_missing_blocks() {
-        test_decrypt_credential_missing_blocks(SignatureAlgorithm::ES256);
+        test_decrypt_credential_missing_blocks(SignatureAlgorithm::Es256);
     }
 
     #[test]
     #[cfg(feature = "ed25519")]
     fn test_ed25519_decrypt_credential_missing_blocks() {
-        test_decrypt_credential_missing_blocks(SignatureAlgorithm::EDDSA);
+        test_decrypt_credential_missing_blocks(SignatureAlgorithm::Eddsa);
     }
 
     /// This is a copy of the function that genereated deprecated key handles.
@@ -420,7 +420,7 @@ mod test {
     #[test]
     fn test_encrypt_credential_size() {
         let mut env = TestEnv::new();
-        let private_key = PrivateKey::new(&mut env, SignatureAlgorithm::ES256);
+        let private_key = PrivateKey::new(&mut env, SignatureAlgorithm::Es256);
 
         let rp_id_hash = [0x55; 32];
         let encrypted_id =
@@ -431,7 +431,7 @@ mod test {
     #[test]
     fn test_check_cred_protect_fail() {
         let mut env = TestEnv::new();
-        let private_key = PrivateKey::new(&mut env, SignatureAlgorithm::ES256);
+        let private_key = PrivateKey::new(&mut env, SignatureAlgorithm::Es256);
 
         let rp_id_hash = [0x55; 32];
         let encrypted_id = encrypt_to_credential_id(
@@ -451,7 +451,7 @@ mod test {
     #[test]
     fn test_check_cred_protect_success() {
         let mut env = TestEnv::new();
-        let private_key = PrivateKey::new(&mut env, SignatureAlgorithm::ES256);
+        let private_key = PrivateKey::new(&mut env, SignatureAlgorithm::Es256);
 
         let rp_id_hash = [0x55; 32];
         let encrypted_id = encrypt_to_credential_id(

diff --git a/src/ctap/crypto_wrapper.rs b/src/ctap/crypto_wrapper.rs
@@ -92,11 +92,11 @@ impl PrivateKey {
     /// Panics if the algorithm is [`SignatureAlgorithm::Unknown`].
     pub fn new(env: &mut impl Env, alg: SignatureAlgorithm) -> Self {
         match alg {
-            SignatureAlgorithm::ES256 => {
+            SignatureAlgorithm::Es256 => {
                 PrivateKey::Ecdsa(env.key_store().generate_ecdsa_seed().unwrap())
             }
             #[cfg(feature = "ed25519")]
-            SignatureAlgorithm::EDDSA => {
+            SignatureAlgorithm::Eddsa => {
                 let bytes = env.rng().gen_uniform_u8x32();
                 Self::new_ed25519_from_bytes(&bytes).unwrap()
             }
@@ -106,7 +106,7 @@ impl PrivateKey {
 
     /// Creates a new ecdsa private key.
     pub fn new_ecdsa(env: &mut impl Env) -> PrivateKey {
-        Self::new(env, SignatureAlgorithm::ES256)
+        Self::new(env, SignatureAlgorithm::Es256)
     }
 
     /// Helper function that creates a private key of type ECDSA.
@@ -166,9 +166,9 @@ impl PrivateKey {
     /// The associated COSE signature algorithm identifier.
     pub fn signature_algorithm(&self) -> SignatureAlgorithm {
         match self {
-            PrivateKey::Ecdsa(_) => SignatureAlgorithm::ES256,
+            PrivateKey::Ecdsa(_) => SignatureAlgorithm::Es256,
             #[cfg(feature = "ed25519")]
-            PrivateKey::Ed25519(_) => SignatureAlgorithm::EDDSA,
+            PrivateKey::Ed25519(_) => SignatureAlgorithm::Eddsa,
         }
     }
 
@@ -209,10 +209,10 @@ impl TryFrom<cbor::Value> for PrivateKey {
         }
         let key_bytes = extract_byte_string(array.pop().unwrap())?;
         match SignatureAlgorithm::try_from(array.pop().unwrap())? {
-            SignatureAlgorithm::ES256 => PrivateKey::new_ecdsa_from_bytes(&key_bytes)
+            SignatureAlgorithm::Es256 => PrivateKey::new_ecdsa_from_bytes(&key_bytes)
                 .ok_or(Ctap2StatusCode::CTAP2_ERR_INVALID_CBOR),
             #[cfg(feature = "ed25519")]
-            SignatureAlgorithm::EDDSA => PrivateKey::new_ed25519_from_bytes(&key_bytes)
+            SignatureAlgorithm::Eddsa => PrivateKey::new_ed25519_from_bytes(&key_bytes)
                 .ok_or(Ctap2StatusCode::CTAP2_ERR_INVALID_CBOR),
             _ => Err(Ctap2StatusCode::CTAP2_ERR_INVALID_CBOR),
         }
@@ -292,7 +292,7 @@ mod test {
     #[test]
     fn test_new_ecdsa_from_bytes() {
         let mut env = TestEnv::new();
-        let private_key = PrivateKey::new(&mut env, SignatureAlgorithm::ES256);
+        let private_key = PrivateKey::new(&mut env, SignatureAlgorithm::Es256);
         let key_bytes = private_key.to_bytes();
         assert_eq!(
             PrivateKey::new_ecdsa_from_bytes(&key_bytes),
@@ -304,7 +304,7 @@ mod test {
     #[cfg(feature = "ed25519")]
     fn test_new_ed25519_from_bytes() {
         let mut env = TestEnv::new();
-        let private_key = PrivateKey::new(&mut env, SignatureAlgorithm::EDDSA);
+        let private_key = PrivateKey::new(&mut env, SignatureAlgorithm::Eddsa);
         let key_bytes = private_key.to_bytes();
         assert_eq!(
             PrivateKey::new_ed25519_from_bytes(&key_bytes),
@@ -362,13 +362,13 @@ mod test {
 
     #[test]
     fn test_ecdsa_private_key_signature_algorithm() {
-        test_private_key_signature_algorithm(SignatureAlgorithm::ES256);
+        test_private_key_signature_algorithm(SignatureAlgorithm::Es256);
     }
 
     #[test]
     #[cfg(feature = "ed25519")]
     fn test_ed25519_private_key_signature_algorithm() {
-        test_private_key_signature_algorithm(SignatureAlgorithm::EDDSA);
+        test_private_key_signature_algorithm(SignatureAlgorithm::Eddsa);
     }
 
     fn test_private_key_from_to_cbor(signature_algorithm: SignatureAlgorithm) {
@@ -380,13 +380,13 @@ mod test {
 
     #[test]
     fn test_ecdsa_private_key_from_to_cbor() {
-        test_private_key_from_to_cbor(SignatureAlgorithm::ES256);
+        test_private_key_from_to_cbor(SignatureAlgorithm::Es256);
     }
 
     #[test]
     #[cfg(feature = "ed25519")]
     fn test_ed25519_private_key_from_to_cbor() {
-        test_private_key_from_to_cbor(SignatureAlgorithm::EDDSA);
+        test_private_key_from_to_cbor(SignatureAlgorithm::Eddsa);
     }
 
     fn test_private_key_from_bad_cbor(signature_algorithm: SignatureAlgorithm) {
@@ -404,13 +404,13 @@ mod test {
 
     #[test]
     fn test_ecdsa_private_key_from_bad_cbor() {
-        test_private_key_from_bad_cbor(SignatureAlgorithm::ES256);
+        test_private_key_from_bad_cbor(SignatureAlgorithm::Es256);
     }
 
     #[test]
     #[cfg(feature = "ed25519")]
     fn test_ed25519_private_key_from_bad_cbor() {
-        test_private_key_from_bad_cbor(SignatureAlgorithm::EDDSA);
+        test_private_key_from_bad_cbor(SignatureAlgorithm::Eddsa);
     }
 
     #[test]

diff --git a/src/ctap/ctap1.rs b/src/ctap/ctap1.rs
@@ -21,7 +21,7 @@ use crate::api::attestation_store::{self, Attestation, AttestationStore};
 use crate::env::Env;
 use alloc::vec::Vec;
 use arrayref::array_ref;
-use core::convert::{Into, TryFrom};
+use core::convert::TryFrom;
 
 // For now, they're the same thing with apdu.rs containing the authoritative definition
 pub type Ctap1StatusCode = ApduStatusCode;
@@ -49,9 +49,9 @@ impl TryFrom<u8> for Ctap1Flags {
     }
 }
 
-impl Into<u8> for Ctap1Flags {
-    fn into(self) -> u8 {
-        self as u8
+impl From<Ctap1Flags> for u8 {
+    fn from(flags: Ctap1Flags) -> u8 {
+        flags as u8
     }
 }
 
@@ -79,9 +79,7 @@ impl TryFrom<&[u8]> for U2fCommand {
     fn try_from(message: &[u8]) -> Result<Self, Ctap1StatusCode> {
         let apdu: Apdu = match Apdu::try_from(message) {
             Ok(apdu) => apdu,
-            Err(apdu_status_code) => {
-                return Err(Ctap1StatusCode::try_from(apdu_status_code).unwrap())
-            }
+            Err(apdu_status_code) => return Err(apdu_status_code),
         };
 
         let lc = apdu.lc as usize;
@@ -373,7 +371,7 @@ mod test {
     fn create_authenticate_message(
         application: &[u8; 32],
         flags: Ctap1Flags,
-        key_handle: &Vec<u8>,
+        key_handle: &[u8],
     ) -> Vec<u8> {
         let mut message = vec![
             Ctap1Command::CTAP1_CLA,
@@ -496,7 +494,7 @@ mod test {
         let mut env = TestEnv::new();
         env.user_presence()
             .set(|| panic!("Unexpected user presence check in CTAP1"));
-        let sk = PrivateKey::new(&mut env, SignatureAlgorithm::ES256);
+        let sk = PrivateKey::new(&mut env, SignatureAlgorithm::Es256);
         let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0));
 
         let rp_id = "example.com";
@@ -514,7 +512,7 @@ mod test {
         let mut env = TestEnv::new();
         env.user_presence()
             .set(|| panic!("Unexpected user presence check in CTAP1"));
-        let sk = PrivateKey::new(&mut env, SignatureAlgorithm::ES256);
+        let sk = PrivateKey::new(&mut env, SignatureAlgorithm::Es256);
         let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0));
 
         let rp_id = "example.com";
@@ -533,7 +531,7 @@ mod test {
         let mut env = TestEnv::new();
         env.user_presence()
             .set(|| panic!("Unexpected user presence check in CTAP1"));
-        let sk = PrivateKey::new(&mut env, SignatureAlgorithm::ES256);
+        let sk = PrivateKey::new(&mut env, SignatureAlgorithm::Es256);
         let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0));
 
         let rp_id = "example.com";
@@ -571,7 +569,7 @@ mod test {
         let mut env = TestEnv::new();
         env.user_presence()
             .set(|| panic!("Unexpected user presence check in CTAP1"));
-        let sk = PrivateKey::new(&mut env, SignatureAlgorithm::ES256);
+        let sk = PrivateKey::new(&mut env, SignatureAlgorithm::Es256);
         let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0));
 
         let rp_id = "example.com";
@@ -591,7 +589,7 @@ mod test {
         let mut env = TestEnv::new();
         env.user_presence()
             .set(|| panic!("Unexpected user presence check in CTAP1"));
-        let sk = PrivateKey::new(&mut env, SignatureAlgorithm::ES256);
+        let sk = PrivateKey::new(&mut env, SignatureAlgorithm::Es256);
         let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0));
 
         let rp_id = "example.com";
@@ -611,7 +609,7 @@ mod test {
         let mut env = TestEnv::new();
         env.user_presence()
             .set(|| panic!("Unexpected user presence check in CTAP1"));
-        let sk = PrivateKey::new(&mut env, SignatureAlgorithm::ES256);
+        let sk = PrivateKey::new(&mut env, SignatureAlgorithm::Es256);
         let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0));
 
         let rp_id = "example.com";
@@ -639,7 +637,7 @@ mod test {
         let mut env = TestEnv::new();
         env.user_presence()
             .set(|| panic!("Unexpected user presence check in CTAP1"));
-        let sk = PrivateKey::new(&mut env, SignatureAlgorithm::ES256);
+        let sk = PrivateKey::new(&mut env, SignatureAlgorithm::Es256);
         let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0));
 
         let rp_id = "example.com";
@@ -667,7 +665,7 @@ mod test {
         let mut env = TestEnv::new();
         env.user_presence()
             .set(|| panic!("Unexpected user presence check in CTAP1"));
-        let sk = PrivateKey::new(&mut env, SignatureAlgorithm::ES256);
+        let sk = PrivateKey::new(&mut env, SignatureAlgorithm::Es256);
         let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0));
 
         let rp_id = "example.com";