Add `crypto_sign_pk_from_sk` to top-level API by jakemas · Pull Request #714 · pq-code-package/mldsa-native

jakemas · 2025-11-18T20:27:22Z

Resolves C Implementation: Public Key from Private #44

mldsa/src/sign.h

mkannwischer

Thanks @jakemas!
Two blockers:

The function needs to be added to mldsa_native.h
I think some of the declassifications need to be removed

I also wonder if we should rename the function to crypto_sign_pk_from_sk to align with other functions?

mldsa/src/sign.h

mldsa/src/sign.c

mkannwischer

Thanks @jakemas for the changes! This looks good to me now.

mldsa/src/sign.c

github-actions

Mac Mini (M1, 2020) benchmarks (opt)

Details

Benchmark suite	Current: `d5e1c73`	Previous: `674b8bc`	Ratio
`ML-DSA-44 keypair`	`46487` cycles	`46496` cycles	`1.00`
`ML-DSA-44 sign`	`132757` cycles	`132772` cycles	`1.00`
`ML-DSA-44 verify`	`47842` cycles	`47842` cycles	`1`
`ML-DSA-65 keypair`	`81474` cycles	`81453` cycles	`1.00`
`ML-DSA-65 sign`	`219244` cycles	`219163` cycles	`1.00`
`ML-DSA-65 verify`	`80118` cycles	`80109` cycles	`1.00`
`ML-DSA-87 keypair`	`132625` cycles	`132596` cycles	`1.00`
`ML-DSA-87 sign`	`281125` cycles	`281212` cycles	`1.00`
`ML-DSA-87 verify`	`130395` cycles	`130395` cycles	`1`

This comment was automatically generated by workflow using github-action-benchmark.

github-actions

Arm Cortex-A76 (Raspberry Pi 5) benchmarks (opt)

Details

Benchmark suite	Current: `d5e1c73`	Previous: `674b8bc`	Ratio
`ML-DSA-44 keypair`	`115791` cycles	`115546` cycles	`1.00`
`ML-DSA-44 sign`	`376960` cycles	`377228` cycles	`1.00`
`ML-DSA-44 verify`	`120301` cycles	`120526` cycles	`1.00`
`ML-DSA-65 keypair`	`200397` cycles	`200179` cycles	`1.00`
`ML-DSA-65 sign`	`623689` cycles	`622967` cycles	`1.00`
`ML-DSA-65 verify`	`197920` cycles	`198351` cycles	`1.00`
`ML-DSA-87 keypair`	`327697` cycles	`327201` cycles	`1.00`
`ML-DSA-87 sign`	`791495` cycles	`791268` cycles	`1.00`
`ML-DSA-87 verify`	`324898` cycles	`324856` cycles	`1.00`

This comment was automatically generated by workflow using github-action-benchmark.

github-actions

Mac Mini (M1, 2020) benchmarks (no-opt)

Details

Benchmark suite	Current: `d5e1c73`	Previous: `674b8bc`	Ratio
`ML-DSA-44 keypair`	`115249` cycles	`115268` cycles	`1.00`
`ML-DSA-44 sign`	`430426` cycles	`430432` cycles	`1.00`
`ML-DSA-44 verify`	`122140` cycles	`122206` cycles	`1.00`
`ML-DSA-65 keypair`	`197283` cycles	`197166` cycles	`1.00`
`ML-DSA-65 sign`	`700190` cycles	`700196` cycles	`1.00`
`ML-DSA-65 verify`	`197625` cycles	`197602` cycles	`1.00`
`ML-DSA-87 keypair`	`325694` cycles	`325565` cycles	`1.00`
`ML-DSA-87 sign`	`884149` cycles	`884047` cycles	`1.00`
`ML-DSA-87 verify`	`329030` cycles	`328865` cycles	`1.00`

This comment was automatically generated by workflow using github-action-benchmark.

github-actions

Arm Cortex-A76 (Raspberry Pi 5) benchmarks (no-opt)

Details

Benchmark suite	Current: `d5e1c73`	Previous: `674b8bc`	Ratio
`ML-DSA-44 keypair`	`213789` cycles	`213817` cycles	`1.00`
`ML-DSA-44 sign`	`782200` cycles	`784050` cycles	`1.00`
`ML-DSA-44 verify`	`229962` cycles	`229513` cycles	`1.00`
`ML-DSA-65 keypair`	`385235` cycles	`384799` cycles	`1.00`
`ML-DSA-65 sign`	`1307962` cycles	`1314461` cycles	`1.00`
`ML-DSA-65 verify`	`375315` cycles	`375898` cycles	`1.00`
`ML-DSA-87 keypair`	`605968` cycles	`607035` cycles	`1.00`
`ML-DSA-87 sign`	`1623238` cycles	`1623460` cycles	`1.00`
`ML-DSA-87 verify`	`617584` cycles	`617252` cycles	`1.00`

This comment was automatically generated by workflow using github-action-benchmark.

oqs-bot

Intel Xeon 4th gen (c7i)

Details

Benchmark suite	Current: `d5e1c73`	Previous: `674b8bc`	Ratio
`ML-DSA-44 keypair`	`34826` cycles	`35939` cycles	`0.97`
`ML-DSA-44 sign`	`120719` cycles	`120718` cycles	`1.00`
`ML-DSA-44 verify`	`38106` cycles	`38141` cycles	`1.00`
`ML-DSA-65 keypair`	`61681` cycles	`62873` cycles	`0.98`
`ML-DSA-65 sign`	`200838` cycles	`201460` cycles	`1.00`
`ML-DSA-65 verify`	`62662` cycles	`62704` cycles	`1.00`
`ML-DSA-87 keypair`	`94020` cycles	`95260` cycles	`0.99`
`ML-DSA-87 sign`	`234791` cycles	`235558` cycles	`1.00`
`ML-DSA-87 verify`	`94036` cycles	`94737` cycles	`0.99`

This comment was automatically generated by workflow using github-action-benchmark.

oqs-bot

Intel Xeon 4th gen (c7i) (no-opt)

Details

Benchmark suite	Current: `d5e1c73`	Previous: `674b8bc`	Ratio
`ML-DSA-44 keypair`	`95908` cycles	`96057` cycles	`1.00`
`ML-DSA-44 sign`	`349152` cycles	`349268` cycles	`1.00`
`ML-DSA-44 verify`	`101404` cycles	`101803` cycles	`1.00`
`ML-DSA-65 keypair`	`163022` cycles	`163944` cycles	`0.99`
`ML-DSA-65 sign`	`565178` cycles	`565147` cycles	`1.00`
`ML-DSA-65 verify`	`165073` cycles	`164858` cycles	`1.00`
`ML-DSA-87 keypair`	`267098` cycles	`268355` cycles	`1.00`
`ML-DSA-87 sign`	`723299` cycles	`723103` cycles	`1.00`
`ML-DSA-87 verify`	`270824` cycles	`271795` cycles	`1.00`

This comment was automatically generated by workflow using github-action-benchmark.

oqs-bot

AMD EPYC 3rd gen (c6a)

Details

Benchmark suite	Current: `d5e1c73`	Previous: `674b8bc`	Ratio
`ML-DSA-44 keypair`	`69326` cycles	`69288` cycles	`1.00`
`ML-DSA-44 sign`	`185135` cycles	`184700` cycles	`1.00`
`ML-DSA-44 verify`	`69807` cycles	`69124` cycles	`1.01`
`ML-DSA-65 keypair`	`119325` cycles	`119443` cycles	`1.00`
`ML-DSA-65 sign`	`294920` cycles	`295201` cycles	`1.00`
`ML-DSA-65 verify`	`115135` cycles	`115118` cycles	`1.00`
`ML-DSA-87 keypair`	`202804` cycles	`201971` cycles	`1.00`
`ML-DSA-87 sign`	`386640` cycles	`385373` cycles	`1.00`
`ML-DSA-87 verify`	`193896` cycles	`193641` cycles	`1.00`

This comment was automatically generated by workflow using github-action-benchmark.

oqs-bot

Intel Xeon 3rd gen (c6i)

Details

Benchmark suite	Current: `d5e1c73`	Previous: `674b8bc`	Ratio
`ML-DSA-44 keypair`	`56848` cycles	`56917` cycles	`1.00`
`ML-DSA-44 sign`	`180172` cycles	`179672` cycles	`1.00`
`ML-DSA-44 verify`	`61090` cycles	`60918` cycles	`1.00`
`ML-DSA-65 keypair`	`99403` cycles	`99920` cycles	`0.99`
`ML-DSA-65 sign`	`297649` cycles	`296088` cycles	`1.01`
`ML-DSA-65 verify`	`99947` cycles	`100006` cycles	`1.00`
`ML-DSA-87 keypair`	`153370` cycles	`153471` cycles	`1.00`
`ML-DSA-87 sign`	`352161` cycles	`351626` cycles	`1.00`
`ML-DSA-87 verify`	`152016` cycles	`151962` cycles	`1.00`

This comment was automatically generated by workflow using github-action-benchmark.

oqs-bot

AMD EPYC 4th gen (c7a)

Details

Benchmark suite	Current: `d5e1c73`	Previous: `674b8bc`	Ratio
`ML-DSA-44 keypair`	`41377` cycles	`42240` cycles	`0.98`
`ML-DSA-44 sign`	`130261` cycles	`130842` cycles	`1.00`
`ML-DSA-44 verify`	`44326` cycles	`44445` cycles	`1.00`
`ML-DSA-65 keypair`	`72592` cycles	`72530` cycles	`1.00`
`ML-DSA-65 sign`	`212974` cycles	`212854` cycles	`1.00`
`ML-DSA-65 verify`	`72910` cycles	`72838` cycles	`1.00`
`ML-DSA-87 keypair`	`109007` cycles	`111974` cycles	`0.97`
`ML-DSA-87 sign`	`249340` cycles	`252780` cycles	`0.99`
`ML-DSA-87 verify`	`110323` cycles	`110309` cycles	`1.00`

This comment was automatically generated by workflow using github-action-benchmark.

oqs-bot

Graviton2

Details

Benchmark suite	Current: `d5e1c73`	Previous: `674b8bc`	Ratio
`ML-DSA-44 keypair`	`116104` cycles	`115816` cycles	`1.00`
`ML-DSA-44 sign`	`377627` cycles	`377624` cycles	`1.00`
`ML-DSA-44 verify`	`120720` cycles	`120558` cycles	`1.00`
`ML-DSA-65 keypair`	`201022` cycles	`200332` cycles	`1.00`
`ML-DSA-65 sign`	`624955` cycles	`623594` cycles	`1.00`
`ML-DSA-65 verify`	`198562` cycles	`198416` cycles	`1.00`
`ML-DSA-87 keypair`	`328680` cycles	`327949` cycles	`1.00`
`ML-DSA-87 sign`	`792082` cycles	`792354` cycles	`1.00`
`ML-DSA-87 verify`	`325473` cycles	`325213` cycles	`1.00`

This comment was automatically generated by workflow using github-action-benchmark.

oqs-bot

AMD EPYC 3rd gen (c6a) (no-opt)

Details

Benchmark suite	Current: `d5e1c73`	Previous: `674b8bc`	Ratio
`ML-DSA-44 keypair`	`135698` cycles	`135799` cycles	`1.00`
`ML-DSA-44 sign`	`541044` cycles	`541174` cycles	`1.00`
`ML-DSA-44 verify`	`148924` cycles	`148797` cycles	`1.00`
`ML-DSA-65 keypair`	`229510` cycles	`228686` cycles	`1.00`
`ML-DSA-65 sign`	`893688` cycles	`889946` cycles	`1.00`
`ML-DSA-65 verify`	`239113` cycles	`237363` cycles	`1.01`
`ML-DSA-87 keypair`	`373804` cycles	`373747` cycles	`1.00`
`ML-DSA-87 sign`	`1107920` cycles	`1107872` cycles	`1.00`
`ML-DSA-87 verify`	`387419` cycles	`387864` cycles	`1.00`

This comment was automatically generated by workflow using github-action-benchmark.

test/test_mldsa.c

hanno-becker

One question regarding declassification/zeroing of pk in case of failure remaining.

This change will make #713 harder, unfortunately, but we can cross that bridge when we get to it.

jakemas · 2025-11-21T17:34:54Z

One question regarding declassification/zeroing of pk in case of failure remaining.

This change will make #713 harder, unfortunately, but we can cross that bridge when we get to it.

Ok, let me know what you think -- I've added the zero of pk in failure case. If I have the declassifications in the incorrect place, please let me know specifically which ones and exactly where you would like me to move them and I'll do it :)

mldsa/src/sign.c

hanno-becker

Thanks @jakemas! I think some zeroizations are still missing, see comments. Also, the implementation can be streamlined and improved to not leak which check has failed; we also need to document that the function leaks whether the sk was valid (not just in its return value, but in its operation).

Refactor keygen to use a new function that derives t0 t1 tr pk from rho s1 s2 so that this function can also be called by a utility function pk_to_sk that generates the pk given the sk. We also include ct_memcmp for constant time comparison. Signed-off-by: Jake Massimo <jakemas@amazon.com>

In the very end of verify one has to compare the input challenge to the re-computed challenge. If they are equal (and some previous checks on h and z passed), the signature is valid. Currently, our constant-time tests do not declassify the message and we, hence, need to declassify in this final step. Before thi commit, the declassification would happen on the recomputed challenge just before the memcmp. Now that a constant-time memcmp was added in #714, we might as well use that and declassify only the result on the memcmp which feels a bit more naturual and is easier to justify. Signed-off-by: Matthias J. Kannwischer <matthias@kannwischer.eu>

In the very end of verify one has to compare the input challenge to the re-computed challenge. If they are equal (and some previous checks on h and z passed), the signature is valid. Currently, our constant-time tests do not declassify the message and we, hence, need to declassify in this final step. Before thi commit, the declassification would happen on the recomputed challenge just before the memcmp. Now that a constant-time memcmp was added in #714, we might as well use that; that plus a constant-time selections allows us to not use any declassifications in verification, i.e., we do not leak the verification result through timing. Signed-off-by: Matthias J. Kannwischer <matthias@kannwischer.eu>

At the end of verify one has to compare the input challenge to the re-computed challenge. If they are equal (and some previous checks on h and z passed), the signature is valid. Currently, our constant-time tests do not declassify the message and we, hence, need to declassify in this final step. Before this commit, the declassification would happen on the recomputed challenge just before the memcmp. Now that a constant-time memcmp was added in #714, we might as well use that; that, plus a constant-time selection, allows us to not use message-dependent branches. For now, we still declassify the result of the verification itself to allow branching on it, e.g. in crypto_sign_open. This may be further improved in subsequent work, making crypto_sign_open constant flow and leaving a potential declassification of the verification result to the call-sites. Signed-off-by: Matthias J. Kannwischer <matthias@kannwischer.eu>

At the end of verify one has to compare the input challenge to the re-computed challenge. If they are equal (and some previous checks on h and z passed), the signature is valid. Currently, our constant-time tests do not declassify the message and we, hence, need to declassify in this final step. Before this commit, the declassification would happen on the recomputed challenge just before the memcmp. Now that a constant-time memcmp was added in #714, we might as well use that; that, plus a constant-time selection, allows us to not use message-dependent branches. For now, we still declassify the result of the verification itself to allow branching on it, e.g. in crypto_sign_open. This may be further improved in subsequent work, making crypto_sign_open constant flow and leaving a potential declassification of the verification result to the call-sites. Signed-off-by: Matthias J. Kannwischer <matthias@kannwischer.eu> Signed-off-by: Hanno Becker <beckphan@amazon.co.uk>

hanno-becker · 2026-01-03T04:58:17Z

mldsa/src/ct.h

+   * XOR twice with s, separated by a value barrier, to prevent the compile
+   * from dropping the s computation in the loop.
+   */
+  return (uint8_t)((mld_value_barrier_u32((uint32_t)r) ^ s) ^ s);


This should not have been merged. The value barrier is in the wrong place, allowing the compiler to elide s, and somehow the CT mask from mlkem-native was dropped.

jakemas force-pushed the mldsa-pk-from-sk branch 5 times, most recently from 510f9bb to bd3181c Compare November 19, 2025 19:40

jakemas marked this pull request as ready for review November 19, 2025 20:13

jakemas requested a review from a team as a code owner November 19, 2025 20:13

jakemas mentioned this pull request Nov 20, 2025

Import mldsa-native aws/aws-lc#2849

Closed

2 tasks

hanno-becker reviewed Nov 20, 2025

View reviewed changes

mldsa/src/sign.h Outdated Show resolved Hide resolved

hanno-becker reviewed Nov 20, 2025

View reviewed changes

mldsa/src/sign.h Outdated Show resolved Hide resolved

hanno-becker reviewed Nov 20, 2025

View reviewed changes

mldsa/src/sign.h Outdated Show resolved Hide resolved

mkannwischer requested changes Nov 20, 2025

View reviewed changes

mldsa/src/sign.h Outdated Show resolved Hide resolved

mldsa/src/sign.h Outdated Show resolved Hide resolved

mldsa/src/sign.c Outdated Show resolved Hide resolved

mldsa/src/sign.c Outdated Show resolved Hide resolved

mldsa/src/sign.c Outdated Show resolved Hide resolved

jakemas force-pushed the mldsa-pk-from-sk branch 2 times, most recently from 00ff754 to 7fa39ba Compare November 20, 2025 20:23

mkannwischer changed the title ~~C: Public Key from Secret Key function~~ Add crypto_sign_pk_from_sk to top-level API Nov 21, 2025

mkannwischer approved these changes Nov 21, 2025

View reviewed changes

mkannwischer force-pushed the mldsa-pk-from-sk branch from 7fa39ba to d5e1c73 Compare November 21, 2025 02:35

hanno-becker reviewed Nov 21, 2025

View reviewed changes

mldsa/src/sign.c Outdated Show resolved Hide resolved

mkannwischer added the benchmark label Nov 21, 2025

github-actions bot reviewed Nov 21, 2025

View reviewed changes

oqs-bot reviewed Nov 21, 2025

View reviewed changes

hanno-becker reviewed Nov 21, 2025

View reviewed changes

test/test_mldsa.c Show resolved Hide resolved

hanno-becker requested changes Nov 21, 2025

View reviewed changes

jakemas force-pushed the mldsa-pk-from-sk branch from 1cd4dd7 to 7535659 Compare November 21, 2025 17:33

jakemas force-pushed the mldsa-pk-from-sk branch 2 times, most recently from 79cbd83 to 3f1cb8a Compare November 24, 2025 19:39

hanno-becker reviewed Nov 25, 2025

View reviewed changes

mldsa/src/sign.c Outdated Show resolved Hide resolved

hanno-becker reviewed Nov 25, 2025

View reviewed changes

mldsa/src/sign.c Outdated Show resolved Hide resolved

hanno-becker requested changes Nov 25, 2025

View reviewed changes

jakemas force-pushed the mldsa-pk-from-sk branch from 3637256 to 4a13c70 Compare November 25, 2025 19:13

hanno-becker approved these changes Nov 25, 2025

View reviewed changes

hanno-becker requested a review from mkannwischer November 25, 2025 19:37

mkannwischer force-pushed the mldsa-pk-from-sk branch from 4a13c70 to 42479c1 Compare November 28, 2025 07:37

mkannwischer approved these changes Nov 28, 2025

View reviewed changes

mkannwischer merged commit b248d02 into main Nov 28, 2025
328 checks passed

mkannwischer deleted the mldsa-pk-from-sk branch November 28, 2025 08:08

mkannwischer mentioned this pull request Nov 28, 2025

verify: Switch to constant-time memcmp #750

Closed

sgmenda mentioned this pull request Dec 3, 2025

ML-DSA: Missing Private Key Validation Checks aws/aws-lc#2874

Merged

hanno-becker reviewed Jan 3, 2026

View reviewed changes

hanno-becker mentioned this pull request Jan 3, 2026

Align mld_ct_memcmp with mlkem-native #837

Merged

Conversation

jakemas commented Nov 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mkannwischer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mkannwischer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Mac Mini (M1, 2020) benchmarks (opt)

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Arm Cortex-A76 (Raspberry Pi 5) benchmarks (opt)

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Mac Mini (M1, 2020) benchmarks (no-opt)

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Arm Cortex-A76 (Raspberry Pi 5) benchmarks (no-opt)

Uh oh!

oqs-bot left a comment

Choose a reason for hiding this comment

Intel Xeon 4th gen (c7i)

Uh oh!

oqs-bot left a comment

Choose a reason for hiding this comment

Intel Xeon 4th gen (c7i) (no-opt)

Uh oh!

oqs-bot left a comment

Choose a reason for hiding this comment

AMD EPYC 3rd gen (c6a)

Uh oh!

oqs-bot left a comment

Choose a reason for hiding this comment

Intel Xeon 3rd gen (c6i)

Uh oh!

oqs-bot left a comment

Choose a reason for hiding this comment

AMD EPYC 4th gen (c7a)

Uh oh!

oqs-bot left a comment

Choose a reason for hiding this comment

Graviton2

Uh oh!

oqs-bot left a comment

Choose a reason for hiding this comment

AMD EPYC 3rd gen (c6a) (no-opt)

Uh oh!

Uh oh!

hanno-becker left a comment

Choose a reason for hiding this comment

Uh oh!

jakemas commented Nov 21, 2025

Uh oh!

Uh oh!

Uh oh!

hanno-becker left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

hanno-becker Jan 3, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

jakemas commented Nov 18, 2025 •

edited

Loading

hanno-becker left a comment •

edited

Loading