Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add EdDSA/Minisign signatures to #397 (sign-hash/sign-file CTAP command) #583

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
7 changes: 5 additions & 2 deletions fido2/crypto.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need to check, memcpy with zero length is fine

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Normally yes, but I call it with data2 == NULL, which would be UB even with len2 == 0. I could call it passing data1 twice, but I think this is cleaner.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case the check should be if (data2 != NULL)

Copy link
Author

@stevenwdv stevenwdv Oct 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's also possible, but personally I think it looks weird because this would make the following a legal call:

crypto_ed25519_sign(some_data, some_len, NULL, 100, buf)

Although to be fair, with the current conditional the following is legal:

crypto_ed25519_sign(some_data, some_len, (uint8_t*)123, 0, buf)

We could have if (data2 || len2) (or maybe if (data && len2) if we want to be lenient), but in any case it will only matter with invalid parameters.
Or maybe I'm missing an advantage of if (data2)?

{
memcpy(data + len1, data2, len2);
}

#if defined(STM32L432xx)

Expand Down
128 changes: 128 additions & 0 deletions fido2/ctap.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -1429,6 +1437,112 @@ 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;
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_ex(SH.pinAuth, SH.hash, SH.hash_len);
check_retr(ret);
}
ret = ctap2_user_presence_test();
check_retr(ret);

if (! is_solo_sign_rpid(SH.rp.id))
{
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;
}

CborEncoder map;
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.type);

int32_t cose_alg = read_cose_alg_from_masked_credential(&SH.cred.credential.id);
if (cose_alg == COSE_ALG_EDDSA)
stevenwdv marked this conversation as resolved.
Show resolved Hide resolved
{
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;
}

crypto_ed25519_load_key((uint8_t*)&SH.cred.credential.id, cred_size);

uint8_t main_signature[EDDSA_SIGNATURE_SIZE];
crypto_ed25519_sign(SH.hash, SH.hash_len,
NULL, 0,
main_signature);

ret = cbor_encode_int(&map, SH_RESP_signature);
check_ret(ret);
ret = cbor_encode_byte_string(&map, main_signature, EDDSA_SIGNATURE_SIZE);
check_ret(ret);

if (SH.trusted_comment_present)
{
crypto_ed25519_sign(main_signature, EDDSA_SIGNATURE_SIZE,
SH.trusted_comment, SH.trusted_comment_len,
main_signature);

ret = cbor_encode_int(&map, SH_RESP_global_signature);
check_ret(ret);
ret = cbor_encode_byte_string(&map, main_signature, EDDSA_SIGNATURE_SIZE);
check_ret(ret);
}

}
else if (cose_alg == COSE_ALG_ES256)
{
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;
}

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);
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, SH_RESP_signature);
check_ret(ret);
ret = cbor_encode_byte_string(&map, signature_der, 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);
return 0;
}

uint8_t ctap_cred_metadata(CborEncoder * encoder)
{
CborEncoder map;
Expand Down Expand Up @@ -1848,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;
Expand Down Expand Up @@ -2304,6 +2425,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())
Expand Down Expand Up @@ -2387,6 +2509,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");
Expand Down
30 changes: 30 additions & 0 deletions fido2/ctap.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -39,6 +40,14 @@
#define GA_pinAuth 0x06
#define GA_pinProtocol 0x07

#define SH_hash 0x01
#define SH_credential 0x02
#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
Expand Down Expand Up @@ -124,6 +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 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
Expand Down Expand Up @@ -322,6 +337,21 @@ typedef struct

} CTAP_getAssertion;

typedef struct
{
uint8_t pinAuth[16];

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;
struct rpId rp;
} CTAP_signHash;

typedef struct
{
int cmd;
Expand Down
112 changes: 112 additions & 0 deletions fido2/ctap_parse.c
Original file line number Diff line number Diff line change
Expand Up @@ -1008,6 +1008,118 @@ 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_hash:
printf1(TAG_SH, "SH_hash\n");

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 long\n");
return CTAP1_ERR_INVALID_LENGTH;
}
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;

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);
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;
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);
}
return 0;
}

static uint8_t parse_cred_mgmt_subcommandparams(CborValue * val, CTAP_credMgmt * CM)
{
size_t map_length;
Expand Down
1 change: 1 addition & 0 deletions fido2/ctap_parse.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions fido2/log.c
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ struct logtag tagtable[] = {
{TAG_NFC_APDU, "NAPDU"},
{TAG_CCID, "CCID"},
{TAG_CM, "CRED_MGMT"},
{TAG_SH, "SH"},
};


Expand Down
1 change: 1 addition & 0 deletions fido2/log.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down