Skip to content

feat: add AES Key Wrap/Unwrap (RFC 3394) support#26

Closed
harry-pham-wise wants to merge 1 commit into
masterfrom
fix
Closed

feat: add AES Key Wrap/Unwrap (RFC 3394) support#26
harry-pham-wise wants to merge 1 commit into
masterfrom
fix

Conversation

@harry-pham-wise

@harry-pham-wise harry-pham-wise commented Jun 4, 2026

Copy link
Copy Markdown

Context

Add KeyWrap and KeyUnwrap methods to the AES Cipher struct, implementing RFC 3394 AES Key Wrap using the existing github.com/ProtonMail/go-crypto library (already an indirect dependency, now promoted to direct).

Summary:

  • Added KeyWrap(plainKeyBytes) and KeyUnwrap(wrappedKeyBytes) methods to Cipher
  • Added tests including RFC 3394 test vector validation
  • Bumped version to 1.13.0

Checklist

User prompts that led to this change
  1. "add a function KeyWrap in aes/aes_cipher.go, use existing library if needed. should implement AES Key Wrap (RFC 3394)"

    • Initial request to add key wrap functionality
  2. "hmm that's a lot of code, any library that can just wrap, unwrap?"

    • Prompted refactor from manual implementation to using ProtonMail's existing keywrap library
  3. "can you double check the code will work if I send in plainKeyBytes for wrap?"

    • Verification that the function works with standard AES key byte inputs
  4. "rename it to plainKeyBytes then, add test in aes/aes_cipher_test.go too."

    • Renamed parameter and added test coverage including RFC 3394 test vectors

🤖 Generated with Claude Code

Co-Authored-By: Claude <noreply@anthropic.com>
@harry-pham-wise harry-pham-wise requested a review from a team as a code owner June 4, 2026 03:21
Copilot AI review requested due to automatic review settings June 4, 2026 03:21
@platon-github-app-production

Copy link
Copy Markdown

Comment /request-review to automatically request reviews from the following teams:

You can also request review from a specific team by commenting /request-review team-name, or you can add a description with --notes "<message>"

💡 If you see something that doesn't look right, check the configuration guide.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Changes: New feature (1), Test improvement (1)

This PR adds AES Key Wrap/Unwrap support (RFC 3394) to the existing AES Cipher wrapper by delegating to the github.com/ProtonMail/go-crypto keywrap implementation, and introduces tests to validate round-trips and an RFC 3394 test vector.

Changes:

  • Add KeyWrap / KeyUnwrap methods on aes.Cipher using ProtonMail’s keywrap implementation.
  • Add unit tests covering wrap/unwrap round-trip, an RFC 3394 known-answer vector, and invalid unwrap input.
  • Promote github.com/ProtonMail/go-crypto to a direct dependency and bump the library version.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
VERSION.txt Bumps module version to 1.13.0.
go.mod Promotes github.com/ProtonMail/go-crypto to a direct dependency (required by new keywrap import).
aes/aes_cipher.go Adds KeyWrap and KeyUnwrap methods implementing RFC 3394 via keywrap.Wrap/Unwrap.
aes/aes_cipher_test.go Adds tests for wrap/unwrap round-trip, RFC 3394 test vector, and invalid unwrap input.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread aes/aes_cipher_test.go
Comment on lines +95 to +103
wrapped, err := cipher.KeyWrap(plainKeyBytes)
if err != nil {
t.Fatalf("Did not expect a KeyWrap error but got %q", err)
}

unwrapped, err := cipher.KeyUnwrap(wrapped)
if err != nil {
t.Fatalf("Did not expect a KeyUnwrap error but got %q", err)
}
Comment thread aes/aes_cipher_test.go
Comment on lines +118 to +121
wrapped, err := cipher.KeyWrap(plainKeyBytes)
if err != nil {
t.Fatalf("Did not expect a KeyWrap error but got %q", err)
}
@harry-pham-wise harry-pham-wise marked this pull request as draft June 4, 2026 03:42
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.

2 participants