Add support for Zephyr RTOS

[Frauschi]

Hi everybody,

This PR adds support for the Zephyr RTOS, so liboqs can be used as a module (see here).

The main changes to the existing library code are focussed on the CMake build environment to enable the library to be built with the Zephyr SDK. All changes should not influence existing builds outside the Zephyr environment. (However, some changes also improve the integration of liboqs directly in a CMake-based project without installing it system-wide.)

One bigger source code change is related to random number generation. Zephyr doesn't support any of the currently present RNG interfaces. To be most flexible, I added a generic OQS_USE_EXTERNAL_RNG option (disabled by default). In this case, an externally defined method OQS_randombytes_external() must be present during link time, which is then used to obtain random data. For Zephyr, I implemented that method to generate random data using their RNG interface.

The main Zephyr integration is all contained in the new zephyr folder in the root directory (common practice for Zephyr modules to have this folder). In that directory, there is a custom CMakeLists.txt to integrate the existing CMake infrastructure into Zephyr. Furthermore, a Kconfig file is provided to configure the build (and especially the algorithm selection). Finally, the new OQS_randombytes_external method is implemented in a small C source file here.

As this is my first PR for this project, I'm not sure if the changes are upstreamable in the current form. But I'm looking forward to your feedback.

Kind regards,
Tobi

[baentsch]

Thanks for the contribution and new platform! Please provide feedback to some single comments/questions to help me understand things better. According to the Zephyr documentation there should be a person maintaining this module. Can we safely assume you'll do this -- also giving us a person to point questions to coming towards liboqs (related to this integration)? It would also be good to know that there is alignment on the platforms Zephyr and liboqs support: Are/have you already ascertain(ed) that?

[Frauschi]

Thanks for the contribution and new platform! Please provide feedback to some single comments/questions to help me understand things better. According to the Zephyr documentation there should be a person maintaining this module. Can we safely assume you'll do this -- also giving us a person to point questions to coming towards liboqs (related to this integration)? It would also be good to know that there is alignment on the platforms Zephyr and liboqs support: Are/have you already ascertain(ed) that?

Yes, I am happy to maintain the port.

I haven‘t completely looked into the supported platforms actually. What exactly are you looking for in that regard?

[baentsch]

Yes, I am happy to maintain the port.

Thanks!

I haven‘t completely looked into the supported platforms actually. What exactly are you looking for in that regard?

Is there a 1:1 match regarding the platforms that Zephyr supports and the ones that liboqs supports? Or is there some deficiency that would need to be addressed in liboqs to get there? Or is that not necessary?

[Frauschi]

Is there a 1:1 match regarding the platforms that Zephyr supports and the ones that liboqs supports? Or is there some deficiency that would need to be addressed in liboqs to get there? Or is that not necessary?

Currently, the matching platforms are 32-bit ARM, 64-bit ARM, x86 and “native build” on Linux (which is then probably one of 32/64-bit ARM, x86, or x86_64). In the latest changes of the commits, I added a dependency from within Zephyr to only enable the module if one of these supported platforms is selected. Otherwise, the liboqs module is disabled and a warning is raised in Zephyr.

To fully support all platforms of Zephyr in liboqs, we would have to add support for arc, mips, riscv, sparc, and xtensa. I think the most popular platforms of these are riscv and xtensa (ESP32 controllers). On these platforms, the native C code should run without problems.

I think it doesn't make sense to add those platforms now until there is actual request (also, I don't have hardware to test liboqs on those platforms, actually).

[baentsch]

I think it doesn't make sense to add those platforms now until there is actual request

Fully agree.

I added a dependency from within Zephyr to only enable the module if one of these supported platforms is selected.

Good addition. Resolves my concerns around platforms: Thanks!

[SWilson4]

Hi @Frauschi, thanks for the PR. Is it absolutely necessary to introduce the OQS_USE_EXTERNAL_RNG option? We have an API function to set a custom RNG; might this suffice for your purposes?

[baentsch]

Your latest comment @SWilson4 reminded me of an old-ish issue: Would it make sense to address #1490 along with this PR? What'd you say, @Frauschi ?

[Frauschi]

Would it make sense to address #1490 along with this PR?

This is indeed exactly the same issue I faced with Zephyr, as both the getentropy() method and /dev/urandom are not available in Zephyr.

I see one big problem with the solution proposed in #1490, for which I added the OQS_USE_EXTERNAL_RNG option: When we disable both default OQS_randombytes_system() methods during compilation, we rely on the user to set a proper method using OQS_randombytes_custom_algorithm() during runtime. When the user forgets to do that, we cause a runtime error, as no randombytes method is available.
In my opinion, it is better to always have a default implementation for randombytes to prevent runtime errors (the user still has the option to provide a custum method using the available API mentioned by @SWilson4). With the new OQS_USE_EXTERNAL_RNG, however, a link time error is caused, forcing the user to provide a proper default implementation (I provide one for Zephyr) without necessary steps for him during runtime.

If it is desired, I can of course revert the OQS_USE_EXTERNAL_RNG option and implement the solution proposed in #1490. What do you think @SWilson4 @baentsch?

[baentsch]

When we disable both default OQS_randombytes_system() methods during compilation, we rely on the user to set a proper method using OQS_randombytes_custom_algorithm() during runtime. When the user forgets to do that, we cause a runtime error, as no randombytes method is available.

Well, two thoughts:

1. Do we really want to play nanny for users changing good default configs? They should know what they do and deal with ensuing problems themselves if they activate e.g., a generic "OQS_EMBEDDED_API" option (maybe some good documentation for that option would be helpful :). We also could issue a cmake warning at build time if such generic embedded system config as proposed in Add option to exclude the compilation of OQS_randombytes_system() when a custom algorithm is specified #1490 gets activated, e.g., "no default RNG available. Be sure to call OQS_randombytes_custom_algorithm". This in place of an explicit flag stating that one does that (your "OQS_USE_EXTERNAL_RNG" proposal -- which arguably also would need to be amended for other thus possibly "incapacitated" calls, e.g., SHA2, etc).

2. Or (and?) possibly more simple: What about being more defensive here and output a runtime warning if oqs_randombytes_algorithm is NULL?

Also tagging @mabarger for input as to the propensity/time to contribute as per #1490 (comment)

[Frauschi]

1. Do we really want to play nanny for users changing good default configs? They should know what they do and deal with ensuing problems themselves if they activate e.g., a generic "OQS_EMBEDDED_API" option

True, I reverted my OQS_USE_EXTERNAL_RNG option and added a generic OQS_EMBEDDED_BUILD option as proposed in #1490.

2. Or (and?) possibly more simple: What about being more defensive here and output a runtime warning if oqs_randombytes_algorithm is NULL?

Added that, too.

@baentsch @SWilson4 what do you think about the new RNG setup?

[baentsch]

@Frauschi I don't quite "feel" like reviewing this so late in my day -- but will do before Monday. Independent of that could you please rebase the PR to the latest "main" to ascertain that CI passes?

[baentsch]

Conceptually looks good. Let's see what CI says. Besides some single comments that would be good to address, I am missing documentation for the new config variable. It may be good to explain differences to OQS_BUILD_ONLY_LIB and how it impacts OQS_DIST_BUILD.

[Frauschi]

missing documentation for the new config variable

I added documentation for the new option in CONFIGURE.md and also added a README for the Zephyr port in the zephyr directory.

It may be good to explain differences to OQS_BUILD_ONLY_LIB and how it impacts OQS_DIST_BUILD.

I didn't mention these two explicitly, as they are not really related to the new option. With the explanation of the new option, however, I think this should also be clear to a potential user of OQS_EMBEDDED_BUILD.

[baentsch]

Apart from a nit (typo) LGTM. Thanks! Let's CI decide now. Also tagging @SWilson4 @dstebila for second review.

[baentsch]

Thanks again for your contribution, @Frauschi ! Merged.