Skip to content

Commit

Permalink
More tests and error handling fixes for COSE_Encrypt (laurencelundbla…
Browse files Browse the repository at this point in the history 
…de#275)

This adds test coverage for most things that can go wrong with a COSE_Encrypt and fixes the error handling for these things.

The shell script that processes diag into test input is improved.


* More tests and error handling fixes for COSE_Encrypt

* Fill out error handling for COSE_Encrypt decoding

* Fix left over merge issue

* Add .diag files to Xcode project

* error checking in script for making test messages

* Describe test cases; fix rcpt test case; rename some

* straggler files

---------

Co-authored-by: Laurence Lundblade <[email protected]>
  • Loading branch information
@laurencelundblade
laurencelundblade and Laurence Lundblade authored Nov 15, 2023
1 parent ade036c commit 00992ed
Show file tree
Hide file tree
Showing 35 changed files with 910 additions and 120 deletions.
7 changes: 5 additions & 2 deletions inc/t_cose/t_cose_common.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -328,8 +328,7 @@ enum t_cose_err_t {
* when a byte string is expected. */
T_COSE_ERR_PARAMETER_CBOR = 10,

/** No algorithm ID was found when one is needed. For example,
* when verifying a \c COSE_Sign1. */
/** No algorithm ID found, algorithm ID encoded wrong, not in protected header or such. */
T_COSE_ERR_NO_ALG_ID = 11,

/** No kid (key ID) was found when one is needed. For example,
Expand Down Expand Up @@ -651,6 +650,10 @@ enum t_cose_err_t {

/** External AAD is passed as an argument for non AEAD cipher. */
T_COSE_ERR_AAD_WITH_NON_AEAD = 91,

/** An initialization vector (IV) is empty, wrong type or such. */
T_COSE_ERR_BAD_IV = 92,

};


Expand Down
3 changes: 1 addition & 2 deletions inc/t_cose/t_cose_encrypt_enc.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,6 @@ struct t_cose_encrypt_enc {
* t_cose_recipient_enc being used. You can even have serveral with
* different algorithms (but there can only be one payload encryption
* algorithm).
* TODO: decode-only mode to get parameters to look up keys
*/
void
t_cose_encypt_enc_init(struct t_cose_encrypt_enc *context,
Expand Down Expand Up @@ -302,7 +301,7 @@ t_cose_encrypt_set_cek(struct t_cose_encrypt_enc *context,
* described in RFC 9052 section 5.2. It needs to be the size of the
* CBOR-encoded protected headers, the externally supplied data and some overhead.
*
* TODO: size calculation mode that will tell the caller how bit it should be
* TODO: size calculation mode that will tell the caller how big it should be
*/
static void
t_cose_encrypt_set_enc_struct_buffer(struct t_cose_encrypt_enc *context,
Expand Down
2 changes: 1 addition & 1 deletion inc/t_cose/t_cose_recipient_dec.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ struct t_cose_recipient_dec;
* \retval T_COSE_ERR_DECLINE
* \retval T_COSE_ERR_KID_UNMATCHED
* \retval T_COSE_ERR_UNSUPPORTED_KEY_EXCHANGE_ALG
* \retval ....
* \retval T_COSE_ERR_NO_MORE No more recipients to decode. End of recipients array.
*
*
* The error returned is important as it determines whether
Expand Down
90 changes: 57 additions & 33 deletions src/t_cose_encrypt_dec.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ is_soft_verify_error(enum t_cose_err_t error)
* While this is called only once, it is split out for code readability.
*
* This loops over all the configured recipient decoders calling them
* until one succeeds or has a hard failure. This involve multiple
* until one succeeds or has a hard failure. This performs multiple
* attempts at the CBOR decode of the COSE_Recipient.
*/
static enum t_cose_err_t
Expand All @@ -76,10 +76,11 @@ decrypt_one_recipient(struct t_cose_encrypt_dec_ctx *me,
QCBORDecode_SaveCursor(cbor_decoder, &saved_cursor);

/* Loop over the configured recipients */
for(rcpnt_decoder = me->recipient_list;
rcpnt_decoder != NULL;
rcpnt_decoder = (struct t_cose_recipient_dec *)rcpnt_decoder->base_obj.next) {
rcpnt_decoder = me->recipient_list;
while(1) {

// TODO: decode-only mode for recipients

return_value =
rcpnt_decoder->decode_cb(
rcpnt_decoder, /* in: me ptr of the recipient decoder */
Expand All @@ -92,15 +93,14 @@ decrypt_one_recipient(struct t_cose_encrypt_dec_ctx *me,
cek /* out: the returned CEK */
);

/* This is pretty much the same as for decrypting recipients */
if(return_value == T_COSE_SUCCESS) {
/* Only need to find one success and this is it so done.*/
return T_COSE_SUCCESS;
/* Only need to find one success. This is it, so we are done.
* We have the CEK. */
break;
}

if(return_value == T_COSE_ERR_NO_MORE) {
/* Tried all the recipient decoders. None succeeded and
* none gave a hard failure. */
/* The end of the recipients array. No more COSE_Recipients. */
return T_COSE_ERR_NO_MORE;
}

Expand All @@ -109,13 +109,19 @@ decrypt_one_recipient(struct t_cose_encrypt_dec_ctx *me,
/* Something very wrong. */
}

/* Loop continues on for the next recipient */
QCBORDecode_RestoreCursor(cbor_decoder, &saved_cursor);
/* Try going on to next configured recipient decoder if there is one */
rcpnt_decoder = (struct t_cose_recipient_dec *)rcpnt_decoder->base_obj.next;
if(rcpnt_decoder == NULL) {
/* Got to end of list and no recipient decoder succeeded. */
return T_COSE_ERR_DECLINE;
}

/* Loop continues on for the next recipient decoder */
QCBORDecode_RestoreCursor(cbor_decoder, &saved_cursor);
}

/* Got to end of list and no recipient attempted to verify */
return T_COSE_ERR_DECLINE;
return T_COSE_SUCCESS;
}


Expand Down Expand Up @@ -151,14 +157,21 @@ t_cose_encrypt_dec_detached(struct t_cose_encrypt_dec_ctx* me,
Q_USEFUL_BUF_MAKE_STACK_UB( enc_struct_buffer, T_COSE_ENCRYPT_STRUCT_DEFAULT_SIZE);
struct q_useful_buf_c enc_structure;
bool alg_id_prot;
enum t_cose_err_t previous_return_value;


/* --- Get started decoding array of four and tags --- */
/* --- Get started decoding array of 4 and tags --- */
QCBORDecode_Init(&cbor_decoder, message, QCBOR_DECODE_MODE_NORMAL);

QCBORDecode_EnterArray(&cbor_decoder, &array_item);
cbor_error = QCBORDecode_GetError(&cbor_decoder);
if(cbor_error != QCBOR_SUCCESS) {
goto Done;
}

const uint64_t signing_tag_nums[] = {CBOR_TAG_COSE_ENCRYPT, CBOR_TAG_COSE_ENCRYPT0, CBOR_TAG_INVALID64};
const uint64_t signing_tag_nums[] = {CBOR_TAG_COSE_ENCRYPT,
CBOR_TAG_COSE_ENCRYPT0,
CBOR_TAG_INVALID64};
return_value = t_cose_tags_and_type(signing_tag_nums,
me->option_flags,
&array_item,
Expand All @@ -174,7 +187,7 @@ t_cose_encrypt_dec_detached(struct t_cose_encrypt_dec_ctx* me,
/* The location of body header parameters is 0, 0 */
header_location.nesting = 0;
header_location.index = 0;
body_params_list = NULL;
body_params_list = NULL;
rcpnt_params_list = NULL;

return_value =
Expand All @@ -192,6 +205,10 @@ t_cose_encrypt_dec_detached(struct t_cose_encrypt_dec_ctx* me,
}

nonce_cbor = t_cose_param_find_iv(body_params_list);
if(q_useful_buf_c_is_empty(nonce_cbor)) {
return T_COSE_ERR_BAD_IV;
}

ce_alg.cose_alg_id = t_cose_param_find_alg_id(body_params_list, &alg_id_prot);
if(ce_alg.cose_alg_id == T_COSE_ALGORITHM_NONE) {
return T_COSE_ERR_NO_ALG_ID;
Expand All @@ -201,21 +218,20 @@ t_cose_encrypt_dec_detached(struct t_cose_encrypt_dec_ctx* me,
if(!t_cose_params_empty(protected_params)) {
return T_COSE_ERR_PROTECTED_NOT_ALLOWED;
}

} else {
/* Make sure alg id is protected for aead algorithms */
if(alg_id_prot != true) {
return T_COSE_ERR_NO_ALG_ID;
}
}

all_params_list = body_params_list;

ce_alg.bits_in_key = bits_in_crypto_alg(ce_alg.cose_alg_id);
if(ce_alg.bits_in_key == UINT32_MAX) {
return T_COSE_ERR_UNSUPPORTED_ENCRYPTION_ALG;
}

all_params_list = body_params_list;


/* --- The Ciphertext --- */
if(!q_useful_buf_c_is_null(detached_ciphertext)) {
QCBORDecode_GetNull(&cbor_decoder);
Expand All @@ -233,19 +249,24 @@ t_cose_encrypt_dec_detached(struct t_cose_encrypt_dec_ctx* me,
cek_key = me->cek;

} else if (message_type == T_COSE_OPT_MESSAGE_TYPE_ENCRYPT) {
if(me->recipient_list == NULL) {
return T_COSE_ERR_FAIL; // TODO: need better error here
}

header_location.nesting = 1;
header_location.index = 0;

/* --- Enter array of recipients --- */
QCBORDecode_EnterArray(&cbor_decoder, NULL);
cbor_error = QCBORDecode_GetError(&cbor_decoder);
if(cbor_error != QCBOR_SUCCESS) {
return_value = qcbor_decode_error_to_t_cose_error(cbor_error, T_COSE_ERR_ENCRYPT_FORMAT);
goto Done;
}

/* Loop over the array of COSE_Recipients */
while(1) {
previous_return_value = return_value;

return_value = decrypt_one_recipient(me,
header_location,
ce_alg,
Expand All @@ -270,6 +291,14 @@ t_cose_encrypt_dec_detached(struct t_cose_encrypt_dec_ctx* me,
break;
}

if(return_value == T_COSE_ERR_NO_MORE) {
/* Got to the end of the COSE_Recipients array without
* success, so return the error the previous recipient decoder
* returned. */
return_value = previous_return_value;
goto Done;
}

if(return_value != T_COSE_ERR_DECLINE) {
/* Either we got to the end of the list and on
* recipient decoder attempted, or some decoder
Expand Down Expand Up @@ -304,18 +333,15 @@ t_cose_encrypt_dec_detached(struct t_cose_encrypt_dec_ctx* me,
}

} else {
/* Message type is not right. */
// TODO: better error here.
return T_COSE_ERR_FAIL;
/* This never happens because of checks in t_cose_tags_and_type() */
}

/* --- Close of CBOR decode --- */
/* --- Close of CBOR decode of the array of 4 --- */
/* This tolerates extra items. Someday we'll have a better ExitArray()
* and efficiently catch this (mostly harmless) error. */
QCBORDecode_ExitArray(&cbor_decoder);

cbor_error = QCBORDecode_Finish(&cbor_decoder);
if(cbor_error != QCBOR_SUCCESS) {
// TODO: there is probably more to be done here...
return_value = T_COSE_ERR_CBOR_DECODE;
goto Done;
}
if(returned_parameters != NULL) {
Expand Down Expand Up @@ -351,7 +377,7 @@ t_cose_encrypt_dec_detached(struct t_cose_encrypt_dec_ctx* me,
}
else {
/* --- Make the Enc_structure ---- */
/* The Enc_structure from RFC 9052 section 5.3 that is AAD input
/* The Enc_structure from RFC 9052 section 5.3 that is input as AAD
* to the AEAD to integrity-protect COSE headers and
* parameters. */
if(!q_useful_buf_is_null(me->extern_enc_struct_buffer)) {
Expand All @@ -373,7 +399,6 @@ t_cose_encrypt_dec_detached(struct t_cose_encrypt_dec_ctx* me,
goto Done;
}

// TODO: handle AE algorithms
return_value =
t_cose_crypto_aead_decrypt(
ce_alg.cose_alg_id, /* in: cose alg id to decrypt payload */
Expand All @@ -389,10 +414,9 @@ t_cose_encrypt_dec_detached(struct t_cose_encrypt_dec_ctx* me,
t_cose_crypto_free_symmetric_key(cek_key);
}

if (message_type != T_COSE_OPT_MESSAGE_TYPE_ENCRYPT0) {
t_cose_crypto_free_symmetric_key(cek_key);
}

Done:
if(cbor_error != QCBOR_SUCCESS) {
return_value = qcbor_decode_error_to_t_cose_error(cbor_error, T_COSE_ERR_ENCRYPT_FORMAT);
}
return return_value;
}
20 changes: 10 additions & 10 deletions src/t_cose_encrypt_enc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ t_cose_encrypt_enc_detached(struct t_cose_encrypt_enc *me,
cek_bytes, /* in: key bytes */
&cek_handle); /* out: key handle */
}
if (return_value != T_COSE_SUCCESS) {
if(return_value != T_COSE_SUCCESS) {
goto Done;
}
/* At this point cek_handle has the encryption key for the AEAD */
Expand All @@ -166,15 +166,15 @@ t_cose_encrypt_enc_detached(struct t_cose_encrypt_enc *me,
&encrypt_output /* out: ciphertext */);
} else {
/* ---- Make the Enc_structure ---- */
/* Per RFC 9052 section 5.3 the structure that is authenticated
* along with the payload by the AEAD.
*
* Enc_structure = [
* context : "Encrypt",
* protected : empty_or_serialized_map,
* external_aad : bstr
* ]
*/
/* Per RFC 9052 section 5.3 this is the structure that is authenticated
* along with the payload by the AEAD.
*
* Enc_structure = [
* context : "Encrypt",
* protected : empty_or_serialized_map,
* external_aad : bstr
* ]
*/
if(!q_useful_buf_is_null(me->extern_enc_struct_buffer)) {
/* Caller gave us a (bigger) buffer for Enc_structure */
enc_struct_buffer = me->extern_enc_struct_buffer;
Expand Down
4 changes: 4 additions & 0 deletions src/t_cose_recipient_dec_esdh.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,10 @@ t_cose_recipient_dec_esdh_cb_private(struct t_cose_recipient_dec *me_x,
/* One recipient */
QCBORDecode_EnterArray(cbor_decoder, NULL);
cbor_error = QCBORDecode_GetError(cbor_decoder);
if(cbor_error == QCBOR_ERR_NO_MORE_ITEMS) {
cose_result = T_COSE_ERR_NO_MORE;
goto done;
}
if(cbor_error != QCBOR_SUCCESS) {
cose_result = qcbor_decode_error_to_t_cose_error(cbor_error, T_COSE_ERR_RECIPIENT_FORMAT);
goto done;
Expand Down
2 changes: 1 addition & 1 deletion src/t_cose_util.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ struct q_useful_buf_c get_short_circuit_kid(void);


/**
* \brief Map QCBOR decode error to COSE errors.
* \brief Map QCBOR decode error to t_cose errors.
*
* \param[in] qcbor_error The QCBOR error to map.
*
Expand Down
Loading

0 comments on commit 00992ed

Please sign in to comment.