Skip to content

Commit

Permalink
address review comments
Browse files Browse the repository at this point in the history
add fido_int_array_contains()
fix check in u2f.c
initialize fido_int_array_t in touch.c
remove len field, only use count and compute len as needed.
return attested credential type in fido_cred_get_type()
fix an issue with creating only an array of 1 elements in cbor.c
  • Loading branch information
bobomb committed Jan 8, 2024
1 parent 71e4714 commit 9ad9918
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 67 deletions.
28 changes: 5 additions & 23 deletions src/cbor.c
Original file line number Diff line number Diff line change
Expand Up @@ -478,7 +478,7 @@ cbor_encode_pubkey_param(fido_int_array_t *cose_alg_array)

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

if ((item = cbor_new_definite_array(1)) == NULL)
if ((item = cbor_new_definite_array(cose_alg_array->count)) == NULL)
goto fail;

for (size_t i = 0; i < fido_int_array_get_count(cose_alg_array); i++) {
Expand Down Expand Up @@ -1097,7 +1097,6 @@ decode_attcred(const unsigned char **buf, size_t *len, fido_int_array_t *cose_al
struct cbor_load_result cbor;
uint16_t id_len;
int ok = -1;
bool cose_match = false;

fido_log_xxd(*buf, *len, "%s", __func__);

Expand Down Expand Up @@ -1133,28 +1132,11 @@ decode_attcred(const unsigned char **buf, size_t *len, fido_int_array_t *cose_al
goto fail;
}

for (size_t i = 0; i < fido_int_array_get_count(cose_alg_array); i++) {
int cose_alg = cose_alg_array->ptr[i];
if (attcred->type != cose_alg) {
fido_log_debug("%s: cose_alg mismatch (%d != %d)", __func__,
attcred->type, cose_alg);
}
else {
cose_match = true;
}
}

if (!cose_match) {
fido_log_debug("%s: cose_alg failed to match any", __func__);
goto fail;
}
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;
}

/* set the credential type to the attested credential type */
int cose[1] = { attcred->type };
fido_int_array_set(cose_alg_array, cose, 1);



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

Expand Down
34 changes: 4 additions & 30 deletions src/cred.c
Original file line number Diff line number Diff line change
Expand Up @@ -986,15 +986,7 @@ fido_cred_set_fmt(fido_cred_t *cred, const char *fmt)
int
fido_cred_set_type(fido_cred_t *cred, int cose_alg)
{
int cose_array[1] = { cose_alg };
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_array, 1) != 0)
return (FIDO_ERR_INTERNAL);

return (FIDO_OK);
return fido_cred_set_type_array(cred, &cose_alg, 1);
}

int
Expand All @@ -1020,28 +1012,10 @@ fido_cred_set_type_array(fido_cred_t* cred, int *cose_alg_array, size_t count)
int
fido_cred_type(const fido_cred_t *cred)
{
if (fido_int_array_is_empty(&cred->type))
return 0;
/* return only the first, to ensure backwards compatibility */
return cred->type.ptr[0];
}

const int *
fido_cred_type_array_ptr(const fido_cred_t* cred)
{
if (fido_int_array_is_empty(&cred->type))
return 0;

return (cred->type.ptr);
}

size_t
fido_cred_type_array_len(const fido_cred_t* cred)
{
if (fido_int_array_is_empty(&cred->type))
return 0;
if (cred->attcred.type != 0 || cred->type.count == 0)
return (cred->attcred.type);

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

uint8_t
Expand Down
27 changes: 19 additions & 8 deletions src/int_array.c
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ fido_int_array_new(void)
void
fido_int_array_reset(fido_int_array_t *array)
{
freezero(array->ptr, array->len);
freezero(array->ptr, array->count);
explicit_bzero(array, sizeof(*array));
}

Expand All @@ -38,7 +38,6 @@ fido_int_array_set(fido_int_array_t *array, const int *ptr, size_t count)
}

memcpy(array->ptr, ptr, len);
array->len = len;
array->count = count;

return 0;
Expand All @@ -56,17 +55,16 @@ fido_int_array_append(fido_int_array_t *array, const int *ptr, size_t count)
return -1;
}

if (SIZE_MAX - array->len < len) {
if (SIZE_MAX - (array->count * sizeof(int)) < len) {
fido_log_debug("%s: overflow", __func__);
return -1;
}
if ((tmp = realloc(array->ptr, array->len + len)) == NULL) {
if ((tmp = realloc(array->ptr, (array->count * sizeof(int)) + len)) == NULL) {
fido_log_debug("%s: realloc", __func__);
return -1;
}
array->ptr = tmp;
memcpy(&array->ptr[array->len], ptr, len);
array->len += len;
memcpy(&array->ptr[(array->count * sizeof(int))], ptr, len);
array->count += count;

return 0;
Expand All @@ -80,14 +78,13 @@ fido_int_array_free(fido_int_array_t *array)

free(array->ptr);
array->ptr = NULL;
array->len = 0;
array->count = 0;
}

int
fido_int_array_is_empty(const fido_int_array_t *array)
{
return array == NULL || array->ptr == NULL || array->len == 0;
return array == NULL || array->ptr == NULL || array->count == 0;
}

size_t
Expand All @@ -97,4 +94,18 @@ fido_int_array_get_count(const fido_int_array_t *array)
return 0;
else
return array->count;
}

bool
fido_int_array_contains(const fido_int_array_t* array, int element)
{
if (array == NULL || array->ptr == NULL || array->count == 0)
return false;

for (size_t i = 0; i < array->count; i++) {
if (element == array->ptr[i])
return true;
}

return false;
}
2 changes: 1 addition & 1 deletion src/int_array.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ extern "C" {

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

Expand All @@ -29,6 +28,7 @@ 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);

#ifdef __cplusplus
} /* extern "C" */
Expand Down
3 changes: 1 addition & 2 deletions src/touch.c
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ fido_dev_get_touch_begin(fido_dev_t *dev)
int ms = dev->timeout_ms;
int r = FIDO_ERR_INTERNAL;
int cose[1] = { COSE_ES256 };
fido_int_array_t cose_array;
fido_int_array_t cose_array = { cose, 1 };

memset(&f, 0, sizeof(f));
memset(argv, 0, sizeof(argv));
Expand Down Expand Up @@ -49,7 +49,6 @@ fido_dev_get_touch_begin(fido_dev_t *dev)
goto fail;
}

fido_int_array_set(&cose_array, cose, 1);

if ((argv[0] = cbor_build_bytestring(cdh, sizeof(cdh))) == NULL ||
(argv[1] = cbor_encode_rp_entity(&rp)) == NULL ||
Expand Down
9 changes: 6 additions & 3 deletions src/u2f.c
Original file line number Diff line number Diff line change
Expand Up @@ -677,10 +677,13 @@ u2f_register(fido_dev_t *dev, fido_cred_t *cred, int *ms)
return (FIDO_ERR_UNSUPPORTED_OPTION);
}

if (fido_cred_type(cred) != COSE_ES256 || cred->cdh.ptr == NULL ||
if (!fido_int_array_contains((const fido_int_array_t *)&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);
for (size_t i = 0; i < cred->type.count; i++) {
fido_log_debug("%s: types=%d", __func__, cred->type.ptr[i]);
}
fido_log_debug("%s: cdh=(%p,%zu)" , __func__,
(void *)cred->cdh.ptr, cred->cdh.len);
return (FIDO_ERR_INVALID_ARGUMENT);
}

Expand Down

0 comments on commit 9ad9918

Please sign in to comment.