Merge pull request #224 from kaczmarczyck/store-user-data

Adds user data to stored credentials

src/ctap/data_formats.rs

@@ -56,7 +56,7 @@ impl TryFrom<cbor::Value> for PublicKeyCredentialRpEntity {
 }
 
 // https://www.w3.org/TR/webauthn/#dictdef-publickeycredentialuserentity
-#[cfg_attr(any(test, feature = "debug_ctap"), derive(Debug, PartialEq))]
+#[cfg_attr(any(test, feature = "debug_ctap"), derive(Clone, Debug, PartialEq))]
 pub struct PublicKeyCredentialUserEntity {
     pub user_id: Vec<u8>,
     pub user_name: Option<String>,
@@ -497,10 +497,12 @@ pub struct PublicKeyCredentialSource {
     pub private_key: ecdsa::SecKey, // TODO(kaczmarczyck) open for other algorithms
     pub rp_id: String,
     pub user_handle: Vec<u8>, // not optional, but nullable
-    pub other_ui: Option<String>,
+    pub user_display_name: Option<String>,
     pub cred_random: Option<Vec<u8>>,
     pub cred_protect_policy: Option<CredentialProtectionPolicy>,
     pub creation_order: u64,
+    pub user_name: Option<String>,
+    pub user_icon: Option<String>,
 }
 
 // We serialize credentials for the persistent storage using CBOR maps. Each field of a credential
@@ -510,10 +512,12 @@ enum PublicKeyCredentialSourceField {
     PrivateKey = 1,
     RpId = 2,
     UserHandle = 3,
-    OtherUi = 4,
+    UserDisplayName = 4,
     CredRandom = 5,
     CredProtectPolicy = 6,
     CreationOrder = 7,
+    UserName = 8,
+    UserIcon = 9,
     // When a field is removed, its tag should be reserved and not used for new fields. We document
     // those reserved tags below.
     // Reserved tags: none.
@@ -534,10 +538,12 @@ impl From<PublicKeyCredentialSource> for cbor::Value {
             PublicKeyCredentialSourceField::PrivateKey => Some(private_key.to_vec()),
             PublicKeyCredentialSourceField::RpId => Some(credential.rp_id),
             PublicKeyCredentialSourceField::UserHandle => Some(credential.user_handle),
-            PublicKeyCredentialSourceField::OtherUi => credential.other_ui,
+            PublicKeyCredentialSourceField::UserDisplayName => credential.user_display_name,
             PublicKeyCredentialSourceField::CredRandom => credential.cred_random,
             PublicKeyCredentialSourceField::CredProtectPolicy => credential.cred_protect_policy,
             PublicKeyCredentialSourceField::CreationOrder => credential.creation_order,
+            PublicKeyCredentialSourceField::UserName => credential.user_name,
+            PublicKeyCredentialSourceField::UserIcon => credential.user_icon,
         }
     }
 }
@@ -552,10 +558,12 @@ impl TryFrom<cbor::Value> for PublicKeyCredentialSource {
                 PublicKeyCredentialSourceField::PrivateKey => private_key,
                 PublicKeyCredentialSourceField::RpId => rp_id,
                 PublicKeyCredentialSourceField::UserHandle => user_handle,
-                PublicKeyCredentialSourceField::OtherUi => other_ui,
+                PublicKeyCredentialSourceField::UserDisplayName => user_display_name,
                 PublicKeyCredentialSourceField::CredRandom => cred_random,
                 PublicKeyCredentialSourceField::CredProtectPolicy => cred_protect_policy,
                 PublicKeyCredentialSourceField::CreationOrder => creation_order,
+                PublicKeyCredentialSourceField::UserName => user_name,
+                PublicKeyCredentialSourceField::UserIcon => user_icon,
             } = extract_map(cbor_value)?;
         }
 
@@ -568,12 +576,14 @@ impl TryFrom<cbor::Value> for PublicKeyCredentialSource {
             .ok_or(Ctap2StatusCode::CTAP2_ERR_INVALID_CBOR)?;
         let rp_id = extract_text_string(ok_or_missing(rp_id)?)?;
         let user_handle = extract_byte_string(ok_or_missing(user_handle)?)?;
-        let other_ui = other_ui.map(extract_text_string).transpose()?;
+        let user_display_name = user_display_name.map(extract_text_string).transpose()?;
         let cred_random = cred_random.map(extract_byte_string).transpose()?;
         let cred_protect_policy = cred_protect_policy
             .map(CredentialProtectionPolicy::try_from)
             .transpose()?;
         let creation_order = creation_order.map(extract_unsigned).unwrap_or(Ok(0))?;
+        let user_name = user_name.map(extract_text_string).transpose()?;
+        let user_icon = user_icon.map(extract_text_string).transpose()?;
         // We don't return whether there were unknown fields in the CBOR value. This means that
         // deserialization is not injective. In particular deserialization is only an inverse of
         // serialization at a given version of OpenSK. This is not a problem because:
@@ -590,10 +600,12 @@ impl TryFrom<cbor::Value> for PublicKeyCredentialSource {
             private_key,
             rp_id,
             user_handle,
-            other_ui,
+            user_display_name,
             cred_random,
             cred_protect_policy,
             creation_order,
+            user_name,
+            user_icon,
         })
     }
 }
@@ -1360,10 +1372,12 @@ mod test {
             private_key: crypto::ecdsa::SecKey::gensk(&mut rng),
             rp_id: "example.com".to_string(),
             user_handle: b"foo".to_vec(),
-            other_ui: None,
+            user_display_name: None,
             cred_random: None,
             cred_protect_policy: None,
             creation_order: 0,
+            user_name: None,
+            user_icon: None,
         };
 
         assert_eq!(
@@ -1372,7 +1386,7 @@ mod test {
         );
 
         let credential = PublicKeyCredentialSource {
-            other_ui: Some("other".to_string()),
+            user_display_name: Some("Display Name".to_string()),
             ..credential
         };
 
@@ -1396,6 +1410,26 @@ mod test {
             ..credential
         };
 
+        assert_eq!(
+            PublicKeyCredentialSource::try_from(cbor::Value::from(credential.clone())),
+            Ok(credential.clone())
+        );
+
+        let credential = PublicKeyCredentialSource {
+            user_name: Some("name".to_string()),
+            ..credential
+        };
+
+        assert_eq!(
+            PublicKeyCredentialSource::try_from(cbor::Value::from(credential.clone())),
+            Ok(credential.clone())
+        );
+
+        let credential = PublicKeyCredentialSource {
+            user_icon: Some("icon".to_string()),
+            ..credential
+        };
+
         assert_eq!(
             PublicKeyCredentialSource::try_from(cbor::Value::from(credential.clone())),
             Ok(credential)

src/ctap/mod.rs

@@ -317,10 +317,12 @@ where
             private_key: sk,
             rp_id: String::from(""),
             user_handle: vec![],
-            other_ui: None,
+            user_display_name: None,
             cred_random,
             cred_protect_policy: None,
             creation_order: 0,
+            user_name: None,
+            user_icon: None,
         }))
     }
 
@@ -534,12 +536,18 @@ where
                 user_handle: user.user_id,
                 // This input is user provided, so we crop it to 64 byte for storage.
                 // The UTF8 encoding is always preserved, so the string might end up shorter.
-                other_ui: user
+                user_display_name: user
                     .user_display_name
                     .map(|s| truncate_to_char_boundary(&s, 64).to_string()),
                 cred_random: cred_random.map(|c| c.to_vec()),
                 cred_protect_policy,
                 creation_order: self.persistent_store.new_creation_order()?,
+                user_name: user
+                    .user_name
+                    .map(|s| truncate_to_char_boundary(&s, 64).to_string()),
+                user_icon: user
+                    .user_icon
+                    .map(|s| truncate_to_char_boundary(&s, 64).to_string()),
             };
             self.persistent_store.store_credential(credential_source)?;
             random_id
@@ -651,9 +659,9 @@ where
         let user = if !credential.user_handle.is_empty() {
             Some(PublicKeyCredentialUserEntity {
                 user_id: credential.user_handle,
-                user_name: None,
+                user_name: credential.user_name,
-                user_display_name: credential.other_ui,
+                user_display_name: credential.user_display_name,
-                user_icon: None,
+                user_icon: credential.user_icon,
             })
         } else {
             None
@@ -772,7 +780,9 @@ where
         // Remove user identifiable information without uv.
         if !has_uv {
             for credential in &mut applicable_credentials {
-                credential.other_ui = None;
+                credential.user_name = None;
+                credential.user_display_name = None;
+                credential.user_icon = None;
             }
         }
         applicable_credentials.sort_unstable_by_key(|c| c.creation_order);
@@ -1169,10 +1179,12 @@ mod test {
             private_key: excluded_private_key,
             rp_id: String::from("example.com"),
             user_handle: vec![],
-            other_ui: None,
+            user_display_name: None,
             cred_random: None,
             cred_protect_policy: None,
             creation_order: 0,
+            user_name: None,
+            user_icon: None,
         };
         assert!(ctap_state
             .persistent_store
@@ -1357,9 +1369,10 @@ mod test {
         );
     }
 
-    fn check_assertion_response(
+    fn check_assertion_response_with_user(
         response: Result<ResponseData, Ctap2StatusCode>,
-        expected_user_id: Vec<u8>,
+        expected_user: PublicKeyCredentialUserEntity,
+        flags: u8,
         expected_number_of_credentials: Option<u64>,
     ) {
         match response.unwrap() {
@@ -1373,20 +1386,33 @@ mod test {
                 let expected_auth_data = vec![
                     0xA3, 0x79, 0xA6, 0xF6, 0xEE, 0xAF, 0xB9, 0xA5, 0x5E, 0x37, 0x8C, 0x11, 0x80,
                     0x34, 0xE2, 0x75, 0x1E, 0x68, 0x2F, 0xAB, 0x9F, 0x2D, 0x30, 0xAB, 0x13, 0xD2,
-                    0x12, 0x55, 0x86, 0xCE, 0x19, 0x47, 0x00, 0x00, 0x00, 0x00, 0x01,
+                    0x12, 0x55, 0x86, 0xCE, 0x19, 0x47, flags, 0x00, 0x00, 0x00, 0x01,
                 ];
                 assert_eq!(auth_data, expected_auth_data);
+                assert_eq!(user, Some(expected_user));
+                assert_eq!(number_of_credentials, expected_number_of_credentials);
+            }
+            _ => panic!("Invalid response type"),
+        }
+    }
+
+    fn check_assertion_response(
+        response: Result<ResponseData, Ctap2StatusCode>,
+        expected_user_id: Vec<u8>,
+        expected_number_of_credentials: Option<u64>,
+    ) {
         let expected_user = PublicKeyCredentialUserEntity {
             user_id: expected_user_id,
             user_name: None,
             user_display_name: None,
             user_icon: None,
         };
-                assert_eq!(user, Some(expected_user));
+        check_assertion_response_with_user(
-                assert_eq!(number_of_credentials, expected_number_of_credentials);
+            response,
-            }
+            expected_user,
-            _ => panic!("Invalid response type"),
+            0x00,
-        }
+            expected_number_of_credentials,
+        );
     }
 
     #[test]
@@ -1557,12 +1583,14 @@ mod test {
             private_key: private_key.clone(),
             rp_id: String::from("example.com"),
             user_handle: vec![0x1D],
-            other_ui: None,
+            user_display_name: None,
             cred_random: None,
             cred_protect_policy: Some(
                 CredentialProtectionPolicy::UserVerificationOptionalWithCredentialIdList,
             ),
             creation_order: 0,
+            user_name: None,
+            user_icon: None,
         };
         assert!(ctap_state
             .persistent_store
@@ -1616,10 +1644,12 @@ mod test {
             private_key,
             rp_id: String::from("example.com"),
             user_handle: vec![0x1D],
-            other_ui: None,
+            user_display_name: None,
             cred_random: None,
             cred_protect_policy: Some(CredentialProtectionPolicy::UserVerificationRequired),
             creation_order: 0,
+            user_name: None,
+            user_icon: None,
         };
         assert!(ctap_state
             .persistent_store
@@ -1650,22 +1680,48 @@ mod test {
     }
 
     #[test]
-    fn test_process_get_next_assertion_two_credentials() {
+    fn test_process_get_next_assertion_two_credentials_with_uv() {
         let mut rng = ThreadRng256 {};
+        let key_agreement_key = crypto::ecdh::SecKey::gensk(&mut rng);
+        let pin_uv_auth_token = [0x88; 32];
+        let pin_protocol_v1 = PinProtocolV1::new_test(key_agreement_key, pin_uv_auth_token);
+
         let user_immediately_present = |_| Ok(());
         let mut ctap_state = CtapState::new(&mut rng, user_immediately_present, DUMMY_CLOCK_VALUE);
+        ctap_state.pin_protocol_v1 = pin_protocol_v1;
 
         let mut make_credential_params = create_minimal_make_credential_parameters();
-        make_credential_params.user.user_id = vec![0x01];
+        let user1 = PublicKeyCredentialUserEntity {
+            user_id: vec![0x01],
+            user_name: Some("user1".to_string()),
+            user_display_name: Some("User One".to_string()),
+            user_icon: Some("icon1".to_string()),
+        };
+        make_credential_params.user = user1.clone();
         assert!(ctap_state
             .process_make_credential(make_credential_params, DUMMY_CHANNEL_ID)
             .is_ok());
         let mut make_credential_params = create_minimal_make_credential_parameters();
-        make_credential_params.user.user_id = vec![0x02];
+        let user2 = PublicKeyCredentialUserEntity {
+            user_id: vec![0x02],
+            user_name: Some("user2".to_string()),
+            user_display_name: Some("User Two".to_string()),
+            user_icon: Some("icon2".to_string()),
+        };
+        make_credential_params.user = user2.clone();
         assert!(ctap_state
             .process_make_credential(make_credential_params, DUMMY_CHANNEL_ID)
             .is_ok());
 
+        ctap_state
+            .persistent_store
+            .set_pin_hash(&[0u8; 16])
+            .unwrap();
+        let pin_uv_auth_param = Some(vec![
+            0x6F, 0x52, 0x83, 0xBF, 0x1A, 0x91, 0xEE, 0x67, 0xE9, 0xD4, 0x4C, 0x80, 0x08, 0x79,
+            0x90, 0x8D,
+        ]);
+
         let get_assertion_params = AuthenticatorGetAssertionParameters {
             rp_id: String::from("example.com"),
             client_data_hash: vec![0xCD],
@@ -1673,20 +1729,20 @@ mod test {
             extensions: None,
             options: GetAssertionOptions {
                 up: false,
-                uv: false,
+                uv: true,
             },
-            pin_uv_auth_param: None,
+            pin_uv_auth_param,
-            pin_uv_auth_protocol: None,
+            pin_uv_auth_protocol: Some(1),
         };
         let get_assertion_response = ctap_state.process_get_assertion(
             get_assertion_params,
             DUMMY_CHANNEL_ID,
             DUMMY_CLOCK_VALUE,
         );
-        check_assertion_response(get_assertion_response, vec![0x02], Some(2));
+        check_assertion_response_with_user(get_assertion_response, user2, 0x04, Some(2));
 
         let get_assertion_response = ctap_state.process_get_next_assertion(DUMMY_CLOCK_VALUE);
-        check_assertion_response(get_assertion_response, vec![0x01], None);
+        check_assertion_response_with_user(get_assertion_response, user1, 0x04, None);
 
         let get_assertion_response = ctap_state.process_get_next_assertion(DUMMY_CLOCK_VALUE);
         assert_eq!(
@@ -1696,23 +1752,32 @@ mod test {
     }
 
     #[test]
-    fn test_process_get_next_assertion_three_credentials() {
+    fn test_process_get_next_assertion_three_credentials_no_uv() {
         let mut rng = ThreadRng256 {};
         let user_immediately_present = |_| Ok(());
         let mut ctap_state = CtapState::new(&mut rng, user_immediately_present, DUMMY_CLOCK_VALUE);
 
         let mut make_credential_params = create_minimal_make_credential_parameters();
         make_credential_params.user.user_id = vec![0x01];
+        make_credential_params.user.user_name = Some("removed".to_string());
+        make_credential_params.user.user_display_name = Some("removed".to_string());
+        make_credential_params.user.user_icon = Some("removed".to_string());
         assert!(ctap_state
             .process_make_credential(make_credential_params, DUMMY_CHANNEL_ID)
             .is_ok());
         let mut make_credential_params = create_minimal_make_credential_parameters();
         make_credential_params.user.user_id = vec![0x02];
+        make_credential_params.user.user_name = Some("removed".to_string());
+        make_credential_params.user.user_display_name = Some("removed".to_string());
+        make_credential_params.user.user_icon = Some("removed".to_string());
         assert!(ctap_state
             .process_make_credential(make_credential_params, DUMMY_CHANNEL_ID)
             .is_ok());
         let mut make_credential_params = create_minimal_make_credential_parameters();
         make_credential_params.user.user_id = vec![0x03];
+        make_credential_params.user.user_name = Some("removed".to_string());
+        make_credential_params.user.user_display_name = Some("removed".to_string());
+        make_credential_params.user.user_icon = Some("removed".to_string());
         assert!(ctap_state
             .process_make_credential(make_credential_params, DUMMY_CHANNEL_ID)
             .is_ok());
@@ -1827,10 +1892,12 @@ mod test {
             private_key,
             rp_id: String::from("example.com"),
             user_handle: vec![],
-            other_ui: None,
+            user_display_name: None,
             cred_random: None,
             cred_protect_policy: None,
             creation_order: 0,
+            user_name: None,
+            user_icon: None,
         };
         assert!(ctap_state
             .persistent_store

src/ctap/pin_protocol_v1.rs

@@ -631,6 +631,22 @@ impl PinProtocolV1 {
         }
         Ok(())
     }
+
+    #[cfg(test)]
+    pub fn new_test(
+        key_agreement_key: crypto::ecdh::SecKey,
+        pin_uv_auth_token: [u8; 32],
+    ) -> PinProtocolV1 {
+        PinProtocolV1 {
+            key_agreement_key,
+            pin_uv_auth_token,
+            consecutive_pin_mismatches: 0,
+            #[cfg(feature = "with_ctap2_1")]
+            permissions: 0xFF,
+            #[cfg(feature = "with_ctap2_1")]
+            permissions_rp_id: None,
+        }
+    }
 }
 
 #[cfg(test)]

src/ctap/storage.rs

@@ -719,10 +719,12 @@ mod test {
             private_key,
             rp_id: String::from(rp_id),
             user_handle,
-            other_ui: None,
+            user_display_name: None,
             cred_random: None,
             cred_protect_policy: None,
             creation_order: 0,
+            user_name: None,
+            user_icon: None,
         }
     }
 
@@ -896,12 +898,14 @@ mod test {
             private_key,
             rp_id: String::from("example.com"),
             user_handle: vec![0x00],
-            other_ui: None,
+            user_display_name: None,
             cred_random: None,
             cred_protect_policy: Some(
                 CredentialProtectionPolicy::UserVerificationOptionalWithCredentialIdList,
             ),
             creation_order: 0,
+            user_name: None,
+            user_icon: None,
         };
         assert!(persistent_store.store_credential(credential).is_ok());
 
@@ -940,10 +944,12 @@ mod test {
             private_key: key0,
             rp_id: String::from("example.com"),
             user_handle: vec![0x00],
-            other_ui: None,
+            user_display_name: None,
             cred_random: None,
             cred_protect_policy: None,
             creation_order: 0,
+            user_name: None,
+            user_icon: None,
         };
         assert_eq!(found_credential, Some(expected_credential));
     }
@@ -960,10 +966,12 @@ mod test {
             private_key,
             rp_id: String::from("example.com"),
             user_handle: vec![0x00],
-            other_ui: None,
+            user_display_name: None,
             cred_random: None,
             cred_protect_policy: Some(CredentialProtectionPolicy::UserVerificationRequired),
             creation_order: 0,
+            user_name: None,
+            user_icon: None,
         };
         assert!(persistent_store.store_credential(credential).is_ok());
 
@@ -1138,10 +1146,12 @@ mod test {
             private_key,
             rp_id: String::from("example.com"),
             user_handle: vec![0x00],
-            other_ui: None,
+            user_display_name: None,
             cred_random: None,
             cred_protect_policy: None,
             creation_order: 0,
+            user_name: None,
+            user_icon: None,
         };
         let serialized = serialize_credential(credential.clone()).unwrap();
         let reconstructed = deserialize_credential(&serialized).unwrap();