From 7769e783bb5e797743cd54d34ab605d5bbb49a81 Mon Sep 17 00:00:00 2001 From: kaczmarczyck <43844792+kaczmarczyck@users.noreply.github.com> Date: Mon, 6 Mar 2023 11:42:56 +0100 Subject: [PATCH] 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 | 11 +++++++++++ src/ctap/command.rs | 17 +++++++++------- src/ctap/key_material.rs | 19 ------------------ src/ctap/mod.rs | 37 +++++++++++++++++------------------ src/ctap/storage.rs | 37 +++++------------------------------ src/ctap/storage/key.rs | 4 ++-- src/env/test/customization.rs | 9 ++++++++- src/env/tock/mod.rs | 12 ++++++++++-- src/test_helpers/mod.rs | 6 +++--- 9 files changed, 67 insertions(+), 85 deletions(-) delete mode 100644 src/ctap/key_material.rs diff --git a/src/api/customization.rs b/src/api/customization.rs index dd45018..742320d 100644 --- a/src/api/customization.rs +++ b/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, 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 } diff --git a/src/ctap/command.rs b/src/ctap/command.rs index 5453f01..53bf231 100644 --- a/src/ctap/command.rs +++ b/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 for AuthenticatorConfigParameters { #[derive(Clone, Debug, PartialEq, Eq)] pub struct AuthenticatorAttestationMaterial { pub certificate: Vec, - pub private_key: [u8; key_material::ATTESTATION_PRIVATE_KEY_LENGTH], + pub private_key: [u8; ATTESTATION_PRIVATE_KEY_LENGTH], } impl TryFrom for AuthenticatorAttestationMaterial { @@ -513,10 +516,10 @@ impl TryFrom 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!( diff --git a/src/ctap/key_material.rs b/src/ctap/key_material.rs deleted file mode 100644 index a8ae6da..0000000 --- a/src/ctap/key_material.rs +++ /dev/null @@ -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")); diff --git a/src/ctap/mod.rs b/src/ctap/mod.rs index c324074..60acc31 100644 --- a/src/ctap/mod.rs +++ b/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 CtapState { }; 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 CtapState { 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 { diff --git a/src/ctap/storage.rs b/src/ctap/storage.rs index 2fb0939..2f15ee7 100644 --- a/src/ctap/storage.rs +++ b/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) -> Result, 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() diff --git a/src/ctap/storage/key.rs b/src/ctap/storage/key.rs index 6b6f28a..5243f2a 100644 --- a/src/ctap/storage/key.rs +++ b/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. - AAGUID = 3; + /// Used for the AAGUID before, but deprecated. + _AAGUID = 3; // This is the persistent key limit: // - When adding a (persistent) key above this message, make sure its value is smaller than diff --git a/src/env/test/customization.rs b/src/env/test/customization.rs index 642d729..f86a89a 100644 --- a/src/env/test/customization.rs +++ b/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, 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 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 for TestCustomization { .collect::>(); Self { + aaguid, allows_pin_protocol_v1, default_cred_protect, default_min_pin_length, diff --git a/src/env/tock/mod.rs b/src/env/tock/mod.rs index 19c640f..7d675c6 100644 --- a/src/env/tock/mod.rs +++ b/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 { diff --git a/src/test_helpers/mod.rs b/src/test_helpers/mod.rs index 6420a10..62c84e8 100644 --- a/src/test_helpers/mod.rs +++ b/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( state: &mut CtapState, env: &mut E, ) -> Result { - 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,