You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I have been toying a little bit with Claude with regards to SDitH inclusion. What about these kinds of liboqs wrappings for the code provided by SDitH?
#include <oqs/sig.h>
#include "sdith_signature.h" /* from the vendored SDitH source */
OQS_STATUS OQS_SIG_sdith_cat1_short_keypair(uint8_t *public_key, uint8_t *secret_key) {
const signature_parameters *p = &CAT1_SHORT_PARAMETERS;
uint8_t entropy[/* sdith_keygen_entropy_bytes(p), compile-time-known per scheme */];
uint8_t tmp[/* sdith_keygen_tmp_bytes(p) */];
OQS_randombytes(entropy, sizeof(entropy));
sdith_keygen(p, secret_key, public_key, entropy, tmp);
OQS_MEM_cleanse(tmp, sizeof(tmp));
return OQS_SUCCESS;
}
/* ...sign / verify follow the same shape, forwarding to sdith_sign / sdith_verify */
Should macros for the lengths be employed or we continue to use the runtime SDitH procedures like sdith_keygen_tmp_bytes?
Hey @RodriM11, I've been taking a look at this and have a way forward with this same wrapper approach you suggested. I can keep working through the integration and report my findings here and in the next status call
I've had some success with the wrapper approach and shimming FIPS202 and AES. There's still some outstanding work, including tests to evaluate the impact of glue code, but in the meantime I opened a draft PR #2574 for feedback
An important note about the PR: I had Claude to review all the changes, and it pointed out that CI won't pass because the SDitH source raises several hundred warnings when built with -DOQS_STRICT_WARNINGS=ON . Some examples include non-ISO C patterns like __uint128_t. This is the first included algorithm in liboqs to raise such warnings.
I didn't include a fix, since I want CI to confirm this and think it deserves more attention than just muting these errors. Should upstream be modified to compile under these flags before inclusion, or should these warnings be ignored until they are fixed? If we want CI to pass regardless, a short addition to src/CMakeLists.txt can avoid treading warnings aas errors (copying src/common/sha3/xkcp_low/CMakeLists.txt with does the same thing):
Thanks for this effort @owaldron ! I'm afraid there is no single solution but a combination of things. Let me take them one by one:
CI won't pass because the SDitH source raises several hundred warnings when built with -DOQS_STRICT_WARNINGS=ON
That to me is no problem at the start as long as it passes with -DOQS_STRICT_WARNINGS=OFF: For me, the most important thing is to ascertain everything does compile (and works! :). So, one possible "procedure": Get one compiler on one architecture without strict warnings to pass all tests and then expand from there (e.g., re-#define macros, etc.)
This is the first included algorithm in liboqs to raise such warnings.
I'm not sure it is -- all algorithms went through some "enhancement phase" while landing in OQS -- with a possible exception of the old PQClean algs (that had imo been vetted there for such "niceties").
think it deserves more attention than just muting these errors
Excellent thinking and penultimate goal! This shouldn't get in the way of making initial progress, though: My personal approach in such cases is either note it clearly in comments and/or create a separate GH issue (depending on when I think I can get to it, i.e., before or after the PR lands).
copying src/common/sha3/xkcp_low/CMakeLists.txt with does the same thing
This is surely not a desirable long-term pattern -- particularly for "common" code that all algs should be able to rely on. There are various other instances of such "80% complete" code and a key reason why I so adamantly insist on OQS being called a research project when someone wants to use it to "cut [commercial development] corners"
In the case of a brand new algorithm however, I wouldn't mind at all taking such approach at the beginning: This is a new alg; no-one else relies on it (yet), so this approach would get us going. The important thing is to note it visibly, e.g., with a GH issue reading "Activate OQS_SDITH_WARN_AS_ERROR=ON". At that time, I'd suggest working with the upstream: We then have something working and it is in both projects' mutual interest to strengthen the code base.
I agree that this should be treaded eventually, but shouldn't block integration in the meantime. To move forward I'm going to document the issue on GitHub, try and stop these compiler warnings from blocking SDitH, and continue working on other SDitH integration issues that might arise. We can then work with upstream maintainers to resolve this as a separate issue.
Activity
RodriM11 commentedon Sep 4, 2026
The SDitH team will join the September 8th Status Meeting to start discussions on SDitH inclusion.
RodriM11 commentedon Sep 9, 2026
I have been toying a little bit with Claude with regards to SDitH inclusion. What about these kinds of
liboqswrappings for the code provided by SDitH?Should macros for the lengths be employed or we continue to use the runtime SDitH procedures like
sdith_keygen_tmp_bytes?owaldron commentedon Sep 15, 2026
Hey @RodriM11, I've been taking a look at this and have a way forward with this same wrapper approach you suggested. I can keep working through the integration and report my findings here and in the next status call
RodriM11 commentedon Sep 15, 2026
thanks @owaldron !
owaldron commentedon Sep 17, 2026
I've had some success with the wrapper approach and shimming FIPS202 and AES. There's still some outstanding work, including tests to evaluate the impact of glue code, but in the meantime I opened a draft PR #2574 for feedback
RodriM11 commentedon Sep 18, 2026
wow @owaldron ! Thank you very much for the hyper quick PR!!
owaldron commentedon Sep 22, 2026
Hey @RodriM11, I've opened a PR #2586 ready for review.
An important note about the PR: I had Claude to review all the changes, and it pointed out that CI won't pass because the SDitH source raises several hundred warnings when built with
-DOQS_STRICT_WARNINGS=ON. Some examples include non-ISO C patterns like__uint128_t. This is the first included algorithm in liboqs to raise such warnings.I didn't include a fix, since I want CI to confirm this and think it deserves more attention than just muting these errors. Should upstream be modified to compile under these flags before inclusion, or should these warnings be ignored until they are fixed? If we want CI to pass regardless, a short addition to
src/CMakeLists.txtcan avoid treading warnings aas errors (copyingsrc/common/sha3/xkcp_low/CMakeLists.txtwith does the same thing):RodriM11 commentedon Sep 23, 2026
tagging @xuganyu96 @dstebila @baentsch for their input in this
baentsch commentedon Sep 23, 2026
Thanks for this effort @owaldron ! I'm afraid there is no single solution but a combination of things. Let me take them one by one:
That to me is no problem at the start as long as it passes with -DOQS_STRICT_WARNINGS=OFF: For me, the most important thing is to ascertain everything does compile (and works! :). So, one possible "procedure": Get one compiler on one architecture without strict warnings to pass all tests and then expand from there (e.g., re-#define macros, etc.)
I'm not sure it is -- all algorithms went through some "enhancement phase" while landing in OQS -- with a possible exception of the old PQClean algs (that had imo been vetted there for such "niceties").
Excellent thinking and penultimate goal! This shouldn't get in the way of making initial progress, though: My personal approach in such cases is either note it clearly in comments and/or create a separate GH issue (depending on when I think I can get to it, i.e., before or after the PR lands).
This is surely not a desirable long-term pattern -- particularly for "common" code that all algs should be able to rely on. There are various other instances of such "80% complete" code and a key reason why I so adamantly insist on OQS being called a research project when someone wants to use it to "cut [commercial development] corners"
In the case of a brand new algorithm however, I wouldn't mind at all taking such approach at the beginning: This is a new alg; no-one else relies on it (yet), so this approach would get us going. The important thing is to note it visibly, e.g., with a GH issue reading "Activate OQS_SDITH_WARN_AS_ERROR=ON". At that time, I'd suggest working with the upstream: We then have something working and it is in both projects' mutual interest to strengthen the code base.
owaldron commentedon Sep 23, 2026
Thank you so much @baentsch for the advice.
I agree that this should be treaded eventually, but shouldn't block integration in the meantime. To move forward I'm going to document the issue on GitHub, try and stop these compiler warnings from blocking SDitH, and continue working on other SDitH integration issues that might arise. We can then work with upstream maintainers to resolve this as a separate issue.