From 1df37ba67abcedd7ef97f0947e79bbe8a226ca3b Mon Sep 17 00:00:00 2001
From: Szczepan Zalega <szczepan@nitrokey.com>
Date: Sat, 8 Jan 2022 14:03:12 +0100
Subject: [PATCH] Make the Credential ID shorter to work with some services

Some services do not accept arbitrary long key handle (aka Credential
ID), which makes the FIDO operations failing. This patch removes some
fields from credential data serialization while making credential ID,
and with this it reduces key handle size by around 30% (from ~320 to ~220
using test site [1]). Tested on Gitlab, and this patch makes it working
correctly (both registering and signing, as opposed to 500 error code
returned otherwise). Presumably the hidden limit is 255 bytes, which
would be compatible with CTAP1.

Resident Keys stay the same, with full metadata stored on the device.

[1] webauthn.bin.coffee
---
 src/credential.rs |  2 ++
 src/lib.rs        | 58 ++++++++++++++++++++++++++++++++++-------------
 2 files changed, 44 insertions(+), 16 deletions(-)

diff --git a/src/credential.rs b/src/credential.rs
index a21a00a..ceb568e 100644
--- a/src/credential.rs
+++ b/src/credential.rs
@@ -89,7 +89,9 @@ pub struct CredentialData {
     pub key: Key,
 
     // extensions
+    #[serde(skip_serializing_if = "Option::is_none")]
     pub hmac_secret: Option<bool>,
+    #[serde(skip_serializing_if = "Option::is_none")]
     pub cred_protect: Option<CredentialProtectionPolicy>,
 
     // TODO: add `sig_counter: Option<CounterId>`,
diff --git a/src/lib.rs b/src/lib.rs
index ed3aeae..2973231 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -35,6 +35,10 @@ use ctap_types::{
         Result as U2fResult,
         Error as U2fError,
     },
+    webauthn::{
+        PublicKeyCredentialRpEntity,
+        PublicKeyCredentialUserEntity
+    }
 };
 
 use littlefs2::path::{Path, PathBuf};
@@ -1835,31 +1839,53 @@ where UP: UserPresence,
         let nonce = syscall!(self.trussed.random_bytes(12)).bytes.as_slice().try_into().unwrap();
         info!("nonce = {:?}", &nonce);
 
-        let credential = Credential::new(
-            credential::CtapVersion::Fido21Pre,
-            &parameters.rp,
-            &parameters.user,
-            algorithm as i32,
-            key_parameter,
-            self.state.persistent.timestamp(&mut self.trussed)?,
-            hmac_secret_requested.clone(),
-            cred_protect_requested,
-            nonce,
-        );
-
-        // info!("made credential {:?}", &credential);
-
         // 12.b generate credential ID { = AEAD(Serialize(Credential)) }
         let kek = self.state.persistent.key_encryption_key(&mut self.trussed)?;
-        let credential_id = credential.id_using_hash(&mut self.trussed, kek, &rp_id_hash)?;
 
         // store it.
         // TODO: overwrite, error handling with KeyStoreFull
 
-        let serialized_credential = credential.serialize()?;
+        // Introduce smaller Credential struct for ID, with extra metadata removed. This ensures
+        // ID will stay below 255 bytes.
+        let credential_thin = Credential::new(
+            credential::CtapVersion::Fido21Pre,
+            &PublicKeyCredentialRpEntity{
+                id: parameters.rp.id.clone(),
+                name: None,
+                url: None
+            },
+            &PublicKeyCredentialUserEntity {
+                id: parameters.user.id.clone(),
+                icon: None,
+                name: None,
+                display_name: None
+            },
+            algorithm as i32,
+            key_parameter.clone(),
+            self.state.persistent.timestamp(&mut self.trussed)?,
+            None,
+            None,
+            nonce,
+        );
 
+        let credential_id = credential_thin.id_using_hash(&mut self.trussed, kek, &rp_id_hash)?;
 
         if rk_requested {
+            // Create full credential for the Resident Key usage, and store it in local memory.
+            let credential = Credential::new(
+                credential::CtapVersion::Fido21Pre,
+                &parameters.rp,
+                &parameters.user,
+                algorithm as i32,
+                key_parameter,
+                self.state.persistent.timestamp(&mut self.trussed)?,
+                hmac_secret_requested.clone(),
+                cred_protect_requested,
+                nonce,
+            );
+            // info!("made credential {:?}", &credential);
+            let serialized_credential = credential.serialize()?;
+
             // first delete any other RK cred with same RP + UserId if there is one.
             self.delete_resident_key_by_user_id(&rp_id_hash, &credential.user.id).ok();