feat(sig): add derandomized keypair generation for ML-DSA (OQS_SIG_keypair_derand) - #2476
Conversation
Add OQS_SIG_keypair_derand, mirroring the existing OQS_KEM derand API: a
keypair_derand function pointer and length_keypair_seed field on OQS_SIG,
a top-level OQS_SIG_keypair_derand wrapper, and per-scheme
OQS_SIG_ml_dsa_{44,65,87}_keypair_derand backed by FIPS-204
ML-DSA.KeyGen_internal over a 32-byte seed.
- copy_from_upstream: propagate the family-level derandomized_keypair flag
into SIG scheme scope and derive the per-impl derand symbol as
<signature_keypair>_internal (upstream mldsa-native META exposes no
separate derand symbol); guard the whole keypair_derand definition on
derandomized_keypair so non-derand schemes emit neither decl nor def.
- docs: add "Keypair seed size (bytes)" to the SIG parameter-set tables.
- tests: sig_test_derand verifies determinism (same seed -> identical
pk/sk), seed-dependence (different seed -> different keypair), a
sign/verify round-trip on the derived key, and the NULL/unsupported
contract for schemes without derandomized keygen.
Implements open-quantum-safe#2475
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Paul Clark <paul@systemslibrarian.dev>
- fuzz_test_sig: exercise OQS_SIG_keypair_derand under libFuzzer/ASAN - vectors_sig: cross-check derand keygen against FIPS-204 ACVP keyGen known answers (the ACVP seed is xi), proving KeyGen_internal(xi) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Paul Clark <paul@systemslibrarian.dev>
9fe9fe9 to
c729851
Compare
Signed-off-by: Norman Ashley <nashley@cisco.com>
| name: ml_dsa | ||
| default_implementation: ref | ||
| upstream_location: mldsa-native | ||
| derandomized_keypair: true |
There was a problem hiding this comment.
This should be maintained upstream in the META file.
Until the mechanism is approved by the liboqs maintainers, could you implement this via patches to be META files, please?
| if scheme.get('derandomized_keypair'): | ||
| for imp in scheme['metadata']['implementations']: | ||
| if 'signature_keypair_derand' not in imp and 'signature_keypair' in imp: | ||
| imp['signature_keypair_derand'] = imp['signature_keypair'] + '_internal' |
There was a problem hiding this comment.
This is incredibly fragile and I guarantee you that this is going to break in the future.
If liboqs wants to support an additional API, the symbol for it should be defined in the META files rather than appending an _internal to a symbol that is already there. Again this can be implemented as a patch to the META files first and later it should be upstreamed.
There was a problem hiding this comment.
Completely concur (that this is fragile). Along the same lines, I'm also concerned with this API changing its behaviour based on whether or not the struct it's contained in is callocd or mallocd...
The key question in my eyes: Is this additional API indeed one that is truly generic, i.e., supported by all algorithms, or is it specific to ML-DSA (it seems so as only those files' are touched by the PR)? If so, wouldn't it be more apt to call the API ml_dsa_keypair_derand?
There was a problem hiding this comment.
The key question in my eyes: Is this additional API indeed one that is truly generic, i.e., supported by all algorithms, or is it specific to ML-DSA (it seems so as only those files' are touched by the PR)? If so, wouldn't it be more apt to call the API
ml_dsa_keypair_derand?
While not all algorithms support using seed values, there are other algorithms that do. This PR will address the first algorithm, and the others will be addressed in separate PRs.
There was a problem hiding this comment.
While not all algorithms support using seed values, there are other algorithms that do. This PR will address the first algorithm, and the others will be addressed in separate PRs.
Why separate PRs? If one introduces a design for an API meant for multi-algorithm use, it should be introduced showing that it serves all of them.
Otherwise the API may need to be revised again and again as the "separate PRs" roll in. In the worst case, someone starts to use the initial API before those PRs arrive and the initial API becomes immutable, making re-factor hard and the library even less usable (different APIs for different algs and the same functionality are absolute anathema to the core value of OQS: One API for all PQ algs).
This statement makes me reject this PR until there is a clear declaration which further algorithms the new API shall serve and until when these are integrated such as to ensure a coherent approach.
Anything other leads to just another set of open issues, "design-by-coincidence" (or first example) and brittle/seldomly used code paths.
There was a problem hiding this comment.
@baentsch, Just to clarify... The previous ML_DSA code base did not support deterministic key generation. The current version now does. Separate PRs would only be necessary if other algorithm authors introduced new APIs that OQS desired to support. I am not aware of any other upstream projects implementing this feature.
There was a problem hiding this comment.
Thanks for the clarification @ashman-p ! Then back to my question above "wouldn't it be more apt to call the API ml_dsa_keypair_derand"?
baentsch
left a comment
There was a problem hiding this comment.
Provide plan how this change is going to serve other algorithms.
Signed-off-by: ashman-p <nashley@cisco.com>
| length-public-key: 1312 | ||
| length-secret-key: 2560 | ||
| length-signature: 2420 | ||
| length-keypair-seed: 32 |
There was a problem hiding this comment.
Shouldn't the derandomized_keypair appear also here?
There was a problem hiding this comment.
@RodriM11, can you elaborate? In what context and why?
|
The changes in general LGTM. The approach is very similar to the inclusion made for KEMs, with the difference that, in PR #2070 the API was defined for every algorithm, with dummy failing implementations made for every algorithm that did not support derandomized KeyPair generation (i.e. ML-KEM). In this case, this implementation is made only for ML-DSA specifically, when there is another algorithm that actually supports it (SLH-DSA). I wonder if it makes sense to, once putting the effort onto this, actually expand it for every SIG algorithm (which is what #2475 originally describes). I believe the work wouldn't be a lot, since most of the expansion can be done in a similar way to #2070 via the |
I'd even state that that'd be a minimum requirement. Otherwise this PR truly only adds a bespoke But in general, I still think it's a bad idea to add "generically looking" APIs that in reality only a subset of algorithms support: This makes for
All properties not desirable in a "security library". --> If there's truly a need for this (and willingness by someone to support it), why not create a separate API for algorithms with separate features, e.g., a "post-standardization API" (outside of "common")? This would make the distinction in algorithms clearly visible for users. Also reviewers could focus: Those that support such limited scope API look and validate such bespoke code, those that don't, don't. |
|
I have a draft for making the necessary changes to mldsa-native to support derand keygen without patches: pq-code-package/mldsa-native#1311. I'll hold that PR back until a decision has been made here. I have no strong opinion whether or not liboqs should add such a new API. Should the consensus be that such an API would be useful, we are happy to support it. My view is that any standardized algorithm is going to offer such an API. |
Thanks for that take @mkannwischer ! Supports a) the request to validate/make available a new API for more than just 1 alg and 2) my suggestion of having a specific API set for standardized algs -- if there's enough people willing to support that and enough downstream interest. |
@mkannwischer, thanks for sharing your work that enables a patch-free implementation. I’d love to see it. At the risk of seeming to speak for @systemslibrarian, would you be open to contributing to this PR’s collaborative effort? Otherwise, i would recommend you publish your PR and let’s discuss it. I think we all mostly just want to see the feature/functionality implemented and usable. @baentsch, if you think a new API is the better path, can you put out what you think it should look like and how it works across existing algorithms? Thank you all. |
I did not mean to say that my PR is an alternative to this one. It implements the same API and it meant be be compatible (just removing the need for patches and implementing the return value normalization discussed in #2469). |
There's a myriad of options how to skin that cat. Expecting TL;DR syndrome: The most obvious one is what an object oriented environment would call a "derived class". If reading on, details: In C, that would be an OQS_STD_SIG struct using the current OQS structs/APIs as the first component and the new, std-alg-only function pointers & data structs as additional elements --> All core/"research" algs would remain as easily to use as before and don't have to be concerned with features relevant only to some algorithms: integration, testing, usage of those would remain (or even become again easy, if also applied to KEMs) as-is; all "finished"/std algs get their specific features/APIs; boilerplate gets eliminated; internal structures would become more easily understandable again. We could develop that into a fully formed architecture improvement proposal, if anyone had an interest in that: Better usage structure, much better focused PR review & testing, better internal structures than the ad-hoc "alg-specific-feature-bolt-on" approach the project currently pursues and that I think is brittle and will (imo already does) lead to more and more internal (and downstream) complexity, bugs and reduced usability. That said I won't do that effort on my own: 1) I already made too many proposals that no-one had interest or time to cooperate in for improving the project. 2) I think this is an effort wasting energy as other FOSS caters to PQ std algs much better -- all the while it's an afterthought in OQS, as the mere presence of this PR already proves. As a compromise/litmus test below a proposal written up by an AI so I don't waste too much time creating proposals no-one reads anyway: oqs-stdalgs-extension-proposal.md. Feedback welcome (and if only by another AI :-). |
Closes #2475.
Summary
Exposes deterministic / derandomized keypair generation for the
OQS_SIGfamily, mirroring the existingOQS_KEMderand API:keypair_derand(pk, sk, seed)function pointer and alength_keypair_seedfield toOQS_SIG, plus a top-levelOQS_SIG_keypair_derand(...)wrapper — the same shapeOQS_KEMalready has.OQS_SIG_ml_dsa_{44,65,87}_keypair_derandover FIPS-204's seed-based keygen (ML-DSA.KeyGen_internal(ξ), a 32-byte seed). For schemes without seed-based keygen,keypair_derandisNULL,length_keypair_seedis0, and the wrapper returnsOQS_ERROR.derandomized_keypairflag throughcopy_from_upstreamso the per-scheme symbol and the decl/def guards are generated, and regenerates the SIG docs (adds the "Keypair seed size (bytes)" column already present on the KEM docs).This unblocks seed/passphrase-recoverable post-quantum signing keys (e.g. deriving an ML-DSA signing key from a BIP-39 mnemonic), which the KEM side already supports via
keypair_derand.Testing (all in CI)
tests/test_sig.c— verifies determinism (same seed → identical pk/sk) and that the derived keypair is functional (sign/verify), plus the NULL-contract for schemes without derand.tests/vectors_sig.c— the ACVP keyGen vector'sseedis exactly FIPS-204's ξ, so this now also feeds it toOQS_SIG_keypair_derandand requires the result to match NIST's known-answer pk/sk — proving the public API really computesKeyGen_internal(ξ). Runs undertests/test_acvp_vectors.py.tests/fuzz_test_sig.c— exercisesOQS_SIG_keypair_derandunder libFuzzer/ASAN where supported.Locally: ACVP keyGen KAT green for ML-DSA-44/65/87 (with a negative control), libFuzzer/ASAN clean,
astyleclean on changed files, andtest_code_conventions.pypasses (265).Version / API checklist
OQS_SIGand a newOQS_SIG_keypair_derandsymbol, mirroringOQS_KEM. No algorithms are added/removed/renamed. Flagging for maintainer judgment on whether oqs-provider / benchmarking-list updates are wanted, given the change is purely additive.Generative AI disclosure
Portions of the test scaffolding and this description were drafted with the help of generative AI (Claude). The contributor has reviewed and verified the changes — including running the ACVP KAT, fuzzer, and convention checks — and affirms them.