diff --git a/src/fido/cbor_config.c b/src/fido/cbor_config.c index 6eb8c11..7ce3f6f 100644 --- a/src/fido/cbor_config.c +++ b/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); diff --git a/src/fido/cbor_cred_mgmt.c b/src/fido/cbor_cred_mgmt.c index 2b072f1..9c40515 100644 --- a/src/fido/cbor_cred_mgmt.c +++ b/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; diff --git a/tests/pico-fido/test_043_credential_metadata.py b/tests/pico-fido/test_043_credential_metadata.py index 25f9b1d..d27882e 100644 --- a/tests/pico-fido/test_043_credential_metadata.py +++ b/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"}], + )