From 9fe363661e4cb8d7341ca078ac6ffb4198858aee Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Sat, 30 Nov 2019 14:28:02 +0000 Subject: [PATCH 1/4] verify_pin: Don't set APDU data for empty PIN --- src/transaction.rs | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/transaction.rs b/src/transaction.rs index ebe5a01..df37b4f 100644 --- a/src/transaction.rs +++ b/src/transaction.rs @@ -163,15 +163,23 @@ impl<'tx> Transaction<'tx> { /// Verify device PIN. #[cfg(feature = "untested")] pub fn verify_pin(&self, pin: &[u8]) -> Result<(), Error> { - // TODO(tarcieri): allow unpadded (with `0xFF`) PIN shorter than CB_PIN_MAX? - if pin.len() != CB_PIN_MAX { + if pin.len() > CB_PIN_MAX { return Err(Error::SizeError); } - let response = APDU::new(Ins::Verify) - .params(0x00, 0x80) - .data(pin) - .transmit(self, 261)?; + let mut query = APDU::new(Ins::Verify); + query.params(0x00, 0x80); + + // Empty pin means we are querying the number of retries. We set no data in this + // case; if we instead sent [0xff; CB_PIN_MAX] it would count as an attempt and + // decrease the retry counter. + if !pin.is_empty() { + let mut data = Zeroizing::new([0xff; CB_PIN_MAX]); + data[0..pin.len()].copy_from_slice(pin); + query.data(data.as_ref()); + } + + let response = query.transmit(self, 261)?; match response.status_words() { StatusWords::Success => Ok(()), From 4b5cd8dd455d04f5d20994fe2923bccfae084b64 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Sat, 30 Nov 2019 15:22:14 +0000 Subject: [PATCH 2/4] Make PIN verification failure a StatusWord case Retry count is now u8, as it cannot exceed 16 (being returned in the lower half of SW2). --- src/apdu.rs | 10 ++++++++++ src/error.rs | 2 +- src/transaction.rs | 4 ++-- src/yubikey.rs | 15 +++------------ 4 files changed, 16 insertions(+), 15 deletions(-) diff --git a/src/apdu.rs b/src/apdu.rs index b3e8e13..e407749 100644 --- a/src/apdu.rs +++ b/src/apdu.rs @@ -352,6 +352,12 @@ pub(crate) enum StatusWords { /// Successful execution Success, + /// PIN verification failure + VerifyFailError { + /// Remaining verification attempts + tries: u8, + }, + /// Security status not satisfied SecurityStatusError, @@ -379,6 +385,7 @@ impl StatusWords { pub fn code(self) -> u32 { match self { StatusWords::None => 0, + StatusWords::VerifyFailError { tries } => 0x63c0 & tries as u32, StatusWords::SecurityStatusError => 0x6982, StatusWords::AuthBlockedError => 0x6983, StatusWords::IncorrectParamError => 0x6a80, @@ -399,6 +406,9 @@ impl From for StatusWords { fn from(sw: u32) -> Self { match sw { 0x0000 => StatusWords::None, + sw if sw & 0xfff0 == 0x63c0 => StatusWords::VerifyFailError { + tries: (sw & 0x000f) as u8, + }, 0x6982 => StatusWords::SecurityStatusError, 0x6983 => StatusWords::AuthBlockedError, 0x6a80 => StatusWords::IncorrectParamError, diff --git a/src/error.rs b/src/error.rs index c8da1a9..79b7a78 100644 --- a/src/error.rs +++ b/src/error.rs @@ -68,7 +68,7 @@ pub enum Error { /// Wrong PIN WrongPin { /// Number of tries remaining - tries: u32, + tries: u8, }, /// Invalid object diff --git a/src/transaction.rs b/src/transaction.rs index df37b4f..68fa89f 100644 --- a/src/transaction.rs +++ b/src/transaction.rs @@ -184,7 +184,7 @@ impl<'tx> Transaction<'tx> { match response.status_words() { StatusWords::Success => Ok(()), StatusWords::AuthBlockedError => Err(Error::WrongPin { tries: 0 }), - StatusWords::Other(sw) if sw >> 8 == 0x63 => Err(Error::WrongPin { tries: sw & 0xf }), + StatusWords::VerifyFailError { tries } => Err(Error::WrongPin { tries }), _ => Err(Error::GenericError), } } @@ -215,7 +215,7 @@ impl<'tx> Transaction<'tx> { match status_words { StatusWords::Success => Ok(()), StatusWords::AuthBlockedError => Err(Error::PinLocked), - StatusWords::Other(sw) if sw >> 8 == 0x63 => Err(Error::WrongPin { tries: sw & 0xf }), + StatusWords::VerifyFailError { tries } => Err(Error::WrongPin { tries }), _ => { error!( "failed changing pin, token response code: {:x}.", diff --git a/src/yubikey.rs b/src/yubikey.rs index f1f5c0f..cedfc80 100644 --- a/src/yubikey.rs +++ b/src/yubikey.rs @@ -396,7 +396,7 @@ impl YubiKey { /// Get the number of PIN retries #[cfg(feature = "untested")] - pub fn get_pin_retries(&mut self) -> Result { + pub fn get_pin_retries(&mut self) -> Result { let txn = self.begin_transaction()?; // Force a re-select to unverify, because once verified the spec dictates that @@ -414,24 +414,15 @@ impl YubiKey { /// Set the number of PIN retries #[cfg(feature = "untested")] - pub fn set_pin_retries(&mut self, pin_tries: usize, puk_tries: usize) -> Result<(), Error> { + pub fn set_pin_retries(&mut self, pin_tries: u8, puk_tries: u8) -> Result<(), Error> { // Special case: if either retry count is 0, it's a successful no-op if pin_tries == 0 || puk_tries == 0 { return Ok(()); } - if pin_tries > 0xff || puk_tries > 0xff || pin_tries < 1 || puk_tries < 1 { - return Err(Error::RangeError); - } - let txn = self.begin_transaction()?; - let templ = [ - 0, - Ins::SetPinRetries.code(), - pin_tries as u8, - puk_tries as u8, - ]; + let templ = [0, Ins::SetPinRetries.code(), pin_tries, puk_tries]; let status_words = txn.transfer_data(&templ, &[], 255)?.status_words(); From cfef291ad9ed0937a3bda1ad267730d3fe6dd6e4 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Sat, 30 Nov 2019 15:24:10 +0000 Subject: [PATCH 3/4] Use u16 for raw StatusWords --- src/apdu.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/apdu.rs b/src/apdu.rs index e407749..1c6e30b 100644 --- a/src/apdu.rs +++ b/src/apdu.rs @@ -280,7 +280,7 @@ impl Response { /// Get the raw [`StatusWords`] code for this response. #[cfg(feature = "untested")] - pub fn code(&self) -> u32 { + pub fn code(&self) -> u16 { self.status_words.code() } @@ -317,7 +317,7 @@ impl From> for Response { } let sw = StatusWords::from( - (bytes[bytes.len() - 2] as u32) << 8 | (bytes[bytes.len() - 1] as u32), + (bytes[bytes.len() - 2] as u16) << 8 | (bytes[bytes.len() - 1] as u16), ); let len = bytes.len() - 2; @@ -377,15 +377,15 @@ pub(crate) enum StatusWords { NotSupportedError, /// Other/unrecognized status words - Other(u32), + Other(u16), } impl StatusWords { /// Get the numerical response code for these status words - pub fn code(self) -> u32 { + pub fn code(self) -> u16 { match self { StatusWords::None => 0, - StatusWords::VerifyFailError { tries } => 0x63c0 & tries as u32, + StatusWords::VerifyFailError { tries } => 0x63c0 & tries as u16, StatusWords::SecurityStatusError => 0x6982, StatusWords::AuthBlockedError => 0x6983, StatusWords::IncorrectParamError => 0x6a80, @@ -402,8 +402,8 @@ impl StatusWords { } } -impl From for StatusWords { - fn from(sw: u32) -> Self { +impl From for StatusWords { + fn from(sw: u16) -> Self { match sw { 0x0000 => StatusWords::None, sw if sw & 0xfff0 == 0x63c0 => StatusWords::VerifyFailError { @@ -420,8 +420,8 @@ impl From for StatusWords { } } -impl From for u32 { - fn from(sw: StatusWords) -> u32 { +impl From for u16 { + fn from(sw: StatusWords) -> u16 { sw.code() } } From a61a6fd94b8404ae88d2a08ba5d1b7147c6a3052 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Sat, 30 Nov 2019 15:34:14 +0000 Subject: [PATCH 4/4] Define more YubiKey-recognized status words Recognized values sourced from https://github.com/Yubico/yubikey-manager NotFoundError and NoSpaceError are specified in SP 800-73-4 Table 6. --- src/apdu.rs | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/src/apdu.rs b/src/apdu.rs index 1c6e30b..60de357 100644 --- a/src/apdu.rs +++ b/src/apdu.rs @@ -352,21 +352,42 @@ pub(crate) enum StatusWords { /// Successful execution Success, + /// https://github.com/Yubico/yubikey-manager/blob/1f22620b623c6b345dd9f9193ec765a542dddc80/ykman/driver_ccid.py#L53 + NoInputDataError, + /// PIN verification failure VerifyFailError { /// Remaining verification attempts tries: u8, }, + /// https://github.com/Yubico/yubikey-manager/blob/1f22620b623c6b345dd9f9193ec765a542dddc80/ykman/driver_ccid.py#L55 + WrongLengthError, + /// Security status not satisfied SecurityStatusError, /// Authentication method blocked AuthBlockedError, + /// https://github.com/Yubico/yubikey-manager/blob/1f22620b623c6b345dd9f9193ec765a542dddc80/ykman/driver_ccid.py#L58 + DataInvalidError, + + /// https://github.com/Yubico/yubikey-manager/blob/1f22620b623c6b345dd9f9193ec765a542dddc80/ykman/driver_ccid.py#L59 + ConditionsNotSatisfiedError, + + /// https://github.com/Yubico/yubikey-manager/blob/1f22620b623c6b345dd9f9193ec765a542dddc80/ykman/driver_ccid.py#L60 + CommandNotAllowedError, + /// Incorrect parameter in command data field IncorrectParamError, + /// Data object or application not found + NotFoundError, + + /// Not enough memory + NoSpaceError, + // // Custom Yubico Status Word extensions // @@ -376,6 +397,9 @@ pub(crate) enum StatusWords { /// Not supported error NotSupportedError, + /// https://github.com/Yubico/yubikey-manager/blob/1f22620b623c6b345dd9f9193ec765a542dddc80/ykman/driver_ccid.py#L65 + CommandAbortedError, + /// Other/unrecognized status words Other(u16), } @@ -385,12 +409,20 @@ impl StatusWords { pub fn code(self) -> u16 { match self { StatusWords::None => 0, + StatusWords::NoInputDataError => 0x6285, StatusWords::VerifyFailError { tries } => 0x63c0 & tries as u16, + StatusWords::WrongLengthError => 0x6700, StatusWords::SecurityStatusError => 0x6982, StatusWords::AuthBlockedError => 0x6983, + StatusWords::DataInvalidError => 0x6984, + StatusWords::ConditionsNotSatisfiedError => 0x6985, + StatusWords::CommandNotAllowedError => 0x6986, StatusWords::IncorrectParamError => 0x6a80, + StatusWords::NotFoundError => 0x6a82, + StatusWords::NoSpaceError => 0x6a84, StatusWords::IncorrectSlotError => 0x6b00, StatusWords::NotSupportedError => 0x6d00, + StatusWords::CommandAbortedError => 0x6f00, StatusWords::Success => 0x9000, StatusWords::Other(n) => n, } @@ -406,14 +438,22 @@ impl From for StatusWords { fn from(sw: u16) -> Self { match sw { 0x0000 => StatusWords::None, + 0x6285 => StatusWords::NoInputDataError, sw if sw & 0xfff0 == 0x63c0 => StatusWords::VerifyFailError { tries: (sw & 0x000f) as u8, }, + 0x6700 => StatusWords::WrongLengthError, 0x6982 => StatusWords::SecurityStatusError, 0x6983 => StatusWords::AuthBlockedError, + 0x6984 => StatusWords::DataInvalidError, + 0x6985 => StatusWords::ConditionsNotSatisfiedError, + 0x6986 => StatusWords::CommandNotAllowedError, 0x6a80 => StatusWords::IncorrectParamError, + 0x6a82 => StatusWords::NotFoundError, + 0x6a84 => StatusWords::NoSpaceError, 0x6b00 => StatusWords::IncorrectSlotError, 0x6d00 => StatusWords::NotSupportedError, + 0x6f00 => StatusWords::CommandAbortedError, 0x9000 => StatusWords::Success, _ => StatusWords::Other(sw), }