Feat/crypto registry aes gcm siv #759
Conversation
Signed-off-by: Mehrn0ush <[email protected]>
Signed-off-by: Mehrn0ush <[email protected]>
Signed-off-by: Mehrn0ush <[email protected]>
bhess
left a comment
bhess
left a comment
There was a problem hiding this comment.
Thank you, @Mehrn0ush, for adding AES-GCM-SIV, this looks good to me.
Please check my comment regarding the pattern.
I also noticed some changes duplicated from your other PRs (XChaCha20, TUAK, and bcrypt). You might want to remove those from this PR to ensure a clean merge.
| "url": "https://doi.org/10.17487/RFC8452" | ||
| } | ||
| ], | ||
| "pattern": "AES[-(128|192|256)]-GCM-SIV[-{tagLength}][-{ivLength}]", |
There was a problem hiding this comment.
After reading the RFC, I understand that in AES-GCM-SIV the tagLength and ivLength are fixed at 16 bytes and 12 bytes, respectively. Since these values are well-defined by the RFC, the [-{tagLength}][-{ivLength}] portion can be removed from the pattern.
stevespringett
added
the
cap: cryptography-registry
There was a problem hiding this comment.
Pull request overview
This PR adds AES-GCM-SIV support to the Cryptography Registry as requested in issue #758. However, it also includes several unrelated changes that were not mentioned in the PR description.
Changes:
- Adds AES-GCM-SIV (RFC 8452) as an AEAD variant to the AES family
- Adds XChaCha20-Poly1305 to ChaCha20 family variants (not mentioned in PR description)
- Removes BLAKE2b-HMAC variant (not mentioned in PR description)
- Fixes spelling errors in TUAK patterns (TUAC -> TUAK)
- Fixes typo in bcrypt pattern (closing parenthesis -> closing brace)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }, | ||
| { | ||
| "pattern": "XChaCha20-Poly1305", | ||
| "primitive": "ae" |
There was a problem hiding this comment.
The addition of XChaCha20-Poly1305 is not mentioned in the PR description or issue #758. This change should either be removed from this PR and submitted separately with appropriate justification, or the PR description should be updated to explain why this variant is being added.
| }, | |
| { | |
| "pattern": "XChaCha20-Poly1305", | |
| "primitive": "ae" |
| { | ||
| "pattern": "BLAKE2s-(160|256)", | ||
| "primitive": "hash" | ||
| }, | ||
| { | ||
| "pattern": "BLAKE2b-(160|256|384|512)-HMAC", | ||
| "primitive": "mac" | ||
| } |
There was a problem hiding this comment.
The removal of the BLAKE2b-HMAC variant (lines 944-946 in the original) is not mentioned in the PR description or issue #758. This deletion should either be reverted and submitted in a separate PR with appropriate justification, or the PR description should be updated to explain why this variant is being removed.
stevespringett
left a comment
stevespringett
left a comment
There was a problem hiding this comment.
Please update the pattern based on the suggestion from @bhess
3 participants
As discussed in ticket #758, this PR adds AES-GCM-SIV as an AEAD variant to the Cryptography Registry.
Fixes #758
Details
aevariant under the existing AES familyScope
schema/cryptography-defs.json)