Add signing hooks for observing signature attempts and implementing restartable signing#1237
Add signing hooks for observing signature attempts and implementing restartable signing#1237hanno-becker wants to merge 9 commits into
Conversation
CBMC Results (ML-DSA-44, REDUCE-RAM)Full Results (208 proofs)
|
CBMC Results (ML-DSA-65)Full Results (208 proofs)
|
CBMC Results (ML-DSA-44)Full Results (208 proofs)
|
CBMC Results (ML-DSA-87)
Full Results (208 proofs)
|
ba5df63 to
004fcc0
Compare
CBMC Results (ML-DSA-65, REDUCE-RAM)Full Results (208 proofs)
|
CBMC Results (ML-DSA-87, REDUCE-RAM)Full Results (208 proofs)
|
b2ef39d to
4acce0a
Compare
ec79a2c to
9686b66
Compare
mkannwischer
left a comment
There was a problem hiding this comment.
Thanks @hanno-becker. I have done a design review for now and have some questions - a full review will follow later.
Could you elaborate why you went for hooks to implement this approach rather than adding a _restartable() API that gets a context argument?
It would be good to add an example of how the new hooks can be used. We'll also need to add documentation.
| /* Signing succeeded: record the attempt that succeeded. No-op in the | ||
| * default build. */ | ||
| mld_sign_finish(attempt, context); |
There was a problem hiding this comment.
What is mld_sign_finish needed for? It looks like the same information is passed to the last mld_sign_attempt
There was a problem hiding this comment.
It's for ease of use and a symmetric API. For example, if you only care about pairing your benchmarks with information of how many signing attempts were performanced, finish gives you that information most directly.
But you are right, in theory one can get away without it. I'd prefer not to.
Hooks are more flexible and less work for us. The details of how to build a restartable API on top of them are left to the user -- by design. For example, you'd probably also want randomized signing to be restartable; that's something you can easily build yourself. If, on the other hand, we were to do it ourselves, we'd have to define some kind of restart context, which then clashes with the customer's context and is also hard to keep stable if we introduce, say, structured keys. The hooks are the core mechanism. And the hooks are useful for statistics and benchmarking too, not just for restartability. |
What documentation do you want, beyond the extensive one in
|
7096d33 to
b56162f
Compare
Move the kappa upper bound MLD_MAX_KAPPA -- the largest counter kappa keeping the y-sampling nonces within uint16_t -- from sign.c to params.h, beside MLDSA_L which it derives from. Replace the open-coded `UINT16_MAX - MLDSA_L` in the polyvec contracts and comments with it. No functional change. Signed-off-by: Hanno Becker <beckphan@amazon.co.uk>
Functions carrying the optional MLD_CONFIG_CONTEXT_PARAMETER that do not otherwise use it must consume it to avoid -Wunused-parameter, but only when a context is actually configured. This was previously done via an #if-guarded ((void)context). Introduce MLD_CONTEXT_UNUSED(context) in context.h, expanding to ((void)(context)) when a context parameter is configured and to an empty statement otherwise, and use it in the default (stack) MLD_FREE and in the skip-PCT stub. No functional change. Signed-off-by: Hanno Becker <beckphan@amazon.co.uk>
Redefine MLD_MAX_SIGNING_ATTEMPTS as the effective bound on signing attempts: the configured MLD_CONFIG_MAX_SIGNING_ATTEMPTS if set, otherwise the hard type-safety bound MLD_MAX_KAPPA / MLDSA_L. mld_get_max_signing_attempts() returns it directly, and its contract bounds the result by MLD_MAX_KAPPA / MLDSA_L (the hard bound), independent of the configured value. This keeps the type-safety bound (used by the loop and casts) distinct from the configured attempt limit, and prepares for the signing hooks, which clamp the resume point against the effective bound. No functional change. Signed-off-by: Hanno Becker <beckphan@amazon.co.uk>
The runtime of ML-DSA signatures is theoretically unbounded. Even the
bound of max 814 signature attempts -- approved by NIST as lowering the
chance of signing failure below 2^{-256} -- leaves a performance
distribution whose peak percentiles can be unacceptable for constrained
systems with strict timing requirements.
The library already provides MLD_CONFIG_MAX_SIGNING_ATTEMPTS to cap the
number of signing attempts. However, FIPS 204 forbids lowering that bound
below 814, rendering the option non-compliant in constrained environments.
As a remedy, this commit introduces three optional 'signing hooks' around
the core rejection-sampling loop, keyed by the attempt counter (the loop
iteration; the spec counter is kappa = attempt*MLDSA_L):
- `attempt = sign_resume()`: Return the attempt to resume the operation
from. `0` for an initial / non-restartable signing operation.
- `sign_attempt(attempt)`: Called before each attempt. Returns 0 to
proceed, or non-zero to pause: signing then returns MLD_ERR_SIGNING_PAUSED
with `attempt` as the resume point.
- `sign_finish(attempt)`: Called on success with the succeeding attempt.
Together, resume + attempt make signing _restartable_: a caller can pause
after a bounded number of attempts and later resume from the recorded
attempt, bounding per-call runtime while keeping a FIPS-compliant overall
bound and reproducing the same signature as an uninterrupted run.
This only works if the consumer drives the deterministic internal API: the
randomized API would choose a fresh seed on each (re-)start, so the resume
hook requires MLD_CONFIG_NO_RANDOMIZED_API and is incompatible with
MLD_CONFIG_KEYGEN_PCT (whose one-shot internal signature cannot be resumed).
The attempt and finish hooks also serve standalone uses with any API, e.g.
logging or benchmarking the per-signature attempt distribution.
Pausing is independent of MLD_CONFIG_MAX_SIGNING_ATTEMPTS, the upper bound on
the total number of attempts (>= 814 for FIPS 204 compliance) after which
signing gives up with the distinct error MLD_ERR_SIGN_ATTEMPTS_EXHAUSTED; it
must not be lowered as a restart mechanism.
sign_{resume|attempt|finish} can be implemented on mutable globals or on a
context, so they are introduced under the existing MLD_CONFIG_CONTEXT_PARAMETER
wrapper, which lets functions be called with or without an
application-specific context pointer.
This commit adds the core machinery in mldsa/:
- Independent MLD_CONFIG_SIGN_HOOK_{RESUME,ATTEMPT,FINISH} options. When set,
the integration provides, respectively:
- `attempt = mld_sign_hook_resume([ctx])`: provide initial/resume attempt
- `mld_sign_hook_attempt(attempt [, ctx])`: per-attempt hook; pause via
non-zero return
- `mld_sign_hook_finish(attempt [, ctx])`: note the succeeding attempt
The unset dummies leave the default build unchanged.
- The MLD_ERR_SIGNING_PAUSED error code, returned when the attempt hook pauses.
CBMC proofs, a test, and the regenerated configs follow in separate commits.
Signed-off-by: Hanno Becker <beckphan@amazon.co.uk>
Now that the context parameter feeds both the allocation hooks and the signing hooks, expand its documentation in mldsa_native_config.h: describe the trailing parameter added to every public API function, that mldsa-native treats the value as opaque and only forwards it to the hook macros, the unset-default behaviour, and the requirement that MLD_CONFIG_CONTEXT_PARAMETER_TYPE be defined if and only if MLD_CONFIG_CONTEXT_PARAMETER is. Documentation only. Signed-off-by: Hanno Becker <beckphan@amazon.co.uk>
Verify the three signing-hook call sites added in the previous commit: mld_sign_resume, mld_sign_attempt and mld_sign_finish. In the CBMC configuration the hooks reduce to their contract dummies, so each harness just checks the call-by-contract stub against its (empty) contract, mirroring how sign.c invokes them (the CBMC config carries no context parameter, so the `context` argument is consumed by the call macro exactly as at the call sites). Also refresh the sign_signature_internal harness Makefile for the reworked signing loop. Signed-off-by: Hanno Becker <beckphan@amazon.co.uk>
Add a test analogous to test_alloc, driven by a custom config (test_sign_hook_config.h) that enables the three signing-hook options and forward-declares the hook context; the context type and the hook implementations live in the test. The hooks pause signing after a configurable number of attempts and resume from the recorded attempt, letting one context drive: - a known-answer smoke test, reproducing the test-vector signature both one-shot and single-step; - a restartable-signing equivalence test, checking that pausing after a random number of attempts and resuming yields the same signature as an uninterrupted run, over many random signing-randomness draws; - a distribution test, gathering the attempts-per-signature histogram over many one-shot signatures and checking the measured attempts/signature ratio against the expected geometric mean for the parameter set. The test requires the internal signing API, so it compiles to a no-op main under MLD_CONFIG_NO_SIGN_API. Statistics are printed via printf only (no putchar), so freestanding platforms with a minimal stdio shim can run it. Wire it up in the Makefile, the test make fragments and scripts/tests (new --sign-hook subcommand). Mirror alloc's CI plumbing: add a sign_hook input to the functest / multi-functest actions and disable it for the config variations it is incompatible with -- those passing their own MLD_CONFIG_FILE (would clash with the test's fixed config) and those enabling MLD_CONFIG_KEYGEN_PCT (incompatible with SIGN_HOOK_RESUME). The generated configs follow in a separate commit. Signed-off-by: Hanno Becker <beckphan@amazon.co.uk>
Rerun scripts/autogen after the preceding commits, updating various files. Done separately to ease reviewing of the core of the changes. Signed-off-by: Hanno Becker <beckphan@amazon.co.uk>
Signed-off-by: Hanno Becker <beckphan@amazon.co.uk>
There was a problem hiding this comment.
Thanks @hanno-becker for all the work!
Found a few small issues - otherwise I'm very happy with this feature!
Zephyr CI is having some issues, could you fix that?
What documentation do you want, beyond the extensive one in mldsa_native_config.h?
Maybe a paragraph in the root README like we have for DMLD_CONFIG_REDUCE_RAM? We may want to revisit the README before v1 anyway though.
| /* #define MLD_CONFIG_CONTEXT_PARAMETER_TYPE void* */ | ||
|
|
||
| /** | ||
| * Signing hooks: MLD_CONFIG_SIGN_HOOK_RESUME / _ATTEMPT / _FINISH |
There was a problem hiding this comment.
Shall we mark the feature as experimental allowing us to still tweak it after v1 is out?
| * uint16_t iff kappa <= UINT16_MAX - MLDSA_L. With kappa = attempt*MLDSA_L this | ||
| * bounds the number of signing attempts by MLD_MAX_KAPPA / MLDSA_L; see | ||
| * MLD_MAX_SIGNING_ATTEMPTS in sign.c. */ | ||
| #define MLD_MAX_KAPPA (UINT16_MAX - MLDSA_L) |
There was a problem hiding this comment.
We need to include stdint.h ... this only works out of luck.
(but the int32_t casts would fail if we would be out of luck)
| if (iterations >= MLD_SIGN_HOOK_MIN_CHECK_N) | ||
| { | ||
| uint64_t lhs = 100u * a; | ||
| uint64_t mid = (uint64_t)MLD_SIGN_HOOK_EXPECTED_RATIO_X100 * s; |
There was a problem hiding this comment.
This never gets executed in CI if I see correctly. Should we add a separate job for it?
There was a problem hiding this comment.
This is a good point. I'll switch to 2000 iterations by default at a 10% interval (according to Claude, covering >99.999% of the cases), as well as a single 100k iteration run with a narrower band.
| * params.h). The default is chosen so that the failure probability is | ||
| * < 2^{-256}, that is, signatures will practically always succeed. */ |
There was a problem hiding this comment.
This may read confusing to the reader.
If the default is chosen such that the failure probability is < 2^{-256}, you would use the NIST bound of 814.
There was a problem hiding this comment.
Any bound >= 814 has that probability. I deliberately omitted the specific value here because I felt it's irrelevant to the user. But point taken, if you're confused, so will be others. I'll reconsider.
There was a problem hiding this comment.
I'll just drop "The default is chosen so that the failure probability is * < 2^{-256}, that is, signatures will practically always succeed." here.
| * When an option is unset, the hook is a no-op (resume to 0, attempt proceeds), | ||
| * i.e. ordinary one-shot signing. | ||
| * | ||
| * Independent of MLD_CONFIG_MAX_SIGNING_ATTEMPTS (the hard attempt bound, >= |
There was a problem hiding this comment.
Do we still need MLD_CONFIG_MAX_SIGNING_ATTEMPTS? May be worth removing to reduce complexity for v1.
There was a problem hiding this comment.
One could emulate this using an attempt-hook, if we gave the attempt-hook the possibility to inject a failure, not just a pause. But I think the added complexity of this approach -- both on our side, and on the user's side -- outweighs the benefit of removing this option.
One could consider defaulting to 814, but that's slightly odd seeing FIPS204 says one "SHOULD NOT" do this.
Setting this to 814 is quite natural, IMO, to improve observability in case of a faulty implementation looping; esp. on low-end systems.
Altogether, I prefer leaving this option.
There was a problem hiding this comment.
I'm not in favor of defaulting to 814 due to the SHOULD NOT.
Still I don't see why any consumer would set MLD_CONFIG_MAX_SIGNING_ATTEMPTS to anything other than 814 or the max value.
What do you think about making it a binary option to use 814 instead of the default?
There was a problem hiding this comment.
I'm in favor of keeping it as it is. It gives the consumer more flexibility, if only for testing purposes, without making things any more difficult on our end.
There was a problem hiding this comment.
keypair and keypair_internal can now also return MLD_ERR_SIGNING_PAUSED in case PCT and HOOK_ATTEMPT is configured. That should be documented.
There was a problem hiding this comment.
No, that's not allowed -- there would be no way to restart the PCT. The combination of hooks and PCT is checked for.
There was a problem hiding this comment.
Can you please double check this? The way I read the config (+ config validation) is that PCT is incompatible with RESUME, but not with ATTEMPT. Am I getting something wrong here?
There was a problem hiding this comment.
You spotted a real issue there, thank you. Initially, attempt couldn't signal pause (pause was handled via MAX_SIGNING_ATTEMPTS), in which case it was safe to use with PCT. Now, it is indeed ATTEMPT which cannot be used with PCT, while RESUME and FINISH, in theory, could.
Uh oh!
There was an error while loading. Please reload this page.