extmod/modussl_mbedtls: add support for PSK cipher suites (ESP32 only) #5544

tve · 2020-01-16T07:43:08Z

This PR proposes to add support for PSK cipher suites to ussl, specifically for the esp32 but it should be supportable for other platforms that use mbedtls as well (there is nothing esp32-specific in this PR except for enabling the cipher suites in the mbedtls config, which is port-specific).
The PSK cipher suites are awesome because they do away with all the cert and key stuff. Instead each client can get an id and hex-key and use that for bidirectional auth when connecting to the server. Very useful for MQTT, for example.
Docs can be read at: https://micropython-tve.readthedocs.io/en/tls-psk/library/ussl.html

tve · 2020-01-16T07:57:51Z

Looks like I need to add a feature-define to allow the PSK cipher suites to be enabled only on platforms that configure mbedtls appropriately.

dpgeorge · 2020-01-22T06:27:23Z

Thanks for the contribution.

The main things to discuss would be:

is it a useful (and secure?) feature to have for general purpose use?
how the API looks, whether it belongs to the ssl module or somewhere else (eg a new module like sslpsk).

Some background/prior art:

an old request for this feature in CPython: https://bugs.python.org/issue19084
implementation of PSK for CPython as a separate library: https://github.com/drbild/sslpsk

tve · 2020-01-22T17:15:11Z

Thanks for reviewing!

is it a useful (and secure?) feature to have for general purpose use?

"Useful" depends on the point of view. "Secure" depends on the usage. The reason I implemented this is that I'm sick of insecure IoT and I find the whole cert thing a PITA to use, which is why I believe very few people actually use it. In comparison PSK is much easier.

Looking at MicroPython specifically I would argue that it is insecure by default. Examples for this are the unencrypted webrepl, no checking of TLS certs on ESP32, and the pyboard tool using telnet. I'm trying to get to ta point where users can opt for encryption with no more overhead than assigning a name and password to each device (or group of devices).

Ease of use is why I like the PSK cipher suites: they are super-simple to manage at small scale, specifically with Mosquitto. All that's needed is a ident:key file and the keys are trivial to generate (feed anything random into md5sum). The result is two-way auth (both server auths client and client auths server), which means it's as secure as TLS with verified server cert and verified client cert.

I'm working on an MQTT repl (see https://forum.micropython.org/viewtopic.php?f=2&t=7596) which would allow me to fully interact with devices over a single secure authenticated network connection (that moreover is outbound from the device, so no "open ports" on the device).
If we extended the functionality to also support server sockets the webrepl could be made to use PSK and from a user perspective it would be no more difficult to set-up than today, e.g. setting up a password on first use. I don't think the in-browser client would work with TLS-PSK, however :-(.

The big downside to the PSK stuff is that not enough software supports it. Basically a chicken and egg problem. Also, the attitude evident in https://bugs.python.org/issue19084 which is that outside of HTTPS nothing matters ("it is not a priority for protocols such as HTTPS"). (Incidentally, that's the same issue encountered in getting PSK into Go.)

In terms of cryptographic security, TLS-PSK uses the same crypto as the PK stuff, just that the key exchange is different. I'm not a cryptographer, but from all I can tell if you use equivalent ciphers it comes out to a wash. The weakest point in both cases is key management, whether we're talking about PSK or certs/pk's.

Also worth noting is that the PSK crypto is supposedly a lot faster than PK (I say "supposedly" because I haven't measured myself). I wonder how it would change the usability of TLS on an esp8266 (dunno whether the SDK supports the PSK ciphers).

how the API looks, whether it belongs to the ssl module or somewhere else (eg a new module like sslpsk).

In favor of keeping it separate in MP are that there is no CPython API to emulate and that it keeps this "concern" separate. Against is that it's probably quite a bit of duplicated code and that it makes the use of the PSK stuff harder 'cause one can't just pass in the ssl_params into existing libraries. E.g., to use Peter's mqtt_as library I'd have to clone it and modify it to use a different module for its wrap_socket. This is also the issue I'm facing with paho-mqtt in CPython: I have to either clone and hack or subclass and copy-override the connect and reconnect methods to use sslpsk.

Hey, if I'm the only one in the community who wants this then I should just continue to use my fork and not bother you... I'm also happy to commit to writing a MicroPython-focused tutorial on how to set PSK up and use it with Mosquitto.

peterhinch · 2020-01-23T07:32:55Z

@tve Going by forum posts there is considerable interest in easily used, reliable, fast crypto. I would certainly wish to support it in mqtt_as.

Jongy · 2020-01-25T01:36:15Z

Personally, when I needed basic crypto for one of my IoT projects, it took me little to get sick of trying to use TLS, so I "downgraded" my crypto to AES-CTR + SHA256 HMAC with a pre-shared key (used over MQTT).

Being able to use the same scheme, over MQTT/HTTP, without adding extra code - that will be great!

Jongy · 2020-01-25T01:39:52Z

docs/library/ussl.rst

+
+   - The esp32 implementation does not support cert_reqs: it never validates certs!
+   - The esp32 implementation supports key-exchange and bidirectional authentication
+     using Pre-Shared Keys. Use KW options ``psk_ident=<identity hint>`` and


All other parameters of wrap_socket are irrelevant for PSK mode, are they? Why not add it as a separate function, then? This way you won't need all these new checks you've added in wrap_socket for parameters' correctness.

You are correct. However, the reason I added these params is libraries. For example, mqtt_as accepts an ssl_params dict and passes that straight through to wrap_socket, so the way I'm doing it allows the user to choose PSK without change to mqtt_as. What you describe would require forking mqtt_as to change the call to wrap_socket into something different.

I see. For your current use of it it's understandable, but I think that libraries should adapt themselves to the core, and not otherwise :)

Also, a simple Pythonic workaround until libraries are updated can be to have a ssl.py file containing a wrap_socket function that demuxes the call to ussl.wrap_socket or ussl.wrap_socket_psk based on the arguments.

I don't think it's realistic to expect each library to add code to distinguish the form of certificate and to me, a big plus of TLS-PSK is ease of use and that completely vanishes if you have to go fix libraries.

Hmm.

a big plus of TLS-PSK is ease of use

Agreed on this one. Well, at least it should be compile-time configurable, but that's a cleanup to be added later, as you said.

Jongy · 2020-01-25T01:50:34Z

docs/library/ussl.rst

+present an id/ident and typically the server allows a different key
+to be associated with each client id. In effect this is very similar
+to username and password pairs, except that unlike a password the
+key is not directly transmitted to the server, thus a connection to a


I hope that in today's times most services don't transmit passwords directly :) But instead only a salted hash or something similar.

The strength of TLS-PSK (unlike standard password authentication) is that the PSK (or any part/form of it) will never be compromised, so a malicious server talking to a valid client can't deduce any useful information about it. This should be noted here (it's one of the strengths of this mode, you don't need to care about malicious servers/clients either way)

Good point, will add!

Jongy · 2020-01-25T01:54:24Z

extmod/modussl_mbedtls.c

-    mbedtls_x509_crt_init(&o->cert);
-    mbedtls_pk_init(&o->pkey);
+    if (psk_mode) {
+        memset(&o->cacert, 0, sizeof(o->cacert));


Are these memsets needed? I believe mbedtls is smart enough to keep away from these objects when in PSK mode.

I ended up doing this to avoid having to add a "mode" field so the various places that call a series of mbedtls_..._free all know which structs were allocated and which not. I briefly looked at the MP heap allocator and I do not believe that it zeroes allocated memory, or at least there's a #define there, which means it's not guaranteed. And it wouldn't be good for the mbedtls free functions to be called on some random struct. I'm happy for suggestions if this too weird. (I guess at minimum it evidently needs a comment!)

It's a bad practice to call compound _free functions without initializing those objects first, anyway (whether it's zeros or not).

I think it's too weird and a mode variable should be added. (Or instead, use some inner variable of mbedtls, which probably keeps this state itself)

Jongy · 2020-01-25T01:55:17Z

extmod/modussl_mbedtls.c

@@ -148,7 +167,18 @@ STATIC mp_obj_ssl_socket_t *socket_new(mp_obj_t sock, struct ssl_args *args) {
        goto cleanup;
    }

-    mbedtls_ssl_conf_authmode(&o->conf, MBEDTLS_SSL_VERIFY_NONE);
+    if (psk_mode) {
+        const char *ident = mp_obj_str_get_str(args->psk_ident.u_obj);


Use mp_obj_str_get_data and remove the strlen.

Jongy · 2020-01-25T01:56:37Z

extmod/modussl_mbedtls.c

+        const char *ident = mp_obj_str_get_str(args->psk_ident.u_obj);
+        size_t key_len;
+        const byte *key = (const byte*)mp_obj_str_get_data(args->psk_key.u_obj, &key_len);
+        //if (key_len > MBEDTLS_PSK_MAX_LEN) {


Should probably keep this check and raise a ValueError, also for the ident, but it should be in the beginning of this function.

I don't remember why I left that in there. My intention was that this would be caught by mbedtls and hit line 251 ret == MBEDTLS_ERR_PK_BAD_INPUT_DATA.

mbedtls errors can be a bit obscure. Especially when it's directly related to user input, I think a concise exception is preferred.

agreed, but did you look at the code?

} else if (ret == MBEDTLS_ERR_PK_BAD_INPUT_DATA) { mp_raise_ValueError("invalid key");

this catches any issue with the key, not just "too long". It's a bit a balance between the number or different error messages and the space they end up using.

Haha I admit, looked only on your diff. That's fine. Just need to remove that comment.

tve · 2020-01-25T04:41:21Z

@Jongy Thanks for taking the time to review! This needs more work, I'm currently focusing on tools and background data (e.g. performance) so the whole concept becomes more attractive to others so they can chime in and say "hey, I'd like this feature merged".

tve · 2020-01-26T07:40:48Z

I did a first round of quick tests. This was on an esp32 with a tiny test pgm opening an MQTT connection to a mosquitto server running on an ODroid XU4 (rPi-4 type of performance):

mqtt unencrypted: 18ms
mqtt TLS-PSK: 101ms using TLS_PSK_WITH_AES_256_GCM_SHA384 (no PFS)
mqtt TLS-PKI: 1236ms using TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (has PFS), server uses a 2048 bit key

The code being timed is essentially:

s.connect((addr, port))
s = ssl.wrap_socket(s, <appropriate params>)
s.write(b"\x10\x10\x00\x04MQTT\x04\x02\x00\x3c\x00\x04hack")
ok = s.read(4) == b'\x20\x02\x00\x00'

I ran 10 iterations and took the median. The 10x improvement is noteworthy! However, I don't understand why a cipher without PFS (perfect forward secrecy) is selected in the TLS-PSK case, especially because I checked and both server and client announce that they do support appropriate ciphers. Note that in both cases the symmetric crypto is identical (AES_256_GCM_SHA384).

I then disabled the non-PFS ciphers for TLS-PSK and here's what I got:

mqtt TLS-PSK: 974ms using TLS_ECDHE_PSK_WITH_AES_256_CBC_SHA384 (has PFS)

So now the timing is comparable to the RSA cipher. Interesting...

I should add that the PSK versions include mutual auth, while the PKI test doesn't even include cert verification, never mind mutual auth (although I think in most cases auth would use a password exchange not client certs).

More tests to be done...

tve · 2020-01-28T05:56:29Z

I just ran the same performance test on the esp8266, for grins, using the ready-made v1.12 download image. Note that the esp8266 uses axTLS, not mbedTLS.

mqtt unencrypted: 282ms
mqtt TLS-PKI: 622ms using cipher suite TLS_RSA_WITH_AES_256_CBC_SHA256 (no PFS)
"interesting..."
maybe I should try TLS-PKI on the esp32 with the PFS cipher suites disabled to see how that compares.

tve · 2020-04-09T01:30:54Z

I'm closing this PR for a couple of reasons:

using PSK ciphers is an uphill battle because so little software supports it; every step of the way it's a PITA to find some obscure patch somewhere
if one picks a cipher suite with PFS there is no performance advantage to the PSK ciphers, without PFS it's probably possible to find a low-security PKI cipher suite that takes less time, but that's probably a poor decision
it's less work to get a cipher from let's encrypt (or to use self-signed certs) than to battle getting PSK into software, using the DNS challenge with acme-dns allows even devices without internet exposure to renew certs
CPython doesn't support PSK ciphers, the unix port of micropython uses axtls: more hassles

So the bottom line is that I decided to throw in the towel, go with PKI and focus my energy elsewhere :-). If someone else is interested in picking up the PR, it's still all in github...

edwin-oetelaar · 2021-10-10T12:45:32Z

extmod/modussl_mbedtls.c

-    mbedtls_pk_free(&o->pkey);
-    mbedtls_x509_crt_free(&o->cert);
-    mbedtls_x509_crt_free(&o->cacert);
+    if (psk_mode) {


looks like an inversion : if PSK we do NOT need to free the CERTS since they are uninitialized.

bitmaptools: add alphablend

tve added 2 commits January 15, 2020 23:25

extmod/modussl_mbedtls: add support for PSK cipher suites (ESP32 only)

476e4f3

extmod/modussl_mbedtls: fix docs

16befdc

dpgeorge added the extmod Relates to extmod/ directory in source label Jan 22, 2020

Jongy suggested changes Jan 25, 2020

View reviewed changes

Carglglz mentioned this pull request Jan 28, 2020

esp32: SSL handshake fails in latest build: esp32-idf3-v1.12-68-g3032ae115 on 2020-01-15 #5543

Closed

tve closed this Apr 9, 2020

edwin-oetelaar reviewed Oct 10, 2021

View reviewed changes

tannewt added a commit to tannewt/circuitpython that referenced this pull request Nov 12, 2021

Merge pull request micropython#5544 from jepler/alphablend

bb71f8c

bitmaptools: add alphablend

dpgeorge mentioned this pull request Jul 15, 2024

Adds TLS-PSK support to Python SSL context #14396

Open

Uh oh!

extmod/modussl_mbedtls: add support for PSK cipher suites (ESP32 only) #5544

extmod/modussl_mbedtls: add support for PSK cipher suites (ESP32 only) #5544

Uh oh!

Conversation

tve commented Jan 16, 2020

Uh oh!

tve commented Jan 16, 2020

Uh oh!

dpgeorge commented Jan 22, 2020

Uh oh!

tve commented Jan 22, 2020

Uh oh!

peterhinch commented Jan 23, 2020

Uh oh!

Jongy commented Jan 25, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tve Jan 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Jongy Jan 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tve commented Jan 25, 2020

Uh oh!

tve commented Jan 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tve commented Jan 28, 2020

Uh oh!

tve commented Apr 9, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tve Jan 25, 2020 •

edited

Loading

Jongy Jan 25, 2020 •

edited

Loading

tve commented Jan 26, 2020 •

edited

Loading