From ebef6dad5c8c2bb59c0f040d6fb3ef2865b66721 Mon Sep 17 00:00:00 2001 From: Radoslav Gerganov Date: Tue, 17 Mar 2020 17:23:18 +0200 Subject: [PATCH 1/8] PoC: Add vendor command for signing an arbitrary SHA256 hash This patch adds new CTAP2 vendor command with command value 0x50. The command arguments are credentialId and user specified SHA256 hash. It returns a DER encoded signature of the given hash, using the key which corresponds to the specified credentialId. Example request: {1: , 2: {"id": , "type": "public-key"}, 3: [pinAuth]} Example response: {1: } Issue: #395 --- fido2/ctap.c | 46 +++++++++++++++++++++++++++++++ fido2/ctap.h | 11 ++++++++ fido2/ctap_parse.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++ fido2/ctap_parse.h | 1 + fido2/log.c | 1 + fido2/log.h | 1 + 6 files changed, 127 insertions(+) diff --git a/fido2/ctap.c b/fido2/ctap.c index f752c67b..dbc780bc 100644 --- a/fido2/ctap.c +++ b/fido2/ctap.c @@ -1429,6 +1429,45 @@ uint8_t ctap_get_next_assertion(CborEncoder * encoder) return 0; } +uint8_t ctap_sign_hash(CborEncoder * encoder, uint8_t * request, int length) +{ + CTAP_signHash SH; + CborEncoder map; + uint8_t sigbuf[64]; + uint8_t sigder[72]; + + int ret = ctap_parse_sign_hash(&SH, request, length); + if (ret != 0) + { + printf2(TAG_ERR,"error, ctap_parse_sign_hash failed\n"); + return ret; + } + if (ctap_is_pin_set() == 1) + { + ret = verify_pin_auth(SH.pinAuth, SH.clientDataHash); + check_retr(ret); + } + ret = ctap2_user_presence_test(CTAP2_UP_DELAY_MS); + check_retr(ret); + ret = cbor_encoder_create_map(encoder, &map, 1); + check_ret(ret); + + unsigned int cred_size = get_credential_id_size(&SH.cred); + crypto_ecc256_load_key((uint8_t*)&SH.cred.credential.id, cred_size, NULL, 0); + crypto_ecc256_sign(SH.clientDataHash, CLIENT_DATA_HASH_SIZE, sigbuf); + int sigder_sz = ctap_encode_der_sig(sigbuf,sigder); + printf1(TAG_SH,"der sig [%d]: ", sigder_sz); dump_hex1(TAG_SH, sigder, sigder_sz); + + ret = cbor_encode_int(&map, 1); + check_ret(ret); + ret = cbor_encode_byte_string(&map, sigder, sigder_sz); + check_ret(ret); + + ret = cbor_encoder_close_container(encoder, &map); + check_ret(ret); + return 0; +} + uint8_t ctap_cred_metadata(CborEncoder * encoder) { CborEncoder map; @@ -2304,6 +2343,7 @@ uint8_t ctap_request(uint8_t * pkt_raw, int length, CTAP_RESPONSE * resp) { case CTAP_MAKE_CREDENTIAL: case CTAP_GET_ASSERTION: + case CTAP_SOLO_SIGN: case CTAP_CBOR_CRED_MGMT: case CTAP_CBOR_CRED_MGMT_PRE: if (ctap_device_locked()) @@ -2387,6 +2427,12 @@ uint8_t ctap_request(uint8_t * pkt_raw, int length, CTAP_RESPONSE * resp) status = CTAP2_ERR_NOT_ALLOWED; } break; + case CTAP_SOLO_SIGN: + printf1(TAG_CTAP,"CTAP_SOLO_SIGN\n"); + status = ctap_sign_hash(&encoder, pkt_raw, length); + resp->length = cbor_encoder_get_buffer_size(&encoder, buf); + dump_hex1(TAG_DUMP, buf, resp->length); + break; case CTAP_CBOR_CRED_MGMT: case CTAP_CBOR_CRED_MGMT_PRE: printf1(TAG_CTAP,"CTAP_CBOR_CRED_MGMT_PRE\n"); diff --git a/fido2/ctap.h b/fido2/ctap.h index c08f9360..7d33b910 100644 --- a/fido2/ctap.h +++ b/fido2/ctap.h @@ -19,6 +19,7 @@ #define CTAP_CBOR_CRED_MGMT 0x0A #define CTAP_VENDOR_FIRST 0x40 #define CTAP_CBOR_CRED_MGMT_PRE 0x41 +#define CTAP_SOLO_SIGN 0x50 #define CTAP_VENDOR_LAST 0xBF #define MC_clientDataHash 0x01 @@ -39,6 +40,9 @@ #define GA_pinAuth 0x06 #define GA_pinProtocol 0x07 +#define SH_clientDataHash 0x01 +#define SH_credential 0x02 +#define SH_pinAuth 0x03 #define CM_cmd 0x01 #define CM_cmdMetadata 0x01 #define CM_cmdRPBegin 0x02 @@ -322,6 +326,13 @@ typedef struct } CTAP_getAssertion; +typedef struct +{ + uint8_t pinAuth[16]; + uint8_t clientDataHash[CLIENT_DATA_HASH_SIZE]; + CTAP_credentialDescriptor cred; +} CTAP_signHash; + typedef struct { int cmd; diff --git a/fido2/ctap_parse.c b/fido2/ctap_parse.c index 115bf4ea..85e09414 100644 --- a/fido2/ctap_parse.c +++ b/fido2/ctap_parse.c @@ -1008,6 +1008,73 @@ uint8_t parse_allow_list(CTAP_getAssertion * GA, CborValue * it) return 0; } +uint8_t ctap_parse_sign_hash(CTAP_signHash * SH, uint8_t * request, int length) +{ + int key; + size_t i, map_length; + CborParser parser; + CborValue it,map; + + memset(SH, 0, sizeof(CTAP_signHash)); + int ret = cbor_parser_init(request, length, CborValidateCanonicalFormat, &parser, &it); + check_ret(ret); + + CborType type = cbor_value_get_type(&it); + if (type != CborMapType) + { + printf2(TAG_ERR,"Error, expecting cbor map\n"); + return CTAP2_ERR_INVALID_CBOR_TYPE; + } + + ret = cbor_value_enter_container(&it,&map); + check_ret(ret); + + ret = cbor_value_get_map_length(&it, &map_length); + check_ret(ret); + + printf1(TAG_SH, "SH map has %d elements\n", map_length); + + for (i = 0; i < map_length; i++) + { + type = cbor_value_get_type(&map); + if (type != CborIntegerType) + { + printf2(TAG_ERR,"Error, expecting int for map key\n"); + return CTAP2_ERR_INVALID_CBOR_TYPE; + } + ret = cbor_value_get_int_checked(&map, &key); + check_ret(ret); + + ret = cbor_value_advance(&map); + check_ret(ret); + + switch(key) + { + case SH_clientDataHash: + printf1(TAG_SH, "SH_clientHash\n"); + + ret = parse_fixed_byte_string(&map, SH->clientDataHash, CLIENT_DATA_HASH_SIZE); + check_retr(ret); + + printf1(TAG_SH," "); dump_hex1(TAG_SH, SH->clientDataHash, CLIENT_DATA_HASH_SIZE); + break; + case SH_credential: + printf1(TAG_SH, "SH_credential\n"); + ret = parse_credential_descriptor(&map, &SH->cred); + check_ret(ret); + break; + case SH_pinAuth: + printf1(TAG_SH, "SH_pinAuth\n"); + ret = parse_fixed_byte_string(&map, SH->pinAuth, 16); + check_retr(ret); + break; + } + ret = cbor_value_advance(&map); + check_ret(ret); + } + return 0; +} + static uint8_t parse_cred_mgmt_subcommandparams(CborValue * val, CTAP_credMgmt * CM) { size_t map_length; diff --git a/fido2/ctap_parse.h b/fido2/ctap_parse.h index e0be8071..da6dbc2c 100644 --- a/fido2/ctap_parse.h +++ b/fido2/ctap_parse.h @@ -35,6 +35,7 @@ uint8_t parse_cose_key(CborValue * it, COSE_key * cose); uint8_t ctap_parse_make_credential(CTAP_makeCredential * MC, CborEncoder * encoder, uint8_t * request, int length); uint8_t ctap_parse_get_assertion(CTAP_getAssertion * GA, uint8_t * request, int length); +uint8_t ctap_parse_sign_hash(CTAP_signHash * SH, uint8_t * request, int length); uint8_t ctap_parse_cred_mgmt(CTAP_credMgmt * CM, uint8_t * request, int length); uint8_t ctap_parse_client_pin(CTAP_clientPin * CP, uint8_t * request, int length); uint8_t parse_credential_descriptor(CborValue * arr, CTAP_credentialDescriptor * cred); diff --git a/fido2/log.c b/fido2/log.c index bb9a41ad..3a151f28 100644 --- a/fido2/log.c +++ b/fido2/log.c @@ -52,6 +52,7 @@ struct logtag tagtable[] = { {TAG_NFC_APDU, "NAPDU"}, {TAG_CCID, "CCID"}, {TAG_CM, "CRED_MGMT"}, + {TAG_SH, "SH"}, }; diff --git a/fido2/log.h b/fido2/log.h index d415255f..4ac78dc2 100644 --- a/fido2/log.h +++ b/fido2/log.h @@ -49,6 +49,7 @@ typedef enum TAG_NFC_APDU = (1 << 20), TAG_CCID = (1 << 21), TAG_CM = (1 << 22), + TAG_SH = (1 << 23), TAG_NO_TAG = (1UL << 30), TAG_FILENO = (1UL << 31) From 51c0b87717694220f342c14dc639659ed5c11d4c Mon Sep 17 00:00:00 2001 From: stevenwdv Date: Fri, 15 Oct 2021 12:59:16 +0200 Subject: [PATCH 2/8] Added EdDSA/Minisign to sign-hash/sign-file command (first version) with support for arbitrary-length hashes up to 64B and a trusted comment in the EdDSA case. Also fixed existing bug: get_credential_id_size(SH.cred.type) should be get_credential_id_size(&SH.cred) Also now cose_alg is checked. I'm not sure yet if it is safe to use arbitrary-length hashes with verify_pin_auth_ex. Maybe a min length should be set? --- fido2/crypto.c | 7 ++++-- fido2/ctap.c | 55 ++++++++++++++++++++++++++++++++++++---------- fido2/ctap.h | 14 ++++++++++-- fido2/ctap_parse.c | 48 +++++++++++++++++++++++++++++++++++----- 4 files changed, 103 insertions(+), 21 deletions(-) diff --git a/fido2/crypto.c b/fido2/crypto.c index 34c26714..924511b6 100644 --- a/fido2/crypto.c +++ b/fido2/crypto.c @@ -424,8 +424,11 @@ void crypto_ed25519_sign(uint8_t * data1, int len1, uint8_t * data2, int len2, u const int len = len1 + len2; // 0 <= len <= 1024 uint8_t data[len1 + len2]; - memcpy(data, data1, len1); - memcpy(data + len1, data2, len2); + memcpy(data, data1, len1); + if (len2) + { + memcpy(data + len1, data2, len2); + } #if defined(STM32L432xx) diff --git a/fido2/ctap.c b/fido2/ctap.c index dbc780bc..e55c4af7 100644 --- a/fido2/ctap.c +++ b/fido2/ctap.c @@ -1444,24 +1444,55 @@ uint8_t ctap_sign_hash(CborEncoder * encoder, uint8_t * request, int length) } if (ctap_is_pin_set() == 1) { - ret = verify_pin_auth(SH.pinAuth, SH.clientDataHash); + ret = verify_pin_auth_ex(SH.pinAuth, SH.hash, SH.hash_len); check_retr(ret); } - ret = ctap2_user_presence_test(CTAP2_UP_DELAY_MS); + ret = ctap2_user_presence_test(); check_retr(ret); - ret = cbor_encoder_create_map(encoder, &map, 1); + ret = cbor_encoder_create_map(encoder, &map, SH.trusted_comment_present ? 2 : 1); check_ret(ret); - unsigned int cred_size = get_credential_id_size(&SH.cred); - crypto_ecc256_load_key((uint8_t*)&SH.cred.credential.id, cred_size, NULL, 0); - crypto_ecc256_sign(SH.clientDataHash, CLIENT_DATA_HASH_SIZE, sigbuf); - int sigder_sz = ctap_encode_der_sig(sigbuf,sigder); - printf1(TAG_SH,"der sig [%d]: ", sigder_sz); dump_hex1(TAG_SH, sigder, sigder_sz); + unsigned int cred_size = get_credential_id_size(SH.cred.type); - ret = cbor_encode_int(&map, 1); - check_ret(ret); - ret = cbor_encode_byte_string(&map, sigder, sigder_sz); - check_ret(ret); + int32_t cose_alg = read_cose_alg_from_masked_credential(&SH.cred.credential.id); + if (cose_alg == COSE_ALG_EDDSA) + { + crypto_ed25519_load_key((uint8_t*)&SH.cred.credential.id, cred_size); + + crypto_ed25519_sign(SH.hash, SH.hash_len, NULL, 0, sigder); + + ret = cbor_encode_int(&map, 1); + check_ret(ret); + ret = cbor_encode_byte_string(&map, sigder, 64); + check_ret(ret); + + if (SH.trusted_comment_present) + { + crypto_ed25519_sign(sigder, 64, SH.trusted_comment, SH.trusted_comment_len, sigder); + + ret = cbor_encode_int(&map, 2); + check_ret(ret); + ret = cbor_encode_byte_string(&map, sigder, 64); + check_ret(ret); + } + } + else + { + if (SH.trusted_comment_present) + { + printf2(TAG_ERR,"error, trusted comment is only supported with EdDSA\n"); + return CTAP2_ERR_INVALID_OPTION; + } + crypto_ecc256_load_key((uint8_t*)&SH.cred.credential.id, cred_size, NULL, 0); + crypto_ecc256_sign(SH.hash, SH.hash_len, sigbuf); + int sigder_sz = ctap_encode_der_sig(sigbuf,sigder); + printf1(TAG_SH,"der sig [%d]: ", sigder_sz); dump_hex1(TAG_SH, sigder, sigder_sz); + + ret = cbor_encode_int(&map, 1); + check_ret(ret); + ret = cbor_encode_byte_string(&map, sigder, sigder_sz); + check_ret(ret); + } ret = cbor_encoder_close_container(encoder, &map); check_ret(ret); diff --git a/fido2/ctap.h b/fido2/ctap.h index 7d33b910..73afcfc4 100644 --- a/fido2/ctap.h +++ b/fido2/ctap.h @@ -40,9 +40,10 @@ #define GA_pinAuth 0x06 #define GA_pinProtocol 0x07 -#define SH_clientDataHash 0x01 +#define SH_hash 0x01 #define SH_credential 0x02 #define SH_pinAuth 0x03 +#define SH_trustedComment 0x04 #define CM_cmd 0x01 #define CM_cmdMetadata 0x01 #define CM_cmdRPBegin 0x02 @@ -128,6 +129,8 @@ #define DISPLAY_NAME_LIMIT 32 // Must be minimum of 64 bytes but can be more. #define ICON_LIMIT 128 // Must be minimum of 64 bytes but can be more. #define CTAP_MAX_MESSAGE_SIZE 1200 +#define SIGN_HASH_HASH_MAX_SIZE 64 //up to 512 bits +#define SIGN_HASH_TRUSTED_COMMENT_MAX_SIZE 128 #define CREDENTIAL_RK_FLASH_PAD 2 // size of RK should be 8-byte aligned to store in flash easily. #define CREDENTIAL_TAG_SIZE 16 @@ -329,7 +332,14 @@ typedef struct typedef struct { uint8_t pinAuth[16]; - uint8_t clientDataHash[CLIENT_DATA_HASH_SIZE]; + + uint8_t hash[SIGN_HASH_HASH_MAX_SIZE]; + uint8_t hash_len; + + uint8_t trusted_comment[SIGN_HASH_TRUSTED_COMMENT_MAX_SIZE]; + uint8_t trusted_comment_len; + uint8_t trusted_comment_present; + CTAP_credentialDescriptor cred; } CTAP_signHash; diff --git a/fido2/ctap_parse.c b/fido2/ctap_parse.c index 85e09414..a7b8dd47 100644 --- a/fido2/ctap_parse.c +++ b/fido2/ctap_parse.c @@ -1050,14 +1050,52 @@ uint8_t ctap_parse_sign_hash(CTAP_signHash * SH, uint8_t * request, int length) switch(key) { - case SH_clientDataHash: - printf1(TAG_SH, "SH_clientHash\n"); + case SH_hash: + printf1(TAG_SH, "SH_hash\n"); - ret = parse_fixed_byte_string(&map, SH->clientDataHash, CLIENT_DATA_HASH_SIZE); - check_retr(ret); + if (cbor_value_get_type(&map) == CborByteStringType) + { + size_t hash_len = SIGN_HASH_HASH_MAX_SIZE; + ret = cbor_value_copy_byte_string(&map, SH->hash, &hash_len, NULL); + if (ret == CborErrorOutOfMemory) + { + printf2(TAG_ERR,"Error, hash too large\n"); + return CTAP2_ERR_LIMIT_EXCEEDED; + } + check_ret(ret); + SH->hash_len = hash_len; + } + else + { + printf2(TAG_ERR, "error, CborByteStringType expected for hash\r\n"); + return CTAP2_ERR_INVALID_CBOR_TYPE; + } + + printf1(TAG_SH," "); dump_hex1(TAG_SH, SH->hash, SH->hash_len); + break; - printf1(TAG_SH," "); dump_hex1(TAG_SH, SH->clientDataHash, CLIENT_DATA_HASH_SIZE); + case SH_trustedComment: + printf1(TAG_SH, "SH_trustedComment\n"); + if (cbor_value_get_type(&map) == CborByteStringType) + { + size_t trusted_comment_len = SIGN_HASH_TRUSTED_COMMENT_MAX_SIZE; + ret = cbor_value_copy_byte_string(&map, SH->trusted_comment, &trusted_comment_len, NULL); + if (ret == CborErrorOutOfMemory) + { + printf2(TAG_ERR,"Error, trusted comment too large\n"); + return CTAP2_ERR_LIMIT_EXCEEDED; + } + check_ret(ret); + SH->trusted_comment_len = trusted_comment_len; + SH->trusted_comment_present = 1; + } + else + { + printf2(TAG_ERR, "error, CborByteStringType expected for trusted comment\r\n"); + return CTAP2_ERR_INVALID_CBOR_TYPE; + } break; + case SH_credential: printf1(TAG_SH, "SH_credential\n"); ret = parse_credential_descriptor(&map, &SH->cred); From 7708b9752ccf66aaf66ce909b3c1db3f19088505 Mon Sep 17 00:00:00 2001 From: stevenwdv Date: Wed, 20 Oct 2021 12:00:52 +0200 Subject: [PATCH 3/8] Only accept hash of 32/64B in ctap_parse_sign_hash --- fido2/ctap_parse.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/fido2/ctap_parse.c b/fido2/ctap_parse.c index a7b8dd47..e95a7330 100644 --- a/fido2/ctap_parse.c +++ b/fido2/ctap_parse.c @@ -1057,10 +1057,10 @@ uint8_t ctap_parse_sign_hash(CTAP_signHash * SH, uint8_t * request, int length) { size_t hash_len = SIGN_HASH_HASH_MAX_SIZE; ret = cbor_value_copy_byte_string(&map, SH->hash, &hash_len, NULL); - if (ret == CborErrorOutOfMemory) + if ((hash_len != 32 && hash_len != 64) || ret == CborErrorOutOfMemory) { - printf2(TAG_ERR,"Error, hash too large\n"); - return CTAP2_ERR_LIMIT_EXCEEDED; + printf2(TAG_ERR,"Error, invalid hash length, should be 32/64 B\n"); + return CTAP1_ERR_INVALID_LENGTH; } check_ret(ret); SH->hash_len = hash_len; From 1fd5b74c2d599da27814349b5bc74d8aff3f558d Mon Sep 17 00:00:00 2001 From: stevenwdv Date: Wed, 20 Oct 2021 18:19:45 +0200 Subject: [PATCH 4/8] Authenticate sign-hash credential, with special "solo-sign-hash:" RP ID prefix --- fido2/ctap.c | 16 ++++++++++++++++ fido2/ctap.h | 2 ++ fido2/ctap_parse.c | 7 +++++++ 3 files changed, 25 insertions(+) diff --git a/fido2/ctap.c b/fido2/ctap.c index e55c4af7..cb005128 100644 --- a/fido2/ctap.c +++ b/fido2/ctap.c @@ -1449,6 +1449,22 @@ uint8_t ctap_sign_hash(CborEncoder * encoder, uint8_t * request, int length) } ret = ctap2_user_presence_test(); check_retr(ret); + + const char * sign_hash_prefix = "solo-sign-hash:"; + const char * rpId = (const char*)SH.rp.id; + const char * colon = strchr(rpId, ':'); + if (! colon || strncmp(rpId, sign_hash_prefix, colon - rpId + 1) != 0) + { + printf2(TAG_ERR, "Error: invalid RP ID, should start with 'solo-sign-hash:'\n"); + return CTAP2_ERR_INVALID_CREDENTIAL; + } + + if (! ctap_authenticate_credential(&SH.rp, &SH.cred)) + { + printf2(TAG_ERR, "Error: invalid credential\n"); + return CTAP2_ERR_INVALID_CREDENTIAL; + } + ret = cbor_encoder_create_map(encoder, &map, SH.trusted_comment_present ? 2 : 1); check_ret(ret); diff --git a/fido2/ctap.h b/fido2/ctap.h index 73afcfc4..25a26cfd 100644 --- a/fido2/ctap.h +++ b/fido2/ctap.h @@ -44,6 +44,7 @@ #define SH_credential 0x02 #define SH_pinAuth 0x03 #define SH_trustedComment 0x04 +#define SH_rpId 0x05 #define CM_cmd 0x01 #define CM_cmdMetadata 0x01 #define CM_cmdRPBegin 0x02 @@ -341,6 +342,7 @@ typedef struct uint8_t trusted_comment_present; CTAP_credentialDescriptor cred; + struct rpId rp; } CTAP_signHash; typedef struct diff --git a/fido2/ctap_parse.c b/fido2/ctap_parse.c index e95a7330..892579d8 100644 --- a/fido2/ctap_parse.c +++ b/fido2/ctap_parse.c @@ -1106,6 +1106,13 @@ uint8_t ctap_parse_sign_hash(CTAP_signHash * SH, uint8_t * request, int length) ret = parse_fixed_byte_string(&map, SH->pinAuth, 16); check_retr(ret); break; + case SH_rpId: + printf1(TAG_SH, "SH_rpId\n"); + ret = parse_rp_id(&SH->rp, &map); + check_retr(ret); + + printf1(TAG_GA," ID: %s\n", SH->rp.id); + break; } ret = cbor_value_advance(&map); check_ret(ret); From 861d78349325f087ffb63b45959430ec65629d53 Mon Sep 17 00:00:00 2001 From: stevenwdv Date: Mon, 25 Oct 2021 18:03:31 +0200 Subject: [PATCH 5/8] sign-hash: validate hash length per signature algorithm, check if signature algorithm is supported --- fido2/ctap.c | 22 +++++++++++++++++++++- fido2/ctap_parse.c | 4 ++-- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/fido2/ctap.c b/fido2/ctap.c index cb005128..65593b43 100644 --- a/fido2/ctap.c +++ b/fido2/ctap.c @@ -1473,6 +1473,13 @@ uint8_t ctap_sign_hash(CborEncoder * encoder, uint8_t * request, int length) int32_t cose_alg = read_cose_alg_from_masked_credential(&SH.cred.credential.id); if (cose_alg == COSE_ALG_EDDSA) { + // Enforce 64-byte hash (Minisign ED: Blake2b-512) + if (SH.hash_len != 64) + { + printf2(TAG_ERR, "Error, invalid hash length for EdDSA, must be 64 B\n"); + return CTAP1_ERR_INVALID_LENGTH; + } + crypto_ed25519_load_key((uint8_t*)&SH.cred.credential.id, cred_size); crypto_ed25519_sign(SH.hash, SH.hash_len, NULL, 0, sigder); @@ -1492,13 +1499,21 @@ uint8_t ctap_sign_hash(CborEncoder * encoder, uint8_t * request, int length) check_ret(ret); } } - else + else if (cose_alg == COSE_ALG_ES256) { + // Enforce 32-byte hash (ES256: SHA-256) + if (SH.hash_len != 32) + { + printf2(TAG_ERR, "Error, invalid hash length for ES256, must be 32 B\n"); + return CTAP1_ERR_INVALID_LENGTH; + } + if (SH.trusted_comment_present) { printf2(TAG_ERR,"error, trusted comment is only supported with EdDSA\n"); return CTAP2_ERR_INVALID_OPTION; } + crypto_ecc256_load_key((uint8_t*)&SH.cred.credential.id, cred_size, NULL, 0); crypto_ecc256_sign(SH.hash, SH.hash_len, sigbuf); int sigder_sz = ctap_encode_der_sig(sigbuf,sigder); @@ -1509,6 +1524,11 @@ uint8_t ctap_sign_hash(CborEncoder * encoder, uint8_t * request, int length) ret = cbor_encode_byte_string(&map, sigder, sigder_sz); check_ret(ret); } + else + { + printf2(TAG_ERR, "Error, unsupported credential algorithm\n"); + return CTAP2_ERR_UNSUPPORTED_ALGORITHM; + } ret = cbor_encoder_close_container(encoder, &map); check_ret(ret); diff --git a/fido2/ctap_parse.c b/fido2/ctap_parse.c index 892579d8..df2397b2 100644 --- a/fido2/ctap_parse.c +++ b/fido2/ctap_parse.c @@ -1057,9 +1057,9 @@ uint8_t ctap_parse_sign_hash(CTAP_signHash * SH, uint8_t * request, int length) { size_t hash_len = SIGN_HASH_HASH_MAX_SIZE; ret = cbor_value_copy_byte_string(&map, SH->hash, &hash_len, NULL); - if ((hash_len != 32 && hash_len != 64) || ret == CborErrorOutOfMemory) + if (ret == CborErrorOutOfMemory) { - printf2(TAG_ERR,"Error, invalid hash length, should be 32/64 B\n"); + printf2(TAG_ERR,"Error, hash too long\n"); return CTAP1_ERR_INVALID_LENGTH; } check_ret(ret); From d067d94f9acdde95e6178e2b7e02a71637da2277 Mon Sep 17 00:00:00 2001 From: stevenwdv Date: Mon, 25 Oct 2021 18:16:38 +0200 Subject: [PATCH 6/8] sign-hash: some cleanup --- fido2/ctap.c | 69 ++++++++++++++++++++++++++++------------------------ fido2/ctap.h | 11 +++++++-- 2 files changed, 46 insertions(+), 34 deletions(-) diff --git a/fido2/ctap.c b/fido2/ctap.c index 65593b43..457bf47e 100644 --- a/fido2/ctap.c +++ b/fido2/ctap.c @@ -1432,16 +1432,13 @@ uint8_t ctap_get_next_assertion(CborEncoder * encoder) uint8_t ctap_sign_hash(CborEncoder * encoder, uint8_t * request, int length) { CTAP_signHash SH; - CborEncoder map; - uint8_t sigbuf[64]; - uint8_t sigder[72]; - int ret = ctap_parse_sign_hash(&SH, request, length); if (ret != 0) { - printf2(TAG_ERR,"error, ctap_parse_sign_hash failed\n"); + printf2(TAG_ERR, "Error, ctap_parse_sign_hash failed\n"); return ret; } + if (ctap_is_pin_set() == 1) { ret = verify_pin_auth_ex(SH.pinAuth, SH.hash, SH.hash_len); @@ -1450,21 +1447,24 @@ uint8_t ctap_sign_hash(CborEncoder * encoder, uint8_t * request, int length) ret = ctap2_user_presence_test(); check_retr(ret); - const char * sign_hash_prefix = "solo-sign-hash:"; - const char * rpId = (const char*)SH.rp.id; - const char * colon = strchr(rpId, ':'); - if (! colon || strncmp(rpId, sign_hash_prefix, colon - rpId + 1) != 0) { - printf2(TAG_ERR, "Error: invalid RP ID, should start with 'solo-sign-hash:'\n"); - return CTAP2_ERR_INVALID_CREDENTIAL; - } + const char * sign_hash_prefix = "solo-sign-hash:"; + const char * rpId = (const char*)SH.rp.id; + const char * colon = strchr(rpId, ':'); + if (! colon || strncmp(rpId, sign_hash_prefix, colon - rpId + 1) != 0) + { + printf2(TAG_ERR, "Error: invalid RP ID, should start with 'solo-sign-hash:'\n"); + return CTAP2_ERR_INVALID_CREDENTIAL; + } - if (! ctap_authenticate_credential(&SH.rp, &SH.cred)) - { - printf2(TAG_ERR, "Error: invalid credential\n"); - return CTAP2_ERR_INVALID_CREDENTIAL; + if (! ctap_authenticate_credential(&SH.rp, &SH.cred)) + { + printf2(TAG_ERR, "Error: invalid credential\n"); + return CTAP2_ERR_INVALID_CREDENTIAL; + } } + CborEncoder map; ret = cbor_encoder_create_map(encoder, &map, SH.trusted_comment_present ? 2 : 1); check_ret(ret); @@ -1473,8 +1473,7 @@ uint8_t ctap_sign_hash(CborEncoder * encoder, uint8_t * request, int length) int32_t cose_alg = read_cose_alg_from_masked_credential(&SH.cred.credential.id); if (cose_alg == COSE_ALG_EDDSA) { - // Enforce 64-byte hash (Minisign ED: Blake2b-512) - if (SH.hash_len != 64) + if (SH.hash_len != SIGN_HASH_HASH_EDDSA_SIZE) { printf2(TAG_ERR, "Error, invalid hash length for EdDSA, must be 64 B\n"); return CTAP1_ERR_INVALID_LENGTH; @@ -1482,27 +1481,32 @@ uint8_t ctap_sign_hash(CborEncoder * encoder, uint8_t * request, int length) crypto_ed25519_load_key((uint8_t*)&SH.cred.credential.id, cred_size); - crypto_ed25519_sign(SH.hash, SH.hash_len, NULL, 0, sigder); + uint8_t main_signature[EDDSA_SIGNATURE_SIZE]; + crypto_ed25519_sign(SH.hash, SH.hash_len, + NULL, 0, + main_signature); - ret = cbor_encode_int(&map, 1); + ret = cbor_encode_int(&map, SH_RESP_signature); check_ret(ret); - ret = cbor_encode_byte_string(&map, sigder, 64); + ret = cbor_encode_byte_string(&map, main_signature, EDDSA_SIGNATURE_SIZE); check_ret(ret); if (SH.trusted_comment_present) { - crypto_ed25519_sign(sigder, 64, SH.trusted_comment, SH.trusted_comment_len, sigder); + crypto_ed25519_sign(main_signature, EDDSA_SIGNATURE_SIZE, + SH.trusted_comment, SH.trusted_comment_len, + main_signature); - ret = cbor_encode_int(&map, 2); + ret = cbor_encode_int(&map, SH_RESP_global_signature); check_ret(ret); - ret = cbor_encode_byte_string(&map, sigder, 64); + ret = cbor_encode_byte_string(&map, main_signature, EDDSA_SIGNATURE_SIZE); check_ret(ret); } + } else if (cose_alg == COSE_ALG_ES256) { - // Enforce 32-byte hash (ES256: SHA-256) - if (SH.hash_len != 32) + if (SH.hash_len != SIGN_HASH_HASH_ES256_SIZE) { printf2(TAG_ERR, "Error, invalid hash length for ES256, must be 32 B\n"); return CTAP1_ERR_INVALID_LENGTH; @@ -1510,19 +1514,20 @@ uint8_t ctap_sign_hash(CborEncoder * encoder, uint8_t * request, int length) if (SH.trusted_comment_present) { - printf2(TAG_ERR,"error, trusted comment is only supported with EdDSA\n"); + printf2(TAG_ERR, "Error, trusted comment is only supported with EdDSA\n"); return CTAP2_ERR_INVALID_OPTION; } crypto_ecc256_load_key((uint8_t*)&SH.cred.credential.id, cred_size, NULL, 0); - crypto_ecc256_sign(SH.hash, SH.hash_len, sigbuf); - int sigder_sz = ctap_encode_der_sig(sigbuf,sigder); - printf1(TAG_SH,"der sig [%d]: ", sigder_sz); dump_hex1(TAG_SH, sigder, sigder_sz); + uint8_t tmp_sigbuf[64], signature_der[72]; + crypto_ecc256_sign(SH.hash, SH.hash_len, tmp_sigbuf); + int sigder_sz = ctap_encode_der_sig(tmp_sigbuf, signature_der); - ret = cbor_encode_int(&map, 1); + ret = cbor_encode_int(&map, SH_RESP_signature); check_ret(ret); - ret = cbor_encode_byte_string(&map, sigder, sigder_sz); + ret = cbor_encode_byte_string(&map, signature_der, sigder_sz); check_ret(ret); + } else { diff --git a/fido2/ctap.h b/fido2/ctap.h index 25a26cfd..dbb2a9ae 100644 --- a/fido2/ctap.h +++ b/fido2/ctap.h @@ -45,6 +45,9 @@ #define SH_pinAuth 0x03 #define SH_trustedComment 0x04 #define SH_rpId 0x05 +#define SH_RESP_signature 0x01 +#define SH_RESP_global_signature 0x02 + #define CM_cmd 0x01 #define CM_cmdMetadata 0x01 #define CM_cmdRPBegin 0x02 @@ -130,8 +133,12 @@ #define DISPLAY_NAME_LIMIT 32 // Must be minimum of 64 bytes but can be more. #define ICON_LIMIT 128 // Must be minimum of 64 bytes but can be more. #define CTAP_MAX_MESSAGE_SIZE 1200 -#define SIGN_HASH_HASH_MAX_SIZE 64 //up to 512 bits -#define SIGN_HASH_TRUSTED_COMMENT_MAX_SIZE 128 +#define EDDSA_SIGNATURE_SIZE 64 + +#define SIGN_HASH_HASH_EDDSA_SIZE 64 // Minisign ED: Blake2b-512 +#define SIGN_HASH_HASH_ES256_SIZE 32 // ES256: SHA-256 +#define SIGN_HASH_HASH_MAX_SIZE 64 // Up to 512 bits +#define SIGN_HASH_TRUSTED_COMMENT_MAX_SIZE 128 #define CREDENTIAL_RK_FLASH_PAD 2 // size of RK should be 8-byte aligned to store in flash easily. #define CREDENTIAL_TAG_SIZE 16 From c26a983d8b435ed627cab807420fb8c2017bd658 Mon Sep 17 00:00:00 2001 From: stevenwdv Date: Wed, 27 Oct 2021 18:15:21 +0200 Subject: [PATCH 7/8] sign-hash: simplify prefix check --- fido2/ctap.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/fido2/ctap.c b/fido2/ctap.c index 457bf47e..c9da95ea 100644 --- a/fido2/ctap.c +++ b/fido2/ctap.c @@ -1449,9 +1449,7 @@ uint8_t ctap_sign_hash(CborEncoder * encoder, uint8_t * request, int length) { const char * sign_hash_prefix = "solo-sign-hash:"; - const char * rpId = (const char*)SH.rp.id; - const char * colon = strchr(rpId, ':'); - if (! colon || strncmp(rpId, sign_hash_prefix, colon - rpId + 1) != 0) + if (strncmp((const char*)SH.rp.id, sign_hash_prefix, strlen(sign_hash_prefix)) != 0) { printf2(TAG_ERR, "Error: invalid RP ID, should start with 'solo-sign-hash:'\n"); return CTAP2_ERR_INVALID_CREDENTIAL; From fd8033bf45ad2089392f36a88078bc8d2af4182e Mon Sep 17 00:00:00 2001 From: stevenwdv Date: Fri, 29 Oct 2021 18:57:11 +0200 Subject: [PATCH 8/8] Disallow solo-sign-hash RP ID in ctap_get_assertion --- fido2/ctap.c | 34 +++++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/fido2/ctap.c b/fido2/ctap.c index c9da95ea..2082909d 100644 --- a/fido2/ctap.c +++ b/fido2/ctap.c @@ -1165,6 +1165,14 @@ static int add_existing_user_info(CTAP_credentialDescriptor * cred) return 0; } +// @rpId NULL-terminated RP ID +// @return true if rpId starts with the solo-sign-hash prefix +bool is_solo_sign_rpid(const uint8_t * const rpId) +{ + const char sign_hash_prefix[] = "solo-sign-hash:"; + return strncmp((const char*)rpId, sign_hash_prefix, sizeof(sign_hash_prefix) - 1) == 0; +} + // @return the number of valid credentials // sorts the credentials. Most recent creds will be first, invalid ones last. int ctap_filter_invalid_credentials(CTAP_getAssertion * GA) @@ -1447,19 +1455,16 @@ uint8_t ctap_sign_hash(CborEncoder * encoder, uint8_t * request, int length) ret = ctap2_user_presence_test(); check_retr(ret); + if (! is_solo_sign_rpid(SH.rp.id)) { - const char * sign_hash_prefix = "solo-sign-hash:"; - if (strncmp((const char*)SH.rp.id, sign_hash_prefix, strlen(sign_hash_prefix)) != 0) - { - printf2(TAG_ERR, "Error: invalid RP ID, should start with 'solo-sign-hash:'\n"); - return CTAP2_ERR_INVALID_CREDENTIAL; - } + printf2(TAG_ERR, "Error: invalid RP ID, should start with 'solo-sign-hash:'\n"); + return CTAP2_ERR_INVALID_CREDENTIAL; + } - if (! ctap_authenticate_credential(&SH.rp, &SH.cred)) - { - printf2(TAG_ERR, "Error: invalid credential\n"); - return CTAP2_ERR_INVALID_CREDENTIAL; - } + if (! ctap_authenticate_credential(&SH.rp, &SH.cred)) + { + printf2(TAG_ERR, "Error: invalid credential\n"); + return CTAP2_ERR_INVALID_CREDENTIAL; } CborEncoder map; @@ -1957,6 +1962,13 @@ uint8_t ctap_get_assertion(CborEncoder * encoder, uint8_t * request, int length) { return CTAP2_ERR_MISSING_PARAMETER; } + + if (is_solo_sign_rpid(GA.rp.id)) + { + printf2(TAG_ERR, "Error: solo-sign-hash RP ID cannot be used with getAssertion\n"); + return CTAP2_ERR_INVALID_CREDENTIAL; + } + CborEncoder map; int map_size = 3;