Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

esp_tls & esp_http_client : configure a certificate verify callback with user data parameter (IDFGH-10827) #12034

Closed
Pacheu opened this issue Aug 8, 2023 · 1 comment
Assignees
Labels
Resolution: NA Issue resolution is unavailable Status: Done Issue is done internally Type: Feature Request Feature request for IDF

Comments

@Pacheu
Copy link

Pacheu commented Aug 8, 2023

Is your feature request related to a problem?

Hi !
Here is the context:

I recently wanted to enable mbedtls certificate expiry check, from the mbedtls component embedded in esp-idf, so I enabled it in using menuconfig.

However, there are cases were I want to filter out error flags from the mbedtls verify certificate result so that the certificate is actually accepted, even if expired, but also even if "it starts in the future", because the system time is not set yet.
flags = (MBEDTLS_X509_BADCERT_FUTURE | MBEDTLS_X509_BADCERT_EXPIRED

It can simply be done when using directly mbedtls API. But when using the abstraction module that is esp_tls, which is used by the esp_http_client module, one cannot easily 'catch' the verify result to parse the flags.

The only entry I found there is through:

  • For esp_http_client: the parameter from the struct esp_http_client_config_t: esp_err_t (*crt_bundle_attach)(void *conf); /*!< Function pointer to esp_crt_bundle_attach. Enables the use of certification bundle for server verification, must be enabled in menuconfig */
  • For esp_tls: the same named parameter from the struct esp_tls_cfg_t: esp_err_t (*crt_bundle_attach)(void *conf); /*!< Function pointer to esp_crt_bundle_attach. Enables the use of certification bundle for server verification, must be enabled in menuconfig */

Using this parameter, I can give my http_client a function called on initialization to set my own 'cert bundle' (which is directly the expected CA chain to verify the server with) and a verify callback:

/* There is an alive static mbedtls_x509_crt _remote_cert_chain initialize and set in my module */
static esp_err_t _my_crt_bundle_attach(void *conf)
{
    esp_err_t ret = ESP_OK;
    mbedtls_ssl_config *ssl_conf =(mbedtls_ssl_config*)conf;
    mbedtls_ssl_conf_ca_chain(ssl_conf, &_remote_cert_chain, NULL);
    mbedtls_ssl_conf_verify(ssl_conf, _my_crt_verify_callback, NULL);
    return ret;
}

static int _my_crt_verify_callback(void* param, mbedtls_x509_crt* cert, int certificate_depth, uint32_t* flags)
{
    // I could use mbedtls_x509_crt_info() to print out some info on the certs received and the chain 
    //Now I can Parse the flags: Let's say I want to ignore the expiry check result

    uint32_t flags_filter = MBEDTLS_X509_BADCERT_FUTURE | MBEDTLS_X509_BADCERT_EXPIRED;

    *flags = *flags & ~flags_filter;
    if(*flags != 0)
    {
    	ESP_LOGE(HTTPS_TAG, "%s(): resulting flags are not 0, Failed to verify certificate", __func__);
    }
    return ret;
}

So fare so good, I can do half of what I would like to do.
But now, the issue is I cannot populate the void* param paramter of my callback, since I cannot give it through the _my_crt_bundle_attach(void* conf) function. Indeed, this function has a specific footprint given by the parameters from the config structure from esp_tls and esp_http, and only the mbedtls_ssl_config* conf is passed.

Describe the solution you'd like.

As I am programming in C++, I just wrote the code snippets above to be in C so that it keeps things very simple. But I wished to be able to pass user data through the API so that these attach and callback functions can work using specific objects and use the benefits of object oriented programming.

For example, let's say I have 2 clients, but I want to filter out different flags, have a different _remote_cert_chain which is not static but a local member from my objects, etc ... The only way I can do this for now is by duplicating the functions and variables and modify their behavior.

So, how about this:

  • For both esp_http_client and esp_tls: the parameter from the config structs becomes: esp_err_t (*crt_bundle_attach)(void *conf, void* user_data)
  • Or maybe even better so that the ESP x509 Certificate Bundle module works the same way:
    • Use the cacerts parameter to give the remove_ca_chain, and add to both esp_http_client and esp_tls new parameters to the config structs: int(*)(void *, mbedtls_x509_crt *, int, uint32_t *) f_vrfy, void * p_vrfy just like the mbedtls API.
    • In the ssl configuration, the job done by _my_crt_bundle_attach when calling mbedtls_ssl_conf_verify can be done if those parameters are given.
  • Or anything that allows giving user data to the verify callback ...

Depending on the solution, the 2 funcs can become

/* If the bundle attach method is kept */
static esp_err_t _my_crt_bundle_attach(void *conf, void* user_data)
{
    esp_err_t ret = ESP_OK;
    mbedtls_x509_crt* premote_cert_chain = NULL;
    if(user_data)
    {
        //get any context information from it
        my_struct_t* pmy_struct = (my_struct_t*)user_data;
        premote_cert_chain = &pmy_struct->remote_cert_chain;
    }
    mbedtls_ssl_config *ssl_conf =(mbedtls_ssl_config*)conf;
    mbedtls_ssl_conf_ca_chain(ssl_conf, premote_cert_chain, NULL); // a NULL premote_cert_chain will trigger an error, but lets keep it simple
    mbedtls_ssl_conf_verify(ssl_conf, _my_crt_verify_callback, user_data);
    return ret;
}

/* In both solutions*/
static int _my_crt_verify_callback(void* param, mbedtls_x509_crt* cert, int certificate_depth, uint32_t* flags)
{
    // I could use mbedtls_x509_crt_info() to print out some info on the certs received and the chain 
    //Now I can Parse the flags: Let's say I want to ignore the expiry check result
    uint32_t flags_filter = 0;
    if(param)
    {
        //get any context information from it
        my_struct_t* pmy_struct = (my_struct_t*)param;
        flags_filter |= pmy_struct->flags_filter;
    }

    *flags = *flags & ~flags_filter;
    if(*flags != 0)
    {
    	ESP_LOGE(HTTPS_TAG, "%s(): resulting flags are not 0, Failed to verify certificate", __func__);
    }
    return ret;
}

Describe alternatives you've considered.

I considered using directly mbedlts API and write my http_client from scratch, but that would take me ages to implement it and adapt my existing code (like https://github.com/espressif/esp-idf/tree/5eddb9e016/examples/protocols/https_mbedtls).

Why re-invent the wheel while esp_tls & esp_http_client exist ?

Additional context.

I know this would imply some modifications to the commonly used esp_tls & esp_http_client config structures, to a point where this is not a small change. Moreover, the first solution proposed here also makes mandatory to update the ESP x509 Certificate Bundle module.

Maybe these esp_err_t (*crt_bundle_attach)(void *conf) parameters were only meant to be used with this bundle module, with the esp_crt_bundle_attach function, and I derived the expected behavior, but I feel like I am not doing something no one would do and my suggestion is meaningful.

I hope readers understood my point well and that this suggestion is indeed relevant.

I look forward to reading any comment on that matter !
M. Pacheu

@Pacheu Pacheu added the Type: Feature Request Feature request for IDF label Aug 8, 2023
@espressif-bot espressif-bot added the Status: Opened Issue is new label Aug 8, 2023
@github-actions github-actions bot changed the title esp_tls & esp_http_client : configure a certificate verify callback with user data parameter esp_tls & esp_http_client : configure a certificate verify callback with user data parameter (IDFGH-10827) Aug 8, 2023
@mahavirj
Copy link
Member

@Pacheu

ESP-TLS provides few wrapper APIs to collect internal error codes from the TLS stack. They could help in this scenario, please see reference below:

/**
* @brief Returns last error in esp_tls with detailed mbedtls related error codes.
* The error information is cleared internally upon return
*
* @param[in] h esp-tls error handle.
* @param[out] esp_tls_code last error code returned from mbedtls api (set to zero if none)
* This pointer could be NULL if caller does not care about esp_tls_code
* @param[out] esp_tls_flags last certification verification flags (set to zero if none)
* This pointer could be NULL if caller does not care about esp_tls_code
*
* @return
* - ESP_ERR_INVALID_STATE if invalid parameters
* - ESP_OK (0) if no error occurred
* - specific error code (based on ESP_ERR_ESP_TLS_BASE) otherwise
*/
esp_err_t esp_tls_get_and_clear_last_error(esp_tls_error_handle_t h, int *esp_tls_code, int *esp_tls_flags);
/**
* @brief Returns the last error captured in esp_tls of a specific type
* The error information is cleared internally upon return
*
* @param[in] h esp-tls error handle.
* @param[in] err_type specific error type
* @param[out] error_code last error code returned from mbedtls api (set to zero if none)
* This pointer could be NULL if caller does not care about esp_tls_code
* @return
* - ESP_ERR_INVALID_STATE if invalid parameters
* - ESP_OK if a valid error returned and was cleared
*/
esp_err_t esp_tls_get_and_clear_error_type(esp_tls_error_handle_t h, esp_tls_error_type_t err_type, int *error_code);
/**
* @brief Returns the ESP-TLS error_handle
*
* @param[in] tls handle to esp_tls context
*
* @param[out] error_handle pointer to the error handle.
*
* @return
* - ESP_OK on success and error_handle will be updated with the ESP-TLS error handle.
*
* - ESP_ERR_INVALID_ARG if (tls == NULL || error_handle == NULL)
*/
esp_err_t esp_tls_get_error_handle(esp_tls_t *tls, esp_tls_error_handle_t *error_handle);

Regarding the specific errors related certificate verification in case of ESP-TLS certificate bundle feature, I see that it may require an additional fix as below:

--- components/esp-tls/esp_tls_mbedtls.c
+++ components/esp-tls/esp_tls_mbedtls.c
@@ -212,7 +212,7 @@ int esp_mbedtls_handshake(esp_tls_t *tls, const esp_tls_cfg_t *cfg)
             mbedtls_print_error_msg(ret);
             ESP_INT_EVENT_TRACKER_CAPTURE(tls->error_handle, ESP_TLS_ERR_TYPE_MBEDTLS, -ret);
             ESP_INT_EVENT_TRACKER_CAPTURE(tls->error_handle, ESP_TLS_ERR_TYPE_ESP, ESP_ERR_MBEDTLS_SSL_HANDSHAKE_FAILED);
-            if (cfg->cacert_buf != NULL || cfg->use_global_ca_store == true) {
+            if (cfg->cacert_buf != NULL || cfg->use_global_ca_store == true || cfg->crt_bundle_attach != NULL) {
                 /* This is to check whether handshake failed due to invalid certificate*/
                 esp_mbedtls_verify_certificate(tls);
             }

Please check if this can meet your requirement and then I will create an internal fix for this.

@espressif-bot espressif-bot added Status: In Progress Work is in progress Status: Done Issue is done internally Resolution: NA Issue resolution is unavailable and removed Status: Opened Issue is new Status: In Progress Work is in progress labels Aug 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: NA Issue resolution is unavailable Status: Done Issue is done internally Type: Feature Request Feature request for IDF
Projects
None yet
Development

No branches or pull requests

3 participants