diff --git a/inc/t_cose/t_cose_parameters.h b/inc/t_cose/t_cose_parameters.h index 9f46566a..3f7c358c 100644 --- a/inc/t_cose/t_cose_parameters.h +++ b/inc/t_cose/t_cose_parameters.h @@ -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, @@ -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 @@ -486,7 +487,6 @@ 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, @@ -494,6 +494,21 @@ t_cose_headers_decode(QCBORDecodeContext *cbor_decoder, 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 * @@ -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) { diff --git a/src/t_cose_encrypt_dec.c b/src/t_cose_encrypt_dec.c index 5cc54d1f..b2025cbc 100644 --- a/src/t_cose_encrypt_dec.c +++ b/src/t_cose_encrypt_dec.c @@ -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 --- */ @@ -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 */ @@ -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) { diff --git a/src/t_cose_mac_validate.c b/src/t_cose_mac_validate.c index 5f832707..a4efd97a 100644 --- a/src/t_cose_mac_validate.c +++ b/src/t_cose_mac_validate.c @@ -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, diff --git a/src/t_cose_parameters.c b/src/t_cose_parameters.c index d608760f..d4662a6b 100644 --- a/src/t_cose_parameters.c +++ b/src/t_cose_parameters.c @@ -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, @@ -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, @@ -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; } diff --git a/src/t_cose_recipient_dec_esdh.c b/src/t_cose_recipient_dec_esdh.c index 8df95017..0c0b1e8e 100644 --- a/src/t_cose_recipient_dec_esdh.c +++ b/src/t_cose_recipient_dec_esdh.c @@ -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 */ diff --git a/src/t_cose_recipient_dec_keywrap.c b/src/t_cose_recipient_dec_keywrap.c index 1be130de..7e594a09 100644 --- a/src/t_cose_recipient_dec_keywrap.c +++ b/src/t_cose_recipient_dec_keywrap.c @@ -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; @@ -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 */ @@ -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); diff --git a/src/t_cose_recipient_enc_keywrap.c b/src/t_cose_recipient_enc_keywrap.c index 6eb2b7f8..7bafe35f 100644 --- a/src/t_cose_recipient_enc_keywrap.c +++ b/src/t_cose_recipient_enc_keywrap.c @@ -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 = ¶ms[1]; } params2 = params; t_cose_params_append(¶ms2, 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; } diff --git a/src/t_cose_sign_verify.c b/src/t_cose_sign_verify.c index 908a252d..95961747 100644 --- a/src/t_cose_sign_verify.c +++ b/src/t_cose_sign_verify.c @@ -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, @@ -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, diff --git a/test/t_cose_param_test.c b/test/t_cose_param_test.c index 5afac2ce..888c1d7d 100644 --- a/test/t_cose_param_test.c +++ b/test/t_cose_param_test.c @@ -440,7 +440,7 @@ static const uint8_t crit_alg_id_encoded_cbor[] = { static const uint8_t empty_preferred_encoded_cbor[] = {0x40, 0xA0}; -static const uint8_t empty_alt_encoded_cbor[] = {0x40, 0xA0}; +static const uint8_t empty_alt_encoded_cbor[] = {0x41, 0xA0, 0xA0}; #if FIXES_FOR_INDEF_LEN static const uint8_t empty_preferred_indef[] = {0x5f, 0xff, 0xbf, 0xff}; @@ -876,7 +876,7 @@ param_test(void) /* Test is driven by data in param_tests and param_combo_tests. * This is all a bit more complicated than expected, but it is - * a data driven tests. */ + * a data driven test. */ /* The single parameter tests */ for(int i = 0; ; i++) { @@ -895,7 +895,7 @@ param_test(void) QCBOREncode_Init(&qcbor_encoder, encode_buffer); t_cose_result = t_cose_headers_encode(&qcbor_encoder, &(param_test->unencoded), - NULL); + &encoded_prot_params); if(t_cose_result != param_test->encode_result) { return i * 1000 + 1; @@ -932,7 +932,6 @@ param_test(void) t_cose_result = t_cose_headers_decode(&decode_context, (struct t_cose_header_location){0,0}, - false, param_decoder, NULL, ¶m_storage, @@ -1027,7 +1026,7 @@ param_test(void) /* Empty parameters section test */ QCBOREncode_Init(&qcbor_encoder, encode_buffer); - t_cose_result = t_cose_headers_encode(&qcbor_encoder, NULL, NULL); + t_cose_result = t_cose_headers_encode(&qcbor_encoder, NULL, &encoded_prot_params); if(t_cose_result != param_test->encode_result) { return -900; @@ -1040,10 +1039,13 @@ param_test(void) } if(qcbor_result == QCBOR_SUCCESS) { - if(q_useful_buf_compare(encoded_params, Q_USEFUL_BUF_FROM_BYTE_ARRAY_LITERAL(empty_alt_encoded_cbor))) { + if(q_useful_buf_compare(encoded_params, Q_USEFUL_BUF_FROM_BYTE_ARRAY_LITERAL(empty_preferred_encoded_cbor))) { return -900; } } + if(!t_cose_params_empty_bstr(encoded_prot_params)) { + return -901; + } T_COSE_PARAM_STORAGE_INIT(param_storage, param_array); @@ -1053,7 +1055,6 @@ param_test(void) t_cose_result = t_cose_headers_decode(&decode_context, (struct t_cose_header_location){0,0}, - false, param_decoder, NULL, ¶m_storage, &decoded_parameter, @@ -1072,27 +1073,53 @@ param_test(void) QCBORDecode_Init(&decode_context, UsefulBuf_FROM_BYTE_ARRAY_LITERAL(unprot_non_aead_alg), 0); t_cose_result = t_cose_headers_decode(&decode_context, (struct t_cose_header_location){0,0}, - true, param_decoder, NULL, ¶m_storage, &decoded_parameter, &encoded_prot_params); if(t_cose_result != T_COSE_SUCCESS) { - return 0; + return 800; + } + if(!t_cose_params_empty(encoded_prot_params)) { + return -901; + } + if(!t_cose_params_empty_bstr(encoded_prot_params)) { + return -902; } + /* Empty protected headers in alt form */ + QCBORDecode_Init(&decode_context, UsefulBuf_FROM_BYTE_ARRAY_LITERAL(empty_alt_encoded_cbor), 0); + t_cose_result = t_cose_headers_decode(&decode_context, + (struct t_cose_header_location){0,0}, + param_decoder, NULL, + ¶m_storage, + &decoded_parameter, + &encoded_prot_params); + if(t_cose_result != T_COSE_SUCCESS) { + return 800; + } + if(!t_cose_params_empty(encoded_prot_params)) { + return -901; + } + if(t_cose_params_empty_bstr(encoded_prot_params)) { + return -902; + } + + /* Protected headers must be empty and they are NOT */ QCBORDecode_Init(&decode_context, UsefulBuf_FROM_BYTE_ARRAY_LITERAL(common_params_encoded_cbor), 0); t_cose_result = t_cose_headers_decode(&decode_context, (struct t_cose_header_location){0,0}, - true, param_decoder, NULL, ¶m_storage, &decoded_parameter, &encoded_prot_params); - if(t_cose_result != T_COSE_ERR_PROTECTED_NOT_ALLOWED) { + if(t_cose_result != T_COSE_SUCCESS) { return -90000; } + if(t_cose_params_empty(encoded_prot_params)) { + return -901; + } return 0; } @@ -1193,7 +1220,6 @@ common_params_test(void) t_cose_result = t_cose_headers_decode(&decode_context, (struct t_cose_header_location){0,0}, - false, NULL, NULL, ¶m_storage, @@ -1275,7 +1301,6 @@ common_params_test(void) t_cose_result = t_cose_headers_decode(&decode_context, (struct t_cose_header_location){0,0}, - false, NULL, NULL, ¶m_storage,