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

More rework for empty protected headers #272

Merged
merged 3 commits into from
Nov 6, 2023
Merged
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
43 changes: 41 additions & 2 deletions inc/t_cose/t_cose_parameters.h
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
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
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
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
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
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
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
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