From 73624e856066b069cfd81a8a3ca65540ee080bf2 Mon Sep 17 00:00:00 2001 From: Henning Fleddermann Date: Mon, 15 Jul 2019 17:51:25 +0200 Subject: [PATCH 1/3] modify comments on esp_tls_cfg, to clarify that other formats besides PEM (such as DER) might be used as well depending on mbedtls-support Signed-off-by: David Cermak --- components/esp-tls/esp_tls.h | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/components/esp-tls/esp_tls.h b/components/esp-tls/esp_tls.h index 1c2f785ea5..ebdffa49e2 100644 --- a/components/esp-tls/esp_tls.h +++ b/components/esp-tls/esp_tls.h @@ -90,22 +90,28 @@ typedef struct esp_tls_cfg { - where 'h2' is the protocol name */ const unsigned char *cacert_pem_buf; /*!< Certificate Authority's certificate in a buffer. - This buffer should be NULL terminated */ + Format may be PEM or DER, depending on mbedtls-support + This buffer should be NULL terminated in case of PEM */ unsigned int cacert_pem_bytes; /*!< Size of Certificate Authority certificate - pointed to by cacert_pem_buf */ + pointed to by cacert_pem_buf + (including NULL-terminator in case of PEM format) */ const unsigned char *clientcert_pem_buf;/*!< Client certificate in a buffer - This buffer should be NULL terminated */ + Format may be PEM or DER, depending on mbedtls-support + This buffer should be NULL terminated in case of PEM */ unsigned int clientcert_pem_bytes; /*!< Size of client certificate pointed to by - clientcert_pem_buf */ + clientcert_pem_buf + (including NULL-terminator in case of PEM format) */ const unsigned char *clientkey_pem_buf; /*!< Client key in a buffer - This buffer should be NULL terminated */ + Format may be PEM or DER, depending on mbedtls-support + This buffer should be NULL terminated in case of PEM */ unsigned int clientkey_pem_bytes; /*!< Size of client key pointed to by - clientkey_pem_buf */ + clientkey_pem_buf + (including NULL-terminator in case of PEM format) */ const unsigned char *clientkey_password;/*!< Client key decryption password string */ From 546b6254335435cc01ce26cded765855af8c4144 Mon Sep 17 00:00:00 2001 From: Henning Fleddermann Date: Mon, 15 Jul 2019 17:53:39 +0200 Subject: [PATCH 2/3] add _der variants for esp_transport_ssl_set_(client_cert|client_key|cert_data) Signed-off-by: David Cermak Merges https://github.com/espressif/esp-idf/pull/3783 --- .../tcp_transport/include/esp_transport_ssl.h | 33 +++++++++++++++++++ components/tcp_transport/transport_ssl.c | 27 +++++++++++++++ 2 files changed, 60 insertions(+) diff --git a/components/tcp_transport/include/esp_transport_ssl.h b/components/tcp_transport/include/esp_transport_ssl.h index 0f83c1d6e4..3e2bfb42da 100644 --- a/components/tcp_transport/include/esp_transport_ssl.h +++ b/components/tcp_transport/include/esp_transport_ssl.h @@ -40,6 +40,17 @@ esp_transport_handle_t esp_transport_ssl_init(); */ void esp_transport_ssl_set_cert_data(esp_transport_handle_t t, const char *data, int len); +/** + * @brief Set SSL certificate data (as DER format). + * Note that, this function stores the pointer to data, rather than making a copy. + * So this data must remain valid until after the connection is cleaned up + * + * @param t ssl transport + * @param[in] data The der data + * @param[in] len The length + */ +void esp_transport_ssl_set_cert_data_der(esp_transport_handle_t t, const char *data, int len); + /** * @brief Enable global CA store for SSL connection * @@ -58,6 +69,17 @@ void esp_transport_ssl_enable_global_ca_store(esp_transport_handle_t t); */ void esp_transport_ssl_set_client_cert_data(esp_transport_handle_t t, const char *data, int len); +/** + * @brief Set SSL client certificate data for mutual authentication (as DER format). + * Note that, this function stores the pointer to data, rather than making a copy. + * So this data must remain valid until after the connection is cleaned up + * + * @param t ssl transport + * @param[in] data The der data + * @param[in] len The length + */ +void esp_transport_ssl_set_client_cert_data_der(esp_transport_handle_t t, const char *data, int len); + /** * @brief Set SSL client key data for mutual authentication (as PEM format). * Note that, this function stores the pointer to data, rather than making a copy. @@ -69,6 +91,17 @@ void esp_transport_ssl_set_client_cert_data(esp_transport_handle_t t, const char */ void esp_transport_ssl_set_client_key_data(esp_transport_handle_t t, const char *data, int len); +/** + * @brief Set SSL client key data for mutual authentication (as DER format). + * Note that, this function stores the pointer to data, rather than making a copy. + * So this data must remain valid until after the connection is cleaned up + * + * @param t ssl transport + * @param[in] data The der data + * @param[in] len The length + */ +void esp_transport_ssl_set_client_key_data_der(esp_transport_handle_t t, const char *data, int len); + /** * @brief Skip validation of certificate's common name field * diff --git a/components/tcp_transport/transport_ssl.c b/components/tcp_transport/transport_ssl.c index 1651a2f87f..b8a2e7281e 100644 --- a/components/tcp_transport/transport_ssl.c +++ b/components/tcp_transport/transport_ssl.c @@ -178,6 +178,15 @@ void esp_transport_ssl_set_cert_data(esp_transport_handle_t t, const char *data, } } +void esp_transport_ssl_set_cert_data_der(esp_transport_handle_t t, const char *data, int len) +{ + transport_ssl_t *ssl = esp_transport_get_context_data(t); + if (t && ssl) { + ssl->cfg.cacert_pem_buf = (void *)data; + ssl->cfg.cacert_pem_bytes = len; + } +} + void esp_transport_ssl_set_client_cert_data(esp_transport_handle_t t, const char *data, int len) { transport_ssl_t *ssl = esp_transport_get_context_data(t); @@ -187,6 +196,15 @@ void esp_transport_ssl_set_client_cert_data(esp_transport_handle_t t, const char } } +void esp_transport_ssl_set_client_cert_data_der(esp_transport_handle_t t, const char *data, int len) +{ + transport_ssl_t *ssl = esp_transport_get_context_data(t); + if (t && ssl) { + ssl->cfg.clientcert_pem_buf = (void *)data; + ssl->cfg.clientcert_pem_bytes = len; + } +} + void esp_transport_ssl_set_client_key_data(esp_transport_handle_t t, const char *data, int len) { transport_ssl_t *ssl = esp_transport_get_context_data(t); @@ -196,6 +214,15 @@ void esp_transport_ssl_set_client_key_data(esp_transport_handle_t t, const char } } +void esp_transport_ssl_set_client_key_data_der(esp_transport_handle_t t, const char *data, int len) +{ + transport_ssl_t *ssl = esp_transport_get_context_data(t); + if (t && ssl) { + ssl->cfg.clientkey_pem_buf = (void *)data; + ssl->cfg.clientkey_pem_bytes = len; + } +} + void esp_transport_ssl_skip_common_name_check(esp_transport_handle_t t) { transport_ssl_t *ssl = esp_transport_get_context_data(t); From 25dd5e39afba5f66fe571ed235270b3972223679 Mon Sep 17 00:00:00 2001 From: David Cermak Date: Fri, 2 Aug 2019 09:20:02 +0200 Subject: [PATCH 3/3] esp-tls: Naming variables refering to certificates and keys in a neutral way to suggest that both PEM and DER format could be used, added comments descibing important details about using these formats --- components/esp-tls/esp_tls.c | 34 ++++----- components/esp-tls/esp_tls.h | 74 +++++++++++++++---- .../esp_https_server/src/https_server.c | 26 +++---- components/tcp_transport/transport_ssl.c | 12 +-- .../main/https_request_example_main.c | 4 +- 5 files changed, 98 insertions(+), 52 deletions(-) diff --git a/components/esp-tls/esp_tls.c b/components/esp-tls/esp_tls.c index 65cd789f39..e2d33cb0e9 100644 --- a/components/esp-tls/esp_tls.c +++ b/components/esp-tls/esp_tls.c @@ -339,8 +339,8 @@ static esp_err_t set_server_config(esp_tls_cfg_server_t *cfg, esp_tls_t *tls) } #endif - if (cfg->cacert_pem_buf != NULL) { - esp_ret = set_ca_cert(tls, cfg->cacert_pem_buf, cfg->cacert_pem_bytes); + if (cfg->cacert_buf != NULL) { + esp_ret = set_ca_cert(tls, cfg->cacert_buf, cfg->cacert_bytes); if (esp_ret != ESP_OK) { return esp_ret; } @@ -348,14 +348,14 @@ static esp_err_t set_server_config(esp_tls_cfg_server_t *cfg, esp_tls_t *tls) mbedtls_ssl_conf_authmode(&tls->conf, MBEDTLS_SSL_VERIFY_NONE); } - if (cfg->servercert_pem_buf != NULL && cfg->serverkey_pem_buf != NULL) { + if (cfg->servercert_buf != NULL && cfg->serverkey_buf != NULL) { esp_tls_pki_t pki = { .public_cert = &tls->servercert, .pk_key = &tls->serverkey, - .publiccert_pem_buf = cfg->servercert_pem_buf, - .publiccert_pem_bytes = cfg->servercert_pem_bytes, - .privkey_pem_buf = cfg->serverkey_pem_buf, - .privkey_pem_bytes = cfg->serverkey_pem_bytes, + .publiccert_pem_buf = cfg->servercert_buf, + .publiccert_pem_bytes = cfg->servercert_bytes, + .privkey_pem_buf = cfg->serverkey_buf, + .privkey_pem_bytes = cfg->serverkey_bytes, .privkey_password = cfg->serverkey_password, .privkey_password_len = cfg->serverkey_password_len, }; @@ -421,8 +421,8 @@ static esp_err_t set_client_config(const char *hostname, size_t hostlen, esp_tls if (esp_ret != ESP_OK) { return esp_ret; } - } else if (cfg->cacert_pem_buf != NULL) { - esp_err_t esp_ret = set_ca_cert(tls, cfg->cacert_pem_buf, cfg->cacert_pem_bytes); + } else if (cfg->cacert_buf != NULL) { + esp_err_t esp_ret = set_ca_cert(tls, cfg->cacert_buf, cfg->cacert_bytes); if (esp_ret != ESP_OK) { return esp_ret; } @@ -430,14 +430,14 @@ static esp_err_t set_client_config(const char *hostname, size_t hostlen, esp_tls mbedtls_ssl_conf_authmode(&tls->conf, MBEDTLS_SSL_VERIFY_NONE); } - if (cfg->clientcert_pem_buf != NULL && cfg->clientkey_pem_buf != NULL) { + if (cfg->clientcert_buf != NULL && cfg->clientkey_buf != NULL) { esp_tls_pki_t pki = { .public_cert = &tls->clientcert, .pk_key = &tls->clientkey, - .publiccert_pem_buf = cfg->clientcert_pem_buf, - .publiccert_pem_bytes = cfg->clientcert_pem_bytes, - .privkey_pem_buf = cfg->clientkey_pem_buf, - .privkey_pem_bytes = cfg->clientkey_pem_bytes, + .publiccert_pem_buf = cfg->clientcert_buf, + .publiccert_pem_bytes = cfg->clientcert_bytes, + .privkey_pem_buf = cfg->clientkey_buf, + .privkey_pem_bytes = cfg->clientkey_bytes, .privkey_password = cfg->clientkey_password, .privkey_password_len = cfg->clientkey_password_len, }; @@ -446,8 +446,8 @@ static esp_err_t set_client_config(const char *hostname, size_t hostlen, esp_tls ESP_LOGE(TAG, "Failed to set server pki context"); return esp_ret; } - } else if (cfg->clientcert_pem_buf != NULL || cfg->clientkey_pem_buf != NULL) { - ESP_LOGE(TAG, "You have to provide both clientcert_pem_buf and clientkey_pem_buf for mutual authentication"); + } else if (cfg->clientcert_buf != NULL || cfg->clientkey_buf != NULL) { + ESP_LOGE(TAG, "You have to provide both clientcert_buf and clientkey_buf for mutual authentication"); return ESP_ERR_INVALID_STATE; } return ESP_OK; @@ -628,7 +628,7 @@ static int esp_tls_low_level_conn(const char *hostname, int hostlen, int port, c ESP_LOGE(TAG, "mbedtls_ssl_handshake returned -0x%x", -ret); ESP_INT_EVENT_TRACKER_CAPTURE(tls->error_handle, ERR_TYPE_MBEDTLS, -ret); ESP_INT_EVENT_TRACKER_CAPTURE(tls->error_handle, ERR_TYPE_ESP, ESP_ERR_MBEDTLS_SSL_HANDSHAKE_FAILED); - if (cfg->cacert_pem_buf != NULL || cfg->use_global_ca_store == true) { + if (cfg->cacert_buf != NULL || cfg->use_global_ca_store == true) { /* This is to check whether handshake failed due to invalid certificate*/ verify_certificate(tls); } diff --git a/components/esp-tls/esp_tls.h b/components/esp-tls/esp_tls.h index ebdffa49e2..ee7fe49de7 100644 --- a/components/esp-tls/esp_tls.h +++ b/components/esp-tls/esp_tls.h @@ -78,6 +78,16 @@ typedef enum esp_tls_role { /** * @brief ESP-TLS configuration parameters + * + * @note Note about format of certificates: + * - This structure includes certificates of a Certificate Authority, of client or server as well + * as private keys, which may be of PEM or DER format. In case of PEM format, the buffer must be + * NULL terminated (with NULL character included in certificate size). + * - Certificate Authority's certificate may be a chain of certificates in case of PEM format, + * but could be only one certificate in case of DER format + * - Variables names of certificates and private key buffers and sizes are defined as unions providing + * backward compatibility for legacy *_pem_buf and *_pem_bytes names which suggested only PEM format + * was supported. It is encouraged to use generic names such as cacert_buf and cacert_bytes. */ typedef struct esp_tls_cfg { const char **alpn_protos; /*!< Application protocols required for HTTP2. @@ -89,29 +99,47 @@ typedef struct esp_tls_cfg { const char **alpn_protos = { "h2", NULL }; - where 'h2' is the protocol name */ - const unsigned char *cacert_pem_buf; /*!< Certificate Authority's certificate in a buffer. + union { + const unsigned char *cacert_buf; /*!< Certificate Authority's certificate in a buffer. Format may be PEM or DER, depending on mbedtls-support This buffer should be NULL terminated in case of PEM */ - - unsigned int cacert_pem_bytes; /*!< Size of Certificate Authority certificate - pointed to by cacert_pem_buf + const unsigned char *cacert_pem_buf; /*!< CA certificate buffer legacy name */ + }; + + union { + unsigned int cacert_bytes; /*!< Size of Certificate Authority certificate + pointed to by cacert_buf (including NULL-terminator in case of PEM format) */ + unsigned int cacert_pem_bytes; /*!< Size of Certificate Authority certificate legacy name */ + }; - const unsigned char *clientcert_pem_buf;/*!< Client certificate in a buffer + union { + const unsigned char *clientcert_buf; /*!< Client certificate in a buffer Format may be PEM or DER, depending on mbedtls-support This buffer should be NULL terminated in case of PEM */ + const unsigned char *clientcert_pem_buf; /*!< Client certificate legacy name */ + }; - unsigned int clientcert_pem_bytes; /*!< Size of client certificate pointed to by + union { + unsigned int clientcert_bytes; /*!< Size of client certificate pointed to by clientcert_pem_buf (including NULL-terminator in case of PEM format) */ + unsigned int clientcert_pem_bytes; /*!< Size of client certificate legacy name */ + }; - const unsigned char *clientkey_pem_buf; /*!< Client key in a buffer + union { + const unsigned char *clientkey_buf; /*!< Client key in a buffer Format may be PEM or DER, depending on mbedtls-support This buffer should be NULL terminated in case of PEM */ + const unsigned char *clientkey_pem_buf; /*!< Client key legacy name */ + }; - unsigned int clientkey_pem_bytes; /*!< Size of client key pointed to by + union { + unsigned int clientkey_bytes; /*!< Size of client key pointed to by clientkey_pem_buf (including NULL-terminator in case of PEM format) */ + unsigned int clientkey_pem_bytes; /*!< Size of client key legacy name */ + }; const unsigned char *clientkey_password;/*!< Client key decryption password string */ @@ -144,23 +172,41 @@ typedef struct esp_tls_cfg_server { const char **alpn_protos = { "h2", NULL }; - where 'h2' is the protocol name */ - const unsigned char *cacert_pem_buf; /*!< Client CA certificate in a buffer. + union { + const unsigned char *cacert_buf; /*!< Client CA certificate in a buffer. This buffer should be NULL terminated */ + const unsigned char *cacert_pem_buf; /*!< Client CA certificate legacy name */ + }; - unsigned int cacert_pem_bytes; /*!< Size of client CA certificate + union { + unsigned int cacert_bytes; /*!< Size of client CA certificate pointed to by cacert_pem_buf */ + unsigned int cacert_pem_bytes; /*!< Size of client CA certificate legacy name */ + }; - const unsigned char *servercert_pem_buf; /*!< Server certificate in a buffer + union { + const unsigned char *servercert_buf; /*!< Server certificate in a buffer This buffer should be NULL terminated */ + const unsigned char *servercert_pem_buf; /*!< Server certificate legacy name */ + }; - unsigned int servercert_pem_bytes; /*!< Size of server certificate pointed to by + union { + unsigned int servercert_bytes; /*!< Size of server certificate pointed to by servercert_pem_buf */ + unsigned int servercert_pem_bytes; /*!< Size of server certificate legacy name */ + }; - const unsigned char *serverkey_pem_buf; /*!< Server key in a buffer + union { + const unsigned char *serverkey_buf; /*!< Server key in a buffer This buffer should be NULL terminated */ + const unsigned char *serverkey_pem_buf; /*!< Server key legacy name */ + }; - unsigned int serverkey_pem_bytes; /*!< Size of server key pointed to by + union { + unsigned int serverkey_bytes; /*!< Size of server key pointed to by serverkey_pem_buf */ + unsigned int serverkey_pem_bytes; /*!< Size of server key legacy name */ + }; const unsigned char *serverkey_password; /*!< Server key decryption password string */ diff --git a/components/esp_https_server/src/https_server.c b/components/esp_https_server/src/https_server.c index f012ffec8e..47c2abcb0f 100644 --- a/components/esp_https_server/src/https_server.c +++ b/components/esp_https_server/src/https_server.c @@ -135,11 +135,11 @@ static void free_secure_context(void *ctx) assert(ctx != NULL); esp_tls_cfg_server_t *cfg = (esp_tls_cfg_server_t *)ctx; ESP_LOGI(TAG, "Server shuts down, releasing SSL context"); - if (cfg->servercert_pem_buf) { - free((void *)cfg->servercert_pem_buf); + if (cfg->servercert_buf) { + free((void *)cfg->servercert_buf); } - if (cfg->serverkey_pem_buf) { - free((void *)cfg->serverkey_pem_buf); + if (cfg->serverkey_buf) { + free((void *)cfg->serverkey_buf); } free(cfg); } @@ -150,22 +150,22 @@ static esp_tls_cfg_server_t *create_secure_context(const struct httpd_ssl_config if (!cfg) { return NULL; } - cfg->servercert_pem_buf = (unsigned char *)malloc(config->cacert_len); - if (!cfg->servercert_pem_buf) { + cfg->servercert_buf = (unsigned char *)malloc(config->cacert_len); + if (!cfg->servercert_buf) { free(cfg); return NULL; } - memcpy((char *)cfg->servercert_pem_buf, config->cacert_pem, config->cacert_len); - cfg->servercert_pem_bytes = config->cacert_len; + memcpy((char *)cfg->servercert_buf, config->cacert_pem, config->cacert_len); + cfg->servercert_bytes = config->cacert_len; - cfg->serverkey_pem_buf = (unsigned char *)malloc(config->prvtkey_len); - if (!cfg->serverkey_pem_buf) { - free((void *)cfg->servercert_pem_buf); + cfg->serverkey_buf = (unsigned char *)malloc(config->prvtkey_len); + if (!cfg->serverkey_buf) { + free((void *)cfg->servercert_buf); free(cfg); return NULL; } - memcpy((char *)cfg->serverkey_pem_buf, config->prvtkey_pem, config->prvtkey_len); - cfg->serverkey_pem_bytes = config->prvtkey_len; + memcpy((char *)cfg->serverkey_buf, config->prvtkey_pem, config->prvtkey_len); + cfg->serverkey_bytes = config->prvtkey_len; return cfg; } diff --git a/components/tcp_transport/transport_ssl.c b/components/tcp_transport/transport_ssl.c index b8a2e7281e..b576d1a487 100644 --- a/components/tcp_transport/transport_ssl.c +++ b/components/tcp_transport/transport_ssl.c @@ -182,8 +182,8 @@ void esp_transport_ssl_set_cert_data_der(esp_transport_handle_t t, const char *d { transport_ssl_t *ssl = esp_transport_get_context_data(t); if (t && ssl) { - ssl->cfg.cacert_pem_buf = (void *)data; - ssl->cfg.cacert_pem_bytes = len; + ssl->cfg.cacert_buf = (void *)data; + ssl->cfg.cacert_bytes = len; } } @@ -200,8 +200,8 @@ void esp_transport_ssl_set_client_cert_data_der(esp_transport_handle_t t, const { transport_ssl_t *ssl = esp_transport_get_context_data(t); if (t && ssl) { - ssl->cfg.clientcert_pem_buf = (void *)data; - ssl->cfg.clientcert_pem_bytes = len; + ssl->cfg.clientcert_buf = (void *)data; + ssl->cfg.clientcert_bytes = len; } } @@ -218,8 +218,8 @@ void esp_transport_ssl_set_client_key_data_der(esp_transport_handle_t t, const c { transport_ssl_t *ssl = esp_transport_get_context_data(t); if (t && ssl) { - ssl->cfg.clientkey_pem_buf = (void *)data; - ssl->cfg.clientkey_pem_bytes = len; + ssl->cfg.clientkey_buf = (void *)data; + ssl->cfg.clientkey_bytes = len; } } diff --git a/examples/protocols/https_request/main/https_request_example_main.c b/examples/protocols/https_request/main/https_request_example_main.c index 742a4657e1..0cd31ec21a 100644 --- a/examples/protocols/https_request/main/https_request_example_main.c +++ b/examples/protocols/https_request/main/https_request_example_main.c @@ -75,8 +75,8 @@ static void https_get_task(void *pvParameters) while(1) { esp_tls_cfg_t cfg = { - .cacert_pem_buf = server_root_cert_pem_start, - .cacert_pem_bytes = server_root_cert_pem_end - server_root_cert_pem_start, + .cacert_buf = server_root_cert_pem_start, + .cacert_bytes = server_root_cert_pem_end - server_root_cert_pem_start, }; struct esp_tls *tls = esp_tls_conn_http_new(WEB_URL, &cfg);