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

Improve liboqs integration #7026

Merged

Conversation

Frauschi
Copy link
Contributor

@Frauschi Frauschi commented Dec 4, 2023

Hi everybody,

this PR improves the integration of liboqs, focussing on two key aspects.

First, I added PQC support for the Zephyr port using liboqs by adding the necessary files to the CMakeLists.txt for the Zephyr module. The integration of liboqs as an additional Zephyr module is currently based on a liboqs fork of mine, which I am trying to upstream (see open-quantum-safe/liboqs#1621).

Furthermore, I added support for proper random number generation from within liboqs using the WolfSSL interface. Before, random data is either obtained from OpenSSL, from /dev/urandom, or using the get_entropy() method, depending on the build configuration of liboqs. With the proposed changes, we use the OQS_randombytes_custom_algorithm() interface to provide a custom RNG method for liboqs. Using a custom method, we can obtain RNG data from WolfSSL. I further added support in different places to use the correct RNG, which is provided in the related context as arguments. In case no RNG is provided by the wrapping context, a new default RNG object is used. As we switch the RNG object to use during runtime, I had to protect the interface with a mutex.

I placed the new liboqs related code in new header and source files, which are located in the new wolfCrypt/port/liboqs directory. The files are added both to the Make and CMake build infrastructure. However, I'm not sure if this is the right place for these files, to be honest.

I look forward to your feedback!

Kind regards,
Tobi

@wolfSSL-Bot
Copy link

Can one of the admins verify this patch?

Frauschi and others added 6 commits December 16, 2023 12:40
Signed-off-by: Tobias Frauenschläger <t.frauenschlaeger@me.com>
Improve the interface to liboqs by properly configuring and using the
RNG provided by WolfSSL from within liboqs.

Signed-off-by: Tobias Frauenschläger <t.frauenschlaeger@me.com>
Added a RNG argument to the wc_dilithium_sign_msg method to properly
generate necessary random data using the desired WolfSSL RNG object.

Signed-off-by: Tobias Frauenschläger <t.frauenschlaeger@me.com>
Added a RNG argument to the wc_falcon_sign_msg method to properly
generate necessary random data using the desired WolfSSL RNG object.

Signed-off-by: Tobias Frauenschläger <t.frauenschlaeger@me.com>
Added a RNG argument to the wc_sphincs_sign_msg method to properly
generate necessary random data using the desired WolfSSL RNG object.

Signed-off-by: Tobias Frauenschläger <t.frauenschlaeger@me.com>
When using WolfSSL on zephyr, we need POSIX names for networking systems
calls. This can either be enabled with CONFIG_NET_SOCKETS_POSIX_NAMES or
with CONFIG_POSIX_API. This commit enables support for the latter.

Signed-off-by: Tobias Frauenschläger <t.frauenschlaeger@me.com>
@Frauschi
Copy link
Contributor Author

As open-quantum-safe/liboqs#1621 is merged now, liboqs has official Zephyr support. Therefore, it should be safe to use the module from within the WolfSSL Zephyr port without relying on my fork of the library.

@douzzer
Copy link
Contributor

douzzer commented Dec 20, 2023

@Frauschi that's good news -- thanks for the update. This PR is working its way through our review+testing process and we'll keep you posted.

@anhu
Copy link
Member

anhu commented Dec 20, 2023

Hi @Frauschi !!
First of all, thank you so much for this change.

May I ask why you need to wolfSSL's rng facility? Is it something particular about Zephyr?

Warm regards, Anthony

@Frauschi
Copy link
Contributor Author

May I ask why you need to wolfSSL's rng facility? Is it something particular about Zephyr?

Hi @anhu,

The integration of the WolfSSL RNG facility is not directly related to Zephyr. The embedded nature of Zephyr and the hardware platforms it runs on merely increases the need for proper high quality random data, especially as the default random sources of liboqs aren‘t available in embedded environments. Hence, we needed a coupling between liboqs and WolfSSL for our Zephyr platforms anyways. In this context, I thought that a well defined interface between both libraries regarding random data would also benefit other environments like Linux (we also have Linux platforms in our research project) and ease software development.
Furthermore, we are able to use the crypto callback API of WolfSSL to integrate external TRNGs into our platforms using a well defined interface, which will also propergate down to liboqs without additional work or complexity.

@anhu
Copy link
Member

anhu commented Dec 21, 2023

Thank you @Frauschi !! That is an excellent explanation.

I've looked over your changes and they look good to me from a general perspective. I was the original author of most of the code you have touched. Back to @douzzer for general code review.

anhu
anhu previously approved these changes Dec 21, 2023
Copy link
Contributor

@douzzer douzzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

There was a trivial rebase conflict in wc_port.c that I've fixed to unblock merge.

Oh, and your choice of wolfcrypt/src/port/ for the liboqs-specific files is consistent with our previous decision to put the af_alg/, aria/, devcrypto/, and kcapi implementations there.

@douzzer douzzer merged commit bcfaf03 into wolfSSL:master Jan 3, 2024
92 checks passed
@Frauschi
Copy link
Contributor Author

Frauschi commented Jan 3, 2024

Thanks for the feedback and also for the rebase 😄 I‘m happy that this is merged 😄

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.

None yet

4 participants