From 9b4b401fd286c29d8e14c84fe01901bd13cba942 Mon Sep 17 00:00:00 2001 From: Ludvig Michaelsson Date: Tue, 9 Jan 2024 13:06:52 +0100 Subject: [PATCH 01/10] cbor: cbor_encode_pubkey_param() in two One function to encode the array and an internal helper to encode each member of the array. Co-Authored-By: Mofidul Jamal --- src/cbor.c | 50 ++++++++++++++++++++++++++++++++++++++------------ src/cred.c | 2 +- src/extern.h | 4 ++-- src/touch.c | 4 ++-- 4 files changed, 43 insertions(+), 17 deletions(-) diff --git a/src/cbor.c b/src/cbor.c index b242cfa9..4b4d3230 100644 --- a/src/cbor.c +++ b/src/cbor.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2018-2022 Yubico AB. All rights reserved. + * Copyright (c) 2018-2024 Yubico AB. All rights reserved. * Use of this source code is governed by a BSD-style * license that can be found in the LICENSE file. * SPDX-License-Identifier: BSD-2-Clause @@ -468,18 +468,16 @@ cbor_encode_user_entity(const fido_user_t *user) return (item); } -cbor_item_t * +static cbor_item_t * cbor_encode_pubkey_param(int cose_alg) { - cbor_item_t *item = NULL; cbor_item_t *body = NULL; struct cbor_pair alg; int ok = -1; memset(&alg, 0, sizeof(alg)); - if ((item = cbor_new_definite_array(1)) == NULL || - (body = cbor_new_definite_map(2)) == NULL || + if ((body = cbor_new_definite_map(2)) == NULL || cose_alg > -1 || cose_alg < INT16_MIN) goto fail; @@ -496,26 +494,54 @@ cbor_encode_pubkey_param(int cose_alg) } if (cbor_map_add(body, alg) == false || - cbor_add_string(body, "type", "public-key") < 0 || - cbor_array_push(item, body) == false) + cbor_add_string(body, "type", "public-key") < 0) goto fail; ok = 0; fail: if (ok < 0) { - if (item != NULL) { - cbor_decref(&item); - item = NULL; + if (body != NULL) { + cbor_decref(&body); + body = NULL; } } - if (body != NULL) - cbor_decref(&body); if (alg.key != NULL) cbor_decref(&alg.key); if (alg.value != NULL) cbor_decref(&alg.value); + return (body); +} + +cbor_item_t * +cbor_encode_pubkey_param_array(int cose_alg) +{ + cbor_item_t *item = NULL; + cbor_item_t *body = NULL; + bool r = false; + + if ((item = cbor_new_definite_array(1)) == NULL) { + fido_log_debug("%s: cbor_new_definite_array", __func__); + goto fail; + } + + if ((body = cbor_encode_pubkey_param(cose_alg)) == NULL) { + fido_log_debug("%s: cbor_encode_pubkey_param", __func__); + goto fail; + } + + r = cbor_array_push(item, body); + cbor_decref(&body); + +fail: + if (r != true) { + if (item != NULL) { + cbor_decref(&item); + item = NULL; + } + } + return (item); } diff --git a/src/cred.c b/src/cred.c index 1fb0dfbd..491b8f16 100644 --- a/src/cred.c +++ b/src/cred.c @@ -74,7 +74,7 @@ fido_dev_make_cred_tx(fido_dev_t *dev, fido_cred_t *cred, const char *pin, if ((argv[0] = fido_blob_encode(&cred->cdh)) == NULL || (argv[1] = cbor_encode_rp_entity(&cred->rp)) == NULL || (argv[2] = cbor_encode_user_entity(&cred->user)) == NULL || - (argv[3] = cbor_encode_pubkey_param(cred->type)) == NULL) { + (argv[3] = cbor_encode_pubkey_param_array(cred->type)) == NULL) { fido_log_debug("%s: cbor encode", __func__); r = FIDO_ERR_INTERNAL; goto fail; diff --git a/src/extern.h b/src/extern.h index 93b209d4..172e8506 100644 --- a/src/extern.h +++ b/src/extern.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2018-2022 Yubico AB. All rights reserved. + * Copyright (c) 2018-2024 Yubico AB. All rights reserved. * Use of this source code is governed by a BSD-style * license that can be found in the LICENSE file. * SPDX-License-Identifier: BSD-2-Clause @@ -50,7 +50,7 @@ cbor_item_t *cbor_encode_pin_auth(const fido_dev_t *, const fido_blob_t *, cbor_item_t *cbor_encode_pin_opt(const fido_dev_t *); cbor_item_t *cbor_encode_pubkey(const fido_blob_t *); cbor_item_t *cbor_encode_pubkey_list(const fido_blob_array_t *); -cbor_item_t *cbor_encode_pubkey_param(int); +cbor_item_t *cbor_encode_pubkey_param_array(int); cbor_item_t *cbor_encode_rp_entity(const fido_rp_t *); cbor_item_t *cbor_encode_str_array(const fido_str_array_t *); cbor_item_t *cbor_encode_user_entity(const fido_user_t *); diff --git a/src/touch.c b/src/touch.c index 6844e2c2..bb205a4e 100644 --- a/src/touch.c +++ b/src/touch.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2018-2022 Yubico AB. All rights reserved. + * Copyright (c) 2018-2024 Yubico AB. All rights reserved. * Use of this source code is governed by a BSD-style * license that can be found in the LICENSE file. * SPDX-License-Identifier: BSD-2-Clause @@ -49,7 +49,7 @@ fido_dev_get_touch_begin(fido_dev_t *dev) if ((argv[0] = cbor_build_bytestring(cdh, sizeof(cdh))) == NULL || (argv[1] = cbor_encode_rp_entity(&rp)) == NULL || (argv[2] = cbor_encode_user_entity(&user)) == NULL || - (argv[3] = cbor_encode_pubkey_param(COSE_ES256)) == NULL) { + (argv[3] = cbor_encode_pubkey_param_array(COSE_ES256)) == NULL) { fido_log_debug("%s: cbor encode", __func__); goto fail; } From e6d5e0cd4bb4682eb52069ea92384df56d8daaf3 Mon Sep 17 00:00:00 2001 From: Ludvig Michaelsson Date: Tue, 9 Jan 2024 13:51:58 +0100 Subject: [PATCH 02/10] winhello: split pack_cose() in two One function to encode the array and a helper to translate each parameter from the libfido2 representation to the winhello representation. Co-Authored-By: Mofidul Jamal --- src/winhello.c | 31 ++++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/src/winhello.c b/src/winhello.c index 7805976b..7c17d34c 100644 --- a/src/winhello.c +++ b/src/winhello.c @@ -43,7 +43,6 @@ struct winhello_assert { struct winhello_cred { WEBAUTHN_RP_ENTITY_INFORMATION rp; WEBAUTHN_USER_ENTITY_INFORMATION user; - WEBAUTHN_COSE_CREDENTIAL_PARAMETER alg; WEBAUTHN_COSE_CREDENTIAL_PARAMETERS cose; WEBAUTHN_CLIENT_DATA cd; WEBAUTHN_AUTHENTICATOR_MAKE_CREDENTIAL_OPTIONS opt; @@ -355,9 +354,11 @@ pack_user(wchar_t **name, wchar_t **icon, wchar_t **display_name, } static int -pack_cose(WEBAUTHN_COSE_CREDENTIAL_PARAMETER *alg, - WEBAUTHN_COSE_CREDENTIAL_PARAMETERS *cose, int type) +pack_cose(WEBAUTHN_COSE_CREDENTIAL_PARAMETER *alg, int type) { + alg->dwVersion = WEBAUTHN_COSE_CREDENTIAL_PARAMETER_CURRENT_VERSION; + alg->pwszCredentialType = WEBAUTHN_CREDENTIAL_TYPE_PUBLIC_KEY; + switch (type) { case COSE_ES256: alg->lAlg = WEBAUTHN_COSE_ALGORITHM_ECDSA_P256_WITH_SHA256; @@ -366,7 +367,7 @@ pack_cose(WEBAUTHN_COSE_CREDENTIAL_PARAMETER *alg, alg->lAlg = WEBAUTHN_COSE_ALGORITHM_ECDSA_P384_WITH_SHA384; break; case COSE_EDDSA: - alg->lAlg = -8; /* XXX */; + alg->lAlg = -8; /* XXX */ break; case COSE_RS256: alg->lAlg = WEBAUTHN_COSE_ALGORITHM_RSASSA_PKCS1_V1_5_WITH_SHA256; @@ -375,10 +376,21 @@ pack_cose(WEBAUTHN_COSE_CREDENTIAL_PARAMETER *alg, fido_log_debug("%s: type %d", __func__, type); return -1; } - alg->dwVersion = WEBAUTHN_COSE_CREDENTIAL_PARAMETER_CURRENT_VERSION; - alg->pwszCredentialType = WEBAUTHN_CREDENTIAL_TYPE_PUBLIC_KEY; + + return 0; +} + +static int +pack_cose_array(WEBAUTHN_COSE_CREDENTIAL_PARAMETERS *cose, int type) +{ + if ((cose->pCredentialParameters = calloc(1, + sizeof(*cose->pCredentialParameters))) == NULL) { + fido_log_debug("%s: calloc", __func__); + return -1; + } cose->cCredentialParameters = 1; - cose->pCredentialParameters = alg; + if (pack_cose(cose->pCredentialParameters[0], type) != 0) + return -1; return 0; } @@ -705,8 +717,8 @@ translate_fido_cred(struct winhello_cred *ctx, const fido_cred_t *cred, fido_log_debug("%s: pack_user", __func__); return FIDO_ERR_INTERNAL; } - if (pack_cose(&ctx->alg, &ctx->cose, cred->type) < 0) { - fido_log_debug("%s: pack_cose", __func__); + if (pack_cose_array(&ctx->cose, cred->type) < 0) { + fido_log_debug("%s: pack_cose_array", __func__); return FIDO_ERR_INTERNAL; } if (pack_cd(&ctx->cd, &cred->cd) < 0) { @@ -844,6 +856,7 @@ winhello_cred_free(struct winhello_cred *ctx) free(e->pvExtension); } free(ctx->opt.Extensions.pExtensions); + free(ctx->cose.pCredentialParameters); free(ctx); } From 42563e88a23f3e03c5bfcdb4c55332ff0149034e Mon Sep 17 00:00:00 2001 From: Ludvig Michaelsson Date: Tue, 9 Jan 2024 13:13:53 +0100 Subject: [PATCH 03/10] cred: fido_cred_set_type() returns attested type Our routines check that the resulting credential is of the same type as the requested type. This means that this not a functional change. Co-Authored-By: Mofidul Jamal --- src/cred.c | 5 ++++- src/credman.c | 9 +++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/cred.c b/src/cred.c index 491b8f16..d9ae34a0 100644 --- a/src/cred.c +++ b/src/cred.c @@ -1087,7 +1087,10 @@ fido_cred_set_type(fido_cred_t *cred, int cose_alg) int fido_cred_type(const fido_cred_t *cred) { - return (cred->type); + if (cred->attcred.type != 0) + return (cred->attcred.type); + + return (cred->type); /* compat: return requested type */ } uint8_t diff --git a/src/credman.c b/src/credman.c index 898de148..43cf8776 100644 --- a/src/credman.c +++ b/src/credman.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019-2022 Yubico AB. All rights reserved. + * Copyright (c) 2019-2024 Yubico AB. All rights reserved. * Use of this source code is governed by a BSD-style * license that can be found in the LICENSE file. * SPDX-License-Identifier: BSD-2-Clause @@ -281,11 +281,8 @@ credman_parse_rk(const cbor_item_t *key, const cbor_item_t *val, void *arg) case 7: return (cbor_decode_cred_id(val, &cred->attcred.id)); case 8: - if (cbor_decode_pubkey(val, &cred->attcred.type, - &cred->attcred.pubkey) < 0) - return (-1); - cred->type = cred->attcred.type; /* XXX */ - return (0); + return (cbor_decode_pubkey(val, &cred->attcred.type, + &cred->attcred.pubkey)); case 10: if (cbor_decode_uint64(val, &prot) < 0 || prot > INT_MAX || fido_cred_set_prot(cred, (int)prot) != FIDO_OK) From 0825df294f17cf74436ba685df34648fc992b152 Mon Sep 17 00:00:00 2001 From: Ludvig Michaelsson Date: Tue, 9 Jan 2024 13:36:10 +0100 Subject: [PATCH 04/10] types: add fido_int_array_t While here, also add `fido_int_array_contains()` helper function. Co-Authored-By: Mofidul Jamal --- src/extern.h | 1 + src/fido/types.h | 5 +++++ src/types.c | 9 +++++++++ 3 files changed, 15 insertions(+) diff --git a/src/extern.h b/src/extern.h index 172e8506..2ebef61b 100644 --- a/src/extern.h +++ b/src/extern.h @@ -203,6 +203,7 @@ void fido_cert_array_free(fido_cert_array_t *); void fido_opt_array_free(fido_opt_array_t *); void fido_str_array_free(fido_str_array_t *); void fido_algo_free(fido_algo_t *); +bool fido_int_array_contains(const fido_int_array_t *, int); int fido_str_array_pack(fido_str_array_t *, const char * const *, size_t); /* misc */ diff --git a/src/fido/types.h b/src/fido/types.h index ab9b02a6..26e2abfd 100644 --- a/src/fido/types.h +++ b/src/fido/types.h @@ -171,6 +171,11 @@ typedef struct fido_cred_ea { bool att; } fido_cred_ea_t; +typedef struct fido_int_array { + int *ptr; + size_t len; +} fido_int_array_t; + typedef struct fido_cred { fido_blob_t cd; /* client data */ fido_blob_t cdh; /* client data hash */ diff --git a/src/types.c b/src/types.c index f31f8da1..c841c24e 100644 --- a/src/types.c +++ b/src/types.c @@ -89,3 +89,12 @@ fido_str_array_pack(fido_str_array_t *sa, const char * const *v, size_t n) return 0; } + +bool +fido_int_array_contains(const fido_int_array_t *ia, int v) +{ + for (size_t i = 0; i < ia->len; i++) + if (ia->ptr[i] == v) + return true; + return false; +} From e3159df864ae24025c3e8555f51cc7ce7aff00bd Mon Sep 17 00:00:00 2001 From: Ludvig Michaelsson Date: Mon, 17 Jun 2024 13:47:04 +0200 Subject: [PATCH 05/10] misc: adopt fido_int_array_t in alg plumbing Co-Authored-By: Mofidul Jamal --- src/cbor.c | 31 +++++++++++++------------------ src/cred.c | 3 ++- src/extern.h | 2 +- src/touch.c | 4 +++- src/winhello.c | 22 ++++++++++++++++------ 5 files changed, 35 insertions(+), 27 deletions(-) diff --git a/src/cbor.c b/src/cbor.c index 4b4d3230..dcd75d17 100644 --- a/src/cbor.c +++ b/src/cbor.c @@ -515,34 +515,29 @@ cbor_encode_pubkey_param(int cose_alg) } cbor_item_t * -cbor_encode_pubkey_param_array(int cose_alg) +cbor_encode_pubkey_param_array(const fido_int_array_t *algs) { cbor_item_t *item = NULL; cbor_item_t *body = NULL; - bool r = false; - if ((item = cbor_new_definite_array(1)) == NULL) { - fido_log_debug("%s: cbor_new_definite_array", __func__); + if ((item = cbor_new_definite_array(algs->len)) == NULL) goto fail; - } - if ((body = cbor_encode_pubkey_param(cose_alg)) == NULL) { - fido_log_debug("%s: cbor_encode_pubkey_param", __func__); - goto fail; + for (size_t i = 0; i < algs->len; i++) { + if ((body = cbor_encode_pubkey_param(algs->ptr[i])) == NULL || + cbor_array_push(item, body) == false) + goto fail; + cbor_decref(&body); } - r = cbor_array_push(item, body); - cbor_decref(&body); - + return (item); fail: - if (r != true) { - if (item != NULL) { - cbor_decref(&item); - item = NULL; - } - } + if (body != NULL) + cbor_decref(&body); + if (item != NULL) + cbor_decref(&item); - return (item); + return (NULL); } cbor_item_t * diff --git a/src/cred.c b/src/cred.c index d9ae34a0..5430763a 100644 --- a/src/cred.c +++ b/src/cred.c @@ -58,6 +58,7 @@ fido_dev_make_cred_tx(fido_dev_t *dev, fido_cred_t *cred, const char *pin, fido_opt_t uv = cred->uv; es256_pk_t *pk = NULL; cbor_item_t *argv[10]; + const fido_int_array_t algs = { &cred->type, 1 }; const uint8_t cmd = CTAP_CBOR_MAKECRED; int r; @@ -74,7 +75,7 @@ fido_dev_make_cred_tx(fido_dev_t *dev, fido_cred_t *cred, const char *pin, if ((argv[0] = fido_blob_encode(&cred->cdh)) == NULL || (argv[1] = cbor_encode_rp_entity(&cred->rp)) == NULL || (argv[2] = cbor_encode_user_entity(&cred->user)) == NULL || - (argv[3] = cbor_encode_pubkey_param_array(cred->type)) == NULL) { + (argv[3] = cbor_encode_pubkey_param_array(&algs)) == NULL) { fido_log_debug("%s: cbor encode", __func__); r = FIDO_ERR_INTERNAL; goto fail; diff --git a/src/extern.h b/src/extern.h index 2ebef61b..07f2f3dc 100644 --- a/src/extern.h +++ b/src/extern.h @@ -50,7 +50,7 @@ cbor_item_t *cbor_encode_pin_auth(const fido_dev_t *, const fido_blob_t *, cbor_item_t *cbor_encode_pin_opt(const fido_dev_t *); cbor_item_t *cbor_encode_pubkey(const fido_blob_t *); cbor_item_t *cbor_encode_pubkey_list(const fido_blob_array_t *); -cbor_item_t *cbor_encode_pubkey_param_array(int); +cbor_item_t *cbor_encode_pubkey_param_array(const fido_int_array_t *); cbor_item_t *cbor_encode_rp_entity(const fido_rp_t *); cbor_item_t *cbor_encode_str_array(const fido_str_array_t *); cbor_item_t *cbor_encode_user_entity(const fido_user_t *); diff --git a/src/touch.c b/src/touch.c index bb205a4e..91866bfe 100644 --- a/src/touch.c +++ b/src/touch.c @@ -20,6 +20,8 @@ fido_dev_get_touch_begin(fido_dev_t *dev) fido_user_t user; int ms = dev->timeout_ms; int r = FIDO_ERR_INTERNAL; + int alg = COSE_ES256; + const fido_int_array_t algs = { &alg, 1 }; memset(&f, 0, sizeof(f)); memset(argv, 0, sizeof(argv)); @@ -49,7 +51,7 @@ fido_dev_get_touch_begin(fido_dev_t *dev) if ((argv[0] = cbor_build_bytestring(cdh, sizeof(cdh))) == NULL || (argv[1] = cbor_encode_rp_entity(&rp)) == NULL || (argv[2] = cbor_encode_user_entity(&user)) == NULL || - (argv[3] = cbor_encode_pubkey_param_array(COSE_ES256)) == NULL) { + (argv[3] = cbor_encode_pubkey_param_array(&algs)) == NULL) { fido_log_debug("%s: cbor encode", __func__); goto fail; } diff --git a/src/winhello.c b/src/winhello.c index 7c17d34c..0d940719 100644 --- a/src/winhello.c +++ b/src/winhello.c @@ -381,16 +381,25 @@ pack_cose(WEBAUTHN_COSE_CREDENTIAL_PARAMETER *alg, int type) } static int -pack_cose_array(WEBAUTHN_COSE_CREDENTIAL_PARAMETERS *cose, int type) +pack_cose_array(WEBAUTHN_COSE_CREDENTIAL_PARAMETERS *cose, + const fido_int_array_t *algs) { - if ((cose->pCredentialParameters = calloc(1, + if (algs->ptr == NULL || algs->len > ULONG_MAX) { + fido_log_debug("%s: algs (%p, %zu)", __func__, + (void *) algs->ptr, algs->len); + return -1; + } + if ((cose->pCredentialParameters = calloc(algs->len, sizeof(*cose->pCredentialParameters))) == NULL) { fido_log_debug("%s: calloc", __func__); return -1; } - cose->cCredentialParameters = 1; - if (pack_cose(cose->pCredentialParameters[0], type) != 0) - return -1; + for (size_t i = 0; i < algs->len; i++) { + if (pack_cose(&cose->pCredentialParameters[i], + algs->ptr[i]) != 0) + return -1; + cose->cCredentialParameters++; + } return 0; } @@ -707,6 +716,7 @@ translate_fido_cred(struct winhello_cred *ctx, const fido_cred_t *cred, const char *pin, int ms) { WEBAUTHN_AUTHENTICATOR_MAKE_CREDENTIAL_OPTIONS *opt; + const fido_int_array_t algs = { &cred->type, 1 }; if (pack_rp(&ctx->rp_id, &ctx->rp_name, &ctx->rp, &cred->rp) < 0) { fido_log_debug("%s: pack_rp", __func__); @@ -717,7 +727,7 @@ translate_fido_cred(struct winhello_cred *ctx, const fido_cred_t *cred, fido_log_debug("%s: pack_user", __func__); return FIDO_ERR_INTERNAL; } - if (pack_cose_array(&ctx->cose, cred->type) < 0) { + if (pack_cose_array(&ctx->cose, &algs) < 0) { fido_log_debug("%s: pack_cose_array", __func__); return FIDO_ERR_INTERNAL; } From 85822e5e9d2b9fb225bb89ad75ffb0dd2522a54f Mon Sep 17 00:00:00 2001 From: Ludvig Michaelsson Date: Mon, 17 Jun 2024 14:04:19 +0200 Subject: [PATCH 06/10] cred: adopt fido_int_array_t in cred type Co-Authored-By: Mofidul Jamal --- src/cbor.c | 16 +++++++-------- src/cred.c | 52 +++++++++++++++++++++++++++++++++--------------- src/extern.h | 4 ++-- src/fido/types.h | 2 +- src/u2f.c | 15 +++++++++----- src/winhello.c | 3 +-- 6 files changed, 58 insertions(+), 34 deletions(-) diff --git a/src/cbor.c b/src/cbor.c index dcd75d17..874d1582 100644 --- a/src/cbor.c +++ b/src/cbor.c @@ -1094,8 +1094,8 @@ cbor_decode_pubkey(const cbor_item_t *item, int *type, void *key) } static int -decode_attcred(const unsigned char **buf, size_t *len, int cose_alg, - fido_attcred_t *attcred) +decode_attcred(const unsigned char **buf, size_t *len, + const fido_int_array_t *algs, fido_attcred_t *attcred) { cbor_item_t *item = NULL; struct cbor_load_result cbor; @@ -1136,9 +1136,9 @@ decode_attcred(const unsigned char **buf, size_t *len, int cose_alg, goto fail; } - if (attcred->type != cose_alg) { - fido_log_debug("%s: cose_alg mismatch (%d != %d)", __func__, - attcred->type, cose_alg); + if (!fido_int_array_contains(algs, attcred->type)) { + fido_log_debug("%s: unexpected cose alg (%d)", __func__, + attcred->type); goto fail; } @@ -1181,7 +1181,7 @@ decode_attobj(const cbor_item_t *key, const cbor_item_t *val, void *arg) fido_log_debug("%s: fido_blob_decode", __func__); goto fail; } - if (cbor_decode_cred_authdata(val, cred->type, + if (cbor_decode_cred_authdata(val, &cred->type, &cred->authdata_cbor, &cred->authdata, &cred->attcred, &cred->authdata_ext) < 0) { fido_log_debug("%s: cbor_decode_cred_authdata", @@ -1367,7 +1367,7 @@ decode_assert_extensions(const unsigned char **buf, size_t *len, } int -cbor_decode_cred_authdata(const cbor_item_t *item, int cose_alg, +cbor_decode_cred_authdata(const cbor_item_t *item, const fido_int_array_t *algs, fido_blob_t *authdata_cbor, fido_authdata_t *authdata, fido_attcred_t *attcred, fido_cred_ext_t *authdata_ext) { @@ -1401,7 +1401,7 @@ cbor_decode_cred_authdata(const cbor_item_t *item, int cose_alg, if (attcred != NULL) { if ((authdata->flags & CTAP_AUTHDATA_ATT_CRED) == 0 || - decode_attcred(&buf, &len, cose_alg, attcred) < 0) + decode_attcred(&buf, &len, algs, attcred) < 0) return (-1); } diff --git a/src/cred.c b/src/cred.c index 5430763a..661b1a42 100644 --- a/src/cred.c +++ b/src/cred.c @@ -34,7 +34,7 @@ parse_makecred_reply(const cbor_item_t *key, const cbor_item_t *val, void *arg) fido_log_debug("%s: fido_blob_decode", __func__); return (-1); } - return (cbor_decode_cred_authdata(val, cred->type, + return (cbor_decode_cred_authdata(val, &cred->type, &cred->authdata_cbor, &cred->authdata, &cred->attcred, &cred->authdata_ext)); case 3: /* attestation statement */ @@ -58,16 +58,15 @@ fido_dev_make_cred_tx(fido_dev_t *dev, fido_cred_t *cred, const char *pin, fido_opt_t uv = cred->uv; es256_pk_t *pk = NULL; cbor_item_t *argv[10]; - const fido_int_array_t algs = { &cred->type, 1 }; const uint8_t cmd = CTAP_CBOR_MAKECRED; int r; memset(&f, 0, sizeof(f)); memset(argv, 0, sizeof(argv)); - if (cred->cdh.ptr == NULL || cred->type == 0) { - fido_log_debug("%s: cdh=%p, type=%d", __func__, - (void *)cred->cdh.ptr, cred->type); + if (cred->cdh.ptr == NULL || cred->type.len == 0) { + fido_log_debug("%s: cdh=%p, type.len=%zu", __func__, + (void *)cred->cdh.ptr, cred->type.len); r = FIDO_ERR_INVALID_ARGUMENT; goto fail; } @@ -75,7 +74,7 @@ fido_dev_make_cred_tx(fido_dev_t *dev, fido_cred_t *cred, const char *pin, if ((argv[0] = fido_blob_encode(&cred->cdh)) == NULL || (argv[1] = cbor_encode_rp_entity(&cred->rp)) == NULL || (argv[2] = cbor_encode_user_entity(&cred->user)) == NULL || - (argv[3] = cbor_encode_pubkey_param_array(&algs)) == NULL) { + (argv[3] = cbor_encode_pubkey_param_array(&cred->type)) == NULL) { fido_log_debug("%s: cbor encode", __func__); r = FIDO_ERR_INTERNAL; goto fail; @@ -589,13 +588,14 @@ fido_cred_reset_tx(fido_cred_t *cred) free(cred->user.icon); free(cred->user.name); free(cred->user.display_name); + free(cred->type.ptr); fido_cred_empty_exclude_list(cred); memset(&cred->rp, 0, sizeof(cred->rp)); memset(&cred->user, 0, sizeof(cred->user)); memset(&cred->ext, 0, sizeof(cred->ext)); + memset(&cred->type, 0, sizeof(cred->type)); - cred->type = 0; cred->rk = FIDO_OPT_OMIT; cred->uv = FIDO_OPT_OMIT; cred->ea.mode = 0; @@ -644,7 +644,7 @@ fido_cred_set_authdata(fido_cred_t *cred, const unsigned char *ptr, size_t len) goto fail; } - if (cbor_decode_cred_authdata(item, cred->type, &cred->authdata_cbor, + if (cbor_decode_cred_authdata(item, &cred->type, &cred->authdata_cbor, &cred->authdata, &cred->attcred, &cred->authdata_ext) < 0) { fido_log_debug("%s: cbor_decode_cred_authdata", __func__); goto fail; @@ -685,7 +685,7 @@ fido_cred_set_authdata_raw(fido_cred_t *cred, const unsigned char *ptr, goto fail; } - if (cbor_decode_cred_authdata(item, cred->type, &cred->authdata_cbor, + if (cbor_decode_cred_authdata(item, &cred->type, &cred->authdata_cbor, &cred->authdata, &cred->attcred, &cred->authdata_ext) < 0) { fido_log_debug("%s: cbor_decode_cred_authdata", __func__); goto fail; @@ -1071,27 +1071,47 @@ fido_cred_set_fmt(fido_cred_t *cred, const char *fmt) return (FIDO_OK); } -int -fido_cred_set_type(fido_cred_t *cred, int cose_alg) +static int +fido_cred_append_type(fido_cred_t *cred, int cose_alg) { - if (cred->type != 0) - return (FIDO_ERR_INVALID_ARGUMENT); + int *list_ptr; + if (cose_alg != COSE_ES256 && cose_alg != COSE_ES384 && cose_alg != COSE_RS256 && cose_alg != COSE_EDDSA) return (FIDO_ERR_INVALID_ARGUMENT); - cred->type = cose_alg; + if (cred->type.len == SIZE_MAX) + return (FIDO_ERR_INVALID_ARGUMENT); + + if ((list_ptr = recallocarray(cred->type.ptr, cred->type.len, + cred->type.len + 1, sizeof(*list_ptr))) == NULL) + return (FIDO_ERR_INTERNAL); + + list_ptr[cred->type.len++] = cose_alg; + cred->type.ptr = list_ptr; return (FIDO_OK); } +int +fido_cred_set_type(fido_cred_t *cred, int cose_alg) +{ + if (cred->type.len != 0) + return (FIDO_ERR_INVALID_ARGUMENT); + + return (fido_cred_append_type(cred, cose_alg)); +} + int fido_cred_type(const fido_cred_t *cred) { - if (cred->attcred.type != 0) + if (cred->type.len == 0) + return (COSE_UNSPEC); + + if (cred->attcred.type != COSE_UNSPEC) return (cred->attcred.type); - return (cred->type); /* compat: return requested type */ + return (cred->type.ptr[0]); /* compat: return (first) requested type */ } uint8_t diff --git a/src/extern.h b/src/extern.h index 07f2f3dc..736e86d6 100644 --- a/src/extern.h +++ b/src/extern.h @@ -60,8 +60,8 @@ cbor_item_t *es256_pk_encode(const es256_pk_t *, int); int cbor_decode_attstmt(const cbor_item_t *, fido_attstmt_t *); int cbor_decode_attobj(const cbor_item_t *, fido_cred_t *); int cbor_decode_bool(const cbor_item_t *, bool *); -int cbor_decode_cred_authdata(const cbor_item_t *, int, fido_blob_t *, - fido_authdata_t *, fido_attcred_t *, fido_cred_ext_t *); +int cbor_decode_cred_authdata(const cbor_item_t *, const fido_int_array_t *, + fido_blob_t *, fido_authdata_t *, fido_attcred_t *, fido_cred_ext_t *); int cbor_decode_assert_authdata(const cbor_item_t *, fido_blob_t *, fido_authdata_t *, fido_assert_extattr_t *); int cbor_decode_cred_id(const cbor_item_t *, fido_blob_t *); diff --git a/src/fido/types.h b/src/fido/types.h index 26e2abfd..762c07fc 100644 --- a/src/fido/types.h +++ b/src/fido/types.h @@ -185,7 +185,7 @@ typedef struct fido_cred { fido_opt_t rk; /* resident key */ fido_opt_t uv; /* user verification */ fido_cred_ext_t ext; /* extensions */ - int type; /* cose algorithm */ + fido_int_array_t type; /* cose algorithm(s) */ char *fmt; /* credential format */ fido_cred_ext_t authdata_ext; /* decoded extensions */ fido_blob_t authdata_cbor; /* cbor-encoded payload */ diff --git a/src/u2f.c b/src/u2f.c index 2620a2eb..20031504 100644 --- a/src/u2f.c +++ b/src/u2f.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2018-2022 Yubico AB. All rights reserved. + * Copyright (c) 2018-2024 Yubico AB. All rights reserved. * Use of this source code is governed by a BSD-style * license that can be found in the LICENSE file. * SPDX-License-Identifier: BSD-2-Clause @@ -678,10 +678,15 @@ u2f_register(fido_dev_t *dev, fido_cred_t *cred, int *ms) return (FIDO_ERR_UNSUPPORTED_OPTION); } - if (cred->type != COSE_ES256 || cred->cdh.ptr == NULL || - cred->rp.id == NULL || cred->cdh.len != SHA256_DIGEST_LENGTH) { - fido_log_debug("%s: type=%d, cdh=(%p,%zu)" , __func__, - cred->type, (void *)cred->cdh.ptr, cred->cdh.len); + if (!fido_int_array_contains(&cred->type, COSE_ES256)) { + fido_log_debug("%s: type", __func__); + return (FIDO_ERR_INVALID_ARGUMENT); + } + + if (cred->cdh.ptr == NULL || cred->rp.id == NULL || + cred->cdh.len != SHA256_DIGEST_LENGTH) { + fido_log_debug("%s: cdh=(%p,%zu)" , __func__, + (void *)cred->cdh.ptr, cred->cdh.len); return (FIDO_ERR_INVALID_ARGUMENT); } diff --git a/src/winhello.c b/src/winhello.c index 0d940719..8a7f6c7c 100644 --- a/src/winhello.c +++ b/src/winhello.c @@ -716,7 +716,6 @@ translate_fido_cred(struct winhello_cred *ctx, const fido_cred_t *cred, const char *pin, int ms) { WEBAUTHN_AUTHENTICATOR_MAKE_CREDENTIAL_OPTIONS *opt; - const fido_int_array_t algs = { &cred->type, 1 }; if (pack_rp(&ctx->rp_id, &ctx->rp_name, &ctx->rp, &cred->rp) < 0) { fido_log_debug("%s: pack_rp", __func__); @@ -727,7 +726,7 @@ translate_fido_cred(struct winhello_cred *ctx, const fido_cred_t *cred, fido_log_debug("%s: pack_user", __func__); return FIDO_ERR_INTERNAL; } - if (pack_cose_array(&ctx->cose, &algs) < 0) { + if (pack_cose_array(&ctx->cose, &cred->type) < 0) { fido_log_debug("%s: pack_cose_array", __func__); return FIDO_ERR_INTERNAL; } From e2dedc04a61a361e74ef44a37c431119ca6129fa Mon Sep 17 00:00:00 2001 From: Ludvig Michaelsson Date: Tue, 9 Jan 2024 14:23:56 +0100 Subject: [PATCH 07/10] cred: export fido_cred_append_type() --- fuzz/export.gnu | 1 + src/cred.c | 2 +- src/export.gnu | 1 + src/export.llvm | 1 + src/export.msvc | 1 + src/fido.h | 1 + 6 files changed, 6 insertions(+), 1 deletion(-) diff --git a/fuzz/export.gnu b/fuzz/export.gnu index 62dfda92..c1ed4964 100644 --- a/fuzz/export.gnu +++ b/fuzz/export.gnu @@ -118,6 +118,7 @@ fido_cbor_info_uv_modality; fido_cbor_info_versions_len; fido_cbor_info_versions_ptr; + fido_cred_append_type; fido_cred_attstmt_len; fido_cred_attstmt_ptr; fido_cred_authdata_len; diff --git a/src/cred.c b/src/cred.c index 661b1a42..9315e439 100644 --- a/src/cred.c +++ b/src/cred.c @@ -1071,7 +1071,7 @@ fido_cred_set_fmt(fido_cred_t *cred, const char *fmt) return (FIDO_OK); } -static int +int fido_cred_append_type(fido_cred_t *cred, int cose_alg) { int *list_ptr; diff --git a/src/export.gnu b/src/export.gnu index f22d663b..72c32f35 100644 --- a/src/export.gnu +++ b/src/export.gnu @@ -120,6 +120,7 @@ fido_cbor_info_uv_modality; fido_cbor_info_versions_len; fido_cbor_info_versions_ptr; + fido_cred_append_type; fido_cred_attstmt_len; fido_cred_attstmt_ptr; fido_cred_authdata_len; diff --git a/src/export.llvm b/src/export.llvm index b1b1cdf3..e5a8d8b1 100644 --- a/src/export.llvm +++ b/src/export.llvm @@ -118,6 +118,7 @@ _fido_cbor_info_uv_attempts _fido_cbor_info_uv_modality _fido_cbor_info_versions_len _fido_cbor_info_versions_ptr +_fido_cred_append_type _fido_cred_attstmt_len _fido_cred_attstmt_ptr _fido_cred_authdata_len diff --git a/src/export.msvc b/src/export.msvc index 449f1eaf..cffce5ad 100644 --- a/src/export.msvc +++ b/src/export.msvc @@ -119,6 +119,7 @@ fido_cbor_info_uv_attempts fido_cbor_info_uv_modality fido_cbor_info_versions_len fido_cbor_info_versions_ptr +fido_cred_append_type fido_cred_attstmt_len fido_cred_attstmt_ptr fido_cred_authdata_len diff --git a/src/fido.h b/src/fido.h index 3f7f1e5b..e878a108 100644 --- a/src/fido.h +++ b/src/fido.h @@ -148,6 +148,7 @@ int fido_assert_set_sig(fido_assert_t *, size_t, const unsigned char *, size_t); int fido_assert_set_winhello_appid(fido_assert_t *, const char *); int fido_assert_verify(const fido_assert_t *, size_t, int, const void *); int fido_cbor_info_algorithm_cose(const fido_cbor_info_t *, size_t); +int fido_cred_append_type(fido_cred_t *, int); int fido_cred_empty_exclude_list(fido_cred_t *); bool fido_cred_entattest(const fido_cred_t *); int fido_cred_exclude(fido_cred_t *, const unsigned char *, size_t); From 425299d6467285ea3dd786d6fb242200f3c1a79f Mon Sep 17 00:00:00 2001 From: Ludvig Michaelsson Date: Thu, 27 Jun 2024 11:26:04 +0200 Subject: [PATCH 08/10] regress: add basic test for fido_cred_append_type() While here, use C style comments. --- regress/cred.c | 35 +++++++++++++++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/regress/cred.c b/regress/cred.c index a9be954d..43a17e12 100644 --- a/regress/cred.c +++ b/regress/cred.c @@ -2847,7 +2847,7 @@ valid_tpm_rs256_cred(bool xfail) assert(fido_cred_set_uv(c, FIDO_OPT_TRUE) == FIDO_OK); assert(fido_cred_set_fmt(c, "tpm") == FIDO_OK); assert(fido_cred_set_attstmt(c, attstmt_tpm_rs256, sizeof(attstmt_tpm_rs256)) == FIDO_OK); - // XXX: RHEL9 has deprecated SHA-1 for signing. + /* XXX: RHEL9 has deprecated SHA-1 for signing */ assert(fido_cred_verify(c) == (xfail ? FIDO_ERR_INVALID_SIG : FIDO_OK)); assert(fido_cred_prot(c) == 0); assert(fido_cred_pubkey_len(c) == sizeof(pubkey_tpm_rs256)); @@ -2880,7 +2880,7 @@ valid_tpm_es256_cred(bool xfail) assert(memcmp(fido_cred_x5c_list_ptr(c, 1), x509_1_tpm_es256, sizeof(x509_1_tpm_es256)) == 0); assert(fido_cred_x5c_list_len(c, 2) == 0); assert(fido_cred_x5c_list_ptr(c, 2) == NULL); - // XXX: RHEL9 has deprecated SHA-1 for signing. + /* XXX: RHEL9 has deprecated SHA-1 for signing */ assert(fido_cred_verify(c) == (xfail ? FIDO_ERR_INVALID_SIG : FIDO_OK)); assert(fido_cred_prot(c) == 0); assert(fido_cred_pubkey_len(c) == sizeof(pubkey_tpm_es256)); @@ -3021,6 +3021,36 @@ entattest(void) assert(fido_dev_close(dev) == FIDO_OK); fido_dev_free(&dev); } +static void +multiple_cose(void) +{ + fido_cred_t *c; + + c = alloc_cred(); + assert(fido_cred_type(c) == COSE_UNSPEC); + assert(fido_cred_append_type(c, COSE_EDDSA) == FIDO_OK); + assert(fido_cred_type(c) == COSE_EDDSA); /* compat: only algorithm req. */ + assert(fido_cred_append_type(c, COSE_ES256) == FIDO_OK); + assert(fido_cred_type(c) == COSE_EDDSA); /* compat: first algorithm req. */ + assert(fido_cred_set_clientdata_hash(c, cdh, sizeof(cdh)) == FIDO_OK); + assert(fido_cred_set_rp(c, rp_id, rp_name) == FIDO_OK); + assert(fido_cred_set_authdata(c, authdata, sizeof(authdata)) == FIDO_OK); + assert(fido_cred_set_rk(c, FIDO_OPT_FALSE) == FIDO_OK); + assert(fido_cred_set_uv(c, FIDO_OPT_FALSE) == FIDO_OK); + assert(fido_cred_set_x509(c, x509, sizeof(x509)) == FIDO_OK); + assert(fido_cred_set_sig(c, sig, sizeof(sig)) == FIDO_OK); + assert(fido_cred_set_fmt(c, "packed") == FIDO_OK); + assert(fido_cred_type(c) == COSE_ES256); /* actual algorithm used */ + assert(fido_cred_verify(c) == FIDO_OK); + assert(fido_cred_prot(c) == 0); + assert(fido_cred_pubkey_len(c) == sizeof(pubkey)); + assert(memcmp(fido_cred_pubkey_ptr(c), pubkey, sizeof(pubkey)) == 0); + assert(fido_cred_id_len(c) == sizeof(id)); + assert(memcmp(fido_cred_id_ptr(c), id, sizeof(id)) == 0); + assert(fido_cred_aaguid_len(c) == sizeof(aaguid)); + assert(memcmp(fido_cred_aaguid_ptr(c), aaguid, sizeof(aaguid)) == 0); + free_cred(c); +} int main(void) @@ -3058,6 +3088,7 @@ main(void) attestation_object(); makecred(); entattest(); + multiple_cose(); exit(0); } From ab3245816f0bcea30942f89fb8d48c9fa50c189a Mon Sep 17 00:00:00 2001 From: Ludvig Michaelsson Date: Fri, 15 Nov 2024 11:03:34 +0100 Subject: [PATCH 09/10] man: document fido_cred_append_type --- man/CMakeLists.txt | 3 ++- man/fido_cred_set_authdata.3 | 7 +++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/man/CMakeLists.txt b/man/CMakeLists.txt index f77c3891..a9402a50 100644 --- a/man/CMakeLists.txt +++ b/man/CMakeLists.txt @@ -211,8 +211,9 @@ list(APPEND MAN_ALIAS fido_credman_metadata_new fido_credman_rp_name fido_credman_metadata_new fido_credman_rp_new fido_credman_metadata_new fido_credman_set_dev_rk - fido_cred_set_authdata fido_cred_set_attstmt + fido_cred_set_authdata fido_cred_append_type fido_cred_set_authdata fido_cred_set_attobj + fido_cred_set_authdata fido_cred_set_attstmt fido_cred_set_authdata fido_cred_set_authdata_raw fido_cred_set_authdata fido_cred_set_blob fido_cred_set_authdata fido_cred_set_clientdata diff --git a/man/fido_cred_set_authdata.3 b/man/fido_cred_set_authdata.3 index a5898774..d50ddd7f 100644 --- a/man/fido_cred_set_authdata.3 +++ b/man/fido_cred_set_authdata.3 @@ -99,6 +99,8 @@ typedef enum { .Fn fido_cred_set_fmt "fido_cred_t *cred" "const char *ptr" .Ft int .Fn fido_cred_set_type "fido_cred_t *cred" "int cose_alg" +.Ft int +.Fn fido_cred_append_type "fido_cred_t *cred" "int cose_alg" .Sh DESCRIPTION The .Nm @@ -392,6 +394,11 @@ The type of a credential may only be set once. Note that not all authenticators support COSE_RS256, COSE_ES384, or COSE_EDDSA. .Pp +Unlike +.Fn fido_cred_set_type , +.Fn fido_cred_append_type +may be called repeatedly to request multiple different types. +.Pp Use of the .Nm set of functions may happen in two distinct situations: From a416c5985b329755d905711b1d94ee5f5136ffef Mon Sep 17 00:00:00 2001 From: Ludvig Michaelsson Date: Thu, 5 Dec 2024 07:41:33 +0100 Subject: [PATCH 10/10] man: clarify fido_cred_type() return value --- man/fido_cred_new.3 | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/man/fido_cred_new.3 b/man/fido_cred_new.3 index 79eb06a5..bf63d0e8 100644 --- a/man/fido_cred_new.3 +++ b/man/fido_cred_new.3 @@ -322,6 +322,16 @@ The .Fn fido_cred_type function returns the COSE algorithm of .Fa cred . +If multiple types were requested through +.Fn fido_cred_append_type , +the return value of +.Fn fido_cred_type +may change after a successful call to +.Fn fido_cred_set_attobj , +.Fn fido_cred_set_authdata , +.Fn fido_cred_set_authdata_raw , +or +.Fn fido_dev_make_cred . .Pp The .Fn fido_cred_flags