From 8f20a75b17dde0096163209a9532c33038a834dd Mon Sep 17 00:00:00 2001
From: Fabian Kaczmarczyck <kaczmarczyck@google.com>
Date: Fri, 17 Apr 2020 17:13:21 +0200
Subject: [PATCH] add 2.1 features to GetInfo

---
 README.md                |  4 ++-
 src/ctap/command.rs      | 45 ++++++++++++++++++------------
 src/ctap/data_formats.rs | 43 +++++++++++++++++++++++++++-
 src/ctap/mod.rs          | 60 +++++++++++++++++++++-------------------
 src/ctap/response.rs     | 36 ++++++++++++++++++------
 5 files changed, 132 insertions(+), 56 deletions(-)

diff --git a/README.md b/README.md
index 3f71d71..fb7d1c9 100644
--- a/README.md
+++ b/README.md
@@ -28,7 +28,9 @@ few limitations:
 Although we tested and implemented our firmware based on the published
 [CTAP2.0 specifications](https://fidoalliance.org/specs/fido-v2.0-ps-20190130/fido-client-to-authenticator-protocol-v2.0-ps-20190130.html),
 our implementation was not reviewed nor officially tested and doesn't claim to
-be FIDO Certified.
+be FIDO Certified. With the upcoming next version of the
+[CTAP2.1 specifications](https://fidoalliance.org/specs/fido2/fido-client-to-authenticator-protocol-v2.1-rd-20191217.html),
+we started adding features, so master is currently between version 2.0 and 2.1.
 
 ### Cryptography
 
diff --git a/src/ctap/command.rs b/src/ctap/command.rs
index 129c671..9ccf104 100644
--- a/src/ctap/command.rs
+++ b/src/ctap/command.rs
@@ -13,16 +13,20 @@
 // limitations under the License.
 
 use super::data_formats::{
-    ok_or_missing, read_array, read_byte_string, read_integer, read_map, read_text_string,
-    read_unsigned, ClientPinSubCommand, CoseKey, Extensions, GetAssertionOptions,
-    MakeCredentialOptions, PublicKeyCredentialDescriptor, PublicKeyCredentialRpEntity,
-    PublicKeyCredentialType, PublicKeyCredentialUserEntity,
+    ok_or_missing, read_array, read_byte_string, read_map, read_text_string, read_unsigned,
+    ClientPinSubCommand, CoseKey, Extensions, GetAssertionOptions, MakeCredentialOptions,
+    PublicKeyCredentialDescriptor, PublicKeyCredentialParameter, PublicKeyCredentialRpEntity,
+    PublicKeyCredentialUserEntity,
 };
 use super::status_code::Ctap2StatusCode;
 use alloc::string::String;
 use alloc::vec::Vec;
 use core::convert::TryFrom;
 
+// Depending on your memory, you can use Some(n) to limit request sizes.
+// You might also want to set the max credential size in process_get_info then.
+pub const MAX_CREDENTIAL_COUNT_IN_LIST: Option<u64> = None;
+
 // CTAP specification (version 20190130) section 6.1
 #[cfg_attr(any(test, feature = "debug_ctap"), derive(Debug, PartialEq))]
 pub enum Command {
@@ -106,7 +110,7 @@ pub struct AuthenticatorMakeCredentialParameters {
     pub client_data_hash: Vec<u8>,
     pub rp: PublicKeyCredentialRpEntity,
     pub user: PublicKeyCredentialUserEntity,
-    pub pub_key_cred_params: Vec<(PublicKeyCredentialType, i64)>,
+    pub pub_key_cred_params: Vec<PublicKeyCredentialParameter>,
     pub exclude_list: Option<Vec<PublicKeyCredentialDescriptor>>,
     pub extensions: Option<Extensions>,
     // Even though options are optional, we can use the default if not present.
@@ -134,12 +138,9 @@ impl TryFrom<cbor::Value> for AuthenticatorMakeCredentialParameters {
         let cred_param_vec = read_array(ok_or_missing(param_map.get(&cbor_unsigned!(4)))?)?;
         let mut pub_key_cred_params = vec![];
         for cred_param_map_value in cred_param_vec {
-            let cred_param_map = read_map(cred_param_map_value)?;
-            let cred_type = PublicKeyCredentialType::try_from(ok_or_missing(
-                cred_param_map.get(&cbor_text!("type")),
-            )?)?;
-            let alg = read_integer(ok_or_missing(cred_param_map.get(&cbor_text!("alg")))?)?;
-            pub_key_cred_params.push((cred_type, alg));
+            if let Ok(cred_param) = PublicKeyCredentialParameter::try_from(cred_param_map_value) {
+                pub_key_cred_params.push(cred_param);
+            }
         }
 
         let exclude_list = match param_map.get(&cbor_unsigned!(5)) {
@@ -147,6 +148,11 @@ impl TryFrom<cbor::Value> for AuthenticatorMakeCredentialParameters {
                 let exclude_list_vec = read_array(entry)?;
                 let mut exclude_list = vec![];
                 for exclude_list_value in exclude_list_vec {
+                    if let Some(count) = MAX_CREDENTIAL_COUNT_IN_LIST {
+                        if exclude_list.len() as u64 >= count {
+                            break;
+                        }
+                    }
                     exclude_list.push(PublicKeyCredentialDescriptor::try_from(exclude_list_value)?);
                 }
                 Some(exclude_list)
@@ -218,6 +224,11 @@ impl TryFrom<cbor::Value> for AuthenticatorGetAssertionParameters {
                 let allow_list_vec = read_array(entry)?;
                 let mut allow_list = vec![];
                 for allow_list_value in allow_list_vec {
+                    if let Some(count) = MAX_CREDENTIAL_COUNT_IN_LIST {
+                        if allow_list.len() as u64 >= count {
+                            break;
+                        }
+                    }
                     allow_list.push(PublicKeyCredentialDescriptor::try_from(allow_list_value)?);
                 }
                 Some(allow_list)
@@ -316,8 +327,10 @@ impl TryFrom<cbor::Value> for AuthenticatorClientPinParameters {
 #[cfg(test)]
 mod test {
     use super::super::data_formats::{
-        AuthenticatorTransport, PublicKeyCredentialRpEntity, PublicKeyCredentialUserEntity,
+        AuthenticatorTransport, PublicKeyCredentialRpEntity, PublicKeyCredentialType,
+        PublicKeyCredentialUserEntity,
     };
+    use super::super::CREDENTIAL_PARAMETER;
     use super::*;
     use alloc::collections::BTreeMap;
 
@@ -336,10 +349,7 @@ mod test {
                 "displayName" => "bar",
                 "icon" => "example.com/foo/icon.png",
             },
-            4 => cbor_array![ cbor_map! {
-                "type" => "public-key",
-                "alg" => -7
-            } ],
+            4 => cbor_array![CREDENTIAL_PARAMETER],
             5 => cbor_array![],
             8 => vec![0x12, 0x34],
             9 => 1,
@@ -362,7 +372,6 @@ mod test {
             user_display_name: Some("bar".to_string()),
             user_icon: Some("example.com/foo/icon.png".to_string()),
         };
-        let pub_key_cred_param = (PublicKeyCredentialType::PublicKey, -7);
         let options = MakeCredentialOptions {
             rk: false,
             uv: false,
@@ -371,7 +380,7 @@ mod test {
             client_data_hash,
             rp,
             user,
-            pub_key_cred_params: vec![pub_key_cred_param],
+            pub_key_cred_params: vec![CREDENTIAL_PARAMETER],
             exclude_list: Some(vec![]),
             extensions: None,
             options,
diff --git a/src/ctap/data_formats.rs b/src/ctap/data_formats.rs
index 3a2bed9..a4bbe39 100644
--- a/src/ctap/data_formats.rs
+++ b/src/ctap/data_formats.rs
@@ -121,6 +121,36 @@ impl TryFrom<&cbor::Value> for PublicKeyCredentialType {
     }
 }
 
+#[derive(PartialEq)]
+#[cfg_attr(any(test, feature = "debug_ctap"), derive(Debug))]
+pub struct PublicKeyCredentialParameter {
+    pub cred_type: PublicKeyCredentialType,
+    pub alg: SignatureAlgorithm,
+}
+
+impl TryFrom<&cbor::Value> for PublicKeyCredentialParameter {
+    type Error = Ctap2StatusCode;
+
+    fn try_from(cbor_value: &cbor::Value) -> Result<Self, Ctap2StatusCode> {
+        let cred_param_map = read_map(cbor_value)?;
+        let cred_type = PublicKeyCredentialType::try_from(ok_or_missing(
+            cred_param_map.get(&cbor_text!("type")),
+        )?)?;
+        let alg =
+            SignatureAlgorithm::try_from(ok_or_missing(cred_param_map.get(&cbor_text!("alg")))?)?;
+        Ok(Self { cred_type, alg })
+    }
+}
+
+impl From<PublicKeyCredentialParameter> for cbor::Value {
+    fn from(cred_param: PublicKeyCredentialParameter) -> Self {
+        cbor_map_options! {
+            "type" => cred_param.cred_type,
+            "alg" => cred_param.alg as i64,
+        }
+    }
+}
+
 #[cfg_attr(any(test, feature = "debug_ctap"), derive(Debug, PartialEq))]
 pub enum AuthenticatorTransport {
     Usb,
@@ -369,12 +399,23 @@ impl From<PackedAttestationStatement> for cbor::Value {
     }
 }
 
-#[cfg_attr(test, derive(PartialEq))]
+#[derive(PartialEq)]
 #[cfg_attr(any(test, feature = "debug_ctap"), derive(Debug))]
 pub enum SignatureAlgorithm {
     ES256 = ecdsa::PubKey::ES256_ALGORITHM as isize,
 }
 
+impl TryFrom<&cbor::Value> for SignatureAlgorithm {
+    type Error = Ctap2StatusCode;
+
+    fn try_from(cbor_value: &cbor::Value) -> Result<Self, Ctap2StatusCode> {
+        match read_integer(cbor_value)? {
+            ecdsa::PubKey::ES256_ALGORITHM => Ok(SignatureAlgorithm::ES256),
+            _ => Err(Ctap2StatusCode::CTAP2_ERR_UNSUPPORTED_ALGORITHM),
+        }
+    }
+}
+
 #[derive(Clone)]
 #[cfg_attr(test, derive(PartialEq))]
 #[cfg_attr(any(test, feature = "debug_ctap"), derive(Debug))]
diff --git a/src/ctap/mod.rs b/src/ctap/mod.rs
index e4a1151..7624435 100644
--- a/src/ctap/mod.rs
+++ b/src/ctap/mod.rs
@@ -25,12 +25,13 @@ mod timed_permission;
 
 use self::command::{
     AuthenticatorClientPinParameters, AuthenticatorGetAssertionParameters,
-    AuthenticatorMakeCredentialParameters, Command,
+    AuthenticatorMakeCredentialParameters, Command, MAX_CREDENTIAL_COUNT_IN_LIST,
 };
 use self::data_formats::{
-    ClientPinSubCommand, CoseKey, GetAssertionHmacSecretInput, PackedAttestationStatement,
-    PublicKeyCredentialDescriptor, PublicKeyCredentialSource, PublicKeyCredentialType,
-    PublicKeyCredentialUserEntity, SignatureAlgorithm,
+    AuthenticatorTransport, ClientPinSubCommand, CoseKey, GetAssertionHmacSecretInput,
+    PackedAttestationStatement, PublicKeyCredentialDescriptor, PublicKeyCredentialParameter,
+    PublicKeyCredentialSource, PublicKeyCredentialType, PublicKeyCredentialUserEntity,
+    SignatureAlgorithm,
 };
 use self::hid::ChannelID;
 use self::key_material::{AAGUID, ATTESTATION_CERTIFICATE, ATTESTATION_PRIVATE_KEY};
@@ -99,6 +100,11 @@ pub const FIDO2_VERSION_STRING: &str = "FIDO_2_0";
 #[cfg(feature = "with_ctap1")]
 pub const U2F_VERSION_STRING: &str = "U2F_V2";
 
+pub const CREDENTIAL_PARAMETER: PublicKeyCredentialParameter = PublicKeyCredentialParameter {
+    cred_type: PublicKeyCredentialType::PublicKey,
+    alg: SignatureAlgorithm::ES256,
+};
+
 fn check_pin_auth(hmac_key: &[u8], hmac_contents: &[u8], pin_auth: &[u8]) -> bool {
     if pin_auth.len() != PIN_AUTH_LENGTH {
         return false;
@@ -413,15 +419,7 @@ where
             }
         }
 
-        let has_es_256 = pub_key_cred_params
-            .iter()
-            .any(|(credential_type, algorithm)| {
-                // Even though there is only one type now, checking seems safer in
-                // case of extension so you can't forget to update here.
-                *credential_type == PublicKeyCredentialType::PublicKey
-                    && *algorithm == SignatureAlgorithm::ES256 as i64
-            });
-        if !has_es_256 {
+        if !pub_key_cred_params.contains(&CREDENTIAL_PARAMETER) {
             return Err(Ctap2StatusCode::CTAP2_ERR_UNSUPPORTED_ALGORITHM);
         }
 
@@ -751,7 +749,7 @@ where
 
     fn process_get_info(&self) -> Result<ResponseData, Ctap2StatusCode> {
         let mut options_map = BTreeMap::new();
-        // TODO(kaczmarczyck) add FIDO 2.1 options
+        // TODO(kaczmarczyck) add authenticatorConfig and credProtect options
         options_map.insert(String::from("rk"), true);
         options_map.insert(String::from("up"), true);
         options_map.insert(
@@ -772,6 +770,13 @@ where
                 pin_protocols: Some(vec![
                     CtapState::<R, CheckUserPresence>::PIN_PROTOCOL_VERSION,
                 ]),
+                max_credential_count_in_list: MAX_CREDENTIAL_COUNT_IN_LIST,
+                // You can use ENCRYPTED_CREDENTIAL_ID_SIZE here, but if your
+                // browser passes that value, it might be used to fingerprint.
+                max_credential_id_length: None,
+                transports: Some(vec![AuthenticatorTransport::Usb]),
+                algorithms: Some(vec![CREDENTIAL_PARAMETER]),
+                firmware_version: None,
             },
         ))
     }
@@ -1093,7 +1098,7 @@ mod test {
         let mut ctap_state = CtapState::new(&mut rng, user_immediately_present);
         let info_reponse = ctap_state.process_command(&[0x04], DUMMY_CHANNEL_ID);
 
-        let mut expected_response = vec![0x00, 0xA6, 0x01];
+        let mut expected_response = vec![0x00, 0xA8, 0x01];
         // The difference here is a longer array of supported versions.
         #[cfg(not(feature = "with_ctap1"))]
         expected_response.extend(&[0x81, 0x68, 0x46, 0x49, 0x44, 0x4F, 0x5F, 0x32, 0x5F, 0x30]);
@@ -1107,10 +1112,16 @@ mod test {
             0x03, 0x50,
         ]);
         expected_response.extend(AAGUID);
-        expected_response.extend(&[
-            0x04, 0xA3, 0x62, 0x72, 0x6B, 0xF5, 0x62, 0x75, 0x70, 0xF5, 0x69, 0x63, 0x6C, 0x69,
-            0x65, 0x6E, 0x74, 0x50, 0x69, 0x6E, 0xF4, 0x05, 0x19, 0x04, 0x00, 0x06, 0x81, 0x01,
-        ]);
+        expected_response.extend(
+            [
+                0x04, 0xA3, 0x62, 0x72, 0x6B, 0xF5, 0x62, 0x75, 0x70, 0xF5, 0x69, 0x63, 0x6C, 0x69,
+                0x65, 0x6E, 0x74, 0x50, 0x69, 0x6E, 0xF4, 0x05, 0x19, 0x04, 0x00, 0x06, 0x81, 0x01,
+                0x09, 0x81, 0x63, 0x75, 0x73, 0x62, 0x0A, 0x81, 0xA2, 0x63, 0x61, 0x6C, 0x67, 0x26,
+                0x64, 0x74, 0x79, 0x70, 0x65, 0x6A, 0x70, 0x75, 0x62, 0x6C, 0x69, 0x63, 0x2D, 0x6B,
+                0x65, 0x79,
+            ]
+            .iter(),
+        );
 
         assert_eq!(info_reponse, expected_response);
     }
@@ -1128,10 +1139,7 @@ mod test {
             user_display_name: None,
             user_icon: None,
         };
-        let pub_key_cred_params = vec![(
-            PublicKeyCredentialType::PublicKey,
-            SignatureAlgorithm::ES256 as i64,
-        )];
+        let pub_key_cred_params = vec![CREDENTIAL_PARAMETER];
         let options = MakeCredentialOptions {
             rk: true,
             uv: false,
@@ -1228,12 +1236,8 @@ mod test {
         let user_immediately_present = |_| Ok(());
         let mut ctap_state = CtapState::new(&mut rng, user_immediately_present);
 
-        let pub_key_cred_params = vec![(
-            PublicKeyCredentialType::PublicKey,
-            SignatureAlgorithm::ES256 as i64 + 1, // any different number works
-        )];
         let mut make_credential_params = create_minimal_make_credential_parameters();
-        make_credential_params.pub_key_cred_params = pub_key_cred_params;
+        make_credential_params.pub_key_cred_params = vec![];
         let make_credential_response =
             ctap_state.process_make_credential(make_credential_params, DUMMY_CHANNEL_ID);
 
diff --git a/src/ctap/response.rs b/src/ctap/response.rs
index 4b8a089..9960f6c 100644
--- a/src/ctap/response.rs
+++ b/src/ctap/response.rs
@@ -13,8 +13,8 @@
 // limitations under the License.
 
 use super::data_formats::{
-    CoseKey, PackedAttestationStatement, PublicKeyCredentialDescriptor,
-    PublicKeyCredentialUserEntity,
+    AuthenticatorTransport, CoseKey, PackedAttestationStatement, PublicKeyCredentialDescriptor,
+    PublicKeyCredentialParameter, PublicKeyCredentialUserEntity,
 };
 use alloc::collections::BTreeMap;
 use alloc::string::String;
@@ -109,6 +109,11 @@ pub struct AuthenticatorGetInfoResponse {
     pub options: Option<BTreeMap<String, bool>>,
     pub max_msg_size: Option<u64>,
     pub pin_protocols: Option<Vec<u64>>,
+    pub max_credential_count_in_list: Option<u64>,
+    pub max_credential_id_length: Option<u64>,
+    pub transports: Option<Vec<AuthenticatorTransport>>,
+    pub algorithms: Option<Vec<PublicKeyCredentialParameter>>,
+    pub firmware_version: Option<u64>,
 }
 
 impl From<AuthenticatorGetInfoResponse> for cbor::Value {
@@ -120,6 +125,11 @@ impl From<AuthenticatorGetInfoResponse> for cbor::Value {
             options,
             max_msg_size,
             pin_protocols,
+            max_credential_count_in_list,
+            max_credential_id_length,
+            transports,
+            algorithms,
+            firmware_version,
         } = get_info_response;
 
         let options_cbor: Option<cbor::Value> = options.map(|options| {
@@ -131,12 +141,17 @@ impl From<AuthenticatorGetInfoResponse> for cbor::Value {
         });
 
         cbor_map_options! {
-            1 => cbor_array_vec!(versions),
-            2 => extensions.map(|vec| cbor_array_vec!(vec)),
-            3 => &aaguid,
-            4 => options_cbor,
-            5 => max_msg_size,
-            6 => pin_protocols.map(|vec| cbor_array_vec!(vec)),
+            0x01 => cbor_array_vec!(versions),
+            0x02 => extensions.map(|vec| cbor_array_vec!(vec)),
+            0x03 => &aaguid,
+            0x04 => options_cbor,
+            0x05 => max_msg_size,
+            0x06 => pin_protocols.map(|vec| cbor_array_vec!(vec)),
+            0x07 => max_credential_count_in_list,
+            0x08 => max_credential_id_length,
+            0x09 => transports.map(|vec| cbor_array_vec!(vec)),
+            0x0A => algorithms.map(|vec| cbor_array_vec!(vec)),
+            0x0E => firmware_version,
         }
     }
 }
@@ -228,6 +243,11 @@ mod test {
             options: None,
             max_msg_size: None,
             pin_protocols: None,
+            max_credential_count_in_list: None,
+            max_credential_id_length: None,
+            transports: None,
+            algorithms: None,
+            firmware_version: None,
         };
         let response_cbor: Option<cbor::Value> =
             ResponseData::AuthenticatorGetInfo(get_info_response).into();
-- 
GitLab