Feat/crypto registry aes gcm siv #759
Conversation
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.
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
stevespringett
left a comment
stevespringett
left a comment
There was a problem hiding this comment.
Thanks for the PR. Could you resolve the issue that @bhess mentioned? Once corrected, we'll merge this in. Thanks again
Mehrn0ush
force-pushed
the
feat/crypto-registry-aes-gcm-siv
branch
from
36fd6f4 to
c36a357
Compare
Signed-off-by: Mehrn0ush <mehrnoush.vaseghi@gmail.com>
Signed-off-by: Mehrn0ush <mehrnoush.vaseghi@gmail.com>
Mehrn0ush
force-pushed
the
feat/crypto-registry-aes-gcm-siv
branch
from
c36a357 to
52a3a59
Compare
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)