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

Mostly public key encoding stuff, plus some cleanup. #6

Merged
merged 8 commits into from
Nov 14, 2024

Conversation

danvangeest
Copy link
Collaborator

No description provided.

Copy link
Collaborator Author

@danvangeest danvangeest left a comment

Choose a reason for hiding this comment

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

Comments for other reviewers.

The signed-data content type as described in {{RFC5652}} does not encode public keys directly, but CMS content types defined by other documents do.
For example, {{?RFC5272}} describes Certificate Management over CMS, and its PKIData content utilises the SubjectPublicKeyInfo type to encode public keys for certificate requests.
When the SubjectPublicKeyInfo type is used in CMS, ML-DSA public keys MUST be encoded as described in {{!I-D.ietf-lamps-dilithium-certificates}}
{{!RFC5280}} defines the SubjectPublicKeyInfo ASN.1 type.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ietf-lamps-dilithium-certificates refers to use for public key encoding and ANS.1, so I added more text.

In the pre-hash mode, a message digest (the "pre-hash") is calculated separately and supplied to the signature algorithm as described above.
In the pure mode, the message to be signed or verified is instead supplied directly to the signature algorithm.
ML-DSA also supports a pre-hash and pure mode, though we follow the convention set by EdDSA in CMS {{?RFC8419}} and SLH-DSA in CMS {{?I-D.ietf-lamps-cms-sphincs-plus}} in that only the pure mode of ML-DSA is used in CMS.
That is, the pre-hash mode of ML-DSA MUST NOT be used in CMS.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I removed this line because it's not in the other PQ documents, and if it's ever added in the future this makes that easier maybe?

@@ -212,10 +214,6 @@ One method is used when signed attributes are present in the signedAttrs field o
Each method produces a different "message digest" to be supplied to the signature algorithm in question, but because the pure mode of ML-DSA is used, the "message digest" is in fact the entire message.
Use of signed attributes is preferred, but the conventions for signed-data without signed attributes is also described below for completeness.

EDNOTE: Would it make sense to make a stronger statement here?
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think no, at least to align with cms-sphincs-plus. The CMS-EUF-CMA work can address this. If that determines we need more text here and in cms-sphincs-plus we can do that then.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I know the WG wanted a quick turnaround on this draft - are we expecting it to wait on the CMS-EUF-CMA work (the interim for which will likely be in the new year)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I will post a summary of the EUF-CMA mitigations to the list since the presentation was cut short. Hopefully responses to that will give an indication of where the WG wants to go from there.

cms-sphincs-plus has gone through WGLC, so I don't think the EUF-CMA stuff would prevent this draft from doing the same. If the WG determines that the EUF-CMA issue needs to be solved immediately, both drafts would need to be updated. But there is a more general EUF-CMA solution that also allows these drafts to proceed now.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Makes sense to me. There are mitigations that can be applied in the interim for the EUF-CMA issue (e.g. only accepting one of signed attributes/no signed attributes, not both), so it's not like implementers have no options if they're worried about it prior to a separate RFC addressing the problem.

I'm happy with the text you've suggested - it could be worth noting the issue in the Security Considerations perhaps, and that future CMS RFCs may address this. On the other hand, I guess it's not an issue that's specific to ML-DSA.

@@ -233,11 +231,6 @@ This is as true for ML-DSA as it is for SLH-DSA, although ML-DSA signature gener
ML-DSA has a context string input that can be used to ensure that different signatures are generated for different application contexts.
When using ML-DSA as described in this document, the context string is not used.

EDNOTE: It's been suggested that the context string could be used to separate content-only/signed attributes signatures.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

see previous comment

@danvangeest danvangeest changed the title Comments from my initial review Mostly public key encoding stuff, plus some cleanup. Nov 9, 2024
@@ -69,7 +63,7 @@ informative:

--- abstract

The Module-Lattice-Based Digital Signature Algorithm (ML-DSA), as defined in FIPS 204, is a post-quantum digital signature scheme that aims to be secure against an adversary in posession of a Cryptographically Relevant Quantum Computer (CRQC).
The Module-Lattice-Based Digital Signature Algorithm (ML-DSA), as defined in FIPS 204 {{FIPS204}}, is a post-quantum digital signature scheme that aims to be secure against an adversary in possession of a Cryptographically Relevant Quantum Computer (CRQC).
Copy link
Collaborator

Choose a reason for hiding this comment

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

IIRC idnits has previously complained when we included a reference in the abstract, although I note that it isn't complaining currently. Not sure what's different in this case

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

https://ietf.github.io/id-guidelines/ still says to avoid citations in the abstract so I'll remove it.


Prior to standardisation, the algorithm was known as Dilithium. ML-DSA and Dilithium are not compatible.
It offers smaller signatures and significantly faster runtimes than SLH-DSA {{FIPS205}}, an alternative post-quantum signature algorithm also standardised by NIST.
This document specifies the use of the ML-DSA in CMS at three security levels: ML-DSA-44, ML-DSA-65, and ML-DSA-87. See {{Appendix B of I-D.ietf-lamps-dilithium-certificates}} for more information on the security levels and keys sizes of ML-DSA.
Copy link
Collaborator

Choose a reason for hiding this comment

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

s/keys sizes/key sizes/

@@ -212,10 +214,6 @@ One method is used when signed attributes are present in the signedAttrs field o
Each method produces a different "message digest" to be supplied to the signature algorithm in question, but because the pure mode of ML-DSA is used, the "message digest" is in fact the entire message.
Use of signed attributes is preferred, but the conventions for signed-data without signed attributes is also described below for completeness.

EDNOTE: Would it make sense to make a stronger statement here?
Copy link
Collaborator

Choose a reason for hiding this comment

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

I know the WG wanted a quick turnaround on this draft - are we expecting it to wait on the CMS-EUF-CMA work (the interim for which will likely be in the new year)?

@@ -212,10 +214,6 @@ One method is used when signed attributes are present in the signedAttrs field o
Each method produces a different "message digest" to be supplied to the signature algorithm in question, but because the pure mode of ML-DSA is used, the "message digest" is in fact the entire message.
Use of signed attributes is preferred, but the conventions for signed-data without signed attributes is also described below for completeness.

EDNOTE: Would it make sense to make a stronger statement here?
Copy link
Collaborator

Choose a reason for hiding this comment

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

Makes sense to me. There are mitigations that can be applied in the interim for the EUF-CMA issue (e.g. only accepting one of signed attributes/no signed attributes, not both), so it's not like implementers have no options if they're worried about it prior to a separate RFC addressing the problem.

I'm happy with the text you've suggested - it could be worth noting the issue in the Security Considerations perhaps, and that future CMS RFCs may address this. On the other hand, I guess it's not an issue that's specific to ML-DSA.

adam-r-ncsc and others added 2 commits November 12, 2024 16:11
Co-authored-by: Daniel Van Geest <[email protected]>
Co-authored-by: Daniel Van Geest <[email protected]>
@danvangeest
Copy link
Collaborator Author

Requesting approval so I can merge, if there are no other comments and I didn't miss anything.

@danvangeest danvangeest merged commit 36e356b into main Nov 14, 2024
2 checks passed
@danvangeest danvangeest deleted the dvg/my_original_review branch November 14, 2024 15:37
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.

3 participants