From 320306bbd64f0c30bc720c8bb0cd267c4b020dce Mon Sep 17 00:00:00 2001 From: lukas Date: Fri, 30 Oct 2015 10:34:34 +0100 Subject: [PATCH] relax restrictions on "jti" claim verification The RFC has the following to say about the "jti" claim: > The "jti" (JWT ID) claim provides a unique identifier for the JWT. > The identifier value MUST be assigned in a manner that ensures that > there is a negligible probability that the same value will be > accidentally assigned to a different data object; if the application > uses multiple issuers, collisions MUST be prevented among values > produced by different issuers as well. The "jti" claim can be used > to prevent the JWT from being replayed. The "jti" value is a case- > sensitive string. Use of this claim is OPTIONAL. > -> https://tools.ietf.org/html/rfc7519#section-4.1.7 The current implementation expects that the `jti` is built as: MD5( secret + ":" + iat ) anything else is not concidered a valid `jti`. There are several issues with the current implementation: 1. `secret + ":" + iat` is not exactly collision safe :) using random UUIDs is safer I'd say. 2. It was only verified when it was present in the payload, so it was skipped when it was missing from the payload 2. In the RFC there are no rules defined on how the "jti" claim should be validated This pull request simply relaxes the verification of the "jti" claim to be present. This is in line with other implementations like the one for Java or Python. (FYI, there are other implementations that perform additional checks, the one seen most common is to verify against a value supplied in the `verify` call) It is my opinion that the library should just provide basic validation and make no additional assumption about the "jti" claim. Any additional verification / validation (e.g. replay checks, value checks) must be checked by the application and should be out of the scope for the library. --- lib/jwt.rb | 5 ++--- spec/jwt_spec.rb | 17 +++++------------ 2 files changed, 7 insertions(+), 15 deletions(-) diff --git a/lib/jwt.rb b/lib/jwt.rb index 4c91c946..d0ae953f 100644 --- a/lib/jwt.rb +++ b/lib/jwt.rb @@ -179,9 +179,8 @@ def decode(jwt, key = nil, verify = true, options = {}, &keyfinder) if options[:verify_sub] && options.include?(:sub) fail(JWT::InvalidSubError, "Invalid subject. Expected #{options[:sub]}, received #{payload['sub'] || ''}") unless payload['sub'].to_s == options[:sub].to_s end - if options[:verify_jti] && payload.include?('jti') - fail(JWT::InvalidJtiError, 'need iat for verify jwt id') unless payload.include?('iat') - fail(JWT::InvalidJtiError, 'Not a uniq jwt id') unless options[:jti].to_s == Digest::MD5.hexdigest("#{key}:#{payload['iat']}") + if options[:verify_jti] + fail(JWT::InvalidJtiError, 'Missing jti') if payload['jti'].to_s == '' end [payload, header] diff --git a/spec/jwt_spec.rb b/spec/jwt_spec.rb index 738c2815..5f83fc33 100644 --- a/spec/jwt_spec.rb +++ b/spec/jwt_spec.rb @@ -359,27 +359,21 @@ context 'jwt id claim' do let :jti do - new_payload = payload.merge('iat' => Time.now.to_i) - key = data[:secret] - new_payload.merge('jti' => Digest::MD5.hexdigest("#{key}:#{new_payload['iat']}")) + payload.merge('jti' => 'some-random-uuid-or-whatever') end let(:token) { JWT.encode jti, data[:secret] } + let(:invalid_token) { JWT.encode payload, data[:secret] } - let :invalid_token do - jti.delete('iat') - JWT.encode jti, data[:secret] - end - - it 'invalid jti should raise JWT::InvalidJtiError' do + it 'missing jti should raise JWT::InvalidJtiError' do expect do - JWT.decode invalid_token, data[:secret], true, :verify_jti => true, 'jti' => jti['jti'] + JWT.decode invalid_token, data[:secret], true, verify_jti: true end.to raise_error JWT::InvalidJtiError end it 'valid jti should not raise JWT::InvalidJtiError' do expect do - JWT.decode token, data[:secret], true, verify_jti: true, jti: jti['jti'] + JWT.decode token, data[:secret], true, verify_jti: true end.to_not raise_error end end @@ -408,5 +402,4 @@ expect(JWT.secure_compare('Foo', 'Bar')).to eq false end end - end