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

Allow multiple cred pub key types during credential creation #722

Closed
wants to merge 18 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 43 additions & 31 deletions src/cbor.c
Original file line number Diff line number Diff line change
Expand Up @@ -469,44 +469,57 @@ cbor_encode_user_entity(const fido_user_t *user)
}

cbor_item_t *
cbor_encode_pubkey_param(int cose_alg)
cbor_encode_pubkey_param(fido_int_array_t *cose_alg_array)
{
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 ||
cose_alg > -1 || cose_alg < INT16_MIN)
goto fail;
if ((item = cbor_new_definite_array(cose_alg_array->count)) == NULL)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Might be good to check the length of cose_alg_array and error out if it's zero before creating these structs in libcbor

goto fail;

alg.key = cbor_build_string("alg");
for (size_t i = 0; i < fido_int_array_get_count(cose_alg_array); i++) {
int cose_alg = cose_alg_array->ptr[i];

if (-cose_alg - 1 > UINT8_MAX)
alg.value = cbor_build_negint16((uint16_t)(-cose_alg - 1));
else
alg.value = cbor_build_negint8((uint8_t)(-cose_alg - 1));
if ((body = cbor_new_definite_map(2)) == NULL ||
cose_alg > -1 || cose_alg < INT16_MIN)
goto fail;

if (alg.key == NULL || alg.value == NULL) {
fido_log_debug("%s: cbor_build", __func__);
goto fail;
}
alg.key = cbor_build_string("alg");

if (cbor_map_add(body, alg) == false ||
cbor_add_string(body, "type", "public-key") < 0 ||
cbor_array_push(item, body) == false)
goto fail;
if (-cose_alg - 1 > UINT8_MAX)
alg.value = cbor_build_negint16((uint16_t)(-cose_alg - 1));
else
alg.value = cbor_build_negint8((uint8_t)(-cose_alg - 1));

if (alg.key == NULL || alg.value == NULL) {
fido_log_debug("%s: cbor_build", __func__);
goto fail;
}

if (cbor_map_add(body, alg) == false ||
cbor_add_string(body, "type", "public-key") < 0 ||
cbor_array_push(item, body) == false)
goto fail;

ok = 0;
if (body != NULL)
cbor_decref(&body);
if (alg.key != NULL)
cbor_decref(&alg.key);
if (alg.value != NULL)
cbor_decref(&alg.value);

memset(&alg, 0, sizeof(alg));
}

return (item);
fail:
if (ok < 0) {
if (item != NULL) {
cbor_decref(&item);
item = NULL;
}

if (item != NULL) {
cbor_decref(&item);
item = NULL;
}

if (body != NULL)
Expand Down Expand Up @@ -1073,7 +1086,7 @@ 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,
decode_attcred(const unsigned char **buf, size_t *len, fido_int_array_t *cose_alg_array,
fido_attcred_t *attcred)
{
cbor_item_t *item = NULL;
Expand Down Expand Up @@ -1115,12 +1128,11 @@ 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(cose_alg_array, attcred->type)) {
fido_log_debug("%s: attcred->type=%d failed to match any in cose_alg_array", __func__, attcred->type);
goto fail;
}

*buf += cbor.read;
*len -= cbor.read;

Expand Down Expand Up @@ -1288,7 +1300,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, fido_int_array_t *cose_alg,
fido_blob_t *authdata_cbor, fido_authdata_t *authdata,
fido_attcred_t *attcred, fido_cred_ext_t *authdata_ext)
{
Expand Down
55 changes: 44 additions & 11 deletions src/cred.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down Expand Up @@ -62,17 +62,17 @@ fido_dev_make_cred_tx(fido_dev_t *dev, fido_cred_t *cred, const char *pin,
memset(&f, 0, sizeof(f));
memset(argv, 0, sizeof(argv));

if (cred->cdh.ptr == NULL || cred->type == 0) {
if (cred->cdh.ptr == NULL || fido_int_array_is_empty(&cred->type)) {
fido_log_debug("%s: cdh=%p, type=%d", __func__,
(void *)cred->cdh.ptr, cred->type);
(void *)cred->cdh.ptr, fido_cred_type((const fido_cred_t *) & cred));
r = FIDO_ERR_INVALID_ARGUMENT;
goto fail;
}

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(&cred->type)) == NULL) {
fido_log_debug("%s: cbor encode", __func__);
r = FIDO_ERR_INTERNAL;
goto fail;
Expand Down Expand Up @@ -568,7 +568,7 @@ fido_cred_reset_tx(fido_cred_t *cred)
memset(&cred->user, 0, sizeof(cred->user));
memset(&cred->ext, 0, sizeof(cred->ext));

cred->type = 0;
fido_int_array_reset(&cred->type);
cred->rk = FIDO_OPT_OMIT;
cred->uv = FIDO_OPT_OMIT;
}
Expand Down Expand Up @@ -618,7 +618,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;
Expand Down Expand Up @@ -659,7 +659,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;
Expand Down Expand Up @@ -986,21 +986,54 @@ fido_cred_set_fmt(fido_cred_t *cred, const char *fmt)
int
fido_cred_set_type(fido_cred_t *cred, int cose_alg)
bobomb marked this conversation as resolved.
Show resolved Hide resolved
{
if (cred->type != 0)
return fido_cred_set_type_array(cred, &cose_alg, 1);
}

int
fido_cred_set_type_array(fido_cred_t* cred, int *cose_alg_array, size_t count)
bobomb marked this conversation as resolved.
Show resolved Hide resolved
{
if (cose_alg_array == NULL || count == 0)
return (FIDO_ERR_INVALID_ARGUMENT);

for (size_t i = 0; i < count; i++) {
int cose_alg = cose_alg_array[i];

if (cose_alg != COSE_ES256 && cose_alg != COSE_ES384 &&
cose_alg != COSE_RS256 && cose_alg != COSE_EDDSA)
return (FIDO_ERR_INVALID_ARGUMENT);
}

if (fido_int_array_set(&cred->type, cose_alg_array, count) != 0)
return (FIDO_ERR_INTERNAL);

return (FIDO_OK);
}

int fido_cred_add_type(fido_cred_t *cred, int cose_alg)
{
if (cose_alg != COSE_ES256 && cose_alg != COSE_ES384 &&
cose_alg != COSE_RS256 && cose_alg != COSE_EDDSA)
cose_alg != COSE_RS256 && cose_alg != COSE_EDDSA)
return (FIDO_ERR_INVALID_ARGUMENT);

cred->type = cose_alg;
if (cred->type.ptr == NULL || cred->type.count == 0) {
if (fido_int_array_set(&cred->type, &cose_alg, 1) != 0)
return (FIDO_ERR_INTERNAL);
}
else {
if (fido_int_array_append(&cred->type, &cose_alg, 1) != 0)
return (FIDO_ERR_INTERNAL);
}

return (FIDO_OK);
}

int
fido_cred_type(const fido_cred_t *cred)
{
return (cred->type);
if (cred->attcred.type != 0 || cred->type.count == 0)
return (cred->attcred.type);

return (cred->type.ptr[0]); /* compat: return requested type */
}

uint8_t
Expand Down
4 changes: 3 additions & 1 deletion src/credman.c
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,9 @@ credman_parse_rk(const cbor_item_t *key, const cbor_item_t *val, void *arg)
if (cbor_decode_pubkey(val, &cred->attcred.type,
&cred->attcred.pubkey) < 0)
return (-1);
cred->type = cred->attcred.type; /* XXX */
/* set cred type to what is returned by attested cred */
int cred_type[1] = { cred->attcred.type };
fido_int_array_set(&cred->type, cred_type, 1);
return (0);
case 10:
if (cbor_decode_uint64(val, &prot) < 0 || prot > INT_MAX ||
Expand Down
12 changes: 10 additions & 2 deletions src/extern.h
Original file line number Diff line number Diff line change
Expand Up @@ -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(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 *);
Expand All @@ -59,7 +59,7 @@ cbor_item_t *es256_pk_encode(const es256_pk_t *, int);
/* cbor decoding functions */
int cbor_decode_attstmt(const cbor_item_t *, fido_attstmt_t *);
int cbor_decode_bool(const cbor_item_t *, bool *);
int cbor_decode_cred_authdata(const cbor_item_t *, int, fido_blob_t *,
int cbor_decode_cred_authdata(const cbor_item_t *, 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 *);
Expand Down Expand Up @@ -203,6 +203,14 @@ 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 *);
int fido_str_array_pack(fido_str_array_t *, const char * const *, size_t);
int fido_int_array_is_empty(const fido_int_array_t *);
int fido_int_array_set(fido_int_array_t *, const int *, size_t);
int fido_int_array_append(fido_int_array_t *, const int *, size_t);
void fido_int_array_free(fido_int_array_t *);
void fido_int_array_reset(fido_int_array_t *);
void fido_free_int_array(fido_int_array_t *);
size_t fido_int_array_get_count(const fido_int_array_t *);
bool fido_int_array_contains(const fido_int_array_t* array, int element);

/* misc */
void fido_assert_reset_rx(fido_assert_t *);
Expand Down
5 changes: 4 additions & 1 deletion src/fido.h
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,8 @@ int fido_cred_set_rk(fido_cred_t *, fido_opt_t);
int fido_cred_set_rp(fido_cred_t *, const char *, const char *);
int fido_cred_set_sig(fido_cred_t *, const unsigned char *, size_t);
int fido_cred_set_type(fido_cred_t *, int);
int fido_cred_set_type_array(fido_cred_t* cred, int* cose_alg_array, size_t count);
int fido_cred_add_type(fido_cred_t*, int);
int fido_cred_set_uv(fido_cred_t *, fido_opt_t);
int fido_cred_type(const fido_cred_t *);
int fido_cred_set_user(fido_cred_t *, const unsigned char *, size_t,
Expand Down Expand Up @@ -223,7 +225,8 @@ size_t fido_cred_id_len(const fido_cred_t *);
size_t fido_cred_largeblob_key_len(const fido_cred_t *);
size_t fido_cred_pin_minlen(const fido_cred_t *);
size_t fido_cred_pubkey_len(const fido_cred_t *);
size_t fido_cred_sig_len(const fido_cred_t *);
size_t fido_cred_sig_len(const fido_cred_t*);
size_t fido_cred_type_array_len(const fido_cred_t* cred);
size_t fido_cred_user_id_len(const fido_cred_t *);
size_t fido_cred_x5c_len(const fido_cred_t *);

Expand Down
41 changes: 23 additions & 18 deletions src/fido/types.h
Original file line number Diff line number Diff line change
Expand Up @@ -166,25 +166,30 @@ typedef struct fido_cred_ext {
size_t minpinlen; /* minimum pin length */
} fido_cred_ext_t;

typedef struct fido_int_array {
int* ptr;
size_t count;
} fido_int_array_t;

typedef struct fido_cred {
fido_blob_t cd; /* client data */
fido_blob_t cdh; /* client data hash */
fido_rp_t rp; /* relying party */
fido_user_t user; /* user entity */
fido_blob_array_t excl; /* list of credential ids to exclude */
fido_opt_t rk; /* resident key */
fido_opt_t uv; /* user verification */
fido_cred_ext_t ext; /* extensions */
int type; /* cose algorithm */
char *fmt; /* credential format */
fido_cred_ext_t authdata_ext; /* decoded extensions */
fido_blob_t authdata_cbor; /* cbor-encoded payload */
fido_blob_t authdata_raw; /* cbor-decoded payload */
fido_authdata_t authdata; /* decoded authdata payload */
fido_attcred_t attcred; /* returned credential (key + id) */
fido_attstmt_t attstmt; /* attestation statement (x509 + sig) */
fido_blob_t largeblob_key; /* decoded large blob key */
fido_blob_t blob; /* CTAP 2.1 credBlob */
fido_blob_t cd; /* client data */
fido_blob_t cdh; /* client data hash */
fido_rp_t rp; /* relying party */
fido_user_t user; /* user entity */
fido_blob_array_t excl; /* list of credential ids to exclude */
fido_opt_t rk; /* resident key */
fido_opt_t uv; /* user verification */
fido_cred_ext_t ext; /* extensions */
fido_int_array_t type; /* cose algorithm array */
char *fmt; /* credential format */
fido_cred_ext_t authdata_ext; /* decoded extensions */
fido_blob_t authdata_cbor; /* cbor-encoded payload */
fido_blob_t authdata_raw; /* cbor-decoded payload */
fido_authdata_t authdata; /* decoded authdata payload */
fido_attcred_t attcred; /* returned credential (key + id) */
fido_attstmt_t attstmt; /* attestation statement (x509 + sig) */
fido_blob_t largeblob_key; /* decoded large blob key */
fido_blob_t blob; /* CTAP 2.1 credBlob */
} fido_cred_t;

typedef struct fido_assert_extattr {
Expand Down
6 changes: 5 additions & 1 deletion src/touch.c
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,15 @@ fido_dev_get_touch_begin(fido_dev_t *dev)
fido_user_t user;
int ms = dev->timeout_ms;
int r = FIDO_ERR_INTERNAL;
int cose[1] = { COSE_ES256 };
fido_int_array_t cose_array = { cose, 1 };

memset(&f, 0, sizeof(f));
memset(argv, 0, sizeof(argv));
memset(cdh, 0, sizeof(cdh));
memset(&rp, 0, sizeof(rp));
memset(&user, 0, sizeof(user));
memset(&cose_array, 0, sizeof(cose_array));

if (fido_dev_is_fido2(dev) == false)
return (u2f_get_touch_begin(dev, &ms));
Expand All @@ -46,10 +49,11 @@ fido_dev_get_touch_begin(fido_dev_t *dev)
goto fail;
}


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(&cose_array)) == NULL) {
fido_log_debug("%s: cbor encode", __func__);
goto fail;
}
Expand Down
Loading
Loading