wolfSSL pkg addition with examples

[danielinux]

Contribution description

This patch superseeds PR #9894 and contains wolfSSL support in a RIOT pkg.

This patch includes:

• independent modules that can be selected and integrated separately into applications
• single cryptography algorithm selectable as separate modules
• Support for TLS over TCP/IP using sockets
• Support for DTLS over UDP/IP using sock_udp interface

Modules

wolfCrypt:

• wolfcrypt_poly1305
• wolfcrypt_chacha
• wolfcrypt_ed25519
• wolfcrypt_aes
• wolfcrypt_pwdbased
• wolfcrypt_asn
• wolfcrypt_random
• wolfcrypt_* (e.g. ecc, rsa, sha3, ...)

wolfSSL:

• wolfssl_tls
• wolfsll_tls13
• wolfssl_dtls
• wolfssl_* (e.g. ocsp, crl, ...)

Testing procedure: sock_udp

The example dtls-wolfssl provided uses similar shell commands to those in dtls-echo:

dtlss
start dtls server on port 11111

dtlsc addr
start dtls client demo connection to server at address addr

wolftest
run all sanity checks to ensure that the crypto engine is working on the target

Compile and run dtls-wolfssl

wolfSSL works on any 32-bit target with support for gnrc_networking.

The test procedure for native riot-to-riot test is the following:

Prepare the bridge interface linking two tuntap:

./../../dist/tools/tapsetup/tapsetup --create 2

Run the server

    $ make all; PORT=tap1 make term
    > ifconfig

copy the server address

   > dtlss

Run the client

    $ PORT=tap0 make term
    > dtlsc <IPv6's server address[%netif]>

Testing procedure: full TCP/IP TLS demo

The examples wolfssl-client and wolfssl-server provide a TLS demo with full TCP/IP connectivity, and supports all 32-bit platforms, used in combination with the lwIP package. The TCP socket support depends on lwip_sock_tcp.

Issues/PRs references

This PR includes all previous work made in #9894 and #7348, rebased as single commit on the current master.

The wolfSSL library package temporarly lives in the feature branch https://github.com/danielinux/wolfssl/tree/riot-os until the new callbacks for GNRC are merged in.

Acknowledgements

Thanks to @cladmi for reviewing the modules integration and helping with modularization of the single components.

Thanks to @bergzand for testing wolfcrypt_ed25519 and suggesting code-size optimization settings.

Please consider for inclusion.

Any feedback is welcome.

-- @danielinux -- @kaleb-himes -- @wolfSSL

[jcarrano]

@danielinux I'm rushing to finish #9565 so that you can use it to embed the certificates.

[danielinux]

I made the CI robots happy by fixing a few issues in the code and in the markdown files.

Please let me know if there are any further actions needed to speed up the review of this PR.

Thanks,

-- @danielinux -- @wolfSSL

[MrKevinWeiss]

release status ping, should I change the milestone?

I think @MichelRottleuthner has something to say

[MrKevinWeiss]

Something tells me this will continue to be pushed to the future release.

[danielinux]

It would be nice to hear any comment about this PR.

wolfSSL's support original PR (#7348) is almost two years old while typing this. Since then, we have widely improved the integration. Now we have a proper pkg, we build single wolfcrypt/wolfssl components into separate modules, and we support both posix and GNRC socket interfaces.

The latest version of this code is a few weeks old and is based on wolfSSL 4.0.0-stable.

I have updated this branch regularly in the past months, to sync up to latest RIOT and wolfSSL master branches and test/fix the examples.

[MrKevinWeiss]

I don't think it will get in this release but I can bug @MichelRottleuthner to help with the review

[MichelRottleuthner]

I finally found some time to spend on this PR and I'm willing to help wherever I can to get this merged.
First: I feel really sorry for how long this PR is already waiting to get merged, especially considering how valuable it is. I know how frustrating it can be when it takes that long to get a feature in.
And I hope it didn't discourage you to to further contribute to RIOT.
There are a lot of people in the community that want this feature in - but sadly even more people that are short on time ;)
Anyway, thank you a lot for staying active!

That being said, let's move on to the technical stuff.
It's very much appreciated that you separated and integrated everything as individual modules.
Looking at the history of all the related PRs clearly shows a huge improvement.

I didn't do a detailed code review yet but when quickly browsing thru the code I couldn't spot major problems.
The PR comes with quite a lot of different examples/tests:
under examples we have:
dtls-wolfssl, wolfssl-client, wolfssl-server
under tests we have:
wolfcrypt-ed25519-verify, wolfssl_crypto_benchmark, wolfssl_crypto_test

While I generally like to have more than one or two examples/tests, I think it adds unnecessary complexity for getting a first version of a new feature in.
tests/wolfssl_crypto_test could basically be removed as this is already contained as wolftest command in examples/dtls-wolfssl, right?
Would it be possible to integrate the other tests i.e. wolfcrypt-ed25519-verify, wolfssl_crypto_benchmark in a similar way?

When trying to build examples/wolfssl-client or examples/wolfssl-server I get the following compilation error:

$ make
Building application "wolf_server" for "native" with MCU "native".
"make" -C /home/michel/devel/wolfssl/RIOT/examples/wolfssl_server/bin/pkg/native/wolfssl/src
In file included from /home/michel/devel/wolfssl/RIOT/examples/wolfssl_server/bin/pkg/native/wolfssl/wolfssl/ssl.h:134,
                 from /home/michel/devel/wolfssl/RIOT/examples/wolfssl_server/bin/pkg/native/wolfssl/wolfssl/internal.h:29,
                 from /home/michel/devel/wolfssl/RIOT/examples/wolfssl_server/bin/pkg/native/wolfssl/src/wolfio.c:37:
/home/michel/devel/wolfssl/RIOT/examples/wolfssl_server/bin/pkg/native/wolfssl/src/wolfio.c: In function ‘wolfIO_Recv’:
/home/michel/devel/wolfssl/RIOT/examples/wolfssl_server/bin/pkg/native/wolfssl/wolfssl/wolfio.h:274:27: error: implicit declaration of function ‘recv’ [-Werror=implicit-function-declaration]
  274 |     #define RECV_FUNCTION recv
      |                           ^~~~
/home/michel/devel/wolfssl/RIOT/examples/wolfssl_server/bin/pkg/native/wolfssl/src/wolfio.c:624:18: note: in expansion of macro ‘RECV_FUNCTION’
  624 |     recvd = (int)RECV_FUNCTION(sd, buf, sz, rdFlags);
      |                  ^~~~~~~~~~~~~
/home/michel/devel/wolfssl/RIOT/examples/wolfssl_server/bin/pkg/native/wolfssl/src/wolfio.c: In function ‘wolfIO_Send’:
/home/michel/devel/wolfssl/RIOT/examples/wolfssl_server/bin/pkg/native/wolfssl/wolfssl/wolfio.h:273:27: error: implicit declaration of function ‘send’ [-Werror=implicit-function-declaration]
  273 |     #define SEND_FUNCTION send
      |                           ^~~~
/home/michel/devel/wolfssl/RIOT/examples/wolfssl_server/bin/pkg/native/wolfssl/src/wolfio.c:634:17: note: in expansion of macro ‘SEND_FUNCTION’
  634 |     sent = (int)SEND_FUNCTION(sd, buf, sz, wrFlags);
      |                 ^~~~~~~~~~~~~
cc1: all warnings being treated as errors
make[2]: *** [/home/michel/devel/wolfssl/RIOT/Makefile.base:83: /home/michel/devel/wolfssl/RIOT/examples/wolfssl_server/bin/native/wolfssl/wolfio.o] Error 1
make[1]: *** [/home/michel/devel/wolfssl/RIOT/Makefile.base:20: ALL--/home/michel/devel/wolfssl/RIOT/examples/wolfssl_server/bin/pkg/native/wolfssl/src] Error 2
make: *** [/home/michel/devel/wolfssl/RIOT/examples/wolfssl_server/../../Makefile.include:485: /home/michel/devel/wolfssl/RIOT/examples/wolfssl_server/bin/native/application_wolf_server.a] Error 2

The system setup used.
So for now I didn't consider these applications for further testing and as of now focused on running examples/dtls-wolfssl on variuos platforms.

Here is a first overview of what I found working on which platform:

board	wolftest	dtlss	dtlsc
native	PASSED	PASSED	PASSED
samr21-xpro	PASSED	?	FAILED (2)
nucleo-f303re	PASSED	FAILED	PASSED
nucleo-f207zg	PASSED	PASSED	?
nucleo-f091rc	FAILED (1)	FAILED	FAILED (2)
nucleo-f767zi	PASSED	PASSED	PASSED
nucleo-l476rg	PASSED	PASSED	PASSED

The failed ones under dtlss didn't actually print an error on the server side but made the client fail to connect.
For the ones marked with '?' I couldn't exactly tell which side was inducing the problems, I will investigate further.
Details on how the nodes failed for the ones that have a number:

...
RANDOM   test passed!
ecc_test_curve_size 32 failed!: -125
ECC      test failed!
 error = -125
#! exit -1: powering off

...
connecting to server...
Error allocating ssl session

Do you have any hints on what I could try to further investigate?
Could this be related to stacksize being to small or maybe timeouts that are too tight for rather slow platforms?

Some low priority comments regarding the shell commands of dtls-wolfssl:
is it expected/required that the system stops after running the wolftest command?
At least I didn't expect that and its not mentioned in the command description.
Also, the dtlss description says that it can be used to start and stop a server, but actually it just starts it and then auto-closes after fist connection.

[danielinux]

I understand how difficult it can be to find time to review these PR, so no worries!
Thanks for your time, I'll try to address the issues here.

It's very much appreciated that you separated and integrated everything as individual modules.
Looking at the history of all the related PRs clearly shows a huge improvement.

Kudos to @cladmi for the patience guiding me through this part.

The PR comes with quite a lot of different examples/tests:
under examples we have:
dtls-wolfssl, wolfssl-client, wolfssl-server

WolfSSL support can integrate with either GNRC (via wolfssl + gnrc_sock_udp/tcp) or POSIX socket calls (via wolfssl_socket + posix_socket modules).

dtls-wolfssl is an example using GNRC sockets. wolfssl-[client|server] are
the older demos, that depend on POSIX-compliant sockets.

I agree we could remove these examples if they are not needed/less interesting,
but on the other hand they are the only example that use wolfssl_socket module (with
posix_socket).

I've checked the problem with compiling these tests, it's due to a missing
include, I'll push a fix.

under tests we have:
wolfcrypt-ed25519-verify, wolfssl_crypto_benchmark, wolfssl_crypto_test

These are wolfcrypt-only test/benchmark units, that have also been used to
evaluate performance of single in other contexts (see @bergzand's work on
secure boot).

On some platforms, I expect that there might not be enough resources (code
size/heap/stack) to run full SSL (e.g. DTLS), while single wolfcrypt
algorithms (that can now be selected as separate modules) might be the right fit
for specific purposes on constrained targets. For this reason I think it might be useful
to keep some of the 'tests'.

When trying to build examples/wolfssl-client or examples/wolfssl-server I get the following compilation error:

I'll push a fix for this.

So for now I didn't consider these applications for further testing and as of now focused on running examples/dtls-wolfssl on variuos platforms.

Here is a first overview of what I found working on which platform:
[...]

The failed ones under dtlss didn't actually print an error on the server side but made the client fail to connect.
For the ones marked with '?' I couldn't exactly tell which side was inducing the problems, I will investigate further.
Details on how the nodes failed for the ones that have a number:

RANDOM test passed!
ecc_test_curve_size 32 failed!: -125

Error -125 is 'out of memory error'. The target could not provide the memory
required to perform the ecc_test.

connecting to server...
Error allocating ssl session

Sounds like a memory problem again, wolfSSL_new is failing to create the session
object (malloc failed?).

Do you have any hints on what I could try to further investigate?
Could this be related to stacksize being to small or maybe timeouts that are too tight for rather slow platforms?

Stack (or more likely heap) might be a problem initializing large objects (TLS session) on some platforms. Wolfcrypt-only tests have more chances to run on those.

Perhaps I should add targets to add targets to BOARD_INSUFFICIENT_MEMORY depending
on the module selection. Feel free to suggest a solution.

Some low priority comments regarding the shell commands of dtls-wolfssl:

I took notes about these, I will fix the dtls example behavior.

[MichelRottleuthner]

I added some remarks on the code, most of them not really critical, see below.

[MichelRottleuthner]

same as above

[danielinux]

this is also fixed (see new blacklist in the unified test app)

[MichelRottleuthner]

Stack (or more likely heap) might be a problem initializing large objects (TLS session) on some >platforms. Wolfcrypt-only tests have more chances to run on those.

Ok. IIRC, wolfssl also has some option to work with static memory - isn't that used for RIOT? If it is used, I'd actually expect that the build would fail with some "region whatever overflowed" message for too constrained devices. If not, what needs to be done to use static memory?

Perhaps I should add targets to add targets to BOARD_INSUFFICIENT_MEMORY depending
on the module selection. Feel free to suggest a solution.

If there is a way to let it fail on build we can use make buildtest to derive a list. I think setting BOARD_INSUFFICIENT_MEMORY based on the selected modules isn't really required. It should just be ensured that the list is valid for the default application configuration. The CI wouldn't enable disable modules on its own, and a dev who changes the configuration would still have to check if everything works as expected. In any case using (big enough) static memory would help by just failing to build if it can not be ensured to work.

I just tested (only compilation for now) of the wolfssl_client/server applications with your fix. Building for an arm-none-eabi-gcc target (nucleo-l476rg) failed with implicit declaration of strnlen. Looking at the code it should be safe to just replace with strlen() because in both cases the string is not user-provided, and also it is in a non-critical part of an example application. On the wolfssl_client application I also got an "undefined reference to `inet_pton'" error that could be fixed by adding "USEMODULE += posix_inet" to the Makefile.

regarding wolfcrypt-ed25519-verify, wolfssl_crypto_benchmark, wolfssl_crypto_test:

(...) These are wolfcrypt-only test/benchmark units (...)

I wonder if the binary size grows too much if those get combined into a singe application providing three test commands?

[MichelRottleuthner]

Thanks for adding the fixes. I tried again with examples/wolfssl_server and examples/wolfssl_client.
The server crashes for me on native:

/home/michel/devel/wolfssl/RIOT/examples/wolfssl_server/bin/native/wolf_server.elf tap1 
RIOT native interrupts/signals initialized.
LED_RED_OFF
LED_GREEN_ON
RIOT native board initialized.
RIOT native hardware initialization complete.

main(): This is RIOT! (Version: 2019.07-devel-607-gcd5c1-prtest_10308_wolfssl)
This is the wolfSSL Server!
Server is running on 127.0.0.1 and listening on port 11111
make: *** [/home/michel/devel/wolfssl/RIOT/examples/wolfssl_server/../../Makefile.include:558: term] Segmentation fault (core dumped)

Coming from the main the crash happens in the wolfSSL_CTX_use_PrivateKey_buffer function.
Here is a gdb backtrace of the crash:

#0  0xf7d61ad4 in free () from /usr/lib32/libc.so.6
#1  0x5655911f in free (ptr=0xfaf414a4) at /home/michel/devel/wolfssl/RIOT/cpu/native/syscalls.c:175
#2  0x5657f894 in wc_RsaCleanup (key=0x565c3130) at /home/michel/devel/wolfssl/RIOT/examples/wolfssl_server/bin/pkg/native/wolfssl/wolfcrypt/src/rsa.c:241
#3  0x5657fa69 in wc_FreeRsaKey (key=0x565c3130) at /home/michel/devel/wolfssl/RIOT/examples/wolfssl_server/bin/pkg/native/wolfssl/wolfcrypt/src/rsa.c:520
#4  0x5655f88b in ProcessBuffer (ctx=0x565c2170, buff=0x5659e5a0 <server_key_der_2048> "0\202\004\245\002\001", sz=1193, format=2, type=1, ssl=0x0, used=0x0, userChain=0)
    at /home/michel/devel/wolfssl/RIOT/examples/wolfssl_server/bin/pkg/native/wolfssl/src/ssl.c:4929
#5  0x56560f1c in wolfSSL_CTX_use_PrivateKey_buffer (ctx=0x565c2170, in=0x5659e5a0 <server_key_der_2048> "0\202\004\245\002\001", sz=1193, format=2)
    at /home/michel/devel/wolfssl/RIOT/examples/wolfssl_server/bin/pkg/native/wolfssl/src/ssl.c:11284
#6  0x56558065 in main () at /home/michel/devel/wolfssl/RIOT/examples/wolfssl_server/main.c:117

For arm-none-eabi targets I can't compile. Removing strnlen or adding #define _DEFAULT_SOURCE 1 at the top of main.c could be used to solve this.

To actually enable radio support on platforms that come with a radio USEMODULE += netdev_default should be added to the Makefile. For 802.15.4 radios this wont work with the default LWIP_IPV4 = 1 setting. Though, overwriting actually doesn't make a difference for the application as values like the IP are hardcoded in v4 style and calls to socket(AF_INET, ...) etc. all use ..INET instead of ..INET6.
Did you try these examples on an MCU platform? Because if these examples will only work on native anyway, I don't think we should keep them. Which MCU-platform did you use for running these examples?

[MichelRottleuthner]

I gave the tests/wolfssl_crypto_benchmark a try.
Out-of-the-box I didn't manage to run it on any MCU platform until I figured out that the default benchmark configuration is just too heavy. Adding #define BENCH_EMBEDDED to pkg/wolfssl/include/user_settings.h solved that. This should be done automatically if compiling for an MCU target.

Additionally I needed to add USEMODULE += xtimer to stop the benchmark from freezing and USEMODULE += printf_float to properly print the results.
We should figure out if these dependencies should be added to any of the submodules of the wolfssl package or if it is just a dependency of this benchmark application. Depending on that they should be added to the respective Makefile.dep or the app's Makefile.

[danielinux]

Stack (or more likely heap) might be a problem initializing large objects (TLS session) on some >platforms. Wolfcrypt-only tests have more chances to run on those.

Ok. IIRC, wolfssl also has some option to work with static memory - isn't that used for RIOT? If it is used, I'd actually expect that the build would fail with some "region whatever overflowed" message for too constrained devices. If not, what needs to be done to use static memory?

Static memory 'moves' the problem of using the heap by allocating large objects in the stack, and it's genereally more memory consuming than normal allocations, because it also pre-allocates large pools of static memory to be able to be flexible enough for a generic use case. I have asked @kaleb-himes and he recommends to keep WOLFSSL_STATIC off in memory constrained environments.

[MichelRottleuthner]

Makes sense. I didn't know how variable the size of the buffers are that wolfssl needs (and by that how bad the overhead of static memory would be). That also means that we have to either exclude all platforms that have no proper implementation of malloc available (e.g. only one-way-malloc on MSP430, not sure about others) or as an alternative we use the tlsf based malloc implementation there. Adding support for tlsf can be easily done later in a follow-up though.

[danielinux]

Michel, I fixed most of the comments above (still working on the others).
I've reworked the posix tls examples, and unified wolfssl_server / wolfssl_client in a shell application (tls-wolfssl-posix) that now has:

• netif_default
• ecc256 keys used by default as PK, this should make it work with less resources
• IPv6-only sockets, to be LoWPAN friendly

[MichelRottleuthner]

Very nice progress!
I will do another round of testing with the new test applications (hopefully tomorrow)

[kaspar030]

I will probably have to blacklist all Atmel samX devices, due to a conflicting symbol, unless anyone has better ideas on this regard.

Maybe a #define _SAML21_AES_COMPONENT_ above the include list somewhere would blank out the sam0 part of the conflict? edit That would make the header guard of sam0's aes.h to kick in and not include aes.h's contents.

[danielinux]

Maybe a #define _SAML21_AES_COMPONENT_ above the include list somewhere would blank out the sam0 part of the conflict?

it worked by defining both _SAML21_AES_COMPONENT_ and _SAME54_AES_COMPONENT_ for the two platforms. Thanks @kaspar030 !

[danielinux]

@MichelRottleuthner : reporting in, 🤖 🤖 happy. Is there anything else to address?

[MichelRottleuthner]

@danielinux very nice! Please go ahead and squash the fixing commits so there are only a few cleanly separated commits left, e.g. one adding the package, and one for each test app etc.
I'll give it a final round of testing and code/documentation check tomorrow (I don't have the hardware at hand) and If everything is fine we're ready to go! 🎉

[MichelRottleuthner]

Re-tested everything on native nucleo-l476rg and samr21-xpro:
Pass: ✓ Fail:✗

test	native	nucleo-l476rg	samr21-xpro
examples/dtls-wolfssl (psk)	✓	✓	✓
examples/dtls-wolfssl (ecc)	✓	✓	✓
examples/dtls-wolfssl (rsa)	✓	✓	✗*
tests/pkg_wolfssl	✓	✓	✓
tests/pkg_wolfcrypt-ed25519-verify	✓	✓	✓

*expected due to memory constraints

Please just fix the small (nitpicky but quickly to do) doc/style things noted below. Feel free to directly ammend/squash and thats's it!

[danielinux]

@MichelRottleuthner Waiting for you to double check on the latest fixup. I will squash again if everything looks OK. Thanks!!

[MichelRottleuthner]

Perfect! Enough annoying whitespace fixes ;) go ahead and squash

[MichelRottleuthner]

Finally: ACK
Thanks again for hanging in for so long! Looking forward to build some nicely secured applications with wolfSSL :)

[miri64]

What is the plan for sock_dtls now? Will @pokgak port it to our version?

[miri64]

Congratulations on finally merging it btw 🎉

[danielinux]

It was our pleasure!
Expect more contributions from us very soon :) 🐺

[MichelRottleuthner]

What is the plan for sock_dtls now? Will @pokgak port it to our version?

I'd be fine with that obviously, but I don't care as long as it gets done eventually.

[danielinux]

What is the plan for sock_dtls now? Will @pokgak port it to our version?

for now the decision was to keep the symbols with the same names, but of course I am available to support @pokgak in the unification of the API

[kaspar030]

It was our pleasure!

😆

[pokgak]

Congratulations for the merge!

What is the plan for sock_dtls now? Will @pokgak port it to our version?

I plan to work on this soon:tm: :)

[bergzand]

congrats @danielinux!

[OlegHahm]

Wooohoooo! Great news! Thanks for the effort and patience.

[emmanuelsearch]

yay ;) congrats!

[kYc0o]

Congrats @danielinux !!! I love to see this in!