From f8db5b6f72f06b543cad2dc7b67012c222281c52 Mon Sep 17 00:00:00 2001 From: Sam Berry Date: Fri, 19 Jul 2024 14:48:40 +0100 Subject: [PATCH 01/10] Move the function declarations to x509.h This commit moves the function declarations for mbedtls_oid_get_numeric_string and mbedtls_oid_from_numeric_string from oid.h to x509.h. Signed-off-by: Sam Berry --- include/mbedtls/x509.h | 32 +++++++++++++++++++ .../drivers/builtin/include/mbedtls/oid.h | 32 ------------------- 2 files changed, 32 insertions(+), 32 deletions(-) diff --git a/include/mbedtls/x509.h b/include/mbedtls/x509.h index 453f598c7..18df19ce6 100644 --- a/include/mbedtls/x509.h +++ b/include/mbedtls/x509.h @@ -492,6 +492,38 @@ size_t mbedtls_x509_crt_parse_cn_inet_pton(const char *cn, void *dst); p += (size_t) ret; \ } while (0) +/** + * \brief Translate an ASN.1 OID into its numeric representation + * (e.g. "\x2A\x86\x48\x86\xF7\x0D" into "1.2.840.113549") + * + * \param buf buffer to put representation in + * \param size size of the buffer + * \param oid OID to translate + * + * \return Length of the string written (excluding final NULL) or + * MBEDTLS_ERR_OID_BUF_TOO_SMALL in case of error + */ +int mbedtls_oid_get_numeric_string(char *buf, size_t size, const mbedtls_asn1_buf *oid); + +/** + * \brief Translate a string containing a dotted-decimal + * representation of an ASN.1 OID into its encoded form + * (e.g. "1.2.840.113549" into "\x2A\x86\x48\x86\xF7\x0D"). + * On success, this function allocates oid->buf from the + * heap. It must be freed by the caller using mbedtls_free(). + * + * \param oid #mbedtls_asn1_buf to populate with the DER-encoded OID + * \param oid_str string representation of the OID to parse + * \param size length of the OID string, not including any null terminator + * + * \return 0 if successful + * \return #MBEDTLS_ERR_ASN1_INVALID_DATA if \p oid_str does not + * represent a valid OID + * \return #MBEDTLS_ERR_ASN1_ALLOC_FAILED if the function fails to + * allocate oid->buf + */ +int mbedtls_oid_from_numeric_string(mbedtls_asn1_buf *oid, const char *oid_str, size_t size); + #ifdef __cplusplus } #endif diff --git a/tf-psa-crypto/drivers/builtin/include/mbedtls/oid.h b/tf-psa-crypto/drivers/builtin/include/mbedtls/oid.h index 03669443c..e0ad35e31 100644 --- a/tf-psa-crypto/drivers/builtin/include/mbedtls/oid.h +++ b/tf-psa-crypto/drivers/builtin/include/mbedtls/oid.h @@ -482,38 +482,6 @@ typedef struct mbedtls_oid_descriptor_t { #endif } mbedtls_oid_descriptor_t; -/** - * \brief Translate an ASN.1 OID into its numeric representation - * (e.g. "\x2A\x86\x48\x86\xF7\x0D" into "1.2.840.113549") - * - * \param buf buffer to put representation in - * \param size size of the buffer - * \param oid OID to translate - * - * \return Length of the string written (excluding final NULL) or - * MBEDTLS_ERR_OID_BUF_TOO_SMALL in case of error - */ -int mbedtls_oid_get_numeric_string(char *buf, size_t size, const mbedtls_asn1_buf *oid); - -/** - * \brief Translate a string containing a dotted-decimal - * representation of an ASN.1 OID into its encoded form - * (e.g. "1.2.840.113549" into "\x2A\x86\x48\x86\xF7\x0D"). - * On success, this function allocates oid->buf from the - * heap. It must be freed by the caller using mbedtls_free(). - * - * \param oid #mbedtls_asn1_buf to populate with the DER-encoded OID - * \param oid_str string representation of the OID to parse - * \param size length of the OID string, not including any null terminator - * - * \return 0 if successful - * \return #MBEDTLS_ERR_ASN1_INVALID_DATA if \p oid_str does not - * represent a valid OID - * \return #MBEDTLS_ERR_ASN1_ALLOC_FAILED if the function fails to - * allocate oid->buf - */ -int mbedtls_oid_from_numeric_string(mbedtls_asn1_buf *oid, const char *oid_str, size_t size); - /** * \brief Translate an X.509 extension OID into local values * From 4f76194eafbb4cb1026c520a6afcb00171723cd9 Mon Sep 17 00:00:00 2001 From: Sam Berry Date: Fri, 19 Jul 2024 14:56:30 +0100 Subject: [PATCH 02/10] Move function mbedtls_oid_get_numeric_string to x509.c This commit moves the mbedtls_oid_get_numeric_string function definition from oid.c to x509.c. Signed-off-by: Sam Berry --- library/x509.c | 69 +++++++++++++++++++++++++ tf-psa-crypto/drivers/builtin/src/oid.c | 69 ------------------------- 2 files changed, 69 insertions(+), 69 deletions(-) diff --git a/library/x509.c b/library/x509.c index a80ab53fc..be7b277bb 100644 --- a/library/x509.c +++ b/library/x509.c @@ -805,6 +805,75 @@ static char nibble_to_hex_digit(int i) return (i < 10) ? (i + '0') : (i - 10 + 'A'); } +/* Return the x.y.z.... style numeric string for the given OID */ +int mbedtls_oid_get_numeric_string(char *buf, size_t size, + const mbedtls_asn1_buf *oid) +{ + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + char *p = buf; + size_t n = size; + unsigned int value = 0; + + if (size > INT_MAX) { + /* Avoid overflow computing return value */ + return MBEDTLS_ERR_ASN1_INVALID_LENGTH; + } + + if (oid->len <= 0) { + /* OID must not be empty */ + return MBEDTLS_ERR_ASN1_OUT_OF_DATA; + } + + for (size_t i = 0; i < oid->len; i++) { + /* Prevent overflow in value. */ + if (value > (UINT_MAX >> 7)) { + return MBEDTLS_ERR_ASN1_INVALID_DATA; + } + if ((value == 0) && ((oid->p[i]) == 0x80)) { + /* Overlong encoding is not allowed */ + return MBEDTLS_ERR_ASN1_INVALID_DATA; + } + + value <<= 7; + value |= oid->p[i] & 0x7F; + + if (!(oid->p[i] & 0x80)) { + /* Last byte */ + if (n == size) { + int component1; + unsigned int component2; + /* First subidentifier contains first two OID components */ + if (value >= 80) { + component1 = '2'; + component2 = value - 80; + } else if (value >= 40) { + component1 = '1'; + component2 = value - 40; + } else { + component1 = '0'; + component2 = value; + } + ret = mbedtls_snprintf(p, n, "%c.%u", component1, component2); + } else { + ret = mbedtls_snprintf(p, n, ".%u", value); + } + if (ret < 2 || (size_t) ret >= n) { + return MBEDTLS_ERR_OID_BUF_TOO_SMALL; + } + n -= (size_t) ret; + p += ret; + value = 0; + } + } + + if (value != 0) { + /* Unterminated subidentifier */ + return MBEDTLS_ERR_ASN1_OUT_OF_DATA; + } + + return (int) (size - n); +} + /* * Store the name in printable form into buf; no more * than size characters will be written diff --git a/tf-psa-crypto/drivers/builtin/src/oid.c b/tf-psa-crypto/drivers/builtin/src/oid.c index 0e7896c60..565fac606 100644 --- a/tf-psa-crypto/drivers/builtin/src/oid.c +++ b/tf-psa-crypto/drivers/builtin/src/oid.c @@ -918,75 +918,6 @@ FN_OID_GET_ATTR2(mbedtls_oid_get_pkcs12_pbe_alg, cipher_alg) #endif /* MBEDTLS_PKCS12_C && MBEDTLS_CIPHER_C */ -/* Return the x.y.z.... style numeric string for the given OID */ -int mbedtls_oid_get_numeric_string(char *buf, size_t size, - const mbedtls_asn1_buf *oid) -{ - int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - char *p = buf; - size_t n = size; - unsigned int value = 0; - - if (size > INT_MAX) { - /* Avoid overflow computing return value */ - return MBEDTLS_ERR_ASN1_INVALID_LENGTH; - } - - if (oid->len <= 0) { - /* OID must not be empty */ - return MBEDTLS_ERR_ASN1_OUT_OF_DATA; - } - - for (size_t i = 0; i < oid->len; i++) { - /* Prevent overflow in value. */ - if (value > (UINT_MAX >> 7)) { - return MBEDTLS_ERR_ASN1_INVALID_DATA; - } - if ((value == 0) && ((oid->p[i]) == 0x80)) { - /* Overlong encoding is not allowed */ - return MBEDTLS_ERR_ASN1_INVALID_DATA; - } - - value <<= 7; - value |= oid->p[i] & 0x7F; - - if (!(oid->p[i] & 0x80)) { - /* Last byte */ - if (n == size) { - int component1; - unsigned int component2; - /* First subidentifier contains first two OID components */ - if (value >= 80) { - component1 = '2'; - component2 = value - 80; - } else if (value >= 40) { - component1 = '1'; - component2 = value - 40; - } else { - component1 = '0'; - component2 = value; - } - ret = mbedtls_snprintf(p, n, "%c.%u", component1, component2); - } else { - ret = mbedtls_snprintf(p, n, ".%u", value); - } - if (ret < 2 || (size_t) ret >= n) { - return MBEDTLS_ERR_OID_BUF_TOO_SMALL; - } - n -= (size_t) ret; - p += ret; - value = 0; - } - } - - if (value != 0) { - /* Unterminated subidentifier */ - return MBEDTLS_ERR_ASN1_OUT_OF_DATA; - } - - return (int) (size - n); -} - static int oid_parse_number(unsigned int *num, const char **p, const char *bound) { int ret = MBEDTLS_ERR_ASN1_INVALID_DATA; From 4aee6a25ca425191e900b04773d85630bc7c8b89 Mon Sep 17 00:00:00 2001 From: Sam Berry Date: Fri, 19 Jul 2024 15:00:41 +0100 Subject: [PATCH 03/10] Move mbedtls_oid_get_numeric_string unit tests to test_suite_x509parse This commit moves all related mbedtls_oid_get_numeric_string unit tests from test_suite_oid to test_suite_x509parse. Signed-off-by: Sam Berry --- tests/suites/test_suite_x509parse.data | 49 +++++++++++++++++++ tests/suites/test_suite_x509parse.function | 26 ++++++++++ .../tests/suites/test_suite_oid.data | 49 ------------------- .../tests/suites/test_suite_oid.function | 24 --------- 4 files changed, 75 insertions(+), 73 deletions(-) diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data index 459738a5c..ebfc23f5a 100644 --- a/tests/suites/test_suite_x509parse.data +++ b/tests/suites/test_suite_x509parse.data @@ -3439,3 +3439,52 @@ x509_crt_parse_authoritykeyid:"../framework/data_files/authorityKeyId_subjectKey X509 CRT parse Authority Key Id - Wrong Issuer sequence depends_on:PSA_WANT_ALG_MD5:MBEDTLS_RSA_C x509_crt_parse_authoritykeyid:"../framework/data_files/clusterfuzz-testcase-minimized-fuzz_x509crt-6666050834661376.crt.der":"":"":"":MBEDTLS_ERR_X509_INVALID_EXTENSIONS+MBEDTLS_ERR_ASN1_OUT_OF_DATA + +OID get numeric string - hardware module name +oid_get_numeric_string:"2B06010505070804":0:"1.3.6.1.5.5.7.8.4" + +OID get numeric string - multi-byte subidentifier +oid_get_numeric_string:"29903C":0:"1.1.2108" + +OID get numeric string - second component greater than 39 +oid_get_numeric_string:"81010000863A00":0:"2.49.0.0.826.0" + +OID get numeric string - multi-byte first subidentifier +oid_get_numeric_string:"8837":0:"2.999" + +OID get numeric string - second subidentifier not terminated +oid_get_numeric_string:"0081":MBEDTLS_ERR_ASN1_OUT_OF_DATA:"" + +OID get numeric string - empty oid buffer +oid_get_numeric_string:"":MBEDTLS_ERR_ASN1_OUT_OF_DATA:"" + +OID get numeric string - no final / all bytes have top bit set +oid_get_numeric_string:"818181":MBEDTLS_ERR_ASN1_OUT_OF_DATA:"" + +OID get numeric string - 0.39 +oid_get_numeric_string:"27":0:"0.39" + +OID get numeric string - 1.0 +oid_get_numeric_string:"28":0:"1.0" + +OID get numeric string - 1.39 +oid_get_numeric_string:"4f":0:"1.39" + +OID get numeric string - 2.0 +oid_get_numeric_string:"50":0:"2.0" + +OID get numeric string - 1 byte first subidentifier beyond 2.39 +oid_get_numeric_string:"7f":0:"2.47" + +# Encodes the number 0x0400000000 as a subidentifier which overflows 32-bits +OID get numeric string - 32-bit overflow +oid_get_numeric_string:"C080808000":MBEDTLS_ERR_ASN1_INVALID_DATA:"" + +OID get numeric string - 32-bit overflow, second subidentifier +oid_get_numeric_string:"2BC080808000":MBEDTLS_ERR_ASN1_INVALID_DATA:"" + +OID get numeric string - overlong encoding +oid_get_numeric_string:"8001":MBEDTLS_ERR_ASN1_INVALID_DATA:"" + +OID get numeric string - overlong encoding, second subidentifier +oid_get_numeric_string:"2B8001":MBEDTLS_ERR_ASN1_INVALID_DATA:"" diff --git a/tests/suites/test_suite_x509parse.function b/tests/suites/test_suite_x509parse.function index 9fc0e55df..66a7c8df9 100644 --- a/tests/suites/test_suite_x509parse.function +++ b/tests/suites/test_suite_x509parse.function @@ -10,6 +10,8 @@ #include "mbedtls/base64.h" #include "mbedtls/error.h" #include "mbedtls/pk.h" +#include "mbedtls/asn1.h" +#include "mbedtls/asn1write.h" #include "string.h" #if MBEDTLS_X509_MAX_INTERMEDIATE_CA > 19 @@ -1747,3 +1749,27 @@ exit: mbedtls_x509_crt_free(&crt); } /* END_CASE */ + +/* BEGIN_CASE depends_on:MBEDTLS_x509_USE_C */ +void oid_get_numeric_string(data_t *oid, int error_ret, char *result_str) +{ + char buf[256]; + mbedtls_asn1_buf input_oid = { 0, 0, NULL }; + int ret; + + input_oid.tag = MBEDTLS_ASN1_OID; + /* Test that an empty OID is not dereferenced */ + input_oid.p = oid->len ? oid->x : (void *) 1; + input_oid.len = oid->len; + + ret = mbedtls_oid_get_numeric_string(buf, sizeof(buf), &input_oid); + + if (error_ret == 0) { + TEST_EQUAL(ret, strlen(result_str)); + TEST_ASSERT(ret >= 3); + TEST_EQUAL(strcmp(buf, result_str), 0); + } else { + TEST_EQUAL(ret, error_ret); + } +} +/* END_CASE */ diff --git a/tf-psa-crypto/tests/suites/test_suite_oid.data b/tf-psa-crypto/tests/suites/test_suite_oid.data index 8919d4209..3521c97e8 100644 --- a/tf-psa-crypto/tests/suites/test_suite_oid.data +++ b/tf-psa-crypto/tests/suites/test_suite_oid.data @@ -105,55 +105,6 @@ oid_get_md_alg_id:"2b24030201":MBEDTLS_MD_RIPEMD160 OID hash id - invalid oid oid_get_md_alg_id:"2B864886f70d0204":-1 -OID get numeric string - hardware module name -oid_get_numeric_string:"2B06010505070804":0:"1.3.6.1.5.5.7.8.4" - -OID get numeric string - multi-byte subidentifier -oid_get_numeric_string:"29903C":0:"1.1.2108" - -OID get numeric string - second component greater than 39 -oid_get_numeric_string:"81010000863A00":0:"2.49.0.0.826.0" - -OID get numeric string - multi-byte first subidentifier -oid_get_numeric_string:"8837":0:"2.999" - -OID get numeric string - second subidentifier not terminated -oid_get_numeric_string:"0081":MBEDTLS_ERR_ASN1_OUT_OF_DATA:"" - -OID get numeric string - empty oid buffer -oid_get_numeric_string:"":MBEDTLS_ERR_ASN1_OUT_OF_DATA:"" - -OID get numeric string - no final / all bytes have top bit set -oid_get_numeric_string:"818181":MBEDTLS_ERR_ASN1_OUT_OF_DATA:"" - -OID get numeric string - 0.39 -oid_get_numeric_string:"27":0:"0.39" - -OID get numeric string - 1.0 -oid_get_numeric_string:"28":0:"1.0" - -OID get numeric string - 1.39 -oid_get_numeric_string:"4f":0:"1.39" - -OID get numeric string - 2.0 -oid_get_numeric_string:"50":0:"2.0" - -OID get numeric string - 1 byte first subidentifier beyond 2.39 -oid_get_numeric_string:"7f":0:"2.47" - -# Encodes the number 0x0400000000 as a subidentifier which overflows 32-bits -OID get numeric string - 32-bit overflow -oid_get_numeric_string:"C080808000":MBEDTLS_ERR_ASN1_INVALID_DATA:"" - -OID get numeric string - 32-bit overflow, second subidentifier -oid_get_numeric_string:"2BC080808000":MBEDTLS_ERR_ASN1_INVALID_DATA:"" - -OID get numeric string - overlong encoding -oid_get_numeric_string:"8001":MBEDTLS_ERR_ASN1_INVALID_DATA:"" - -OID get numeric string - overlong encoding, second subidentifier -oid_get_numeric_string:"2B8001":MBEDTLS_ERR_ASN1_INVALID_DATA:"" - OID from numeric string - hardware module name oid_from_numeric_string:"1.3.6.1.5.5.7.8.4":0:"2B06010505070804" diff --git a/tf-psa-crypto/tests/suites/test_suite_oid.function b/tf-psa-crypto/tests/suites/test_suite_oid.function index 337f84310..5cfa35b64 100644 --- a/tf-psa-crypto/tests/suites/test_suite_oid.function +++ b/tf-psa-crypto/tests/suites/test_suite_oid.function @@ -119,30 +119,6 @@ void mbedtls_oid_get_md_hmac(data_t *oid, int exp_md_id) } /* END_CASE */ -/* BEGIN_CASE */ -void oid_get_numeric_string(data_t *oid, int error_ret, char *result_str) -{ - char buf[256]; - mbedtls_asn1_buf input_oid = { 0, 0, NULL }; - int ret; - - input_oid.tag = MBEDTLS_ASN1_OID; - /* Test that an empty OID is not dereferenced */ - input_oid.p = oid->len ? oid->x : (void *) 1; - input_oid.len = oid->len; - - ret = mbedtls_oid_get_numeric_string(buf, sizeof(buf), &input_oid); - - if (error_ret == 0) { - TEST_EQUAL(ret, strlen(result_str)); - TEST_ASSERT(ret >= 3); - TEST_EQUAL(strcmp(buf, result_str), 0); - } else { - TEST_EQUAL(ret, error_ret); - } -} -/* END_CASE */ - /* BEGIN_CASE */ void oid_from_numeric_string(char *oid_str, int error_ret, data_t *exp_oid_buf) From c71abc3fd3ec0670d79405550e174349db373f93 Mon Sep 17 00:00:00 2001 From: Sam Berry Date: Fri, 19 Jul 2024 15:11:10 +0100 Subject: [PATCH 04/10] Move mbedtls_oid_from_numeric_string to x509_create.c This commit moves the mbedtls_oid_from_numeric_string function definition from oid.c to x509_create.c Signed-off-by: Sam Berry --- library/x509_create.c | 129 ++++++++++++++++++++++++ tf-psa-crypto/drivers/builtin/src/oid.c | 125 ----------------------- 2 files changed, 129 insertions(+), 125 deletions(-) diff --git a/library/x509_create.c b/library/x509_create.c index 839b5df22..e428461a2 100644 --- a/library/x509_create.c +++ b/library/x509_create.c @@ -278,6 +278,135 @@ error: return MBEDTLS_ERR_X509_INVALID_NAME; } +#if defined(MBEDTLS_OID_C) + +/* Return the OID for the given x.y.z.... style numeric string */ +int mbedtls_oid_from_numeric_string(mbedtls_asn1_buf *oid, + const char *oid_str, size_t size) +{ + int ret = MBEDTLS_ERR_ASN1_INVALID_DATA; + const char *str_ptr = oid_str; + const char *str_bound = oid_str + size; + unsigned int val = 0; + unsigned int component1, component2; + size_t encoded_len; + unsigned char *resized_mem; + + /* Count the number of dots to get a worst-case allocation size. */ + size_t num_dots = 0; + for (size_t i = 0; i < size; i++) { + if (oid_str[i] == '.') { + num_dots++; + } + } + /* Allocate maximum possible required memory: + * There are (num_dots + 1) integer components, but the first 2 share the + * same subidentifier, so we only need num_dots subidentifiers maximum. */ + if (num_dots == 0 || (num_dots > MBEDTLS_OID_MAX_COMPONENTS - 1)) { + return MBEDTLS_ERR_ASN1_INVALID_DATA; + } + /* Each byte can store 7 bits, calculate number of bytes for a + * subidentifier: + * + * bytes = ceil(subidentifer_size * 8 / 7) + */ + size_t bytes_per_subidentifier = (((sizeof(unsigned int) * 8) - 1) / 7) + + 1; + size_t max_possible_bytes = num_dots * bytes_per_subidentifier; + oid->p = mbedtls_calloc(max_possible_bytes, 1); + if (oid->p == NULL) { + return MBEDTLS_ERR_ASN1_ALLOC_FAILED; + } + unsigned char *out_ptr = oid->p; + unsigned char *out_bound = oid->p + max_possible_bytes; + + ret = oid_parse_number(&component1, &str_ptr, str_bound); + if (ret != 0) { + goto error; + } + if (component1 > 2) { + /* First component can't be > 2 */ + ret = MBEDTLS_ERR_ASN1_INVALID_DATA; + goto error; + } + if (str_ptr >= str_bound || *str_ptr != '.') { + ret = MBEDTLS_ERR_ASN1_INVALID_DATA; + goto error; + } + str_ptr++; + + ret = oid_parse_number(&component2, &str_ptr, str_bound); + if (ret != 0) { + goto error; + } + if ((component1 < 2) && (component2 > 39)) { + /* Root nodes 0 and 1 may have up to 40 children, numbered 0-39 */ + ret = MBEDTLS_ERR_ASN1_INVALID_DATA; + goto error; + } + if (str_ptr < str_bound) { + if (*str_ptr == '.') { + str_ptr++; + } else { + ret = MBEDTLS_ERR_ASN1_INVALID_DATA; + goto error; + } + } + + if (component2 > (UINT_MAX - (component1 * 40))) { + ret = MBEDTLS_ERR_ASN1_INVALID_DATA; + goto error; + } + ret = oid_subidentifier_encode_into(&out_ptr, out_bound, + (component1 * 40) + component2); + if (ret != 0) { + goto error; + } + + while (str_ptr < str_bound) { + ret = oid_parse_number(&val, &str_ptr, str_bound); + if (ret != 0) { + goto error; + } + if (str_ptr < str_bound) { + if (*str_ptr == '.') { + str_ptr++; + } else { + ret = MBEDTLS_ERR_ASN1_INVALID_DATA; + goto error; + } + } + + ret = oid_subidentifier_encode_into(&out_ptr, out_bound, val); + if (ret != 0) { + goto error; + } + } + + encoded_len = (size_t) (out_ptr - oid->p); + resized_mem = mbedtls_calloc(encoded_len, 1); + if (resized_mem == NULL) { + ret = MBEDTLS_ERR_ASN1_ALLOC_FAILED; + goto error; + } + memcpy(resized_mem, oid->p, encoded_len); + mbedtls_free(oid->p); + oid->p = resized_mem; + oid->len = encoded_len; + + oid->tag = MBEDTLS_ASN1_OID; + + return 0; + +error: + mbedtls_free(oid->p); + oid->p = NULL; + oid->len = 0; + return ret; +} + +#endif /* MBEDTLS_OID_C */ + int mbedtls_x509_string_to_names(mbedtls_asn1_named_data **head, const char *name) { int ret = MBEDTLS_ERR_X509_INVALID_NAME; diff --git a/tf-psa-crypto/drivers/builtin/src/oid.c b/tf-psa-crypto/drivers/builtin/src/oid.c index 565fac606..e54a8b113 100644 --- a/tf-psa-crypto/drivers/builtin/src/oid.c +++ b/tf-psa-crypto/drivers/builtin/src/oid.c @@ -969,129 +969,4 @@ static int oid_subidentifier_encode_into(unsigned char **p, return 0; } -/* Return the OID for the given x.y.z.... style numeric string */ -int mbedtls_oid_from_numeric_string(mbedtls_asn1_buf *oid, - const char *oid_str, size_t size) -{ - int ret = MBEDTLS_ERR_ASN1_INVALID_DATA; - const char *str_ptr = oid_str; - const char *str_bound = oid_str + size; - unsigned int val = 0; - unsigned int component1, component2; - size_t encoded_len; - unsigned char *resized_mem; - - /* Count the number of dots to get a worst-case allocation size. */ - size_t num_dots = 0; - for (size_t i = 0; i < size; i++) { - if (oid_str[i] == '.') { - num_dots++; - } - } - /* Allocate maximum possible required memory: - * There are (num_dots + 1) integer components, but the first 2 share the - * same subidentifier, so we only need num_dots subidentifiers maximum. */ - if (num_dots == 0 || (num_dots > MBEDTLS_OID_MAX_COMPONENTS - 1)) { - return MBEDTLS_ERR_ASN1_INVALID_DATA; - } - /* Each byte can store 7 bits, calculate number of bytes for a - * subidentifier: - * - * bytes = ceil(subidentifer_size * 8 / 7) - */ - size_t bytes_per_subidentifier = (((sizeof(unsigned int) * 8) - 1) / 7) - + 1; - size_t max_possible_bytes = num_dots * bytes_per_subidentifier; - oid->p = mbedtls_calloc(max_possible_bytes, 1); - if (oid->p == NULL) { - return MBEDTLS_ERR_ASN1_ALLOC_FAILED; - } - unsigned char *out_ptr = oid->p; - unsigned char *out_bound = oid->p + max_possible_bytes; - - ret = oid_parse_number(&component1, &str_ptr, str_bound); - if (ret != 0) { - goto error; - } - if (component1 > 2) { - /* First component can't be > 2 */ - ret = MBEDTLS_ERR_ASN1_INVALID_DATA; - goto error; - } - if (str_ptr >= str_bound || *str_ptr != '.') { - ret = MBEDTLS_ERR_ASN1_INVALID_DATA; - goto error; - } - str_ptr++; - - ret = oid_parse_number(&component2, &str_ptr, str_bound); - if (ret != 0) { - goto error; - } - if ((component1 < 2) && (component2 > 39)) { - /* Root nodes 0 and 1 may have up to 40 children, numbered 0-39 */ - ret = MBEDTLS_ERR_ASN1_INVALID_DATA; - goto error; - } - if (str_ptr < str_bound) { - if (*str_ptr == '.') { - str_ptr++; - } else { - ret = MBEDTLS_ERR_ASN1_INVALID_DATA; - goto error; - } - } - - if (component2 > (UINT_MAX - (component1 * 40))) { - ret = MBEDTLS_ERR_ASN1_INVALID_DATA; - goto error; - } - ret = oid_subidentifier_encode_into(&out_ptr, out_bound, - (component1 * 40) + component2); - if (ret != 0) { - goto error; - } - - while (str_ptr < str_bound) { - ret = oid_parse_number(&val, &str_ptr, str_bound); - if (ret != 0) { - goto error; - } - if (str_ptr < str_bound) { - if (*str_ptr == '.') { - str_ptr++; - } else { - ret = MBEDTLS_ERR_ASN1_INVALID_DATA; - goto error; - } - } - - ret = oid_subidentifier_encode_into(&out_ptr, out_bound, val); - if (ret != 0) { - goto error; - } - } - - encoded_len = (size_t) (out_ptr - oid->p); - resized_mem = mbedtls_calloc(encoded_len, 1); - if (resized_mem == NULL) { - ret = MBEDTLS_ERR_ASN1_ALLOC_FAILED; - goto error; - } - memcpy(resized_mem, oid->p, encoded_len); - mbedtls_free(oid->p); - oid->p = resized_mem; - oid->len = encoded_len; - - oid->tag = MBEDTLS_ASN1_OID; - - return 0; - -error: - mbedtls_free(oid->p); - oid->p = NULL; - oid->len = 0; - return ret; -} - #endif /* MBEDTLS_OID_C */ From 2bb3f4d6d4b1f181805692f8c0ca9f304a2a5705 Mon Sep 17 00:00:00 2001 From: Sam Berry Date: Fri, 19 Jul 2024 15:14:27 +0100 Subject: [PATCH 05/10] Move mbedtls_oid_from_numeric_string unit tests to test_suite_x509write This commit moves all related mbedtls_oid_from_numeric_string unit tests from test_suite_oid to test_suite_x509write. Signed-off-by: Sam Berry --- tests/suites/test_suite_x509write.data | 49 +++++++++++++++++++ tests/suites/test_suite_x509write.function | 27 ++++++++++ .../tests/suites/test_suite_oid.data | 48 ------------------ .../tests/suites/test_suite_oid.function | 26 ---------- 4 files changed, 76 insertions(+), 74 deletions(-) diff --git a/tests/suites/test_suite_x509write.data b/tests/suites/test_suite_x509write.data index e0dfd0f06..ddbac8993 100644 --- a/tests/suites/test_suite_x509write.data +++ b/tests/suites/test_suite_x509write.data @@ -268,3 +268,52 @@ x509_set_serial_check: Check max extension length x509_set_extension_length_check: + +OID from numeric string - hardware module name +oid_from_numeric_string:"1.3.6.1.5.5.7.8.4":0:"2B06010505070804" + +OID from numeric string - multi-byte subidentifier +oid_from_numeric_string:"1.1.2108":0:"29903C" + +OID from numeric string - second component greater than 39 +oid_from_numeric_string:"2.49.0.0.826.0":0:"81010000863A00" + +OID from numeric string - multi-byte first subidentifier +oid_from_numeric_string:"2.999":0:"8837" + +OID from numeric string - empty string input +oid_from_numeric_string:"":MBEDTLS_ERR_ASN1_INVALID_DATA:"" + +OID from numeric string - first component not a number +oid_from_numeric_string:"abc.1.2":MBEDTLS_ERR_ASN1_INVALID_DATA:"" + +OID from numeric string - second component not a number +oid_from_numeric_string:"1.abc.2":MBEDTLS_ERR_ASN1_INVALID_DATA:"" + +OID from numeric string - first component too large +oid_from_numeric_string:"3.1":MBEDTLS_ERR_ASN1_INVALID_DATA:"" + +OID from numeric string - first component < 2, second > 39 +oid_from_numeric_string:"1.40":MBEDTLS_ERR_ASN1_INVALID_DATA:"" + +OID from numeric string - third component not a number +oid_from_numeric_string:"1.2.abc":MBEDTLS_ERR_ASN1_INVALID_DATA:"" + +OID from numeric string - non-'.' separator between first and second +oid_from_numeric_string:"1/2.3.4":MBEDTLS_ERR_ASN1_INVALID_DATA:"" + +OID from numeric string - non-'.' separator between second and third +oid_from_numeric_string:"1.2/3.4":MBEDTLS_ERR_ASN1_INVALID_DATA:"" + +OID from numeric string - non-'.' separator between third and fourth +oid_from_numeric_string:"1.2.3/4":MBEDTLS_ERR_ASN1_INVALID_DATA:"" + +OID from numeric string - OID greater than max length (129 components) +oid_from_numeric_string:"1.2.3.4.5.6.7.8.1.2.3.4.5.6.7.8.1.2.3.4.5.6.7.8.1.2.3.4.5.6.7.8.1.2.3.4.5.6.7.8.1.2.3.4.5.6.7.8.1.2.3.4.5.6.7.8.1.2.3.4.5.6.7.8.1.2.3.4.5.6.7.8.1.2.3.4.5.6.7.8.1.2.3.4.5.6.7.8.1.2.3.4.5.6.7.8.1.2.3.4.5.6.7.8.1.2.3.4.5.6.7.8.1.2.3.4.5.6.7.8.1.2.3.4.5.6.7.8.1":MBEDTLS_ERR_ASN1_INVALID_DATA:"" + +OID from numeric string - OID with maximum subidentifier +oid_from_numeric_string:"2.4294967215":0:"8FFFFFFF7F" + +OID from numeric string - OID with overflowing subidentifier +oid_from_numeric_string:"2.4294967216":MBEDTLS_ERR_ASN1_INVALID_DATA:"" + diff --git a/tests/suites/test_suite_x509write.function b/tests/suites/test_suite_x509write.function index eb3c2f779..64b4e9e87 100644 --- a/tests/suites/test_suite_x509write.function +++ b/tests/suites/test_suite_x509write.function @@ -6,6 +6,7 @@ #include "mbedtls/pem.h" #include "mbedtls/oid.h" #include "mbedtls/rsa.h" +#include "mbedtls/asn1.h" #include "mbedtls/asn1write.h" #include "mbedtls/pk.h" #include "mbedtls/psa_util.h" @@ -761,3 +762,29 @@ void x509_set_extension_length_check() TEST_ASSERT(MBEDTLS_ERR_X509_BAD_INPUT_DATA == ret); } /* END_CASE */ + +/* BEGIN_CASE depends_on:MBEDTLS_X509_USE_C */ +void oid_from_numeric_string(char *oid_str, int error_ret, + data_t *exp_oid_buf) +{ + mbedtls_asn1_buf oid = { 0, 0, NULL }; + mbedtls_asn1_buf exp_oid = { 0, 0, NULL }; + int ret; + + exp_oid.tag = MBEDTLS_ASN1_OID; + exp_oid.p = exp_oid_buf->x; + exp_oid.len = exp_oid_buf->len; + + ret = mbedtls_oid_from_numeric_string(&oid, oid_str, strlen(oid_str)); + + if (error_ret == 0) { + TEST_EQUAL(oid.len, exp_oid.len); + TEST_ASSERT(memcmp(oid.p, exp_oid.p, oid.len) == 0); + mbedtls_free(oid.p); + oid.p = NULL; + oid.len = 0; + } else { + TEST_EQUAL(ret, error_ret); + } +} +/* END_CASE */ diff --git a/tf-psa-crypto/tests/suites/test_suite_oid.data b/tf-psa-crypto/tests/suites/test_suite_oid.data index 3521c97e8..42b050580 100644 --- a/tf-psa-crypto/tests/suites/test_suite_oid.data +++ b/tf-psa-crypto/tests/suites/test_suite_oid.data @@ -105,54 +105,6 @@ oid_get_md_alg_id:"2b24030201":MBEDTLS_MD_RIPEMD160 OID hash id - invalid oid oid_get_md_alg_id:"2B864886f70d0204":-1 -OID from numeric string - hardware module name -oid_from_numeric_string:"1.3.6.1.5.5.7.8.4":0:"2B06010505070804" - -OID from numeric string - multi-byte subidentifier -oid_from_numeric_string:"1.1.2108":0:"29903C" - -OID from numeric string - second component greater than 39 -oid_from_numeric_string:"2.49.0.0.826.0":0:"81010000863A00" - -OID from numeric string - multi-byte first subidentifier -oid_from_numeric_string:"2.999":0:"8837" - -OID from numeric string - empty string input -oid_from_numeric_string:"":MBEDTLS_ERR_ASN1_INVALID_DATA:"" - -OID from numeric string - first component not a number -oid_from_numeric_string:"abc.1.2":MBEDTLS_ERR_ASN1_INVALID_DATA:"" - -OID from numeric string - second component not a number -oid_from_numeric_string:"1.abc.2":MBEDTLS_ERR_ASN1_INVALID_DATA:"" - -OID from numeric string - first component too large -oid_from_numeric_string:"3.1":MBEDTLS_ERR_ASN1_INVALID_DATA:"" - -OID from numeric string - first component < 2, second > 39 -oid_from_numeric_string:"1.40":MBEDTLS_ERR_ASN1_INVALID_DATA:"" - -OID from numeric string - third component not a number -oid_from_numeric_string:"1.2.abc":MBEDTLS_ERR_ASN1_INVALID_DATA:"" - -OID from numeric string - non-'.' separator between first and second -oid_from_numeric_string:"1/2.3.4":MBEDTLS_ERR_ASN1_INVALID_DATA:"" - -OID from numeric string - non-'.' separator between second and third -oid_from_numeric_string:"1.2/3.4":MBEDTLS_ERR_ASN1_INVALID_DATA:"" - -OID from numeric string - non-'.' separator between third and fourth -oid_from_numeric_string:"1.2.3/4":MBEDTLS_ERR_ASN1_INVALID_DATA:"" - -OID from numeric string - OID greater than max length (129 components) -oid_from_numeric_string:"1.2.3.4.5.6.7.8.1.2.3.4.5.6.7.8.1.2.3.4.5.6.7.8.1.2.3.4.5.6.7.8.1.2.3.4.5.6.7.8.1.2.3.4.5.6.7.8.1.2.3.4.5.6.7.8.1.2.3.4.5.6.7.8.1.2.3.4.5.6.7.8.1.2.3.4.5.6.7.8.1.2.3.4.5.6.7.8.1.2.3.4.5.6.7.8.1.2.3.4.5.6.7.8.1.2.3.4.5.6.7.8.1.2.3.4.5.6.7.8.1.2.3.4.5.6.7.8.1":MBEDTLS_ERR_ASN1_INVALID_DATA:"" - -OID from numeric string - OID with maximum subidentifier -oid_from_numeric_string:"2.4294967215":0:"8FFFFFFF7F" - -OID from numeric string - OID with overflowing subidentifier -oid_from_numeric_string:"2.4294967216":MBEDTLS_ERR_ASN1_INVALID_DATA:"" - mbedtls_oid_get_md_hmac - RIPEMD160 depends_on:PSA_WANT_ALG_RIPEMD160 mbedtls_oid_get_md_hmac:"2B06010505080104":MBEDTLS_MD_RIPEMD160 diff --git a/tf-psa-crypto/tests/suites/test_suite_oid.function b/tf-psa-crypto/tests/suites/test_suite_oid.function index 5cfa35b64..e96425e1a 100644 --- a/tf-psa-crypto/tests/suites/test_suite_oid.function +++ b/tf-psa-crypto/tests/suites/test_suite_oid.function @@ -118,29 +118,3 @@ void mbedtls_oid_get_md_hmac(data_t *oid, int exp_md_id) } } /* END_CASE */ - -/* BEGIN_CASE */ -void oid_from_numeric_string(char *oid_str, int error_ret, - data_t *exp_oid_buf) -{ - mbedtls_asn1_buf oid = { 0, 0, NULL }; - mbedtls_asn1_buf exp_oid = { 0, 0, NULL }; - int ret; - - exp_oid.tag = MBEDTLS_ASN1_OID; - exp_oid.p = exp_oid_buf->x; - exp_oid.len = exp_oid_buf->len; - - ret = mbedtls_oid_from_numeric_string(&oid, oid_str, strlen(oid_str)); - - if (error_ret == 0) { - TEST_EQUAL(oid.len, exp_oid.len); - TEST_ASSERT(memcmp(oid.p, exp_oid.p, oid.len) == 0); - mbedtls_free(oid.p); - oid.p = NULL; - oid.len = 0; - } else { - TEST_EQUAL(ret, error_ret); - } -} -/* END_CASE */ From 3da783b468cc5dbd4bfca7e5ec81ba1011de9834 Mon Sep 17 00:00:00 2001 From: Sam Berry Date: Fri, 13 Sep 2024 15:09:24 +0100 Subject: [PATCH 06/10] Move static OID functions to x509.c This commit moves static functions that are necessary for mbedtls_oid_get_numeric_string and mbedtls_oid_from_numeric_string from oid.c to x509.c Signed-off-by: Sam Berry --- library/x509_create.c | 51 +++++++++++++++++++++++++ tf-psa-crypto/drivers/builtin/src/oid.c | 51 ------------------------- 2 files changed, 51 insertions(+), 51 deletions(-) diff --git a/library/x509_create.c b/library/x509_create.c index e428461a2..fa254fec5 100644 --- a/library/x509_create.c +++ b/library/x509_create.c @@ -280,6 +280,57 @@ error: #if defined(MBEDTLS_OID_C) +static int oid_parse_number(unsigned int *num, const char **p, const char *bound) +{ + int ret = MBEDTLS_ERR_ASN1_INVALID_DATA; + + *num = 0; + + while (*p < bound && **p >= '0' && **p <= '9') { + ret = 0; + if (*num > (UINT_MAX / 10)) { + return MBEDTLS_ERR_ASN1_INVALID_DATA; + } + *num *= 10; + *num += **p - '0'; + (*p)++; + } + return ret; +} + +static size_t oid_subidentifier_num_bytes(unsigned int value) +{ + size_t num_bytes = 0; + + do { + value >>= 7; + num_bytes++; + } while (value != 0); + + return num_bytes; +} + +static int oid_subidentifier_encode_into(unsigned char **p, + unsigned char *bound, + unsigned int value) +{ + size_t num_bytes = oid_subidentifier_num_bytes(value); + + if ((size_t) (bound - *p) < num_bytes) { + return MBEDTLS_ERR_OID_BUF_TOO_SMALL; + } + (*p)[num_bytes - 1] = (unsigned char) (value & 0x7f); + value >>= 7; + + for (size_t i = 2; i <= num_bytes; i++) { + (*p)[num_bytes - i] = 0x80 | (unsigned char) (value & 0x7f); + value >>= 7; + } + *p += num_bytes; + + return 0; +} + /* Return the OID for the given x.y.z.... style numeric string */ int mbedtls_oid_from_numeric_string(mbedtls_asn1_buf *oid, const char *oid_str, size_t size) diff --git a/tf-psa-crypto/drivers/builtin/src/oid.c b/tf-psa-crypto/drivers/builtin/src/oid.c index e54a8b113..980f235b8 100644 --- a/tf-psa-crypto/drivers/builtin/src/oid.c +++ b/tf-psa-crypto/drivers/builtin/src/oid.c @@ -918,55 +918,4 @@ FN_OID_GET_ATTR2(mbedtls_oid_get_pkcs12_pbe_alg, cipher_alg) #endif /* MBEDTLS_PKCS12_C && MBEDTLS_CIPHER_C */ -static int oid_parse_number(unsigned int *num, const char **p, const char *bound) -{ - int ret = MBEDTLS_ERR_ASN1_INVALID_DATA; - - *num = 0; - - while (*p < bound && **p >= '0' && **p <= '9') { - ret = 0; - if (*num > (UINT_MAX / 10)) { - return MBEDTLS_ERR_ASN1_INVALID_DATA; - } - *num *= 10; - *num += **p - '0'; - (*p)++; - } - return ret; -} - -static size_t oid_subidentifier_num_bytes(unsigned int value) -{ - size_t num_bytes = 0; - - do { - value >>= 7; - num_bytes++; - } while (value != 0); - - return num_bytes; -} - -static int oid_subidentifier_encode_into(unsigned char **p, - unsigned char *bound, - unsigned int value) -{ - size_t num_bytes = oid_subidentifier_num_bytes(value); - - if ((size_t) (bound - *p) < num_bytes) { - return MBEDTLS_ERR_OID_BUF_TOO_SMALL; - } - (*p)[num_bytes - 1] = (unsigned char) (value & 0x7f); - value >>= 7; - - for (size_t i = 2; i <= num_bytes; i++) { - (*p)[num_bytes - i] = 0x80 | (unsigned char) (value & 0x7f); - value >>= 7; - } - *p += num_bytes; - - return 0; -} - #endif /* MBEDTLS_OID_C */ From 5125a1bf21ac7bb006aed32a4815761453979ec3 Mon Sep 17 00:00:00 2001 From: Harry Ramsey Date: Fri, 13 Sep 2024 17:47:40 +0100 Subject: [PATCH 07/10] Add ChangeLog for moving OID string conversion functions Signed-off-by: Harry Ramsey --- ...split-numeric-string-conversions-out-of-the-oid-module.txt | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 ChangeLog.d/split-numeric-string-conversions-out-of-the-oid-module.txt diff --git a/ChangeLog.d/split-numeric-string-conversions-out-of-the-oid-module.txt b/ChangeLog.d/split-numeric-string-conversions-out-of-the-oid-module.txt new file mode 100644 index 000000000..2ccd39f6a --- /dev/null +++ b/ChangeLog.d/split-numeric-string-conversions-out-of-the-oid-module.txt @@ -0,0 +1,4 @@ +Changes + * Functions regarding numeric string conversions for OIDs have been moved + from the OID module and now reside in X.509 module. This helps to reduce + the code size as these functions are not commonly used outside of X.509. From e5b261f1e8c1732ba75c359b603a4a6a712e2e62 Mon Sep 17 00:00:00 2001 From: Harry Ramsey Date: Fri, 13 Sep 2024 23:41:43 +0100 Subject: [PATCH 08/10] Fix ChangeLog format error Signed-off-by: Harry Ramsey --- ...lit-numeric-string-conversions-out-of-the-oid-module.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ChangeLog.d/split-numeric-string-conversions-out-of-the-oid-module.txt b/ChangeLog.d/split-numeric-string-conversions-out-of-the-oid-module.txt index 2ccd39f6a..938e9eccb 100644 --- a/ChangeLog.d/split-numeric-string-conversions-out-of-the-oid-module.txt +++ b/ChangeLog.d/split-numeric-string-conversions-out-of-the-oid-module.txt @@ -1,4 +1,4 @@ Changes - * Functions regarding numeric string conversions for OIDs have been moved - from the OID module and now reside in X.509 module. This helps to reduce - the code size as these functions are not commonly used outside of X.509. + * Functions regarding numeric string conversions for OIDs have been moved + from the OID module and now reside in X.509 module. This helps to reduce + the code size as these functions are not commonly used outside of X.509. From 94c3065d7f81644279495d1b63bc5d85073ccfa9 Mon Sep 17 00:00:00 2001 From: Harry Ramsey Date: Thu, 19 Sep 2024 16:13:32 +0100 Subject: [PATCH 09/10] Fix x509 parse syntax typo Co-authored-by: David Horstmann Signed-off-by: Harry Ramsey --- tests/suites/test_suite_x509parse.function | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/suites/test_suite_x509parse.function b/tests/suites/test_suite_x509parse.function index 66a7c8df9..fae36571b 100644 --- a/tests/suites/test_suite_x509parse.function +++ b/tests/suites/test_suite_x509parse.function @@ -1750,7 +1750,7 @@ exit: } /* END_CASE */ -/* BEGIN_CASE depends_on:MBEDTLS_x509_USE_C */ +/* BEGIN_CASE depends_on:MBEDTLS_X509_USE_C */ void oid_get_numeric_string(data_t *oid, int error_ret, char *result_str) { char buf[256]; From 3b712627506d3a0268d6ca0f3cffc186b8d0ca4f Mon Sep 17 00:00:00 2001 From: Harry Ramsey Date: Thu, 26 Sep 2024 11:38:25 +0100 Subject: [PATCH 10/10] Remove MBEDTLS_OID_C guard from static functions This commit removes the MBEDTLS_OID_C guard from the static functions in the library/x509_create.c as this function is no longer included in the oid.c file. Signed-off-by: Harry Ramsey --- library/x509_create.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/library/x509_create.c b/library/x509_create.c index fa254fec5..130983189 100644 --- a/library/x509_create.c +++ b/library/x509_create.c @@ -278,8 +278,6 @@ error: return MBEDTLS_ERR_X509_INVALID_NAME; } -#if defined(MBEDTLS_OID_C) - static int oid_parse_number(unsigned int *num, const char **p, const char *bound) { int ret = MBEDTLS_ERR_ASN1_INVALID_DATA; @@ -456,8 +454,6 @@ error: return ret; } -#endif /* MBEDTLS_OID_C */ - int mbedtls_x509_string_to_names(mbedtls_asn1_named_data **head, const char *name) { int ret = MBEDTLS_ERR_X509_INVALID_NAME;