Major refactor on resident keys.

Now, credential ids have shorter and fixed length (40) to avoid issues with some servers, which have maximum credential id length constraints.

Fixes #184

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

src/fido/cbor_cred_mgmt.c

@@ -259,7 +259,7 @@ int cbor_cred_mgmt(const uint8_t *data, size_t len) {
         }
 
         Credential cred = { 0 };
-        if (credential_load(file_get_data(cred_ef) + 32, file_get_size(cred_ef) - 32, rpIdHash.data, &cred) != 0) {
+        if (credential_load(file_get_data(cred_ef) + 32 + CRED_RESIDENT_LEN, file_get_size(cred_ef) - 32 - CRED_RESIDENT_LEN, rpIdHash.data, &cred) != 0) {
             CBOR_ERROR(CTAP2_ERR_NOT_ALLOWED);
         }
 
@@ -316,7 +316,9 @@ int cbor_cred_mgmt(const uint8_t *data, size_t len) {
         CBOR_CHECK(cbor_encode_uint(&mapEncoder, 0x07));
         CBOR_CHECK(cbor_encoder_create_map(&mapEncoder, &mapEncoder2, 2));
         CBOR_CHECK(cbor_encode_text_stringz(&mapEncoder2, "id"));
-        CBOR_CHECK(cbor_encode_byte_string(&mapEncoder2, cred.id.data, cred.id.len));
+        uint8_t cred_idr[CRED_RESIDENT_LEN] = {0};
+        credential_derive_resident(cred.id.data, cred.id.len, cred_idr);
+        CBOR_CHECK(cbor_encode_byte_string(&mapEncoder2, cred_idr, sizeof(cred_idr)));
         CBOR_CHECK(cbor_encode_text_stringz(&mapEncoder2, "type"));
         CBOR_CHECK(cbor_encode_text_stringz(&mapEncoder2, "public-key"));
         CBOR_CHECK(cbor_encoder_close_container(&mapEncoder, &mapEncoder2));
@@ -372,7 +374,7 @@ int cbor_cred_mgmt(const uint8_t *data, size_t len) {
         }
         for (int i = 0; i < MAX_RESIDENT_CREDENTIALS; i++) {
             file_t *ef = search_dynamic_file((uint16_t)(EF_CRED + i));
-            if (file_has_data(ef) && memcmp(file_get_data(ef) + 32, credentialId.id.data, MIN(file_get_size(ef) - 32, credentialId.id.len)) == 0) {
+            if (file_has_data(ef) && memcmp(file_get_data(ef) + 32, credentialId.id.data, CRED_RESIDENT_LEN) == 0) {
                 uint8_t *rp_id_hash = file_get_data(ef);
                 if (delete_file(ef) != 0) {
                     CBOR_ERROR(CTAP2_ERR_NOT_ALLOWED);
@@ -414,10 +416,10 @@ int cbor_cred_mgmt(const uint8_t *data, size_t len) {
         }
         for (int i = 0; i < MAX_RESIDENT_CREDENTIALS; i++) {
             file_t *ef = search_dynamic_file((uint16_t)(EF_CRED + i));
-            if (file_has_data(ef) && memcmp(file_get_data(ef) + 32, credentialId.id.data, MIN(file_get_size(ef) - 32, credentialId.id.len)) == 0) {
+            if (file_has_data(ef) && memcmp(file_get_data(ef) + 32, credentialId.id.data, CRED_RESIDENT_LEN) == 0) {
                 Credential cred = { 0 };
                 uint8_t *rp_id_hash = file_get_data(ef);
-                if (credential_load(rp_id_hash + 32, file_get_size(ef) - 32, rp_id_hash, &cred) != 0) {
+                if (credential_load(rp_id_hash + 32 + CRED_RESIDENT_LEN, file_get_size(ef) - 32 - CRED_RESIDENT_LEN, rp_id_hash, &cred) != 0) {
                     CBOR_ERROR(CTAP2_ERR_NOT_ALLOWED);
                 }
                 if (memcmp(user.id.data, cred.userId.data, MIN(user.id.len, cred.userId.len)) != 0) {
@@ -427,9 +429,9 @@ int cbor_cred_mgmt(const uint8_t *data, size_t len) {
                 uint8_t newcred[MAX_CRED_ID_LENGTH];
                 size_t newcred_len = 0;
                 if (credential_create(&cred.rpId, &cred.userId, &user.parent.name,
-                                      &user.displayName, &cred.opts, &cred.extensions,
+                                    &user.displayName, &cred.opts, &cred.extensions,
-                                      cred.use_sign_count, (int)cred.alg,
+                                    cred.use_sign_count, (int)cred.alg,
-                                      (int)cred.curve, newcred, &newcred_len) != 0) {
+                                    (int)cred.curve, newcred, &newcred_len) != 0) {
                     credential_free(&cred);
                     CBOR_ERROR(CTAP2_ERR_NOT_ALLOWED);
                 }

src/fido/cbor_get_assertion.c

@@ -295,27 +295,48 @@ int cbor_get_assertion(const uint8_t *data, size_t len, bool next) {
                 if (strcmp(allowList[e].type.data, "public-key") != 0) {
                     continue;
                 }
-                if (credential_load(allowList[e].id.data, allowList[e].id.len, rp_id_hash, &creds[creds_len]) != 0) {
+                if (credential_is_resident(allowList[e].id.data, allowList[e].id.len)) {
-                    credential_free(&creds[creds_len]);
+                    for (int i = 0; i < MAX_RESIDENT_CREDENTIALS && creds_len < MAX_CREDENTIAL_COUNT_IN_LIST; i++) {
-                }
+                        file_t *ef = search_dynamic_file((uint16_t)(EF_CRED + i));
-                else {
+                        if (!file_has_data(ef) || memcmp(file_get_data(ef), rp_id_hash, 32) != 0) {
-                    creds_len++;
+                            continue;
-                    silent = false; // If we are able to load a credential, we are not silent
+                        }
-                    // Even we provide allowList, we need to check if the credential is resident
+                        if (memcmp(file_get_data(ef) + 32, allowList[e].id.data, CRED_RESIDENT_LEN) == 0) {
-                    if (!resident) {
+                            if (credential_load(file_get_data(ef) + 32 + CRED_RESIDENT_LEN, file_get_size(ef) - 32 - CRED_RESIDENT_LEN, rp_id_hash, &creds[creds_len]) != 0) {
-                        for (int i = 0; i < MAX_RESIDENT_CREDENTIALS && creds_len < MAX_CREDENTIAL_COUNT_IN_LIST; i++) {
+                                // Should never happen
-                            file_t *ef = search_dynamic_file((uint16_t)(EF_CRED + i));
+                                credential_free(&creds[creds_len]);
-                            if (!file_has_data(ef) || memcmp(file_get_data(ef), rp_id_hash, 32) != 0) {
                                 continue;
                             }
-                            if (memcmp(file_get_data(ef) + 32, allowList[e].id.data, allowList[e].id.len) == 0) {
+                            resident = true;
-                                resident = true;
+                            creds_len++;
+                            silent = false; // If we are able to load a credential, we are not silent
+                            break;
+                        }
+                    }
+                }
+                else {
+                    if (credential_load(allowList[e].id.data, allowList[e].id.len, rp_id_hash, &creds[creds_len]) != 0) {
+                        credential_free(&creds[creds_len]);
+                    }
+                    else {
+                        creds_len++;
+                        silent = false; // If we are able to load a credential, we are not silent
+                        // Even we provide allowList, we need to check if the credential is resident
+                        if (!resident) {
+                            for (int i = 0; i < MAX_RESIDENT_CREDENTIALS && creds_len < MAX_CREDENTIAL_COUNT_IN_LIST; i++) {
+                                file_t *ef = search_dynamic_file((uint16_t)(EF_CRED + i));
+                                if (!file_has_data(ef) || memcmp(file_get_data(ef), rp_id_hash, 32) != 0) {
+                                    continue;
+                                }
+                                if (memcmp(file_get_data(ef) + 32, allowList[e].id.data, allowList[e].id.len) == 0) {
+                                    resident = true;
+                                    break;
+                                }
+                            }
+                            if (resident) {
                                 break;
                             }
                         }
-                        if (resident) {
-                            break;
-                        }
                     }
                 }
             }
@@ -326,7 +347,7 @@ int cbor_get_assertion(const uint8_t *data, size_t len, bool next) {
                 if (!file_has_data(ef) || memcmp(file_get_data(ef), rp_id_hash, 32) != 0) {
                     continue;
                 }
-                int ret = credential_load(file_get_data(ef) + 32, file_get_size(ef) - 32, rp_id_hash,  &creds[creds_len]);
+                int ret = credential_load(file_get_data(ef) + 32 + CRED_RESIDENT_LEN, file_get_size(ef) - 32 - CRED_RESIDENT_LEN, rp_id_hash, &creds[creds_len]);
                 if (ret != 0) {
                     credential_free(&creds[creds_len]);
                 }
@@ -343,8 +364,7 @@ int cbor_get_assertion(const uint8_t *data, size_t len, bool next) {
                     if (creds[i].extensions.credProtect == CRED_PROT_UV_REQUIRED && !(flags & FIDO2_AUT_FLAG_UV)) {
                         credential_free(&creds[i]);
                     }
-                    else if (creds[i].extensions.credProtect == CRED_PROT_UV_OPTIONAL_WITH_LIST &&
+                    else if (creds[i].extensions.credProtect == CRED_PROT_UV_OPTIONAL_WITH_LIST && resident == true && !(flags & FIDO2_AUT_FLAG_UV)) {
-                             resident == true && !(flags & FIDO2_AUT_FLAG_UV)) {
                         credential_free(&creds[i]);
                     }
                     else {
@@ -427,10 +447,16 @@ int cbor_get_assertion(const uint8_t *data, size_t len, bool next) {
         }
         else {
             selcred = &creds[0];
+            if (resident && allowList_len > 1) {
+                numberOfCredentials = 1;
+            }
             if (numberOfCredentials > 1) {
                 asserted = true;
                 residentx = resident;
                 for (int i = 0; i < MAX_CREDENTIAL_COUNT_IN_LIST; i++) {
+                    credential_free(&credsx[i]);
+                }
+                for (int i = 0; i < numberOfCredentials; i++) {
                     credsx[i] = creds[i];
                 }
                 numberOfCredentialsx = numberOfCredentials;
@@ -633,7 +659,14 @@ int cbor_get_assertion(const uint8_t *data, size_t len, bool next) {
     CBOR_CHECK(cbor_encoder_create_map(&mapEncoder, &mapEncoder2, 2));
     CBOR_CHECK(cbor_encode_text_stringz(&mapEncoder2, "id"));
     if (selcred) {
-        CBOR_CHECK(cbor_encode_byte_string(&mapEncoder2, selcred->id.data, selcred->id.len));
+        if (resident) {
+            uint8_t cred_idr[CRED_RESIDENT_LEN] = {0};
+            credential_derive_resident(selcred->id.data, selcred->id.len, cred_idr);
+            CBOR_CHECK(cbor_encode_byte_string(&mapEncoder2, cred_idr, sizeof(cred_idr)));
+        }
+        else {
+            CBOR_CHECK(cbor_encode_byte_string(&mapEncoder2, selcred->id.data, selcred->id.len));
+        }
     }
     else {
         CBOR_CHECK(cbor_encode_byte_string(&mapEncoder2, (uint8_t *)"\x00\x01\x00\x01\x00\x01\x00\x01\x00\x01\x00\x01\x00\x01\x00\x01", 16));
@@ -660,8 +693,7 @@ int cbor_get_assertion(const uint8_t *data, size_t len, bool next) {
         }
         CBOR_CHECK(cbor_encoder_create_map(&mapEncoder, &mapEncoder2, lu));
         CBOR_CHECK(cbor_encode_text_stringz(&mapEncoder2, "id"));
-        CBOR_CHECK(cbor_encode_byte_string(&mapEncoder2, selcred->userId.data,
+        CBOR_CHECK(cbor_encode_byte_string(&mapEncoder2, selcred->userId.data, selcred->userId.len));
-                                           selcred->userId.len));
         if (numberOfCredentials > 1 && allowList_len == 0) {
             if (selcred->userName.present == true) {
                 CBOR_CHECK(cbor_encode_text_stringz(&mapEncoder2, "name"));

src/fido/cbor_make_credential.c

@@ -347,10 +347,26 @@ int cbor_make_credential(const uint8_t *data, size_t len) {
             continue;
         }
         Credential ecred = {0};
-        if (credential_load(excludeList[e].id.data, excludeList[e].id.len, rp_id_hash, &ecred) == 0 && (ecred.extensions.credProtect != CRED_PROT_UV_REQUIRED ||
+        if (credential_is_resident(excludeList[e].id.data, excludeList[e].id.len)) {
-             (flags & FIDO2_AUT_FLAG_UV))) {
+            for (int i = 0; i < MAX_RESIDENT_CREDENTIALS; i++) {
+                file_t *ef_cred = search_dynamic_file((uint16_t)(EF_CRED + i));
+                if (!file_has_data(ef_cred) || memcmp(file_get_data(ef_cred), rp_id_hash, 32) != 0) {
+                    continue;
+                }
+                uint8_t *cred_idr = file_get_data(ef_cred) + 32;
+                if (memcmp(cred_idr, excludeList[e].id.data, CRED_RESIDENT_LEN) == 0) {
+                    if (credential_load(file_get_data(ef_cred) + 32 + CRED_RESIDENT_LEN, file_get_size(ef_cred) - 32 - CRED_RESIDENT_LEN, rp_id_hash, &ecred) == 0 && (ecred.extensions.credProtect != CRED_PROT_UV_REQUIRED || (flags & FIDO2_AUT_FLAG_UV))) {
+                        credential_free(&ecred);
+                        CBOR_ERROR(CTAP2_ERR_CREDENTIAL_EXCLUDED);
+                    }
+                }
+            }
+        }
+        else {
+            if (credential_load(excludeList[e].id.data, excludeList[e].id.len, rp_id_hash, &ecred) == 0 && (ecred.extensions.credProtect != CRED_PROT_UV_REQUIRED || (flags & FIDO2_AUT_FLAG_UV))) {
                 credential_free(&ecred);
-            CBOR_ERROR(CTAP2_ERR_CREDENTIAL_EXCLUDED);
+                CBOR_ERROR(CTAP2_ERR_CREDENTIAL_EXCLUDED);
+            }
         }
         credential_free(&ecred);
     }
@@ -520,15 +536,23 @@ int cbor_make_credential(const uint8_t *data, size_t len) {
     CBOR_CHECK(COSE_key(&ekey, &encoder, &mapEncoder));
     size_t rs = cbor_encoder_get_buffer_size(&encoder, cbor_buf);
 
-    size_t aut_data_len = 32 + 1 + 4 + (16 + 2 + cred_id_len + rs) + ext_len;
+    size_t aut_data_len = 32 + 1 + 4 + (16 + 2 + (options.rk == ptrue ? CRED_RESIDENT_LEN : cred_id_len) + rs) + ext_len;
     aut_data = (uint8_t *) calloc(1, aut_data_len + clientDataHash.len);
     uint8_t *pa = aut_data;
     memcpy(pa, rp_id_hash, 32); pa += 32;
     *pa++ = flags;
     pa += put_uint32_t_be(ctr, pa);
     memcpy(pa, aaguid, 16); pa += 16;
-    pa += put_uint16_t_be(cred_id_len, pa);
+    if (options.rk == ptrue) {
-    memcpy(pa, cred_id, cred_id_len); pa += (uint16_t)cred_id_len;
+        uint8_t cred_idr[CRED_RESIDENT_LEN] = {0};
+        pa += put_uint16_t_be(sizeof(cred_idr), pa);
+        credential_derive_resident(cred_id, cred_id_len, cred_idr);
+        memcpy(pa, cred_idr, sizeof(cred_idr)); pa += sizeof(cred_idr);
+    }
+    else {
+        pa += put_uint16_t_be(cred_id_len, pa);
+        memcpy(pa, cred_id, cred_id_len); pa += (uint16_t)cred_id_len;
+    }
     memcpy(pa, cbor_buf, rs); pa += (uint16_t)rs;
     memcpy(pa, ext, ext_len); pa += (uint16_t)ext_len;
     if ((size_t)(pa - aut_data) != aut_data_len) {

src/fido/credential.c

@@ -314,7 +314,7 @@ int credential_store(const uint8_t *cred_id, size_t cred_id_len, const uint8_t *
         if (memcmp(file_get_data(ef), rp_id_hash, 32) != 0) {
             continue;
         }
-        ret = credential_load(file_get_data(ef) + 32, file_get_size(ef) - 32, rp_id_hash, &rcred);
+        ret = credential_load(file_get_data(ef) + 32 + CRED_RESIDENT_LEN, file_get_size(ef) - 32 - CRED_RESIDENT_LEN, rp_id_hash, &rcred);
         if (ret != 0) {
             credential_free(&rcred);
             continue;
@@ -330,11 +330,14 @@ int credential_store(const uint8_t *cred_id, size_t cred_id_len, const uint8_t *
     if (sloti == -1) {
         return -1;
     }
-    uint8_t *data = (uint8_t *) calloc(1, cred_id_len + 32);
+    uint8_t cred_idr[CRED_RESIDENT_LEN] = {0};
+    credential_derive_resident(cred_id, cred_id_len, cred_idr);
+    uint8_t *data = (uint8_t *) calloc(1, cred_id_len + 32 + CRED_RESIDENT_LEN);
     memcpy(data, rp_id_hash, 32);
-    memcpy(data + 32, cred_id, cred_id_len);
+    memcpy(data + 32, cred_idr, CRED_RESIDENT_LEN);
+    memcpy(data + 32 + CRED_RESIDENT_LEN, cred_id, cred_id_len);
     file_t *ef = file_new((uint16_t)(EF_CRED + sloti));
-    file_put_data(ef, data, (uint16_t)cred_id_len + 32);
+    file_put_data(ef, data, (uint16_t)cred_id_len + 32 + CRED_RESIDENT_LEN);
     free(data);
 
     if (new_record == true) { //increase rps
@@ -421,3 +424,19 @@ int credential_derive_large_blob_key(const uint8_t *cred_id, size_t cred_id_len,
     mbedtls_md_hmac(md_info, outk, 32, cred_id, cred_id_len, outk);
     return 0;
 }
+
+int credential_derive_resident(const uint8_t *cred_id, size_t cred_id_len, uint8_t *outk) {
+    memset(outk, 0, CRED_RESIDENT_LEN);
+    const mbedtls_md_info_t *md_info = mbedtls_md_info_from_type(MBEDTLS_MD_SHA256);
+    uint8_t *cred_idr = outk + CRED_RESIDENT_HEADER_LEN;
+    mbedtls_md_hmac(md_info, cred_idr, 32, (uint8_t *) "SLIP-0022", 9, cred_idr);
+    mbedtls_md_hmac(md_info, cred_idr, 32, (uint8_t *) cred_id, CRED_PROTO_LEN, cred_idr);
+    mbedtls_md_hmac(md_info, cred_idr, 32, (uint8_t *) "resident", 8, cred_idr);
+    mbedtls_md_hmac(md_info, cred_idr, 32, cred_id, cred_id_len, cred_idr);
+    memcpy(outk, CRED_PROTO_RESIDENT, CRED_PROTO_RESIDENT_LEN);
+    return 0;
+}
+
+bool credential_is_resident(const uint8_t *cred_id, size_t cred_id_len) {
+    return memcmp(cred_id, CRED_PROTO_RESIDENT, CRED_PROTO_RESIDENT_LEN) == 0;
+}

src/fido/credential.h

@@ -58,6 +58,7 @@ typedef struct Credential {
 
 #define CRED_PROTO_21_S                     "\xf1\xd0\x02\x01"
 #define CRED_PROTO_22_S                     "\xf1\xd0\x02\x02"
+#define CRED_PROTO_23_S                     "\xf1\xd0\x02\x03"
 
 #define CRED_PROTO                          CRED_PROTO_22_S
 
@@ -66,6 +67,11 @@ typedef struct Credential {
 #define CRED_TAG_LEN                        16
 #define CRED_SILENT_TAG_LEN                 16
 
+#define CRED_PROTO_RESIDENT                 CRED_PROTO_23_S
+#define CRED_PROTO_RESIDENT_LEN             4
+#define CRED_RESIDENT_HEADER_LEN            (CRED_PROTO_RESIDENT_LEN + 4)
+#define CRED_RESIDENT_LEN                   (CRED_RESIDENT_HEADER_LEN + 32)
+
 typedef enum
 {
     CRED_PROTO_21 = 0x01,
@@ -94,5 +100,7 @@ extern int credential_derive_hmac_key(const uint8_t *cred_id, size_t cred_id_len
 extern int credential_derive_large_blob_key(const uint8_t *cred_id,
                                             size_t cred_id_len,
                                             uint8_t *outk);
+extern int credential_derive_resident(const uint8_t *cred_id, size_t cred_id_len, uint8_t *outk);
+extern bool credential_is_resident(const uint8_t *cred_id, size_t cred_id_len);
 
 #endif // _CREDENTIAL_H_

tests/pico-fido/test_022_discoverable.py

@@ -58,8 +58,9 @@ def test_with_allow_list_after_reset(device, MCRes_DC, GARes_DC):
     device.reset()
 
     # It returns a silent authentication
-    ga_res = device.doGA(allow_list=allow_list)
+    with pytest.raises(CtapError) as e:
-
+        ga_res = device.doGA(allow_list=allow_list)
+    assert e.value.code == CtapError.ERR.NO_CREDENTIALS
 
 
 def test_resident_key(MCRes_DC, info):

tests/pico-fido/test_033_credprotect.py

@@ -22,6 +22,7 @@ import pytest
 from fido2.ctap2.extensions import CredProtectExtension
 from fido2.webauthn import UserVerificationRequirement
 from fido2.ctap import CtapError
+from utils import generate_random_user
 
 class CredProtect:
     UserVerificationOptional = 1
@@ -30,140 +31,139 @@ class CredProtect:
 
 @pytest.fixture(scope="class")
 def MCCredProtectOptional(resetdevice):
-    res = resetdevice.doMC(rk=True, extensions={'credentialProtectionPolicy': CredProtectExtension.POLICY.OPTIONAL})['res'].attestation_object
+    res = resetdevice.doMC(rk=True, user=generate_random_user(), extensions={'credentialProtectionPolicy': CredProtectExtension.POLICY.OPTIONAL})['res'].attestation_object
     return res
 
 @pytest.fixture(scope="class")
 def MCCredProtectOptionalList(resetdevice):
-    res = resetdevice.doMC(rk=True, extensions={'credentialProtectionPolicy': CredProtectExtension.POLICY.OPTIONAL_WITH_LIST})['res'].attestation_object
+    res = resetdevice.doMC(rk=True, user=generate_random_user(), extensions={'credentialProtectionPolicy': CredProtectExtension.POLICY.OPTIONAL_WITH_LIST})['res'].attestation_object
     return res
 
 @pytest.fixture(scope="class")
 def MCCredProtectRequired(resetdevice):
-    res = resetdevice.doMC(rk=True, extensions={'credentialProtectionPolicy': CredProtectExtension.POLICY.REQUIRED})['res'].attestation_object
+    res = resetdevice.doMC(rk=True, user=generate_random_user(), extensions={'credentialProtectionPolicy': CredProtectExtension.POLICY.REQUIRED})['res'].attestation_object
     return res
 
+class TestCredProtect(object):
+    def test_credprotect_make_credential_1(self, MCCredProtectOptional):
+        assert MCCredProtectOptional.auth_data.extensions
+        assert "credProtect" in MCCredProtectOptional.auth_data.extensions
+        assert MCCredProtectOptional.auth_data.extensions["credProtect"] == 1
 
-def test_credprotect_make_credential_1(MCCredProtectOptional):
+    def test_credprotect_make_credential_2(self, MCCredProtectOptionalList):
-    assert MCCredProtectOptional.auth_data.extensions
+        assert MCCredProtectOptionalList.auth_data.extensions
-    assert "credProtect" in MCCredProtectOptional.auth_data.extensions
+        assert "credProtect" in MCCredProtectOptionalList.auth_data.extensions
-    assert MCCredProtectOptional.auth_data.extensions["credProtect"] == 1
+        assert MCCredProtectOptionalList.auth_data.extensions["credProtect"] == 2
 
-def test_credprotect_make_credential_2(MCCredProtectOptionalList):
+    def test_credprotect_make_credential_3(self, MCCredProtectRequired):
-    assert MCCredProtectOptionalList.auth_data.extensions
+        assert MCCredProtectRequired.auth_data.extensions
-    assert "credProtect" in MCCredProtectOptionalList.auth_data.extensions
+        assert "credProtect" in MCCredProtectRequired.auth_data.extensions
-    assert MCCredProtectOptionalList.auth_data.extensions["credProtect"] == 2
+        assert MCCredProtectRequired.auth_data.extensions["credProtect"] == 3
 
-def test_credprotect_make_credential_3(MCCredProtectRequired):
+    def test_credprotect_optional_excluded(self, device, MCCredProtectOptional):
-    assert MCCredProtectRequired.auth_data.extensions
+        """ CredProtectOptional Cred should be visible to be excluded with no UV """
-    assert "credProtect" in MCCredProtectRequired.auth_data.extensions
+        exclude_list = [
-    assert MCCredProtectRequired.auth_data.extensions["credProtect"] == 3
+            {
+                "id": MCCredProtectOptional.auth_data.credential_data.credential_id[:],
+                "type": "public-key",
+            }
+        ]
 
-def test_credprotect_optional_excluded(device, MCCredProtectOptional):
+        with pytest.raises(CtapError) as e:
-    """ CredProtectOptional Cred should be visible to be excluded with no UV """
+            device.doMC(rk=True, extensions={'credentialProtectionPolicy': CredProtectExtension.POLICY.OPTIONAL}, exclude_list=exclude_list)
-    exclude_list = [
-        {
-            "id": MCCredProtectOptional.auth_data.credential_data.credential_id[:],
-            "type": "public-key",
-        }
-    ]
 
-    with pytest.raises(CtapError) as e:
+        assert e.value.code == CtapError.ERR.CREDENTIAL_EXCLUDED
-        device.doMC(rk=True, extensions={'credentialProtectionPolicy': CredProtectExtension.POLICY.OPTIONAL}, exclude_list=exclude_list)
 
-    assert e.value.code == CtapError.ERR.CREDENTIAL_EXCLUDED
+    def test_credprotect_optional_list_excluded(self, device, MCCredProtectOptionalList):
+        """ CredProtectOptionalList Cred should be visible to be excluded with no UV """
+        exclude_list = [
+            {
+                "id": MCCredProtectOptionalList.auth_data.credential_data.credential_id[:],
+                "type": "public-key",
+            }
+        ]
 
-def test_credprotect_optional_list_excluded(device, MCCredProtectOptionalList):
+        with pytest.raises(CtapError) as e:
-    """ CredProtectOptionalList Cred should be visible to be excluded with no UV """
+            device.MC(options={'rk': True}, extensions={'credProtect': CredProtect.UserVerificationOptionalWithCredentialId}, exclude_list=exclude_list)
-    exclude_list = [
-        {
-            "id": MCCredProtectOptionalList.auth_data.credential_data.credential_id[:],
-            "type": "public-key",
-        }
-    ]
 
-    with pytest.raises(CtapError) as e:
+        assert e.value.code == CtapError.ERR.CREDENTIAL_EXCLUDED
-        device.MC(options={'rk': True}, extensions={'credProtect': CredProtect.UserVerificationOptionalWithCredentialId}, exclude_list=exclude_list)
 
-    assert e.value.code == CtapError.ERR.CREDENTIAL_EXCLUDED
+    def test_credprotect_required_not_excluded_with_no_uv(self, device, MCCredProtectRequired):
+        """ CredProtectRequired Cred should NOT be visible to be excluded with no UV """
+        exclude_list = [
+            {
+                "id": MCCredProtectRequired.auth_data.credential_data.credential_id[:],
+                "type": "public-key",
+            }
+        ]
 
-def test_credprotect_required_not_excluded_with_no_uv(device, MCCredProtectRequired):
+        # works
-    """ CredProtectRequired Cred should NOT be visible to be excluded with no UV """
+        device.doMC(rk=True, extensions={'credentialProtectionPolicy': CredProtectExtension.POLICY.REQUIRED}, exclude_list=exclude_list)
-    exclude_list = [
-        {
-            "id": MCCredProtectRequired.auth_data.credential_data.credential_id[:],
-            "type": "public-key",
-        }
-    ]
 
-    # works
+    def test_credprotect_optional_works_with_no_allowList_no_uv(self, device, MCCredProtectOptional):
-    device.doMC(rk=True, extensions={'credentialProtectionPolicy': CredProtectExtension.POLICY.REQUIRED}, exclude_list=exclude_list)
 
-def test_credprotect_optional_works_with_no_allowList_no_uv(device, MCCredProtectOptional):
+        # works
+        res = device.doGA()['res'].get_assertions()[0]
 
-    # works
+        # If there's only one credential, this is None
-    res = device.doGA()['res'].get_assertions()[0]
+        assert res.number_of_credentials == None
 
-    # If there's only one credential, this is None
+    def test_credprotect_optional_and_list_works_no_uv(self, device, MCCredProtectOptional, MCCredProtectOptionalList, MCCredProtectRequired):
-    assert res.number_of_credentials == None
+        allow_list = [
+            {
+                "id": MCCredProtectOptional.auth_data.credential_data.credential_id[:],
+                "type": "public-key",
+            },
+            {
+                "id": MCCredProtectOptionalList.auth_data.credential_data.credential_id[:],
+                "type": "public-key",
+            },
+            {
+                "id": MCCredProtectRequired.auth_data.credential_data.credential_id[:],
+                "type": "public-key",
+            },
+        ]
+        # works
+        res1 = device.doGA(allow_list=allow_list, user_verification=False)['res'].get_assertions()[0]
+        assert res1.number_of_credentials in (None, 2)
 
-def test_credprotect_optional_and_list_works_no_uv(device, MCCredProtectOptional, MCCredProtectOptionalList, MCCredProtectRequired):
+        results = device.doGA(allow_list=allow_list, user_verification=False)['res'].get_assertions()
-    allow_list = [
-        {
-            "id": MCCredProtectOptional.auth_data.credential_data.credential_id[:],
-            "type": "public-key",
-        },
-        {
-            "id": MCCredProtectOptionalList.auth_data.credential_data.credential_id[:],
-            "type": "public-key",
-        },
-        {
-            "id": MCCredProtectRequired.auth_data.credential_data.credential_id[:],
-            "type": "public-key",
-        },
-    ]
-    # works
-    res1 = device.doGA(allow_list=allow_list, user_verification=False)['res'].get_assertions()[0]
-    assert res1.number_of_credentials in (None, 2)
 
-    results = device.doGA(allow_list=allow_list, user_verification=False)['res'].get_assertions()
+        # the required credProtect is not returned.
+        for res in results:
+            assert res.credential["id"] != MCCredProtectRequired.auth_data.credential_data.credential_id[:]
 
-    # the required credProtect is not returned.
+    def test_hmac_secret_and_credProtect_make_credential(self, resetdevice, MCCredProtectOptional):
-    for res in results:
-        assert res.credential["id"] != MCCredProtectRequired.auth_data.credential_data.credential_id[:]
 
-def test_hmac_secret_and_credProtect_make_credential(resetdevice, MCCredProtectOptional
+        res = resetdevice.doMC(extensions={'credentialProtectionPolicy': CredProtectExtension.POLICY.OPTIONAL, 'hmacCreateSecret': True})['res'].attestation_object
-):
 
-    res = resetdevice.doMC(extensions={'credentialProtectionPolicy': CredProtectExtension.POLICY.OPTIONAL, 'hmacCreateSecret': True})['res'].attestation_object
+        for ext in ["credProtect", "hmac-secret"]:
+            assert res.auth_data.extensions
+            assert ext in res.auth_data.extensions
+            assert res.auth_data.extensions[ext] == True
 
-    for ext in ["credProtect", "hmac-secret"]:
+class TestCredProtectUv:
-        assert res.auth_data.extensions
+    def test_credprotect_all_with_uv(self, device, MCCredProtectOptional, MCCredProtectOptionalList, MCCredProtectRequired, client_pin):
-        assert ext in res.auth_data.extensions
+        allow_list = [
-        assert res.auth_data.extensions[ext] == True
+            {
+                "id": MCCredProtectOptional.auth_data.credential_data.credential_id[:],
+                "type": "public-key",
+            },
+            {
+                "id": MCCredProtectOptionalList.auth_data.credential_data.credential_id[:],
+                "type": "public-key",
+            },
+            {
+                "id": MCCredProtectRequired.auth_data.credential_data.credential_id[:],
+                "type": "public-key",
+            },
+        ]
 
+        pin = "12345678"
 
-def test_credprotect_all_with_uv(device, MCCredProtectOptional, MCCredProtectOptionalList, MCCredProtectRequired, client_pin):
+        client_pin.set_pin(pin)
-    allow_list = [
-        {
-            "id": MCCredProtectOptional.auth_data.credential_data.credential_id[:],
-            "type": "public-key",
-        },
-        {
-            "id": MCCredProtectOptionalList.auth_data.credential_data.credential_id[:],
-            "type": "public-key",
-        },
-        {
-            "id": MCCredProtectRequired.auth_data.credential_data.credential_id[:],
-            "type": "public-key",
-        },
-    ]
 
-    pin = "12345678"
+        res1 = device.doGA(user_verification=UserVerificationRequirement.REQUIRED, allow_list=allow_list)['res'].get_assertions()[0]
 
-    client_pin.set_pin(pin)
+        assert res1.number_of_credentials in (None, 3)
-
-    res1 = device.doGA(user_verification=UserVerificationRequirement.REQUIRED, allow_list=allow_list)['res'].get_assertions()[0]
-
-    assert res1.number_of_credentials in (None, 3)