From fa047af2e05e0817909fbbe92ef4915f4ad1435b Mon Sep 17 00:00:00 2001 From: Laurence Lundblade Date: Sat, 4 Nov 2023 12:29:59 +0100 Subject: [PATCH] Support for empty protected headers (#267) * Support for empty protected headers * decoding/encoding of unprotected alg id param * documentation formatting --------- Co-authored-by: Laurence Lundblade --- inc/t_cose/t_cose_common.h | 8 ++- inc/t_cose/t_cose_parameters.h | 88 +++++++++++++++++++++++++++-- src/t_cose_encrypt_dec.c | 3 +- src/t_cose_mac_validate.c | 3 +- src/t_cose_parameters.c | 46 ++++++++------- src/t_cose_recipient_dec_esdh.c | 3 +- src/t_cose_recipient_dec_keywrap.c | 3 +- src/t_cose_sign_verify.c | 2 + src/t_cose_signature_verify_eddsa.c | 2 +- src/t_cose_signature_verify_main.c | 2 +- test/t_cose_param_test.c | 54 ++++++++++++++++-- test/t_cose_param_test.h | 2 + 12 files changed, 180 insertions(+), 36 deletions(-) diff --git a/inc/t_cose/t_cose_common.h b/inc/t_cose/t_cose_common.h index de37da77..c671cf00 100644 --- a/inc/t_cose/t_cose_common.h +++ b/inc/t_cose/t_cose_common.h @@ -9,7 +9,6 @@ * See BSD-3-Clause license in README.md */ - #ifndef __T_COSE_COMMON_H__ #define __T_COSE_COMMON_H__ @@ -636,7 +635,12 @@ enum t_cose_err_t { * to be larger because there are too many protected * headers, party u/v identities were added or * supp info was added. TODO: see xxxx*/ - T_COSE_ERR_KDF_CONTEXT_SIZE = 88 + T_COSE_ERR_KDF_CONTEXT_SIZE = 88, + + /** Protected headers exists when they are not allowed. This typically occurs when the + * crypto algorithm is not AEAD and thus can't protect the headers. */ + T_COSE_ERR_PROTECTED_NOT_ALLOWED = 89, + }; diff --git a/inc/t_cose/t_cose_parameters.h b/inc/t_cose/t_cose_parameters.h index c473552a..574ad122 100644 --- a/inc/t_cose/t_cose_parameters.h +++ b/inc/t_cose/t_cose_parameters.h @@ -431,6 +431,7 @@ 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 @@ -462,6 +463,11 @@ t_cose_headers_encode(QCBOREncodeContext *cbor_encoder, * non-integer and non-string header parameters. It typically switches * on the parameter label. * + * If \c no_protected is \c true, then the protected headers must be + * an empty byte string. This is used when the cryptographic algorithm + * used can't protect headers, for example non-AEAD ciphers for COSE + * encryption. + * * The crit parameter will be decoded and any parameter label * listed in it will be marked as crit in the list returned. It is up * to the caller to check the list for crit parameters and error @@ -480,6 +486,7 @@ 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, @@ -557,6 +564,11 @@ t_cose_params_append(struct t_cose_parameter **existing, static struct t_cose_parameter t_cose_param_make_alg_id(int32_t alg_id); +/* For the rare case of a non-AEAD algorithm */ +static struct t_cose_parameter +t_cose_param_make_unprot_alg_id(int32_t alg_id); + + /** * Make a struct t_cose_parameter for an unsigned integer content typ. * @@ -642,19 +654,52 @@ t_cose_param_find_bstr(const struct t_cose_parameter *parameter_list, int64_t la /** - * \brief Find the algorithm ID parameter in a linked list + * \brief Find the algorithm ID parameter in a linked list. * * \param[in] parameter_list The parameter list to search. - * \param[in] prot If \c true, parameter must be protected and vice versa. + * \param[in] prot Place to return whether ID is protected or not. * * \return The algorithm ID or \ref T_COSE_ALGORITHM_NONE. * * This returns \ref T_COSE_ALGORITHM_NONE on all errors including * errors such as the parameter not being present, the parameter being - * of the wrong type and the parameter not being protected. + * of the wrong type. */ int32_t -t_cose_param_find_alg_id(const struct t_cose_parameter *parameter_list, bool prot); +t_cose_param_find_alg_id(const struct t_cose_parameter *parameter_list, bool *prot); + + +/** + * \brief Find the protected algorithm ID parameter in a linked list. + * + * \param[in] parameter_list The parameter list to search. + * + * \return The algorithm ID or \ref T_COSE_ALGORITHM_NONE. + * + * This returns \ref T_COSE_ALGORITHM_NONE on all errors including + * errors such as the parameter not being present, the parameter being + * of the wrong type or the parameter not being protected. + */ +static int32_t +t_cose_param_find_alg_id_prot(const struct t_cose_parameter *parameter_list); + + +/** + * \brief Find the unprotected algorithm ID parameter in a linked list. + * + * \param[in] parameter_list The parameter list to search. + * + * \return The algorithm ID or \ref T_COSE_ALGORITHM_NONE. + * + * This is for the unusual case were the algorithm ID must NOT be a + * protected parameter, such as for non-AEAD algorithms. + * + * This returns \ref T_COSE_ALGORITHM_NONE on all errors including + * errors such as the parameter not being present, the parameter being + * of the wrong type or the parameter being protected. + */ +static int32_t +t_cose_param_find_alg_id_unprot(const struct t_cose_parameter *parameter_list); /** @@ -848,6 +893,15 @@ t_cose_param_make_alg_id(int32_t alg_id) return parameter; } +static inline struct t_cose_parameter +t_cose_param_make_unprot_alg_id(int32_t alg_id) { + struct t_cose_parameter parameter; + + parameter = t_cose_param_make_alg_id(alg_id); + parameter.in_protected = false; + return parameter; +} + static inline struct t_cose_parameter t_cose_param_make_unprot_bstr(struct q_useful_buf_c string, int32_t label) { @@ -950,6 +1004,32 @@ t_cose_param_make_partial_iv(struct q_useful_buf_c iv) return parameter; } +static inline int32_t +t_cose_param_find_alg_id_prot(const struct t_cose_parameter *parameter_list) +{ + int32_t alg_id; + bool prot; + alg_id = t_cose_param_find_alg_id(parameter_list, &prot); + if(prot != true) { + return T_COSE_ALGORITHM_NONE; + } + + return alg_id; +} + + +static inline int32_t +t_cose_param_find_alg_id_unprot(const struct t_cose_parameter *parameter_list) +{ + int32_t alg_id; + bool prot; + alg_id = t_cose_param_find_alg_id(parameter_list, &prot); + if(prot != true) { + return T_COSE_ALGORITHM_NONE; + } + + return alg_id; +} static inline void diff --git a/src/t_cose_encrypt_dec.c b/src/t_cose_encrypt_dec.c index 8e25a08d..0b502baf 100644 --- a/src/t_cose_encrypt_dec.c +++ b/src/t_cose_encrypt_dec.c @@ -180,6 +180,7 @@ 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 */ @@ -191,7 +192,7 @@ t_cose_encrypt_dec_detached(struct t_cose_encrypt_dec_ctx* me, } nonce_cbor = t_cose_param_find_iv(body_params_list); - ce_alg.cose_alg_id = t_cose_param_find_alg_id(body_params_list, true); + ce_alg.cose_alg_id = t_cose_param_find_alg_id_prot(body_params_list); all_params_list = body_params_list; ce_alg.bits_in_key = bits_in_crypto_alg(ce_alg.cose_alg_id); diff --git a/src/t_cose_mac_validate.c b/src/t_cose_mac_validate.c index 4392ede4..f074dd5e 100644 --- a/src/t_cose_mac_validate.c +++ b/src/t_cose_mac_validate.c @@ -80,6 +80,7 @@ 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, @@ -146,7 +147,7 @@ t_cose_mac_validate_private(struct t_cose_mac_validate_ctx *me, * payload, to save a bigger buffer containing the entire ToBeMaced. */ return_value = t_cose_crypto_hmac_validate_setup(&hmac_ctx, - t_cose_param_find_alg_id(decoded_params, true), + t_cose_param_find_alg_id_prot(decoded_params), me->validation_key); if(return_value) { diff --git a/src/t_cose_parameters.c b/src/t_cose_parameters.c index 6ce2c93b..312db5f3 100644 --- a/src/t_cose_parameters.c +++ b/src/t_cose_parameters.c @@ -487,13 +487,14 @@ 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, struct t_cose_parameter **decoded_params, struct q_useful_buf_c *protected_parameters) { - /* Approximate stack usage + /* Approximate stack usage * 64-bit 32-bit * local vars 24 12 * largest call, t_cose_params_decode 416 352 @@ -503,27 +504,36 @@ 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 --- */ - 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) { + if(no_protected) { + QCBORDecode_GetByteString(cbor_decoder, &empty_protected); + if(empty_protected.len != 0) { + return_value = T_COSE_ERR_PROTECTED_NOT_ALLOWED; 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, @@ -722,7 +732,7 @@ t_cose_param_find(const struct t_cose_parameter *parameter_list, int64_t label) * Public function. See t_cose_parameters.h */ int32_t -t_cose_param_find_alg_id(const struct t_cose_parameter *parameter_list, bool prot) +t_cose_param_find_alg_id(const struct t_cose_parameter *parameter_list, bool *prot) { const struct t_cose_parameter *p_found; @@ -734,9 +744,7 @@ t_cose_param_find_alg_id(const struct t_cose_parameter *parameter_list, bool pro return T_COSE_ALGORITHM_NONE; } - if(prot != p_found->in_protected) { /* effective exclusive OR */ - return T_COSE_ALGORITHM_NONE; - } + *prot = p_found->in_protected; return (int32_t)p_found->value.int64; } diff --git a/src/t_cose_recipient_dec_esdh.c b/src/t_cose_recipient_dec_esdh.c index 5c832e29..16c84164 100644 --- a/src/t_cose_recipient_dec_esdh.c +++ b/src/t_cose_recipient_dec_esdh.c @@ -143,6 +143,7 @@ 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 callback */ p_storage, /* in: parameter storage */ @@ -181,7 +182,7 @@ t_cose_recipient_dec_esdh_cb_private(struct t_cose_recipient_dec *me_x, goto done_free_ec; } - alg = t_cose_param_find_alg_id(*params, true); + alg = t_cose_param_find_alg_id_prot(*params); // TODO: indention of all case statements. Which is it? switch(alg) { diff --git a/src/t_cose_recipient_dec_keywrap.c b/src/t_cose_recipient_dec_keywrap.c index 6a97b263..1be130de 100644 --- a/src/t_cose_recipient_dec_keywrap.c +++ b/src/t_cose_recipient_dec_keywrap.c @@ -50,6 +50,7 @@ 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 */ @@ -80,7 +81,7 @@ t_cose_recipient_dec_keywrap_cb_private(struct t_cose_recipient_dec *me_x, T_COSE_ERR_RECIPIENT_FORMAT); } - cose_algorithm_id = t_cose_param_find_alg_id(*params, false); + cose_algorithm_id = t_cose_param_find_alg_id_unprot(*params); // TODO: should probably check the kid here diff --git a/src/t_cose_sign_verify.c b/src/t_cose_sign_verify.c index 80d7aa10..100e4964 100644 --- a/src/t_cose_sign_verify.c +++ b/src/t_cose_sign_verify.c @@ -142,6 +142,7 @@ decode_cose_signature(QCBORDecodeContext *cbor_decoder, return_value = t_cose_headers_decode(cbor_decoder, loc, + false, special_decode_cb, special_decode_ctx, param_storage, @@ -482,6 +483,7 @@ 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/src/t_cose_signature_verify_eddsa.c b/src/t_cose_signature_verify_eddsa.c index 61eed355..6125ac16 100644 --- a/src/t_cose_signature_verify_eddsa.c +++ b/src/t_cose_signature_verify_eddsa.c @@ -35,7 +35,7 @@ t_cose_signature_verify_eddsa_cb(struct t_cose_signature_verify *me_x, struct q_useful_buf_c tbs; /* --- Check the algorithm --- */ - cose_algorithm_id = t_cose_param_find_alg_id(parameter_list, true); + cose_algorithm_id = t_cose_param_find_alg_id_prot(parameter_list); if(cose_algorithm_id == T_COSE_ALGORITHM_NONE) { return_value = T_COSE_ERR_NO_ALG_ID; goto Done; diff --git a/src/t_cose_signature_verify_main.c b/src/t_cose_signature_verify_main.c index 31334e73..000adc5e 100644 --- a/src/t_cose_signature_verify_main.c +++ b/src/t_cose_signature_verify_main.c @@ -96,7 +96,7 @@ t_cose_signature_verify_main_cb(struct t_cose_signature_verify *me_x, struct q_useful_buf_c tbs_hash; /* --- Get the parameters values needed --- */ - cose_algorithm_id = t_cose_param_find_alg_id(parameter_list, true); + cose_algorithm_id = t_cose_param_find_alg_id_prot(parameter_list); if(cose_algorithm_id == T_COSE_ALGORITHM_NONE) { return_value = T_COSE_ERR_NO_ALG_ID; goto Done; diff --git a/test/t_cose_param_test.c b/test/t_cose_param_test.c index 2262d440..8ce71262 100644 --- a/test/t_cose_param_test.c +++ b/test/t_cose_param_test.c @@ -448,6 +448,13 @@ static const uint8_t empty_preferred_indef[] = {0x5f, 0xff, 0xbf, 0xff}; static const uint8_t empty_alt_indef[] = {0x5f, 0xbf, 0xff, 0xff, 0xbf, 0xff}; #endif +static const uint8_t unprot_non_aead_alg[] = { + 0x40, + 0xA2, + 0x18, 0x2C, 0xFB, 0x40, 0x09, 0x1E, 0xB8, 0x51, 0xEB, 0x85, 0x1F, + 0x01, 0x39, 0xFF, 0xFD +}; + /* Alternative to UsefulBuf_FROM_BYTE_ARRAY_LITERAL() & * UsefulBuf_FROM_SZ_LITERAL()that works for static data initialization. */ @@ -925,6 +932,7 @@ 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, @@ -1019,9 +1027,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, NULL); if(t_cose_result != param_test->encode_result) { return -900; @@ -1047,6 +1053,7 @@ 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, @@ -1061,6 +1068,32 @@ param_test(void) } } + /* Protected headers must be empty and they are */ + 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; + } + + /* 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) { + return -90000; + } + return 0; } @@ -1114,6 +1147,15 @@ common_params_test(void) return -1; } + /* One-off test for unprotected alg ID */ + param_array[0] = t_cose_param_make_unprot_alg_id(T_COSE_ALGORITHM_ES256); // TODO: change this to AES-CTR + if(param_array[0].in_protected == true) { + return -101; + } + if(param_array[0].value.int64 != T_COSE_ALGORITHM_ES256) { + return -102; + } + qcbor_result = QCBOREncode_Finish(&qcbor_encoder, &encoded_params); if(qcbor_result != QCBOR_SUCCESS) { return -2; @@ -1124,7 +1166,7 @@ common_params_test(void) } /* --- Decode what was encoded ---*/ - if(t_cose_param_find_alg_id(NULL, true) != T_COSE_ALGORITHM_NONE) { + if(t_cose_param_find_alg_id_prot(NULL) != T_COSE_ALGORITHM_NONE) { return -4; } @@ -1151,6 +1193,7 @@ 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, @@ -1166,7 +1209,7 @@ common_params_test(void) } /* Check that they decoded correctly */ - if(t_cose_param_find_alg_id(dec, true) != T_COSE_ALGORITHM_ES256) { + if(t_cose_param_find_alg_id_prot(dec) != T_COSE_ALGORITHM_ES256) { return -11; } @@ -1232,6 +1275,7 @@ 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, diff --git a/test/t_cose_param_test.h b/test/t_cose_param_test.h index 64262cba..f099c2da 100644 --- a/test/t_cose_param_test.h +++ b/test/t_cose_param_test.h @@ -14,8 +14,10 @@ #include +/* This tests the generic params encoding and decoding functions */ int32_t param_test(void); +/* This tests the utility functions for specific params like alg ID and iv */ int32_t common_params_test(void); #endif /* t_cose_param_test_h */