COSE: 1) Test key slot leak fix, 2) Test feature disable fix
Fixes so tests don't leak key slots or run out of key slots.
Tests run correctly when T_COSE_DISABLE_SHORT_CIRCUIT_SIGN
is defined. Other minor test improvements.
Change-Id: Ibe3364a795dbebe625e7c3d300d110968e536767
Signed-off-by: Laurence Lundblade <lgl@securitytheory.com>
diff --git a/lib/ext/t_cose/README.md b/lib/ext/t_cose/README.md
index 57d9f71..98662e9 100644
--- a/lib/ext/t_cose/README.md
+++ b/lib/ext/t_cose/README.md
@@ -31,8 +31,9 @@
## Code Status
-As of October 2019, the code is in reasonable working order and the public interface is
-fairly stable. There is a crypto adaptaion layer for [OpenSSL](https://www.openssl.org).
+As of December 2019, the code is in reasonable working order and the public interface is
+fairly stable. There is a crypto adaptaion layer for [OpenSSL](https://www.openssl.org)
+and for [Arm MBed Crypto](https://github.com/ARMmbed/mbed-crypto).
### The to-do list:
* Add some more tests, particular test vectors from C_COSE or such
@@ -40,35 +41,136 @@
## Building and Dependencies
-There is a simple makefile.
+Except for the crypto library set up, t_cose is very portable and
+should largely just work in any environment. It needs a few standard
+libraries and [QCBOR](https://github.com/laurencelundblade/QCBOR)
+(which is also very portable). Hence the rest of this section is about
+crypto library set up.
-[QCBOR](https://github.com/laurencelundblade/QCBOR) is required
+### Currently Supported Libraries
-### Crypto Library
-Some cryptographic library that supports ECDSA and at least SHA-256 is required.
+Here's three crypto library configurations that are supported. Others
+can be added with relative ease over time.
-#### Test Crypto
-Out of the box, this compiles, links and runs with no additional crypto library, but only in
-a test mode. This allows for quickly geting started. This mode allows more than half the tests to run.
-It is however not good for real commercial use as it doesn't do real public key signing and
-verification.
+#### Test Crypto -- Makefile.test
-For this test mode a bundled SHA-256 hash implementation is used, the ECDSA signing
-is stubbed to do nothing and the test short-circuit signing is used.
+This configuration should work instantly on any device and is useful
+to do quite a large amount of testing with, but can't be put to full
+commercial use. What it lacks is any integration with an ECDSA
+implementation so it can't produce real ECDSA signatures. It does
+however produce some fake signatures called "short-circuit
+signatures" that are very useful for testing. See header
+documentation for details on short-circuit sigs.
-#### OpenSSL Crypto
-This OpenSSL integration supports SHA-256, SHA-384 and SHA-512 with ECDSA to support
-the COSE algorithms ES256, ES384 and ES512.
+This configuration (and only this configuration) uses an bundled
+SHA-256 implementation (SHA-256 is simple and easy to bundle, ECDSA is
+not).
-To enable this:
-* #define T_COSE_USE_OPENSSL_CRYPTO
-* Make with crypto_adapters/t_cose_openssl_crypto.c
-* Define the include path to OpenSSL headers in your build environment
-* Link with the OpenSSL libary
+To use this, edit the makefile for the location of QCBOR and then just
+do
-#### PSA Crypto
+ make -f Makefile.test
+#### OpenSSL Crypto -- Makefile.ossl
+This OpenSSL integration supports SHA-256, SHA-384 and SHA-512 with
+ECDSA to support the COSE algorithms ES256, ES384 and ES512. It is a
+full and tested integration with OpenSSL crypto.
+
+To use this, edit the makefile for the location of QCBOR and OpenSSL
+and do:
+
+ make -f Makefile.ossl
+
+The specific things that Makefile.ossl does is:
+* Links the crypto_adapters/t_cose_openssl_crypto.o into libt_cose.a
+* Links test/test/t_cose_make_openssl_test_key.o into the test binary
+* `#define T_COSE_USE_OPENSSL_CRYPTO`
+
+Note that the internally supplied b_con_hash is not used in this case
+by virtue of the Makefile not linking to it.
+
+#### PSA Crypto -- Makefile.psa
+
+This build configuration works for Arm PSA Crypto compatible libraries
+like the MBed Crypto Library.
+
+This integration supports SHA-256, SHA-384 and SHA-512 with ECDSA to support
+the COSE algorithms ES256, ES384 and ES512. It is a full implementation but
+needs on-target testing.
+
+To use this, edit the makefile for the location of CBOR and your
+PSA-compatible cryptographic library and do:
+
+ make -f Makefile.psa
+
+The specific things that Makefile.psa does is:
+ * Links the crypto_adapters/t_cose_psa_crypto.o into libt_cose.a
+ * Links test/test/t_cose_make_psa_test_key.o into the test binary
+ * `#define T_COSE_USE_PSA_CRYPTO`
+
+Note that the internally supplied b_con_hash is not used in this case
+by virtue of the Makefile not linking to it.
+
+Following are some notes on things discovered doing this integration.
+
+PSA Crypto is an API that Arm is standardizing. As of December 2019
+it is close to complete after some years of development. It seems
+the 1.0 version is soon to be released. The API has
+evolved over these years and some of the earlier versions are not
+compatible with the current ones. There are commercial implementations
+using earlier APIs so this variation must be handled by the crypto
+adpatation layer here. There are no official mechanisms, like a
+#define to help handle variations in these older versions.
+
+The MBed Crypto Library is an implementation of the PSA Crypto API and
+is versions separately. Presumably there are or will be implementations of
+the PSA Crypto API that are not the MBed Crypto Library.
+
+t_cose has been made to work against the released 1.1.0 version of
+MBed released in June 2019 and the 2.0.0 version released in September
+2019. Also, it works against the 1.1 version that is in TF-M which has
+different internals than the 1.1.0 version on the public GitHub.
+
+The PSA Crypto API in MBed 1.1.0 is different from that in MBed 2.0.0.
+t_cose has one configuration that covers both which hinges off a
+#define that happens to occur in 1.1.0 and not in 2.0.0. It can auto-detect
+which is which so you shouldn't have to worry about it. To overide
+the auto-detect `#define T_COSE_USE_PSA_CRYPTO_FROM_MBED_CRYPTO11`
+or `#define T_COSE_USE_PSA_CRYPTO_FROM_MBED_CRYPTO20`.
+
+Presumably, this will soon become less messy with the release of
+PSA Crypto 1.0. Presumably the older implementations like MBed
+Crypto 1.1 will stop being used. Also, PSA Crypto 1.0 has
+official #defines to manage API versions.
+
+### General Crypto Library Strategy
+
+The functions that t_cose needs from the crypto library are all
+defined in src/t_cose_crypto.h. This is a porting or adaption
+layer. There are no #ifdefs in the main t_cose code for different
+crypto libraries. When it needs a crypto function it just calls the
+interface defined in t_cose_crypto.h.
+
+When integrating t_cose with a new cryptographic library, what is
+necessary is to write some code, an "adaptor", that implements
+t_cose_crypto.h using the new target cryptographic library. This can
+be done without changes to any t_cose code for many cryptographic
+libraries. See the interface documentation in t_cose_crypto.h for what
+needs to be implemented.
+
+That said, there is one case where t_cose source code needs to be
+modified. This is for hash algorithm implementations that are linked
+into and run inline with t_cose and that have a context structure. In
+this case t_cose_crypto.h should be modified to use that context
+structure. Use the OpenSSL configuration as an example.
+
+To complete the set up for a new cryptographic library and test it, a
+new test adaptation file is also needed. This file makes public key
+pairs of the correct type for use with testing. This file is usually
+named test/t_cose_make_xxxx_test_key.c and is linked in with the test
+app. The keys it makes are passed through t_cose untouched, through
+the t_cose_crypto.h interface into the underlying crypto.
## Memory Usage
@@ -112,7 +214,7 @@
### Mixed code style
QCBOR uses camelCase and t_cose follows
-[Arm's coding guidelines](https://git.trustedfirmware.org/trusted-firmware-m.git/tree/docs/coding_guide.rst)
+[Arm's coding guidelines](https://git.trustedfirmware.org/trusted-firmware-m.git/tree/docs/about/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/lib/ext/t_cose/test/run_tests.c b/lib/ext/t_cose/test/run_tests.c
index 8f65e0e..837957e 100644
--- a/lib/ext/t_cose/test/run_tests.c
+++ b/lib/ext/t_cose/test/run_tests.c
@@ -13,6 +13,7 @@
#include "run_tests.h"
#include "UsefulBuf.h"
#include <stdbool.h>
+#include <stddef.h>
#include "t_cose_test.h"
#include "t_cose_sign_verify_test.h"
@@ -22,7 +23,7 @@
Test configuration
*/
-typedef int (test_fun_t)(void);
+typedef int_fast32_t (test_fun_t)(void);
typedef const char * (test_fun2_t)(void);
@@ -48,26 +49,38 @@
#endif
static test_entry s_tests[] = {
+ TEST_ENTRY(sign1_structure_decode_test),
+ TEST_ENTRY(crit_parameters_test),
+ TEST_ENTRY(bad_parameters_test),
+
#ifndef T_COSE_DISABLE_SIGN_VERIFY_TESTS
- /* Many tests can be run without a crypto library integration and provide
- * good test coverage of everything but the signing and verification. These
- * tests can't be run with signing and verification short circuited */
+ /* Many tests can be run without a crypto library integration and
+ * provide good test coverage of everything but the signing and
+ * verification. These tests can't be run with signing and
+ * verification short circuited. They must have a real crypto
+ * library integrated. */
TEST_ENTRY(sign_verify_basic_test),
TEST_ENTRY(sign_verify_make_cwt_test),
TEST_ENTRY(sign_verify_sig_fail_test),
TEST_ENTRY(sign_verify_get_size_test),
-#endif
- TEST_ENTRY(sign1_structure_decode_test),
+#endif /* T_COSE_DISABLE_SIGN_VERIFY_TESTS */
+
+#ifndef T_COSE_DISABLE_SHORT_CIRCUIT_SIGN
+ /* These tests can't run if short-circuit signatures are disabled.
+ * The most critical ones are replicated in the group of tests
+ * that require a real crypto library. Typically short-circuit
+ * signing is only disabled for extreme code size savings so these
+ * tests are typically always run.
+ */
TEST_ENTRY(content_type_test),
TEST_ENTRY(all_header_parameters_test),
TEST_ENTRY(cose_example_test),
- TEST_ENTRY(crit_parameters_test),
- TEST_ENTRY(bad_parameters_test),
+ TEST_ENTRY(short_circuit_signing_error_conditions_test),
+ TEST_ENTRY(short_circuit_self_test),
TEST_ENTRY(short_circuit_decode_only_test),
TEST_ENTRY(short_circuit_make_cwt_test),
- TEST_ENTRY(short_circuit_signing_error_conditions_test),
TEST_ENTRY(short_circuit_verify_fail_test),
- TEST_ENTRY(short_circuit_self_test),
+#endif /* T_COSE_DISABLE_SHORT_CIRCUIT_SIGN */
#ifdef T_COSE_ENABLE_HASH_FAIL_TEST
TEST_ENTRY(short_circuit_hash_fail_test),
@@ -106,12 +119,12 @@
}
bool bDidSomeOutput = false;
- for(int n = nMax; n > 0; n/=10) {
- int x = nNum/n;
- if(x || bDidSomeOutput){
+ for(int32_t n = nMax; n > 0; n/=10) {
+ int32_t nDigitValue = nNum/n;
+ if(nDigitValue || bDidSomeOutput){
bDidSomeOutput = true;
- UsefulOutBuf_AppendByte(&OutBuf, '0' + x);
- nNum -= x * n;
+ UsefulOutBuf_AppendByte(&OutBuf, (uint8_t)('0' + nDigitValue));
+ nNum -= nDigitValue * n;
}
}
if(!bDidSomeOutput){
@@ -131,6 +144,7 @@
void *poutCtx,
int *pNumTestsRun)
{
+ // int (-32767 to 32767 according to C standard) used by conscious choice
int nTestsFailed = 0;
int nTestsRun = 0;
UsefulBuf_MAKE_STACK_UB(StringStorage, 12);
@@ -256,7 +270,7 @@
(*pfOutput)(szWhat, pOutCtx, 0);
(*pfOutput)(" ", pOutCtx, 0);
- (*pfOutput)(NumToString(uSize, buffer), pOutCtx, 0);
+ (*pfOutput)(NumToString((int32_t)uSize, buffer), pOutCtx, 0);
(*pfOutput)("", pOutCtx, 1);
}
diff --git a/lib/ext/t_cose/test/t_cose_make_openssl_test_key.c b/lib/ext/t_cose/test/t_cose_make_openssl_test_key.c
index 8137a99..84e578c 100644
--- a/lib/ext/t_cose/test/t_cose_make_openssl_test_key.c
+++ b/lib/ext/t_cose/test/t_cose_make_openssl_test_key.c
@@ -1,7 +1,7 @@
/*
* t_cose_make_openssl_test_key.c
*
- * Copyright 2019, Laurence Lundblade
+ * Copyright 2019-2020, Laurence Lundblade
*
* SPDX-License-Identifier: BSD-3-Clause
*
@@ -197,5 +197,17 @@
}
+/*
+ * Public function, see t_cose_make_test_pub_key.h
+ */
+int check_for_key_pair_leaks()
+{
+ /* So far no good way to do this for OpenSSL or malloc() in general
+ in a nice portable way. The PSA version does check so there is
+ some coverage of the code even though there is no check here.
+ */
+ return 0;
+}
+
diff --git a/lib/ext/t_cose/test/t_cose_make_psa_test_key.c b/lib/ext/t_cose/test/t_cose_make_psa_test_key.c
index a58c6c9..245d4b7 100644
--- a/lib/ext/t_cose/test/t_cose_make_psa_test_key.c
+++ b/lib/ext/t_cose/test/t_cose_make_psa_test_key.c
@@ -1,7 +1,7 @@
/*
* t_cose_make_psa_test_key.c
*
- * Copyright 2019, Laurence Lundblade
+ * Copyright 2019-2020, Laurence Lundblade
*
* SPDX-License-Identifier: BSD-3-Clause
*
@@ -82,7 +82,7 @@
#define PSA_KEY_TYPE_ECC_KEY_PAIR PSA_KEY_TYPE_ECC_KEYPAIR
#endif /* T_COSE_USE_PSA_CRYPTO_FROM_MBED_CRYPTO11 */
- switch(cose_algorithm_id) {
+ switch(cose_algorithm_id) {
case COSE_ALGORITHM_ES256:
private_key = private_key_256;
private_key_len = sizeof(private_key_256);
@@ -109,7 +109,12 @@
}
- psa_crypto_init(); /* OK to call this multiple times */
+ /* OK to call this multiple times */
+ crypto_result = psa_crypto_init();
+ if(crypto_result != PSA_SUCCESS) {
+ return T_COSE_ERR_FAIL;
+ }
+
/* When importing a key with the PSA API there are two main
* things to do.
@@ -196,6 +201,30 @@
*/
void free_ecdsa_key_pair(struct t_cose_key key_pair)
{
- psa_close_key(key_pair.k.key_handle);
+ psa_close_key((psa_key_handle_t)key_pair.k.key_handle);
+}
+
+
+/*
+ * Public function, see t_cose_make_test_pub_key.h
+ */
+int check_for_key_pair_leaks()
+{
+#if defined(T_COSE_USE_PSA_CRYPTO_FROM_MBED_CRYPTO11)
+ /* No way to check for leaks with MBED Crypto 1.1 */
+ return 0;
+
+#else
+ mbedtls_psa_stats_t stats;
+
+ mbedtls_psa_get_stats(&stats);
+
+ return (int)(stats.volatile_slots +
+ stats.persistent_slots +
+ stats.external_slots +
+ stats.half_filled_slots +
+ stats.cache_slots);
+
+#endif /* T_COSE_USE_PSA_CRYPTO_FROM_MBED_CRYPTO11 */
}
diff --git a/lib/ext/t_cose/test/t_cose_make_test_messages.c b/lib/ext/t_cose/test/t_cose_make_test_messages.c
index 9d61ade..9293b63 100644
--- a/lib/ext/t_cose/test/t_cose_make_test_messages.c
+++ b/lib/ext/t_cose/test/t_cose_make_test_messages.c
@@ -120,7 +120,7 @@
* \c T_COSE_SIGN1_MAX_SIZE_PROTECTED_PARAMETERS.
*/
static inline struct q_useful_buf_c
-encode_protected_parameters(int32_t test_message_options,
+encode_protected_parameters(uint32_t test_message_options,
int32_t cose_algorithm_id,
struct q_useful_buf buffer_for_protected_parameters)
{
@@ -267,7 +267,7 @@
* lots of different test parameters.
*/
static inline void
-add_unprotected_parameters(int32_t test_message_options,
+add_unprotected_parameters(uint32_t test_message_options,
QCBOREncodeContext *cbor_encode_ctx,
struct q_useful_buf_c kid)
{
@@ -385,7 +385,7 @@
*/
static enum t_cose_err_t
t_cose_sign1_test_message_encode_parameters(struct t_cose_sign1_sign_ctx *me,
- int32_t test_mess_options,
+ uint32_t test_mess_options,
QCBOREncodeContext *cbor_encode_ctx)
{
enum t_cose_err_t return_value;
@@ -565,7 +565,7 @@
*/
enum t_cose_err_t
t_cose_test_message_sign1_sign(struct t_cose_sign1_sign_ctx *me,
- int32_t test_message_options,
+ uint32_t test_message_options,
struct q_useful_buf_c payload,
struct q_useful_buf out_buf,
struct q_useful_buf_c *result)
diff --git a/lib/ext/t_cose/test/t_cose_make_test_messages.h b/lib/ext/t_cose/test/t_cose_make_test_messages.h
index 3dde9fd..d1babce 100644
--- a/lib/ext/t_cose/test/t_cose_make_test_messages.h
+++ b/lib/ext/t_cose/test/t_cose_make_test_messages.h
@@ -39,7 +39,7 @@
/** Make test message with a bstr label, which is not allowed by
* COSE */
-#define T_COSE_TEST_PARAMETER_LABEL 0x80000000
+#define T_COSE_TEST_PARAMETER_LABEL 0x80000000U
/** Format of the crit parameter is made invalid */
#define T_COSE_TEST_BAD_CRIT_PARAMETER 0x40000000
@@ -135,7 +135,7 @@
*/
enum t_cose_err_t
t_cose_test_message_sign1_sign(struct t_cose_sign1_sign_ctx *me,
- int32_t test_message_options,
+ uint32_t test_message_options,
struct q_useful_buf_c payload,
struct q_useful_buf out_buf,
struct q_useful_buf_c *result);
diff --git a/lib/ext/t_cose/test/t_cose_make_test_pub_key.h b/lib/ext/t_cose/test/t_cose_make_test_pub_key.h
index 585c0b2..62f217a 100644
--- a/lib/ext/t_cose/test/t_cose_make_test_pub_key.h
+++ b/lib/ext/t_cose/test/t_cose_make_test_pub_key.h
@@ -1,7 +1,7 @@
/*
* t_cose_make_test_pub_key.h
*
- * Copyright 2019, Laurence Lundblade
+ * Copyright 2019-2020, Laurence Lundblade
*
* SPDX-License-Identifier: BSD-3-Clause
*
@@ -30,3 +30,12 @@
void free_ecdsa_key_pair(struct t_cose_key key_pair);
+/**
+ \brief Called by test frame work to see if there were key pair or mem leaks.
+
+ \return 0 if no leaks, non-zero if there is a leak.
+ */
+int check_for_key_pair_leaks(void);
+
+
+
diff --git a/lib/ext/t_cose/test/t_cose_sign_verify_test.c b/lib/ext/t_cose/test/t_cose_sign_verify_test.c
index 8dbefb7..cf1f9fd 100644
--- a/lib/ext/t_cose/test/t_cose_sign_verify_test.c
+++ b/lib/ext/t_cose/test/t_cose_sign_verify_test.c
@@ -22,7 +22,7 @@
int_fast32_t sign_verify_basic_test_alg(int32_t cose_alg)
{
struct t_cose_sign1_sign_ctx sign_ctx;
- enum t_cose_err_t return_value;
+ int32_t return_value;
Q_USEFUL_BUF_MAKE_STACK_UB( signed_cose_buffer, 300);
struct q_useful_buf_c signed_cose;
struct t_cose_key key_pair;
@@ -41,12 +41,13 @@
}
t_cose_sign1_set_signing_key(&sign_ctx, key_pair, NULL_Q_USEFUL_BUF_C);
- t_cose_sign1_sign(&sign_ctx,
+ return_value = t_cose_sign1_sign(&sign_ctx,
Q_USEFUL_BUF_FROM_SZ_LITERAL("payload"),
signed_cose_buffer,
&signed_cose);
if(return_value) {
- return 2000 + return_value;
+ return_value += 2000;
+ goto Done;
}
/* Verification */
@@ -59,19 +60,22 @@
&payload, /* Payload from signed_cose */
NULL); /* Don't return parameters */
if(return_value) {
- return 5000 + return_value;
+ return_value += 5000;
+ goto Done;
}
- /* OpenSSL uses malloc to allocate buffers for keys, so they have to
- * be freed */
- free_ecdsa_key_pair(key_pair);
/* compare payload output to the one expected */
if(q_useful_buf_compare(payload, Q_USEFUL_BUF_FROM_SZ_LITERAL("payload"))) {
- return 6000;
+ return_value = 6000;
+ goto Done;
}
- return 0;
+Done:
+ /* Many crypto libraries allocate memory, slots, etc for keys */
+ free_ecdsa_key_pair(key_pair);
+
+ return return_value;
}
@@ -82,10 +86,10 @@
{
int_fast32_t return_value;
- return_value = sign_verify_basic_test_alg(T_COSE_ALGORITHM_ES256);
- if(return_value) {
+ return_value = sign_verify_basic_test_alg(T_COSE_ALGORITHM_ES256);
+ if(return_value) {
return 20000 + return_value;
- }
+ }
#ifndef T_COSE_DISABLE_ES384
return_value = sign_verify_basic_test_alg(T_COSE_ALGORITHM_ES384);
@@ -113,7 +117,7 @@
{
struct t_cose_sign1_sign_ctx sign_ctx;
QCBOREncodeContext cbor_encode;
- enum t_cose_err_t return_value;
+ int32_t return_value;
Q_USEFUL_BUF_MAKE_STACK_UB( signed_cose_buffer, 300);
struct q_useful_buf_c signed_cose;
struct t_cose_key key_pair;
@@ -138,7 +142,8 @@
return_value = t_cose_sign1_encode_parameters(&sign_ctx, &cbor_encode);
if(return_value) {
- return 2000 + return_value;
+ return_value += 2000;
+ goto Done;
}
QCBOREncode_AddSZString(&cbor_encode, "payload");
@@ -146,18 +151,21 @@
return_value = t_cose_sign1_encode_signature(&sign_ctx, &cbor_encode);
if(return_value) {
- return 3000 + return_value;
+ return_value += 3000;
+ goto Done;
}
cbor_error = QCBOREncode_Finish(&cbor_encode, &signed_cose);
if(cbor_error) {
- return 4000 + cbor_error;
+ return_value = 4000 + cbor_error;
+ goto Done;
}
/* tamper with the pay load to see that the signature verification fails */
tamper_offset = q_useful_buf_find_bytes(signed_cose, Q_USEFUL_BUF_FROM_SZ_LITERAL("payload"));
if(tamper_offset == SIZE_MAX) {
- return 99;
+ return_value = 99;
+ goto Done;
}
((char *)signed_cose.ptr)[tamper_offset] = 'h';
@@ -172,12 +180,15 @@
NULL); /* Don't return parameters */
if(return_value != T_COSE_ERR_SIG_VERIFY) {
- return 5000 + return_value;
+ return_value += 5000;
}
+ return_value = 0; /* This was supposed to fail, so it needs to be reset */
+
+Done:
free_ecdsa_key_pair(key_pair);
- return 0;
+ return return_value;
}
@@ -188,7 +199,7 @@
{
struct t_cose_sign1_sign_ctx sign_ctx;
QCBOREncodeContext cbor_encode;
- enum t_cose_err_t return_value;
+ int32_t return_value;
Q_USEFUL_BUF_MAKE_STACK_UB( signed_cose_buffer, 300);
struct q_useful_buf_c signed_cose;
struct t_cose_key key_pair;
@@ -222,7 +233,8 @@
QCBOREncode_Init(&cbor_encode, signed_cose_buffer);
return_value = t_cose_sign1_encode_parameters(&sign_ctx, &cbor_encode);
if(return_value) {
- return 2000 + return_value;
+ return_value += 2000;
+ goto Done;
}
@@ -243,7 +255,8 @@
/* -- Finish up the COSE_Sign1. This is where the signing happens -- */
return_value = t_cose_sign1_encode_signature(&sign_ctx, &cbor_encode);
if(return_value) {
- return 2000 + return_value;
+ return_value += 3000;
+ goto Done;
}
/* Finally close off the CBOR formatting and get the pointer and length
@@ -251,7 +264,8 @@
*/
cbor_error = QCBOREncode_Finish(&cbor_encode, &signed_cose);
if(cbor_error) {
- return 3000 + cbor_error;
+ return_value = cbor_error + 4000;
+ goto Done;
}
/* --- Done making COSE Sign1 object --- */
@@ -272,7 +286,8 @@
expected_rfc8392_first_part = Q_USEFUL_BUF_FROM_BYTE_ARRAY_LITERAL(rfc8392_first_part_bytes);
actual_rfc8392_first_part = q_useful_buf_head(signed_cose, sizeof(rfc8392_first_part_bytes));
if(q_useful_buf_compare(actual_rfc8392_first_part, expected_rfc8392_first_part)) {
- return -1;
+ return_value = -1;
+ goto Done;
}
/* --- Start verifying the COSE Sign1 object --- */
@@ -287,7 +302,8 @@
NULL); /* Don't return parameters */
if(return_value) {
- return 4000 + return_value;
+ return_value += 5000;
+ goto Done;
}
/* Format the expected payload CBOR fragment */
@@ -303,13 +319,15 @@
/* compare payload output to the one expected */
expected_payload = q_useful_buf_tail(expected_rfc8392_first_part, kid_encoded_len + 8);
if(q_useful_buf_compare(payload, expected_payload)) {
- return 5000;
+ return_value = 6000;
}
/* --- Done verifying the COSE Sign1 object --- */
+Done:
+ /* Many crypto libraries allocate memory, slots, etc for keys */
free_ecdsa_key_pair(key_pair);
- return 0;
+ return return_value;
}
@@ -425,47 +443,51 @@
}
result = size_test(T_COSE_ALGORITHM_ES256, NULL_Q_USEFUL_BUF_C, key_pair);
+ free_ecdsa_key_pair(key_pair);
if(result) {
- return result;
+ return 2000 + result;
}
- free_ecdsa_key_pair(key_pair);
#ifndef T_COSE_DISABLE_ES384
+
return_value = make_ecdsa_key_pair(T_COSE_ALGORITHM_ES384, &key_pair);
if(return_value) {
- return 1000 + return_value;
+ return 3000 + return_value;
}
result = size_test(T_COSE_ALGORITHM_ES384, NULL_Q_USEFUL_BUF_C, key_pair);
+ free_ecdsa_key_pair(key_pair);
if(result) {
- return result;
+ return 4000 + result;
}
- free_ecdsa_key_pair(key_pair);
-#endif
+#endif /* T_COSE_DISABLE_ES384 */
+
#ifndef T_COSE_DISABLE_ES512
+
return_value = make_ecdsa_key_pair(T_COSE_ALGORITHM_ES512, &key_pair);
if(return_value) {
- return 1000 + return_value;
+ return 5000 + return_value;
}
result = size_test(T_COSE_ALGORITHM_ES512, NULL_Q_USEFUL_BUF_C, key_pair);
if(result) {
- return result;
+ free_ecdsa_key_pair(key_pair);
+ return 6000 + result;
}
-
result = size_test(T_COSE_ALGORITHM_ES512,
Q_USEFUL_BUF_FROM_SZ_LITERAL("greasy kid stuff"),
key_pair);
+ free_ecdsa_key_pair(key_pair);
if(result) {
- return result;
+ return 7000 + result;
}
- free_ecdsa_key_pair(key_pair);
-#endif
+#endif /* T_COSE_DISABLE_ES512 */
+
return 0;
}
diff --git a/lib/ext/t_cose/test/t_cose_test.c b/lib/ext/t_cose/test/t_cose_test.c
index 30db83d..eae4760 100644
--- a/lib/ext/t_cose/test/t_cose_test.c
+++ b/lib/ext/t_cose/test/t_cose_test.c
@@ -14,6 +14,7 @@
#include "t_cose_make_test_messages.h"
#include "q_useful_buf.h"
#include "t_cose_crypto.h" /* For signature size constant */
+#include "t_cose_util.h" /* for get_short_circuit_kid */
/*
@@ -470,7 +471,7 @@
/*
* Public function, see t_cose_test.h
*/
-int cose_example_test()
+int_fast32_t cose_example_test()
{
enum t_cose_err_t return_value;
Q_USEFUL_BUF_MAKE_STACK_UB( signed_cose_buffer, 200);
@@ -512,7 +513,7 @@
}
-static enum t_cose_err_t run_test_sign_and_verify(int32_t test_mess_options)
+static enum t_cose_err_t run_test_sign_and_verify(uint32_t test_mess_options)
{
struct t_cose_sign1_sign_ctx sign_ctx;
struct t_cose_sign1_verify_ctx verify_ctx;
@@ -562,8 +563,12 @@
}
-/* copied from t_cose_util.c */
-#ifndef T_COSE_DISABLE_SHORT_CIRCUIT_SIGN
+
+#ifdef T_COSE_DISABLE_SHORT_CIRCUIT_SIGN
+/* copied from t_cose_util.c so these tests that depend on
+ * short circuit signatures can run even when it is
+ * is disabled. TODO: is this dependency real?*/
+
/* This is a random hard coded key ID that is used to indicate
* short-circuit signing. It is OK to hard code this as the
* probability of collision with this ID is very low and the same
@@ -589,7 +594,7 @@
return ss_kid;
}
-#endif
+#endif /* T_COSE_DISABLE_SHORT_CIRCUIT_SIGN */
int_fast32_t all_header_parameters_test()
{
@@ -633,12 +638,10 @@
/* Get parameters for checking */
¶meters);
-#ifndef T_COSE_DISABLE_SHORT_CIRCUIT_SIGN
// Need to compare to short circuit kid
if(q_useful_buf_compare(parameters.kid, get_short_circuit_kid())) {
return 2;
}
-#endif
if(parameters.cose_algorithm_id != T_COSE_ALGORITHM_ES256) {
return 3;
@@ -648,7 +651,7 @@
if(parameters.content_type_uint != 1) {
return 4;
}
-#endif
+#endif /* T_COSE_DISABLE_CONTENT_TYPE */
if(q_useful_buf_compare(parameters.iv, Q_USEFUL_BUF_FROM_SZ_LITERAL("iv"))) {
return 5;
@@ -662,8 +665,8 @@
}
struct test_case {
- int32_t test_option;
- int result;
+ uint32_t test_option;
+ enum t_cose_err_t result;
};
static struct test_case bad_parameters_tests_table[] = {
@@ -716,7 +719,7 @@
for(test = bad_parameters_tests_table; test->test_option; test++) {
if(run_test_sign_and_verify(test->test_option) != test->result) {
- return (int)(test - bad_parameters_tests_table);
+ return (int_fast32_t)(test - bad_parameters_tests_table);
}
}
@@ -770,7 +773,7 @@
for(test = crit_tests_table; test->test_option; test++) {
if(run_test_sign_and_verify(test->test_option) != test->result) {
- return (int)(test - crit_tests_table);
+ return (int_fast32_t)(test - crit_tests_table);
}
}
@@ -927,8 +930,9 @@
&payload,
NULL);
if(result != sample->expected_error) {
- /* Returns 100 * index of the input + error code not expected */
- return (int32_t)(sample - sign1_sample_inputs+1)*100 + result;
+ /* Returns 100 * index of the input + the unexpected error code */
+ const size_t sample_index = (size_t)(sample - sign1_sample_inputs) / sizeof(struct sign1_sample);
+ return (int32_t)((sample_index+1)*100 + result);
}
}
diff --git a/lib/ext/t_cose/test/t_cose_test.h b/lib/ext/t_cose/test/t_cose_test.h
index 4802b5d..3177255 100644
--- a/lib/ext/t_cose/test/t_cose_test.h
+++ b/lib/ext/t_cose/test/t_cose_test.h
@@ -1,7 +1,7 @@
/*
* t_cose_test.h
*
- * Copyright 2019, Laurence Lundblade
+ * Copyright 2019-2020, Laurence Lundblade
*
* SPDX-License-Identifier: BSD-3-Clause
*
@@ -40,7 +40,7 @@
*
* \return non-zero on failure.
*
- * This test makes a simple COSE_Sign1 modify the payload and see that
+ * This test makes a simple COSE_Sign1 modifies the payload and sees that
* verification fails. It uses short-circuit signatures so no keys or
* even integration with public key crypto is necessary.
*/
@@ -64,7 +64,9 @@
/*
- * Test the decode only mode.
+ * Test the decode only mode, the mode where the
+ * headers are returned, but the signature is no
+ * verified.
*/
int_fast32_t short_circuit_decode_only_test(void);