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

ExternalMu Shuffle #65

Merged
merged 12 commits into from
Jan 14, 2025
Merged

ExternalMu Shuffle #65

merged 12 commits into from
Jan 14, 2025

Conversation

seanturner
Copy link
Contributor

Moved " Pre-hash Mode" section to an Appendix.

There are editorial tweaks, but more importantly 2119 language is removed from the Appendix. I want to call attention to the four (4) 2119 language changes:

  • reworked some of this into Security Considerations: This specification uses exclusively ExternalMu-ML-DSA for pre-hashed use cases, and thus HashML-DSA as defined in [FIPS204] and identified by id-hash-ml-dsa-44-with-sha512, id-hash-ml-dsa-65-with-sha512, and id-hash-ml-dsa-87-with-sha512 MUST NOT be used in X.509 and related PKIX protocols.
  • Implementions are RECOMMENDED -> whole paragraph re-written.
  • An ML-DSA key and certificate [MAY->can] be used with either ML-DSA or ExternalMu-ML-DSA interchangeably.
  • Implementors [SHOULD->should] to pay careful attention to how the public key or its hash is delivered to the ExternalMu-ML-DSA.Prehash() routine, and from where they are sourcing this data.

Also, if this PR is adopted we can close #54.

Moved " Pre-hash Mode" section to an Appendix.

There are editorial tweaks, but more importantly 2119 language is removed from the Appendix.  I want to call attention to the four (4) 2119 language changes:
* reworked some of this into Security Considerations: This specification uses exclusively ExternalMu-ML-DSA for pre-hashed use cases, and thus HashML-DSA as defined in [FIPS204] and identified by `id-hash-ml-dsa-44-with-sha512`, `id-hash-ml-dsa-65-with-sha512`, and `id-hash-ml-dsa-87-with-sha512` MUST NOT be used in X.509 and related PKIX protocols.
* Implementions are RECOMMENDED -> whole paragraph re-written.
* An ML-DSA key and certificate [MAY->can] be used with either ML-DSA
or ExternalMu-ML-DSA interchangeably.
* Implementors [SHOULD->should] to pay careful attention to how the public key or its hash is delivered to the `ExternalMu-ML-DSA.Prehash()` routine, and from where they are sourcing this data.
Copy link
Contributor

@csosto-pk csosto-pk left a comment

Choose a reason for hiding this comment

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

Good overall, but I proposed some improvements.

draft-ietf-lamps-dilithium-certificates.md Outdated Show resolved Hide resolved
draft-ietf-lamps-dilithium-certificates.md Outdated Show resolved Hide resolved
message pairs such that `H(tr || m1) = H(tr || m2)` since a simple hash
collision `H(m1) = H(m2)` will not suffice. HashML-DSA removes both of
these enhanced security properties and therefore is a weaker signature
algorithm.
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove "therefore is a weaker signature algorithm." In practice it is not weaker because SHA-3 does not have collisions. Better to not be too alarmist.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a less alarmist wording that still captures the same idea?

Maybe "... and therefore weakens some of the security properties built in to the ML-DSA design" ?

Copy link
Contributor

Choose a reason for hiding this comment

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

I am proposing the below rephrase

[...] Also, this binding means that in the unlikely, theoretical case of a collision attack against SHA-3, an attacker would have to perform a public-key-specific collision search in order to find message pairs such that H(tr || m1) = H(tr || m2) since a simple hash collision H(m1) = H(m2) will not suffice. HashML-DSA removes both of these enhanced security properties. and therefore is a weaker signature algorithm.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the length and order of paragraphs gives undue emphasis to the security aspect. I don't mind mentioning the difference, but it should be toned down.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Concretely, can we move the security paragraph second?

draft-ietf-lamps-dilithium-certificates.md Outdated Show resolved Hide resolved
draft-ietf-lamps-dilithium-certificates.md Outdated Show resolved Hide resolved
draft-ietf-lamps-dilithium-certificates.md Outdated Show resolved Hide resolved
Copy link
Contributor

@ounsworth ounsworth left a comment

Choose a reason for hiding this comment

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

I'm good with moving this to an appendix.
I also like Panos' comments. Once those are resolved, I'm good with this being merged.

draft-ietf-lamps-dilithium-certificates.md Outdated Show resolved Hide resolved
draft-ietf-lamps-dilithium-certificates.md Outdated Show resolved Hide resolved
draft-ietf-lamps-dilithium-certificates.md Outdated Show resolved Hide resolved
seanturner and others added 2 commits January 7, 2025 08:13
Co-authored-by: Mike Ounsworth <mike@ounsworth.ca>
that ML-DSA and HashML-DSA are incompatible algorithms that require
different `Verify()` routines. This forwards to the protocol the
complexity of informing the client whether to use `ML-DSA.Verify()` or
`HashML-DSA.Verify()`. Additionally, since
Copy link
Collaborator

Choose a reason for hiding this comment

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

Also there is a hash that has to be specified in HashML-DSA.

@seanturner
Copy link
Contributor Author

@ounsworth @bwesterb @csosto-pk PTAL

@bwesterb
Copy link
Collaborator

bwesterb commented Jan 14, 2025

Apart from the comments I left just now, I'm ok with this change. It's still a bit wordy, but not too bad.

@bwesterb bwesterb self-requested a review January 14, 2025 13:40
@seanturner seanturner merged commit bf24775 into main Jan 14, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants