From 5190853a1839419e1a84cb28db4df383ca351982 Mon Sep 17 00:00:00 2001 From: Joe Powell <56188788+darracott@users.noreply.github.com> Date: Wed, 15 Feb 2023 15:26:59 +0000 Subject: [PATCH] Update t_cose from v1.1 to v1.1.1 (#5014) --- 3rdparty/exported/t_cose/CONTRIBUTING.md | 85 +++++++++++++++++++ .../t_cose/inc/t_cose/t_cose_sign1_verify.h | 11 +++ .../exported/t_cose/src/t_cose_sign1_verify.c | 10 ++- 3rdparty/exported/t_cose/test/t_cose_test.c | 78 +++++++++-------- CHANGELOG.md | 1 + cgmanifest.json | 2 +- 6 files changed, 145 insertions(+), 42 deletions(-) create mode 100644 3rdparty/exported/t_cose/CONTRIBUTING.md diff --git a/3rdparty/exported/t_cose/CONTRIBUTING.md b/3rdparty/exported/t_cose/CONTRIBUTING.md new file mode 100644 index 000000000..e4c069eb9 --- /dev/null +++ b/3rdparty/exported/t_cose/CONTRIBUTING.md @@ -0,0 +1,85 @@ +## Branch strategy + +The master branch is maintained at commercial quality. It only supports +COSE_Sign1. Changes against it must be very complete. Periodically a +new 1.x release is made from the master branch. + +The "dev" branch is to become t_cose 2.0 when it gets to commercial +quality. It is to support COSE_Mac, COSE_Encrypt, multiple signers and +such. PRs to it mainly need to not break the build and the CI. + + +## Tests and CI + +Generally any PR should still have all these tests passing before it +will be merged. + +### Basic tests + +Running the standard make will produce `t_cose_test` for a particular +crypto library. It runs a thorough set of tests. + +### CI tests + +GitHub CI is used to build and test against several crypto libraries. + +### tdv/b.sh tests + +In the "tdv" repository there are some makes files and build scripts +that do some more thorough testing. They are too large and take too +long to run to make them part of normal CI. Here's a few things they +do: + +* Build with a fairly maximal set of compiler warning flags set (e.g., + -Wall) + +* Build and test every combination of `#define` for configuration (this + is what takes a long time) + +* Build for use in a C++ program + +* Measure and output code size + + +## PR Characteristics + +I review PRs pretty thoroughly largely because I assume I am going to +have to provide support for them long term as I aim to make sure this +is high-quality, tested commercial code. No one pays me for this and +there is no team or company behind this. + +Here's a few notes on getting PRs merged: + +* Only change what you need to. Don't beautify code that is not + related to your change. Don't adjust tabs and spaces. Don't fix + spelling mistakes or comment formatting that is not related to the + change. This helps reduce merge conflicts and makes review + easier. If you want to beautify and such, please do, but in + separate PRs so intent is clear. + +* Make sure all the tests mentioned above in Tests and CI are passing + +* Target the right branch. It is fairly easy to change the target + branch of a PR in GitHub. + + + +## Makefile and project files + +There are makefiles for both cmake and regular make. Both sets need to +be kept up to date and functioning. + +The GitHub CI uses cmake. + +There are several "Makefile"s for different crypto +libraries. Generally, a change to one means a change to the others +(though we can probably make that better with a little more work). + + +## Coding Style + +QCBOR uses camelCase and t_cose follows [Arm's coding +guidelines](https://git.trustedfirmware.org/TF-M/trusted-firmware-m.git/tree/docs/contributing/coding_guide.rst) +resulting in code with mixed styles. For better or worse, an Arm-style +version of UsefulBuf is created and used and so there is a duplicate +of UsefulBuf. The two are identical. They just have different names. diff --git a/3rdparty/exported/t_cose/inc/t_cose/t_cose_sign1_verify.h b/3rdparty/exported/t_cose/inc/t_cose/t_cose_sign1_verify.h index 2da0bce73..87a20d5e3 100644 --- a/3rdparty/exported/t_cose/inc/t_cose/t_cose_sign1_verify.h +++ b/3rdparty/exported/t_cose/inc/t_cose/t_cose_sign1_verify.h @@ -177,6 +177,17 @@ struct t_cose_parameters { #define T_COSE_OPT_DECODE_ONLY 0x00000008 +/** + * This option disables verification that critical header parameters are + * known. + * + * Without this flag set, an error is raised during verification if there + * is an unknown header parameter in the critical header parameters list. + * However, if this flag is set then that part of verification is skipped. + */ +#define T_COSE_OPT_UNKNOWN_CRIT_ALLOWED 0x00000020 + + /** * The maximum number of unprocessed tags that can be returned by * t_cose_sign1_get_nth_tag(). The CWT diff --git a/3rdparty/exported/t_cose/src/t_cose_sign1_verify.c b/3rdparty/exported/t_cose/src/t_cose_sign1_verify.c index d6ee7fdd3..e4d004aca 100644 --- a/3rdparty/exported/t_cose/src/t_cose_sign1_verify.c +++ b/3rdparty/exported/t_cose/src/t_cose_sign1_verify.c @@ -466,10 +466,12 @@ t_cose_sign1_verify_internal(struct t_cose_sign1_verify_ctx *me, goto Done; } - return_value = check_critical_labels(&critical_parameter_labels, - &unknown_parameter_labels); - if(return_value != T_COSE_SUCCESS) { - goto Done; + if (!(me->option_flags & T_COSE_OPT_UNKNOWN_CRIT_ALLOWED)) { + return_value = check_critical_labels(&critical_parameter_labels, + &unknown_parameter_labels); + if(return_value != T_COSE_SUCCESS) { + goto Done; + } } #ifndef T_COSE_DISABLE_SHORT_CIRCUIT_SIGN diff --git a/3rdparty/exported/t_cose/test/t_cose_test.c b/3rdparty/exported/t_cose/test/t_cose_test.c index 0ef5de0e8..e514ae5df 100644 --- a/3rdparty/exported/t_cose/test/t_cose_test.c +++ b/3rdparty/exported/t_cose/test/t_cose_test.c @@ -715,7 +715,8 @@ int_fast32_t cose_example_test() } -static enum t_cose_err_t run_test_sign_and_verify(uint32_t test_mess_options) +static enum t_cose_err_t run_test_sign_and_verify(uint32_t test_mess_options, + uint32_t verify_options) { struct t_cose_sign1_sign_ctx sign_ctx; struct t_cose_sign1_verify_ctx verify_ctx; @@ -747,7 +748,7 @@ static enum t_cose_err_t run_test_sign_and_verify(uint32_t test_mess_options) /* --- Start verifying the COSE Sign1 object --- */ - t_cose_sign1_verify_init(&verify_ctx, T_COSE_OPT_ALLOW_SHORT_CIRCUIT); + t_cose_sign1_verify_init(&verify_ctx, T_COSE_OPT_ALLOW_SHORT_CIRCUIT | verify_options); /* No key necessary with short circuit */ @@ -839,44 +840,45 @@ int_fast32_t all_header_parameters_test() struct test_case { uint32_t test_option; + uint32_t verify_option; enum t_cose_err_t result; }; static struct test_case bad_parameters_tests_table[] = { - {T_COSE_TEST_EMPTY_PROTECTED_PARAMETERS, T_COSE_ERR_UNSUPPORTED_SIGNING_ALG}, + {T_COSE_TEST_EMPTY_PROTECTED_PARAMETERS, 0, T_COSE_ERR_UNSUPPORTED_SIGNING_ALG}, - {T_COSE_TEST_UNCLOSED_PROTECTED, T_COSE_ERR_PARAMETER_CBOR}, + {T_COSE_TEST_UNCLOSED_PROTECTED, 0, T_COSE_ERR_PARAMETER_CBOR}, #ifndef T_COSE_DISABLE_CONTENT_TYPE - {T_COSE_TEST_DUP_CONTENT_ID, T_COSE_ERR_DUPLICATE_PARAMETER}, + {T_COSE_TEST_DUP_CONTENT_ID, 0, T_COSE_ERR_DUPLICATE_PARAMETER}, - {T_COSE_TEST_TOO_LARGE_CONTENT_TYPE, T_COSE_ERR_BAD_CONTENT_TYPE}, + {T_COSE_TEST_TOO_LARGE_CONTENT_TYPE, 0, T_COSE_ERR_BAD_CONTENT_TYPE}, #endif /* T_COSE_DISABLE_CONTENT_TYPE */ - {T_COSE_TEST_NOT_WELL_FORMED_2, T_COSE_ERR_CBOR_NOT_WELL_FORMED}, + {T_COSE_TEST_NOT_WELL_FORMED_2, 0, T_COSE_ERR_CBOR_NOT_WELL_FORMED}, - {T_COSE_TEST_KID_IN_PROTECTED, T_COSE_ERR_DUPLICATE_PARAMETER}, + {T_COSE_TEST_KID_IN_PROTECTED, 0, T_COSE_ERR_DUPLICATE_PARAMETER}, - {T_COSE_TEST_TOO_MANY_UNKNOWN, T_COSE_ERR_TOO_MANY_PARAMETERS}, + {T_COSE_TEST_TOO_MANY_UNKNOWN, 0, T_COSE_ERR_TOO_MANY_PARAMETERS}, - {T_COSE_TEST_UNPROTECTED_NOT_MAP, T_COSE_ERR_PARAMETER_CBOR}, + {T_COSE_TEST_UNPROTECTED_NOT_MAP, 0, T_COSE_ERR_PARAMETER_CBOR}, - {T_COSE_TEST_BAD_CRIT_PARAMETER, T_COSE_ERR_CRIT_PARAMETER}, + {T_COSE_TEST_BAD_CRIT_PARAMETER, 0, T_COSE_ERR_CRIT_PARAMETER}, - {T_COSE_TEST_NOT_WELL_FORMED_1, T_COSE_ERR_CBOR_NOT_WELL_FORMED}, + {T_COSE_TEST_NOT_WELL_FORMED_1, 0, T_COSE_ERR_CBOR_NOT_WELL_FORMED}, - {T_COSE_TEST_NO_UNPROTECTED_PARAMETERS, T_COSE_ERR_PARAMETER_CBOR}, + {T_COSE_TEST_NO_UNPROTECTED_PARAMETERS, 0, T_COSE_ERR_PARAMETER_CBOR}, - {T_COSE_TEST_NO_PROTECTED_PARAMETERS, T_COSE_ERR_PARAMETER_CBOR}, + {T_COSE_TEST_NO_PROTECTED_PARAMETERS, 0, T_COSE_ERR_PARAMETER_CBOR}, - {T_COSE_TEST_EXTRA_PARAMETER, T_COSE_SUCCESS}, + {T_COSE_TEST_EXTRA_PARAMETER, 0, T_COSE_SUCCESS}, - {T_COSE_TEST_PARAMETER_LABEL, T_COSE_ERR_PARAMETER_CBOR}, + {T_COSE_TEST_PARAMETER_LABEL, 0, T_COSE_ERR_PARAMETER_CBOR}, - {T_COSE_TEST_BAD_PROTECTED, T_COSE_ERR_PARAMETER_CBOR}, + {T_COSE_TEST_BAD_PROTECTED, 0, T_COSE_ERR_PARAMETER_CBOR}, - {0, 0} + {0, 0, 0} }; @@ -888,7 +890,7 @@ int_fast32_t bad_parameters_test() struct test_case *test; for(test = bad_parameters_tests_table; test->test_option; test++) { - if(run_test_sign_and_verify(test->test_option) != test->result) { + if(run_test_sign_and_verify(test->test_option, test->verify_option) != test->result) { return (int_fast32_t)(test - bad_parameters_tests_table + 1); } } @@ -903,34 +905,36 @@ static struct test_case crit_tests_table[] = { /* Test existance of the critical header. Also makes sure that * it works with the max number of labels allowed in it. */ - {T_COSE_TEST_CRIT_PARAMETER_EXIST, T_COSE_SUCCESS}, + {T_COSE_TEST_CRIT_PARAMETER_EXIST, 0, T_COSE_SUCCESS}, /* Exceed the max number of labels by one and get an error */ - {T_COSE_TEST_TOO_MANY_CRIT_PARAMETER_EXIST, T_COSE_ERR_CRIT_PARAMETER}, + {T_COSE_TEST_TOO_MANY_CRIT_PARAMETER_EXIST, 0, T_COSE_ERR_CRIT_PARAMETER}, /* A critical parameter exists in the protected section, but the * format of the internals of this parameter is not the expected CBOR */ - {T_COSE_TEST_BAD_CRIT_LABEL, T_COSE_ERR_CRIT_PARAMETER}, + {T_COSE_TEST_BAD_CRIT_LABEL, 0, T_COSE_ERR_CRIT_PARAMETER}, - /* A critical label is listed in the protected section, but - * the label doesn't exist. This works for integer-labeled header params. - */ - {T_COSE_TEST_UNKNOWN_CRIT_UINT_PARAMETER, T_COSE_ERR_UNKNOWN_CRITICAL_PARAMETER}, + /* The critical parameter includes an unknown uint label. */ + {T_COSE_TEST_UNKNOWN_CRIT_UINT_PARAMETER, 0, T_COSE_ERR_UNKNOWN_CRITICAL_PARAMETER}, - /* A critical label is listed in the protected section, but - * the label doesn't exist. This works for string-labeled header params. + /* The critical parameter includes an unknown tstr label. */ + {T_COSE_TEST_UNKNOWN_CRIT_TSTR_PARAMETER, 0, T_COSE_ERR_UNKNOWN_CRITICAL_PARAMETER}, + + /* The critical parameter includes an unknown label, but criticality + * checking is disabled. */ - {T_COSE_TEST_UNKNOWN_CRIT_TSTR_PARAMETER, T_COSE_ERR_UNKNOWN_CRITICAL_PARAMETER}, + {T_COSE_TEST_UNKNOWN_CRIT_UINT_PARAMETER, T_COSE_OPT_UNKNOWN_CRIT_ALLOWED, T_COSE_SUCCESS}, + {T_COSE_TEST_UNKNOWN_CRIT_TSTR_PARAMETER, T_COSE_OPT_UNKNOWN_CRIT_ALLOWED, T_COSE_SUCCESS}, /* The critical labels list is not protected */ - {T_COSE_TEST_CRIT_NOT_PROTECTED, T_COSE_ERR_PARAMETER_NOT_PROTECTED}, + {T_COSE_TEST_CRIT_NOT_PROTECTED, 0, T_COSE_ERR_PARAMETER_NOT_PROTECTED}, - {T_COSE_TEST_EMPTY_CRIT_PARAMETER, T_COSE_ERR_CRIT_PARAMETER}, + {T_COSE_TEST_EMPTY_CRIT_PARAMETER, 0, T_COSE_ERR_CRIT_PARAMETER}, - {T_COSE_TEST_TOO_MANY_TSTR_CRIT_LABLELS, T_COSE_ERR_CRIT_PARAMETER}, + {T_COSE_TEST_TOO_MANY_TSTR_CRIT_LABLELS, 0, T_COSE_ERR_CRIT_PARAMETER}, - {0, 0} + {0, 0, 0} }; @@ -942,7 +946,7 @@ int_fast32_t crit_parameters_test() struct test_case *test; for(test = crit_tests_table; test->test_option; test++) { - if(run_test_sign_and_verify(test->test_option) != test->result) { + if(run_test_sign_and_verify(test->test_option, test->verify_option) != test->result) { return (int_fast32_t)(test - crit_tests_table + 1); } } @@ -1633,7 +1637,7 @@ int_fast32_t indef_array_and_map_test() */ /* General test with indefinite lengths */ - return_value = run_test_sign_and_verify(T_COSE_TEST_INDEFINITE_MAPS_ARRAYS); + return_value = run_test_sign_and_verify(T_COSE_TEST_INDEFINITE_MAPS_ARRAYS, 0); if(return_value != T_COSE_SUCCESS) { return 1000 + (int32_t) return_value; } @@ -1641,7 +1645,7 @@ int_fast32_t indef_array_and_map_test() /* Test critical parameters encoded as indefinite length */ t_opts = T_COSE_TEST_INDEFINITE_MAPS_ARRAYS | T_COSE_TEST_UNKNOWN_CRIT_UINT_PARAMETER; - return_value = run_test_sign_and_verify(t_opts); + return_value = run_test_sign_and_verify(t_opts, 0); if(return_value != T_COSE_ERR_UNKNOWN_CRITICAL_PARAMETER) { return 2000 + (int32_t) return_value; } @@ -1649,7 +1653,7 @@ int_fast32_t indef_array_and_map_test() /* Another general test with indefinite lengths */ t_opts = T_COSE_TEST_INDEFINITE_MAPS_ARRAYS | T_COSE_TEST_ALL_PARAMETERS; - return_value = run_test_sign_and_verify(t_opts); + return_value = run_test_sign_and_verify(t_opts, 0); if(return_value != T_COSE_SUCCESS) { return 3000 + (int32_t) return_value; } diff --git a/CHANGELOG.md b/CHANGELOG.md index c449c24a7..2af530d0e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Dependencies - Upgraded OpenEnclave to [0.18.5](https://github.com/openenclave/openenclave/releases/tag/v0.18.5). +- Upgraded t_cose from [v1.1 to v1.1.1](https://github.com/laurencelundblade/t_cose/compare/v1.1...v1.1.1). v1.1.1 can optionally allow unknown critical header parameters in COSE_Sign1 envelopes which is desirable for CCF C++ applications. ## [4.0.0-dev4] diff --git a/cgmanifest.json b/cgmanifest.json index 1f70158c5..3268bed04 100644 --- a/cgmanifest.json +++ b/cgmanifest.json @@ -150,7 +150,7 @@ "type": "git", "git": { "repositoryUrl": "https://github.com/laurencelundblade/t_cose", - "commitHash": "e7a40d257a24d9c97066f920d419c5b11484a342" + "commitHash": "7d1b6686990e5bf6b84df422f93fb7f547669012" } } },