From 62b7b35ab06f917b64844b8d21416391a70a79c4 Mon Sep 17 00:00:00 2001 From: Heiko Schaefer Date: Wed, 8 Sep 2021 23:56:18 +0200 Subject: [PATCH] Clean up Command, simplify serialization. --- openpgp-card/src/apdu.rs | 50 ++++++-------- openpgp-card/src/apdu/command.rs | 108 ++++++++++++++++++------------- 2 files changed, 81 insertions(+), 77 deletions(-) diff --git a/openpgp-card/src/apdu.rs b/openpgp-card/src/apdu.rs index 648fb74..12d4f82 100644 --- a/openpgp-card/src/apdu.rs +++ b/openpgp-card/src/apdu.rs @@ -14,16 +14,9 @@ use std::convert::TryFrom; use crate::apdu::{command::Command, response::RawResponse}; use crate::{CardClientBox, Error, StatusBytes}; -// "Maximum amount of bytes in a short APDU command or response" (from pcsc) +/// "Maximum amount of bytes in a short APDU command or response" (from pcsc) const MAX_BUFFER_SIZE: usize = 264; -#[derive(Clone, Copy, PartialEq, Debug)] -pub(crate) enum Le { - None, - Short, - Long, -} - /// Send a Command and return the result as a Response. /// /// If the reply is truncated, this fn assembles all the parts and returns @@ -78,7 +71,7 @@ pub(crate) fn send_command( fn send_command_low_level( card_client: &mut CardClientBox, cmd: Command, - expect_reply: bool, + expect_response: bool, ) -> Result, Error> { let (ext_support, chaining_support, mut max_cmd_bytes, max_rsp_bytes) = if let Some(caps) = card_client.get_caps() { @@ -114,20 +107,19 @@ fn send_command_low_level( max_rsp_bytes ); - // Set Le to 'long', if we're using an extended chunk size. + // Decide if we want to use "extended length fields". // - // According to the Card spec 3.4.1, pg 47, - // 255 is the maximum value for 'Lc' in short mode: - // "A short Lc field consists of one byte not set to '00' (1 to 255 dec.)" - let ext = match (expect_reply, ext_support && max_cmd_bytes > 0xFF) { - (false, _) => Le::None, - (_, true) => Le::Long, - _ => Le::Short, - }; + // Current approach: we only use extended length if the card supports it, + // and only if the current command has more than 255 bytes of data. + // + // (This could be a problem with cards that don't support chained + // responses, when a response if >255 bytes long - e.g. getting public + // key data from cards?) + let ext_len = ext_support && (max_cmd_bytes > 0xFF); log::debug!(" -> full APDU command: {:x?}", cmd); - let buf_size = if !ext_support || ext == Le::Short { + let buf_size = if !ext_len { MAX_BUFFER_SIZE } else { max_rsp_bytes @@ -135,28 +127,24 @@ fn send_command_low_level( log::trace!("buf_size {}", buf_size); - if chaining_support && !cmd.get_data().is_empty() { + if chaining_support && !cmd.data().is_empty() { // Send command in chained mode log::debug!("chained command mode"); // Break up payload into chunks that fit into one command, each - let chunks: Vec<_> = cmd.get_data().chunks(max_cmd_bytes).collect(); + let chunks: Vec<_> = cmd.data().chunks(max_cmd_bytes).collect(); for (i, d) in chunks.iter().enumerate() { let last = i == chunks.len() - 1; let cla = if last { 0x00 } else { 0x10 }; - let partial = Command::new( - cla, - cmd.get_ins(), - cmd.get_p1(), - cmd.get_p2(), - d.to_vec(), - ); + let partial = + Command::new(cla, cmd.ins(), cmd.p1(), cmd.p2(), d.to_vec()); - let serialized = - partial.serialize(ext).map_err(Error::InternalError)?; + let serialized = partial + .serialize(ext_len, expect_response) + .map_err(Error::InternalError)?; log::debug!(" -> chained APDU command: {:x?}", &serialized); @@ -193,7 +181,7 @@ fn send_command_low_level( } unreachable!("This state should be unreachable"); } else { - let serialized = cmd.serialize(ext)?; + let serialized = cmd.serialize(ext_len, expect_response)?; // Can't send this command to the card, because it is too long and // the card doesn't support command chaining. diff --git a/openpgp-card/src/apdu/command.rs b/openpgp-card/src/apdu/command.rs index 334467d..83f7c23 100644 --- a/openpgp-card/src/apdu/command.rs +++ b/openpgp-card/src/apdu/command.rs @@ -4,10 +4,8 @@ //! Data structure for APDU Commands //! (Commands get sent to the card, which will usually send back a `Response`) -use crate::apdu::Le; use anyhow::Result; -#[allow(clippy::upper_case_acronyms)] #[derive(Clone, Debug)] pub(crate) struct Command { // Class byte (CLA) @@ -43,66 +41,84 @@ impl Command { } } - pub(crate) fn get_ins(&self) -> u8 { + pub(crate) fn ins(&self) -> u8 { self.ins } - pub(crate) fn get_p1(&self) -> u8 { + pub(crate) fn p1(&self) -> u8 { self.p1 } - pub(crate) fn get_p2(&self) -> u8 { + pub(crate) fn p2(&self) -> u8 { self.p2 } - pub(crate) fn get_data(&self) -> &[u8] { + pub(crate) fn data(&self) -> &[u8] { &self.data } - fn encode_len(len: u16, ext: Le) -> Vec { - if len > 0xff || ext == Le::Long { - vec![0, (len as u16 >> 8) as u8, (len as u16 & 255) as u8] - } else { - vec![len as u8] - } - } + /// Serialize a Command (for sending to a card). + /// + /// See OpenPGP card spec, chapter 7 (pg 47) + pub(crate) fn serialize( + &self, + ext_len: bool, + expect_response: bool, + ) -> Result> { + // FIXME? (from scd/apdu.c): + // T=0 does not allow the use of Lc together with Le; + // thus disable Le in this case. - pub(crate) fn serialize(&self, ext: Le) -> Result> { - // See OpenPGP card spec, chapter 7 (pg 47) - - // FIXME: 1) get "ext" information (how long can commands and - // responses be), - // FIXME: 2) decide on long vs. short encoding for both Lc and Le - // (must be the same) - - let data_len = Self::encode_len(self.data.len() as u16, ext); + // "number of bytes in the command data field" + let nc = self.data.len() as u16; let mut buf = vec![self.cla, self.ins, self.p1, self.p2]; - - if !self.data.is_empty() { - buf.extend_from_slice(&data_len); - buf.extend_from_slice(&self.data[..]); - } - - // Le - match ext { - // FIXME? (from scd/apdu.c): - // /* T=0 does not allow the use of Lc together with Le; - // thus disable Le in this case. */ - // if (reader_table[slot].is_t0) - // le = -1; - Le::None => (), - - Le::Short => buf.push(0), - Le::Long => { - buf.push(0); - buf.push(0); - if self.data.is_empty() { - buf.push(0) - } - } - } + buf.extend(Self::make_lc(nc, ext_len)); + buf.extend(&self.data); + buf.extend(Self::make_le(nc, ext_len, expect_response)); Ok(buf) } + + /// Encode len for Lc field + fn make_lc(len: u16, ext_len: bool) -> Vec { + if !ext_len { + assert!(len <= 0xff, "unexpected: len > 0xff, but ext says Short"); + } + + if len == 0 { + vec![] + } else if !ext_len { + vec![len as u8] + } else { + vec![0, (len as u16 >> 8) as u8, (len as u16 & 255) as u8] + } + } + + /// Encode value for Le field + /// ("maximum number of bytes expected in the response data field"). + fn make_le(nc: u16, ext_len: bool, expect_response: bool) -> Vec { + match (ext_len, expect_response) { + (_, false) => { + // No response data expected. + // "If the Le field is absent, then Ne is zero" + vec![] + } + (false, true) => { + // A short Le field consists of one byte with any value + vec![0] + } + (true, true) => { + if nc == 0 { + // "three bytes (one byte set to '00' followed by two + // bytes with any value) if the Lc field is absent" + vec![0, 0, 0] + } else { + // "two bytes (with any value) if an extended Lc field + // is present" + vec![0, 0] + } + } + } + } }