esp32: Add ca_cert to ussl #5998
Conversation
- add granular control about which errors to accept.
|
In order to check expiration of certs on esp32 the following is required to be set:
Defaults to being not set ... There's a thread about this topic here |
|
Yup, can't check the expiration date of certs without knowing today's date... Note #5973, which switches the RTC to have a linux epoch, which I assume is required for mbedtls to get the correct date... I'll look into the details... |
|
Not that I have to say much in this, but to me it feels uncomfortable all checks being disabled by default. |
I totally agree! And I'm planning on submitting PRs to fix that, but with 13 PRs in the pipeline I'm waiting for something to make progress otherwise the reworking of additional PRs as the prior ones get modified adds a lot of unnecessary work. WRT to changing the default: what I would like to see is a way to address the root certs issue. I'm not familiar with how root cert stores work but ideally MP would ship with a small set of most common root certs in the flash/filesystem and the ability to add more. This way, you just say "use ssl" and it does what one would expect from a modern secure system. Now I'm sure reality will step in and the grand vision won't be practical, but then I'd like to work out how close we can get. |
From my perspective that could be an option in whatever way (compile option, its own package, etc.), but having it there by default I personally wouldn't like. While it might be "nice" for the folks doing IoT cloud solutions, e.g. connecting to a cloud mqtt broker to not care about the TLS setup and its trust chain, I'm usually rolling out my own trust chain and would expect that the trust anchors I set are the only ones. First thing I do when setting up a system for a dedicated purpose is kicking out all kinds of CAs from the system and the components shipping their own (Firefox at its best - you have to have them, best you can do is deactivation in a very cumbersome way). What I'm trying to say is, that the current use of x509 and the so many root CAs systems and users trust automatically is broken by design (DigiNotar, Norton, ..) and I'd rather have my own CA system I can trust, than some defaults I per definition can at least not trust /as much/. |
|
Haha, you're definitely at the paranoid end of the spectrum! Going that far is always an option. My goal is to make things reasonably secure by default, then someone who doesn't want security can turn it off (e.g. no cert validation), and you can replace the ca roots in your systems :-). |
|
@mirko Cpython provides the concept of creating ssl-contexts which can be used to wrap sockets. In order to create a context with sensible defaults there is the function I just realized that my current implementation moves away from cpython by providing custom flags to |
It's not necessarily the "insecure by default" (though I'm not a big fan of it either), but the "I actually provide and set SSL params like Obviously (again, maybe only to me) if I set |
|
@mirko, I totally agree b 8000 ut I'm unsure about how to proceed as cpython's behavior of wrap_socket is exactly that terrifying... |
While this might be good reasoning in other or even most cases I'd definitely vote for this not being one of them. |
This is a partial support as the CA certificate is not validated. This is so as this functionality is not implemented in all micropython ports. For example, it isn't currently supported in the esp32 port although a draft of the feature has been published: micropython/micropython#5998 As the CA functionality is not there, we cannot use this parameter do decide when to switch to SSL mode as done in the CPython version. Instead we enable SSL when connecting to port 443 or 8443 which are well known ports used for TLS communications. One more thing is that this library relies on short reads to get data from the socket as it always ask for the max buffer length when reading. In micropython, the interface provided for SSL socket is only the one of a stream which doesn't allow short reads. To go around this we change the socket to non blocking which which has the side-effect of allowing short reads. However we then need to do manual polling and timeout on the socket which we do here.
|
Hi, is there any update on this issue? I tried out the code as provided but by this pull but it seems not to work. In the code snippet below despite the fact I provide the correct certificate or one I modify to specifically make it raise an exception it prints out "Website verified". try: |
|
What's the test saying which comes with the PR? |
|
@mirko, I am not sure I understand your question. All I did was try out the PR and the result did not raise a exception for a modified ca_cert. Basically any ca_cert used for a host printed out "Website verified". Please elaborate what the you did since it works for you. |
Not quite sure what's unclear. What I'm asking is whether the test - which is part of this PR - does not what it is expected to do in your setup: https://github.com/micropython/micropython/blob/59fd31808ff5173258d2c235b9253859a6c462b6/tests/esp32/esp32_test_cert.py |
|
Hi, Are there any additional steps to do before the certificate validation will work? Additional PRs? |
|
That's the diff I use on top of the latest release: https://pb.nanl.de/show.php?id=45135&hash=15971716 |
|
@chtinnes or @kekimari could you provide a full snippet including the cert so I can test the behaviour? here is what I get when providing a "faulty cert": |
|
I've often found badssl.com useful for testing many various certificate verification scenarios on clients. |
|
@pumelo Nevermind, I got it working now. I think I had somehow screwed up the firmware on my board before. It works fine now. |
|
I am just wondering if it is possible now to send mqtt messages to gcp IoT from micropython firmware or this pr is the only obstacle to this? If so then my question is when this pr will be merged or something hampering the merge? |
|
@dmitrypotenko |
I know it's possible using Google's code, I've been doing it. What I wonder about is without verification, is it vulnerable to a MitM attack. I am unclear on that but I think it is. Here's Google's code. |
What would a not so greedy cipher be, do you have example code you could share? I got it working by the way on 1.15, an ESP32-WROVER-IE (has PSRAM), just hoping to avoid subtle gotchas--such as a greedy cipher. I did a diff against 1.15 and that code is slightly newer, something about error handling. Suppose I will need to diff patch both sets of changes into one new file, something which will have to wait for another day. |
|
Yes, if you don't do CA verification you're vulnerable against MITM attacks (and don't pin to the (fingerprint of the) server cert). |
- add granular control about which errors to accept. - Cherrypicking pull request micropython#5998
…N4R2 Espressif ESP32-S2 DevKitC-1-N4R2 board
|
Is anyone going to work on closing this issue out ? |
If the original author (@pumelo) doesn't disagree / wanna do it him-/herself, I'm happy to clean up the PR a bit, re-submit and push for feedback and merge. @pumelo any stand on this? |
|
@mirko of course, just go ahead. I'll be very happy if you would take this up and finish it. |
- Add ca_cert to ssl - add granular control about which errors to accept. all credits go to: pumelo see micropython#5998
|
I created now another pull request based on @pumelo's changes, the only major change from my side was to change the cert_reqs paramater to apply all checks by default. |
Add support to validate ca-certs in ussl.wrap_socket
Also add granular control about which errors to tolerate with the cert_reqs parameter which is an or-ed int of the errors which should NOT get tolerated. If you leaf cert_reqs default, all errors will be tolerated.
From my point of view the following things need to be done:
MBEDTLS_X509_BADCERT_FUTURE 0x0200error which in that case should have been raised.Let me know what you think.
I think this does address: #3687, #3646