Skip to content

Commit

Permalink
More rework for empty protected headers (#272)
Browse files Browse the repository at this point in the history 
* More rework for empty protected headers

* Test and key wrap fixes

* Fix check for alt empty parameters form

---------

Co-authored-by: Laurence Lundblade <[email protected]>
  • Loading branch information
@laurencelundblade
laurencelundblade and Laurence Lundblade authored Nov 6, 2023
1 parent 227eb48 commit 3076010
Show file tree
Hide file tree
Showing 9 changed files with 106 additions and 76 deletions.
43 changes: 41 additions & 2 deletions inc/t_cose/t_cose_parameters.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,8 @@ struct t_cose_parameter_storage {
* A pointer and length of the protected header byte string is
* returned so that it can be covered by what ever protection
* mechanism is in used (e.g., hashing or AEAD encryption).
* If there are no protected header parameters an empty string
* will always be returned in \c protected_parameters.
*/
enum t_cose_err_t
t_cose_headers_encode(QCBOREncodeContext *cbor_encoder,
Expand All @@ -431,7 +433,6 @@ t_cose_headers_encode(QCBOREncodeContext *cbor_encoder,
*
* \param[in] cbor_decoder QCBOR decoder to decode from.
* \param[in] location Location in message of the parameters.
* \param[in] no_protected Protected headers must be empty.
* \param[in] special_decode_cb Callback for non-integer and
* non-string parameters.
* \param[in] special_decode_ctx Context for the above callback
Expand Down Expand Up @@ -486,14 +487,28 @@ t_cose_headers_encode(QCBOREncodeContext *cbor_encoder,
enum t_cose_err_t
t_cose_headers_decode(QCBORDecodeContext *cbor_decoder,
const struct t_cose_header_location location,
bool no_protected,
t_cose_param_special_decode_cb *special_decode_cb,
void *special_decode_ctx,
struct t_cose_parameter_storage *parameter_storage,
struct t_cose_parameter **decoded_params,
struct q_useful_buf_c *protected_parameters);



/* Returns true if the CBOR encoded parameters are empty either because
* they are an empty string or a wrapped empty map. This is primarily
* used on protected headers. */
static bool
t_cose_params_empty(struct q_useful_buf_c encoded_params);


/* Returns true of the CBOR-encoded parameters are an empty bstr. This is
* primarily used on protected headers in a few context where they
* must be empty only by being an empty bstr. */
static bool
t_cose_params_empty_bstr(struct q_useful_buf_c encoded_params);


/**
* \brief Check parameter list, particularly for unknown critical parameters
*
Expand Down Expand Up @@ -827,6 +842,30 @@ t_cose_params_common(const struct t_cose_parameter *decoded_params,
*/


static inline bool
t_cose_params_empty_bstr(struct q_useful_buf_c encoded_params)
{
return encoded_params.len == 0;
}

static inline bool
t_cose_params_empty(struct q_useful_buf_c encoded_params)
{
int compare_result;
const uint8_t empty_map[] = {0xa0};

if(t_cose_params_empty_bstr(encoded_params)) {
return true;
}

compare_result = q_useful_buf_compare(encoded_params,
Q_USEFUL_BUF_FROM_BYTE_ARRAY_LITERAL(empty_map));

/* Check for a CBOR-encoded empty array */
return compare_result == 0 ? true : false;
}


static inline struct t_cose_parameter
t_cose_param_make_alg_id(int32_t alg_id)
{
Expand Down
9 changes: 3 additions & 6 deletions src/t_cose_encrypt_dec.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,6 @@ 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;
struct t_cose_parameter *p_param;


/* --- Get started decoding array of four and tags --- */
Expand Down Expand Up @@ -182,7 +181,6 @@ t_cose_encrypt_dec_detached(struct t_cose_encrypt_dec_ctx* me,
t_cose_headers_decode(
&cbor_decoder, /* in: cbor decoder context */
header_location, /* in: location of headers in message */
false, /* in: no_protected headers */
NULL, /* TODO: in: header decode callback function */
NULL, /* TODO: in: header decode callback context */
me->p_storage, /* in: pool of nodes for linked list */
Expand All @@ -200,11 +198,10 @@ t_cose_encrypt_dec_detached(struct t_cose_encrypt_dec_ctx* me,
}
if(t_cose_alg_is_non_aead(ce_alg.cose_alg_id)) {
/* Make sure there are no protected headers for non-aead algorithms */
for(p_param = body_params_list; p_param->next != NULL; p_param = p_param->next) {
if(p_param->in_protected) {
return T_COSE_ERR_PROTECTED_NOT_ALLOWED;
}
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) {
Expand Down
1 change: 0 additions & 1 deletion src/t_cose_mac_validate.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ t_cose_mac_validate_private(struct t_cose_mac_validate_ctx *me,
decoded_params = NULL;
t_cose_headers_decode(&decode_context,
l,
false,
NULL,
NULL,
&me->parameter_storage,
Expand Down
43 changes: 17 additions & 26 deletions src/t_cose_parameters.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,6 @@ param_dup_detect(const struct t_cose_parameter *params_list)
enum t_cose_err_t
t_cose_headers_decode(QCBORDecodeContext *cbor_decoder,
const struct t_cose_header_location location,
const bool no_protected,
t_cose_param_special_decode_cb *special_decode_cb,
void *special_decode_ctx,
struct t_cose_parameter_storage *param_storage,
Expand All @@ -504,36 +503,27 @@ t_cose_headers_decode(QCBORDecodeContext *cbor_decoder,
QCBORError cbor_error;
enum t_cose_err_t return_value;
struct t_cose_parameter *newly_decode_params;
struct q_useful_buf_c empty_protected;

newly_decode_params = NULL;

/* --- The protected parameters --- */
if(no_protected) {
QCBORDecode_GetByteString(cbor_decoder, &empty_protected);
if(empty_protected.len != 0) {
return_value = T_COSE_ERR_PROTECTED_NOT_ALLOWED;
QCBORDecode_EnterBstrWrapped(cbor_decoder,
QCBOR_TAG_REQUIREMENT_NOT_A_TAG,
protected_parameters);

if(protected_parameters->len) {
return_value = t_cose_params_decode(cbor_decoder,
location,
true,
special_decode_cb,
special_decode_ctx,
param_storage,
&newly_decode_params);
if(return_value != T_COSE_SUCCESS) {
goto Done;
}
} else {
QCBORDecode_EnterBstrWrapped(cbor_decoder,
QCBOR_TAG_REQUIREMENT_NOT_A_TAG,
protected_parameters);

if(protected_parameters->len) {
return_value = t_cose_params_decode(cbor_decoder,
location,
true,
special_decode_cb,
special_decode_ctx,
param_storage,
&newly_decode_params);
if(return_value != T_COSE_SUCCESS) {
goto Done;
}
}
QCBORDecode_ExitBstrWrapped(cbor_decoder);
}
QCBORDecode_ExitBstrWrapped(cbor_decoder);

/* --- The unprotected parameters --- */
return_value = t_cose_params_decode(cbor_decoder,
Expand Down Expand Up @@ -705,11 +695,12 @@ t_cose_headers_encode(QCBOREncodeContext *cbor_encoder,
}
if(!protected_present) {
QCBOREncode_AddBytes(cbor_encoder, NULL_Q_USEFUL_BUF_C);
*protected_parameters = NULL_Q_USEFUL_BUF_C;
} else {
QCBOREncode_BstrWrap(cbor_encoder);
return_value = t_cose_params_encode(cbor_encoder,
parameters,
true);
parameters,
true);
if(return_value != T_COSE_SUCCESS) {
goto Done;
}
Expand Down
1 change: 0 additions & 1 deletion src/t_cose_recipient_dec_esdh.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,6 @@ t_cose_recipient_dec_esdh_cb_private(struct t_cose_recipient_dec *me_x,
cose_result = t_cose_headers_decode(
cbor_decoder, /* in: decoder to read from */
loc, /* in: location in COSE message */
false, /* in: no_protected headers */
decode_ephemeral_key, /* in: callback for specials */
NULL, /* in: context for specials callback */
p_storage, /* in: parameter storage */
Expand Down
14 changes: 3 additions & 11 deletions src/t_cose_recipient_dec_keywrap.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ t_cose_recipient_dec_keywrap_cb_private(struct t_cose_recipient_dec *me_x,
struct q_useful_buf_c protected_params;
int32_t cose_algorithm_id;
QCBORError cbor_error;
struct q_useful_buf_c encoded_empty_map;

/* Morph to the object we actually are */
me = (struct t_cose_recipient_dec_keywrap *)me_x;
Expand All @@ -50,7 +49,6 @@ t_cose_recipient_dec_keywrap_cb_private(struct t_cose_recipient_dec *me_x,
/* ---- First and second items -- protected & unprotected headers ---- */
err = t_cose_headers_decode(cbor_decoder, /* in: decoder to read from */
loc, /* in: location in COSE message */
false, /* in: no_protected headers */
NULL, /* in: callback for specials */
NULL, /* in: context for callback */
p_storage, /* in: parameter storage */
Expand All @@ -60,16 +58,10 @@ t_cose_recipient_dec_keywrap_cb_private(struct t_cose_recipient_dec *me_x,
if(err != T_COSE_SUCCESS) {
goto Done;
}

encoded_empty_map = Q_USEFUL_BUF_FROM_SZ_LITERAL("\xa0");
if(!(q_useful_buf_c_is_empty(protected_params) ||
!q_useful_buf_compare(protected_params, encoded_empty_map))) {
/* There's can't be any protected headers here because keywrap
* can't protected them (need an AEAD). While completely empty
* headers are preferred an empty map is allowed. */
// TODO: the right error here
return T_COSE_ERR_FAIL;
if(!t_cose_params_empty(protected_params)) {
return T_COSE_ERR_PROTECTED_NOT_ALLOWED;
}

/* ---- Third item -- ciphertext ---- */
QCBORDecode_GetByteString(cbor_decoder, &ciphertext);

Expand Down
18 changes: 4 additions & 14 deletions src/t_cose_recipient_enc_keywrap.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,32 +42,22 @@ t_cose_recipient_create_keywrap_cb_private(struct t_cose_recipient_enc *me_x,
QCBOREncode_OpenArray(cbor_encoder);

/* Output the header parameters */
params[0] = t_cose_param_make_alg_id(me->keywrap_cose_algorithm_id);
params[0].in_protected = false; /* Override t_cose_make_alg_id_parameter().
* There's no protection in Keywrap */
params[0] = t_cose_param_make_unprot_alg_id(me->keywrap_cose_algorithm_id);
if(!q_useful_buf_c_is_null(me->kid)) {
params[1] = t_cose_param_make_kid(me->kid);
params[0].next = &params[1];
}
params2 = params;
t_cose_params_append(&params2, me->added_params);
// TODO: make sure no custom headers are protected because
// there is no protect with key wrap
return_value = t_cose_headers_encode(cbor_encoder,
params2,
&protected_params_not);
if (return_value != T_COSE_SUCCESS) {
goto Done;
}
if(protected_params_not.len &&
q_useful_buf_compare(protected_params_not, Q_USEFUL_BUF_FROM_SZ_LITERAL("\xa0"))) {
/* Empty protected params can either be an empty bstr or a bstr
* with an empty map. Make sure they are empty here per section 5.4
* of RFC 9052.
*/
// TODO: Erata for 9052?
// 5.4 says this must be an empty bstr.
// 3 says either empty map or empty bstr.
if(!t_cose_params_empty(protected_params_not)) {
/* Section 8.5.2 of RFC 9052 requires the protected header bucket
* be an empty byte string. */
return_value = T_CODE_ERR_PROTECTED_PARAM_NOT_ALLOWED;
goto Done;
}
Expand Down
2 changes: 0 additions & 2 deletions src/t_cose_sign_verify.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,6 @@ decode_cose_signature(QCBORDecodeContext *cbor_decoder,

return_value = t_cose_headers_decode(cbor_decoder,
loc,
false,
special_decode_cb,
special_decode_ctx,
param_storage,
Expand Down Expand Up @@ -483,7 +482,6 @@ t_cose_sign_verify_private(struct t_cose_sign_verify_ctx *me,
decoded_params = NULL;
return_value = t_cose_headers_decode(&cbor_decoder,
header_location,
false,
me->special_param_decode_cb,
me->special_param_decode_ctx,
me->p_storage,
Expand Down
Loading

0 comments on commit 3076010

Please sign in to comment.