Add enforce_hmac_key_length configuration option#716
Conversation
8a2e20e to
c6b8232
Compare
| algorithms: algorithms, | ||
| required_claims: required_claims | ||
| required_claims: required_claims, | ||
| hmac_min_key_length: hmac_min_key_length |
There was a problem hiding this comment.
Could we instead of letting users set the min length just let the gem validate against the recommended minimums, so just enable or disable the feature. Then in future versions we could enable it by default. Something like enforce_hmac_key_length
I think this would be a bit less complex and easier for users to toggle on.
There was a problem hiding this comment.
Or is there a use-case where you would like to enforce a higher minimum than the default...
There was a problem hiding this comment.
Good point! I initially considered making it configurable, but default minimums should be sufficient.
I’ve pushed new changes to the branch using the enforce_hmac_key_length boolean flag instead.
Thanks for the review, @anakinj! Could you please take another look?
| class Hmac | ||
| include JWT::JWA::SigningAlgorithm | ||
|
|
||
| MIN_KEY_LENGTHS = { |
There was a problem hiding this comment.
Future us could wonder where are these values coming from, a comment pointing to the RFC would be useful at this point.
Add boolean toggle to enforce minimum HMAC key lengths based on RFC 7518 Section 3.2 (HS256: 32, HS384: 48, HS512: 64 bytes). Default is false (opt-in), allowing future versions to enable by default.
c6b8232 to
27a6325
Compare
Description
Add
enforce_hmac_key_lengthconfiguration option to enforce minimum key lengths for HMAC algorithms. This is a security hardening measure that helps prevent brute-force attacks on weak keys.Recommended minimum key lengths per RFC 7518 Section 3.2:
See: https://www.rfc-editor.org/rfc/rfc7518.html#section-3.2
Usage:
For backward compatibility, it's disabled by default (false). But it would be nice to enforce it in a future major version like it's done for RSA key validation (minimum 2048 bits). For now, users must explicitly opt-in.
Checklist
Before the PR can be merged be sure the following are checked: