Add support for AES-GCM key wrap algorithms (A128GCMKW, A192GCMKW, A256GCMKW) #38
Conversation
vyperdon
force-pushed
the
feature-add-support-aes-gcm-algorithms
branch
from
7b803be to
9643786
Compare
…A256GCMKW) - Add new internal architecture with `Base`, `Validator`, `Header`, and `NameResolver` classes - Improve code organization with refactored module structure - RuboCop compliance improvements - Deprecated `JWE.check_params`, `JWE.check_alg`, `JWE.check_enc`, `JWE.check_zip`, `JWE.check_key` (use `JWE::Validator` instead) - Deprecated `JWE.param_to_class_name` (use `JWE::NameResolver` instead) - Deprecated internal methods `JWE.apply_zip`, `JWE.generate_header`, `JWE.generate_serialization`
vyperdon
force-pushed
the
feature-add-support-aes-gcm-algorithms
branch
from
9643786 to
8c0c9ee
Compare
|
@anakinj Hey 👋, could you please look into this? We are motivated to get those new algorithms supported, please share your feedback, we are happy to adjust the implementation accordingly 🫡 |
|
Hi @pandwoter I'll try to find time in the very near future to check this out. |
anakinj
left a comment
anakinj
left a comment
There was a problem hiding this comment.
Really nice refactoring happening here. Jotted down a few opinions and questions.
|
|
||
| header_parameters.merge!(zip: zip) if zip | ||
|
|
||
| header_parameters.merge!(alg_cipher.header_parameters) if alg_cipher.need_additional_header_parameters? |
There was a problem hiding this comment.
This need_additional_header_parameters method name bugs me a bit as there is two different additional header parameters in this logic.
Could it be just alg_cipher.require_header_parameters?
Or could the alg specific header_parameters return nothing in the cases it's not required?
|
|
||
| gemspec | ||
|
|
||
| gem 'pry' |
There was a problem hiding this comment.
Just out of curiosity, what extra does pry give over irb.
I feel that irb has gotten pretty powerful over the years and I have moved back to the vanilla tooling when it comes to console debugging.
|
|
||
| module JWE | ||
| # Generates JWE header from algorithm, encryption, and compression parameters | ||
| class Header |
There was a problem hiding this comment.
As there is no state I would prefer module and static methods over a class that needs instantiation in this case.
|
|
||
| module JWE | ||
| # Validates JWE parameters (algorithm, encryption, compression, and key) | ||
| class Validator |
There was a problem hiding this comment.
As the validator has no state I would prefer module and static methods over a class that needs instantiation in this case.
3 participants
Summary
This PR adds support for three new key management algorithms: A128GCMKW, A192GCMKW, and A256GCMKW as defined in RFC 7518.
Changes
New Features:
Backward Compatibility:
Architecture Changes:
I had to introduce a Base class for algorithms because the new AES-GCM key wrap algorithms need to configure
additional parameters in the JWE header (IV and authentication tag). This was not possible with the previous
architecture.
I also found it strange that some methods were publicly available but had no tests. Since I wasn't sure if
they were intended for external use, I marked them as deprecated instead of making them private. This
preserves backward compatibility while discouraging their use.
Let me know if you'd like me to adjust the wording or add anything!