diff --git a/CHANGELOG.md b/CHANGELOG.md index e88bbf5..8d497f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 calling `YubiKey::disconnect(pcsc::Disposition::LeaveCard)` if `Reader::open` succeeds). +### Fixed +- `StatusWords::code` now returns the correct code (including embedded `tries` + count) for `StatusWords::VerifyFailError`. Previously the returned code lost + information and was not round-trip compatible with `StatusWords::from(u16)`. + ## 0.7.0 (2022-11-14) ### Added - Display inner PC/SC errors ([#420]) diff --git a/src/apdu.rs b/src/apdu.rs index 1960e6f..d8942a3 100644 --- a/src/apdu.rs +++ b/src/apdu.rs @@ -353,6 +353,12 @@ pub(crate) enum StatusWords { /// Successful execution Success, + /// The requested data was too large for the response, and there is data remaining. + BytesRemaining { + /// The number of bytes remaining, as indicated in the response. + len: u8, + }, + /// https://github.com/Yubico/yubikey-manager/blob/1f22620b623c6b345dd9f9193ec765a542dddc80/ykman/driver_ccid.py#L53 NoInputDataError, @@ -410,8 +416,9 @@ impl StatusWords { pub fn code(self) -> u16 { match self { StatusWords::None => 0, + StatusWords::BytesRemaining { len } => 0x6100 | len as u16, StatusWords::NoInputDataError => 0x6285, - StatusWords::VerifyFailError { tries } => 0x63c0 & tries as u16, + StatusWords::VerifyFailError { tries } => 0x63c0 | tries as u16, StatusWords::WrongLengthError => 0x6700, StatusWords::SecurityStatusError => 0x6982, StatusWords::AuthBlockedError => 0x6983, @@ -439,6 +446,9 @@ impl From for StatusWords { fn from(sw: u16) -> Self { match sw { 0x0000 => StatusWords::None, + sw if sw & 0xff00 == 0x6100 => Self::BytesRemaining { + len: (sw & 0x00ff) as u8, + }, 0x6285 => StatusWords::NoInputDataError, sw if sw & 0xfff0 == 0x63c0 => StatusWords::VerifyFailError { tries: (sw & 0x000f) as u8, @@ -466,3 +476,40 @@ impl From for u16 { sw.code() } } + +#[cfg(test)] +mod tests { + use super::StatusWords; + + #[test] + fn status_words_round_trip() { + let round_trip = |sw: StatusWords| { + assert_eq!(StatusWords::from(sw.code()), sw); + }; + + round_trip(StatusWords::None); + round_trip(StatusWords::BytesRemaining { len: 1 }); + round_trip(StatusWords::BytesRemaining { len: 10 }); + round_trip(StatusWords::BytesRemaining { len: 0xFF }); + round_trip(StatusWords::Success); + round_trip(StatusWords::NoInputDataError); + round_trip(StatusWords::VerifyFailError { tries: 0x0F }); + round_trip(StatusWords::VerifyFailError { tries: 3 }); + round_trip(StatusWords::VerifyFailError { tries: 2 }); + round_trip(StatusWords::VerifyFailError { tries: 1 }); + round_trip(StatusWords::VerifyFailError { tries: 0 }); + round_trip(StatusWords::WrongLengthError); + round_trip(StatusWords::SecurityStatusError); + round_trip(StatusWords::AuthBlockedError); + round_trip(StatusWords::DataInvalidError); + round_trip(StatusWords::ConditionsNotSatisfiedError); + round_trip(StatusWords::CommandNotAllowedError); + round_trip(StatusWords::IncorrectParamError); + round_trip(StatusWords::NotFoundError); + round_trip(StatusWords::NoSpaceError); + round_trip(StatusWords::IncorrectSlotError); + round_trip(StatusWords::NotSupportedError); + round_trip(StatusWords::CommandAbortedError); + round_trip(StatusWords::Other(0x1337)); + } +} diff --git a/src/transaction.rs b/src/transaction.rs index a66c797..1398741 100644 --- a/src/transaction.rs +++ b/src/transaction.rs @@ -400,11 +400,12 @@ impl<'tx> Transaction<'tx> { .data(&in_data[in_offset..(in_offset + this_size)]) .transmit(self, 261)?; - sw = response.status_words().code(); + sw = response.status_words(); - if !response.is_success() && (sw >> 8 != 0x61) { + match sw { + StatusWords::Success | StatusWords::BytesRemaining { .. } => (), // TODO(tarcieri): is this really OK? - return Ok(Response::new(sw.into(), out_data)); + _ => return Ok(Response::new(sw, out_data)), } if !out_data.is_empty() && (out_data.len() - response.data().len() > max_out) { @@ -425,17 +426,15 @@ impl<'tx> Transaction<'tx> { } } - while sw >> 8 == 0x61 { - trace!( - "The card indicates there is {} bytes more data for us", - sw & 0xff - ); + while let StatusWords::BytesRemaining { len } = sw { + trace!("The card indicates there is {} bytes more data for us", len); let response = Apdu::new(Ins::GetResponseApdu).transmit(self, 261)?; - sw = response.status_words().code(); + sw = response.status_words(); - if sw != StatusWords::Success.code() && (sw >> 8 != 0x61) { - return Ok(Response::new(sw.into(), vec![])); + match sw { + StatusWords::Success | StatusWords::BytesRemaining { .. } => (), + _ => return Ok(Response::new(sw, vec![])), } if out_data.len() + response.data().len() > max_out { @@ -451,7 +450,7 @@ impl<'tx> Transaction<'tx> { out_data.extend_from_slice(&response.data()[..response.data().len()]); } - Ok(Response::new(sw.into(), out_data)) + Ok(Response::new(sw, out_data)) } /// Fetch an object.