Merge pull request #75 from str4d/cert-gen-prep

Preparatory work for certificate generation

src/certificate.rs

@@ -44,6 +44,7 @@ use elliptic_curve::weierstrass::{
 };
 use log::error;
 use rsa::{PublicKey, RSAPublicKey};
+use std::convert::TryFrom;
 use std::fmt;
 use x509_parser::{parse_x509_der, x509::SubjectPublicKeyInfo};
 use zeroize::Zeroizing;
@@ -57,17 +58,43 @@ 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";
 
-#[allow(dead_code)]
-const CERTINFO_UNCOMPRESSED: u8 = 0;
-#[cfg(feature = "untested")]
-const CERTINFO_GZIP: u8 = 1;
-
 const TAG_CERT: u8 = 0x70;
 #[cfg(feature = "untested")]
 const TAG_CERT_COMPRESS: u8 = 0x71;
 #[cfg(feature = "untested")]
 const TAG_CERT_LRC: u8 = 0xFE;
 
+/// Information about how a [`Certificate`] is stored within a YubiKey.
+#[derive(Clone, Copy, Debug, PartialEq)]
+pub enum CertInfo {
+    /// The certificate is uncompressed.
+    Uncompressed,
+
+    /// The certificate is gzip-compressed.
+    Gzip,
+}
+
+impl TryFrom<u8> for CertInfo {
+    type Error = Error;
+
+    fn try_from(value: u8) -> Result<Self, Self::Error> {
+        match value {
+            0x00 => Ok(CertInfo::Uncompressed),
+            0x01 => Ok(CertInfo::Gzip),
+            _ => Err(Error::InvalidObject),
+        }
+    }
+}
+
+impl From<CertInfo> for u8 {
+    fn from(certinfo: CertInfo) -> u8 {
+        match certinfo {
+            CertInfo::Uncompressed => 0x00,
+            CertInfo::Gzip => 0x01,
+        }
+    }
+}
+
 /// Information about a public key within a [`Certificate`].
 #[derive(Clone, Eq, PartialEq)]
 pub enum PublicKeyInfo {
@@ -142,6 +169,14 @@ pub struct Certificate {
     data: Buffer,
 }
 
+impl<'a> TryFrom<&'a [u8]> for Certificate {
+    type Error = Error;
+
+    fn try_from(bytes: &'a [u8]) -> Result<Self, Error> {
+        Self::from_bytes(bytes.to_vec())
+    }
+}
+
 impl Certificate {
     /// Read a certificate from the given slot in the YubiKey
     pub fn read(yubikey: &mut YubiKey, slot: SlotId) -> Result<Self, Error> {
@@ -152,12 +187,17 @@ impl Certificate {
             return Err(Error::InvalidObject);
         }
 
-        Certificate::new(buf)
+        Certificate::from_bytes(buf)
     }
 
     /// 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> {
+    pub fn write(
+        &self,
+        yubikey: &mut YubiKey,
+        slot: SlotId,
+        certinfo: CertInfo,
+    ) -> Result<(), Error> {
         let txn = yubikey.begin_transaction()?;
         write_certificate(&txn, slot, Some(&self.data), certinfo)
     }
@@ -166,11 +206,11 @@ impl Certificate {
     #[cfg(feature = "untested")]
     pub fn delete(yubikey: &mut YubiKey, slot: SlotId) -> Result<(), Error> {
         let txn = yubikey.begin_transaction()?;
-        write_certificate(&txn, slot, None, 0)
+        write_certificate(&txn, slot, None, CertInfo::Uncompressed)
     }
 
     /// Initialize a local certificate struct from the given bytebuffer
-    pub fn new(cert: impl Into<Buffer>) -> Result<Self, Error> {
+    pub fn from_bytes(cert: impl Into<Buffer>) -> Result<Self, Error> {
         let cert = cert.into();
 
         if cert.is_empty() {
@@ -244,7 +284,7 @@ pub(crate) fn write_certificate(
     txn: &Transaction<'_>,
     slot: SlotId,
     data: Option<&[u8]>,
-    certinfo: u8,
+    certinfo: CertInfo,
 ) -> Result<(), Error> {
     let object_id = slot.object_id();
 
@@ -258,15 +298,7 @@ pub(crate) fn write_certificate(
     let mut offset = Tlv::write(&mut buf, TAG_CERT, data)?;
 
     // write compression info and LRC trailer
-    offset += Tlv::write(
+    offset += Tlv::write(&mut buf, TAG_CERT_COMPRESS, &[certinfo.into()])?;
-        &mut buf,
-        TAG_CERT_COMPRESS,
-        if certinfo == CERTINFO_GZIP {
-            &[0x01]
-        } else {
-            &[0x00]
-        },
-    )?;
     offset += Tlv::write(&mut buf, TAG_CERT_LRC, &[])?;
 
     txn.save_object(object_id, &buf[..offset])

src/key.rs

@@ -49,13 +49,18 @@ use std::convert::TryFrom;
 #[cfg(feature = "untested")]
 use crate::{
     apdu::{Ins, StatusWords},
+    certificate::PublicKeyInfo,
     policy::{PinPolicy, TouchPolicy},
     serialization::*,
     settings, Buffer, CB_OBJ_MAX,
 };
 #[cfg(feature = "untested")]
+use elliptic_curve::weierstrass::PublicKey as EcPublicKey;
+#[cfg(feature = "untested")]
 use log::{error, warn};
 #[cfg(feature = "untested")]
+use rsa::{BigUint, RSAPublicKey};
+#[cfg(feature = "untested")]
 use zeroize::Zeroizing;
 
 #[cfg(feature = "untested")]
@@ -410,7 +415,7 @@ impl Key {
             };
 
             if !buf.is_empty() {
-                let cert = Certificate::new(buf)?;
+                let cert = Certificate::from_bytes(buf)?;
                 keys.push(Key { slot, cert });
             }
         }
@@ -429,43 +434,6 @@ impl Key {
     }
 }
 
-/// Information about a generated key
-// TODO(tarcieri): this could use some more work
-#[cfg(feature = "untested")]
-#[derive(Clone, Debug, Eq, PartialEq)]
-pub enum GeneratedKey {
-    /// RSA keys
-    Rsa {
-        /// RSA algorithm
-        algorithm: AlgorithmId,
-
-        /// Modulus
-        modulus: Vec<u8>,
-
-        /// Exponent
-        exp: Vec<u8>,
-    },
-    /// ECC keys
-    Ecc {
-        /// ECC algorithm
-        algorithm: AlgorithmId,
-
-        /// Public curve point (i.e. public key)
-        point: Vec<u8>,
-    },
-}
-
-#[cfg(feature = "untested")]
-impl GeneratedKey {
-    /// Get the algorithm
-    pub fn algorithm(&self) -> AlgorithmId {
-        *match self {
-            GeneratedKey::Rsa { algorithm, .. } => algorithm,
-            GeneratedKey::Ecc { algorithm, .. } => algorithm,
-        }
-    }
-}
-
 /// Generate key
 #[cfg(feature = "untested")]
 #[allow(clippy::cognitive_complexity)]
@@ -475,7 +443,7 @@ pub fn generate(
     algorithm: AlgorithmId,
     pin_policy: PinPolicy,
     touch_policy: TouchPolicy,
-) -> Result<GeneratedKey, Error> {
+) -> Result<PublicKeyInfo, Error> {
     // Keygen messages
     // TODO(tarcieri): extract these into an I18N-handling type?
     const SZ_SETTING_ROCA: &str = "Enable_Unsafe_Keygen_ROCA";
@@ -580,9 +548,41 @@ pub fn generate(
         }
     }
 
+    // TODO(str4d): Response is wrapped in an ASN.1 TLV:
+    //
+    //    0x7f 0x49 -> Application | Constructed | 0x49
     match algorithm {
         AlgorithmId::Rsa1024 | AlgorithmId::Rsa2048 => {
-            let (data, modulus_tlv) = Tlv::parse(response.data())?;
+            // It appears that the inner application-specific value returned by the
+            // YubiKey is constructed such that RSA pubkeys can be parsed in two ways:
+            //
+            // - Use a full ASN.1 parser on the entire datastructure:
+            //
+            //     RSA 1024:
+            //     [127, 73, 129, 136, 129, 129, 128, [ 128 octets ], 130,  3, 1, 0, 1]
+            //     | tag   | len:136 |0x81| len:128 |    modulus    |0x82|len3|  exp  |
+            //
+            //     RSA 2048:
+            //     [127, 73, 130, 1, 9, 129, 130, 1, 0, [ 256 octets ], 130,  3, 1, 0, 1]
+            //     | tag   | len:265  |0x81| len:256  |    modulus    |0x82|len3|  exp  |
+            //
+            // - Skip the first 5 bytes and use crate::serialize::get_length during TLV
+            //   parsing (which treats 128 as a single-byte definite length instead of an
+            //   indefinite length):
+            //
+            //     RSA 1024:
+            //     [127, 73, 129, 136, 129, 129, 128, [ 128 octets ], 130,  3, 1, 0, 1]
+            //     |                      |0x81|len128|   modulus   |0x82|len3|  exp  |
+            //
+            //     RSA 2048:
+            //     [127, 73, 130, 1, 9, 129, 130, 1, 0, [ 256 octets ], 130,  3, 1, 0, 1]
+            //     |                  |0x81| len:256  |    modulus    |0x82|len3|  exp  |
+            //
+            // Because of the above, treat this for now as a 2-byte ASN.1 tag with a
+            // 3-byte length.
+            let data = &response.data()[5..];
+
+            let (data, modulus_tlv) = Tlv::parse(data)?;
             if modulus_tlv.tag != TAG_RSA_MODULUS {
                 error!("Failed to parse public key structure (modulus)");
                 return Err(Error::ParseError);
@@ -596,20 +596,27 @@ pub fn generate(
             }
             let exp = exp_tlv.value.to_vec();
 
-            Ok(GeneratedKey::Rsa {
+            Ok(PublicKeyInfo::Rsa {
                 algorithm,
-                modulus,
+                pubkey: RSAPublicKey::new(
-                exp,
+                    BigUint::from_bytes_be(&modulus),
+                    BigUint::from_bytes_be(&exp),
+                )
+                .map_err(|_| Error::InvalidObject)?,
             })
         }
         AlgorithmId::EccP256 | AlgorithmId::EccP384 => {
+            // 2-byte ASN.1 tag, 1-byte length (because all supported EC pubkey lengths
+            // are shorter than 128 bytes, fitting into a definite short ASN.1 length).
+            let data = &response.data()[3..];
+
             let len = if let AlgorithmId::EccP256 = algorithm {
                 CB_ECC_POINTP256
             } else {
                 CB_ECC_POINTP384
             };
 
-            let (_, tlv) = Tlv::parse(response.data())?;
+            let (_, tlv) = Tlv::parse(data)?;
 
             if tlv.tag != TAG_ECC_POINT {
                 error!("failed to parse public key structure");
@@ -623,7 +630,13 @@ pub fn generate(
             }
 
             let point = tlv.value.to_vec();
-            Ok(GeneratedKey::Ecc { algorithm, point })
+
+            if let AlgorithmId::EccP256 = algorithm {
+                EcPublicKey::from_bytes(point).map(PublicKeyInfo::EcP256)
+            } else {
+                EcPublicKey::from_bytes(point).map(PublicKeyInfo::EcP384)
+            }
+            .ok_or(Error::InvalidObject)
         }
     }
 }

src/serialization.rs

@@ -156,6 +156,8 @@ pub(crate) fn set_length(buffer: &mut [u8], length: usize) -> Result<usize, Erro
 /// Parse length tag, returning the size of the length tag itself as the
 /// returned value, and setting the len parameter to the parsed length.
 pub(crate) fn get_length(buffer: &[u8], len: &mut usize) -> usize {
+    // This is not valid ASN.1 (0x80 is the indefinite length marker).
+    // See comment in key::generate for more context.
     if buffer[0] < 0x81 {
         *len = buffer[0] as usize;
         1