The Wayback Machine - https://web.archive.org/web/20220501182709/https://github.com/jwt/ruby-jwt/pull/312
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improvement/encode hmac without key #312

Merged
merged 1 commit into from Jul 7, 2020
Merged

Improvement/encode hmac without key #312

merged 1 commit into from Jul 7, 2020

Conversation

JotaSe
Copy link
Contributor

@JotaSe JotaSe commented May 17, 2019

Description

For hmac strategy, will raise an exception if the secret key is nil but no when secret key is blank, for both cases should be the same.

Analysis

in lib/jwt/algos/hmac.rbthe key value is used for

SecurityUtils.rbnacl_fixup(algorithm, key.to_s)

and

OpenSSL::HMAC.digest(OpenSSL::Digest.new(algorithm.sub('HS', 'sha')), key.to_s, msg)

in both cases, the key should be a string for https://ruby-doc.org/stdlib-2.4.0/libdoc/openssl/rdoc/OpenSSL/HMAC.html and

# lib/jwt/security_utils.rb:49
# lib/jwt/security_utils.rb:53
key.bytesize

Solution:

in JWT::Algos::Hmac#sign set key ||= '' to convert nil to String

QA

  • Run tests
  • bin/console.rb =>
# Without secret key
token = JWT.encode payload, nil, 'HS256'

# eyJhbGciOiJIUzI1NiJ9.eyJkYXRhIjoidGVzdCJ9.pVzcY2dX8JNM3LzIYeP2B1e1Wcpt1K3TWVvIYSF4x-o
puts token

decoded_token = JWT.decode token, nil, true, { algorithm: 'HS256' }

# Array
# [
#   {"data"=>"test"}, # payload
#   {"alg"=>"HS256"} # header
# ]
puts decoded_token

@sourcelevel-bot
Copy link

@sourcelevel-bot sourcelevel-bot bot commented May 17, 2019

Hello, @JotaSe! This is your first Pull Request that will be reviewed by Ebert, an automatic Code Review service. It will leave comments on this diff with potential issues and style violations found in the code as you push new commits. You can also see all the issues found on this Pull Request on its review page. Please check our documentation for more information.

@rabajaj0509
Copy link

@rabajaj0509 rabajaj0509 commented Oct 3, 2019

@JotaSe good catch! mind squashing the commits to one?

@JotaSe
Copy link
Author

@JotaSe JotaSe commented Oct 3, 2019

Sure @rahulbajaj0509

@JotaSe JotaSe force-pushed the improvement/encode_hmac_without_key branch from 22f6430 to 6fa95b9 Compare Oct 3, 2019
@JotaSe
Copy link
Author

@JotaSe JotaSe commented Oct 3, 2019

Done @rahulbajaj0509

@sourcelevel-bot
Copy link

@sourcelevel-bot sourcelevel-bot bot commented Oct 3, 2019

SourceLevel has finished reviewing this Pull Request and has found:

  • 1 possible new issue (including those that may have been commented here).
  • 2 fixed issues! 🎉

But beware that this branch is 14 commits behind the jwt:master branch, and a review of an up to date branch would produce more accurate results.

You can see more details about this review at https://app.sourcelevel.io/github/jwt/ruby-jwt/pulls/312.

rabajaj0509
rabajaj0509 approved these changes Oct 4, 2019
View changes
Copy link
Contributor

@rabajaj0509 rabajaj0509 left a comment

LGTM!

@excpt excpt added this to the Version 2.2.2 milestone Jul 7, 2020
@excpt excpt self-requested a review Jul 7, 2020
excpt
excpt approved these changes Jul 7, 2020
View changes
@excpt excpt merged commit 95d8d9f into jwt:master Jul 7, 2020
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@excpt excpt

@rabajaj0509 rabajaj0509

Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
Version 2.2.2
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@JotaSe @rabajaj0509 @excpt