Skip to content

Conversation

oskarpearson
Copy link

@oskarpearson oskarpearson commented Jan 29, 2025

  • Remove reference to 'secure_compare' method in ruby-jwt that this code shouldn't be using, so as to support ruby-jwt v2.6.0 and above
  • Add support for ruby-jwt up to and including version 3.* (currently in beta) so that we don't have to bump support again later
  • Add recent Ruby versions to the test matrix (I left version 3.0 in place, although it's no longer officially supported, so as to avoid inconveniencing existing users that are stuck on version 3.0)

Prior versions of this code depended on the method JWT::SecurityUtils.secure_compare

Release v2.6.0 of ruby-jwt moved the JWT::SecurityUtils.secure_compare method to a different namespace as part of jwt/ruby-jwt#521

While we could simply change this code to instead refer to that new namespace, I believe that the intention of ruby-jwt was that secure_compare should not have been used outside of the internals of ruby-jwt.

This is supported by the The ruby-jwt README examples where the ruby-jwt team recommend using OpenSSL.fixed_length_secure_compare instead of JWT::SecurityUtils.secure_compare

The code in this change is based on the logic in
https://github.com/rails/rails/blob/cf6ff17e9a3c6c1139040b519a341f55f0be16cf/activesupport/lib/active_support/security_utils.rb#L33 so as to avoid adding a dependency on ActiveSupport for this single method.

Unlike the code in the activesupport url above we don't fall back to a custom implementation of fixed_length_secure_compare, since OpenSSL.fixed_length_secure_compare is present in OpenSSL 2.2 and this gem already depends on Ruby 3.0 and above, which already includes that version of OpenSSL

This code also doesn't need to handle nil/empty cases, unlike the original implementation of JWT::SecurityUtils.secure_compare because these are already handled in the call to validate_url in one case, and validate_payload itself in the other.

… Ruby vers

- Remove reference to 'secure_compare' method in ruby-jwt that this
  code shouldn't be using, so as to support ruby-jwt v2.6.0 and above
- Add support for ruby-jwt up to and including version 3.* (currently in beta)
  so that we don't have to bump support again later
- Add recent Ruby versions to the test matrix (I left version 3.0 in place,
  although it's no longer officially supported, so as to avoid inconveniencing
  existing users that are stuck on version 3.0)

Prior versions of this code depended on the method `JWT::SecurityUtils.secure_compare`

Release v2.6.0 of ruby-jwt moved the `JWT::SecurityUtils.secure_compare` method to
a different namespace as part of jwt/ruby-jwt#521

While we could simply change this code to instead refer to that new namespace,
I believe that the intention of ruby-jwt was that `secure_compare` should not
have been used outside of the internals of ruby-jwt.

This is supported by the [The ruby-jwt README examples](https://github.com/jwt/ruby-jwt/blob/v3.0.0.beta1/README.md#custom-algorithms)
where the ruby-jwt team recommend using `OpenSSL.fixed_length_secure_compare`
instead of `JWT::SecurityUtils.secure_compare`

The code in this change is based on the logic in
https://github.com/rails/rails/blob/cf6ff17e9a3c6c1139040b519a341f55f0be16cf/activesupport/lib/active_support/security_utils.rb#L33
so as to avoid adding a dependency on ActiveSupport for this single method.

Unlike the code in the activesupport url above we don't fall back to a custom
implementation of `fixed_length_secure_compare`, since
`OpenSSL.fixed_length_secure_compare` [is present in OpenSSL 2.2](https://github.com/ruby/openssl/blob/master/History.md#version-220)
and this gem already depends on Ruby 3.0 and above, which already
[includes that version of OpenSSL](https://stdgems.org/openssl/)

This code also doesn't need to handle nil/empty cases, unlike the
[original implementation of JWT::SecurityUtils.secure_compare](https://github.com/jwt/ruby-jwt/pull/15/files#diff-2961689829b1ae20b5e8222673335c323355389f753c50b8a5b98380086f63b9R98)
because these are already handled in the call to `validate_url` in one case,
and `validate_payload` itself in the other.
@oskarpearson oskarpearson force-pushed the AR-18-add-support-for-jwt-2-and-3 branch from 61f595f to dae4ec0 Compare January 29, 2025 11:15
@oskarpearson oskarpearson merged commit a689237 into master Jan 29, 2025
5 checks passed
@oskarpearson oskarpearson deleted the AR-18-add-support-for-jwt-2-and-3 branch January 29, 2025 11:47
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