Merge pull request #32 from tarcieri/factor-responses-into-apdu-module

Factor `Response` into `apdu` module; improved debugging

.github/workflows/rust.yml

@@ -149,7 +149,7 @@ jobs:
         uses: actions-rs/cargo@v1
         with:
           command: clippy
-          args: -- -D warnings
+          args: --all-features -- -D warnings
 
   # TODO: use actions-rs/audit-check
   security_audit:

README.md

@@ -112,15 +112,17 @@ To trace every message sent to/from the card i.e. the raw
 Application Protocol Data Unit (APDU) messages, use the `trace` log level:
 
 ```
-running 1 test
 [INFO  yubikey_piv::yubikey] trying to connect to reader 'Yubico YubiKey OTP+FIDO+CCID'
 [INFO  yubikey_piv::yubikey] connected to 'Yubico YubiKey OTP+FIDO+CCID' successfully
+[TRACE yubikey_piv::apdu] >>> APDU { cla: 0, ins: 164, p1: 4, p2: 0, lc: 5, data: [160, 0, 0, 3, 8] }
 [TRACE yubikey_piv::transaction] >>> [0, 164, 4, 0, 5, 160, 0, 0, 3, 8]
-[TRACE yubikey_piv::transaction] <<< [97, 17, 79, 6, 0, 0, 16, 0, 1, 0, 121, 7, 79, 5, 160, 0, 0, 3, 8, 144, 0]
+[TRACE yubikey_piv::apdu] <<< Response { status_words: Success, data: [97, 17, 79, 6, 0, 0, 16, 0, 1, 0, 121, 7, 79, 5, 160, 0, 0, 3, 8] }
+[TRACE yubikey_piv::apdu] >>> APDU { cla: 0, ins: 253, p1: 0, p2: 0, lc: 0, data: [] }
 [TRACE yubikey_piv::transaction] >>> [0, 253, 0, 0, 0]
-[TRACE yubikey_piv::transaction] <<< [5, 1, 2, 144, 0]
+[TRACE yubikey_piv::apdu] <<< Response { status_words: Success, data: [5, 1, 2] }
+[TRACE yubikey_piv::apdu] >>> APDU { cla: 0, ins: 248, p1: 0, p2: 0, lc: 0, data: [] }
 [TRACE yubikey_piv::transaction] >>> [0, 248, 0, 0, 0]
-[TRACE yubikey_piv::transaction] <<< [0, 115, 0, 178, 144, 0]
+[TRACE yubikey_piv::apdu] <<< Response { status_words: Success, data: [0, 115, 0, 178] }
 test connect ... ok
 ```
 

src/apdu.rs

@@ -30,7 +30,8 @@
 // (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::Error, response::Response, transaction::Transaction, Buffer};
+use crate::{error::Error, transaction::Transaction, Buffer};
+use log::trace;
 use std::fmt::{self, Debug};
 use zeroize::{Zeroize, Zeroizing};
 
@@ -112,11 +113,13 @@ impl APDU {
 
     /// Transmit this APDU using the given card transaction
     pub fn transmit(&self, txn: &Transaction<'_>, recv_len: usize) -> Result<Response, Error> {
-        let response_bytes = txn.transmit(&self.to_bytes(), recv_len)?;
+        trace!(">>> {:?}", self);
-        Ok(Response::from_bytes(response_bytes))
+        let response = Response::from(txn.transmit(&self.to_bytes(), recv_len)?);
+        trace!("<<< {:?}", &response);
+        Ok(response)
     }
 
-    /// Consume this APDU and return a self-zeroizing buffer
+    /// Serialize this APDU as a self-zeroizing byte buffer
     pub fn to_bytes(&self) -> Buffer {
         let mut bytes = Vec::with_capacity(5 + self.data.len());
         bytes.push(self.cla);
@@ -159,3 +162,162 @@ impl Zeroize for APDU {
         self.data.zeroize();
     }
 }
+
+/// APDU responses
+#[derive(Debug)]
+pub(crate) struct Response {
+    /// Status words
+    status_words: StatusWords,
+
+    /// Buffer
+    data: Vec<u8>,
+}
+
+impl Response {
+    /// Create a new response from the given status words and buffer
+    #[cfg(feature = "untested")]
+    pub fn new(status_words: StatusWords, data: Vec<u8>) -> Response {
+        Response { status_words, data }
+    }
+
+    /// Get the [`StatusWords`] for this response.
+    pub fn status_words(&self) -> StatusWords {
+        self.status_words
+    }
+
+    /// Get the raw [`StatusWords`] code for this response.
+    #[cfg(feature = "untested")]
+    pub fn code(&self) -> u32 {
+        self.status_words.code()
+    }
+
+    /// Do the status words for this response indicate success?
+    pub fn is_success(&self) -> bool {
+        self.status_words.is_success()
+    }
+
+    /// Borrow the response data
+    pub fn data(&self) -> &[u8] {
+        self.data.as_ref()
+    }
+}
+
+impl AsRef<[u8]> for Response {
+    fn as_ref(&self) -> &[u8] {
+        self.data()
+    }
+}
+
+impl Drop for Response {
+    fn drop(&mut self) {
+        self.zeroize();
+    }
+}
+
+impl From<Vec<u8>> for Response {
+    fn from(mut bytes: Vec<u8>) -> Self {
+        if bytes.len() < 2 {
+            return Response {
+                status_words: StatusWords::None,
+                data: bytes,
+            };
+        }
+
+        let sw = StatusWords::from(
+            (bytes[bytes.len() - 2] as u32) << 8 | (bytes[bytes.len() - 1] as u32),
+        );
+
+        let len = bytes.len() - 2;
+        bytes.truncate(len);
+
+        Response {
+            status_words: sw,
+            data: bytes,
+        }
+    }
+}
+
+impl Zeroize for Response {
+    fn zeroize(&mut self) {
+        self.data.zeroize();
+    }
+}
+
+/// Status Words (SW) are 2-byte values returned by a card command.
+///
+/// The first byte of a status word is referred to as SW1 and the second byte
+/// of a status word is referred to as SW2.
+///
+/// See NIST special publication 800-73-4, section 5.6:
+/// <https://csrc.nist.gov/publications/detail/sp/800-73/4/final>
+#[derive(Copy, Clone, Debug, Eq, PartialEq)]
+pub(crate) enum StatusWords {
+    /// No status words present in response
+    None,
+
+    /// Successful execution
+    Success,
+
+    /// Security status not satisfied
+    SecurityStatusError,
+
+    /// Authentication method blocked
+    AuthBlockedError,
+
+    /// Incorrect parameter in command data field
+    IncorrectParamError,
+
+    //
+    // Custom Yubico Status Word extensions
+    //
+    /// Incorrect card slot error
+    IncorrectSlotError,
+
+    /// Not supported error
+    NotSupportedError,
+
+    /// Other/unrecognized status words
+    Other(u32),
+}
+
+impl StatusWords {
+    /// Get the numerical response code for these status words
+    pub fn code(self) -> u32 {
+        match self {
+            StatusWords::None => 0,
+            StatusWords::SecurityStatusError => 0x6982,
+            StatusWords::AuthBlockedError => 0x6983,
+            StatusWords::IncorrectParamError => 0x6a80,
+            StatusWords::IncorrectSlotError => 0x6b00,
+            StatusWords::NotSupportedError => 0x6d00,
+            StatusWords::Success => 0x9000,
+            StatusWords::Other(n) => n,
+        }
+    }
+
+    /// Do these status words indicate success?
+    pub fn is_success(self) -> bool {
+        self == StatusWords::Success
+    }
+}
+
+impl From<u32> for StatusWords {
+    fn from(sw: u32) -> Self {
+        match sw {
+            0x0000 => StatusWords::None,
+            0x6982 => StatusWords::SecurityStatusError,
+            0x6983 => StatusWords::AuthBlockedError,
+            0x6a80 => StatusWords::IncorrectParamError,
+            0x6b00 => StatusWords::IncorrectSlotError,
+            0x6d00 => StatusWords::NotSupportedError,
+            0x9000 => StatusWords::Success,
+            _ => StatusWords::Other(sw),
+        }
+    }
+}
+
+impl From<StatusWords> for u32 {
+    fn from(sw: StatusWords) -> u32 {
+        sw.code()
+    }
+}

src/key.rs

@@ -38,14 +38,14 @@
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 use crate::{
+    apdu::StatusWords,
     certificate::{self, Certificate},
     consts::*,
     error::Error,
-    response::StatusWords,
     serialization::*,
     settings,
     yubikey::YubiKey,
-    AlgorithmId, ObjectId,
+    AlgorithmId, Buffer, ObjectId,
 };
 use log::{debug, error, warn};
 
@@ -309,7 +309,7 @@ pub fn generate(
         }
     }
 
-    let data = response.into_buffer();
+    let data = Buffer::new(response.data().into());
 
     match algorithm {
         YKPIV_ALGO_RSA1024 | YKPIV_ALGO_RSA2048 => {

src/lib.rs

@@ -155,7 +155,6 @@ mod metadata;
 pub mod mgm;
 #[cfg(feature = "untested")]
 pub mod msroots;
-mod response;
 #[cfg(feature = "untested")]
 mod serialization;
 #[cfg(feature = "untested")]

src/response.rs

@@ -1,186 +0,0 @@
-//! Responses to issued commands
-
-// Adapted from yubico-piv-tool:
-// <https://github.com/Yubico/yubico-piv-tool/>
-//
-// Copyright (c) 2014-2016 Yubico AB
-// All rights reserved.
-//
-// Redistribution and use in source and binary forms, with or without
-// modification, are permitted provided that the following conditions are
-// met:
-//
-//   * Redistributions of source code must retain the above copyright
-//     notice, this list of conditions and the following disclaimer.
-//
-//   * Redistributions in binary form must reproduce the above
-//     copyright notice, this list of conditions and the following
-//     disclaimer in the documentation and/or other materials provided
-//     with the distribution.
-//
-// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
-// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
-// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
-// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
-// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
-// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
-// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
-// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
-// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
-// (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::Buffer;
-
-/// Parsed response to a command
-pub(crate) struct Response {
-    /// Status words
-    status_words: StatusWords,
-
-    /// Buffer
-    buffer: Buffer,
-}
-
-impl Response {
-    /// Parse a response from the given buffer
-    pub fn from_bytes(mut buffer: Buffer) -> Self {
-        if buffer.len() >= 2 {
-            let sw = StatusWords::from(
-                (buffer[buffer.len() - 2] as u32) << 8 | (buffer[buffer.len() - 1] as u32),
-            );
-
-            let len = buffer.len() - 2;
-            buffer.truncate(len);
-            Response {
-                status_words: sw,
-                buffer,
-            }
-        } else {
-            Response {
-                status_words: StatusWords::None,
-                buffer,
-            }
-        }
-    }
-
-    /// Create a new response from the given status words and buffer
-    #[cfg(feature = "untested")]
-    pub fn new(status_words: StatusWords, buffer: Buffer) -> Response {
-        Response {
-            status_words,
-            buffer,
-        }
-    }
-
-    /// Get the [`StatusWords`] for this response.
-    pub fn status_words(&self) -> StatusWords {
-        self.status_words
-    }
-
-    /// Get the raw [`StatusWords`] code for this response.
-    #[cfg(feature = "untested")]
-    pub fn code(&self) -> u32 {
-        self.status_words.code()
-    }
-
-    /// Do the status words for this response indicate success?
-    pub fn is_success(&self) -> bool {
-        self.status_words.is_success()
-    }
-
-    /// Borrow the response buffer
-    pub fn buffer(&self) -> &[u8] {
-        self.buffer.as_ref()
-    }
-
-    /// Consume this response, returning its buffer
-    #[cfg(feature = "untested")]
-    pub fn into_buffer(self) -> Buffer {
-        self.buffer
-    }
-}
-
-impl AsRef<[u8]> for Response {
-    fn as_ref(&self) -> &[u8] {
-        self.buffer()
-    }
-}
-
-/// Status Words (SW) are 2-byte values returned by a card command.
-///
-/// The first byte of a status word is referred to as SW1 and the second byte
-/// of a status word is referred to as SW2.
-///
-/// See NIST special publication 800-73-4, section 5.6:
-/// <https://csrc.nist.gov/publications/detail/sp/800-73/4/final>
-#[derive(Copy, Clone, Debug, Eq, PartialEq)]
-pub(crate) enum StatusWords {
-    /// No status words present in response
-    None,
-
-    /// Successful execution
-    Success,
-
-    /// Security status not satisfied
-    SecurityStatusError,
-
-    /// Authentication method blocked
-    AuthBlockedError,
-
-    /// Incorrect parameter in command data field
-    IncorrectParamError,
-
-    //
-    // Custom Yubico Status Word extensions
-    //
-    /// Incorrect card slot error
-    IncorrectSlotError,
-
-    /// Not supported error
-    NotSupportedError,
-
-    /// Other/unrecognized status words
-    Other(u32),
-}
-
-impl StatusWords {
-    /// Get the numerical response code for these status words
-    pub fn code(self) -> u32 {
-        match self {
-            StatusWords::None => 0,
-            StatusWords::SecurityStatusError => 0x6982,
-            StatusWords::AuthBlockedError => 0x6983,
-            StatusWords::IncorrectParamError => 0x6a80,
-            StatusWords::IncorrectSlotError => 0x6b00,
-            StatusWords::NotSupportedError => 0x6d00,
-            StatusWords::Success => 0x9000,
-            StatusWords::Other(n) => n,
-        }
-    }
-
-    /// Do these status words indicate success?
-    pub fn is_success(self) -> bool {
-        self == StatusWords::Success
-    }
-}
-
-impl From<u32> for StatusWords {
-    fn from(sw: u32) -> Self {
-        match sw {
-            0x0000 => StatusWords::None,
-            0x6982 => StatusWords::SecurityStatusError,
-            0x6983 => StatusWords::AuthBlockedError,
-            0x6a80 => StatusWords::IncorrectParamError,
-            0x6b00 => StatusWords::IncorrectSlotError,
-            0x6d00 => StatusWords::NotSupportedError,
-            0x9000 => StatusWords::Success,
-            _ => StatusWords::Other(sw),
-        }
-    }
-}
-
-impl From<StatusWords> for u32 {
-    fn from(sw: StatusWords) -> u32 {
-        sw.code()
-    }
-}

src/transaction.rs

@@ -1,17 +1,18 @@
 //! YubiKey PC/SC transactions
 
-use crate::{apdu::APDU, consts::*, error::Error, yubikey::*, Buffer};
+use crate::{apdu::APDU, consts::*, error::Error, yubikey::*};
 #[cfg(feature = "untested")]
 use crate::{
+    apdu::{Response, StatusWords},
     mgm::MgmKey,
-    response::{Response, StatusWords},
     serialization::*,
-    ObjectId,
+    Buffer, ObjectId,
 };
 use log::{error, trace};
 use std::convert::TryInto;
 #[cfg(feature = "untested")]
 use std::ptr;
+#[cfg(feature = "untested")]
 use zeroize::Zeroizing;
 
 /// Exclusive transaction with the YubiKey's PC/SC card.
@@ -43,10 +44,10 @@ impl<'tx> Transaction<'tx> {
     /// single APDU messages at a time. For larger messages that need to be
     /// split into multiple APDUs, use the [`Transaction::transfer_data`]
     /// method instead.
-    pub fn transmit(&self, send_buffer: &[u8], recv_len: usize) -> Result<Buffer, Error> {
+    pub fn transmit(&self, send_buffer: &[u8], recv_len: usize) -> Result<Vec<u8>, Error> {
         trace!(">>> {:?}", send_buffer);
 
-        let mut recv_buffer = Zeroizing::new(vec![0u8; recv_len]);
+        let mut recv_buffer = vec![0u8; recv_len];
 
         let len = self
             .inner
@@ -54,9 +55,6 @@ impl<'tx> Transaction<'tx> {
             .len();
 
         recv_buffer.truncate(len);
-
-        trace!("<<< {:?}", recv_buffer.as_slice());
-
         Ok(recv_buffer)
     }
 
@@ -91,14 +89,14 @@ impl<'tx> Transaction<'tx> {
             return Err(Error::GenericError);
         }
 
-        if response.buffer().len() < 3 {
+        if response.data().len() < 3 {
             return Err(Error::SizeError);
         }
 
         Ok(Version {
-            major: response.buffer()[0],
+            major: response.data()[0],
-            minor: response.buffer()[1],
+            minor: response.data()[1],
-            patch: response.buffer()[2],
+            patch: response.data()[2],
         })
     }
 
@@ -157,7 +155,7 @@ impl<'tx> Transaction<'tx> {
             resp
         };
 
-        response.buffer()[..4]
+        response.data()[..4]
             .try_into()
             .map(|serial| Serial::from(u32::from_be_bytes(serial)))
             .map_err(|_| Error::SizeError)
@@ -363,7 +361,7 @@ impl<'tx> Transaction<'tx> {
             }
         }
 
-        let data = response.buffer();
+        let data = response.data();
 
         // skip the first 7c tag
         if data[0] != 0x7c {
@@ -404,7 +402,7 @@ impl<'tx> Transaction<'tx> {
         max_out: usize,
     ) -> Result<Response, Error> {
         let mut in_offset = 0;
-        let mut out_data = Zeroizing::new(vec![]);
+        let mut out_data = vec![];
         let mut sw = 0;
 
         loop {
@@ -432,17 +430,17 @@ impl<'tx> Transaction<'tx> {
 
             sw = response.status_words().code();
 
-            if out_data.len() - response.buffer().len() - 2 > max_out {
+            if out_data.len() - response.data().len() - 2 > max_out {
                 error!(
                     "output buffer too small: wanted to write {}, max was {}",
-                    out_data.len() - response.buffer().len() - 2,
+                    out_data.len() - response.data().len() - 2,
                     max_out
                 );
 
                 return Err(Error::SizeError);
             }
 
-            out_data.extend_from_slice(&response.buffer()[..response.buffer().len() - 2]);
+            out_data.extend_from_slice(&response.data()[..response.data().len() - 2]);
 
             in_offset += this_size;
             if in_offset >= in_data.len() {
@@ -460,20 +458,20 @@ impl<'tx> Transaction<'tx> {
             sw = response.status_words().code();
 
             if sw != StatusWords::Success.code() && (sw >> 8 != 0x61) {
-                return Ok(Response::new(sw.into(), Zeroizing::new(vec![])));
+                return Ok(Response::new(sw.into(), vec![]));
             }
 
-            if out_data.len() + response.buffer().len() - 2 > max_out {
+            if out_data.len() + response.data().len() - 2 > max_out {
                 error!(
                     "output buffer too small: wanted to write {}, max was {}",
-                    out_data.len() + response.buffer().len() - 2,
+                    out_data.len() + response.data().len() - 2,
                     max_out
                 );
 
                 return Err(Error::SizeError);
             }
 
-            out_data.extend_from_slice(&response.buffer()[..response.buffer().len() - 2]);
+            out_data.extend_from_slice(&response.data()[..response.data().len() - 2]);
         }
 
         Ok(Response::new(sw.into(), out_data))
@@ -495,7 +493,7 @@ impl<'tx> Transaction<'tx> {
             return Err(Error::GenericError);
         }
 
-        let data = response.into_buffer();
+        let data = Buffer::new(response.data().into());
         let mut outlen = 0;
 
         if data.len() < 2 || !has_valid_length(&data[1..], data.len() - 1) {

src/yubikey.rs

@@ -35,7 +35,11 @@
 
 #[cfg(feature = "untested")]
 use crate::{
-    apdu::APDU, key::SlotId, metadata, mgm::MgmKey, response::StatusWords, serialization::*,
+    apdu::{StatusWords, APDU},
+    key::SlotId,
+    metadata,
+    mgm::MgmKey,
+    serialization::*,
     ObjectId,
 };
 use crate::{consts::*, error::Error, transaction::Transaction, Buffer};
@@ -271,12 +275,12 @@ impl YubiKey {
             .data(&[0x7c, 0x02, 0x80, 0x00])
             .transmit(&txn, 261)?;
 
-        if !challenge.is_success() || challenge.buffer().len() < 12 {
+        if !challenge.is_success() || challenge.data().len() < 12 {
             return Err(Error::AuthenticationError);
         }
 
         // send a response to the cards challenge and a challenge of our own.
-        let response = mgm_key.decrypt(challenge.buffer()[4..12].try_into().unwrap());
+        let response = mgm_key.decrypt(challenge.data()[4..12].try_into().unwrap());
 
         let mut data = [0u8; 22];
         data[0] = 0x7c;
@@ -308,7 +312,7 @@ impl YubiKey {
         let response = mgm_key.encrypt(&challenge);
 
         use subtle::ConstantTimeEq;
-        if response.ct_eq(&authentication.buffer()[4..12]).unwrap_u8() != 1 {
+        if response.ct_eq(&authentication.data()[4..12]).unwrap_u8() != 1 {
             return Err(Error::AuthenticationError);
         }
 
@@ -803,11 +807,11 @@ impl YubiKey {
             }
         }
 
-        if response.buffer()[0] != 0x30 {
+        if response.data()[0] != 0x30 {
             return Err(Error::GenericError);
         }
 
-        Ok(response.into_buffer())
+        Ok(Buffer::new(response.data().into()))
     }
 
     /// Get an auth challenge
@@ -824,7 +828,7 @@ impl YubiKey {
             return Err(Error::AuthenticationError);
         }
 
-        Ok(response.buffer()[4..12].try_into().unwrap())
+        Ok(response.data()[4..12].try_into().unwrap())
     }
 
     /// Verify an auth response