From 633b257e272d04bc2149c44378a395a13e6279eb Mon Sep 17 00:00:00 2001
From: Robin Krahl <robin@nitrokey.com>
Date: Wed, 4 Oct 2023 12:50:12 +0200
Subject: [PATCH] fixup! Reduce ID length for new credentials

---
 src/credential.rs | 18 +++++++---------
 src/ctap1.rs      | 52 +++++++++++++----------------------------------
 2 files changed, 21 insertions(+), 49 deletions(-)

diff --git a/src/credential.rs b/src/credential.rs
index 2f2cd91..1e68dd8 100644
--- a/src/credential.rs
+++ b/src/credential.rs
@@ -433,20 +433,19 @@ impl FullCredential {
 ///
 /// As the credential data is encodeded in the credential ID, we only want to include necessary
 /// data to keep the credential ID as short as possible.
-// TODO: remove ctap, creation_time, use_counter, hmac_secret?
 #[derive(Clone, Debug, serde_indexed::DeserializeIndexed, serde_indexed::SerializeIndexed)]
 pub struct StrippedCredential {
-    ctap: CtapVersion,
-    creation_time: u32,
-    use_counter: bool,
+    pub ctap: CtapVersion,
+    pub creation_time: u32,
+    pub use_counter: bool,
     pub algorithm: i32,
     pub key: Key,
-    nonce: Bytes<12>,
+    pub nonce: Bytes<12>,
     // extensions
     #[serde(skip_serializing_if = "Option::is_none")]
-    hmac_secret: Option<bool>,
+    pub hmac_secret: Option<bool>,
     #[serde(skip_serializing_if = "Option::is_none")]
-    cred_protect: Option<CredentialProtectionPolicy>,
+    pub cred_protect: Option<CredentialProtectionPolicy>,
 }
 
 impl StrippedCredential {
@@ -488,14 +487,13 @@ impl From<&FullCredential> for StrippedCredential {
 #[cfg(test)]
 mod test {
     use super::*;
+    use ctap_types::webauthn::{PublicKeyCredentialRpEntity, PublicKeyCredentialUserEntity};
     use trussed::{
         client::{Chacha8Poly1305, Sha256},
         types::Location,
     };
 
     fn credential_data() -> CredentialData {
-        use ctap_types::webauthn::{PublicKeyCredentialRpEntity, PublicKeyCredentialUserEntity};
-
         CredentialData {
             rp: PublicKeyCredentialRpEntity {
                 id: String::from("John Doe"),
@@ -574,8 +572,6 @@ mod test {
     }
 
     fn random_credential_data() -> CredentialData {
-        use ctap_types::webauthn::{PublicKeyCredentialRpEntity, PublicKeyCredentialUserEntity};
-
         CredentialData {
             rp: PublicKeyCredentialRpEntity {
                 id: random_string(),
diff --git a/src/ctap1.rs b/src/ctap1.rs
index 10b3f77..f3df339 100644
--- a/src/ctap1.rs
+++ b/src/ctap1.rs
@@ -12,7 +12,7 @@ use trussed::{
 
 use crate::{
     constants,
-    credential::{self, Credential, FullCredential, Key},
+    credential::{self, Credential, Key, StrippedCredential},
     SigningAlgorithm, TrussedRequirements, UserPresence,
 };
 
@@ -74,46 +74,23 @@ impl<UP: UserPresence, T: TrussedRequirements> Authenticator for crate::Authenti
                 .to_bytes()
                 .map_err(|_| Error::UnspecifiedCheckingError)?,
         );
-        let nonce = syscall!(self.trussed.random_bytes(12))
-            .bytes
-            .as_slice()
-            .try_into()
-            .unwrap();
-
-        let mut rp_id = heapless::String::new();
-
-        // We do not know the rpId string in U2F.  Just using placeholder.
-        // TODO: Is this true?
-        // <https://fidoalliance.org/specs/fido-v2.1-ps-20210615/fido-client-to-authenticator-protocol-v2.1-ps-20210615.html#cross-version-credentials>
-        rp_id.push_str("u2f").ok();
-        let rp = ctap_types::webauthn::PublicKeyCredentialRpEntity {
-            id: rp_id,
-            name: None,
-            icon: None,
-        };
-
-        let user = ctap_types::webauthn::PublicKeyCredentialUserEntity {
-            id: Bytes::from_slice(&[0u8; 8]).unwrap(),
-            icon: None,
-            name: None,
-            display_name: None,
-        };
+        let nonce = syscall!(self.trussed.random_bytes(12)).bytes;
+        let nonce = Bytes::from_slice(&nonce).unwrap();
 
-        // TODO: create stripped credential?
-        let credential = FullCredential::new(
-            credential::CtapVersion::U2fV2,
-            &rp,
-            &user,
-            SigningAlgorithm::P256 as i32,
-            key,
-            self.state
+        let credential = StrippedCredential {
+            ctap: credential::CtapVersion::U2fV2,
+            creation_time: self
+                .state
                 .persistent
                 .timestamp(&mut self.trussed)
                 .map_err(|_| Error::NotEnoughMemory)?,
-            None,
-            None,
+            use_counter: true,
+            algorithm: SigningAlgorithm::P256 as i32,
+            key,
             nonce,
-        );
+            hmac_secret: None,
+            cred_protect: None,
+        };
 
         // info!("made credential {:?}", &credential);
 
@@ -123,9 +100,8 @@ impl<UP: UserPresence, T: TrussedRequirements> Authenticator for crate::Authenti
             .persistent
             .key_encryption_key(&mut self.trussed)
             .map_err(|_| Error::NotEnoughMemory)?;
-        // TODO: strip?
         let credential_id = credential
-            .id(&mut self.trussed, kek, Some(&reg.app_id))
+            .id(&mut self.trussed, kek, &reg.app_id)
             .map_err(|_| Error::NotEnoughMemory)?;
 
         let mut commitment = Commitment::new();