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

Wolfssl pkg with GNRC support #9894

Closed
wants to merge 1 commit into from

Conversation

danielinux
Copy link
Contributor

Contribution description

This patch is based on #7348 and introduces wolfSSL callback support for GNRC sock API

In this preliminary version, UDP callbacks are supported via the GNRC sock_udp interface.

A new example dtls-wolfssl is provided. It 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

Testing procedure

wolfSSL should now work 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 against host endpoints

Riot-to-host can be tested against the DTLS examples provided in the wolfSSL-examples repository.

Next steps

In case of positive feedback, we will implement TCP callbacks for GNRC and provide HTTPS/TLS1.3 examples over sock_tcp.

Issues/PRs references

This PR replaces #7348 as the previous socket-functionalities are maintained in the existing wolfssl-client and wolfssl-server examples.

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.

-- @danielinux @kaleb-himes @wolfSSL

@cladmi
Copy link
Contributor

cladmi commented Sep 14, 2018

With @danielinux we discussed this PR IRL and worked together to do some changes I wanted. I will summarize my remarks and justification here too as they belong in the comments of this PR for reference and critics.

I am only looking at the build system integration and not the code/implementation itself.


My main concern is that the version only uses only one module for everything, where I would like to have one module for each thing, and that I do not like user_settings.h which currently takes care of the dependency/configuration management where I would like to do it with modules.

With more details:

One module per directory

The Makefile.base file taking care of building a module is currently only made to handle building in one directory. To handle multiple source directories, the standard way is to have one module per directory.
In that case, it made sense from the hierarchy and implementation to split into wolfssl, wolfcrypt and wolfcrypt-test (using wolfcrypt-test name and not wolfcrypt_test prevents issues when wolfcrypt is implemented with submodules).

This leads to also define dependencies between these modules and only build in the directory if the module/package is actually used.

Remove per application global configuration of user_settings.h

Instead of setting wanted algorithms, features, by defining a per application user_settings.h I would prefer to define these different possible features/configurations with modules. (in that case PSEUDOMODULES as they do not generate any archive but add files to a main module or trigger dependencies).

My reasoning behind that is that first it prevent building source files that are not needed.
But also, using a header for selecting features prevents from doing more "per library" configuration. An application could be using two libraries that use wolfssl, the first could require to have ecc enabled while the other one rsa and it would mean that the final application would need to define both in the user_settings, so be aware of this. Also some configurations could depend on available FEATURES like using hardware crypto, or optimized assembly for specific architectures.
For this, I recommended to define these settings as SUBMODULES as they almost always match from one option to a file.

A package global user_settings.h would then be doing the link between RIOT and wolfssl by mapping modules to wolfssl defines:

#ifdef MODULE_WOLFCRYPT_ECC
  #define HAVE_ECC
#endif

We did some POC changes in that direction that I will PR them on his branch. They of course still need to be reviewed by other maintainers and by me as I did not have the whole picture and time to step back on my implementation.

@danielinux
Copy link
Contributor Author

danielinux commented Sep 17, 2018

Post-summit update:

First of all, a big thank you to @cladmi for taking the time to explain the dependencies in the build systems, and enlightening the path to make the wolfSSL pkg as modular as required.

As a result, the module is not now split into pseudo-submodules, so that the support for the single features and algorithms can be enabled depending of what the application needs.

Here are the major changes in this update:

  • The sock_tls interface has been slightly reworked and is now fully documented (see sock_tls.h)
  • Examples and tests have been updated accordingly:
    • dtls example (using socks) depends on sock_tls and wolfssl_ecc
    • wolfssl_crypto_test and wolfssl_crypto_benchmark tests have been simplified and updated to use the corresponding submodules, instead of including c files inline
    • a new test wolfcrypt-ed25519-verify has been added to provide evaluation on code size / speed for signature verification re: OTA
    • existing TLS tests using posix sockets have been updated to include lwIP and compile against the pseudomodule wolfssl_socket

@miri64 miri64 added Area: network Area: Networking Type: new feature The issue requests / The PR implemements a new feature for RIOT Area: pkg Area: External package ports Area: security Area: Security-related libraries and subsystems labels Oct 17, 2018
@danielinux
Copy link
Contributor Author

This is now outdated, please refer to #10308

@MichelRottleuthner
Copy link
Contributor

closing in favour of #10308

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: network Area: Networking Area: pkg Area: External package ports Area: security Area: Security-related libraries and subsystems Type: new feature The issue requests / The PR implemements a new feature for RIOT
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants