Extend credential management tests

This patch adds tests for deleting discoverable credentials and for
updating the user information for existing credentials.

tests/authenticator/mod.rs

@@ -4,8 +4,8 @@ use super::{
     virt::{Ctap2, Ctap2Error},
     webauthn::{
         AttStmtFormat, ClientPin, CredentialData, CredentialManagement, CredentialManagementParams,
-        KeyAgreementKey, MakeCredential, MakeCredentialOptions, PinToken, PubKeyCredParam,
-        PublicKey, Rp, SharedSecret, User,
+        KeyAgreementKey, MakeCredential, MakeCredentialOptions, PinToken, PubKeyCredDescriptor,
+        PubKeyCredParam, PublicKey, Rp, SharedSecret, User,
     },
 };
 
@@ -142,11 +142,12 @@ impl Authenticator<'_, Pin> {
         rps
     }
 
-    pub fn list_credentials(&mut self, rp_id: &str) -> Vec<User> {
+    pub fn list_credentials(&mut self, rp_id: &str) -> Vec<(User, PubKeyCredDescriptor)> {
         let rp_id_hash = rp_id_hash(rp_id);
         let pin_token = self.get_pin_token(0x04, Some(rp_id.to_owned()));
         let params = CredentialManagementParams {
-            rp_id_hash: rp_id_hash.to_vec(),
+            rp_id_hash: Some(rp_id_hash.to_vec()),
+            ..Default::default()
         };
         let mut pin_auth_param = vec![0x04];
         pin_auth_param.extend_from_slice(&params.serialized());
@@ -161,17 +162,54 @@ impl Authenticator<'_, Pin> {
         // TODO: check other fields
         let total_credentials = reply.total_credentials.unwrap();
         let mut credentials = Vec::with_capacity(total_credentials);
-        credentials.push(reply.user.unwrap().into());
+        credentials.push((reply.user.unwrap().into(), reply.credential_id.unwrap()));
 
         for _ in 1..total_credentials {
             let request = CredentialManagement::new(0x05);
             let reply = self.ctap2.exec(request).unwrap();
             // TODO: check other fields
-            credentials.push(reply.user.unwrap().into());
+            credentials.push((reply.user.unwrap().into(), reply.credential_id.unwrap()));
         }
 
         credentials
     }
+
+    pub fn delete_credential(&mut self, id: &[u8]) {
+        let pin_token = self.get_pin_token(0x04, None);
+        let params = CredentialManagementParams {
+            credential_id: Some(PubKeyCredDescriptor::new("public-key", id)),
+            ..Default::default()
+        };
+        let mut pin_auth_param = vec![0x06];
+        pin_auth_param.extend_from_slice(&params.serialized());
+        let pin_auth = pin_token.authenticate(&pin_auth_param);
+        let request = CredentialManagement {
+            subcommand: 0x06,
+            subcommand_params: Some(params),
+            pin_protocol: Some(2),
+            pin_auth: Some(pin_auth),
+        };
+        self.ctap2.exec(request).unwrap();
+    }
+
+    pub fn update_user(&mut self, id: &[u8], user: User) -> Result<(), Ctap2Error> {
+        let pin_token = self.get_pin_token(0x04, None);
+        let params = CredentialManagementParams {
+            credential_id: Some(PubKeyCredDescriptor::new("public-key", id)),
+            user: Some(user),
+            ..Default::default()
+        };
+        let mut pin_auth_param = vec![0x07];
+        pin_auth_param.extend_from_slice(&params.serialized());
+        let pin_auth = pin_token.authenticate(&pin_auth_param);
+        let request = CredentialManagement {
+            subcommand: 0x07,
+            subcommand_params: Some(params),
+            pin_protocol: Some(2),
+            pin_auth: Some(pin_auth),
+        };
+        self.ctap2.exec(request).map(|_| ())
+    }
 }
 
 pub struct CredentialsMetadata {

tests/basic.rs

@@ -457,7 +457,8 @@ impl TestListCredentials {
                 pin_token_rp_id,
             );
             let params = CredentialManagementParams {
-                rp_id_hash: reply.rp_id_hash.unwrap().as_bytes().unwrap().to_owned(),
+                rp_id_hash: Some(reply.rp_id_hash.unwrap().as_bytes().unwrap().to_owned()),
+                ..Default::default()
             };
             let mut pin_auth_param = vec![0x04];
             pin_auth_param.extend_from_slice(&params.serialized());

tests/cred_mgmt.rs

@@ -4,45 +4,208 @@ pub mod authenticator;
 pub mod virt;
 pub mod webauthn;
 
-use std::collections::BTreeMap;
+use std::collections::BTreeSet;
 
 use littlefs2::path::PathBuf;
 
-use authenticator::Authenticator;
+use authenticator::{Authenticator, Pin};
 use virt::{Ctap2Error, Options};
-use webauthn::{Rp, User};
+use webauthn::{CredentialData, PubKeyCredDescriptor, Rp, User};
+
+struct CredMgmt<'a> {
+    authenticator: Authenticator<'a, Pin>,
+    credentials: Vec<(Rp, User, CredentialData)>,
+}
+
+impl<'a> CredMgmt<'a> {
+    fn new(authenticator: Authenticator<'a, Pin>) -> Self {
+        Self {
+            authenticator,
+            credentials: Default::default(),
+        }
+    }
+
+    fn make_credential(&mut self, rp: Rp, user: User) -> Result<CredentialData, Ctap2Error> {
+        self.authenticator
+            .make_credential(rp.clone(), user.clone())
+            .inspect(|credential_data| {
+                self.credentials.push((rp, user, credential_data.clone()));
+            })
+    }
+
+    fn delete_credential(&mut self, id: Vec<u8>) -> Result<(), Ctap2Error> {
+        self.authenticator.delete_credential(&id);
+        self.credentials.retain(|(_, _, data)| data.id != id);
+        Ok(())
+    }
+
+    fn delete_credential_at(&mut self, i: usize) -> Result<(), Ctap2Error> {
+        assert!(i < self.credentials.len());
+        let id = self.credentials[i].2.id.clone();
+        self.delete_credential(id)
+    }
+
+    fn update_user(&mut self, id: Vec<u8>, user: User) -> Result<(), Ctap2Error> {
+        self.authenticator.update_user(&id, user.clone())?;
+        self.credentials
+            .iter_mut()
+            .filter(|(_, _, data)| data.id == id)
+            .for_each(|cred| cred.1 = user.clone());
+        Ok(())
+    }
+
+    fn update_user_at(&mut self, i: usize, user: User) -> Result<(), Ctap2Error> {
+        assert!(i < self.credentials.len());
+        let id = self.credentials[i].2.id.clone();
+        self.update_user(id, user)
+    }
+
+    fn list(&mut self) {
+        let expected_rp_ids = self.rp_ids();
+        let actual_rps = self.authenticator.list_rps();
+        let actual_rp_ids: BTreeSet<_> = actual_rps.iter().map(|rp| rp.id.clone()).collect();
+        assert_eq!(expected_rp_ids, actual_rp_ids);
+        // TODO: check other RP fields than ID
+
+        for rp_id in expected_rp_ids {
+            assert!(actual_rps.iter().any(|rp| rp.id == rp_id));
+            let expected_credentials = self.credentials(&rp_id);
+            let actual_credentials = self.authenticator.list_credentials(&rp_id);
+            let actual_credentials: BTreeSet<_> = actual_credentials.into_iter().collect();
+            assert_eq!(expected_credentials, actual_credentials);
+        }
+    }
+
+    fn rp_ids(&self) -> BTreeSet<String> {
+        self.credentials
+            .iter()
+            .map(|(rp, _, _)| rp.id.clone())
+            .collect()
+    }
+
+    fn credentials(&self, rp_id: &str) -> BTreeSet<(User, PubKeyCredDescriptor)> {
+        self.credentials
+            .iter()
+            .filter(|(rp, _, _)| rp.id == rp_id)
+            .map(|(_, user, data)| {
+                (
+                    user.clone(),
+                    PubKeyCredDescriptor::new("public-key", data.id.clone()),
+                )
+            })
+            .collect()
+    }
+}
+
+fn generate_rp(i: usize) -> Rp {
+    // TODO: set other fields than id
+    let rp_id = format!("rp{i}");
+    Rp::new(rp_id)
+}
+
+fn generate_user(i: u8) -> User {
+    // TODO: set other fields than id
+    let mut user = Vec::from(b"john.doe");
+    user.push(i);
+    User::new(user)
+}
 
 #[test]
 fn test_list_credentials() {
     virt::run_ctap2(|device| {
-        let mut authenticator = Authenticator::new(device).set_pin(b"123456");
-        let mut credentials: BTreeMap<_, _> = (0..10)
-            .map(|i| {
-                // TODO: set other fields than id
-                let rp_id = format!("rp{i}");
-                let user = b"john.doe";
-                authenticator
-                    .make_credential(Rp::new(rp_id.clone()), User::new(user))
-                    .unwrap();
-                (rp_id, user)
-            })
-            .collect();
-
-        let rps = authenticator.list_rps();
-        assert_eq!(rps.len(), 10);
-        for rp in &rps {
-            assert_eq!(rp.name, None);
-            let expected = credentials.remove(&rp.id).unwrap();
-
-            let mut credentials = authenticator.list_credentials(&rp.id);
-            assert_eq!(credentials.len(), 1);
-            let actual = credentials.pop().unwrap();
-
-            assert_eq!(actual.id, expected);
-            assert_eq!(actual.name, None);
-            assert_eq!(actual.display_name, None);
+        let authenticator = Authenticator::new(device).set_pin(b"123456");
+        let mut cred_mgmt = CredMgmt::new(authenticator);
+        for i in 0..10 {
+            let rp = generate_rp(i);
+            let user = generate_user(0);
+            cred_mgmt.make_credential(rp, user).unwrap();
         }
-        assert!(credentials.is_empty());
+
+        cred_mgmt.list();
+    })
+}
+
+#[test]
+fn test_list_credentials_multi() {
+    virt::run_ctap2(|device| {
+        let authenticator = Authenticator::new(device).set_pin(b"123456");
+        let mut cred_mgmt = CredMgmt::new(authenticator);
+        for (i, n) in [1, 3, 1, 3, 2].into_iter().enumerate() {
+            let rp = generate_rp(i);
+            for j in 0..n {
+                let user = generate_user(j);
+                cred_mgmt.make_credential(rp.clone(), user).unwrap();
+            }
+        }
+
+        cred_mgmt.list();
+    })
+}
+
+#[test]
+fn test_list_credentials_delete() {
+    virt::run_ctap2(|device| {
+        let authenticator = Authenticator::new(device).set_pin(b"123456");
+        let mut cred_mgmt = CredMgmt::new(authenticator);
+        for (i, n) in [1, 3, 1, 3, 2].into_iter().enumerate() {
+            let rp = generate_rp(i);
+            for j in 0..n {
+                let user = generate_user(j);
+                cred_mgmt.make_credential(rp.clone(), user).unwrap();
+            }
+        }
+
+        // deletes the only credential for rp2
+        cred_mgmt.delete_credential_at(4).unwrap();
+        // deletes one of three credentials for rp1
+        cred_mgmt.delete_credential_at(2).unwrap();
+
+        cred_mgmt.list();
+    })
+}
+
+#[test]
+fn test_list_credentials_update_user() {
+    virt::run_ctap2(|device| {
+        let authenticator = Authenticator::new(device).set_pin(b"123456");
+        let mut cred_mgmt = CredMgmt::new(authenticator);
+        for (i, n) in [1, 3, 1, 3, 2].into_iter().enumerate() {
+            let rp = generate_rp(i);
+            for j in 0..n {
+                let user = generate_user(j);
+                cred_mgmt.make_credential(rp.clone(), user).unwrap();
+            }
+        }
+
+        // case 1: updates the only credential for rp2
+
+        // changing the user ID fails
+        let user = generate_user(98);
+        assert_eq!(cred_mgmt.update_user_at(4, user), Err(Ctap2Error(0x02)));
+
+        cred_mgmt.list();
+
+        // setting the display name works
+        let mut user = generate_user(0);
+        user.display_name = Some("John Doe".into());
+        cred_mgmt.update_user_at(4, user).unwrap();
+
+        cred_mgmt.list();
+
+        // case 2: updates one of three credentials for rp1
+
+        // changing the user ID fails
+        let user = generate_user(99);
+        assert_eq!(cred_mgmt.update_user_at(2, user), Err(Ctap2Error(0x02)));
+
+        cred_mgmt.list();
+
+        // setting the display name works
+        let mut user = generate_user(1);
+        user.display_name = Some("John Doe".into());
+        cred_mgmt.update_user_at(2, user).unwrap();
+
+        cred_mgmt.list();
     })
 }
 
@@ -137,3 +300,44 @@ fn test_filesystem_full() {
         assert_eq!(metadata.remaining, 0);
     })
 }
+
+#[test]
+fn test_filesystem_full_update_user() {
+    let mut options = Options {
+        max_resident_credential_count: Some(10),
+        ..Default::default()
+    };
+    for i in 0..80 {
+        let path = PathBuf::try_from(format!("/test/{i}").as_str()).unwrap();
+        options.files.push((path, vec![0; 512]));
+    }
+    // TODO: inspect filesystem after run and check remaining blocks
+    virt::run_ctap2_with_options(options, |device| {
+        let authenticator = Authenticator::new(device).set_pin(b"123456");
+        let mut cred_mgmt = CredMgmt::new(authenticator);
+
+        let mut i = 0;
+        loop {
+            let rp = generate_rp(i);
+            let user = generate_user(0);
+            let result = cred_mgmt.make_credential(rp, user);
+
+            if result == Err(Ctap2Error(0x28)) {
+                break;
+            }
+            result.unwrap();
+
+            i += 1;
+        }
+
+        cred_mgmt.list();
+
+        // filesystem is now full, we cannot create new credentials
+        // but: we still want to be able to update existing credentials
+        let mut user = generate_user(0);
+        user.display_name = Some("John Doe".into());
+        cred_mgmt.update_user_at(1, user).unwrap();
+
+        cred_mgmt.list();
+    })
+}

tests/webauthn/mod.rs

@@ -203,6 +203,7 @@ impl From<Value> for ClientPinReply {
     }
 }
 
+#[derive(Clone, Eq, Ord, PartialEq, PartialOrd)]
 pub struct Rp {
     pub id: String,
     pub name: Option<String>,
@@ -245,6 +246,7 @@ impl From<Value> for Rp {
     }
 }
 
+#[derive(Clone, Debug, Eq, Ord, PartialEq, PartialOrd)]
 pub struct User {
     pub id: Vec<u8>,
     pub name: Option<String>,
@@ -516,6 +518,7 @@ impl From<Value> for MakeCredentialReply {
     }
 }
 
+#[derive(Clone, Debug, Eq, Ord, PartialEq, PartialOrd)]
 pub struct PubKeyCredDescriptor {
     pub ty: String,
     pub id: Vec<u8>,
@@ -533,8 +536,8 @@ impl PubKeyCredDescriptor {
 impl From<PubKeyCredDescriptor> for Value {
     fn from(descriptor: PubKeyCredDescriptor) -> Value {
         let mut map = Map::default();
-        map.push("type", descriptor.ty);
         map.push("id", descriptor.id);
+        map.push("type", descriptor.ty);
         map.into()
     }
 }
@@ -645,7 +648,7 @@ impl From<Value> for AuthData {
     }
 }
 
-#[derive(Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq)]
 pub struct CredentialData {
     pub id: Vec<u8>,
     pub public_key: BTreeMap<i32, Value>,
@@ -780,9 +783,11 @@ impl Request for CredentialManagement {
     type Reply = CredentialManagementReply;
 }
 
-#[derive(Clone)]
+#[derive(Clone, Default)]
 pub struct CredentialManagementParams {
-    pub rp_id_hash: Vec<u8>,
+    pub rp_id_hash: Option<Vec<u8>>,
+    pub credential_id: Option<PubKeyCredDescriptor>,
+    pub user: Option<User>,
 }
 
 impl CredentialManagementParams {
@@ -796,7 +801,15 @@ impl CredentialManagementParams {
 impl From<CredentialManagementParams> for Value {
     fn from(params: CredentialManagementParams) -> Value {
         let mut map = Map::default();
-        map.push(1, params.rp_id_hash);
+        if let Some(rp_id_hash) = params.rp_id_hash {
+            map.push(1, rp_id_hash);
+        }
+        if let Some(credential_id) = params.credential_id {
+            map.push(2, credential_id);
+        }
+        if let Some(user) = params.user {
+            map.push(3, user);
+        }
         map.into()
     }
 }
@@ -808,6 +821,7 @@ pub struct CredentialManagementReply {
     pub rp_id_hash: Option<Value>,
     pub total_rps: Option<usize>,
     pub user: Option<Value>,
+    pub credential_id: Option<PubKeyCredDescriptor>,
     pub total_credentials: Option<usize>,
     pub third_party_payment: Option<bool>,
 }
@@ -826,6 +840,7 @@ impl From<Value> for CredentialManagementReply {
             rp_id_hash: map.remove(&4),
             total_rps: map.remove(&5).map(|value| value.deserialized().unwrap()),
             user: map.remove(&6),
+            credential_id: map.remove(&7).map(|value| value.into()),
             total_credentials: map.remove(&9).map(|value| value.deserialized().unwrap()),
             third_party_payment: map.remove(&0x0c).map(|value| value.deserialized().unwrap()),
         }