clientcert never initialized (IDFGH-5771)

[dannybackx]

Environment

• Development Kit: any (esp32 lolin lite)
• Kit version (for WroverKit/PicoKit/DevKitC): N/A
• Module or chip used: ESP32
• IDF version (run git describe --tags to find it): esp-idf-v4.3
• Build System: idf.py
• Compiler version (run xtensa-esp32-elf-gcc --version to find it): xtensa-esp32-elf-gcc (crosstool-NG esp-2020r3) 8.4.0
• Operating System: Linux
• (Windows only) environment type: N/A
• Using an IDE?: No
• Power Supply: USB

Problem Description

When establishing a client-server connection via esp-https-server (so esp32 runs a web server), it's not possible to inspect the client certificate of the https connection.

That's because the certificate is never copied to the clientcert field in the esp-tls layer.

//Detailed problem description goes here.

Suggested fix : do this at the end of esp_mbedtls_server_session_create() by calling mbedtls_ssl_get_peer_cert().

I would give a more detailed diff but I don't see exactly how to copy a mbedtls_x509_crt structure. My current proof of concept has this code :

  mbedtls_ssl_context *ssl = &tls->ssl;
  mbedtls_x509_crt *cert = mbedtls_ssl_get_peer_cert(ssl);
  tls->clientcert = *mbedtls_ssl_get_peer_cert(ssl);

but the last line is obviously wrong as it doesn't get allocation right, and my sample code then crashes on memory allocation code (mbedtls_x509_crt_free is in the panic call stack).

Expected Behavior

For the clientcert field not to be unused in the entire code base

Actual Behavior

The clientcert field only has code that frees it if non-zero.

Code to reproduce this issue

Is rather large, sorry.

https://sourceforge.net/p/emptyesp32/code/HEAD/tree/https/

[AdityaHPatwardhan]

Hi @dannybackx, by default the server is used with MBEDTLS_SSL_VERIFY_NONE option. In that case no peer certificate is asked by the server and thus the output of mbedtls_ssl_get_peer_cert(ssl) is always NULL.

When the appropriate CA certificate is provided in the server configuration for client authentication. Then internally the client certificate obtained at the time of handshake (peer certificate) is verified against the provided CA certificate.

I agree the clientcert field is unused in the entire code-base in case of the server. But it is used in case of client. and since the esp-tls server and client share the same esp_tls_t structure, It remains unused in case of server.

For the code that you have mentioned to work we will have to create a pointer of the type mbedtls_x509_crt *clientcert_ptr and then set this ptr with the output of the mbedtls_ssl_get_peer_cert(ssl).

Is there any reason/requirement in you code for inspecting the peer certificate ?

[dannybackx]

I want access to the certificate for authentication.

[AdityaHPatwardhan]

Hi @dannybackx, I am not sure why you want to access the certificate for authentication when you can just tell mbedtls to do the authentication for you, without having to worry about the peer cert.

Anyways,
Even if the code change is made according to your suggestion. I am not sure how you will be able to access the esp_tls_t structure from esp_https_server, as it is internally handled and I think the TLS context for a particular session is currently not exposed publicly.

[dannybackx]

Maybe I should have said authorization.

As far as I know the builtin mechanism, even with mutual authentication, will only assess that both parties in the communication have valid certificates. I need to go a step further.

[dannybackx]

Also my sample code above shows that mbedtls_ssl_get_peer_cert() exposes the peer certificate. So it is available, it just needs to be extracted by the esp_https_server.

[AdityaHPatwardhan]

Hi @dannybackx
I have prepared a patch, which allows you to access the peer certificate when a new ssl connection is formed.
Please take a look
get_peer_certificate.patch.zip

[dannybackx]

This works, thanks.
Is there any chance to merge this with the general branch ?

[dannybackx]

You also need

hp: {27} diff components/esp_https_server/include/esp_https_server.h~ components/esp_https_server/include/esp_https_server.h
125a126
>     .user_cb = 0,                                 \

[AdityaHPatwardhan]

@dannybackx Yes, thanks for the suggestion. Yes we plan to merge it the master and then it would a part of the upcoming release. ( It would not be a part of already released branches, considering its a new feature).

[dannybackx]

Perfect, thanks

[dannybackx]

Pff reported in August. Four months later, still not in the newest release.

[AdityaHPatwardhan]

Hi @dannybackx Please check that the respective user callback has already been added in the esp_https_server.