From 2e7ee82a58a442938109290ae770b9a58e7ff03d Mon Sep 17 00:00:00 2001 From: Heiko Schaefer Date: Tue, 7 Sep 2021 13:28:01 +0200 Subject: [PATCH] Use StatusBytes in RawResponse (instead of a pair of u8). Replace status bytes constants in the code with StatusBytes enum variants. --- openpgp-card/src/apdu.rs | 40 +++++++++++++++++-------------- openpgp-card/src/apdu/response.rs | 22 ++++++++--------- openpgp-card/src/card_app.rs | 10 ++++---- openpgp-card/src/errors.rs | 17 +++++++++---- 4 files changed, 51 insertions(+), 38 deletions(-) diff --git a/openpgp-card/src/apdu.rs b/openpgp-card/src/apdu.rs index 38b56c5..648fb74 100644 --- a/openpgp-card/src/apdu.rs +++ b/openpgp-card/src/apdu.rs @@ -39,28 +39,30 @@ pub(crate) fn send_command( expect_reply, )?)?; - while resp.status().0 == 0x61 { + while let StatusBytes::OkBytesAvailable(_) = resp.status() { // More data is available for this command from the card + log::debug!(" chained response, getting more data"); - log::debug!(" response was truncated, getting more data"); - - // Get additional data + // Get next chunk of data let next = RawResponse::try_from(send_command_low_level( card_client, commands::get_response(), expect_reply, )?)?; - // Only continue if status is 0x61xx or 0x9000. - if next.status().0 != 0x61 && next.status() != (0x90, 0x0) { - return Err(Error::CardStatus(StatusBytes::from(next.status()))); + match next.status() { + StatusBytes::OkBytesAvailable(_) | StatusBytes::Ok => { + log::debug!( + " appending {} bytes to response", + next.raw_data().len() + ); + + // Append new data to resp.data and overwrite status. + resp.raw_mut_data().extend_from_slice(next.raw_data()); + resp.set_status(next.status()); + } + error => return Err(error.into()), } - - log::debug!(" appending {} bytes to response", next.raw_data().len()); - - // Append new data to resp.data and overwrite status. - resp.raw_mut_data().extend_from_slice(next.raw_data()); - resp.set_status(next.status()); } log::debug!(" final response len: {}", resp.raw_data().len()); @@ -156,11 +158,11 @@ fn send_command_low_level( let serialized = partial.serialize(ext).map_err(Error::InternalError)?; - log::debug!(" -> chunked APDU command: {:x?}", &serialized); + log::debug!(" -> chained APDU command: {:x?}", &serialized); let resp = card_client.transmit(&serialized, buf_size)?; - log::debug!(" <- APDU chunk response: {:x?}", &resp); + log::debug!(" <- APDU response: {:x?}", &resp); if resp.len() < 2 { return Err(Error::ResponseLength(resp.len())); @@ -171,13 +173,15 @@ fn send_command_low_level( let sw1 = resp[resp.len() - 2]; let sw2 = resp[resp.len() - 1]; + let status = StatusBytes::from((sw1, sw2)); + // ISO: "If SW1-SW2 is set to '6883', then the last // command of the chain is expected." - if !((sw1 == 0x90 && sw2 == 0x00) - || (sw1 == 0x68 && sw2 == 0x83)) + if !(status == StatusBytes::Ok + || status == StatusBytes::LastCommandOfChainExpected) { // Unexpected status for a non-final chunked response - return Err(StatusBytes::from((sw1, sw2)).into()); + return Err(status.into()); } // ISO: "If SW1-SW2 is set to '6884', then command diff --git a/openpgp-card/src/apdu/response.rs b/openpgp-card/src/apdu/response.rs index 782b645..c8cd772 100644 --- a/openpgp-card/src/apdu/response.rs +++ b/openpgp-card/src/apdu/response.rs @@ -30,8 +30,7 @@ impl Response { #[derive(Clone, Debug)] pub(crate) struct RawResponse { data: Vec, - sw1: u8, - sw2: u8, + status: StatusBytes, } impl TryFrom for Response { @@ -41,7 +40,7 @@ impl TryFrom for Response { if value.is_ok() { Ok(Response { data: value.data }) } else { - Err(Error::CardStatus(StatusBytes::from(value.status()))) + Err(value.status().into()) } } } @@ -49,7 +48,7 @@ impl TryFrom for Response { impl RawResponse { pub fn check_ok(&self) -> Result<(), StatusBytes> { if !self.is_ok() { - Err(StatusBytes::from((self.sw1, self.sw2))) + Err(self.status()) } else { Ok(()) } @@ -69,18 +68,17 @@ impl RawResponse { &mut self.data } - pub(crate) fn set_status(&mut self, new_status: (u8, u8)) { - self.sw1 = new_status.0; - self.sw2 = new_status.1; + pub(crate) fn set_status(&mut self, new_status: StatusBytes) { + self.status = new_status; } - pub fn status(&self) -> (u8, u8) { - (self.sw1, self.sw2) + pub fn status(&self) -> StatusBytes { + self.status } /// Is the response status "ok"? (0x90, 0x00) pub fn is_ok(&self) -> bool { - self.status() == (0x90, 0x00) + self.status() == StatusBytes::Ok } } @@ -95,7 +93,9 @@ impl TryFrom> for RawResponse { .pop() .ok_or_else(|| Error::ResponseLength(data.len()))?; - Ok(RawResponse { data, sw1, sw2 }) + let status = (sw1, sw2).into(); + + Ok(RawResponse { data, status }) } } diff --git a/openpgp-card/src/card_app.rs b/openpgp-card/src/card_app.rs index 23edb19..6553adc 100644 --- a/openpgp-card/src/card_app.rs +++ b/openpgp-card/src/card_app.rs @@ -19,8 +19,8 @@ use crate::crypto_data::{ CardUploadableKey, Cryptogram, Hash, PublicKeyMaterial, }; use crate::tlv::{tag::Tag, value::Value, Tlv}; -use crate::Error; use crate::{apdu, keys, CardCaps, CardClientBox, KeyType}; +use crate::{Error, StatusBytes}; /// Low-level access to OpenPGP card functionality. /// @@ -264,8 +264,8 @@ impl CardApp { let verify = commands::verify_pw1_81([0x40; 8].to_vec()); let resp = apdu::send_command(&mut self.card_client, verify, false)?; - if !(resp.status() == (0x69, 0x82) - || resp.status() == (0x69, 0x83)) + if !(resp.status() == StatusBytes::SecurityStatusNotSatisfied + || resp.status() == StatusBytes::AuthenticationMethodBlocked) { return Err(anyhow!("Unexpected status for reset, at pw1.")); } @@ -278,8 +278,8 @@ impl CardApp { let resp = apdu::send_command(&mut self.card_client, verify, false)?; - if !(resp.status() == (0x69, 0x82) - || resp.status() == (0x69, 0x83)) + if !(resp.status() == StatusBytes::SecurityStatusNotSatisfied + || resp.status() == StatusBytes::AuthenticationMethodBlocked) { return Err(anyhow!("Unexpected status for reset, at pw3.")); } diff --git a/openpgp-card/src/errors.rs b/openpgp-card/src/errors.rs index 2690f88..fe04744 100644 --- a/openpgp-card/src/errors.rs +++ b/openpgp-card/src/errors.rs @@ -42,10 +42,16 @@ impl From for Error { } } -/// OpenPGP card "Status Bytes" errors -#[derive(thiserror::Error, Debug, PartialEq)] +/// OpenPGP card "Status Bytes" (ok statuses and errors) +#[derive(thiserror::Error, Debug, PartialEq, Copy, Clone)] #[non_exhaustive] pub enum StatusBytes { + #[error("Command correct")] + Ok, + + #[error("Command correct, [{0}] bytes available in response")] + OkBytesAvailable(u8), + #[error("Selected file or DO in termination state")] TerminationState, @@ -74,7 +80,7 @@ pub enum StatusBytes { LastCommandOfChainExpected, #[error("Command chaining not supported")] - CommandChainingUnsupported, + CommandChainingNotSupported, #[error("Security status not satisfied")] SecurityStatusNotSatisfied, @@ -119,6 +125,9 @@ pub enum StatusBytes { impl From<(u8, u8)> for StatusBytes { fn from(status: (u8, u8)) -> Self { match (status.0, status.1) { + (0x90, 0x00) => StatusBytes::Ok, + (0x61, bytes) => StatusBytes::OkBytesAvailable(bytes), + (0x62, 0x85) => StatusBytes::TerminationState, (0x63, 0xC0..=0xCF) => { StatusBytes::PasswordNotChecked(status.1 & 0xf) @@ -130,7 +139,7 @@ impl From<(u8, u8)> for StatusBytes { (0x68, 0x81) => StatusBytes::LogicalChannelNotSupported, (0x68, 0x82) => StatusBytes::SecureMessagingNotSupported, (0x68, 0x83) => StatusBytes::LastCommandOfChainExpected, - (0x68, 0x84) => StatusBytes::CommandChainingUnsupported, + (0x68, 0x84) => StatusBytes::CommandChainingNotSupported, (0x69, 0x82) => StatusBytes::SecurityStatusNotSatisfied, (0x69, 0x83) => StatusBytes::AuthenticationMethodBlocked, (0x69, 0x85) => StatusBytes::ConditionOfUseNotSatisfied,