diff --git a/components/mbedtls/Kconfig b/components/mbedtls/Kconfig index 4800211f43..9ac8eb903b 100644 --- a/components/mbedtls/Kconfig +++ b/components/mbedtls/Kconfig @@ -327,6 +327,12 @@ menu "mbedTLS" help Name of the custom certificate directory or file. This path is evaluated relative to the project root directory. + + config MBEDTLS_CERTIFICATE_BUNDLE_MAX_CERTS + int "Maximum no of certificates allowed in certificate bundle" + default 200 + depends on MBEDTLS_CERTIFICATE_BUNDLE + endmenu config MBEDTLS_ECP_RESTARTABLE diff --git a/components/mbedtls/esp_crt_bundle/esp_crt_bundle.c b/components/mbedtls/esp_crt_bundle/esp_crt_bundle.c index fef27ef6a7..b0b144c294 100644 --- a/components/mbedtls/esp_crt_bundle/esp_crt_bundle.c +++ b/components/mbedtls/esp_crt_bundle/esp_crt_bundle.c @@ -146,27 +146,56 @@ int esp_crt_verify_callback(void *buf, mbedtls_x509_crt *crt, int depth, uint32_ /* Initialize the bundle into an array so we can do binary search for certs, the bundle generated by the python utility is already presorted by subject name */ -static esp_err_t esp_crt_bundle_init(const uint8_t *x509_bundle) +static esp_err_t esp_crt_bundle_init(const uint8_t *x509_bundle, size_t bundle_size) { - s_crt_bundle.num_certs = (x509_bundle[0] << 8) | x509_bundle[1]; - s_crt_bundle.crts = calloc(s_crt_bundle.num_certs, sizeof(x509_bundle)); + if (bundle_size < BUNDLE_HEADER_OFFSET + CRT_HEADER_OFFSET) { + ESP_LOGE(TAG, "Invalid certificate bundle"); + return ESP_ERR_INVALID_ARG; + } - if (s_crt_bundle.crts == NULL) { + uint16_t num_certs = (x509_bundle[0] << 8) | x509_bundle[1]; + if (num_certs > CONFIG_MBEDTLS_CERTIFICATE_BUNDLE_MAX_CERTS) { + ESP_LOGE(TAG, "No. of certs in the certificate bundle = %d exceeds\n" + "Max allowed certificates in the certificate bundle = %d\n" + "Please update the menuconfig option with appropriate value", num_certs, CONFIG_MBEDTLS_CERTIFICATE_BUNDLE_MAX_CERTS); + return ESP_ERR_INVALID_ARG; + } + + const uint8_t **crts = calloc(num_certs, sizeof(x509_bundle)); + if (crts == NULL) { ESP_LOGE(TAG, "Unable to allocate memory for bundle"); return ESP_ERR_NO_MEM; } const uint8_t *cur_crt; + /* This is the maximum region that is allowed to access */ + const uint8_t *bundle_end = x509_bundle + bundle_size; cur_crt = x509_bundle + BUNDLE_HEADER_OFFSET; - for (int i = 0; i < s_crt_bundle.num_certs; i++) { - s_crt_bundle.crts[i] = cur_crt; - + for (int i = 0; i < num_certs; i++) { + crts[i] = cur_crt; + if (cur_crt + CRT_HEADER_OFFSET > bundle_end) { + ESP_LOGE(TAG, "Invalid certificate bundle"); + free(crts); + return ESP_ERR_INVALID_ARG; + } size_t name_len = cur_crt[0] << 8 | cur_crt[1]; size_t key_len = cur_crt[2] << 8 | cur_crt[3]; cur_crt = cur_crt + CRT_HEADER_OFFSET + name_len + key_len; } + if (cur_crt > bundle_end) { + ESP_LOGE(TAG, "Invalid certificate bundle"); + free(crts); + return ESP_ERR_INVALID_ARG; + } + + /* The previous crt bundle is only updated when initialization of the + * current crt_bundle is successful */ + /* Free previous crt_bundle */ + free(s_crt_bundle.crts); + s_crt_bundle.num_certs = num_certs; + s_crt_bundle.crts = crts; return ESP_OK; } @@ -175,7 +204,7 @@ esp_err_t esp_crt_bundle_attach(void *conf) esp_err_t ret = ESP_OK; // If no bundle has been set by the user then use the bundle embedded in the binary if (s_crt_bundle.crts == NULL) { - ret = esp_crt_bundle_init(x509_crt_imported_bundle_bin_start); + ret = esp_crt_bundle_init(x509_crt_imported_bundle_bin_start, x509_crt_imported_bundle_bin_end - x509_crt_imported_bundle_bin_start); } if (ret != ESP_OK) { @@ -206,9 +235,7 @@ void esp_crt_bundle_detach(mbedtls_ssl_config *conf) } } -void esp_crt_bundle_set(const uint8_t *x509_bundle) +esp_err_t esp_crt_bundle_set(const uint8_t *x509_bundle, size_t bundle_size) { - // Free any previously used bundle - free(s_crt_bundle.crts); - esp_crt_bundle_init(x509_bundle); + return esp_crt_bundle_init(x509_bundle, bundle_size); } diff --git a/components/mbedtls/esp_crt_bundle/include/esp_crt_bundle.h b/components/mbedtls/esp_crt_bundle/include/esp_crt_bundle.h index 19826d286b..49069185b3 100644 --- a/components/mbedtls/esp_crt_bundle/include/esp_crt_bundle.h +++ b/components/mbedtls/esp_crt_bundle/include/esp_crt_bundle.h @@ -45,13 +45,19 @@ void esp_crt_bundle_detach(mbedtls_ssl_config *conf); /** * @brief Set the default certificate bundle used for verification * - * Overrides the default certificate bundle. In most use cases the bundle should be + * Overrides the default certificate bundle only in case of successful initialization. In most use cases the bundle should be * set through menuconfig. The bundle needs to be sorted by subject name since binary search is * used to find certificates. * * @param[in] x509_bundle A pointer to the certificate bundle. + * + * @param[in] bundle_size Size of the certificate bundle in bytes. + * + * @return + * - ESP_OK if adding certificates was successful. + * - Other if an error occured or an action must be taken by the calling process. */ -void esp_crt_bundle_set(const uint8_t *x509_bundle); +esp_err_t esp_crt_bundle_set(const uint8_t *x509_bundle, size_t bundle_size); #ifdef __cplusplus diff --git a/components/mbedtls/test/test_esp_crt_bundle.c b/components/mbedtls/test/test_esp_crt_bundle.c index db797c0457..b8ea822435 100644 --- a/components/mbedtls/test/test_esp_crt_bundle.c +++ b/components/mbedtls/test/test_esp_crt_bundle.c @@ -25,6 +25,7 @@ #include "mbedtls/debug.h" #include "esp_crt_bundle.h" +#include "esp_random.h" #include "unity.h" #include "test_utils.h" @@ -252,7 +253,7 @@ esp_err_t client_setup(mbedtls_endpoint_t *client) return ESP_OK; } -int client_task(const uint8_t *bundle, esp_crt_validate_res_t *res) +int client_task(const uint8_t *bundle, size_t bundle_size, esp_crt_validate_res_t *res) { int ret = ESP_FAIL; @@ -268,7 +269,7 @@ int client_task(const uint8_t *bundle, esp_crt_validate_res_t *res) esp_crt_bundle_attach(&client.conf); if (bundle) { /* Set a bundle different from the menuconfig bundle */ - esp_crt_bundle_set(bundle); + esp_crt_bundle_set(bundle, bundle_size); } ESP_LOGI(TAG, "Connecting to %s:%s...", SERVER_ADDRESS, SERVER_PORT); @@ -328,11 +329,11 @@ TEST_CASE("custom certificate bundle", "[mbedtls]") } /* Test with default crt bundle that doesnt contain the ca crt */ - client_task(NULL, &validate_res); + client_task(NULL, 0, &validate_res); TEST_ASSERT(validate_res == ESP_CRT_VALIDATE_FAIL); /* Test with bundle that does contain the CA crt */ - client_task(server_cert_bundle_start, &validate_res); + client_task(server_cert_bundle_start, server_cert_bundle_end - server_cert_bundle_start, &validate_res); TEST_ASSERT(validate_res == ESP_CRT_VALIDATE_OK); exit_flag = true; @@ -391,3 +392,50 @@ TEST_CASE("custom certificate bundle - wrong signature", "[mbedtls]") esp_crt_bundle_detach(NULL); } + +TEST_CASE("custom certificate bundle init API - bound checking", "[mbedtls]") +{ + + uint8_t test_bundle[256] = {0}; + esp_err_t esp_ret; + /* The API should fail with bundle size given as 1 */ + esp_ret = esp_crt_bundle_set(test_bundle, 1); + TEST_ASSERT( esp_ret == ESP_ERR_INVALID_ARG); + + /* Check that the esp_crt_bundle_set API will not accept a bundle + * which has more no. of certs than configured in + * CONFIG_MBEDTLS_CERTIFICATE_BUNDLE_MAX_CERTS */ + + uint8_t rand; + esp_fill_random(&rand, 1); + test_bundle[0] = rand; + + /* Make sure that the number of certs will always be greater than + * CONFIG_MBEDTLS_CERTIFICATE_BUNDLE_MAX_CERTS */ + test_bundle[1] = rand + CONFIG_MBEDTLS_CERTIFICATE_BUNDLE_MAX_CERTS; + + esp_ret = esp_crt_bundle_set(test_bundle, sizeof(test_bundle)); + TEST_ASSERT( esp_ret == ESP_ERR_INVALID_ARG); + + /* The API should fail with bundle_size < BUNDLE_HEADER_OFFSET (2) + CRT_HEADER_OFFSET (4) */ + test_bundle[0] = 0; + test_bundle[1] = 1; /* set num_certs = 1 */ + esp_ret = esp_crt_bundle_set(test_bundle, 5); + TEST_ASSERT(esp_ret == ESP_ERR_INVALID_ARG); + + /* Cert number is greater than actual certs present, The API should fail */ + /* Actual No. of certs present in bundle = 1, setting num_certs to 5 */ + test_bundle[1] = 5; /* num_certs */ + test_bundle[3] = 5; /* cert_1_name_len */ + test_bundle[5] = 10; /* cert_1_pub_key_len */ + /* Actual bundle size becomes BUNDLE_HEADER_OFFSET (2) + CRT_HEADER_OFFSET (4) + cert_1_name_len(5) + cert_1_pub_key_len(10) + * i.e. 21 bytes */ + esp_ret = esp_crt_bundle_set(test_bundle, 21); + TEST_ASSERT(esp_ret == ESP_ERR_INVALID_ARG); + + /* The API should fail if bundle_size < BUNDLE_HEADER_OFFSET (2) + CRT_HEADER_OFFSET (4) + cert_1_name_len(5) + cert_1_pub_key_len(10) */ + esp_ret = esp_crt_bundle_set(test_bundle, 20); + TEST_ASSERT(esp_ret == ESP_ERR_INVALID_ARG); + + esp_crt_bundle_detach(NULL); +} diff --git a/docs/en/migration-guides/protocols.rst b/docs/en/migration-guides/protocols.rst index 12d6b8c548..b39dcd3d4c 100644 --- a/docs/en/migration-guides/protocols.rst +++ b/docs/en/migration-guides/protocols.rst @@ -64,6 +64,10 @@ Remove certs module from X509 library ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ `mbedtls/certs.h` header is no longer available in mbedtls 3.1, most applications can safely remove it from the list of includes. +Breaking change for "esp_crt_bundle_set" API +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +:cpp:func:`esp_crt_bundle_set()` API now requires one additional argument named ``bundle_size``. The return type of the API has also been changed to :cpp:type:`esp_err_t` from ``void``. + ESP HTTPS SERVER -----------------