Merge pull request #63 from iqlusioninc/drop-neo-support

Drop YubiKey NEO support (closes #18)

README.md

@@ -40,11 +40,11 @@ endorsed by Yubico.
 
 ## Supported YubiKeys
 
-- [YubiKey NEO] series (may be dropped in the future, see [#18])
 - [YubiKey 4] series
 - [YubiKey 5] series
 
-NOTE: Nano and USB-C variants of the above are also supported
+NOTE: Nano and USB-C variants of the above are also supported.
+      Pre-YK4 [YubiKey NEO] series is **NOT** supported (see [#18]).
 
 ## Security Warning
 

cli/README.md

@@ -22,11 +22,11 @@ utility with general-purpose public-key encryption and signing support.
 
 ## Supported YubiKeys
 
-- [YubiKey NEO] series (may be dropped in the future, see [#18])
 - [YubiKey 4] series
 - [YubiKey 5] series
 
-NOTE: Nano and USB-C variants of the above are also supported
+NOTE: Nano and USB-C variants of the above are also supported.
+      Pre-YK4 [YubiKey NEO] series is **NOT** supported (see [#18]).
 
 ## Security Warning
 

src/certificate.rs

@@ -145,17 +145,15 @@ impl Certificate {
     /// Write this certificate into the YubiKey in the given slot
     #[cfg(feature = "untested")]
     pub fn write(&self, yubikey: &mut YubiKey, slot: SlotId, certinfo: u8) -> Result<(), Error> {
-        let max_size = yubikey.obj_size_max();
         let txn = yubikey.begin_transaction()?;
-        write_certificate(&txn, slot, Some(&self.data), certinfo, max_size)
+        write_certificate(&txn, slot, Some(&self.data), certinfo)
     }
 
     /// Delete a certificate located at the given slot of the given YubiKey
     #[cfg(feature = "untested")]
     pub fn delete(yubikey: &mut YubiKey, slot: SlotId) -> Result<(), Error> {
-        let max_size = yubikey.obj_size_max();
         let txn = yubikey.begin_transaction()?;
-        write_certificate(&txn, slot, None, 0, max_size)
+        write_certificate(&txn, slot, None, 0)
     }
 
     /// Initialize a local certificate struct from the given bytebuffer
@@ -244,7 +242,6 @@ pub(crate) fn write_certificate(
     slot: SlotId,
     data: Option<&[u8]>,
     certinfo: u8,
-    max_size: usize,
 ) -> Result<(), Error> {
     let mut buf = [0u8; CB_OBJ_MAX];
     let mut offset = 0;
@@ -261,7 +258,7 @@ pub(crate) fn write_certificate(
     req_len += set_length(&mut buf, data.len());
     req_len += data.len();
 
-    if req_len < data.len() || req_len > max_size {
+    if req_len < data.len() || req_len > CB_OBJ_MAX {
         return Err(Error::SizeError);
     }
 

src/consts.rs

@@ -39,21 +39,27 @@ pub const szLOG_SOURCE: &str = "yubikey-piv.rs";
 pub const ADMIN_FLAGS_1_PUK_BLOCKED: u8 = 0x01;
 pub const ADMIN_FLAGS_1_PROTECTED_MGM: u8 = 0x02;
 
+/// PIV Applet ID
+pub const PIV_AID: [u8; 5] = [0xa0, 0x00, 0x00, 0x03, 0x08];
+
+/// MGMT Applet ID.
+/// <https://developers.yubico.com/PIV/Introduction/Admin_access.html>
+pub const MGMT_AID: [u8; 8] = [0xa0, 0x00, 0x00, 0x05, 0x27, 0x47, 0x11, 0x17];
+
+/// YubiKey OTP Applet ID. Needed to query serial on YK4.
+pub const YK_AID: [u8; 8] = [0xa0, 0x00, 0x00, 0x05, 0x27, 0x20, 0x01, 0x01];
+
 pub const CB_ADMIN_TIMESTAMP: usize = 0x04;
 pub const CB_ADMIN_SALT: usize = 16;
 
 pub const CB_ATR_MAX: usize = 33;
 
-pub const CB_BUF_MAX_NEO: usize = 2048;
+pub const CB_BUF_MAX: usize = 3072;
-pub const CB_BUF_MAX_YK4: usize = 3072;
-pub const CB_BUF_MAX: usize = CB_BUF_MAX_YK4;
 
 pub const CB_ECC_POINTP256: usize = 65;
 pub const CB_ECC_POINTP384: usize = 97;
 
-pub const CB_OBJ_MAX_YK4: usize = CB_BUF_MAX_YK4 - 9;
+pub const CB_OBJ_MAX: usize = CB_BUF_MAX - 9;
-pub const CB_OBJ_MAX: usize = CB_OBJ_MAX_YK4;
-pub const CB_OBJ_MAX_NEO: usize = CB_BUF_MAX_NEO - 9;
 
 pub const CB_OBJ_TAG_MIN: usize = 2; // 1 byte tag + 1 byte len
 pub const CB_OBJ_TAG_MAX: usize = (CB_OBJ_TAG_MIN + 2); // 1 byte tag + 3 bytes len
@@ -82,9 +88,7 @@ pub const DES_LEN_3DES: usize = DES_LEN_DES * 3;
 // device types
 
 pub const DEVTYPE_UNKNOWN: u32 = 0x0000_0000;
-pub const DEVTYPE_NEO: u32 = 0x4E45_0000; //"NE"
 pub const DEVTYPE_YK: u32 = 0x594B_0000; //"YK"
-pub const DEVTYPE_NEOr3: u32 = (DEVTYPE_NEO | 0x0000_7233); //"r3"
 pub const DEVTYPE_YK4: u32 = (DEVTYPE_YK | 0x0000_0034); // "4"
 pub const DEVYTPE_YK5: u32 = (DEVTYPE_YK | 0x0000_0035); // "5"
 
@@ -124,8 +128,6 @@ pub const TAG_ECC_POINT: u8 = 0x86;
 
 pub const YKPIV_ALGO_3DES: u8 = 0x03;
 
-pub const YKPIV_ATR_NEO_R3: &[u8] = b";\xFC\x13\0\0\x811\xFE\x15YubikeyNEOr3\xE1\0";
-
 pub const YKPIV_CHUID_SIZE: usize = 59;
 pub const YKPIV_CARDID_SIZE: usize = 16;
 pub const YKPIV_FASCN_SIZE: usize = 25;

src/container.rs

@@ -107,7 +107,6 @@ impl Container {
         let n_containers = containers.len();
         let data_len = n_containers * CONTAINER_REC_LEN;
 
-        let max_size = yubikey.obj_size_max();
         let txn = yubikey.begin_transaction()?;
 
         if n_containers == 0 {
@@ -116,7 +115,7 @@ impl Container {
 
         let req_len = 1 + set_length(&mut buf, data_len) + data_len;
 
-        if req_len > max_size {
+        if req_len > CB_OBJ_MAX {
             return Err(Error::SizeError);
         }
 

src/key.rs

@@ -438,8 +438,7 @@ pub fn generate(
 
     match algorithm {
         AlgorithmId::Rsa1024 | AlgorithmId::Rsa2048 => {
-            if yubikey.device_model() == DEVTYPE_YK4
+            if yubikey.version.major == 4
-                && yubikey.version.major == 4
                 && (yubikey.version.minor < 3
                     || yubikey.version.minor == 3 && (yubikey.version.patch < 5))
             {

src/lib.rs

@@ -18,11 +18,11 @@
 //!
 //! ## Supported YubiKeys
 //!
-//! - [YubiKey NEO] series
 //! - [YubiKey 4] series
 //! - [YubiKey 5] series
 //!
-//! NOTE: Nano and USB-C variants of the above are also supported
+//! NOTE: Nano and USB-C variants of the above are also supported.
+//!       Pre-YK4 [YubiKey NEO] series is **NOT** supported.
 //!
 //! ## Supported Algorithms
 //!

src/metadata.rs

@@ -191,15 +191,10 @@ pub(crate) fn read(txn: &Transaction<'_>, tag: u8) -> Result<Buffer, Error> {
 }
 
 /// Write metadata
-pub(crate) fn write(
+pub(crate) fn write(txn: &Transaction<'_>, tag: u8, data: &[u8]) -> Result<(), Error> {
-    txn: &Transaction<'_>,
-    tag: u8,
-    data: &[u8],
-    max_size: usize,
-) -> Result<(), Error> {
     let mut buf = Zeroizing::new(vec![0u8; CB_OBJ_MAX]);
 
-    if data.len() > max_size - CB_OBJ_TAG_MAX {
+    if data.len() > CB_OBJ_MAX - CB_OBJ_TAG_MAX {
         return Err(Error::GenericError);
     }
 

src/mgm.rs

@@ -167,7 +167,6 @@ impl MgmKey {
     pub fn set_protected(&self, yubikey: &mut YubiKey) -> Result<(), Error> {
         let mut data = Zeroizing::new(vec![0u8; CB_BUF_MAX]);
 
-        let max_size = yubikey.obj_size_max();
         let txn = yubikey.begin_transaction()?;
 
         txn.set_mgm_key(self, None).map_err(|e| {
@@ -200,7 +199,7 @@ impl MgmKey {
         ) {
             error!("could not set protected mgm item, err = {:?}", e);
         } else {
-            metadata::write(&txn, TAG_PROTECTED, &data, max_size).map_err(|e| {
+            metadata::write(&txn, TAG_PROTECTED, &data).map_err(|e| {
                 error!("could not write protected data, err = {:?}", e);
                 e
             })?;
@@ -247,7 +246,7 @@ impl MgmKey {
             &flags_1,
         ) {
             error!("could not set admin flags item, err = {}", e);
-        } else if let Err(e) = metadata::write(&txn, TAG_ADMIN, &data[..cb_data], max_size) {
+        } else if let Err(e) = metadata::write(&txn, TAG_ADMIN, &data[..cb_data]) {
             error!("could not write admin data, err = {}", e);
         }
 

src/msroots.rs

@@ -51,11 +51,10 @@ impl MsRoots {
 
     /// Read `msroots` file from YubiKey
     pub fn read(yubikey: &mut YubiKey) -> Result<Option<Self>, Error> {
-        let cb_data = yubikey.obj_size_max();
         let txn = yubikey.begin_transaction()?;
 
         // allocate first page
-        let mut data = Vec::with_capacity(cb_data);
+        let mut data = Vec::with_capacity(CB_OBJ_MAX);
 
         for object_id in YKPIV_OBJ_MSROOTS1..YKPIV_OBJ_MSROOTS5 {
             let buf = txn.fetch_object(object_id)?;
@@ -106,7 +105,6 @@ impl MsRoots {
         let data = &self.0;
         let data_len = data.len();
         let n_objs: usize;
-        let cb_obj_max = yubikey.obj_size_max();
         let txn = yubikey.begin_transaction()?;
 
         if data_len == 0 {
@@ -114,7 +112,7 @@ impl MsRoots {
         }
 
         // Calculate number of objects required to store blob
-        n_objs = (data_len / (cb_obj_max - CB_OBJ_TAG_MAX)) + 1;
+        n_objs = (data_len / (CB_OBJ_MAX - CB_OBJ_TAG_MAX)) + 1;
 
         if n_objs > 5 {
             return Err(Error::SizeError);
@@ -123,8 +121,8 @@ impl MsRoots {
         for i in 0..n_objs {
             offset = 0;
 
-            data_chunk = if cb_obj_max - CB_OBJ_TAG_MAX < data_len - data_offset {
+            data_chunk = if CB_OBJ_MAX - CB_OBJ_TAG_MAX < data_len - data_offset {
-                cb_obj_max - CB_OBJ_TAG_MAX
+                CB_OBJ_MAX - CB_OBJ_TAG_MAX
             } else {
                 data_len - data_offset
             };

src/transaction.rs

@@ -32,15 +32,6 @@ impl<'tx> Transaction<'tx> {
         })
     }
 
-    /// Get an attribute of the card or card reader.
-    pub fn get_attribute<'buf>(
-        &self,
-        attribute: pcsc::Attribute,
-        buffer: &'buf mut [u8],
-    ) -> Result<&'buf [u8], Error> {
-        Ok(self.inner.get_attribute(attribute, buffer)?)
-    }
-
     /// Transmit a single serialized APDU to the card this transaction is open
     /// with and receive a response.
     ///
@@ -66,7 +57,7 @@ impl<'tx> Transaction<'tx> {
     pub fn select_application(&self) -> Result<(), Error> {
         let response = APDU::new(Ins::SelectApplication)
             .p1(0x04)
-            .data(&AID)
+            .data(&PIV_AID)
             .transmit(self, 0xFF)
             .map_err(|e| {
                 error!("failed communicating with card: '{}'", e);
@@ -102,13 +93,11 @@ impl<'tx> Transaction<'tx> {
 
     /// Get YubiKey device serial number.
     pub fn get_serial(&self, version: Version) -> Result<Serial, Error> {
-        let yk_applet = [0xa0, 0x00, 0x00, 0x05, 0x27, 0x20, 0x01, 0x01];
-
         let response = if version.major < 5 {
-            // get serial from neo/yk4 devices using the otp applet
+            // YK4 requires switching to the yk applet to retrieve the serial
             let sw = APDU::new(Ins::SelectApplication)
                 .p1(0x04)
-                .data(&yk_applet)
+                .data(&YK_AID)
                 .transmit(self, 0xFF)?
                 .status_words();
 
@@ -130,7 +119,7 @@ impl<'tx> Transaction<'tx> {
             // reselect the PIV applet
             let sw = APDU::new(Ins::SelectApplication)
                 .p1(0x04)
-                .data(&AID)
+                .data(&PIV_AID)
                 .transmit(self, 0xFF)?
                 .status_words();
 
@@ -141,7 +130,7 @@ impl<'tx> Transaction<'tx> {
 
             resp
         } else {
-            // get serial from yk5 and later devices using the f8 command
+            // YK5 implements getting the serial as a PIV applet command (0xf8)
             let resp = APDU::new(Ins::GetSerial).transmit(self, 0xFF)?;
 
             if !resp.is_success() {

src/yubikey.rs

@@ -31,12 +31,11 @@
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 use crate::{
-    consts::*,
     error::Error,
     readers::{Reader, Readers},
     transaction::Transaction,
 };
-use log::{error, info, warn};
+use log::{error, info};
 use pcsc::Card;
 use std::{
     convert::TryFrom,
@@ -46,6 +45,7 @@ use std::{
 #[cfg(feature = "untested")]
 use crate::{
     apdu::{Ins, StatusWords, APDU},
+    consts::*,
     metadata,
     mgm::MgmKey,
     Buffer, ObjectId,
@@ -60,13 +60,6 @@ use std::{
     time::{SystemTime, UNIX_EPOCH},
 };
 
-/// PIV Application ID
-pub const AID: [u8; 5] = [0xa0, 0x00, 0x00, 0x03, 0x08];
-
-/// MGMT Application ID.
-/// <https://developers.yubico.com/PIV/Introduction/Admin_access.html>
-pub const MGMT_AID: [u8; 8] = [0xa0, 0x00, 0x00, 0x05, 0x27, 0x47, 0x11, 0x17];
-
 /// Cached YubiKey PIN
 pub type CachedPin = secrecy::SecretVec<u8>;
 
@@ -116,6 +109,12 @@ impl Version {
     }
 }
 
+impl Display for Version {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "{}.{}.{}", self.major, self.minor, self.patch)
+    }
+}
+
 /// YubiKey Device: this is the primary API for opening a session and
 /// performing various operations.
 ///
@@ -126,7 +125,6 @@ impl Version {
 pub struct YubiKey {
     pub(crate) card: Card,
     pub(crate) pin: Option<CachedPin>,
-    pub(crate) is_neo: bool,
     pub(crate) version: Version,
     pub(crate) serial: Serial,
 }
@@ -202,18 +200,6 @@ impl YubiKey {
         self.serial
     }
 
-    /// Get YubiKey device model
-    // TODO(tarcieri): use an emum for this
-    #[cfg(feature = "untested")]
-    pub fn device_model(&self) -> u32 {
-        if self.is_neo {
-            DEVTYPE_NEOr3
-        } else {
-            // TODO(tarcieri): YK5?
-            DEVTYPE_YK4
-        }
-    }
-
     /// Authenticate to the card using the provided management key (MGM).
     #[cfg(feature = "untested")]
     pub fn authenticate(&mut self, mgm_key: MgmKey) -> Result<(), Error> {
@@ -368,7 +354,6 @@ impl YubiKey {
     #[cfg(feature = "untested")]
     pub fn set_pin_last_changed(yubikey: &mut YubiKey) -> Result<(), Error> {
         let mut data = [0u8; CB_BUF_MAX];
-        let max_size = yubikey.obj_size_max();
         let txn = yubikey.begin_transaction()?;
 
         let buffer = metadata::read(&txn, TAG_ADMIN)?;
@@ -394,7 +379,7 @@ impl YubiKey {
             e
         })?;
 
-        metadata::write(&txn, TAG_ADMIN, &data, max_size).map_err(|e| {
+        metadata::write(&txn, TAG_ADMIN, &data).map_err(|e| {
             error!("could not write admin data, err = {}", e);
             e
         })?;
@@ -422,7 +407,6 @@ impl YubiKey {
         let mut tries_remaining: i32 = -1;
         let mut flags = [0];
 
-        let max_size = yubikey.obj_size_max();
         let txn = yubikey.begin_transaction()?;
 
         while tries_remaining != 0 {
@@ -473,7 +457,7 @@ impl YubiKey {
         )
         .is_ok()
         {
-            if metadata::write(&txn, TAG_ADMIN, &data[..cb_data], max_size).is_err() {
+            if metadata::write(&txn, TAG_ADMIN, &data[..cb_data]).is_err() {
                 error!("could not write admin metadata");
             }
         } else {
@@ -565,16 +549,6 @@ impl YubiKey {
 
         Ok(())
     }
-
-    /// Get max object size supported by this device
-    #[cfg(feature = "untested")]
-    pub(crate) fn obj_size_max(&self) -> usize {
-        if self.is_neo {
-            CB_OBJ_MAX_NEO
-        } else {
-            CB_OBJ_MAX
-        }
-    }
 }
 
 impl<'a> TryFrom<&'a Reader<'_>> for YubiKey {
@@ -588,43 +562,18 @@ impl<'a> TryFrom<&'a Reader<'_>> for YubiKey {
 
         info!("connected to reader: {}", reader.name());
 
-        let mut is_neo = false;
+        let (version, serial) = {
-        let version: Version;
-        let serial: Serial;
-
-        {
             let txn = Transaction::new(&mut card)?;
-            let mut atr_buf = [0; CB_ATR_MAX];
-            let atr = txn.get_attribute(pcsc::Attribute::AtrString, &mut atr_buf)?;
-            if atr == YKPIV_ATR_NEO_R3 {
-                is_neo = true;
-            }
-
             txn.select_application()?;
 
-            // now that the PIV application is selected, retrieve the version
+            let v = txn.get_version()?;
-            // and serial number.  Previously the NEO/YK4 required switching
+            let s = txn.get_serial(v)?;
-            // to the yk applet to retrieve the serial, YK5 implements this
+            (v, s)
-            // as a PIV applet command.  Unfortunately, this change requires
+        };
-            // that we retrieve the version number first, so that get_serial
-            // can determine how to get the serial number, which for the NEO/Yk4
-            // will result in another selection of the PIV applet.
-
-            version = txn.get_version().map_err(|e| {
-                warn!("failed to retrieve version: '{}'", e);
-                e
-            })?;
-
-            serial = txn.get_serial(version).map_err(|e| {
-                warn!("failed to retrieve serial number: '{}'", e);
-                e
-            })?;
-        }
 
         let yubikey = YubiKey {
             card,
             pin: None,
-            is_neo,
             version,
             serial,
         };

tests/integration.rs

@@ -4,6 +4,7 @@
 #![warn(missing_docs, rust_2018_idioms, trivial_casts, unused_qualifications)]
 
 use lazy_static::lazy_static;
+use log::trace;
 use std::{env, sync::Mutex};
 use yubikey_piv::{key::Key, YubiKey};
 
@@ -19,7 +20,11 @@ fn init_yubikey() -> Mutex<YubiKey> {
         env_logger::builder().format_timestamp(None).init();
     }
 
-    Mutex::new(YubiKey::open().unwrap())
+    let mut yubikey = YubiKey::open().unwrap();
+    trace!("serial: {}", yubikey.serial());
+    trace!("version: {}", yubikey.version());
+
+    Mutex::new(yubikey)
 }
 
 #[test]
@@ -36,5 +41,5 @@ fn test_list_keys() {
     let mut yubikey = YUBIKEY.lock().unwrap();
     let keys_result = Key::list(&mut yubikey);
     assert!(keys_result.is_ok());
-    dbg!(keys_result.unwrap());
+    trace!("keys: {:?}", keys_result.unwrap());
 }