-
-
Notifications
You must be signed in to change notification settings - Fork 8.2k
extmod/modussl_mbedtls: Wire in support for DTLS #10062
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Note that I only enabled it on the esp32 port so far. The size increase of the |
fa2ad97
to
ba0772f
Compare
extmod/modussl_mbedtls.c
Outdated
@@ -187,7 +230,7 @@ STATIC mp_obj_ssl_socket_t *socket_new(mp_obj_t sock, struct ssl_args *args) { | |||
|
|||
ret = mbedtls_ssl_config_defaults(&o->conf, | |||
args->server_side.u_bool ? MBEDTLS_SSL_IS_SERVER : MBEDTLS_SSL_IS_CLIENT, | |||
MBEDTLS_SSL_TRANSPORT_STREAM, | |||
args->dtls.u_bool ? MBEDTLS_SSL_TRANSPORT_DATAGRAM : MBEDTLS_SSL_TRANSPORT_STREAM, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to check the underlying socket type (SOCK_DGRAM
) instead of adding a new variable for dtls?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @stigbjorlykke after looking into this a bit, I don't believe that it is possible. DTLS lives on top of UDP, so the SOCK_DGRAM type doesn't have that context needed.
Hey all! I would really like to see this change go through and I'm happy to help jump in and push this through. @stigbjorlykke is your comment a requirement before we merge this or is it a suggestion we are ok with accepting for now? Are there any other changes needed by the reviewers? |
This is not a requirement, it's a suggestion for a better API. The socket already know it's type, and adding an extra (superfluous) variable makes it easier to write faulty code. I have no idea how easy it is to check this, which is why I ask. |
I've looked into the comment and I don't believe that the socket does know it's DTLS type, so I believe the extra variable is required. Are you looking at an additional set of the defined SOCK_DGRAM types @stigbjorlykke? Any help to get this merged as soon as possible would really help out our project :) Thanks! |
Any update on this? |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #10062 +/- ##
==========================================
- Coverage 98.36% 98.33% -0.04%
==========================================
Files 159 159
Lines 21088 21106 +18
==========================================
+ Hits 20743 20754 +11
- Misses 345 352 +7 ☔ View full report in Codecov by Sentry. |
Code size report:
|
b81b204
to
69aefcf
Compare
Signed-off-by: Damien Tournoud <damien@platform.sh>
This is an automated heads-up that we've just merged a Pull Request See #13763 A search suggests this PR might apply the STATIC macro to some C code. If it Although this is an automated message, feel free to @-reply to me directly if |
Hey @damz are you still able to update this for the static keyword change that @projectgus mentioned above? If not, I can fork and take this on, as I still think this would be a big win to merge upstream as we use it every day in our projects. |
What else are we waiting on to merge this in @stigbjorlykke or any other maintainers? I haven't seen any concrete feedback and I would like to help get this over the finish line. |
I created #15764 which makes the updates to the static keyword and handles a file renaming that occurred in the upstream repo. |
This commit enables support for DTLS, i.e. TLS over datagram transport protocols like UDP. While support for DTLS is absent in CPython, it is worth supporting it in MicroPython because it is the basis of the ubiquitous CoAP protocol, used in many IoT projects. To select DTLS, a new set of "protocols" are added to SSLContext: - ssl.PROTOCOL_DTLS_CLIENT - ssl.PROTOCOL_DTLS_SERVER If one of these is set, the library assumes that the underlying socket is a datagram-like socket (i.e. UDP or similar). Our own timer callbacks are implemented because the out of the box implementation relies on `gettimeofday()`. This new DTLS feature is enabled on all ports that use mbedTLS. This commit is an update to a previous PR micropython#10062. Addresses issue micropython#5270 which requested DTLS support. Signed-off-by: Keenan Johnson <keenan.johnson@gmail.com>
…vi-rebased Add supervisor.runtime.display & RP2350 DVI autoconfig (rebased version)
Yes thank you so much to @damz for the original PR, and @dpgeorge and @projectgus for the help reviewing! It's been a long time in the making but hopefully this is of use to many people! It's already of great use to me and my project! |
This commit enables support for DTLS, i.e. TLS over datagram transport protocols like UDP. While support for DTLS is absent in CPython, it is worth supporting it in MicroPython because it is the basis of the ubiquitous CoAP protocol, used in many IoT projects. To select DTLS, a new set of "protocols" are added to SSLContext: - ssl.PROTOCOL_DTLS_CLIENT - ssl.PROTOCOL_DTLS_SERVER If one of these is set, the library assumes that the underlying socket is a datagram-like socket (i.e. UDP or similar). Our own timer callbacks are implemented because the out of the box implementation relies on `gettimeofday()`. This new DTLS feature is enabled on all ports that use mbedTLS. This commit is an update to a previous PR micropython#10062. Addresses issue micropython#5270 which requested DTLS support. Signed-off-by: Keenan Johnson <keenan.johnson@gmail.com>
What is this PR trying to solve?
This PR enables support for DTLS (i.e. TLS over datagram transport protocols like UDP). While support for DTLS is absent in cpython, it is worth supporting it in micropython because it is the basis of the ubiquitous CoAP protocol, used in many IOT projects. See #5270
How does this PR solve the problem?
A new set of "protocols" are added to
SSLContext
:ssl.PROTOCOL_DTLS_CLIENT
ssl.PROTOCOL_DTLS_SERVER
They act as you expect. If one of these is set, the library assumes that the underlying socket is a datagram-like socket (i.e. UDP or similar).
Implement our own timer callbacks as the out of the box implementation relies on
gettimeofday()
.TODO: To fully support asyncio for DTLS socket, we will need to return a readable or writable event in
poll(MP_STREAM_POLL, ...)
if_mbedtls_timing_get_delay(self) >= 1
. This is left for future work so as not to interfere with #9871.