Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shane Viktor ML-DSA/KEM merge #26715

Closed

Conversation

vdukhovni
Copy link

This is a rebase of the ML-DSA and then ML-KEM feature branches onto master.
Performed jointly by Viktor and Shane.
The only material conflicts that were judgement calls are in the TLS-related docs, please read text of the changes.

Checklist
  • documentation is added or updated
  • tests are added or updated

slontis and others added 30 commits February 12, 2025 13:03
The key generation algorithm requires a significant portion of the many
algorithms present in FIPS 204.

This work is derived from the BoringSSL code located at
https://boringssl.googlesource.com/boringssl/+/refs/heads/master/crypto/mldsa/mldsa.cc

Instead of c++ templates it uses an ML_DSA_PARAMS object to store constants such as k & l.
To perform hash operations a temporary EVP_MD_CTX object is used, which is supplied with a
prefetched EVP_MD shake128 or shake256 object that reside in the ML_DSA_KEY object.

The ML_DSA_KEY object stores the encoded public and/or private key
whenever a key is loaded or generated. A public  key is always present
if the private key component exists.

Reviewed-by: Viktor Dukhovni <[email protected]>
Reviewed-by: Tim Hudson <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
(Merged from openssl#26127)
… Matrix.

A DSA_KEY when created will alloc enough space to hold its k & l
vectors and then just set the vectors to point to the allocated blob.

Local Vectors and Matricies can then be initialised in a similar way by
passing them an array of Polnomials that are on the local stack.

Reviewed-by: Viktor Dukhovni <[email protected]>
Reviewed-by: Tim Hudson <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
(Merged from openssl#26127)
Reviewed-by: Viktor Dukhovni <[email protected]>
Reviewed-by: Tim Hudson <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
(Merged from openssl#26127)
- Make data encoding work on big-endian systems.

- Fix some ML-DSA-44 specific bugs related to w1-vector bits
  per-coefficient, overall size and high-bits rounding.

- Use "do { ... } while (pointer < end)" style consistently.

- Drop redundant reference counting of provided keys.

- Add parameter blocks for ML-DSA-44 and ML-DSA-87 and turn on
  associated provider glue.  These now pass both keygen and
  siggen tests (to be added separately).

Reviewed-by: Tim Hudson <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
(Merged from openssl#26127)
Reviewed-by: Viktor Dukhovni <[email protected]>
Reviewed-by: Tim Hudson <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
(Merged from openssl#26127)
Reviewed-by: Viktor Dukhovni <[email protected]>
Reviewed-by: Paul Dale <[email protected]>
(Merged from openssl#26451)
The encoded hint data consists of omega + k bytes.
The bytes at the end of omega section of the buffer may be 0,
so the buffer must be cleared initially.

Reviewed-by: Viktor Dukhovni <[email protected]>
Reviewed-by: Paul Dale <[email protected]>
(Merged from openssl#26451)
Reviewed-by: Viktor Dukhovni <[email protected]>
Reviewed-by: Paul Dale <[email protected]>
(Merged from openssl#26451)
The evp_test line buffer was increased to 32K to deal with the large
lines required for PQ messages and signatures.
The test data files were generated by parsing AVCP test files using
a python script.

Reviewed-by: Viktor Dukhovni <[email protected]>
Reviewed-by: Paul Dale <[email protected]>
(Merged from openssl#26451)
Reviewed-by: Viktor Dukhovni <[email protected]>
Reviewed-by: Paul Dale <[email protected]>
(Merged from openssl#26451)
Reviewed-by: Viktor Dukhovni <[email protected]>
Reviewed-by: Tim Hudson <[email protected]>
(Merged from openssl#26483)
Reviewed-by: Viktor Dukhovni <[email protected]>
Reviewed-by: Tim Hudson <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
(Merged from openssl#26400)
Reviewed-by: Viktor Dukhovni <[email protected]>
Reviewed-by: Tim Hudson <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
(Merged from openssl#26400)
Also remove some ACVP test data from ml_dsa.inc since this is now
also done using evp_test.

Reviewed-by: Paul Dale <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#26505)
Reviewed-by: Paul Dale <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#26505)
Using param builder consumes more resources and it is only beneficial
when dealing with bignums.  Directly using the param helpers is a better
alternative.

Reviewed-by: Matt Caswell <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#26529)
Reviewed-by: Viktor Dukhovni <[email protected]>
Reviewed-by: Paul Dale <[email protected]>
Reviewed-by: Tim Hudson <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#26575)
branch.

Reviewed-by: Tim Hudson <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
Reviewed-by: Viktor Dukhovni <[email protected]>
(Merged from openssl#26575)
commandline.

In order to support this gettables are required in both the key and
signature.:

Reviewed-by: Tim Hudson <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
Reviewed-by: Viktor Dukhovni <[email protected]>
(Merged from openssl#26575)
Reviewed-by: Tim Hudson <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
Reviewed-by: Viktor Dukhovni <[email protected]>
(Merged from openssl#26575)
Reviewed-by: Tim Hudson <[email protected]>
Reviewed-by: Shane Lontis <[email protected]>
Reviewed-by: Viktor Dukhovni <[email protected]>
(Merged from openssl#26548)
Reviewed-by: Tim Hudson <[email protected]>
Reviewed-by: Shane Lontis <[email protected]>
Reviewed-by: Viktor Dukhovni <[email protected]>
(Merged from openssl#26548)
Reviewed-by: Tim Hudson <[email protected]>
Reviewed-by: Shane Lontis <[email protected]>
Reviewed-by: Viktor Dukhovni <[email protected]>
(Merged from openssl#26548)
Reviewed-by: Tim Hudson <[email protected]>
Reviewed-by: Shane Lontis <[email protected]>
Reviewed-by: Viktor Dukhovni <[email protected]>
(Merged from openssl#26548)
The ossl_ml_dsa_key_get0_libctx() and the various size macros are better in the intneral header

Reviewed-by: Tim Hudson <[email protected]>
Reviewed-by: Shane Lontis <[email protected]>
Reviewed-by: Viktor Dukhovni <[email protected]>
(Merged from openssl#26548)
Reviewed-by: Tim Hudson <[email protected]>
Reviewed-by: Shane Lontis <[email protected]>
Reviewed-by: Viktor Dukhovni <[email protected]>
(Merged from openssl#26548)
Reviewed-by: Tim Hudson <[email protected]>
Reviewed-by: Shane Lontis <[email protected]>
Reviewed-by: Viktor Dukhovni <[email protected]>
(Merged from openssl#26548)
Reviewed-by: Tim Hudson <[email protected]>
Reviewed-by: Shane Lontis <[email protected]>
Reviewed-by: Viktor Dukhovni <[email protected]>
(Merged from openssl#26548)
Reviewed-by: Tim Hudson <[email protected]>
Reviewed-by: Shane Lontis <[email protected]>
Reviewed-by: Viktor Dukhovni <[email protected]>
(Merged from openssl#26548)
@t8m t8m added triaged: feature The issue/pr requests/adds a feature tests: present The PR has suitable tests present labels Feb 13, 2025
@t-j-h t-j-h added this to the 3.5.0 Feature Complete milestone Feb 13, 2025
@t8m t8m assigned t8m and unassigned vdukhovni Feb 13, 2025
@t8m
Copy link
Member

t8m commented Feb 13, 2025

I'll do the merge tomorrow morning after 24hrs pass.

Copy link
Author

@vdukhovni vdukhovni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving additional commits from @t8m.

Copy link
Contributor

@paulidale paulidale left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving the parts I didn't do.

openssl-machine pushed a commit that referenced this pull request Feb 14, 2025
Reviewed-by: Tomas Mraz <[email protected]>
Reviewed-by: Tim Hudson <[email protected]>
Reviewed-by: Neil Horman <[email protected]>
(Merged from #26715)
openssl-machine pushed a commit that referenced this pull request Feb 14, 2025
Reviewed-by: Tim Hudson <[email protected]>
Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from #26715)
openssl-machine pushed a commit that referenced this pull request Feb 14, 2025
Reviewed-by: Tim Hudson <[email protected]>
Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from #26715)
openssl-machine pushed a commit that referenced this pull request Feb 14, 2025
This has regressed with
#24799

The test configs have to be generated differently based
on the fips provider version.

Reviewed-by: Tim Hudson <[email protected]>
Reviewed-by: Neil Horman <[email protected]>
(Merged from #26715)
openssl-machine pushed a commit that referenced this pull request Feb 14, 2025
Reviewed-by: Tim Hudson <[email protected]>
Reviewed-by: Neil Horman <[email protected]>
(Merged from #26715)
@t8m
Copy link
Member

t8m commented Feb 14, 2025

This is now merged!

@t8m t8m closed this Feb 14, 2025
definability pushed a commit to definability/openssl that referenced this pull request Feb 25, 2025
Reviewed-by: Tomas Mraz <[email protected]>
Reviewed-by: Tim Hudson <[email protected]>
Reviewed-by: Neil Horman <[email protected]>
(Merged from openssl#26715)
definability pushed a commit to definability/openssl that referenced this pull request Feb 25, 2025
Reviewed-by: Tim Hudson <[email protected]>
Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#26715)
definability pushed a commit to definability/openssl that referenced this pull request Feb 25, 2025
Reviewed-by: Tim Hudson <[email protected]>
Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#26715)
definability pushed a commit to definability/openssl that referenced this pull request Feb 25, 2025
This has regressed with
openssl#24799

The test configs have to be generated differently based
on the fips provider version.

Reviewed-by: Tim Hudson <[email protected]>
Reviewed-by: Neil Horman <[email protected]>
(Merged from openssl#26715)
definability pushed a commit to definability/openssl that referenced this pull request Feb 25, 2025
quarckster pushed a commit to quarckster/openssl-fork that referenced this pull request Feb 27, 2025
Reviewed-by: Tomas Mraz <[email protected]>
Reviewed-by: Tim Hudson <[email protected]>
Reviewed-by: Neil Horman <[email protected]>
(Merged from openssl#26715)
quarckster pushed a commit to quarckster/openssl-fork that referenced this pull request Feb 27, 2025
Reviewed-by: Tim Hudson <[email protected]>
Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#26715)
quarckster pushed a commit to quarckster/openssl-fork that referenced this pull request Feb 27, 2025
Reviewed-by: Tim Hudson <[email protected]>
Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#26715)
quarckster pushed a commit to quarckster/openssl-fork that referenced this pull request Feb 27, 2025
This has regressed with
openssl#24799

The test configs have to be generated differently based
on the fips provider version.

Reviewed-by: Tim Hudson <[email protected]>
Reviewed-by: Neil Horman <[email protected]>
(Merged from openssl#26715)
quarckster pushed a commit to quarckster/openssl-fork that referenced this pull request Feb 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approval: done This pull request has the required number of approvals branch: master Merge to master branch extended tests Run extended tests in CI severity: fips change The pull request changes FIPS provider sources style: waived exempted from style checks tests: present The PR has suitable tests present triaged: feature The issue/pr requests/adds a feature
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Merge ML-DSA feature branch Merge ML-KEM feature branch
10 participants