AAGUID customization (#600)

* Moves the AAGUID to Customization

* Removes the AAGUID from storage

The commit is optional on top of the Customization move. I didn't see
the point in storing the AAGUID in persistent storage anymore, so I
removed it.

src/api/customization.rs

@@ -21,7 +21,12 @@ use crate::ctap::data_formats::{CredentialProtectionPolicy, EnterpriseAttestatio
 use alloc::string::String;
 use alloc::vec::Vec;
 
+pub const AAGUID_LENGTH: usize = 16;
+
 pub trait Customization {
+    /// Authenticator Attestation Global Unique Identifier
+    fn aaguid(&self) -> &'static [u8; AAGUID_LENGTH];
+
     // ###########################################################################
     // Constants for adjusting privacy and protection levels.
     // ###########################################################################
@@ -251,6 +256,7 @@ pub trait Customization {
 
 #[derive(Clone)]
 pub struct CustomizationImpl {
+    pub aaguid: &'static [u8; AAGUID_LENGTH],
     pub allows_pin_protocol_v1: bool,
     pub default_cred_protect: Option<CredentialProtectionPolicy>,
     pub default_min_pin_length: u8,
@@ -270,6 +276,7 @@ pub struct CustomizationImpl {
 }
 
 pub const DEFAULT_CUSTOMIZATION: CustomizationImpl = CustomizationImpl {
+    aaguid: &[0; AAGUID_LENGTH],
     allows_pin_protocol_v1: true,
     default_cred_protect: None,
     default_min_pin_length: 4,
@@ -289,6 +296,10 @@ pub const DEFAULT_CUSTOMIZATION: CustomizationImpl = CustomizationImpl {
 };
 
 impl Customization for CustomizationImpl {
+    fn aaguid(&self) -> &'static [u8; AAGUID_LENGTH] {
+        self.aaguid
+    }
+
     fn allows_pin_protocol_v1(&self) -> bool {
         self.allows_pin_protocol_v1
     }

src/ctap/command.rs

@@ -1,4 +1,4 @@
-// Copyright 2019-2021 Google LLC
+// Copyright 2019-2023 Google LLC
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -12,6 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+use super::cbor_read;
 use super::data_formats::{
     extract_array, extract_bool, extract_byte_string, extract_map, extract_text_string,
     extract_unsigned, ok_or_missing, ClientPinSubCommand, ConfigSubCommand, ConfigSubCommandParams,
@@ -21,7 +22,6 @@ use super::data_formats::{
     PublicKeyCredentialRpEntity, PublicKeyCredentialUserEntity, SetMinPinLengthParams,
 };
 use super::status_code::Ctap2StatusCode;
-use super::{cbor_read, key_material};
 use alloc::string::String;
 use alloc::vec::Vec;
 #[cfg(feature = "fuzz")]
@@ -34,6 +34,9 @@ use sk_cbor::destructure_cbor_map;
 // This constant is a consequence of the structure of messages.
 const MIN_LARGE_BLOB_LEN: usize = 17;
 
+/// Expected length of the passed in attestation key bytes during configuration.
+pub const ATTESTATION_PRIVATE_KEY_LENGTH: usize = 32;
+
 // CTAP specification (version 20190130) section 6.1
 #[derive(Debug, PartialEq, Eq)]
 #[allow(clippy::enum_variant_names)]
@@ -498,7 +501,7 @@ impl TryFrom<cbor::Value> for AuthenticatorConfigParameters {
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub struct AuthenticatorAttestationMaterial {
     pub certificate: Vec<u8>,
-    pub private_key: [u8; key_material::ATTESTATION_PRIVATE_KEY_LENGTH],
+    pub private_key: [u8; ATTESTATION_PRIVATE_KEY_LENGTH],
 }
 
 impl TryFrom<cbor::Value> for AuthenticatorAttestationMaterial {
@@ -513,10 +516,10 @@ impl TryFrom<cbor::Value> for AuthenticatorAttestationMaterial {
         }
         let certificate = extract_byte_string(ok_or_missing(certificate)?)?;
         let private_key = extract_byte_string(ok_or_missing(private_key)?)?;
-        if private_key.len() != key_material::ATTESTATION_PRIVATE_KEY_LENGTH {
+        if private_key.len() != ATTESTATION_PRIVATE_KEY_LENGTH {
             return Err(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER);
         }
-        let private_key = array_ref!(private_key, 0, key_material::ATTESTATION_PRIVATE_KEY_LENGTH);
+        let private_key = array_ref!(private_key, 0, ATTESTATION_PRIVATE_KEY_LENGTH);
         Ok(AuthenticatorAttestationMaterial {
             certificate,
             private_key: *private_key,
@@ -1025,14 +1028,14 @@ mod test {
         );
 
         let dummy_cert = [0xddu8; 20];
-        let dummy_pkey = [0x41u8; key_material::ATTESTATION_PRIVATE_KEY_LENGTH];
+        let dummy_pkey = [0x41u8; ATTESTATION_PRIVATE_KEY_LENGTH];
 
         // Attestation key is too short.
         let cbor_value = cbor_map! {
             0x01 => false,
             0x02 => cbor_map! {
                 0x01 => dummy_cert,
-                0x02 => dummy_pkey[..key_material::ATTESTATION_PRIVATE_KEY_LENGTH - 1]
+                0x02 => dummy_pkey[..ATTESTATION_PRIVATE_KEY_LENGTH - 1]
             }
         };
         assert_eq!(

src/ctap/key_material.rs

@@ -1,19 +0,0 @@
-// Copyright 2019-2021 Google LLC
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//      http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-pub const ATTESTATION_PRIVATE_KEY_LENGTH: usize = 32;
-pub const AAGUID_LENGTH: usize = 16;
-
-pub const AAGUID: &[u8; AAGUID_LENGTH] =
-    include_bytes!(concat!(env!("OUT_DIR"), "/opensk_aaguid.bin"));

src/ctap/mod.rs

@@ -23,7 +23,6 @@ mod crypto_wrapper;
 mod ctap1;
 pub mod data_formats;
 pub mod hid;
-pub mod key_material;
 mod large_blobs;
 pub mod main_hid;
 mod pin_protocol;
@@ -873,7 +872,7 @@ impl<E: Env> CtapState<E> {
         };
 
         let mut auth_data = self.generate_auth_data(env, &rp_id_hash, flags)?;
-        auth_data.extend(&storage::aaguid(env)?);
+        auth_data.extend(env.customization().aaguid());
         // The length is fixed to 0x20 or 0x80 and fits one byte.
         if credential_id.len() > 0xFF {
             return Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR);
@@ -1261,7 +1260,7 @@ impl<E: Env> CtapState<E> {
                     String::from("credBlob"),
                     String::from("largeBlobKey"),
                 ]),
-                aaguid: storage::aaguid(env)?,
+                aaguid: *env.customization().aaguid(),
                 options: Some(options),
                 max_msg_size: Some(env.customization().max_msg_size() as u64),
                 // The order implies preference. We favor the new V2.
@@ -1442,7 +1441,7 @@ mod test {
     use super::client_pin::PIN_TOKEN_LENGTH;
     use super::command::{
         AuthenticatorAttestationMaterial, AuthenticatorClientPinParameters,
-        AuthenticatorCredentialManagementParameters,
+        AuthenticatorCredentialManagementParameters, ATTESTATION_PRIVATE_KEY_LENGTH,
     };
     use super::credential_id::CBOR_CREDENTIAL_ID_SIZE;
     use super::data_formats::{
@@ -1528,7 +1527,7 @@ mod test {
                     String::from("credBlob"),
                     String::from("largeBlobKey"),
                 ],
-            0x03 => storage::aaguid(&mut env).unwrap(),
+            0x03 => env.customization().aaguid(),
             0x04 => cbor_map_options! {
                 "ep" => env.customization().enterprise_attestation_mode().map(|_| false),
                 "rk" => true,
@@ -1644,7 +1643,7 @@ mod test {
         match make_credential_response {
             ResponseData::AuthenticatorMakeCredential(make_credential_response) => {
                 let auth_data = make_credential_response.auth_data;
-                let offset = 37 + storage::aaguid(env).unwrap().len();
+                let offset = 37 + env.customization().aaguid().len();
                 assert_eq!(auth_data[offset], 0x00);
                 assert_eq!(auth_data[offset + 1] as usize, CBOR_CREDENTIAL_ID_SIZE);
                 auth_data[offset + 2..offset + 2 + CBOR_CREDENTIAL_ID_SIZE].to_vec()
@@ -1665,7 +1664,7 @@ mod test {
         check_make_response(
             &make_credential_response,
             0x41,
-            &storage::aaguid(&mut env).unwrap(),
+            env.customization().aaguid(),
             0x20,
             &[],
         );
@@ -1684,7 +1683,7 @@ mod test {
         check_make_response(
             &make_credential_response,
             0x41,
-            &storage::aaguid(&mut env).unwrap(),
+            env.customization().aaguid(),
             CBOR_CREDENTIAL_ID_SIZE as u8,
             &[],
         );
@@ -1854,7 +1853,7 @@ mod test {
         check_make_response(
             &make_credential_response,
             0xC1,
-            &storage::aaguid(&mut env).unwrap(),
+            env.customization().aaguid(),
             CBOR_CREDENTIAL_ID_SIZE as u8,
             &expected_extension_cbor,
         );
@@ -1880,7 +1879,7 @@ mod test {
         check_make_response(
             &make_credential_response,
             0xC1,
-            &storage::aaguid(&mut env).unwrap(),
+            env.customization().aaguid(),
             0x20,
             &expected_extension_cbor,
         );
@@ -1903,7 +1902,7 @@ mod test {
         check_make_response(
             &make_credential_response,
             0x41,
-            &storage::aaguid(&mut env).unwrap(),
+            env.customization().aaguid(),
             0x20,
             &[],
         );
@@ -1929,7 +1928,7 @@ mod test {
         check_make_response(
             &make_credential_response,
             0xC1,
-            &storage::aaguid(&mut env).unwrap(),
+            env.customization().aaguid(),
             0x20,
             &expected_extension_cbor,
         );
@@ -1954,7 +1953,7 @@ mod test {
         check_make_response(
             &make_credential_response,
             0xC1,
-            &storage::aaguid(&mut env).unwrap(),
+            env.customization().aaguid(),
             0x20,
             &expected_extension_cbor,
         );
@@ -1986,7 +1985,7 @@ mod test {
         check_make_response(
             &make_credential_response,
             0xC1,
-            &storage::aaguid(&mut env).unwrap(),
+            env.customization().aaguid(),
             0x20,
             &expected_extension_cbor,
         );
@@ -2064,7 +2063,7 @@ mod test {
         check_make_response(
             &make_credential_response,
             0x45,
-            &storage::aaguid(&mut env).unwrap(),
+            env.customization().aaguid(),
             0x20,
             &[],
         );
@@ -2101,7 +2100,7 @@ mod test {
         check_make_response(
             &make_credential_response,
             0x41,
-            &storage::aaguid(&mut env).unwrap(),
+            env.customization().aaguid(),
             CBOR_CREDENTIAL_ID_SIZE as u8,
             &[],
         );
@@ -2800,7 +2799,7 @@ mod test {
         check_make_response(
             &make_credential_response,
             0xC1,
-            &storage::aaguid(&mut env).unwrap(),
+            env.customization().aaguid(),
             CBOR_CREDENTIAL_ID_SIZE as u8,
             &expected_extension_cbor,
         );
@@ -3240,7 +3239,7 @@ mod test {
         );
 
         // Inject dummy values
-        let dummy_key = [0x41u8; key_material::ATTESTATION_PRIVATE_KEY_LENGTH];
+        let dummy_key = [0x41u8; ATTESTATION_PRIVATE_KEY_LENGTH];
         let dummy_cert = [0xddu8; 20];
         let response = ctap_state.process_vendor_configure(
             &mut env,
@@ -3271,7 +3270,7 @@ mod test {
         );
 
         // Try to inject other dummy values and check that initial values are retained.
-        let other_dummy_key = [0x44u8; key_material::ATTESTATION_PRIVATE_KEY_LENGTH];
+        let other_dummy_key = [0x44u8; ATTESTATION_PRIVATE_KEY_LENGTH];
         let response = ctap_state.process_vendor_configure(
             &mut env,
             AuthenticatorVendorConfigureParameters {

src/ctap/storage.rs

@@ -1,4 +1,4 @@
-// Copyright 2019-2021 Google LLC
+// Copyright 2019-2023 Google LLC
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -22,7 +22,7 @@ use crate::ctap::data_formats::{
     extract_array, extract_text_string, PublicKeyCredentialSource, PublicKeyCredentialUserEntity,
 };
 use crate::ctap::status_code::Ctap2StatusCode;
-use crate::ctap::{key_material, INITIAL_SIGNATURE_COUNTER};
+use crate::ctap::INITIAL_SIGNATURE_COUNTER;
 use crate::env::Env;
 use alloc::string::String;
 use alloc::vec;
@@ -54,10 +54,6 @@ pub fn init(env: &mut impl Env) -> Result<(), Ctap2StatusCode> {
         cred_random.extend_from_slice(&cred_random_with_uv);
         env.store().insert(key::CRED_RANDOM_SECRET, &cred_random)?;
     }
-
-    if env.store().find_handle(key::AAGUID)?.is_none() {
-        set_aaguid(env, key_material::AAGUID)?;
-    }
     Ok(())
 }
 
@@ -434,28 +430,6 @@ pub fn commit_large_blob_array(
     )?)
 }
 
-/// Returns the AAGUID.
-pub fn aaguid(env: &mut impl Env) -> Result<[u8; key_material::AAGUID_LENGTH], Ctap2StatusCode> {
-    let aaguid = env
-        .store()
-        .find(key::AAGUID)?
-        .ok_or(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR)?;
-    if aaguid.len() != key_material::AAGUID_LENGTH {
-        return Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR);
-    }
-    Ok(*array_ref![aaguid, 0, key_material::AAGUID_LENGTH])
-}
-
-/// Sets the AAGUID.
-///
-/// If it is already defined, it is overwritten.
-pub fn set_aaguid(
-    env: &mut impl Env,
-    aaguid: &[u8; key_material::AAGUID_LENGTH],
-) -> Result<(), Ctap2StatusCode> {
-    Ok(env.store().insert(key::AAGUID, aaguid)?)
-}
-
 /// Resets the store as for a CTAP reset.
 ///
 /// In particular persistent entries are not reset.
@@ -646,6 +620,7 @@ fn serialize_min_pin_length_rp_ids(rp_ids: Vec<String>) -> Result<Vec<u8>, Ctap2
 mod test {
     use super::*;
     use crate::api::attestation_store::{self, Attestation, AttestationStore};
+    use crate::ctap::command::ATTESTATION_PRIVATE_KEY_LENGTH;
     use crate::ctap::crypto_wrapper::PrivateKey;
     use crate::ctap::data_formats::{
         CredentialProtectionPolicy, PublicKeyCredentialSource, PublicKeyCredentialType,
@@ -963,13 +938,12 @@ mod test {
 
         // Make sure the persistent keys are initialized to dummy values.
         let dummy_attestation = Attestation {
-            private_key: [0x41; key_material::ATTESTATION_PRIVATE_KEY_LENGTH],
+            private_key: [0x41; ATTESTATION_PRIVATE_KEY_LENGTH],
             certificate: vec![0xdd; 20],
         };
         env.attestation_store()
             .set(&attestation_store::Id::Batch, Some(&dummy_attestation))
             .unwrap();
-        assert_eq!(&aaguid(&mut env).unwrap(), key_material::AAGUID);
 
         // The persistent keys stay initialized and preserve their value after a reset.
         reset(&mut env).unwrap();
@@ -977,7 +951,6 @@ mod test {
             env.attestation_store().get(&attestation_store::Id::Batch),
             Ok(Some(dummy_attestation))
         );
-        assert_eq!(&aaguid(&mut env).unwrap(), key_material::AAGUID);
     }
 
     #[test]
@@ -1112,7 +1085,7 @@ mod test {
         let mut env = TestEnv::new();
 
         let dummy_attestation = Attestation {
-            private_key: [0x41; key_material::ATTESTATION_PRIVATE_KEY_LENGTH],
+            private_key: [0x41; ATTESTATION_PRIVATE_KEY_LENGTH],
             certificate: vec![0xdd; 20],
         };
         env.attestation_store()

src/ctap/storage/key.rs

@@ -64,8 +64,8 @@ make_partition! {
     /// Reserved for the attestation store implementation of the environment.
     _RESERVED_ATTESTATION_STORE = 1..3;
 
-    /// The aaguid.
+    /// Used for the AAGUID before, but deprecated.
-    AAGUID = 3;
+    _AAGUID = 3;
 
     // This is the persistent key limit:
     // - When adding a (persistent) key above this message, make sure its value is smaller than

src/env/test/customization.rs

@@ -12,12 +12,13 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use crate::api::customization::{Customization, CustomizationImpl};
+use crate::api::customization::{Customization, CustomizationImpl, AAGUID_LENGTH};
 use crate::ctap::data_formats::{CredentialProtectionPolicy, EnterpriseAttestationMode};
 use alloc::string::String;
 use alloc::vec::Vec;
 
 pub struct TestCustomization {
+    aaguid: &'static [u8; AAGUID_LENGTH],
     allows_pin_protocol_v1: bool,
     default_cred_protect: Option<CredentialProtectionPolicy>,
     default_min_pin_length: u8,
@@ -54,6 +55,10 @@ impl TestCustomization {
 }
 
 impl Customization for TestCustomization {
+    fn aaguid(&self) -> &'static [u8; AAGUID_LENGTH] {
+        self.aaguid
+    }
+
     fn allows_pin_protocol_v1(&self) -> bool {
         self.allows_pin_protocol_v1
     }
@@ -126,6 +131,7 @@ impl Customization for TestCustomization {
 impl From<CustomizationImpl> for TestCustomization {
     fn from(c: CustomizationImpl) -> Self {
         let CustomizationImpl {
+            aaguid,
             allows_pin_protocol_v1,
             default_cred_protect,
             default_min_pin_length,
@@ -155,6 +161,7 @@ impl From<CustomizationImpl> for TestCustomization {
             .collect::<Vec<_>>();
 
         Self {
+            aaguid,
             allows_pin_protocol_v1,
             default_cred_protect,
             default_min_pin_length,

src/env/tock/mod.rs

@@ -17,7 +17,7 @@ use crate::api::attestation_store::AttestationStore;
 use crate::api::connection::{
     HidConnection, SendOrRecvError, SendOrRecvResult, SendOrRecvStatus, UsbEndpoint,
 };
-use crate::api::customization::{CustomizationImpl, DEFAULT_CUSTOMIZATION};
+use crate::api::customization::{CustomizationImpl, AAGUID_LENGTH, DEFAULT_CUSTOMIZATION};
 use crate::api::firmware_protection::FirmwareProtection;
 use crate::api::user_presence::{UserPresence, UserPresenceError, UserPresenceResult};
 use crate::api::{attestation_store, key_store};
@@ -38,6 +38,14 @@ use rng256::Rng256;
 mod clock;
 mod storage;
 
+pub const AAGUID: &[u8; AAGUID_LENGTH] =
+    include_bytes!(concat!(env!("OUT_DIR"), "/opensk_aaguid.bin"));
+
+const TOCK_CUSTOMIZATION: CustomizationImpl = CustomizationImpl {
+    aaguid: AAGUID,
+    ..DEFAULT_CUSTOMIZATION
+};
+
 /// RNG backed by the TockOS rng driver.
 pub struct TockRng256 {}
 
@@ -279,7 +287,7 @@ impl Env for TockEnv {
     }
 
     fn customization(&self) -> &Self::Customization {
-        &DEFAULT_CUSTOMIZATION
+        &TOCK_CUSTOMIZATION
     }
 
     fn main_hid_connection(&mut self) -> &mut Self::HidConnection {

src/test_helpers/mod.rs

@@ -14,11 +14,11 @@
 
 use crate::ctap::command::{
     AuthenticatorAttestationMaterial, AuthenticatorConfigParameters,
-    AuthenticatorVendorConfigureParameters, Command,
+    AuthenticatorVendorConfigureParameters, Command, ATTESTATION_PRIVATE_KEY_LENGTH,
 };
 use crate::ctap::data_formats::ConfigSubCommand;
 use crate::ctap::status_code::Ctap2StatusCode;
-use crate::ctap::{key_material, Channel, CtapState};
+use crate::ctap::{Channel, CtapState};
 use crate::env::Env;
 
 // In tests where we define a dummy user-presence check that immediately returns, the channel
@@ -31,7 +31,7 @@ pub fn enable_enterprise_attestation<E: Env>(
     state: &mut CtapState<E>,
     env: &mut E,
 ) -> Result<AuthenticatorAttestationMaterial, Ctap2StatusCode> {
-    let dummy_key = [0x41; key_material::ATTESTATION_PRIVATE_KEY_LENGTH];
+    let dummy_key = [0x41; ATTESTATION_PRIVATE_KEY_LENGTH];
     let dummy_cert = vec![0xdd; 20];
     let attestation_material = AuthenticatorAttestationMaterial {
         certificate: dummy_cert,