Merge remote-tracking branch 'upstream/develop' into develop

src/apdu.rs

@@ -280,7 +280,7 @@ impl Response {
 
     /// Get the raw [`StatusWords`] code for this response.
     #[cfg(feature = "untested")]
-    pub fn code(&self) -> u32 {
+    pub fn code(&self) -> u16 {
         self.status_words.code()
     }
 
@@ -317,7 +317,7 @@ impl From<Vec<u8>> for Response {
         }
 
         let sw = StatusWords::from(
-            (bytes[bytes.len() - 2] as u32) << 8 | (bytes[bytes.len() - 1] as u32),
+            (bytes[bytes.len() - 2] as u16) << 8 | (bytes[bytes.len() - 1] as u16),
         );
 
         let len = bytes.len() - 2;
@@ -352,15 +352,42 @@ pub(crate) enum StatusWords {
     /// Successful execution
     Success,
 
+    /// https://github.com/Yubico/yubikey-manager/blob/1f22620b623c6b345dd9f9193ec765a542dddc80/ykman/driver_ccid.py#L53
+    NoInputDataError,
+
+    /// PIN verification failure
+    VerifyFailError {
+        /// Remaining verification attempts
+        tries: u8,
+    },
+
+    /// https://github.com/Yubico/yubikey-manager/blob/1f22620b623c6b345dd9f9193ec765a542dddc80/ykman/driver_ccid.py#L55
+    WrongLengthError,
+
     /// Security status not satisfied
     SecurityStatusError,
 
     /// Authentication method blocked
     AuthBlockedError,
 
+    /// https://github.com/Yubico/yubikey-manager/blob/1f22620b623c6b345dd9f9193ec765a542dddc80/ykman/driver_ccid.py#L58
+    DataInvalidError,
+
+    /// https://github.com/Yubico/yubikey-manager/blob/1f22620b623c6b345dd9f9193ec765a542dddc80/ykman/driver_ccid.py#L59
+    ConditionsNotSatisfiedError,
+
+    /// https://github.com/Yubico/yubikey-manager/blob/1f22620b623c6b345dd9f9193ec765a542dddc80/ykman/driver_ccid.py#L60
+    CommandNotAllowedError,
+
     /// Incorrect parameter in command data field
     IncorrectParamError,
 
+    /// Data object or application not found
+    NotFoundError,
+
+    /// Not enough memory
+    NoSpaceError,
+
     //
     // Custom Yubico Status Word extensions
     //
@@ -370,20 +397,32 @@ pub(crate) enum StatusWords {
     /// Not supported error
     NotSupportedError,
 
+    /// https://github.com/Yubico/yubikey-manager/blob/1f22620b623c6b345dd9f9193ec765a542dddc80/ykman/driver_ccid.py#L65
+    CommandAbortedError,
+
     /// Other/unrecognized status words
-    Other(u32),
+    Other(u16),
 }
 
 impl StatusWords {
     /// Get the numerical response code for these status words
-    pub fn code(self) -> u32 {
+    pub fn code(self) -> u16 {
         match self {
             StatusWords::None => 0,
+            StatusWords::NoInputDataError => 0x6285,
+            StatusWords::VerifyFailError { tries } => 0x63c0 & tries as u16,
+            StatusWords::WrongLengthError => 0x6700,
             StatusWords::SecurityStatusError => 0x6982,
             StatusWords::AuthBlockedError => 0x6983,
+            StatusWords::DataInvalidError => 0x6984,
+            StatusWords::ConditionsNotSatisfiedError => 0x6985,
+            StatusWords::CommandNotAllowedError => 0x6986,
             StatusWords::IncorrectParamError => 0x6a80,
+            StatusWords::NotFoundError => 0x6a82,
+            StatusWords::NoSpaceError => 0x6a84,
             StatusWords::IncorrectSlotError => 0x6b00,
             StatusWords::NotSupportedError => 0x6d00,
+            StatusWords::CommandAbortedError => 0x6f00,
             StatusWords::Success => 0x9000,
             StatusWords::Other(n) => n,
         }
@@ -395,23 +434,34 @@ impl StatusWords {
     }
 }
 
-impl From<u32> for StatusWords {
-    fn from(sw: u32) -> Self {
+impl From<u16> for StatusWords {
+    fn from(sw: u16) -> Self {
         match sw {
             0x0000 => StatusWords::None,
+            0x6285 => StatusWords::NoInputDataError,
+            sw if sw & 0xfff0 == 0x63c0 => StatusWords::VerifyFailError {
+                tries: (sw & 0x000f) as u8,
+            },
+            0x6700 => StatusWords::WrongLengthError,
             0x6982 => StatusWords::SecurityStatusError,
             0x6983 => StatusWords::AuthBlockedError,
+            0x6984 => StatusWords::DataInvalidError,
+            0x6985 => StatusWords::ConditionsNotSatisfiedError,
+            0x6986 => StatusWords::CommandNotAllowedError,
             0x6a80 => StatusWords::IncorrectParamError,
+            0x6a82 => StatusWords::NotFoundError,
+            0x6a84 => StatusWords::NoSpaceError,
             0x6b00 => StatusWords::IncorrectSlotError,
             0x6d00 => StatusWords::NotSupportedError,
+            0x6f00 => StatusWords::CommandAbortedError,
             0x9000 => StatusWords::Success,
             _ => StatusWords::Other(sw),
         }
     }
 }
 
-impl From<StatusWords> for u32 {
-    fn from(sw: StatusWords) -> u32 {
+impl From<StatusWords> for u16 {
+    fn from(sw: StatusWords) -> u16 {
         sw.code()
     }
 }

src/certificate.rs

@@ -33,19 +33,141 @@
 use crate::{
     consts::*,
     error::Error,
-    key::{self, SlotId},
+    key::{AlgorithmId, SlotId},
     serialization::*,
     transaction::Transaction,
     yubikey::YubiKey,
     Buffer,
 };
+use ecdsa::{
+    curve::{CompressedCurvePoint, NistP256, NistP384, UncompressedCurvePoint},
+    generic_array::GenericArray,
+};
 use log::error;
-use std::ptr;
+use rsa::{PublicKey, RSAPublicKey};
+use std::fmt;
+use x509_parser::{parse_x509_der, x509::SubjectPublicKeyInfo};
 use zeroize::Zeroizing;
 
+// TODO: Make these der_parser::oid::Oid constants when it has const fn support.
+const OID_RSA_ENCRYPTION: &str = "1.2.840.113549.1.1.1";
+const OID_EC_PUBLIC_KEY: &str = "1.2.840.10045.2.1";
+const OID_NIST_P256: &str = "1.2.840.10045.3.1.7";
+const OID_NIST_P384: &str = "1.3.132.0.34";
+
+/// An encoded point on the Nist P-256 curve.
+#[derive(Clone, Eq, PartialEq)]
+pub enum EcP256Point {
+    /// Compressed encoding of a point on the curve.
+    Compressed(CompressedCurvePoint<NistP256>),
+
+    /// Uncompressed encoding of a point on the curve.
+    Uncompressed(UncompressedCurvePoint<NistP256>),
+}
+
+/// An encoded point on the Nist P-384 curve.
+#[derive(Clone, Eq, PartialEq)]
+pub enum EcP384Point {
+    /// Compressed encoding of a point on the curve.
+    Compressed(CompressedCurvePoint<NistP384>),
+
+    /// Uncompressed encoding of a point on the curve.
+    Uncompressed(UncompressedCurvePoint<NistP384>),
+}
+
+/// Information about a public key within a [`Certificate`].
+#[derive(Clone, Eq, PartialEq)]
+pub enum PublicKeyInfo {
+    /// RSA keys
+    Rsa {
+        /// RSA algorithm
+        algorithm: AlgorithmId,
+
+        /// Public key
+        pubkey: RSAPublicKey,
+    },
+
+    /// EC P-256 keys
+    EcP256(EcP256Point),
+
+    /// EC P-384 keys
+    EcP384(EcP384Point),
+}
+
+impl fmt::Debug for PublicKeyInfo {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "PublicKeyInfo({:?})", self.algorithm())
+    }
+}
+
+impl PublicKeyInfo {
+    fn parse(subject_pki: &SubjectPublicKeyInfo<'_>) -> Result<Self, Error> {
+        match subject_pki.algorithm.algorithm.to_string().as_str() {
+            OID_RSA_ENCRYPTION => {
+                let pubkey = read_pki::rsa_pubkey(subject_pki.subject_public_key.data)?;
+
+                Ok(PublicKeyInfo::Rsa {
+                    algorithm: match pubkey.n().bits() {
+                        1024 => AlgorithmId::Rsa1024,
+                        2048 => AlgorithmId::Rsa2048,
+                        _ => return Err(Error::AlgorithmError),
+                    },
+                    pubkey,
+                })
+            }
+            OID_EC_PUBLIC_KEY => {
+                let key_bytes = &subject_pki.subject_public_key.data;
+                match read_pki::ec_parameters(&subject_pki.algorithm.parameters)? {
+                    AlgorithmId::EccP256 => match key_bytes.len() {
+                        33 => CompressedCurvePoint::<NistP256>::from_bytes(
+                            GenericArray::clone_from_slice(key_bytes),
+                        )
+                        .map(EcP256Point::Compressed),
+                        65 => UncompressedCurvePoint::<NistP256>::from_bytes(
+                            GenericArray::clone_from_slice(key_bytes),
+                        )
+                        .map(EcP256Point::Uncompressed),
+                        _ => None,
+                    }
+                    .map(PublicKeyInfo::EcP256)
+                    .ok_or(Error::InvalidObject),
+                    AlgorithmId::EccP384 => match key_bytes.len() {
+                        49 => CompressedCurvePoint::<NistP384>::from_bytes(
+                            GenericArray::clone_from_slice(key_bytes),
+                        )
+                        .map(EcP384Point::Compressed),
+                        97 => UncompressedCurvePoint::<NistP384>::from_bytes(
+                            GenericArray::clone_from_slice(key_bytes),
+                        )
+                        .map(EcP384Point::Uncompressed),
+                        _ => None,
+                    }
+                    .map(PublicKeyInfo::EcP384)
+                    .ok_or(Error::InvalidObject),
+                    _ => Err(Error::AlgorithmError),
+                }
+            }
+            _ => Err(Error::InvalidObject),
+        }
+    }
+
+    /// Returns the algorithm that this public key can be used with.
+    pub fn algorithm(&self) -> AlgorithmId {
+        match self {
+            PublicKeyInfo::Rsa { algorithm, .. } => *algorithm,
+            PublicKeyInfo::EcP256(_) => AlgorithmId::EccP256,
+            PublicKeyInfo::EcP384(_) => AlgorithmId::EccP384,
+        }
+    }
+}
+
 /// Certificates
 #[derive(Clone, Debug)]
-pub struct Certificate(Buffer);
+pub struct Certificate {
+    subject: String,
+    subject_pki: PublicKeyInfo,
+    data: Buffer,
+}
 
 impl Certificate {
     /// Read a certificate from the given slot in the YubiKey
@@ -57,14 +179,14 @@ impl Certificate {
             return Err(Error::InvalidObject);
         }
 
-        Ok(Certificate(buf))
+        Certificate::new(buf)
     }
 
     /// Write this certificate into the YubiKey in the given slot
     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.0), certinfo, max_size)
+        write_certificate(&txn, slot, Some(&self.data), certinfo, max_size)
     }
 
     /// Delete a certificate located at the given slot of the given YubiKey
@@ -83,25 +205,47 @@ impl Certificate {
             return Err(Error::SizeError);
         }
 
-        Ok(Certificate(cert))
+        let parsed_cert = match parse_x509_der(&cert) {
+            Ok((_, cert)) => cert,
+            _ => return Err(Error::InvalidObject),
+        };
+
+        let subject = format!("{}", parsed_cert.tbs_certificate.subject);
+        let subject_pki = PublicKeyInfo::parse(&parsed_cert.tbs_certificate.subject_pki)?;
+
+        Ok(Certificate {
+            subject,
+            subject_pki,
+            data: cert,
+        })
+    }
+
+    /// Returns the SubjectName field of the certificate.
+    pub fn subject(&self) -> &str {
+        &self.subject
+    }
+
+    /// Returns the SubjectPublicKeyInfo field of the certificate.
+    pub fn subject_pki(&self) -> &PublicKeyInfo {
+        &self.subject_pki
     }
 
     /// Extract the inner buffer
     pub fn into_buffer(self) -> Buffer {
-        self.0
+        self.data
     }
 }
 
 impl AsRef<[u8]> for Certificate {
     fn as_ref(&self) -> &[u8] {
-        self.0.as_ref()
+        self.data.as_ref()
     }
 }
 
 /// Read certificate
 pub(crate) fn read_certificate(txn: &Transaction<'_>, slot: SlotId) -> Result<Buffer, Error> {
     let mut len: usize = 0;
-    let object_id = key::slot_object(slot)?;
+    let object_id = slot.object_id();
 
     let mut buf = match txn.fetch_object(object_id) {
         Ok(b) => b,
@@ -124,10 +268,7 @@ pub(crate) fn read_certificate(txn: &Transaction<'_>, slot: SlotId) -> Result<Bu
             return Ok(Zeroizing::new(vec![]));
         }
 
-        unsafe {
-            ptr::copy(buf.as_ptr().add(offset), buf.as_mut_ptr(), len);
-        }
-
+        buf.copy_within(offset..offset + len, 0);
         buf.truncate(len);
     }
 
@@ -145,7 +286,7 @@ pub(crate) fn write_certificate(
     let mut buf = [0u8; CB_OBJ_MAX];
     let mut offset = 0;
 
-    let object_id = key::slot_object(slot)?;
+    let object_id = slot.object_id();
 
     if data.is_none() {
         return txn.save_object(object_id, &[]);
@@ -184,3 +325,78 @@ pub(crate) fn write_certificate(
 
     txn.save_object(object_id, &buf[..offset])
 }
+
+mod read_pki {
+    use der_parser::{
+        ber::BerObjectContent,
+        der::{parse_der_integer, DerObject},
+        error::BerError,
+        *,
+    };
+    use nom::{combinator, IResult};
+    use rsa::{BigUint, RSAPublicKey};
+
+    use super::{OID_NIST_P256, OID_NIST_P384};
+    use crate::{error::Error, key::AlgorithmId};
+
+    /// From [RFC 8017](https://tools.ietf.org/html/rfc8017#appendix-A.1.1):
+    /// ```text
+    /// RSAPublicKey ::= SEQUENCE {
+    ///     modulus           INTEGER,  -- n
+    ///     publicExponent    INTEGER   -- e
+    /// }
+    /// ```
+    pub(super) fn rsa_pubkey(encoded: &[u8]) -> Result<RSAPublicKey, Error> {
+        fn parse_rsa_pubkey(i: &[u8]) -> IResult<&[u8], DerObject<'_>, BerError> {
+            parse_der_sequence_defined!(i, parse_der_integer >> parse_der_integer)
+        }
+
+        fn rsa_pubkey_parts(i: &[u8]) -> IResult<&[u8], (BigUint, BigUint), BerError> {
+            combinator::map(parse_rsa_pubkey, |object| {
+                let seq = object.as_sequence().expect("is DER sequence");
+                assert_eq!(seq.len(), 2);
+
+                let n = match seq[0].content {
+                    BerObjectContent::Integer(s) => BigUint::from_bytes_be(s),
+                    _ => panic!("expected DER integer"),
+                };
+                let e = match seq[1].content {
+                    BerObjectContent::Integer(s) => BigUint::from_bytes_be(s),
+                    _ => panic!("expected DER integer"),
+                };
+
+                (n, e)
+            })(i)
+        }
+
+        let (n, e) = match rsa_pubkey_parts(encoded) {
+            Ok((_, res)) => res,
+            _ => return Err(Error::InvalidObject),
+        };
+
+        RSAPublicKey::new(n, e).map_err(|_| Error::InvalidObject)
+    }
+
+    /// From [RFC 5480](https://tools.ietf.org/html/rfc5480#section-2.1.1):
+    /// ```text
+    /// ECParameters ::= CHOICE {
+    ///   namedCurve         OBJECT IDENTIFIER
+    ///   -- implicitCurve   NULL
+    ///   -- specifiedCurve  SpecifiedECDomain
+    /// }
+    /// ```
+    pub(super) fn ec_parameters(parameters: &DerObject<'_>) -> Result<AlgorithmId, Error> {
+        let curve_oid = match parameters.as_context_specific() {
+            Ok((_, Some(named_curve))) => {
+                named_curve.as_oid_val().map_err(|_| Error::InvalidObject)
+            }
+            _ => Err(Error::InvalidObject),
+        }?;
+
+        match curve_oid.to_string().as_str() {
+            OID_NIST_P256 => Ok(AlgorithmId::EccP256),
+            OID_NIST_P384 => Ok(AlgorithmId::EccP384),
+            _ => Err(Error::AlgorithmError),
+        }
+    }
+}

src/consts.rs

@@ -124,10 +124,6 @@ pub const TAG_ECC_POINT: u8 = 0x86;
 
 pub const YKPIV_ALGO_TAG: u8 = 0x80;
 pub const YKPIV_ALGO_3DES: u8 = 0x03;
-pub const YKPIV_ALGO_RSA1024: u8 = 0x06;
-pub const YKPIV_ALGO_RSA2048: u8 = 0x07;
-pub const YKPIV_ALGO_ECCP256: u8 = 0x11;
-pub const YKPIV_ALGO_ECCP384: u8 = 0x14;
 
 pub const YKPIV_ATR_NEO_R3: &[u8] = b";\xFC\x13\0\0\x811\xFE\x15YubikeyNEOr3\xE1\0";
 
@@ -142,72 +138,21 @@ pub const YKPIV_CCC_SIZE: usize = 51;
 pub const YKPIV_CERTINFO_UNCOMPRESSED: u8 = 0;
 pub const YKPIV_CERTINFO_GZIP: u8 = 1;
 
-pub const YKPIV_KEY_AUTHENTICATION: u8 = 0x9a;
 pub const YKPIV_KEY_CARDMGM: u8 = 0x9b;
-pub const YKPIV_KEY_SIGNATURE: u8 = 0x9c;
-pub const YKPIV_KEY_KEYMGM: u8 = 0x9d;
-pub const YKPIV_KEY_CARDAUTH: u8 = 0x9e;
-pub const YKPIV_KEY_RETIRED1: u8 = 0x82;
-pub const YKPIV_KEY_RETIRED2: u8 = 0x83;
-pub const YKPIV_KEY_RETIRED3: u8 = 0x84;
-pub const YKPIV_KEY_RETIRED4: u8 = 0x85;
-pub const YKPIV_KEY_RETIRED5: u8 = 0x86;
-pub const YKPIV_KEY_RETIRED6: u8 = 0x87;
-pub const YKPIV_KEY_RETIRED7: u8 = 0x88;
-pub const YKPIV_KEY_RETIRED8: u8 = 0x89;
-pub const YKPIV_KEY_RETIRED9: u8 = 0x8a;
-pub const YKPIV_KEY_RETIRED10: u8 = 0x8b;
-pub const YKPIV_KEY_RETIRED11: u8 = 0x8c;
-pub const YKPIV_KEY_RETIRED12: u8 = 0x8d;
-pub const YKPIV_KEY_RETIRED13: u8 = 0x8e;
-pub const YKPIV_KEY_RETIRED14: u8 = 0x8f;
-pub const YKPIV_KEY_RETIRED15: u8 = 0x90;
-pub const YKPIV_KEY_RETIRED16: u8 = 0x91;
-pub const YKPIV_KEY_RETIRED17: u8 = 0x92;
-pub const YKPIV_KEY_RETIRED18: u8 = 0x93;
-pub const YKPIV_KEY_RETIRED19: u8 = 0x94;
-pub const YKPIV_KEY_RETIRED20: u8 = 0x95;
-pub const YKPIV_KEY_ATTESTATION: u8 = 0xf9;
 
 pub const YKPIV_OBJ_CAPABILITY: u32 = 0x005f_c107;
 pub const YKPIV_OBJ_CHUID: u32 = 0x005f_c102;
-pub const YKPIV_OBJ_AUTHENTICATION: u32 = 0x005f_c105; // cert for 9a key
 pub const YKPIV_OBJ_FINGERPRINTS: u32 = 0x005f_c103;
 pub const YKPIV_OBJ_SECURITY: u32 = 0x005f_c106;
 pub const YKPIV_OBJ_FACIAL: u32 = 0x005f_c108;
 pub const YKPIV_OBJ_PRINTED: u32 = 0x005f_c109;
-pub const YKPIV_OBJ_SIGNATURE: u32 = 0x005f_c10a; // cert for 9c key
-pub const YKPIV_OBJ_KEY_MANAGEMENT: u32 = 0x005f_c10b; // cert for 9d key
-pub const YKPIV_OBJ_CARD_AUTH: u32 = 0x005f_c101; // cert for 9e key
 pub const YKPIV_OBJ_DISCOVERY: u32 = 0x7e;
 pub const YKPIV_OBJ_KEY_HISTORY: u32 = 0x005f_c10c;
 pub const YKPIV_OBJ_IRIS: u32 = 0x005f_c121;
 
-pub const YKPIV_OBJ_RETIRED1: u32 = 0x005f_c10d;
-pub const YKPIV_OBJ_RETIRED2: u32 = 0x005f_c10e;
-pub const YKPIV_OBJ_RETIRED3: u32 = 0x005f_c10f;
-pub const YKPIV_OBJ_RETIRED4: u32 = 0x005f_c110;
-pub const YKPIV_OBJ_RETIRED5: u32 = 0x005f_c111;
-pub const YKPIV_OBJ_RETIRED6: u32 = 0x005f_c112;
-pub const YKPIV_OBJ_RETIRED7: u32 = 0x005f_c113;
-pub const YKPIV_OBJ_RETIRED8: u32 = 0x005f_c114;
-pub const YKPIV_OBJ_RETIRED9: u32 = 0x005f_c115;
-pub const YKPIV_OBJ_RETIRED10: u32 = 0x005f_c116;
-pub const YKPIV_OBJ_RETIRED11: u32 = 0x005f_c117;
-pub const YKPIV_OBJ_RETIRED12: u32 = 0x005f_c118;
-pub const YKPIV_OBJ_RETIRED13: u32 = 0x005f_c119;
-pub const YKPIV_OBJ_RETIRED14: u32 = 0x005f_c11a;
-pub const YKPIV_OBJ_RETIRED15: u32 = 0x005f_c11b;
-pub const YKPIV_OBJ_RETIRED16: u32 = 0x005f_c11c;
-pub const YKPIV_OBJ_RETIRED17: u32 = 0x005f_c11d;
-pub const YKPIV_OBJ_RETIRED18: u32 = 0x005f_c11e;
-pub const YKPIV_OBJ_RETIRED19: u32 = 0x005f_c11f;
-pub const YKPIV_OBJ_RETIRED20: u32 = 0x005f_c120;
-
 // Internal object IDs
 
 pub const YKPIV_OBJ_ADMIN_DATA: u32 = 0x005f_ff00;
-pub const YKPIV_OBJ_ATTESTATION: u32 = 0x005f_ff01;
 pub const YKPIV_OBJ_MSCMAP: u32 = 0x005f_ff10;
 pub const YKPIV_OBJ_MSROOTS1: u32 = 0x005f_ff11;
 pub const YKPIV_OBJ_MSROOTS2: u32 = 0x005f_ff12;

src/container.rs

@@ -158,7 +158,7 @@ impl Container {
 
         Ok(Container {
             name,
-            slot: bytes[name_bytes_len],
+            slot: bytes[name_bytes_len].try_into()?,
             key_spec: bytes[name_bytes_len + 1],
             key_size_bits: u16::from_le_bytes(
                 bytes[(name_bytes_len + 2)..(name_bytes_len + 4)]
@@ -185,7 +185,7 @@ impl Container {
             bytes.extend_from_slice(&self.name[i].to_le_bytes());
         }
 
-        bytes.push(self.slot);
+        bytes.push(self.slot.into());
         bytes.push(self.key_spec);
         bytes.extend_from_slice(&self.key_size_bits.to_le_bytes());
         bytes.push(self.flags);
@@ -204,7 +204,7 @@ impl Debug for Container {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         write!(
             f,
-            "PivContainer {{ name: {:?}, slot: {}, key_spec: {}, key_size_bits: {}, \
+            "PivContainer {{ name: {:?}, slot: {:?}, key_spec: {}, key_size_bits: {}, \
              flags: {}, pin_id: {}, associated_echd_container: {}, cert_fingerprint: {:?} }}",
             &self.name[..],
             self.slot,

src/error.rs

@@ -68,7 +68,7 @@ pub enum Error {
     /// Wrong PIN
     WrongPin {
         /// Number of tries remaining
-        tries: u32,
+        tries: u8,
     },
 
     /// Invalid object

src/key.rs

@@ -45,61 +45,272 @@ use crate::{
     serialization::*,
     settings,
     yubikey::YubiKey,
-    AlgorithmId, Buffer, ObjectId,
+    Buffer, ObjectId,
 };
 use log::{debug, error, warn};
+use std::convert::TryFrom;
 
 /// Slot identifiers.
 /// <https://developers.yubico.com/PIV/Introduction/Certificate_slots.html>
-// TODO(tarcieri): replace these with enums
-pub type SlotId = u8;
+#[derive(Clone, Copy, Debug, PartialEq)]
+pub enum SlotId {
+    /// This certificate and its associated private key is used to authenticate the card
+    /// and the cardholder. This slot is used for things like system login. The end user
+    /// PIN is required to perform any private key operations. Once the PIN has been
+    /// provided successfully, multiple private key operations may be performed without
+    /// additional cardholder consent.
+    Authentication,
 
-/// Get the [`ObjectId`] that corresponds to a given [`SlotId`]
-// TODO(tarcieri): factor this into a slot ID enum
-pub(crate) fn slot_object(slot: SlotId) -> Result<ObjectId, Error> {
-    let id = match slot {
-        YKPIV_KEY_AUTHENTICATION => YKPIV_OBJ_AUTHENTICATION,
-        YKPIV_KEY_SIGNATURE => YKPIV_OBJ_SIGNATURE,
-        YKPIV_KEY_KEYMGM => YKPIV_OBJ_KEY_MANAGEMENT,
-        YKPIV_KEY_CARDAUTH => YKPIV_OBJ_CARD_AUTH,
-        YKPIV_KEY_ATTESTATION => YKPIV_OBJ_ATTESTATION,
-        slot if slot >= YKPIV_KEY_RETIRED1 && (slot <= YKPIV_KEY_RETIRED20) => {
-            YKPIV_OBJ_RETIRED1 + (slot - YKPIV_KEY_RETIRED1) as u32
+    /// This certificate and its associated private key is used for digital signatures for
+    /// the purpose of document signing, or signing files and executables. The end user
+    /// PIN is required to perform any private key operations. The PIN must be submitted
+    /// every time immediately before a sign operation, to ensure cardholder participation
+    /// for every digital signature generated.
+    Signature,
+
+    /// This certificate and its associated private key is used for encryption for the
+    /// purpose of confidentiality. This slot is used for things like encrypting e-mails
+    /// or files. The end user PIN is required to perform any private key operations. Once
+    /// the PIN has been provided successfully, multiple private key operations may be
+    /// performed without additional cardholder consent.
+    KeyManagement,
+
+    /// This certificate and its associated private key is used to support additional
+    /// physical access applications, such as providing physical access to buildings via
+    /// PIV-enabled door locks. The end user PIN is NOT required to perform private key
+    /// operations for this slot.
+    CardAuthentication,
+
+    /// These slots are only available on the YubiKey 4 & 5. They are meant for previously
+    /// used Key Management keys to be able to decrypt earlier encrypted documents or
+    /// emails. In the YubiKey 4 & 5 all 20 of them are fully available for use.
+    Retired(RetiredSlotId),
+
+    /// This slot is only available on YubiKey version 4.3 and newer. It is only used for
+    /// attestation of other keys generated on device with instruction `f9`. This slot is
+    /// not cleared on reset, but can be overwritten.
+    Attestation,
+}
+
+impl TryFrom<u8> for SlotId {
+    type Error = Error;
+
+    fn try_from(value: u8) -> Result<Self, Self::Error> {
+        match value {
+            0x9a => Ok(SlotId::Authentication),
+            0x9c => Ok(SlotId::Signature),
+            0x9d => Ok(SlotId::KeyManagement),
+            0x9e => Ok(SlotId::CardAuthentication),
+            0xf9 => Ok(SlotId::Attestation),
+            _ => RetiredSlotId::try_from(value).map(SlotId::Retired),
         }
-        _ => return Err(Error::InvalidObject),
-    };
+    }
+}
 
-    Ok(id)
+impl From<SlotId> for u8 {
+    fn from(slot: SlotId) -> u8 {
+        match slot {
+            SlotId::Authentication => 0x9a,
+            SlotId::Signature => 0x9c,
+            SlotId::KeyManagement => 0x9d,
+            SlotId::CardAuthentication => 0x9e,
+            SlotId::Retired(retired) => retired.into(),
+            SlotId::Attestation => 0xf9,
+        }
+    }
+}
+
+impl SlotId {
+    /// Returns the [`ObjectId`] that corresponds to a given [`SlotId`].
+    pub(crate) fn object_id(self) -> ObjectId {
+        match self {
+            SlotId::Authentication => 0x005f_c105,
+            SlotId::Signature => 0x005f_c10a,
+            SlotId::KeyManagement => 0x005f_c10b,
+            SlotId::CardAuthentication => 0x005f_c101,
+            SlotId::Retired(retired) => retired.object_id(),
+            SlotId::Attestation => 0x005f_ff01,
+        }
+    }
+}
+
+/// Retired slot IDs.
+#[allow(missing_docs)]
+#[derive(Clone, Copy, Debug, PartialEq)]
+pub enum RetiredSlotId {
+    R1,
+    R2,
+    R3,
+    R4,
+    R5,
+    R6,
+    R7,
+    R8,
+    R9,
+    R10,
+    R11,
+    R12,
+    R13,
+    R14,
+    R15,
+    R16,
+    R17,
+    R18,
+    R19,
+    R20,
+}
+
+impl TryFrom<u8> for RetiredSlotId {
+    type Error = Error;
+
+    fn try_from(value: u8) -> Result<Self, Self::Error> {
+        match value {
+            0x82 => Ok(RetiredSlotId::R1),
+            0x83 => Ok(RetiredSlotId::R2),
+            0x84 => Ok(RetiredSlotId::R3),
+            0x85 => Ok(RetiredSlotId::R4),
+            0x86 => Ok(RetiredSlotId::R5),
+            0x87 => Ok(RetiredSlotId::R6),
+            0x88 => Ok(RetiredSlotId::R7),
+            0x89 => Ok(RetiredSlotId::R8),
+            0x8a => Ok(RetiredSlotId::R9),
+            0x8b => Ok(RetiredSlotId::R10),
+            0x8c => Ok(RetiredSlotId::R11),
+            0x8d => Ok(RetiredSlotId::R12),
+            0x8e => Ok(RetiredSlotId::R13),
+            0x8f => Ok(RetiredSlotId::R14),
+            0x90 => Ok(RetiredSlotId::R15),
+            0x91 => Ok(RetiredSlotId::R16),
+            0x92 => Ok(RetiredSlotId::R17),
+            0x93 => Ok(RetiredSlotId::R18),
+            0x94 => Ok(RetiredSlotId::R19),
+            0x95 => Ok(RetiredSlotId::R20),
+            _ => Err(Error::InvalidObject),
+        }
+    }
+}
+
+impl From<RetiredSlotId> for u8 {
+    fn from(slot: RetiredSlotId) -> u8 {
+        match slot {
+            RetiredSlotId::R1 => 0x82,
+            RetiredSlotId::R2 => 0x83,
+            RetiredSlotId::R3 => 0x84,
+            RetiredSlotId::R4 => 0x85,
+            RetiredSlotId::R5 => 0x86,
+            RetiredSlotId::R6 => 0x87,
+            RetiredSlotId::R7 => 0x88,
+            RetiredSlotId::R8 => 0x89,
+            RetiredSlotId::R9 => 0x8a,
+            RetiredSlotId::R10 => 0x8b,
+            RetiredSlotId::R11 => 0x8c,
+            RetiredSlotId::R12 => 0x8d,
+            RetiredSlotId::R13 => 0x8e,
+            RetiredSlotId::R14 => 0x8f,
+            RetiredSlotId::R15 => 0x90,
+            RetiredSlotId::R16 => 0x91,
+            RetiredSlotId::R17 => 0x92,
+            RetiredSlotId::R18 => 0x93,
+            RetiredSlotId::R19 => 0x94,
+            RetiredSlotId::R20 => 0x95,
+        }
+    }
+}
+
+impl RetiredSlotId {
+    /// Returns the [`ObjectId`] that corresponds to a given [`RetiredSlotId`].
+    pub(crate) fn object_id(self) -> ObjectId {
+        match self {
+            RetiredSlotId::R1 => 0x005f_c10d,
+            RetiredSlotId::R2 => 0x005f_c10e,
+            RetiredSlotId::R3 => 0x005f_c10f,
+            RetiredSlotId::R4 => 0x005f_c110,
+            RetiredSlotId::R5 => 0x005f_c111,
+            RetiredSlotId::R6 => 0x005f_c112,
+            RetiredSlotId::R7 => 0x005f_c113,
+            RetiredSlotId::R8 => 0x005f_c114,
+            RetiredSlotId::R9 => 0x005f_c115,
+            RetiredSlotId::R10 => 0x005f_c116,
+            RetiredSlotId::R11 => 0x005f_c117,
+            RetiredSlotId::R12 => 0x005f_c118,
+            RetiredSlotId::R13 => 0x005f_c119,
+            RetiredSlotId::R14 => 0x005f_c11a,
+            RetiredSlotId::R15 => 0x005f_c11b,
+            RetiredSlotId::R16 => 0x005f_c11c,
+            RetiredSlotId::R17 => 0x005f_c11d,
+            RetiredSlotId::R18 => 0x005f_c11e,
+            RetiredSlotId::R19 => 0x005f_c11f,
+            RetiredSlotId::R20 => 0x005f_c120,
+        }
+    }
 }
 
 /// Personal Identity Verification (PIV) key slots
-pub const SLOTS: [u8; 24] = [
-    YKPIV_KEY_AUTHENTICATION,
-    YKPIV_KEY_SIGNATURE,
-    YKPIV_KEY_KEYMGM,
-    YKPIV_KEY_RETIRED1,
-    YKPIV_KEY_RETIRED2,
-    YKPIV_KEY_RETIRED3,
-    YKPIV_KEY_RETIRED4,
-    YKPIV_KEY_RETIRED5,
-    YKPIV_KEY_RETIRED6,
-    YKPIV_KEY_RETIRED7,
-    YKPIV_KEY_RETIRED8,
-    YKPIV_KEY_RETIRED9,
-    YKPIV_KEY_RETIRED10,
-    YKPIV_KEY_RETIRED11,
-    YKPIV_KEY_RETIRED12,
-    YKPIV_KEY_RETIRED13,
-    YKPIV_KEY_RETIRED14,
-    YKPIV_KEY_RETIRED15,
-    YKPIV_KEY_RETIRED16,
-    YKPIV_KEY_RETIRED17,
-    YKPIV_KEY_RETIRED18,
-    YKPIV_KEY_RETIRED19,
-    YKPIV_KEY_RETIRED20,
-    YKPIV_KEY_CARDAUTH,
+pub const SLOTS: [SlotId; 24] = [
+    SlotId::Authentication,
+    SlotId::Signature,
+    SlotId::KeyManagement,
+    SlotId::Retired(RetiredSlotId::R1),
+    SlotId::Retired(RetiredSlotId::R2),
+    SlotId::Retired(RetiredSlotId::R3),
+    SlotId::Retired(RetiredSlotId::R4),
+    SlotId::Retired(RetiredSlotId::R5),
+    SlotId::Retired(RetiredSlotId::R6),
+    SlotId::Retired(RetiredSlotId::R7),
+    SlotId::Retired(RetiredSlotId::R8),
+    SlotId::Retired(RetiredSlotId::R9),
+    SlotId::Retired(RetiredSlotId::R10),
+    SlotId::Retired(RetiredSlotId::R11),
+    SlotId::Retired(RetiredSlotId::R12),
+    SlotId::Retired(RetiredSlotId::R13),
+    SlotId::Retired(RetiredSlotId::R14),
+    SlotId::Retired(RetiredSlotId::R15),
+    SlotId::Retired(RetiredSlotId::R16),
+    SlotId::Retired(RetiredSlotId::R17),
+    SlotId::Retired(RetiredSlotId::R18),
+    SlotId::Retired(RetiredSlotId::R19),
+    SlotId::Retired(RetiredSlotId::R20),
+    SlotId::CardAuthentication,
 ];
 
+/// Algorithm identifiers
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+pub enum AlgorithmId {
+    /// 1024-bit RSA.
+    Rsa1024,
+    /// 2048-bit RSA.
+    Rsa2048,
+    /// ECDSA with the NIST P256 curve.
+    EccP256,
+    /// ECDSA with the NIST P384 curve.
+    EccP384,
+}
+
+impl TryFrom<u8> for AlgorithmId {
+    type Error = Error;
+
+    fn try_from(value: u8) -> Result<Self, Self::Error> {
+        match value {
+            0x06 => Ok(AlgorithmId::Rsa1024),
+            0x07 => Ok(AlgorithmId::Rsa2048),
+            0x11 => Ok(AlgorithmId::EccP256),
+            0x14 => Ok(AlgorithmId::EccP384),
+            _ => Err(Error::AlgorithmError),
+        }
+    }
+}
+
+impl From<AlgorithmId> for u8 {
+    fn from(id: AlgorithmId) -> u8 {
+        match id {
+            AlgorithmId::Rsa1024 => 0x06,
+            AlgorithmId::Rsa2048 => 0x07,
+            AlgorithmId::EccP256 => 0x11,
+            AlgorithmId::EccP384 => 0x14,
+        }
+    }
+}
+
 /// PIV cryptographic keys stored in a YubiKey
 #[derive(Clone, Debug)]
 pub struct Key {
@@ -120,13 +331,15 @@ impl Key {
             let buf = match certificate::read_certificate(&txn, slot) {
                 Ok(b) => b,
                 Err(e) => {
-                    debug!("error reading certificate in slot {}: {}", slot, e);
+                    debug!("error reading certificate in slot {:?}: {}", slot, e);
                     continue;
                 }
             };
 
-            let cert = Certificate::new(buf)?;
-            keys.push(Key { slot, cert });
+            if !buf.is_empty() {
+                let cert = Certificate::new(buf)?;
+                keys.push(Key { slot, cert });
+            }
         }
 
         Ok(keys)
@@ -202,55 +415,52 @@ pub fn generate(
     let mut templ = [0, Ins::GenerateAsymmetric.code(), 0, 0];
     let setting_roca: settings::BoolValue;
 
-    if yubikey.device_model() == DEVTYPE_YK4
-        && (algorithm == YKPIV_ALGO_RSA1024 || algorithm == YKPIV_ALGO_RSA2048)
-        && yubikey.version.major == 4
-        && (yubikey.version.minor < 3 || yubikey.version.minor == 3 && (yubikey.version.patch < 5))
-    {
-        setting_roca = settings::BoolValue::get(SZ_SETTING_ROCA, true);
-
-        let psz_msg = match setting_roca.source {
-            settings::Source::User => {
-                if setting_roca.value {
-                    SZ_ROCA_ALLOW_USER
-                } else {
-                    SZ_ROCA_BLOCK_USER
-                }
-            }
-            settings::Source::Admin => {
-                if setting_roca.value {
-                    SZ_ROCA_ALLOW_ADMIN
-                } else {
-                    SZ_ROCA_BLOCK_ADMIN
-                }
-            }
-            _ => SZ_ROCA_DEFAULT,
-        };
-
-        warn!(
-            "YubiKey serial number {} is affected by vulnerability CVE-2017-15361 \
-             (ROCA) and should be replaced. On-chip key generation {}  See \
-             YSA-2017-01 <https://www.yubico.com/support/security-advisories/ysa-2017-01/> \
-             for additional information on device replacement and mitigation assistance",
-            yubikey.serial, psz_msg
-        );
-
-        if !setting_roca.value {
-            return Err(Error::NotSupported);
-        }
-    }
-
     match algorithm {
-        YKPIV_ALGO_RSA1024 | YKPIV_ALGO_RSA2048 | YKPIV_ALGO_ECCP256 | YKPIV_ALGO_ECCP384 => (),
-        _ => {
-            error!("invalid algorithm specified");
-            return Err(Error::GenericError);
+        AlgorithmId::Rsa1024 | AlgorithmId::Rsa2048 => {
+            if yubikey.device_model() == DEVTYPE_YK4
+                && yubikey.version.major == 4
+                && (yubikey.version.minor < 3
+                    || yubikey.version.minor == 3 && (yubikey.version.patch < 5))
+            {
+                setting_roca = settings::BoolValue::get(SZ_SETTING_ROCA, true);
+
+                let psz_msg = match setting_roca.source {
+                    settings::Source::User => {
+                        if setting_roca.value {
+                            SZ_ROCA_ALLOW_USER
+                        } else {
+                            SZ_ROCA_BLOCK_USER
+                        }
+                    }
+                    settings::Source::Admin => {
+                        if setting_roca.value {
+                            SZ_ROCA_ALLOW_ADMIN
+                        } else {
+                            SZ_ROCA_BLOCK_ADMIN
+                        }
+                    }
+                    _ => SZ_ROCA_DEFAULT,
+                };
+
+                warn!(
+                    "YubiKey serial number {} is affected by vulnerability CVE-2017-15361 \
+                     (ROCA) and should be replaced. On-chip key generation {}  See \
+                     YSA-2017-01 <https://www.yubico.com/support/security-advisories/ysa-2017-01/> \
+                     for additional information on device replacement and mitigation assistance",
+                    yubikey.serial, psz_msg
+                );
+
+                if !setting_roca.value {
+                    return Err(Error::NotSupported);
+                }
+            }
         }
+        _ => (),
     }
 
     let txn = yubikey.begin_transaction()?;
 
-    templ[3] = slot;
+    templ[3] = slot.into();
 
     let mut offset = 5;
     in_data[..offset].copy_from_slice(&[
@@ -258,7 +468,7 @@ pub fn generate(
         3, // length sans this 2-byte header
         YKPIV_ALGO_TAG,
         1,
-        algorithm,
+        algorithm.into(),
     ]);
 
     if in_data[4] == 0 {
@@ -312,7 +522,7 @@ pub fn generate(
     let data = Buffer::new(response.data().into());
 
     match algorithm {
-        YKPIV_ALGO_RSA1024 | YKPIV_ALGO_RSA2048 => {
+        AlgorithmId::Rsa1024 | AlgorithmId::Rsa2048 => {
             let mut offset = 5;
             let mut len = 0;
 
@@ -340,10 +550,10 @@ pub fn generate(
                 exp,
             })
         }
-        YKPIV_ALGO_ECCP256 | YKPIV_ALGO_ECCP384 => {
+        AlgorithmId::EccP256 | AlgorithmId::EccP384 => {
             let mut offset = 3;
 
-            let len = if algorithm == YKPIV_ALGO_ECCP256 {
+            let len = if let AlgorithmId::EccP256 = algorithm {
                 CB_ECC_POINTP256
             } else {
                 CB_ECC_POINTP384
@@ -367,9 +577,5 @@ pub fn generate(
             let point = data[offset..(offset + len)].to_vec();
             Ok(GeneratedKey::Ecc { algorithm, point })
         }
-        _ => {
-            error!("wrong algorithm");
-            Err(Error::AlgorithmError)
-        }
     }
 }

src/lib.rs

@@ -125,6 +125,7 @@
     html_logo_url = "https://raw.githubusercontent.com/tarcieri/yubikey-piv.rs/develop/img/logo.png",
     html_root_url = "https://docs.rs/yubikey-piv/0.0.2"
 )]
+#![forbid(unsafe_code)]
 #![warn(
     missing_docs,
     rust_2018_idioms,
@@ -166,10 +167,6 @@ pub mod yubikey;
 pub use self::{key::Key, mgm::MgmKey};
 pub use yubikey::YubiKey;
 
-/// Algorithm identifiers
-// TODO(tarcieri): make this an enum
-pub type AlgorithmId = u8;
-
 /// Object identifiers
 pub type ObjectId = u32;
 

src/metadata.rs

@@ -31,48 +31,36 @@
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 use crate::{consts::*, error::Error, serialization::*, transaction::Transaction, Buffer};
-use std::{ptr, slice};
 use zeroize::Zeroizing;
 
 /// Get metadata item
 pub(crate) fn get_item(data: &[u8], tag: u8) -> Result<&[u8], Error> {
-    let mut p_temp: *const u8 = data.as_ptr();
     let mut cb_temp: usize = 0;
-    let mut tag_temp: u8;
+    let mut offset = 0;
 
-    unsafe {
-        while p_temp < data.as_ptr().add(data.len()) {
-            tag_temp = *p_temp;
-            p_temp = p_temp.add(1);
+    while offset < data.len() {
+        let tag_temp = data[offset];
+        offset += 1;
 
-            let p_slice = slice::from_raw_parts(
-                p_temp,
-                data.as_ptr() as usize + data.len() - p_temp as usize,
-            );
-
-            if !has_valid_length(
-                p_slice,
-                data.as_ptr().add(data.len()) as usize - p_temp as usize,
-            ) {
-                return Err(Error::SizeError);
-            }
-
-            p_temp = p_temp.add(get_length(p_slice, &mut cb_temp));
-
-            if tag_temp == tag {
-                // found tag
-                break;
-            }
-
-            p_temp = p_temp.add(cb_temp);
+        if !has_valid_length(&data[offset..], data.len() - 1) {
+            return Err(Error::SizeError);
         }
 
-        if p_temp < data.as_ptr().add(data.len()) {
-            return Ok(slice::from_raw_parts(p_temp, cb_temp));
+        offset += get_length(&data[offset..], &mut cb_temp);
+
+        if tag_temp == tag {
+            // found tag
+            break;
         }
+
+        offset += cb_temp;
     }
 
-    Err(Error::GenericError)
+    if offset < data.len() {
+        Ok(&data[offset..offset + cb_temp])
+    } else {
+        Err(Error::GenericError)
+    }
 }
 
 /// Set metadata item
@@ -83,35 +71,25 @@ pub(crate) fn set_item(
     tag: u8,
     p_item: &[u8],
 ) -> Result<(), Error> {
-    let mut p_temp: *mut u8 = data.as_mut_ptr();
     let mut cb_temp: usize = 0;
     let mut tag_temp: u8 = 0;
     let mut cb_len: usize = 0;
     let cb_item = p_item.len();
-    // Must be signed to have negative offsets
-    let cb_moved: isize;
-    let p_next: *mut u8;
 
-    while p_temp < data[*pcb_data..].as_mut_ptr() {
-        unsafe {
-            tag_temp = *p_temp;
-            p_temp = p_temp.add(1);
+    let mut offset = 0;
 
-            cb_len = get_length(
-                slice::from_raw_parts(
-                    p_temp,
-                    data.as_mut_ptr() as usize + data.len() - p_temp as usize,
-                ),
-                &mut cb_temp,
-            );
-            p_temp = p_temp.add(cb_len);
+    while offset < *pcb_data {
+        tag_temp = data[offset];
+        offset += 1;
 
-            if tag_temp == tag {
-                break;
-            }
+        cb_len = get_length(&data[offset..], &mut cb_temp);
+        offset += cb_len;
 
-            p_temp = p_temp.add(cb_temp);
+        if tag_temp == tag {
+            break;
         }
+
+        offset += cb_temp;
     }
 
     if tag_temp != tag {
@@ -120,75 +98,63 @@ pub(crate) fn set_item(
             return Ok(());
         }
 
-        unsafe {
-            p_temp = data.as_mut_ptr().add(*pcb_data);
-            cb_len = get_length_size(cb_item);
+        // We did not find an existing tag, append
+        offset = *pcb_data;
+        cb_len = get_length_size(cb_item);
 
-            if (*pcb_data + cb_len + cb_item) > cb_data_max {
-                return Err(Error::GenericError);
-            }
-
-            *p_temp = tag;
-            p_temp = p_temp.add(1);
-            p_temp = p_temp.add(set_length(
-                slice::from_raw_parts_mut(
-                    p_temp,
-                    data.as_ptr() as usize + data.len() - p_temp as usize,
-                ),
-                cb_item,
-            ));
-
-            ptr::copy(p_item.as_ptr(), p_temp, cb_item);
+        // If length would cause buffer overflow, return error
+        if (*pcb_data + cb_len + cb_item) > cb_data_max {
+            return Err(Error::GenericError);
         }
 
+        data[offset] = tag;
+        offset += 1;
+        offset += set_length(&mut data[offset..], cb_item);
+        data[offset..offset + cb_item].copy_from_slice(p_item);
+
         *pcb_data += 1 + cb_len + cb_item;
 
         return Ok(());
     }
 
-    if cb_temp == cb_item {
-        unsafe {
-            ptr::copy(p_item.as_ptr(), p_temp, cb_item);
-        }
+    // Found tag
 
+    // Check length, if it matches, overwrite
+    if cb_temp == cb_item {
+        data[offset..offset + cb_item].copy_from_slice(p_item);
         return Ok(());
     }
 
-    p_next = unsafe { p_temp.add(cb_temp) };
-    cb_moved = (cb_item as isize - cb_temp as isize)
+    // Length doesn't match, expand/shrink to fit
+    let next_offset = offset + cb_temp;
+    // Must be signed to have negative offsets
+    let cb_moved: isize = (cb_item as isize - cb_temp as isize)
         + if cb_item != 0 {
             get_length_size(cb_item) as isize
         } else {
+            // For tag, if deleting
             -1
         }
+        // Accounts for different length encoding
         - cb_len as isize;
 
-    if (*pcb_data + cb_moved as usize) > cb_data_max {
+    // If length would cause buffer overflow, return error
+    if (*pcb_data as isize + cb_moved) as usize > cb_data_max {
         return Err(Error::GenericError);
     }
 
-    unsafe {
-        ptr::copy(
-            p_next,
-            p_next.offset(cb_moved),
-            *pcb_data - p_next as usize - data.as_ptr() as usize,
-        );
-    }
-
-    *pcb_data += cb_moved as usize;
+    // Move remaining data
+    data.copy_within(
+        next_offset..*pcb_data,
+        (next_offset as isize + cb_moved) as usize,
+    );
+    *pcb_data = (*pcb_data as isize + cb_moved) as usize;
 
+    // Re-encode item and insert
     if cb_item != 0 {
-        unsafe {
-            p_temp = p_temp.offset(-(cb_len as isize));
-            p_temp = p_temp.add(set_length(
-                slice::from_raw_parts_mut(
-                    p_temp,
-                    data.as_ptr() as usize + data.len() - p_temp as usize,
-                ),
-                cb_item,
-            ));
-            ptr::copy(p_item.as_ptr(), p_temp, cb_item);
-        }
+        offset -= cb_len;
+        offset += set_length(&mut data[offset..], cb_item);
+        data[offset..offset + cb_item].copy_from_slice(p_item);
     }
 
     Ok(())
@@ -219,10 +185,7 @@ pub(crate) fn read(txn: &Transaction<'_>, tag: u8) -> Result<Buffer, Error> {
         return Err(Error::GenericError);
     }
 
-    unsafe {
-        ptr::copy(data.as_ptr().add(offset), data.as_mut_ptr(), pcb_data);
-    }
-
+    data.copy_within(offset..offset + pcb_data, 0);
     data.truncate(pcb_data);
     Ok(data)
 }

src/msroots.rs

@@ -39,7 +39,6 @@
 
 use crate::{consts::*, error::Error, serialization::*, yubikey::YubiKey};
 use log::error;
-use std::{ptr, slice};
 
 /// `msroots` file: PKCS#7-formatted certificate store for enterprise trust roots
 pub struct MsRoots(Vec<u8>);
@@ -51,33 +50,22 @@ impl MsRoots {
     }
 
     /// Read `msroots` file from YubiKey
-    pub fn read(yubikey: &mut YubiKey) -> Result<Vec<Self>, Error> {
-        let mut len: usize = 0;
-        let mut ptr: *mut u8;
-        let mut tag: u8;
-        let mut offset: usize = 0;
-
-        let mut results = vec![];
+    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 p_data = vec![0u8; cb_data];
+        let mut data = Vec::with_capacity(cb_data);
 
         for object_id in YKPIV_OBJ_MSROOTS1..YKPIV_OBJ_MSROOTS5 {
-            let mut buf = txn.fetch_object(object_id)?;
+            let buf = txn.fetch_object(object_id)?;
             let cb_buf = buf.len();
 
-            ptr = buf.as_mut_ptr();
-
             if cb_buf < CB_OBJ_TAG_MIN {
-                return Ok(results);
+                return Ok(None);
             }
 
-            unsafe {
-                tag = *ptr;
-                ptr = ptr.add(1);
-            }
+            let tag = buf[0];
 
             if (TAG_MSROOTS_MID != tag || YKPIV_OBJ_MSROOTS5 == object_id)
                 && (TAG_MSROOTS_END != tag)
@@ -85,38 +73,28 @@ impl MsRoots {
                 // the current object doesn't contain a valid part of a msroots file
 
                 // treat condition as object isn't found
-                return Ok(results);
+                return Ok(None);
             }
 
-            unsafe {
-                ptr = ptr.add(get_length(
-                    slice::from_raw_parts(ptr, buf.as_ptr() as usize + buf.len() - ptr as usize),
-                    &mut len,
-                ));
-            }
+            let mut len: usize = 0;
+            let offset = 1 + get_length(&buf[1..], &mut len);
 
             // check that decoded length represents object contents
-            if len > cb_buf - (ptr as isize - buf.as_mut_ptr() as isize) as usize {
-                return Ok(results);
+            if len > cb_buf - offset {
+                return Ok(None);
             }
 
-            unsafe {
-                ptr::copy(ptr, p_data.as_mut_ptr().add(offset), len);
-            }
-
-            offset += len;
-
-            match MsRoots::new(&p_data[..offset]) {
-                Ok(msroots) => results.push(msroots),
-                Err(res) => error!("error parsing msroots: {:?}", res),
-            }
+            data.extend_from_slice(&buf[offset..offset + len]);
 
             if tag == TAG_MSROOTS_END {
                 break;
             }
         }
 
-        Ok(results)
+        MsRoots::new(&data).map(Some).map_err(|e| {
+            error!("error parsing msroots: {:?}", e);
+            e
+        })
     }
 
     /// Write `msroots` file to YubiKey

src/transaction.rs

@@ -9,6 +9,7 @@ use crate::{
 use crate::{
     apdu::{Response, StatusWords},
     consts::*,
+    key::{AlgorithmId, SlotId},
     mgm::MgmKey,
     serialization::*,
     Buffer, ObjectId,
@@ -16,8 +17,6 @@ use crate::{
 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.
@@ -165,20 +164,28 @@ impl<'tx> Transaction<'tx> {
     /// Verify device PIN.
     #[cfg(feature = "untested")]
     pub fn verify_pin(&self, pin: &[u8]) -> Result<(), Error> {
-        // TODO(tarcieri): allow unpadded (with `0xFF`) PIN shorter than CB_PIN_MAX?
-        if pin.len() != CB_PIN_MAX {
+        if pin.len() > CB_PIN_MAX {
             return Err(Error::SizeError);
         }
 
-        let response = APDU::new(Ins::Verify)
-            .params(0x00, 0x80)
-            .data(pin)
-            .transmit(self, 261)?;
+        let mut query = APDU::new(Ins::Verify);
+        query.params(0x00, 0x80);
+
+        // Empty pin means we are querying the number of retries. We set no data in this
+        // case; if we instead sent [0xff; CB_PIN_MAX] it would count as an attempt and
+        // decrease the retry counter.
+        if !pin.is_empty() {
+            let mut data = Zeroizing::new([0xff; CB_PIN_MAX]);
+            data[0..pin.len()].copy_from_slice(pin);
+            query.data(data.as_ref());
+        }
+
+        let response = query.transmit(self, 261)?;
 
         match response.status_words() {
             StatusWords::Success => Ok(()),
             StatusWords::AuthBlockedError => Err(Error::WrongPin { tries: 0 }),
-            StatusWords::Other(sw) if sw >> 8 == 0x63 => Err(Error::WrongPin { tries: sw & 0xf }),
+            StatusWords::VerifyFailError { tries } => Err(Error::WrongPin { tries }),
             _ => Err(Error::GenericError),
         }
     }
@@ -187,7 +194,6 @@ impl<'tx> Transaction<'tx> {
     #[cfg(feature = "untested")]
     pub fn change_pin(&self, action: i32, current_pin: &[u8], new_pin: &[u8]) -> Result<(), Error> {
         let mut templ = [0, Ins::ChangeReference.code(), 0, 0x80];
-        let mut indata = Zeroizing::new([0u8; 16]);
 
         if current_pin.len() > CB_PIN_MAX || new_pin.len() > CB_PIN_MAX {
             return Err(Error::SizeError);
@@ -199,31 +205,9 @@ impl<'tx> Transaction<'tx> {
             templ[3] = 0x81;
         }
 
-        unsafe {
-            ptr::copy(current_pin.as_ptr(), indata.as_mut_ptr(), current_pin.len());
-
-            if current_pin.len() < CB_PIN_MAX {
-                ptr::write_bytes(
-                    indata.as_mut_ptr().add(current_pin.len()),
-                    0xff,
-                    CB_PIN_MAX - current_pin.len(),
-                );
-            }
-
-            ptr::copy(
-                new_pin.as_ptr(),
-                indata.as_mut_ptr().offset(8),
-                new_pin.len(),
-            );
-
-            if new_pin.len() < CB_PIN_MAX {
-                ptr::write_bytes(
-                    indata.as_mut_ptr().offset(8).add(new_pin.len()),
-                    0xff,
-                    CB_PIN_MAX - new_pin.len(),
-                );
-            }
-        }
+        let mut indata = Zeroizing::new([0xff; CB_PIN_MAX * 2]);
+        indata[0..current_pin.len()].copy_from_slice(current_pin);
+        indata[CB_PIN_MAX..CB_PIN_MAX + new_pin.len()].copy_from_slice(new_pin);
 
         let status_words = self
             .transfer_data(&templ, indata.as_ref(), 0xFF)?
@@ -232,7 +216,7 @@ impl<'tx> Transaction<'tx> {
         match status_words {
             StatusWords::Success => Ok(()),
             StatusWords::AuthBlockedError => Err(Error::PinLocked),
-            StatusWords::Other(sw) if sw >> 8 == 0x63 => Err(Error::WrongPin { tries: sw & 0xf }),
+            StatusWords::VerifyFailError { tries } => Err(Error::WrongPin { tries }),
             _ => {
                 error!(
                     "failed changing pin, token response code: {:x}.",
@@ -283,18 +267,18 @@ impl<'tx> Transaction<'tx> {
     pub(crate) fn authenticated_command(
         &self,
         sign_in: &[u8],
-        algorithm: u8,
-        key: u8,
+        algorithm: AlgorithmId,
+        key: SlotId,
         decipher: bool,
     ) -> Result<Buffer, Error> {
         let in_len = sign_in.len();
         let mut indata = [0u8; 1024];
-        let templ = [0, Ins::Authenticate.code(), algorithm, key];
+        let templ = [0, Ins::Authenticate.code(), algorithm.into(), key.into()];
         let mut len: usize = 0;
 
         match algorithm {
-            YKPIV_ALGO_RSA1024 | YKPIV_ALGO_RSA2048 => {
-                let key_len = if algorithm == YKPIV_ALGO_RSA1024 {
+            AlgorithmId::Rsa1024 | AlgorithmId::Rsa2048 => {
+                let key_len = if let AlgorithmId::Rsa1024 = algorithm {
                     128
                 } else {
                     256
@@ -304,8 +288,8 @@ impl<'tx> Transaction<'tx> {
                     return Err(Error::SizeError);
                 }
             }
-            YKPIV_ALGO_ECCP256 | YKPIV_ALGO_ECCP384 => {
-                let key_len = if algorithm == YKPIV_ALGO_ECCP256 {
+            AlgorithmId::EccP256 | AlgorithmId::EccP384 => {
+                let key_len = if let AlgorithmId::EccP256 = algorithm {
                     32
                 } else {
                     48
@@ -316,7 +300,6 @@ impl<'tx> Transaction<'tx> {
                     return Err(Error::SizeError);
                 }
             }
-            _ => return Err(Error::AlgorithmError),
         }
 
         let bytes = if in_len < 0x80 {
@@ -331,12 +314,10 @@ impl<'tx> Transaction<'tx> {
         let mut offset = 1 + set_length(&mut indata[1..], in_len + bytes + 3);
         indata[offset] = 0x82;
         indata[offset + 1] = 0x00;
-        indata[offset + 2] =
-            if (algorithm == YKPIV_ALGO_ECCP256 || algorithm == YKPIV_ALGO_ECCP384) && decipher {
-                0x85
-            } else {
-                0x81
-            };
+        indata[offset + 2] = match (algorithm, decipher) {
+            (AlgorithmId::EccP256, true) | (AlgorithmId::EccP384, true) => 0x85,
+            _ => 0x81,
+        };
 
         offset += 3;
         offset += set_length(&mut indata[offset..], in_len);

src/yubikey.rs

@@ -36,22 +36,23 @@
 #[cfg(feature = "untested")]
 use crate::{
     apdu::{Ins, StatusWords, APDU},
-    key::SlotId,
+    key::{AlgorithmId, SlotId},
     metadata,
     mgm::MgmKey,
     serialization::*,
-    ObjectId,
+    Buffer, ObjectId,
 };
-use crate::{consts::*, error::Error, transaction::Transaction, Buffer};
+use crate::{consts::*, error::Error, transaction::Transaction};
 #[cfg(feature = "untested")]
 use getrandom::getrandom;
 use log::{error, info, warn};
 use pcsc::{Card, Context};
+#[cfg(feature = "untested")]
+use secrecy::ExposeSecret;
 use std::fmt::{self, Display};
 #[cfg(feature = "untested")]
 use std::{
     convert::TryInto,
-    ptr, slice,
     time::{SystemTime, UNIX_EPOCH},
 };
 #[cfg(feature = "untested")]
@@ -64,6 +65,9 @@ pub const AID: [u8; 5] = [0xa0, 0x00, 0x00, 0x03, 0x08];
 /// <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>;
+
 /// YubiKey Serial Number
 #[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord)]
 pub struct Serial(pub u32);
@@ -119,7 +123,7 @@ impl Version {
 #[cfg_attr(not(feature = "untested"), allow(dead_code))]
 pub struct YubiKey {
     pub(crate) card: Card,
-    pub(crate) pin: Option<Buffer>,
+    pub(crate) pin: Option<CachedPin>,
     pub(crate) is_neo: bool,
     pub(crate) version: Version,
     pub(crate) serial: Serial,
@@ -229,8 +233,10 @@ impl YubiKey {
             pcsc::Disposition::ResetCard,
         )?;
 
-        // TODO(tarcieri): zeroize pin!
-        let pin = self.pin.clone();
+        let pin = self
+            .pin
+            .as_ref()
+            .map(|p| Buffer::new(p.expose_secret().clone()));
 
         let txn = Transaction::new(&mut self.card)?;
         txn.select_application()?;
@@ -357,7 +363,7 @@ impl YubiKey {
     pub fn sign_data(
         &mut self,
         raw_in: &[u8],
-        algorithm: u8,
+        algorithm: AlgorithmId,
         key: SlotId,
     ) -> Result<Buffer, Error> {
         let txn = self.begin_transaction()?;
@@ -371,7 +377,7 @@ impl YubiKey {
     pub fn decrypt_data(
         &mut self,
         input: &[u8],
-        algorithm: u8,
+        algorithm: AlgorithmId,
         key: SlotId,
     ) -> Result<Buffer, Error> {
         let txn = self.begin_transaction()?;
@@ -389,7 +395,7 @@ impl YubiKey {
         }
 
         if !pin.is_empty() {
-            self.pin = Some(Buffer::new(pin.into()))
+            self.pin = Some(CachedPin::new(pin.into()))
         }
 
         Ok(())
@@ -397,7 +403,7 @@ impl YubiKey {
 
     /// Get the number of PIN retries
     #[cfg(feature = "untested")]
-    pub fn get_pin_retries(&mut self) -> Result<u32, Error> {
+    pub fn get_pin_retries(&mut self) -> Result<u8, Error> {
         let txn = self.begin_transaction()?;
 
         // Force a re-select to unverify, because once verified the spec dictates that
@@ -415,24 +421,15 @@ impl YubiKey {
 
     /// Set the number of PIN retries
     #[cfg(feature = "untested")]
-    pub fn set_pin_retries(&mut self, pin_tries: usize, puk_tries: usize) -> Result<(), Error> {
+    pub fn set_pin_retries(&mut self, pin_tries: u8, puk_tries: u8) -> Result<(), Error> {
         // Special case: if either retry count is 0, it's a successful no-op
         if pin_tries == 0 || puk_tries == 0 {
             return Ok(());
         }
 
-        if pin_tries > 0xff || puk_tries > 0xff || pin_tries < 1 || puk_tries < 1 {
-            return Err(Error::RangeError);
-        }
-
         let txn = self.begin_transaction()?;
 
-        let templ = [
-            0,
-            Ins::SetPinRetries.code(),
-            pin_tries as u8,
-            puk_tries as u8,
-        ];
+        let templ = [0, Ins::SetPinRetries.code(), pin_tries, puk_tries];
 
         let status_words = txn.transfer_data(&templ, &[], 255)?.status_words();
 
@@ -455,7 +452,7 @@ impl YubiKey {
         }
 
         if !new_pin.is_empty() {
-            self.pin = Some(Buffer::new(new_pin.into()));
+            self.pin = Some(CachedPin::new(new_pin.into()));
         }
 
         Ok(())
@@ -608,7 +605,7 @@ impl YubiKey {
     pub fn import_private_key(
         &mut self,
         key: SlotId,
-        algorithm: u8,
+        algorithm: AlgorithmId,
         p: Option<&[u8]>,
         q: Option<&[u8]>,
         dp: Option<&[u8]>,
@@ -618,177 +615,106 @@ impl YubiKey {
         pin_policy: u8,
         touch_policy: u8,
     ) -> Result<(), Error> {
-        // TODO(tarcieri): get rid of legacy pointers
-        let (p, p_len) = match p {
-            Some(slice) => (slice.as_ptr(), slice.len()),
-            None => (ptr::null(), 0),
-        };
-
-        let (q, q_len) = match q {
-            Some(slice) => (slice.as_ptr(), slice.len()),
-            None => (ptr::null(), 0),
-        };
-
-        let (dp, dp_len) = match dp {
-            Some(slice) => (slice.as_ptr(), slice.len()),
-            None => (ptr::null(), 0),
-        };
-
-        let (dq, dq_len) = match dq {
-            Some(slice) => (slice.as_ptr(), slice.len()),
-            None => (ptr::null(), 0),
-        };
-
-        let (qinv, qinv_len) = match qinv {
-            Some(slice) => (slice.as_ptr(), slice.len()),
-            None => (ptr::null(), 0),
-        };
-
-        let (ec_data, ec_data_len) = match ec_data {
-            Some(slice) => (slice.as_ptr(), slice.len()),
-            None => (ptr::null(), 0),
-        };
-
         let mut key_data = Zeroizing::new(vec![0u8; 1024]);
-        let mut in_ptr: *mut u8 = key_data.as_mut_ptr();
-        let templ = [0, Ins::ImportKey.code(), algorithm, key];
-        let mut elem_len: u32 = 0;
-        let mut params: [*const u8; 5] = [ptr::null(); 5];
-        let mut lens = [0usize; 5];
-        let n_params: u8;
-        let param_tag: i32;
-
-        if key == YKPIV_KEY_CARDMGM
-            || key < YKPIV_KEY_RETIRED1
-            || key > YKPIV_KEY_RETIRED20 && (key < YKPIV_KEY_AUTHENTICATION)
-            || key > YKPIV_KEY_CARDAUTH && (key != YKPIV_KEY_ATTESTATION)
-        {
-            return Err(Error::KeyError);
-        }
+        let templ = [0, Ins::ImportKey.code(), algorithm.into(), key.into()];
 
         if pin_policy != YKPIV_PINPOLICY_DEFAULT
-            && (pin_policy != YKPIV_PINPOLICY_NEVER)
-            && (pin_policy != YKPIV_PINPOLICY_ONCE)
-            && (pin_policy != YKPIV_PINPOLICY_ALWAYS)
+            && pin_policy != YKPIV_PINPOLICY_NEVER
+            && pin_policy != YKPIV_PINPOLICY_ONCE
+            && pin_policy != YKPIV_PINPOLICY_ALWAYS
         {
             return Err(Error::GenericError);
         }
 
         if touch_policy != YKPIV_TOUCHPOLICY_DEFAULT
-            && (touch_policy != YKPIV_TOUCHPOLICY_NEVER)
-            && (touch_policy != YKPIV_TOUCHPOLICY_ALWAYS)
-            && (touch_policy != YKPIV_TOUCHPOLICY_CACHED)
+            && touch_policy != YKPIV_TOUCHPOLICY_NEVER
+            && touch_policy != YKPIV_TOUCHPOLICY_ALWAYS
+            && touch_policy != YKPIV_TOUCHPOLICY_CACHED
         {
             return Err(Error::GenericError);
         }
 
-        match algorithm {
-            YKPIV_ALGO_RSA1024 | YKPIV_ALGO_RSA2048 => {
-                if p_len + q_len + dp_len + dq_len + qinv_len >= 1024 {
-                    return Err(Error::SizeError);
-                } else {
-                    if algorithm == YKPIV_ALGO_RSA1024 {
-                        elem_len = 64;
+        let (elem_len, params, param_tag) = match algorithm {
+            AlgorithmId::Rsa1024 | AlgorithmId::Rsa2048 => match (p, q, dp, dq, qinv) {
+                (Some(p), Some(q), Some(dp), Some(dq), Some(qinv)) => {
+                    if p.len() + q.len() + dp.len() + dq.len() + qinv.len() >= key_data.len() {
+                        return Err(Error::SizeError);
                     }
 
-                    if algorithm == YKPIV_ALGO_RSA2048 {
-                        elem_len = 128;
+                    (
+                        match algorithm {
+                            AlgorithmId::Rsa1024 => 64,
+                            AlgorithmId::Rsa2048 => 128,
+                            _ => unreachable!(),
+                        },
+                        vec![p, q, dp, dq, qinv],
+                        0x01,
+                    )
+                }
+                _ => return Err(Error::GenericError),
+            },
+            AlgorithmId::EccP256 | AlgorithmId::EccP384 => match ec_data {
+                Some(ec_data) => {
+                    if ec_data.len() >= key_data.len() {
+                        // This can never be true, but check to be explicit.
+                        return Err(Error::SizeError);
                     }
 
-                    if p.is_null() || q.is_null() || dp.is_null() || dq.is_null() || qinv.is_null()
-                    {
-                        return Err(Error::GenericError);
-                    }
-
-                    params[0] = p;
-                    lens[0] = p_len;
-                    params[1] = q;
-                    lens[1] = q_len;
-                    params[2] = dp;
-                    lens[2] = dp_len;
-                    params[3] = dq;
-                    lens[3] = dq_len;
-                    params[4] = qinv;
-                    lens[4] = qinv_len;
-                    param_tag = 0x1;
-                    n_params = 5u8;
-                }
-            }
-            YKPIV_ALGO_ECCP256 | YKPIV_ALGO_ECCP384 => {
-                if ec_data_len >= key_data.len() {
-                    return Err(Error::SizeError);
+                    (
+                        match algorithm {
+                            AlgorithmId::EccP256 => 32,
+                            AlgorithmId::EccP384 => 48,
+                            _ => unreachable!(),
+                        },
+                        vec![ec_data],
+                        0x06,
+                    )
                 }
+                _ => return Err(Error::GenericError),
+            },
+        };
 
-                if algorithm == YKPIV_ALGO_ECCP256 {
-                    elem_len = 32;
-                } else if algorithm == YKPIV_ALGO_ECCP384 {
-                    elem_len = 48;
-                }
+        let mut offset = 0;
 
-                if ec_data.is_null() {
-                    return Err(Error::GenericError);
-                }
+        for (i, param) in params.into_iter().enumerate() {
+            key_data[offset] = param_tag + i as u8;
+            offset += 1;
 
-                params[0] = ec_data;
-                lens[0] = ec_data_len;
-                param_tag = 0x6;
-                n_params = 1;
-            }
-            _ => return Err(Error::AlgorithmError),
-        }
+            offset += set_length(&mut key_data[offset..], elem_len);
 
-        for i in 0..n_params {
-            unsafe {
-                *in_ptr = (param_tag + i as i32) as u8;
-                in_ptr = in_ptr.offset(1);
-
-                in_ptr = in_ptr.add(set_length(
-                    slice::from_raw_parts_mut(
-                        in_ptr,
-                        key_data.as_mut_ptr() as usize - in_ptr as usize,
-                    ),
-                    elem_len as usize,
-                ));
-            }
-
-            let padding = elem_len as usize - lens[i as usize];
-            let remaining = (key_data.as_mut_ptr() as usize) + 1024 - in_ptr as usize;
+            let padding = elem_len - param.len();
+            let remaining = key_data.len() - offset;
 
             if padding > remaining {
                 return Err(Error::AlgorithmError);
             }
 
-            unsafe {
-                ptr::write_bytes(in_ptr, 0, padding);
-                in_ptr = in_ptr.add(padding);
-                ptr::copy(params[i as usize], in_ptr, lens[i as usize]);
-                in_ptr = in_ptr.add(lens[i as usize]);
+            for b in &mut key_data[offset..offset + padding] {
+                *b = 0;
             }
+            offset += padding;
+            key_data[offset..offset + param.len()].copy_from_slice(param);
+            offset += param.len();
         }
 
         if pin_policy != YKPIV_PINPOLICY_DEFAULT {
-            unsafe {
-                *in_ptr = YKPIV_PINPOLICY_TAG;
-                *in_ptr.add(1) = 0x01;
-                *in_ptr.add(2) = pin_policy;
-                in_ptr = in_ptr.add(3);
-            }
+            key_data[offset] = YKPIV_PINPOLICY_TAG;
+            key_data[offset + 1] = 0x01;
+            key_data[offset + 2] = pin_policy;
+            offset += 3;
         }
 
         if touch_policy != YKPIV_TOUCHPOLICY_DEFAULT {
-            unsafe {
-                *in_ptr = YKPIV_TOUCHPOLICY_TAG;
-                *in_ptr.add(1) = 0x01;
-                *in_ptr.add(2) = touch_policy;
-                in_ptr = in_ptr.add(3);
-            }
+            key_data[offset] = YKPIV_TOUCHPOLICY_TAG;
+            key_data[offset + 1] = 0x01;
+            key_data[offset + 2] = touch_policy;
+            offset += 3;
         }
 
         let txn = self.begin_transaction()?;
-        let len = in_ptr as usize - key_data.as_mut_ptr() as usize;
 
         let status_words = txn
-            .transfer_data(&templ, &key_data[..len], 256)?
+            .transfer_data(&templ, &key_data[..offset], 256)?
             .status_words();
 
         match status_words {
@@ -802,7 +728,7 @@ impl YubiKey {
     /// <https://developers.yubico.com/PIV/Introduction/PIV_attestation.html>
     #[cfg(feature = "untested")]
     pub fn attest(&mut self, key: SlotId) -> Result<Buffer, Error> {
-        let templ = [0, Ins::Attest.code(), key, 0];
+        let templ = [0, Ins::Attest.code(), key.into(), 0];
         let txn = self.begin_transaction()?;
         let response = txn.transfer_data(&templ, &[], CB_OBJ_MAX)?;