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

fix(esp-tls): make the wolfSSL backend send entire client certificate… (IDFGH-12621) #13618

Closed
wants to merge 2 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 25 additions & 2 deletions components/esp-tls/esp_tls_wolfssl.c
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ static esp_err_t esp_load_wolfssl_verify_buffer(esp_tls_t *tls, const unsigned c
wolf_fileformat = WOLFSSL_FILETYPE_ASN1;
}
if (type == FILE_TYPE_SELF_CERT) {
if ((*err_ret = wolfSSL_CTX_use_certificate_buffer( (WOLFSSL_CTX *)tls->priv_ctx, cert_buf, cert_len, wolf_fileformat)) == WOLFSSL_SUCCESS) {
if ((*err_ret = wolfSSL_CTX_use_certificate_chain_buffer_format( (WOLFSSL_CTX *)tls->priv_ctx, cert_buf, cert_len, wolf_fileformat)) == WOLFSSL_SUCCESS) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @frankencode Thanks for the PR.
Sorry for the really delayed review. I have confirmed that this change makes it same as mbedTLS.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No hurry. Good things take time.

return ESP_OK;
}
return ESP_FAIL;
Expand Down Expand Up @@ -288,6 +288,11 @@ static esp_err_t set_client_config(const char *hostname, size_t hostlen, esp_tls
free(use_host);
return ESP_ERR_WOLFSSL_SSL_SET_HOSTNAME_FAILED;
}
/* Mimic the semantics of mbedtls_ssl_set_hostname() */
if ((ret = wolfSSL_CTX_UseSNI(tls->priv_ctx, WOLFSSL_SNI_HOST_NAME, use_host, strlen(use_host))) != WOLFSSL_SUCCESS) {
ESP_LOGE(TAG, "wolfSSL_CTX_UseSNI failed, returned %d", ret);
return ESP_ERR_WOLFSSL_SSL_SET_HOSTNAME_FAILED;
}
free(use_host);
}

Expand All @@ -310,6 +315,24 @@ static esp_err_t set_client_config(const char *hostname, size_t hostlen, esp_tls
#endif /* CONFIG_WOLFSSL_HAVE_ALPN */
}

#ifdef CONFIG_WOLFSSL_HAVE_OCSP
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding this change. Where do you plan to add this CONFIG option ? Have you defined it in your own project?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have same real issues building wolfssl as esp-idf component. I have a bit of hacked setup with the crappy scripts that ship with wolfssl which wildly copy around files to create an esp-idf component... That said I also added this OCSP option to the esp-wolfssl package which is still referenced in the esp-tls documentation:
espressif/esp-wolfssl#24 . The good thing about this esp-wolfssl is that it builds wolfssl from github and allows to cleanly track the upstream. At some point someone probably has to update this package to point to the latest wolfssl version. The name of the option will definitely be CONFIG_WOLFSSL_HAVE_OCSP as HAVE_OCSP is also the name used internally in wolfssl's config headers.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh, okay I didn't notice this PR. Will see what can be done about the PR and updating the wolfSSL version

/* enable OCSP certificate status check for this TLS context */
if ((ret = wolfSSL_CTX_EnableOCSP((WOLFSSL_CTX *)tls->priv_ctx, WOLFSSL_OCSP_CHECKALL)) != WOLFSSL_SUCCESS) {
ESP_LOGE(TAG, "wolfSSL_CTX_EnableOCSP failed, returned %d", ret);
return ESP_ERR_WOLFSSL_CTX_SETUP_FAILED;
}
/* enable OCSP stapling for this TLS context */
if ((ret = wolfSSL_CTX_EnableOCSPStapling((WOLFSSL_CTX *)tls->priv_ctx )) != WOLFSSL_SUCCESS) {
ESP_LOGE(TAG, "wolfSSL_CTX_EnableOCSPStapling failed, returned %d", ret);
return ESP_ERR_WOLFSSL_CTX_SETUP_FAILED;
}
/* set option to use OCSP v1 stapling with nounce extension */
if ((ret = wolfSSL_UseOCSPStapling((WOLFSSL *)tls->priv_ssl, WOLFSSL_CSR_OCSP, WOLFSSL_CSR_OCSP_USE_NONCE)) != WOLFSSL_SUCCESS) {
ESP_LOGE(TAG, "wolfSSL_UseOCSPStapling failed, returned %d", ret);
return ESP_ERR_WOLFSSL_SSL_SETUP_FAILED;
}
#endif /* CONFIG_WOLFSSL_HAVE_OCSP */

wolfSSL_set_fd((WOLFSSL *)tls->priv_ssl, tls->sockfd);
return ESP_OK;
}
Expand Down Expand Up @@ -526,7 +549,7 @@ void esp_wolfssl_server_session_delete(esp_tls_t *tls)

esp_err_t esp_wolfssl_init_global_ca_store(void)
{
/* This function is just to provide consistancy between function calls of esp_tls.h and wolfssl */
/* This function is just to provide consistency between function calls of esp_tls.h and wolfssl */
return ESP_OK;
}

Expand Down
Loading