mldsa-native icon indicating copy to clipboard operation
mldsa-native copied to clipboard

API: add failure mode support for randombytes()

Open L-series opened this issue 2 months ago • 5 comments

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:

  1. 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?
  2. Is it appropriate that I am adding the CHECK(x) macro to bench/test_components_mldsa.c?
  3. 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.

L-series avatar Nov 12 '25 15:11 L-series

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.

hanno-becker avatar Nov 12 '25 19:11 hanno-becker

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.

L-series avatar Nov 13 '25 14:11 L-series

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?

hanno-becker avatar Nov 20 '25 05:11 hanno-becker

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.

L-series avatar Nov 20 '25 19:11 L-series

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.

L-series avatar Dec 01 '25 01:12 L-series