certificate chain verification bug in esp-idf

jerryneedell commented 4 years ago

the following request works OK with and ESP32 with the NINA firmware, but not with the esp32s2 native wifi


response = requests.get("https://api.thingspeak.com/channels/1417/feeds.json?results=1")
 fails with 
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "wifi_cheer.py", line 39, in <module>
  File "adafruit_requests.py", line 507, in get
  File "adafruit_requests.py", line 456, in request
  File "adafruit_requests.py", line 405, in _get_socket
  File "adafruit_requests.py", line 401, in _get_socket
OSError: Failed SSL handshake
>>>

jerryneedell commented 4 years ago

If you can provide some guidance on how/where this needs to be done, I'll be happy to try.

tannewt commented 4 years ago

These are the API docs where I would start: https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-reference/protocols/esp_crt_bundle.html

@brentru might have experience with it for nina.

jerryneedell commented 4 years ago

Slowly trying to make some sense of this the way certificates are configured in the sdconfig is quite different for the ESP32S2 https://github.com/adafruit/circuitpython/blob/main/ports/esp32s2/esp-idf-config/sdkconfig.defaults#L548 vs NINA https://github.com/adafruit/nina-fw/blob/master/sdkconfig#L563

It is also not clear to me how to update the certificates used by the ids in the esp32s2 in the Docs it says the certificates used are quite old - Jan 2019 -- and the ones in the NINA build are much newer

The bundle comes with the complete list of root certificates from Mozilla’s NSS root certificate store. Using the gen_crt_bundle.py python utility the certificates’ subject name and public key are stored in a file and embedded in the ESP32 binary.

When generating the bundle you may choose between:

The full root certificate bundle from Mozilla, containing more than 130 certificates. The current bundle was updated Wed Jan 23 04:12:09 2019 GMT.
A pre-selected filter list of the name of the most commonly used root certificates, reducing the amount of certificates to around 35 while still having around 90 % coverage according to market share statistics.
In addition it is possible to specify a path to a certificate file or a directory containing certificates which then will be added to the generated bundle.

It also indicates that one can specify a file when building so I think that is the route to examine ...

askpatrickw commented 4 years ago

I looked into this last week for a hot second and while I could generate the updated bundle

/code/github/circuitpython/ports/esp32s2/esp-idf/components/mbedtls/esp_crt_bundle python gen_crt_bundle.py --input cacrt_all.pem

The build didn't seem to act any different despite the build flags being ON to use the bundle.

/circuitpython/ports/esp32s2/esp-idf-config/sdkconfig.defaults

#
# Certificate Bundle
#
CONFIG_MBEDTLS_CERTIFICATE_BUNDLE=y
CONFIG_MBEDTLS_CERTIFICATE_BUNDLE_DEFAULT_FULL=y
# CONFIG_MBEDTLS_CERTIFICATE_BUNDLE_DEFAULT_CMN is not set
# CONFIG_MBEDTLS_CERTIFICATE_BUNDLE_DEFAULT_NONE is not set
# CONFIG_MBEDTLS_CUSTOM_CERTIFICATE_BUNDLE is not set
# end of Certificate Bundle

Not an answer, but maybe that points you in the right direction

jerryneedell commented 4 years ago

what version of cacrt_all.pem did you use?

jerryneedell commented 4 years ago

Or I should have asked, what process did you use to get the updated certificate files.

askpatrickw commented 4 years ago

I ran that python script from this folder circuitpython/ports/esp32s2/esp-idf/components/mbedtls/esp_crt_bundle

esp-idf/components/mbedtls/esp_crt_bundle on  master via 🐍 system took 2s
➜ python gen_crt_bundle.py --input cacrt_all.pem

gen_crt_bundle.py: Parsing certificates from cacrt_all.pem
gen_crt_bundle.py: Successfully added 135 certificates
gen_crt_bundle.py: Successfully added 135 certificates in total

And then you end up with a new x509_crt_bundle

➜ gst
HEAD detached at 8bc19ba89
Untracked files:
  (use "git add <file>..." to include in what will be committed)

    x509_crt_bundle

jerryneedell commented 4 years ago

Looking over the build process, it looks like the certificates are being updated as part of the build process so I'm to sure this is the actual cause of the SSL handshake errors

If you look in the board build folder after completing a build, this is a file named x509_crt_bundle.S generated and I think this contain the certificate updates from the process noted above. It is run for every build.

tannewt commented 4 years ago

@jerryneedell How does the list of root certs compare to those in nina-fw? The connection issues might actually be due to the clock not being set. Checking the debug logging may provide more insight.

anecdata commented 4 years ago

This looks site-specific, so I think cert-related. I had never encountered the SSL handshake issue with my own server (HTTPS/TLS), but I can replicate with the link above. Doesn't matter if the RTC is 2000 or 2020.

Saola 1 w/Wrover with ESP32S2
6.0.0-rc.0 on 2020-10-16
adafruit-circuitpython-bundle-6.x-mpy-20201101
Requests 1.7.2

BennyE commented 4 years ago

@askpatrickw I see that your build includes 135 certificates, which is the archive from 2019 that was updated a number of times since 2019. You can see this e.g. here https://curl.haxx.se/docs/caextract.html where also a version from 14.10.2020 is available. I don't yet have a build setup, otherwise I would give it a try to update this.

BennyE commented 4 years ago

I set up a CPY build environment on Win10 WSL and, after a long night of disappointments, I could compile featherS2 UF2 builds this morning (actually noon + coffee). Unfortunately, even two different attempts of setting different certificate bundle options in menuconfig didn't lead to success. https to api.github.com & httpbin.org is OK; https to api.thingspeak.com & io.adafruit.com is NOK.

menuconfig

I'm positive of having 138 certificates bundled in, so the "2020 October pack". I set RTC to be accurate to year/month/day and roughly the time as well.

I believe it would be good to offer a way to "ignore invalid certificates" - at least for laboratory and home-lab use. (Edit: Trying to include some code for reference didn't go as expected - sorry) Full code in this repository: https://github.com/BennyE/stellar-dynpsk/blob/master/stellar-dynpsk.py <- look for urlib on ignoring invalid certs and silence warnings as well as "verify=" parameter for requests.

tannewt commented 4 years ago

Ok, I've gotten the connection to work but I'm not sure what the best fix is currently. It looks like the issue is that the root certificate is signed with SHA-1 which mbedtls doesn't support by default. I'm digging into why the nina-firmware doesn't have the same issue.

io.adafruit.com and api.thingspeak.com both use DigiCert root CA's that are fingerprinted with SHA-1.

jepler commented 4 years ago

probably it's been noted before, but the certs for io.adafruit.com and api.thingspeak.com are both wildcards

jepler commented 4 years ago

but www.adafruit.com is also a wildcard and it works, which casts doubt on the wildcard theory.

Both io.adafruit.com (test result) and api.thingspeak.com (test result) get a warning on ssllabs: "Chain issues: Contains anchor". www.adafruit.com (test result) does NOT get this message. (links go to ssllabs test results, not to the named sites)

This may be another red herring, as it's just a warning and shouldn't affect certificate validation: https://qualys-secure.force.com/discussions/s/article/000003197

When scanning through SSL Labs, it shows "Chain issues Contains anchor"

It means that you have added Intermediate as well as Root CA, when you only need the Intermediate as the client will already have Root CA (will be already trusted by browser in browser certificate store).

You can get the same info using openssl command:

echo "" | openssl s_client -connect test.stomt.com:443 2>&1 | grep -A 6 "Certificate chain"

It's not an issue in the sense that the anchor is not allowed, but that the extra certificate (which serves no purpose) is increasing the handshake latency.

Because of TCP slow start, the first bytes on a connection are the slowest. Hence, you can minimize the size of the handshake so that HTTP bytes can start flowing as soon as possible. So the issue is not so much "can the extra certificate fit into the initial window" (it most likely can, even with the old setting of 3 network segments), but "what other, more useful, data could we be sending instead". However, there is no security risk with "Contains anchor", you can largely ignore the "Contains Anchor" warning. Fixing it would possibly save bandwidth slightly and increase the performance.

BennyE commented 4 years ago

@tannewt the SHA-1 "theory" is something that I reviewed, but made a mistake by not looking at the same detail for the Root CA. Thank you for pointing this out. I'd like to suggest to make a parameter available that hands the option to the user to not verify the certificate. I say that not because I don't want to fix the original issue, but often in "homelab" you'll have self-signed certificates that otherwise will create an almost "impossible to fix" entry-level-barrier to new users - unless this case is somehow already handled.

That would be an example on how to make the verify optional (we'd need a parameter): https://github.com/espressif/esp-idf/blob/357a2776032299b8bc4044900a8f1d6950d7ce89/examples/protocols/https_mbedtls/main/https_mbedtls_example_main.c#L128

jepler commented 4 years ago

diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h
index f7e55aef5..aa8a28aeb 100644
--- a/include/mbedtls/config.h
+++ b/include/mbedtls/config.h
@@ -3319,7 +3319,7 @@
  *            on it, and considering stronger message digests instead.
  *
  */
-// #define MBEDTLS_TLS_DEFAULT_ALLOW_SHA1_IN_CERTIFICATES
+#define MBEDTLS_TLS_DEFAULT_ALLOW_SHA1_IN_CERTIFICATES

 /**
  * Allow SHA-1 in the default TLS configuration for TLS 1.2 handshake

Indeed, this change (in a submodule of esp-idf) allows the esp32s2 to connect to io.adafruit.com.

There are at least 3 possible scenarios here (since @BennyE raises the issue of "homeservers"):

self-signed (snakeoil/homeserver) certificates. These servers would also be identified as unsafe by standard web browsers. They do not necessarily use SHA1 hashes. The change above would not allow them to work.
certificates which use SHA1 hashes in the chain to a trusted certificate. I think these servers would be identified as unsafe by standard web browsers. The change above would probably allow them to work.
certificates which use high quality hashes in the chain to a trusted certificate, but due to possible misconfiguration include further certificates in the chain wihch use SHA1 hashes. These servers are NOT identified as unsafe by standard web browsers. The change above allows them to work, but also allows the possibly unsafe category above to work as well. This is what causes io.adafruit.com to fail, I think.

So we COULD figure out the "right" way to allow SHA1, but while it will solve the io.adafruit.com problem it allows more than it should, and it's not a fix for "homeservers".

mbedtls can be built for host computers. When I do so, and test it on io, it succeeds. This uses the host computer's certificate store:

$ ./programs/ssl/ssl_client2  server_name=io.adafruit.com server_port=443 ca_path=/etc/ssl/certs/ request_page=/ allow_sha1=0
…
profile->allowed_mds = f0
MBEDTLS_X509_ID_FLAG( md_alg )= 20
find_parent -> 0  parent_is_trusted=0 signature_is_good=1
md_alg=0x6
profile->allowed_mds = f0
MBEDTLS_X509_ID_FLAG( md_alg )= 20
find_parent -> 0  parent_is_trusted=1 signature_is_good=1

Compare to similar debug statements on esp32s2, "parent_is_trusted" is not true for the second step, so it proceeds to the third step:

W (9956) mbedtls: mbedtls_x509_crt_verify_restartable() yo
W (9956) mbedtls: child is trusted? 0
W (9956) mbedtls: md_alg=0x6
W (9956) mbedtls: profile->allowed_mds = f0
W (9956) mbedtls: MBEDTLS_X509_ID_FLAG( md_alg )= 20
W (9976) mbedtls: find_parent -> 0 parent@0x3fff1908 parent_is_trusted=0 signature_is_good=1

W (9976) mbedtls: child is trusted? 0
W (9986) mbedtls: md_alg=0x6
W (9986) mbedtls: profile->allowed_mds = f0
W (9996) mbedtls: MBEDTLS_X509_ID_FLAG( md_alg )= 20
W (10016) mbedtls: find_parent -> 0 parent@0x3fff2168 parent_is_trusted=0 signature_is_good=1

W (10016) mbedtls: child is trusted? 0
W (10016) mbedtls: md_alg=0x4
W (10016) mbedtls: profile->allowed_mds = f0
W (10026) mbedtls: MBEDTLS_X509_ID_FLAG( md_alg )= 8
W (10026) mbedtls: find_parent -> 0 parent@0x0 parent_is_trusted=0 signature_is_good=0

W (10036) mbedtls: MBEDTLS_X509_BADCERT_NOT_TRUSTED
W (10046) mbedtls: x509_crt_verify_chain -> 0x0 ee_flags = 0x0 chain flags = 0x0
W (10056) mbedtls: chain len=3 f_vrfy@0x400f2138
W (10056) mbedtls: chain[3] BEFORE cur_flags=0x4008 flags=0x0

W (10066) mbedtls: chain[3] AFTER cur_flags=0x4008 flags=0x4008

W (10076) mbedtls: chain[2] BEFORE cur_flags=0x0 flags=0x4008

W (10076) mbedtls: chain[2] AFTER cur_flags=0x0 flags=0x4008

W (10086) mbedtls: chain[1] BEFORE cur_flags=0x0 flags=0x4008

W (10086) mbedtls: chain[1] AFTER cur_flags=0x0 flags=0x4008

W (10096) mbedtls: x509_crt_merge_flags_with_cb -> 0x0 flags=0x4008

W (10106) mbedtls: ssl->session_negotiate->verify_result = 0x4008
W (10116) mbedtls: ssl_tls.c:5813 x509_verify_CLOWNS() returned -9984 (-0x2700)

With the SHA1 signatures allowed, we get:

W (31296) mbedtls: mbedtls_x509_crt_verify_restartable() yo
W (31296) mbedtls: child is trusted? 0
W (31296) mbedtls: md_alg=0x6
W (31296) mbedtls: profile->allowed_mds = f8
W (31306) mbedtls: MBEDTLS_X509_ID_FLAG( md_alg )= 20
W (31326) mbedtls: find_parent -> 0 parent@0x3fff28c4 parent_is_trusted=0 signature_is_good=1

W (31326) mbedtls: child is trusted? 0
W (31336) mbedtls: md_alg=0x6
W (31336) mbedtls: profile->allowed_mds = f8
W (31336) mbedtls: MBEDTLS_X509_ID_FLAG( md_alg )= 20
W (31346) mbedtls: find_parent -> 0 parent@0x0 parent_is_trusted=0 signature_is_good=0

W (31356) mbedtls: MBEDTLS_X509_BADCERT_NOT_TRUSTED
W (31356) mbedtls: x509_crt_verify_chain -> 0x0 ee_flags = 0x0 chain flags = 0x0
W (31366) mbedtls: chain len=2 f_vrfy@0x400f2138
W (31376) mbedtls: chain[2] BEFORE cur_flags=0x8 flags=0x0

I (31396) esp-x509-crt-bundle: Certificate validated
W (31396) mbedtls: chain[2] AFTER cur_flags=0x0 flags=0x0

W (31396) mbedtls: chain[1] BEFORE cur_flags=0x0 flags=0x0

W (31406) mbedtls: chain[1] AFTER cur_flags=0x0 flags=0x0

W (31406) mbedtls: x509_crt_merge_flags_with_cb -> 0x0 flags=0x0

So on a host computer, "is trusted" is verified from within x509_crt_verify_chain and so a SHA1 signed certificate further up the chain is not a problem. On hardware, "is trusted" is NOT verified within x509_crt_verify_chain, but in esp-x509-crt-bundle, which leads to different handling of the certificate.

int esp_crt_verify_callback(void *buf, mbedtls_x509_crt *crt, int data, uint32_t *flags)
{
    mbedtls_x509_crt *child = crt;

    if (*flags != MBEDTLS_X509_BADCERT_NOT_TRUSTED) {
        return 0;
    }

The only flag that esp_crt_verify_callback will "fix" is BADCERT_NOT_TRUSTED (0x8). It won't fix MBEDTLS_X509_BADCRL_BAD_MD.

I think the esp-idf's approach of not providing a certificate store but attempting to fake do it in the verify callback is flawed. If we don't want to fix esp-idf ourselves enabling sha1 certificates is probably the next best thing.

tannewt commented 4 years ago

I'm confused why you are looking in esp-x509-crt-bundle. My understanding is that code is for generating the cert bundle. The esp-tls component is doing the connection management with mbedtls.

tannewt commented 4 years ago

@jepler Want to switch circuitpython to your IDF branch until it is integrated upstream? I think this is the right fix.

krittick commented 4 years ago

Hi, I think I'm still getting this bug today, even after updating my board (Feather S2) to the newest firmware from S3 and the 20201117 library bundle. Issue happens even if I use the firmware directly referenced in the commit for this fix (f8eed1f).

Interestingly, the examples from this bug (thingspeak.com, io.adafruit.com) do appear to work properly now, but my own domain does not. I'm suspecting that it's an issue with my SSL certificate chain based on the SSL Labs report, but I'm not entirely sure there's anything I can do on my end to fix it.

My code:

import wifi
import socketpool
import adafruit_requests
import ssl

for network in wifi.radio.start_scanning_networks():
    print(network, network.ssid, network.rssi, network.channel)

wifi.radio.stop_scanning_networks()

try:
    wifi.radio.connect("SSID_here", "psk_here")
except Exception as e:
    print("Unable to connect to wifi! Error: " + str(e))

pool = socketpool.SocketPool(wifi.radio)
request = adafruit_requests.Session(pool, ssl.create_default_context())
response = request.get("https://toke.haus")
print(response.status_code)
print(response.text)

print("done")

Result:

Traceback (most recent call last):
  File "code.py", line 18, in <module>
  File "adafruit_requests.py", line 594, in get
  File "adafruit_requests.py", line 572, in request
  File "adafruit_requests.py", line 441, in _get_socket
RuntimeError: Sending request failed

Edit: I updated my cert to a LetsEncrypt cert and it now works fine. I'm assuming my SSL cert provider was the cause of the issue and won't be using them again. Might still be worth looking into as a potential blocker for others? FWIW, my cert was a PositiveSSL cert purchased from namecheap.com (issuer was sectigo).

adafruit / circuitpython

certificate chain verification bug in esp-idf #3424