From 5f8cb116a5f50dca899c3e2e200c9b3d07b96961 Mon Sep 17 00:00:00 2001
From: Julien Cretin <cretin@google.com>
Date: Tue, 2 Jun 2020 11:00:26 +0200
Subject: [PATCH] Only write attestation if compiled with batch attestation

---
 src/ctap/mod.rs     | 28 +++++++++++--------
 src/ctap/storage.rs | 65 +++++++++++++++++++++++----------------------
 2 files changed, 50 insertions(+), 43 deletions(-)

diff --git a/src/ctap/mod.rs b/src/ctap/mod.rs
index 758f94a..bd1db51 100644
--- a/src/ctap/mod.rs
+++ b/src/ctap/mod.rs
@@ -531,19 +531,25 @@ where
 
         let mut signature_data = auth_data.clone();
         signature_data.extend(client_data_hash);
-        let (signature, x5c) = if USE_BATCH_ATTESTATION {
-            let attestation_key =
-                crypto::ecdsa::SecKey::from_bytes(self.persistent_store.attestation_private_key()?)
-                    .unwrap();
-            (
-                attestation_key.sign_rfc6979::<crypto::sha256::Sha256>(&signature_data),
-                Some(vec![self.persistent_store.attestation_certificate()?]),
-            )
-        } else {
-            (
+        // We currently use the presence of the attestation private key in the persistent storage to
+        // decide whether batch attestation is needed.
+        let (signature, x5c) = match self.persistent_store.attestation_private_key()? {
+            Some(attestation_private_key) => {
+                let attestation_key =
+                    crypto::ecdsa::SecKey::from_bytes(attestation_private_key).unwrap();
+                let attestation_certificate = self
+                    .persistent_store
+                    .attestation_certificate()?
+                    .ok_or(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR)?;
+                (
+                    attestation_key.sign_rfc6979::<crypto::sha256::Sha256>(&signature_data),
+                    Some(vec![attestation_certificate]),
+                )
+            }
+            None => (
                 sk.sign_rfc6979::<crypto::sha256::Sha256>(&signature_data),
                 None,
-            )
+            ),
         };
         let attestation_statement = PackedAttestationStatement {
             alg: SignatureAlgorithm::ES256 as i64,
diff --git a/src/ctap/storage.rs b/src/ctap/storage.rs
index b8cfa37..e3fef55 100644
--- a/src/ctap/storage.rs
+++ b/src/ctap/storage.rs
@@ -14,9 +14,8 @@
 
 use crate::crypto::rng256::Rng256;
 use crate::ctap::data_formats::PublicKeyCredentialSource;
-use crate::ctap::key_material;
 use crate::ctap::status_code::Ctap2StatusCode;
-use crate::ctap::PIN_AUTH_LENGTH;
+use crate::ctap::{key_material, PIN_AUTH_LENGTH, USE_BATCH_ATTESTATION};
 use alloc::string::String;
 use alloc::vec::Vec;
 use core::convert::TryInto;
@@ -223,23 +222,26 @@ impl PersistentStore {
                 })
                 .unwrap();
         }
-        if self.store.find_one(&Key::AttestationPrivateKey).is_none() {
-            self.store
-                .insert(StoreEntry {
-                    tag: ATTESTATION_PRIVATE_KEY,
-                    data: key_material::ATTESTATION_PRIVATE_KEY,
-                    sensitive: false,
-                })
-                .unwrap();
-        }
-        if self.store.find_one(&Key::AttestationCertificate).is_none() {
-            self.store
-                .insert(StoreEntry {
-                    tag: ATTESTATION_CERTIFICATE,
-                    data: key_material::ATTESTATION_CERTIFICATE,
-                    sensitive: false,
-                })
-                .unwrap();
+        // The following 3 entries are meant to be written by vendor-specific commands.
+        if USE_BATCH_ATTESTATION {
+            if self.store.find_one(&Key::AttestationPrivateKey).is_none() {
+                self.store
+                    .insert(StoreEntry {
+                        tag: ATTESTATION_PRIVATE_KEY,
+                        data: key_material::ATTESTATION_PRIVATE_KEY,
+                        sensitive: false,
+                    })
+                    .unwrap();
+            }
+            if self.store.find_one(&Key::AttestationCertificate).is_none() {
+                self.store
+                    .insert(StoreEntry {
+                        tag: ATTESTATION_CERTIFICATE,
+                        data: key_material::ATTESTATION_CERTIFICATE,
+                        sensitive: false,
+                    })
+                    .unwrap();
+            }
         }
         if self.store.find_one(&Key::Aaguid).is_none() {
             self.store
@@ -435,24 +437,23 @@ impl PersistentStore {
 
     pub fn attestation_private_key(
         &self,
-    ) -> Result<&[u8; ATTESTATION_PRIVATE_KEY_LENGTH], Ctap2StatusCode> {
-        let (_, entry) = self
-            .store
-            .find_one(&Key::AttestationPrivateKey)
-            .ok_or(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR)?;
-        let data = entry.data;
+    ) -> Result<Option<&[u8; ATTESTATION_PRIVATE_KEY_LENGTH]>, Ctap2StatusCode> {
+        let data = match self.store.find_one(&Key::AttestationPrivateKey) {
+            None => return Ok(None),
+            Some((_, entry)) => entry.data,
+        };
         if data.len() != ATTESTATION_PRIVATE_KEY_LENGTH {
             return Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR);
         }
-        Ok(array_ref!(data, 0, ATTESTATION_PRIVATE_KEY_LENGTH))
+        Ok(Some(array_ref!(data, 0, ATTESTATION_PRIVATE_KEY_LENGTH)))
     }
 
-    pub fn attestation_certificate(&self) -> Result<Vec<u8>, Ctap2StatusCode> {
-        let (_, entry) = self
-            .store
-            .find_one(&Key::AttestationCertificate)
-            .ok_or(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR)?;
-        Ok(entry.data.to_vec())
+    pub fn attestation_certificate(&self) -> Result<Option<Vec<u8>>, Ctap2StatusCode> {
+        let data = match self.store.find_one(&Key::AttestationCertificate) {
+            None => return Ok(None),
+            Some((_, entry)) => entry.data,
+        };
+        Ok(Some(data.to_vec()))
     }
 
     pub fn aaguid(&self) -> Result<&[u8; AAGUID_LENGTH], Ctap2StatusCode> {