API: add failure mode support for randombytes()
This PR adds failure mode support for the randombytes() interface.
Marking as draft as there are a few points for which I need clarifications. These are the following:
- What is to be done regarding the OQS_randombytes API integration? From what I understand this is defined as having return type
voidon the liboqs side. Do we make a PR to change this on their end also or do we simply modify the return type of our inlinemld_randombytesfunctions defined in filesintegration/liboqs/config_*.h? - Is it appropriate that I am adding the
CHECK(x)macro tobench/test_components_mldsa.c? - Should I add a test which verifies the behavior in case
randombytes()fails?
As an aside. It could be a good idea to factor our the CHECK macro into its own test header file as the code is currently duplicated in many files across the project. @hanno-becker @mkannwischer please let me know your thoughts.
Thank you @L-series!
What is to be done regarding the OQS_randombytes API integration? From what I understand this is defined as having return type void on the liboqs side. Do we make a PR to change this on their end also or do we simply modify the return type of our inline mld_randombytes functions defined in files integration/liboqs/config_*.h?
So long as OQS' randombytes has return type void, we would merely need to change the wrapper to
#define MLD_CONFIG_CUSTOM_RANDOMBYTES
#if !defined(__ASSEMBLER__)
#include <oqs/rand.h>
#include <stdint.h>
#include "../../mldsa/src/sys.h"
static MLD_INLINE int mld_randombytes(uint8_t *ptr, size_t len)
{
OQS_randombytes(ptr, len);
return 0;
}
or am I missing something?
Is it appropriate that I am adding the CHECK(x) macro to bench/test_components_mldsa.c?
Yes, I see no problem with that.
Should I add a test which verifies the behavior in case randombytes() fails?
The CBMC proofs will need to cover the new behavior, but yes, we also need have specific tests that trigger failure of randombytes() at various points. This will require a custom configuration and a dedicated test; it's not clear on first sight where this should live. It does not fit into the existing tests in tests/*, nor would it make sense to add a new test class (alongside func, KAT, ACVP, stack, bench) for just this. I tend towards a custom 'example', despite being more of a test than an example -- but that line is blurry anyway.
or am I missing something?
No, this is how I implemented it, it does seem like the best solution currently - however it feels slightly wrong.
The CBMC proofs will need to cover the new behavior
I'll look into modifying the proofs and adding a testing example.
Hey @L-series! How are you getting on? Let us know if you need any input. Otherwise, can you mark the PR as draft until it's ready for review?
Hey @hanno-becker, sorry for the delay. Haven't had much time outside of work recently. Will mark as a draft till I fix the failing CI tests.
I've added a test case as well as fixed the failing CI tests. Please let me know what needs to be modified regarding the proofs as I'm not sure what to search for.