diff --git a/Cargo.lock b/Cargo.lock index 26a8391..84e4450 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1136,6 +1136,7 @@ name = "yubikey" version = "0.8.0" dependencies = [ "base16ct", + "cipher", "der", "des", "ecdsa", diff --git a/Cargo.toml b/Cargo.toml index bbbf832..5eef663 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,6 +24,7 @@ sha2 = "=0.11.0-pre.4" x509-cert = { version = "=0.3.0-pre.0", features = [ "builder", "hazmat" ] } [dependencies] +cipher = { version = "=0.5.0-pre.7", features = ["rand_core"] } der = "=0.8.0-rc.1" des = "=0.9.0-pre.2" elliptic-curve = "=0.14.0-rc.1" diff --git a/src/lib.rs b/src/lib.rs index 89e56da..ee859a8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -71,7 +71,7 @@ pub use crate::{ chuid::ChuId, config::Config, error::{Error, Result}, - mgm::{MgmAlgorithmId, MgmKey, MgmType}, + mgm::{MgmAlgorithmId, MgmKey, MgmKey3Des, MgmKeyAlgorithm, MgmKeyOps, MgmType}, piv::Key, policy::{PinPolicy, TouchPolicy}, reader::Context, diff --git a/src/mgm.rs b/src/mgm.rs index 960f49a..b352eeb 100644 --- a/src/mgm.rs +++ b/src/mgm.rs @@ -30,9 +30,13 @@ // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -use crate::{Error, Result}; +use crate::{ + yubikey::{Version, YubiKey}, + Error, Result, +}; +use cipher::{typenum::Unsigned, BlockCipherDecrypt, BlockCipherEncrypt, Key, KeyInit}; use log::error; -use rand_core::{OsRng, RngCore}; +use rand_core::CryptoRngCore; use zeroize::Zeroize; #[cfg(feature = "untested")] @@ -41,11 +45,6 @@ use crate::{ metadata::{AdminData, ProtectedData}, piv::{ManagementSlotId, SlotAlgorithmId}, transaction::Transaction, - yubikey::YubiKey, -}; -use des::{ - cipher::{BlockCipherDecrypt, BlockCipherEncrypt, KeyInit}, - TdesEde3, }; #[cfg(feature = "untested")] use {pbkdf2::pbkdf2_hmac, sha1::Sha1}; @@ -61,14 +60,19 @@ pub(crate) const APPLET_NAME: &str = "YubiKey MGMT"; pub(crate) const APPLET_ID: &[u8] = &[0xa0, 0x00, 0x00, 0x05, 0x27, 0x47, 0x11, 0x17]; mod tdes; -pub(crate) use tdes::DES_LEN_3DES; -use tdes::DES_LEN_DES; +pub use tdes::MgmKey3Des; pub(crate) const ADMIN_FLAGS_1_PROTECTED_MGM: u8 = 0x02; #[cfg(feature = "untested")] const CB_ADMIN_SALT: usize = 16; +/// The default MGM key loaded for both Triple-DES and AES keys +#[cfg(feature = "untested")] +const DEFAULT_MGM_KEY: [u8; 24] = [ + 1, 2, 3, 4, 5, 6, 7, 8, 1, 2, 3, 4, 5, 6, 7, 8, 1, 2, 3, 4, 5, 6, 7, 8, +]; + /// Number of PBKDF2 iterations to use when deriving from a password #[cfg(feature = "untested")] const ITER_MGM_PBKDF2: u32 = 10000; @@ -133,6 +137,21 @@ impl MgmAlgorithmId { } } +/// The algorithm used for the MGM key. +pub trait MgmKeyAlgorithm: + BlockCipherDecrypt + BlockCipherEncrypt + Clone + KeyInit + private::Seal +{ + /// The algorithm ID used in APDU packets + const ALGORITHM_ID: MgmAlgorithmId; + + /// Implemented by specializations to check if the key is weak. + /// + /// Returns an error if the key is weak. + fn check_weak_key(_key: &Key) -> Result<()> { + Ok(()) + } +} + /// Management Key (MGM). /// /// This key is used to authenticate to the management applet running on @@ -140,40 +159,159 @@ impl MgmAlgorithmId { /// /// The only supported algorithm for MGM keys is 3DES. #[derive(Clone)] -pub struct MgmKey([u8; DES_LEN_3DES]); +pub struct MgmKey(MgmKeyKind); + +#[derive(Clone)] +enum MgmKeyKind { + Tdes(MgmKey3Des), +} impl MgmKey { - /// Generate a random MGM key - pub fn generate() -> Self { - let mut key_bytes = [0u8; DES_LEN_3DES]; - OsRng.fill_bytes(&mut key_bytes); - Self(key_bytes) + /// Generates a random MGM key for the given algorithm. + pub fn generate(rng: &mut impl CryptoRngCore) -> Result { + match C::ALGORITHM_ID { + MgmAlgorithmId::ThreeDes => MgmKey3Des::generate(rng).map(MgmKeyKind::Tdes), + } + .map(Self) } - /// Create an MGM key from byte slice. + /// Generates a random MGM key using the preferred algorithm for the given Yubikey's + /// firmware version. + pub fn generate_for(yubikey: &YubiKey, rng: &mut impl CryptoRngCore) -> Result { + match yubikey.version() { + // Initial firmware versions default to 3DES. + Version { major: ..=4, .. } + | Version { + major: 5, + minor: ..=6, + .. + } => MgmKey3Des::generate(rng).map(MgmKeyKind::Tdes), + // Firmware 5.7.0 and above default to AES-192. + Version { + major: 5, + minor: 7.., + .. + } + | Version { major: 6.., .. } => Err(Error::NotSupported), + } + .map(Self) + } + + /// Parses an MGM key from the given byte slice. /// /// Returns an error if the slice is the wrong size or the key is weak. + /// + /// TODO: Can we distinguish DES from AES-192? Or do we take `C` as a parameter and + /// require the caller to know the type of the bytes they are parsing? pub fn from_bytes(bytes: impl AsRef<[u8]>) -> Result { - bytes.as_ref().try_into() + MgmKey3Des::from_bytes(bytes) + .map(MgmKeyKind::Tdes) + .map(Self) } - /// Create an MGM key from the given byte array. + /// Gets the default management key for the given Yubikey's firmware version. /// - /// Returns an error if the key is weak. - pub fn new(key_bytes: [u8; DES_LEN_3DES]) -> Result { - if tdes::is_weak_key(&key_bytes) { + /// Returns an error if the Yubikey's default algorithm is unsupported. + #[cfg(feature = "untested")] + pub fn get_default(yubikey: &YubiKey) -> Result { + match yubikey.version() { + // Initial firmware versions default to 3DES. + Version { major: ..=4, .. } + | Version { + major: 5, + minor: ..=6, + .. + } => Ok(Self(MgmKeyKind::Tdes( + MgmKey3Des::new(DEFAULT_MGM_KEY.into()).expect("valid"), + ))), + // Firmware 5.7.0 and above default to AES-192. + Version { + major: 5, + minor: 7.., + .. + } + | Version { major: 6.., .. } => Err(Error::NotSupported), + } + } + + /// Derives a management key (MGM) with the given algorithm from a stored salt. + /// + /// TODO: Is this supported for AES? Is the algorithm supposed to be dynamic? + #[cfg(feature = "untested")] + pub fn get_derived(yubikey: &mut YubiKey, pin: &[u8]) -> Result { + let txn = yubikey.begin_transaction()?; + + // Check the key algorithm. + let alg = MgmAlgorithmId::query(&txn)?; + if alg != MgmAlgorithmId::ThreeDes { + return Err(Error::NotSupported); + } + + // recover management key + let admin_data = AdminData::read(&txn)?; + let salt = admin_data.get_item(TAG_ADMIN_SALT)?; + + if salt.len() != CB_ADMIN_SALT { error!( - "blacklisting key '{:?}' since it's weak (with odd parity)", - &key_bytes + "derived MGM salt exists, but is incorrect size: {} (expected {})", + salt.len(), + CB_ADMIN_SALT ); - return Err(Error::KeyError); + return Err(Error::GenericError); } - Ok(Self(key_bytes)) + let mut mgm = Key::::default(); + pbkdf2_hmac::(pin, salt, ITER_MGM_PBKDF2, &mut mgm); + MgmKey3Des::new(mgm).map(MgmKeyKind::Tdes).map(Self) + } + + /// Resets the management key for the given YubiKey to the default value for that + /// Yubikey's firmware version. + /// + /// This will wipe any metadata related to derived and PIN-protected management keys. + #[cfg(feature = "untested")] + pub fn set_default(yubikey: &mut YubiKey) -> Result<()> { + Self::get_default(yubikey)?.set_manual(yubikey, false) + } +} + +/// Management Key (MGM). +/// +/// This key is used to authenticate to the management applet running on +/// a YubiKey in order to perform administrative functions. +#[derive(Clone)] +pub struct SpecificMgmKey(Key); + +impl SpecificMgmKey { + /// Generates a random MGM key for this algorithm. + pub fn generate(rng: &mut impl CryptoRngCore) -> Result { + let key = C::generate_key_with_rng(rng).map_err(|e| { + error!("RNG failure: {}", e); + Error::KeyError + })?; + Ok(Self(key)) + } + + /// Parses an MGM key from the given byte slice. + /// + /// Returns an error if the slice is the wrong size or the key is weak. + pub fn from_bytes(bytes: impl AsRef<[u8]>) -> Result { + let key = Key::::try_from(bytes.as_ref()).map_err(|_| Error::SizeError)?; + Self::new(key) + } + + /// Creates an MGM key from the given key. + /// + /// Returns an error if the key is weak. + pub fn new(key: Key) -> Result { + C::check_weak_key(&key)?; + Ok(Self(key)) } - /// Get derived management key (MGM) + /// Derives a management key (MGM) from a stored salt. + /// + /// TODO: Is this supported generically, or only for TDES? #[cfg(feature = "untested")] pub fn get_derived(yubikey: &mut YubiKey, pin: &[u8]) -> Result { let txn = yubikey.begin_transaction()?; @@ -198,14 +336,17 @@ impl MgmKey { return Err(Error::GenericError); } - let mut mgm = [0u8; DES_LEN_3DES]; + let mut mgm = Key::::default(); pbkdf2_hmac::(pin, salt, ITER_MGM_PBKDF2, &mut mgm); - MgmKey::from_bytes(mgm) + Self::new(mgm) } +} +/// The core operations available with a Management Key (MGM). +pub trait MgmKeyOps: AsRef<[u8]> + private::MgmKeyOpsInternal { /// Get protected management key (MGM) #[cfg(feature = "untested")] - pub fn get_protected(yubikey: &mut YubiKey) -> Result { + fn get_protected(yubikey: &mut YubiKey) -> Result { let txn = yubikey.begin_transaction()?; // Check the key algorithm. @@ -221,25 +362,18 @@ impl MgmKey { .get_item(TAG_PROTECTED_MGM) .inspect_err(|e| error!("could not read protected MGM from metadata (err: {:?})", e))?; - if item.len() != DES_LEN_3DES { - error!( - "protected data contains MGM, but is the wrong size: {} (expected {})", - item.len(), - DES_LEN_3DES - ); - - return Err(Error::AuthenticationError); - } + Self::parse_key(alg, item).map_err(|e| match e { + Error::SizeError => { + error!( + "protected data contains MGM, but is the wrong size: {} (expected {:?})", + item.len(), + alg, + ); - MgmKey::from_bytes(item) - } - - /// Resets the management key for the given YubiKey to the default value. - /// - /// This will wipe any metadata related to derived and PIN-protected management keys. - #[cfg(feature = "untested")] - pub fn set_default(yubikey: &mut YubiKey) -> Result<()> { - MgmKey::default().set_manual(yubikey, false) + Error::AuthenticationError + } + _ => e, + }) } /// Configures the given YubiKey to use this management key. @@ -249,7 +383,7 @@ impl MgmKey { /// /// This will wipe any metadata related to derived and PIN-protected management keys. #[cfg(feature = "untested")] - pub fn set_manual(&self, yubikey: &mut YubiKey, require_touch: bool) -> Result<()> { + fn set_manual(&self, yubikey: &mut YubiKey, require_touch: bool) -> Result<()> { let txn = yubikey.begin_transaction()?; txn.set_mgm_key(self, require_touch) @@ -306,7 +440,7 @@ impl MgmKey { /// /// This enables key management operations to be performed with access to the PIN. #[cfg(feature = "untested")] - pub fn set_protected(&self, yubikey: &mut YubiKey) -> Result<()> { + fn set_protected(&self, yubikey: &mut YubiKey) -> Result<()> { let txn = yubikey.begin_transaction()?; txn.set_mgm_key(self, false) @@ -369,38 +503,88 @@ impl MgmKey { Ok(()) } +} + +impl private::MgmKeyOpsInternal for SpecificMgmKey { + fn algorithm_id(&self) -> MgmAlgorithmId { + C::ALGORITHM_ID + } + + fn key_size(&self) -> u8 { + C::KeySize::U8 + } + + fn parse_key(alg: MgmAlgorithmId, bytes: impl AsRef<[u8]>) -> Result { + if alg == C::ALGORITHM_ID { + Self::from_bytes(bytes) + } else { + Err(Error::NotSupported) + } + } + + fn encrypt_block(&self, block: &mut [u8]) -> Result<()> { + C::new(&self.0).encrypt_block(block.try_into().map_err(|_| Error::SizeError)?); + Ok(()) + } + + fn decrypt_block(&self, block: &mut [u8]) -> Result<()> { + C::new(&self.0).decrypt_block(block.try_into().map_err(|_| Error::SizeError)?); + Ok(()) + } +} + +impl private::MgmKeyOpsInternal for MgmKey { + fn algorithm_id(&self) -> MgmAlgorithmId { + match &self.0 { + MgmKeyKind::Tdes(k) => k.algorithm_id(), + } + } + + fn key_size(&self) -> u8 { + match &self.0 { + MgmKeyKind::Tdes(k) => k.key_size(), + } + } - /// Encrypt with 3DES key - pub(crate) fn encrypt(&self, input: &[u8; DES_LEN_DES]) -> [u8; DES_LEN_DES] { - let mut output = input.to_owned(); - TdesEde3::new(&self.0.into()).encrypt_block((&mut output).into()); - output + fn parse_key(alg: MgmAlgorithmId, bytes: impl AsRef<[u8]>) -> Result { + match alg { + MgmAlgorithmId::ThreeDes => MgmKey3Des::from_bytes(bytes).map(MgmKeyKind::Tdes), + } + .map(Self) } - /// Decrypt with 3DES key - pub(crate) fn decrypt(&self, input: &[u8; DES_LEN_DES]) -> [u8; DES_LEN_DES] { - let mut output = input.to_owned(); - TdesEde3::new(&self.0.into()).decrypt_block((&mut output).into()); - output + fn encrypt_block(&self, block: &mut [u8]) -> Result<()> { + match &self.0 { + MgmKeyKind::Tdes(k) => k.encrypt_block(block), + } + } + + fn decrypt_block(&self, block: &mut [u8]) -> Result<()> { + match &self.0 { + MgmKeyKind::Tdes(k) => k.decrypt_block(block), + } } } -impl AsRef<[u8; DES_LEN_3DES]> for MgmKey { - fn as_ref(&self) -> &[u8; DES_LEN_3DES] { - &self.0 +impl MgmKeyOps for SpecificMgmKey {} + +impl MgmKeyOps for MgmKey {} + +impl AsRef<[u8]> for SpecificMgmKey { + fn as_ref(&self) -> &[u8] { + self.0.as_ref() } } -/// Default MGM key configured on all YubiKeys -impl Default for MgmKey { - fn default() -> Self { - MgmKey([ - 1, 2, 3, 4, 5, 6, 7, 8, 1, 2, 3, 4, 5, 6, 7, 8, 1, 2, 3, 4, 5, 6, 7, 8, - ]) +impl AsRef<[u8]> for MgmKey { + fn as_ref(&self) -> &[u8] { + match &self.0 { + MgmKeyKind::Tdes(k) => k.as_ref(), + } } } -impl Drop for MgmKey { +impl Drop for SpecificMgmKey { fn drop(&mut self) { self.0.zeroize(); } @@ -410,6 +594,60 @@ impl<'a> TryFrom<&'a [u8]> for MgmKey { type Error = Error; fn try_from(key_bytes: &'a [u8]) -> Result { - Self::new(key_bytes.try_into().map_err(|_| Error::SizeError)?) + Self::from_bytes(key_bytes) + } +} + +/// Seals the [`MgmKeyAlgorithm`] and [`MgmKeyOps`] traits, and add some internal helpers. +mod private { + use super::MgmAlgorithmId; + use crate::{Error, Result}; + + pub trait Seal {} + impl Seal for des::TdesEde3 {} + + pub trait MgmKeyOpsInternal: Sized { + /// Parses an MGM key from the given byte slice. + /// + /// Returns an error if the algorithm is unsupported, or the slice is the wrong size, + /// or the key is weak. + fn parse_key(alg: MgmAlgorithmId, bytes: impl AsRef<[u8]>) -> Result; + + /// Returns the ID used to identify the key algorithm with APDU packets. + fn algorithm_id(&self) -> MgmAlgorithmId; + + /// Returns the key size in bytes. + fn key_size(&self) -> u8; + + /// Encrypts a block with this key. + /// + /// Returns an error if the block is the wrong size. + fn encrypt_block(&self, block: &mut [u8]) -> Result<()>; + + /// Decrypts a block with this key. + /// + /// Returns an error if the block is the wrong size. + fn decrypt_block(&self, block: &mut [u8]) -> Result<()>; + + /// Given a challenge from a card, decrypts it and return the value + fn card_challenge(&self, challenge: &[u8]) -> Result> { + let mut output = challenge.to_owned(); + self.decrypt_block(output.as_mut_slice())?; + Ok(output) + } + + /// Checks the authentication matches the challenge and auth data + fn check_challenge(&self, challenge: &[u8], auth_data: &[u8]) -> Result<()> { + let mut response = challenge.to_owned(); + + self.encrypt_block(response.as_mut_slice())?; + + use subtle::ConstantTimeEq; + if response.ct_eq(auth_data).unwrap_u8() != 1 { + return Err(Error::AuthenticationError); + } + + Ok(()) + } } } diff --git a/src/mgm/tdes.rs b/src/mgm/tdes.rs index ba1105b..502c747 100644 --- a/src/mgm/tdes.rs +++ b/src/mgm/tdes.rs @@ -1,10 +1,30 @@ +use cipher::Key; use zeroize::Zeroizing; +use crate::{Error, Result}; + +use super::{MgmAlgorithmId, MgmKeyAlgorithm, SpecificMgmKey}; + /// Size of a DES key -pub(super) const DES_LEN_DES: usize = 8; +const DES_LEN_DES: usize = 8; /// Size of a 3DES key -pub(crate) const DES_LEN_3DES: usize = DES_LEN_DES * 3; +const DES_LEN_3DES: usize = DES_LEN_DES * 3; + +impl MgmKeyAlgorithm for des::TdesEde3 { + const ALGORITHM_ID: MgmAlgorithmId = MgmAlgorithmId::ThreeDes; + + fn check_weak_key(key: &Key) -> Result<()> { + if is_weak_key(key.into()) { + Err(Error::KeyError) + } else { + Ok(()) + } + } +} + +/// A Management Key (MGM) using Triple-DES +pub type MgmKey3Des = SpecificMgmKey; /// Weak and semi weak DES keys as taken from: /// %A D.W. Davies @@ -37,7 +57,7 @@ const WEAK_DES_KEYS: &[[u8; DES_LEN_DES]] = &[ /// /// This check is performed automatically when the key is instantiated to /// ensure no such keys are used. -pub(super) fn is_weak_key(key: &[u8; DES_LEN_3DES]) -> bool { +fn is_weak_key(key: &[u8; DES_LEN_3DES]) -> bool { // set odd parity of key let mut tmp = Zeroizing::new([0u8; DES_LEN_3DES]); diff --git a/src/transaction.rs b/src/transaction.rs index 727f23d..9575f28 100644 --- a/src/transaction.rs +++ b/src/transaction.rs @@ -15,7 +15,7 @@ use log::{error, trace}; use zeroize::Zeroizing; #[cfg(feature = "untested")] -use crate::mgm::{MgmKey, DES_LEN_3DES}; +use crate::mgm::MgmKeyOps; const CB_PIN_MAX: usize = 8; @@ -260,14 +260,14 @@ 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: &K, require_touch: bool) -> Result<()> { let p2 = if require_touch { 0xfe } else { 0xff }; - let mut data = [0u8; DES_LEN_3DES + 3]; - data[0] = ALGO_3DES; - data[1] = KEY_CARDMGM; - data[2] = DES_LEN_3DES as u8; - data[3..3 + DES_LEN_3DES].copy_from_slice(new_key.as_ref()); + let mut data = Vec::with_capacity(usize::from(new_key.key_size()) + 3); + data.push(new_key.algorithm_id().into()); + data.push(KEY_CARDMGM); + data.push(new_key.key_size()); + data.extend_from_slice(new_key.as_ref()); let status_words = Apdu::new(Ins::SetMgmKey) .params(0xff, p2) diff --git a/src/yubikey.rs b/src/yubikey.rs index 6c1c298..a43a1a6 100644 --- a/src/yubikey.rs +++ b/src/yubikey.rs @@ -36,7 +36,7 @@ use crate::{ chuid::ChuId, config::Config, error::{Error, Result}, - mgm::MgmKey, + mgm::MgmKeyOps, piv, reader::{Context, Reader}, transaction::Transaction, @@ -67,6 +67,7 @@ use { pub(crate) const ADMIN_FLAGS_1_PUK_BLOCKED: u8 = 0x01; /// 3DES authentication +#[cfg(feature = "untested")] pub(crate) const ALGO_3DES: u8 = 0x03; /// Card management key @@ -360,37 +361,44 @@ impl YubiKey { } /// Authenticate to the card using the provided management key (MGM). - pub fn authenticate(&mut self, mgm_key: MgmKey) -> Result<()> { + pub fn authenticate(&mut self, mgm_key: &K) -> Result<()> { let txn = self.begin_transaction()?; // get a challenge from the card - let challenge = Apdu::new(Ins::Authenticate) - .params(ALGO_3DES, KEY_CARDMGM) + let card_response = Apdu::new(Ins::Authenticate) + .params(mgm_key.algorithm_id().into(), KEY_CARDMGM) .data([TAG_DYN_AUTH, 0x02, 0x80, 0x00]) .transmit(&txn, 261)?; - if !challenge.is_success() || challenge.data().len() < 12 { + if !card_response.is_success() || card_response.data().len() < 5 { return Err(Error::AuthenticationError); } // send a response to the cards challenge and a challenge of our own. - let response = mgm_key.decrypt(challenge.data()[4..12].try_into()?); + let card_challenge = mgm_key.card_challenge(&card_response.data()[4..])?; + let challenge_len = card_challenge.len(); - let mut data = [0u8; 22]; - data[0] = TAG_DYN_AUTH; - data[1] = 20; // 2 + 8 + 2 +8 - data[2] = 0x80; - data[3] = 8; - data[4..12].copy_from_slice(&response); - data[12] = 0x81; - data[13] = 8; - OsRng.fill_bytes(&mut data[14..22]); + // If this exceeds a `u8` then the card is giving us unexpected data. + let auth_len = (2 + challenge_len + 2 + challenge_len) + .try_into() + .map_err(|_| Error::AuthenticationError)?; - let mut challenge = [0u8; 8]; - challenge.copy_from_slice(&data[14..22]); + let mut data = Vec::with_capacity(4 + challenge_len + 2 + challenge_len); + data.push(TAG_DYN_AUTH); + data.push(auth_len); + data.push(0x80); + data.push(challenge_len as u8); + data.extend_from_slice(&card_challenge); + data.push(0x81); + data.push(challenge_len as u8); + + let mut host_challenge = vec![0u8; challenge_len]; + OsRng.fill_bytes(&mut host_challenge); + + data.extend_from_slice(&host_challenge); let authentication = Apdu::new(Ins::Authenticate) - .params(ALGO_3DES, KEY_CARDMGM) + .params(mgm_key.algorithm_id().into(), KEY_CARDMGM) .data(data) .transmit(&txn, 261)?; @@ -399,14 +407,7 @@ impl YubiKey { } // compare the response from the card with our challenge - let response = mgm_key.encrypt(&challenge); - - use subtle::ConstantTimeEq; - if response.ct_eq(&authentication.data()[4..12]).unwrap_u8() != 1 { - return Err(Error::AuthenticationError); - } - - Ok(()) + mgm_key.check_challenge(&host_challenge, &authentication.data()[4..]) } /// Get the PIV keys contained in this YubiKey. diff --git a/tests/integration.rs b/tests/integration.rs index 615426c..d2940b8 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -114,32 +114,39 @@ fn test_verify_pin() { #[test] #[ignore] fn test_set_mgmkey() { + use yubikey::MgmKeyOps; + + let mut rng = OsRng; let mut yubikey = YUBIKEY.lock().unwrap(); + let default_key = MgmKey::get_default(&yubikey).unwrap(); assert!(yubikey.verify_pin(b"123456").is_ok()); assert!(MgmKey::get_protected(&mut yubikey).is_err()); - assert!(yubikey.authenticate(MgmKey::default()).is_ok()); + assert!(yubikey.authenticate(&default_key).is_ok()); // Set a protected management key. - assert!(MgmKey::generate().set_protected(&mut yubikey).is_ok()); + assert!(MgmKey::generate_for(&yubikey, &mut rng) + .unwrap() + .set_protected(&mut yubikey) + .is_ok()); let protected = MgmKey::get_protected(&mut yubikey).unwrap(); - assert!(yubikey.authenticate(MgmKey::default()).is_err()); - assert!(yubikey.authenticate(protected.clone()).is_ok()); + assert!(yubikey.authenticate(&default_key).is_err()); + assert!(yubikey.authenticate(&protected).is_ok()); // Set a manual management key. - let manual = MgmKey::generate(); + let manual = MgmKey::generate_for(&yubikey, &mut rng).unwrap(); assert!(manual.set_manual(&mut yubikey, false).is_ok()); assert!(MgmKey::get_protected(&mut yubikey).is_err()); - assert!(yubikey.authenticate(MgmKey::default()).is_err()); - assert!(yubikey.authenticate(protected.clone()).is_err()); - assert!(yubikey.authenticate(manual.clone()).is_ok()); + assert!(yubikey.authenticate(&default_key).is_err()); + assert!(yubikey.authenticate(&protected).is_err()); + assert!(yubikey.authenticate(&manual).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!(yubikey.authenticate(protected).is_err()); - assert!(yubikey.authenticate(manual).is_err()); - assert!(yubikey.authenticate(MgmKey::default()).is_ok()); + assert!(yubikey.authenticate(&protected).is_err()); + assert!(yubikey.authenticate(&manual).is_err()); + assert!(yubikey.authenticate(&default_key).is_ok()); } // @@ -148,9 +155,10 @@ fn test_set_mgmkey() { fn generate_self_signed_cert() -> Certificate { let mut yubikey = YUBIKEY.lock().unwrap(); + let default_key = MgmKey::get_default(&yubikey).unwrap(); assert!(yubikey.verify_pin(b"123456").is_ok()); - assert!(yubikey.authenticate(MgmKey::default()).is_ok()); + assert!(yubikey.authenticate(&default_key).is_ok()); let slot = SlotId::Retired(RetiredSlotId::R1); @@ -285,9 +293,10 @@ fn test_slot_id_display() { #[ignore] fn test_read_metadata() { let mut yubikey = YUBIKEY.lock().unwrap(); + let default_key = MgmKey::get_default(&yubikey).unwrap(); assert!(yubikey.verify_pin(b"123456").is_ok()); - assert!(yubikey.authenticate(MgmKey::default()).is_ok()); + assert!(yubikey.authenticate(&default_key).is_ok()); let slot = SlotId::Retired(RetiredSlotId::R1);