Do not abort on revoked cred

Signed-off-by: Pol Henarejos <pol.henarejos@cttc.es>

src/fido/cbor_config.c

@@ -261,7 +261,7 @@ int cbor_config(const uint8_t *data, size_t len) {
         }
         else if (vendorCommandId == CTAP_CONFIG_CREDENTIAL_REVOKE) {
             // Keep the legacy slot form (0x03) and accept a resident credential ID in 0x02.
-            bool by_id = vendorParamByteString.present && !vendorParamIntPresent;
+            bool by_id = vendorParamByteString.present && vendorParamByteString.len == CRED_RESIDENT_LEN && !vendorParamIntPresent;
             bool by_slot = vendorParamIntPresent && !vendorParamByteString.present;
             if (!vendorCommandIdPresent || (!by_id && !by_slot)) {
                 CBOR_ERROR(CTAP1_ERR_INVALID_PARAMETER);
@@ -278,7 +278,7 @@ int cbor_config(const uint8_t *data, size_t len) {
         }
         else if (vendorCommandId == CTAP_CONFIG_CREDENTIAL_EXPIRE) {
             // Slot form: 0x03=slot, 0x02=4-byte timestamp. ID form: 0x02=ID, 0x03=timestamp.
-            bool by_id = vendorParamByteString.present && vendorParamByteString.len != sizeof(uint32_t) && vendorParamIntPresent && vendorParamInt <= UINT32_MAX;
+            bool by_id = vendorParamByteString.present && vendorParamByteString.len == CRED_RESIDENT_LEN && vendorParamIntPresent && vendorParamInt <= UINT32_MAX;
             bool by_slot = vendorParamIntPresent && vendorParamByteString.present && vendorParamByteString.len == sizeof(uint32_t);
             if (!vendorCommandIdPresent || (!by_id && !by_slot)) {
                 CBOR_ERROR(CTAP1_ERR_INVALID_PARAMETER);

src/fido/cbor_cred_mgmt.c

@@ -278,6 +278,15 @@ int cbor_cred_mgmt(const uint8_t *data, size_t len) {
         for (int i = 0; i < MAX_RESIDENT_CREDENTIALS; i++) {
             file_t *tef = file_search((uint16_t)(EF_CRED + i));
             if (file_has_data(tef) && credential_resident_matches_rp(tef, rpIdHash.data)) {
+                Credential candidate = { 0 };
+                int candidate_ret = credential_load_resident(tef, rpIdHash.data, &candidate);
+                credential_free(&candidate);
+                if (candidate_ret == CTAP2_ERR_NO_CREDENTIALS) {
+                    continue;
+                }
+                if (candidate_ret != 0) {
+                    CBOR_ERROR(CTAP2_ERR_NOT_ALLOWED);
+                }
                 if (++skip == cred_counter) {
                     if (cred_ef == NULL) {
                         cred_ef = tef;

tests/pico-fido/test_043_credential_metadata.py

@@ -2,7 +2,7 @@
 
 import pytest
 from fido2.ctap import CtapError
-from fido2.ctap2 import Config
+from fido2.ctap2 import Config, CredentialManagement
 from fido2.ctap2.pin import ClientPin, PinProtocolV2
 
 
@@ -25,6 +25,12 @@ def _set_pin_and_reset(device):
     ClientPin(device.client()._backend.ctap2).set_pin(PIN)
 
 
+def _credential_management(device):
+    ctap = device.client()._backend.ctap2
+    token = ClientPin(ctap).get_pin_token(PIN, permissions=ClientPin.PERMISSION.CREDENTIAL_MGMT)
+    return CredentialManagement(ctap, PinProtocolV2(), token)
+
+
 def test_get_info_advertises_credential_metadata_commands(info):
     commands = info[0x15]
     assert CONFIG_CREDENTIAL_EXPIRE in commands
@@ -122,3 +128,34 @@ def test_revoke_resident_credential_blocks_assertion(device):
     with pytest.raises(CtapError) as error:
         device.doGA(rp_id=rp["id"])
     assert error.value.code == CtapError.ERR.NO_CREDENTIALS
+
+
+def test_revoke_does_not_hide_other_credentials(device):
+    _set_pin_and_reset(device)
+    rp = {"id": "credential-metadata-multiple.example", "name": "Credential Metadata"}
+    first = device.doMC(rp=rp, rk=True, user={"id": b"first", "name": "first"})["res"].attestation_object
+    second = device.doMC(rp=rp, rk=True, user={"id": b"second", "name": "second"})["res"].attestation_object
+
+    management = _credential_management(device)
+    credentials = management.enumerate_creds(first.auth_data.rp_id_hash)
+    assert len(credentials) == 2
+    target = next(
+        credential for credential in credentials
+        if credential[CredentialManagement.RESULT.USER]["id"] == b"first"
+    )
+    target_id = target[CredentialManagement.RESULT.CREDENTIAL_ID]["id"]
+
+    _vendor_config(device)._call(
+        Config.CMD.VENDOR_PROTOTYPE,
+        {0x01: CONFIG_CREDENTIAL_REVOKE, 0x02: target_id},
+    )
+
+    management = _credential_management(device)
+    remaining = management.enumerate_creds(first.auth_data.rp_id_hash)
+    assert len(remaining) == 1
+    assert remaining[0][CredentialManagement.RESULT.USER]["id"] == b"second"
+    assert remaining[0][CredentialManagement.RESULT.CREDENTIAL_ID]["id"] != target_id
+    device.doGA(
+        rp_id=rp["id"],
+        allow_list=[{"id": second.auth_data.credential_data.credential_id, "type": "public-key"}],
+    )