diff --git a/src/fido/cbor_get_assertion.c b/src/fido/cbor_get_assertion.c index 72f69b1..1ace962 100644 --- a/src/fido/cbor_get_assertion.c +++ b/src/fido/cbor_get_assertion.c @@ -469,91 +469,93 @@ int cbor_get_assertion(const uint8_t *data, size_t len, bool next) { if (options.up == pfalse) { extensions.hmac_secret = NULL; } - if (extensions.hmac_secret != NULL) { + if (extensions.hmac_secret == ptrue) { l++; } if (credBlob == ptrue) { l++; } - if (extensions.thirdPartyPayment != NULL) { + if (extensions.thirdPartyPayment == ptrue) { l++; } - CBOR_CHECK(cbor_encoder_create_map(&encoder, &mapEncoder, l)); - if (credBlob == ptrue) { - CBOR_CHECK(cbor_encode_text_stringz(&mapEncoder, "credBlob")); - if (selcred->extensions.credBlob.present == true) { - CBOR_CHECK(cbor_encode_byte_string(&mapEncoder, selcred->extensions.credBlob.data, - selcred->extensions.credBlob.len)); + if (l > 0) { + CBOR_CHECK(cbor_encoder_create_map(&encoder, &mapEncoder, l)); + if (credBlob == ptrue) { + CBOR_CHECK(cbor_encode_text_stringz(&mapEncoder, "credBlob")); + if (selcred->extensions.credBlob.present == true) { + CBOR_CHECK(cbor_encode_byte_string(&mapEncoder, selcred->extensions.credBlob.data, + selcred->extensions.credBlob.len)); + } + else { + CBOR_CHECK(cbor_encode_byte_string(&mapEncoder, NULL, 0)); + } } - else { - CBOR_CHECK(cbor_encode_byte_string(&mapEncoder, NULL, 0)); - } - } - if (extensions.hmac_secret != NULL) { + if (extensions.hmac_secret == ptrue) { - CBOR_CHECK(cbor_encode_text_stringz(&mapEncoder, "hmac-secret")); + CBOR_CHECK(cbor_encode_text_stringz(&mapEncoder, "hmac-secret")); - uint8_t sharedSecret[64] = {0}; - mbedtls_ecp_point Qp; - mbedtls_ecp_point_init(&Qp); - mbedtls_mpi_lset(&Qp.Z, 1); - if (mbedtls_mpi_read_binary(&Qp.X, kax.data, kax.len) != 0) { + uint8_t sharedSecret[64] = {0}; + mbedtls_ecp_point Qp; + mbedtls_ecp_point_init(&Qp); + mbedtls_mpi_lset(&Qp.Z, 1); + if (mbedtls_mpi_read_binary(&Qp.X, kax.data, kax.len) != 0) { + mbedtls_ecp_point_free(&Qp); + CBOR_ERROR(CTAP1_ERR_INVALID_PARAMETER); + } + if (mbedtls_mpi_read_binary(&Qp.Y, kay.data, kay.len) != 0) { + mbedtls_ecp_point_free(&Qp); + CBOR_ERROR(CTAP1_ERR_INVALID_PARAMETER); + } + ret = ecdh((uint8_t)hmacSecretPinUvAuthProtocol, &Qp, sharedSecret); mbedtls_ecp_point_free(&Qp); - CBOR_ERROR(CTAP1_ERR_INVALID_PARAMETER); + if (ret != 0) { + mbedtls_platform_zeroize(sharedSecret, sizeof(sharedSecret)); + CBOR_ERROR(CTAP1_ERR_INVALID_PARAMETER); + } + if (verify((uint8_t)hmacSecretPinUvAuthProtocol, sharedSecret, salt_enc.data, (uint16_t)salt_enc.len, salt_auth.data) != 0) { + mbedtls_platform_zeroize(sharedSecret, sizeof(sharedSecret)); + CBOR_ERROR(CTAP2_ERR_EXTENSION_FIRST); + } + uint8_t salt_dec[64] = {0}, poff = ((uint8_t)hmacSecretPinUvAuthProtocol - 1) * IV_SIZE; + ret = decrypt((uint8_t)hmacSecretPinUvAuthProtocol, sharedSecret, salt_enc.data, (uint16_t)salt_enc.len, salt_dec); + if (ret != 0) { + mbedtls_platform_zeroize(sharedSecret, sizeof(sharedSecret)); + CBOR_ERROR(CTAP1_ERR_INVALID_PARAMETER); + } + uint8_t cred_random[64] = {0}, *crd = NULL; + ret = credential_derive_hmac_key(selcred->id.data, selcred->id.len, cred_random); + if (ret != 0) { + mbedtls_platform_zeroize(sharedSecret, sizeof(sharedSecret)); + CBOR_ERROR(CTAP1_ERR_INVALID_PARAMETER); + } + if (flags & FIDO2_AUT_FLAG_UV) { + crd = cred_random + 32; + } + else { + crd = cred_random; + } + uint8_t out1[64] = {0}, hmac_res[80] = {0}; + mbedtls_md_hmac(mbedtls_md_info_from_type(MBEDTLS_MD_SHA256), crd, 32, salt_dec, 32, out1); + if ((uint8_t)salt_enc.len == 64 + poff) { + mbedtls_md_hmac(mbedtls_md_info_from_type(MBEDTLS_MD_SHA256), crd, 32, salt_dec + 32, 32, out1 + 32); + } + encrypt((uint8_t)hmacSecretPinUvAuthProtocol, sharedSecret, out1, (uint16_t)(salt_enc.len - poff), hmac_res); + CBOR_CHECK(cbor_encode_byte_string(&mapEncoder, hmac_res, salt_enc.len)); } - if (mbedtls_mpi_read_binary(&Qp.Y, kay.data, kay.len) != 0) { - mbedtls_ecp_point_free(&Qp); - CBOR_ERROR(CTAP1_ERR_INVALID_PARAMETER); + if (extensions.thirdPartyPayment == ptrue) { + CBOR_CHECK(cbor_encode_text_stringz(&mapEncoder, "thirdPartyPayment")); + if (selcred->extensions.thirdPartyPayment == ptrue) { + CBOR_CHECK(cbor_encode_boolean(&mapEncoder, true)); + } + else { + CBOR_CHECK(cbor_encode_boolean(&mapEncoder, false)); + } } - ret = ecdh((uint8_t)hmacSecretPinUvAuthProtocol, &Qp, sharedSecret); - mbedtls_ecp_point_free(&Qp); - if (ret != 0) { - mbedtls_platform_zeroize(sharedSecret, sizeof(sharedSecret)); - CBOR_ERROR(CTAP1_ERR_INVALID_PARAMETER); - } - if (verify((uint8_t)hmacSecretPinUvAuthProtocol, sharedSecret, salt_enc.data, (uint16_t)salt_enc.len, salt_auth.data) != 0) { - mbedtls_platform_zeroize(sharedSecret, sizeof(sharedSecret)); - CBOR_ERROR(CTAP2_ERR_EXTENSION_FIRST); - } - uint8_t salt_dec[64] = {0}, poff = ((uint8_t)hmacSecretPinUvAuthProtocol - 1) * IV_SIZE; - ret = decrypt((uint8_t)hmacSecretPinUvAuthProtocol, sharedSecret, salt_enc.data, (uint16_t)salt_enc.len, salt_dec); - if (ret != 0) { - mbedtls_platform_zeroize(sharedSecret, sizeof(sharedSecret)); - CBOR_ERROR(CTAP1_ERR_INVALID_PARAMETER); - } - uint8_t cred_random[64] = {0}, *crd = NULL; - ret = credential_derive_hmac_key(selcred->id.data, selcred->id.len, cred_random); - if (ret != 0) { - mbedtls_platform_zeroize(sharedSecret, sizeof(sharedSecret)); - CBOR_ERROR(CTAP1_ERR_INVALID_PARAMETER); - } - if (flags & FIDO2_AUT_FLAG_UV) { - crd = cred_random + 32; - } - else { - crd = cred_random; - } - uint8_t out1[64] = {0}, hmac_res[80] = {0}; - mbedtls_md_hmac(mbedtls_md_info_from_type(MBEDTLS_MD_SHA256), crd, 32, salt_dec, 32, out1); - if ((uint8_t)salt_enc.len == 64 + poff) { - mbedtls_md_hmac(mbedtls_md_info_from_type(MBEDTLS_MD_SHA256), crd, 32, salt_dec + 32, 32, out1 + 32); - } - encrypt((uint8_t)hmacSecretPinUvAuthProtocol, sharedSecret, out1, (uint16_t)(salt_enc.len - poff), hmac_res); - CBOR_CHECK(cbor_encode_byte_string(&mapEncoder, hmac_res, salt_enc.len)); - } - if (extensions.thirdPartyPayment != NULL) { - CBOR_CHECK(cbor_encode_text_stringz(&mapEncoder, "thirdPartyPayment")); - if (selcred->extensions.thirdPartyPayment == ptrue) { - CBOR_CHECK(cbor_encode_boolean(&mapEncoder, true)); - } - else { - CBOR_CHECK(cbor_encode_boolean(&mapEncoder, false)); - } - } - CBOR_CHECK(cbor_encoder_close_container(&encoder, &mapEncoder)); - ext_len = cbor_encoder_get_buffer_size(&encoder, ext); - flags |= FIDO2_AUT_FLAG_ED; + CBOR_CHECK(cbor_encoder_close_container(&encoder, &mapEncoder)); + ext_len = cbor_encoder_get_buffer_size(&encoder, ext); + flags |= FIDO2_AUT_FLAG_ED; + } } uint32_t ctr = get_sign_counter(); diff --git a/src/fido/cbor_make_credential.c b/src/fido/cbor_make_credential.c index 8ccc301..6625532 100644 --- a/src/fido/cbor_make_credential.c +++ b/src/fido/cbor_make_credential.c @@ -348,13 +348,13 @@ int cbor_make_credential(const uint8_t *data, size_t len) { cbor_encoder_init(&encoder, ext, sizeof(ext), 0); int l = 0; uint8_t minPinLen = 0; - if (extensions.hmac_secret != NULL) { + if (extensions.hmac_secret == ptrue) { l++; } if (extensions.credProtect != 0) { l++; } - if (extensions.minPinLength != NULL) { + if (extensions.minPinLength == ptrue) { file_t *ef_minpin = search_by_fid(EF_MINPINLEN, NULL, SPECIFY_EF); if (file_has_data(ef_minpin)) { uint8_t *minpin_data = file_get_data(ef_minpin); @@ -372,29 +372,31 @@ int cbor_make_credential(const uint8_t *data, size_t len) { if (extensions.credBlob.present == true) { l++; } - CBOR_CHECK(cbor_encoder_create_map(&encoder, &mapEncoder, l)); - if (extensions.credBlob.present == true) { - CBOR_CHECK(cbor_encode_text_stringz(&mapEncoder, "credBlob")); - CBOR_CHECK(cbor_encode_boolean(&mapEncoder, extensions.credBlob.len < MAX_CREDBLOB_LENGTH)); - } - if (extensions.credProtect != 0) { - CBOR_CHECK(cbor_encode_text_stringz(&mapEncoder, "credProtect")); - CBOR_CHECK(cbor_encode_uint(&mapEncoder, extensions.credProtect)); - } - if (extensions.hmac_secret != NULL) { + if (l > 0) { + CBOR_CHECK(cbor_encoder_create_map(&encoder, &mapEncoder, l)); + if (extensions.credBlob.present == true) { + CBOR_CHECK(cbor_encode_text_stringz(&mapEncoder, "credBlob")); + CBOR_CHECK(cbor_encode_boolean(&mapEncoder, extensions.credBlob.len < MAX_CREDBLOB_LENGTH)); + } + if (extensions.credProtect != 0) { + CBOR_CHECK(cbor_encode_text_stringz(&mapEncoder, "credProtect")); + CBOR_CHECK(cbor_encode_uint(&mapEncoder, extensions.credProtect)); + } + if (extensions.hmac_secret == ptrue) { - CBOR_CHECK(cbor_encode_text_stringz(&mapEncoder, "hmac-secret")); - CBOR_CHECK(cbor_encode_boolean(&mapEncoder, *extensions.hmac_secret)); - } - if (minPinLen > 0) { + CBOR_CHECK(cbor_encode_text_stringz(&mapEncoder, "hmac-secret")); + CBOR_CHECK(cbor_encode_boolean(&mapEncoder, true)); + } + if (minPinLen > 0) { - CBOR_CHECK(cbor_encode_text_stringz(&mapEncoder, "minPinLength")); - CBOR_CHECK(cbor_encode_uint(&mapEncoder, minPinLen)); - } + CBOR_CHECK(cbor_encode_text_stringz(&mapEncoder, "minPinLength")); + CBOR_CHECK(cbor_encode_uint(&mapEncoder, minPinLen)); + } - CBOR_CHECK(cbor_encoder_close_container(&encoder, &mapEncoder)); - ext_len = cbor_encoder_get_buffer_size(&encoder, ext); - flags |= FIDO2_AUT_FLAG_ED; + CBOR_CHECK(cbor_encoder_close_container(&encoder, &mapEncoder)); + ext_len = cbor_encoder_get_buffer_size(&encoder, ext); + flags |= FIDO2_AUT_FLAG_ED; + } } mbedtls_ecp_keypair ekey; mbedtls_ecp_keypair_init(&ekey); diff --git a/tests/pico-fido/test_037_minpinlength.py b/tests/pico-fido/test_037_minpinlength.py index 5472c82..33b2fdf 100644 --- a/tests/pico-fido/test_037_minpinlength.py +++ b/tests/pico-fido/test_037_minpinlength.py @@ -69,7 +69,7 @@ def test_minpin(SetMinPin, MCMinPin): def test_minpin_bad_rpid(SetMinPinWrongRpid, MCMinPin): assert not MCMinPin.auth_data.extensions - assert "minPinLength" not in MCMinPin.auth_data.extensions + #assert "minPinLength" not in MCMinPin.auth_data.extensions def test_setminpin(device, SetMinPin, MCMinPin): cfg = FidoConfig(device)