Fix: integration tests with untested feature, clippy (#1)

PIV: formatting and lint improvements
iqlusioninc · Aug 14, 2024 · 80a3956 · 80a3956
1 parent 434d224
commit 80a3956
Show file tree

Hide file tree

Showing 6 changed files with 40 additions and 35 deletions.
diff --git a/src/error.rs b/src/error.rs
@@ -192,8 +192,8 @@ impl std::error::Error for Error {
     }
 }
 
-impl From<x509_cert::der::Error> for Error {
-    fn from(_err: x509_cert::der::Error) -> Error {
+impl From<der::Error> for Error {
+    fn from(_err: der::Error) -> Error {
         Error::ParseError
     }
 }
diff --git a/src/lib.rs b/src/lib.rs
@@ -71,7 +71,7 @@ pub use crate::{
     chuid::ChuId,
     config::Config,
     error::{Error, Result},
-    mgm::{MgmKey, MgmType, MgmKeyAlgorithm, MgmKey3Des, MgmKeyAes128, MgmKeyAes192, MgmKeyAes256},
+    mgm::{MgmKey, MgmKey3Des, MgmKeyAes128, MgmKeyAes192, MgmKeyAes256, MgmKeyAlgorithm, MgmType},
     piv::Key,
     policy::{PinPolicy, TouchPolicy},
     reader::Context,

diff --git a/src/mgm.rs b/src/mgm.rs
@@ -92,7 +92,7 @@ pub enum MgmType {
 
 /// The algorithm used for the MGM key.
 pub trait MgmKeyAlgorithm:
-    BlockCipher + BlockDecrypt + BlockEncrypt + KeyInit + private::Seal
+    BlockCipher + BlockDecrypt + BlockEncrypt + Clone + KeyInit + private::Seal
 {
     /// The KeySized used for this algorithm
     const KEY_SIZE: u8;
@@ -172,7 +172,7 @@ impl MgmKeyAlgorithm for des::TdesEee3 {
                     &key_bytes
                 );
 
-                return Err(Error::KeyError)
+                return Err(Error::KeyError);
             }
         }
 
@@ -459,8 +459,7 @@ impl<C: MgmKeyAlgorithm> MgmKey<C> {
 
         let mut output = challenge.to_owned();
 
-        C::new(&self.key)
-            .decrypt_block(GenericArray::from_mut_slice(&mut output));
+        C::new(&self.key).decrypt_block(GenericArray::from_mut_slice(&mut output));
 
         Ok(output)
     }
@@ -473,8 +472,7 @@ impl<C: MgmKeyAlgorithm> MgmKey<C> {
             return Err(Error::AuthenticationError);
         }
 
-        C::new(&self.key)
-            .encrypt_block(GenericArray::from_mut_slice(&mut response));
+        C::new(&self.key).encrypt_block(GenericArray::from_mut_slice(&mut response));
 
         use subtle::ConstantTimeEq;
         if response.ct_eq(auth_data).unwrap_u8() != 1 {

diff --git a/src/transaction.rs b/src/transaction.rs
@@ -244,7 +244,11 @@ impl<'tx> Transaction<'tx> {
 
     /// Set the management key (MGM).
     #[cfg(feature = "untested")]
-    pub fn set_mgm_key<C: MgmKeyAlgorithm>(&self, new_key: &MgmKey<C>, require_touch: bool) -> Result<()> {
+    pub fn set_mgm_key<C: MgmKeyAlgorithm>(
+        &self,
+        new_key: &MgmKey<C>,
+        require_touch: bool,
+    ) -> Result<()> {
         let p2 = if require_touch { 0xfe } else { 0xff };
 
         let mut data = Vec::with_capacity(new_key.key_size() as usize + 3);

diff --git a/src/yubikey.rs b/src/yubikey.rs
@@ -195,8 +195,8 @@ impl YubiKey {
                 if let Some(yk_stored) = yubikey {
                     // We found two YubiKeys, so we won't use either.
                     // Don't reset them.
-                    let _ = yk_stored.disconnect(pcsc::Disposition::LeaveCard);
-                    let _ = yk_found.disconnect(pcsc::Disposition::LeaveCard);
+                    let _ = yk_stored.disconnect(Disposition::LeaveCard);
+                    let _ = yk_found.disconnect(Disposition::LeaveCard);
 
                     error!("multiple YubiKeys detected!");
                     return Err(Error::PcscError { inner: None });
@@ -243,7 +243,7 @@ impl YubiKey {
                 return Ok(yubikey);
             } else {
                 // We didn't want this YubiKey; don't reset it.
-                let _ = yubikey.disconnect(pcsc::Disposition::LeaveCard);
+                let _ = yubikey.disconnect(Disposition::LeaveCard);
             }
         }
 
@@ -263,7 +263,7 @@ impl YubiKey {
         self.card.reconnect(
             pcsc::ShareMode::Shared,
             pcsc::Protocols::T1,
-            pcsc::Disposition::ResetCard,
+            Disposition::ResetCard,
         )?;
 
         let pin = self
@@ -373,7 +373,11 @@ impl YubiKey {
 
         let mut data = Vec::with_capacity(4 + challenge_len + 2 + challenge_len);
         data.push(TAG_DYN_AUTH);
-        data.push((2 + challenge_len + 2 + challenge_len).try_into().unwrap());
+        data.push(
+            (2 + challenge_len + 2 + challenge_len)
+                .try_into()
+                .expect("value fits in u8"),
+        );
         data.push(0x80);
         data.push(challenge_len as u8);
         data.extend_from_slice(&card_challenge);
@@ -672,7 +676,7 @@ impl<'a> TryFrom<&'a Reader<'_>> for YubiKey {
                 // a side-effect of determining this. Avoid disrupting its internal state
                 // any further (e.g. preserve the PIN cache of whatever applet is selected
                 // currently).
-                if let Err((_, e)) = card.disconnect(pcsc::Disposition::LeaveCard) {
+                if let Err((_, e)) = card.disconnect(Disposition::LeaveCard) {
                     error!("Failed to disconnect gracefully from card: {}", e);
                 }
 

diff --git a/tests/integration.rs b/tests/integration.rs
@@ -12,13 +12,13 @@ use signature::hazmat::PrehashVerifier;
 use std::{env, str::FromStr, sync::Mutex, time::Duration};
 use x509_cert::{der::Encode, name::Name, serial_number::SerialNumber, time::Validity};
 use yubikey::{
-    certificate,
     certificate::yubikey_signer,
     certificate::Certificate,
-    piv::{self, AlgorithmId, Key, ManagementSlotId, RetiredSlotId, SlotId},
-    Error, MgmKey, MgmKey3Des, MgmKeyAes128, MgmKeyAes192, MgmKeyAes256, MgmKeyAlgorithm,
-    PinPolicy, Serial, TouchPolicy, YubiKey,
+    piv::{self, AlgorithmId, Key, ManagementAlgorithmId, ManagementSlotId, RetiredSlotId, SlotId},
+    Error, MgmKey3Des, MgmKeyAes192, PinPolicy, Serial, TouchPolicy, YubiKey,
 };
+#[cfg(feature = "untested")]
+use yubikey::{MgmKey, MgmKeyAlgorithm};
 
 static YUBIKEY: Lazy<Mutex<YubiKey>> = Lazy::new(|| {
     // Only show logs if `RUST_LOG` is set
@@ -114,15 +114,15 @@ fn get_mgm_key_meta(yubikey: &mut YubiKey) -> piv::SlotMetadata {
 }
 
 /// Given a default YubiKey, authenticate with the default management key.
-/// 
+///
 /// This is slightly complicated by newer firmwares using AES192 as the MGM default key,
 /// over 3DES.
 fn auth_default_mgm(yubikey: &mut YubiKey) {
     match get_mgm_key_meta(yubikey).algorithm {
-        piv::ManagementAlgorithmId::ThreeDes => {
+        ManagementAlgorithmId::ThreeDes => {
             assert!(yubikey.authenticate(MgmKey3Des::default()).is_ok())
         }
-        piv::ManagementAlgorithmId::Aes192 => {
+        ManagementAlgorithmId::Aes192 => {
             assert!(yubikey.authenticate(MgmKeyAes192::default()).is_ok())
         }
         other => panic!("unexpected management key algorithm: {:?}", other),
@@ -142,35 +142,35 @@ fn test_set_mgmkey() {
     assert!(yubikey.verify_pin(b"123456").is_ok());
 
     fn test_mgm<M: MgmKeyAlgorithm>(yubikey: &mut YubiKey) {
-        assert!(yubikey.authenticate::<M>(MgmKey::default::<M>()).is_ok());
+        assert!(yubikey.authenticate::<M>(MgmKey::default()).is_ok());
         assert!(MgmKey::<M>::get_protected(yubikey).is_err());
 
         // Set a protected management key.
-        assert!(MgmKey::generate::<M>().set_protected(yubikey).is_ok());
-        let protected = MgmKey::get_protected::<M>(yubikey).unwrap();
-        assert!(yubikey.authenticate::<M>(MgmKey::default::<M>()).is_err());
+        assert!(MgmKey::<M>::generate().set_protected(yubikey).is_ok());
+        let protected = MgmKey::<M>::get_protected(yubikey).unwrap();
+        assert!(yubikey.authenticate::<M>(MgmKey::default()).is_err());
         assert!(yubikey.authenticate(protected.clone()).is_ok());
 
         // Set a manual management key.
         let manual = MgmKey::<M>::generate();
-        assert!(manual.set_manual(&mut yubikey, false).is_ok());
-        assert!(MgmKey::<M>::get_protected(&mut yubikey).is_err());
+        assert!(manual.set_manual(yubikey, false).is_ok());
+        assert!(MgmKey::<M>::get_protected(yubikey).is_err());
         assert!(yubikey.authenticate(MgmKey::<M>::default()).is_err());
         assert!(yubikey.authenticate(protected.clone()).is_err());
         assert!(yubikey.authenticate(manual.clone()).is_ok());
 
         // Set back to the default management key.
-        assert!(MgmKey::<M>::set_default(&mut yubikey).is_ok());
-        assert!(MgmKey::<M>::get_protected(&mut yubikey).is_err());
+        assert!(MgmKey::<M>::set_default(yubikey).is_ok());
+        assert!(MgmKey::<M>::get_protected(yubikey).is_err());
         assert!(yubikey.authenticate(protected).is_err());
         assert!(yubikey.authenticate(manual).is_err());
         assert!(yubikey.authenticate(MgmKey::<M>::default()).is_ok());
     }
 
-    match get_mgm_admin(&mut yubikey).algorithm {
+    match get_mgm_key_meta(&mut yubikey).algorithm {
         ManagementAlgorithmId::ThreeDes => test_mgm::<des::TdesEee3>(&mut yubikey),
         ManagementAlgorithmId::Aes192 => test_mgm::<aes::Aes192>(&mut yubikey),
-        ManagementAlgorithmId::Aes128 | ManagementAlgorithmId::Aes192 => {
+        ManagementAlgorithmId::Aes128 | ManagementAlgorithmId::Aes256 => {
             panic!("AES128 or AES256 should not be a default key")
         }
         other => panic!("unexpected management key algorithm: {:?}", other),
@@ -349,8 +349,7 @@ fn test_read_metadata() {
 #[ignore]
 fn test_parse_cert_from_der() {
     let bob_der = std::fs::read("tests/assets/Bob.der").expect(".der file not found");
-    let cert =
-        certificate::Certificate::from_bytes(bob_der).expect("Failed to parse valid certificate");
+    let cert = Certificate::from_bytes(bob_der).expect("Failed to parse valid certificate");
     assert_eq!(
         cert.subject(),
         "CN=Bob",