Skip to content
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

Add minimal support for ssl module using Mbed TLS and sockets #929

Merged
merged 1 commit into from
Nov 6, 2023

Conversation

pguyot
Copy link
Collaborator

@pguyot pguyot commented Nov 5, 2023

Details of changes:

  • Add support for ssl client in binary and passive modes, with no certificate verification
  • Add APIs to otp_socket so it can be called from ssl bio callbacks
  • Fix a bug in lwIP otp_socket's recv revealed by ssl tests
  • Fix a bug in BSD otp_socket's recvfrom revealed by refactoring
  • Fix a bug in esp32 tests where main context and its resources were not properly destroyed
  • Update documentation and workflows to reflect the requirement on Mbed TLS
  • Fix exported types of inet module

This code was tested on:

  • Pico-W
  • ESP32 using ESP-IDF 5.1 release branch
  • Unix (macOS)

using atomvm_netbench associated test.

The test takes 0.5s with Erlang/OTP or AtomVM on macOS.
It takes 7.3s then 1.0s on ESP32
It takes 12.0s then from 2.2s to 2.6s on Pico-W

These changes are made under both the "Apache 2.0" and the "GNU Lesser General
Public License 2.1 or later" license terms (dual license).

SPDX-License-Identifier: Apache-2.0 OR LGPL-2.1-or-later

@pguyot pguyot force-pushed the w44/ssl-over-socket-with-mbedtls branch 2 times, most recently from 54d78d2 to 74bb97e Compare November 5, 2023 22:38
src/platforms/rp2040/src/lib/CMakeLists.txt Outdated Show resolved Hide resolved
CMakeModules/MbedTLS.cmake Outdated Show resolved Hide resolved
CMakeModules/MbedTLS.cmake Outdated Show resolved Hide resolved
src/libAtomVM/otp_socket.h Outdated Show resolved Hide resolved
src/platforms/rp2040/src/lib/mbedtls_config.h Show resolved Hide resolved
@pguyot pguyot force-pushed the w44/ssl-over-socket-with-mbedtls branch from 74bb97e to 977304e Compare November 6, 2023 08:06
src/platforms/rp2040/src/lib/lwipopts.h Outdated Show resolved Hide resolved
CMakeModules/MbedTLS.cmake Outdated Show resolved Hide resolved
@bettio
Copy link
Collaborator

bettio commented Nov 6, 2023

Looks all good. Maybe this might be the first component implementing this structure: #930
Otherwise we can just merge it, and do this on a second step.

@fadushin
Copy link
Collaborator

fadushin commented Nov 6, 2023

Is it your feeling that this is not mature enough to have a place in the Programmer's Guide? Do we want to implement PXIX certificate chain validation first? (As it is, I believe the current implementation would be vulnerable to MitM attacks, without authentication).

Add support for ssl client in binary and passive modes, with no certificate
verification

Add APIs to otp_socket so it can be called from ssl bio callbacks

Fix a bug in lwIP otp_socket's recv revealed by ssl tests

Fix a bug in BSD otp_socket's recvfrom revealed by refactoring

Fix a bug in esp32 tests where main context and its resources were not properly
destroyed

Update documentation and workflows to reflect the requirement on Mbed TLS

Fix exported types of inet module

Signed-off-by: Paul Guyot <pguyot@kallisys.net>
@pguyot pguyot force-pushed the w44/ssl-over-socket-with-mbedtls branch from 977304e to 71cc082 Compare November 6, 2023 17:25
@bettio
Copy link
Collaborator

bettio commented Nov 6, 2023

Looks all good. Maybe this might be the first component implementing this structure: #930 Otherwise we can just merge it, and do this on a second step.

There are still a lot of open points in #930 proposal, so let's just focus in merging this PR, and let's take care of reorganizing code as soon all open points are closed.

@bettio
Copy link
Collaborator

bettio commented Nov 6, 2023

Is it your feeling that this is not mature enough to have a place in the Programmer's Guide? Do we want to implement PXIX certificate chain validation first? (As it is, I believe the current implementation would be vulnerable to MitM attacks, without authentication).

Proper documentation for an ssl module would both require a stable API and a reasonable amount of work, in order to explain caveats and how it differs from OTP one. This kind of work deserves a standalone PR, so let's merge this PR and document ssl module properly as soon as we are ready.

@bettio bettio merged commit 8b3dee0 into atomvm:master Nov 6, 2023
84 checks passed
@pguyot pguyot deleted the w44/ssl-over-socket-with-mbedtls branch November 7, 2023 05:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants