From 80a3956c8baf18687ad3d4063f8f514216aee40c Mon Sep 17 00:00:00 2001 From: "Riad S. Wahby" <716593+kwantam@users.noreply.github.com> Date: Wed, 14 Aug 2024 16:57:20 -0400 Subject: [PATCH] Fix: integration tests with `untested` feature, clippy (#1) PIV: formatting and lint improvements --- src/error.rs | 4 ++-- src/lib.rs | 2 +- src/mgm.rs | 10 ++++------ src/transaction.rs | 6 +++++- src/yubikey.rs | 16 ++++++++++------ tests/integration.rs | 37 ++++++++++++++++++------------------- 6 files changed, 40 insertions(+), 35 deletions(-) diff --git a/src/error.rs b/src/error.rs index 7774b002..2f0d35e8 100644 --- a/src/error.rs +++ b/src/error.rs @@ -192,8 +192,8 @@ impl std::error::Error for Error { } } -impl From for Error { - fn from(_err: x509_cert::der::Error) -> Error { +impl From for Error { + fn from(_err: der::Error) -> Error { Error::ParseError } } diff --git a/src/lib.rs b/src/lib.rs index 2f4cff1d..3fc43b91 100644 --- 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 index 40c73ad2..0bb2da71 100644 --- 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 MgmKey { 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 MgmKey { 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 index b714173a..82dadbef 100644 --- 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(&self, new_key: &MgmKey, require_touch: bool) -> Result<()> { + pub fn set_mgm_key( + &self, + new_key: &MgmKey, + 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 index c3ed08bc..8956d426 100644 --- 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 index 9746f9e0..c545c4ec 100644 --- 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> = 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(yubikey: &mut YubiKey) { - assert!(yubikey.authenticate::(MgmKey::default::()).is_ok()); + assert!(yubikey.authenticate::(MgmKey::default()).is_ok()); assert!(MgmKey::::get_protected(yubikey).is_err()); // Set a protected management key. - assert!(MgmKey::generate::().set_protected(yubikey).is_ok()); - let protected = MgmKey::get_protected::(yubikey).unwrap(); - assert!(yubikey.authenticate::(MgmKey::default::()).is_err()); + assert!(MgmKey::::generate().set_protected(yubikey).is_ok()); + let protected = MgmKey::::get_protected(yubikey).unwrap(); + assert!(yubikey.authenticate::(MgmKey::default()).is_err()); assert!(yubikey.authenticate(protected.clone()).is_ok()); // Set a manual management key. let manual = MgmKey::::generate(); - assert!(manual.set_manual(&mut yubikey, false).is_ok()); - assert!(MgmKey::::get_protected(&mut yubikey).is_err()); + assert!(manual.set_manual(yubikey, false).is_ok()); + assert!(MgmKey::::get_protected(yubikey).is_err()); assert!(yubikey.authenticate(MgmKey::::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::::set_default(&mut yubikey).is_ok()); - assert!(MgmKey::::get_protected(&mut yubikey).is_err()); + assert!(MgmKey::::set_default(yubikey).is_ok()); + assert!(MgmKey::::get_protected(yubikey).is_err()); assert!(yubikey.authenticate(protected).is_err()); assert!(yubikey.authenticate(manual).is_err()); assert!(yubikey.authenticate(MgmKey::::default()).is_ok()); } - match get_mgm_admin(&mut yubikey).algorithm { + match get_mgm_key_meta(&mut yubikey).algorithm { ManagementAlgorithmId::ThreeDes => test_mgm::(&mut yubikey), ManagementAlgorithmId::Aes192 => test_mgm::(&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",