From: David Cermak Date: Fri, 2 Aug 2019 07:20:02 +0000 (+0200) Subject: esp-tls: Naming variables refering to certificates and keys in a neutral way to sugge... X-Git-Tag: v4.1-dev~30^2 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=25dd5e39afba5f66fe571ed235270b3972223679;p=esp-idf 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 --- 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);