esp-idf 4.4 esp-tls change causes esp-https-server to always prompt for client certificates, even when client cert verification is not configured (IDFGH-7047)

[sbytnar]

Environment

Development Kit: none
Module or chip used: ESP32-WROVER
IDF version : v4.4-12-ga0404a9ed2
Build System: idf.py
Compiler version: xtensa-esp32-elf-gcc (crosstool-NG esp-2021r2-patch3) 8.4.0
Operating System: Windows
(Windows only) environment type: WSL
Using an IDE?: No

Problem Description

A HTTPS server not configured to verify client certificates is now prompting for client certificates.

This change disrupts the normal HTTPS workflow for the web browser user that previous never prompted for a client certificate. The HTTPS server is not configured to verify client certificates, and this change basically asks the browser to prompt the user for a certificate. And, if a browser user chooses a cert, it’s ignored (the new callback that was added with this commit also isn’t hooked because prompting for certs is basically the new non-backward compatible feature that's been implemented). And, if the browser user doesn’t choose a cert, then the user is repeatedly asked for a client/peer certificate, which is extremely annoying.

Expected Behavior

When the HTTPS server is not configured to verify client certificates, the web browser does not prompt for a client certificate.

Actual Behavior

The HTTPS server prompts for a client certificate.

Steps to reproduce

The sample web server is sufficient to demonstrate this behavior.
However, the web browser computer must have at least one client certificate configured in their Keychain/Trusted Certificates. It doesn't matter what's in the cert as long as it is usable as a client certificate.

Code to reproduce this issue

The change that causes this problem:

diff --git a/components/esp-tls/esp_tls_mbedtls.c b/components/esp-tls/esp_tls_mbedtls.c
index fb6e10366a..f0764520ae 100644
--- a/components/esp-tls/esp_tls_mbedtls.c
+++ b/components/esp-tls/esp_tls_mbedtls.c
@@ -500,7 +500,7 @@ esp_err_t set_server_config(esp_tls_cfg_server_t *cfg, esp_tls_t *tls)
             return esp_ret;
         }
     } else {
-        mbedtls_ssl_conf_authmode(&tls->conf, MBEDTLS_SSL_VERIFY_NONE);
+        mbedtls_ssl_conf_authmode(&tls->conf, MBEDTLS_SSL_VERIFY_OPTIONAL);
     }
     if (cfg->servercert_buf != NULL && cfg->serverkey_buf != NULL) {

A fix is to revert this change. A better fix would allow MBEDTLS_SSL_VERIFY_OPTIONAL to be a requested option, and MBEDTLS_SSL_VERIFY_NONE is the default.

[sbytnar]

Thank you!

A review comment: some users may want this to be a runtime API flag/option, instead of a compile time option.