From 2eed3c8a2613c39906d461a6c65c73d5f9f9c146 Mon Sep 17 00:00:00 2001 From: Joakim Antman Date: Sun, 6 Oct 2024 23:07:41 +0300 Subject: [PATCH] Do not decode payload when b64 header is false --- lib/jwt/claims.rb | 6 ++-- lib/jwt/claims/crit.rb | 35 +++++++++++++++++++++++ lib/jwt/claims/verifier.rb | 2 +- lib/jwt/encoded_token.rb | 30 ++++++++++++++++++-- lib/jwt/error.rb | 3 ++ spec/jwt/claims/crit_spec.rb | 45 ++++++++++++++++++++++++++++++ spec/jwt/encoded_token_spec.rb | 51 +++++++++++++++++++++++++++++++++- spec/spec_support/token.rb | 2 +- 8 files changed, 165 insertions(+), 9 deletions(-) create mode 100644 lib/jwt/claims/crit.rb create mode 100644 spec/jwt/claims/crit_spec.rb diff --git a/lib/jwt/claims.rb b/lib/jwt/claims.rb index dde6838a..581facf3 100644 --- a/lib/jwt/claims.rb +++ b/lib/jwt/claims.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true require_relative 'claims/audience' +require_relative 'claims/crit' +require_relative 'claims/decode_verifier' require_relative 'claims/expiration' require_relative 'claims/issued_at' require_relative 'claims/issuer' @@ -9,9 +11,8 @@ require_relative 'claims/numeric' require_relative 'claims/required' require_relative 'claims/subject' -require_relative 'claims/decode_verifier' -require_relative 'claims/verifier' require_relative 'claims/verification_methods' +require_relative 'claims/verifier' module JWT # JWT Claim verifications @@ -27,7 +28,6 @@ module JWT # sub # required # numeric - # module Claims # Represents a claim verification error Error = Struct.new(:message, keyword_init: true) diff --git a/lib/jwt/claims/crit.rb b/lib/jwt/claims/crit.rb new file mode 100644 index 00000000..ac339eb0 --- /dev/null +++ b/lib/jwt/claims/crit.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +module JWT + module Claims + # Responsible of validation the crit header + class Crit + # Initializes a new Crit instance. + # + # @param expected_crits [String] the expected crit header values for the JWT token. + def initialize(expected_crits:) + @expected_crits = Array(expected_crits) + end + + # Verifies the critical claim ('crit') in the JWT token header. + # + # @param context [Object] the context containing the JWT payload and header. + # @param _args [Hash] additional arguments (not used). + # @raise [JWT::InvalidCritError] if the crit claim is invalid. + # @return [nil] + def verify!(context:, **_args) + raise(JWT::InvalidCritError, 'Crit header missing') unless context.header['crit'] + raise(JWT::InvalidCritError, 'Crit header should be an array') unless context.header['crit'].is_a?(Array) + + missing = (expected_crits - context.header['crit']) + raise(JWT::InvalidCritError, "Crit header missing expected values: #{missing.join(', ')}") if missing.any? + + nil + end + + private + + attr_reader :expected_crits + end + end +end diff --git a/lib/jwt/claims/verifier.rb b/lib/jwt/claims/verifier.rb index c75a4eb9..81ce8a23 100644 --- a/lib/jwt/claims/verifier.rb +++ b/lib/jwt/claims/verifier.rb @@ -12,7 +12,7 @@ module Verifier jti: ->(options) { Claims::JwtId.new(validator: options[:jti]) }, aud: ->(options) { Claims::Audience.new(expected_audience: options[:aud]) }, sub: ->(options) { Claims::Subject.new(expected_subject: options[:sub]) }, - + crit: ->(options) { Claims::Crit.new(expected_crits: options[:crit]) }, required: ->(options) { Claims::Required.new(required_claims: options[:required]) }, numeric: ->(*) { Claims::Numeric.new } }.freeze diff --git a/lib/jwt/encoded_token.rb b/lib/jwt/encoded_token.rb index cdaacddd..0264c51e 100644 --- a/lib/jwt/encoded_token.rb +++ b/lib/jwt/encoded_token.rb @@ -23,7 +23,7 @@ class EncodedToken # @param jwt [String] the encoded JWT token. # @raise [ArgumentError] if the provided JWT is not a String. def initialize(jwt) - raise ArgumentError 'Provided JWT must be a String' unless jwt.is_a?(String) + raise ArgumentError, 'Provided JWT must be a String' unless jwt.is_a?(String) @jwt = jwt @encoded_header, @encoded_payload, @encoded_signature = jwt.split('.') @@ -57,7 +57,7 @@ def header # # @return [Hash] the payload. def payload - @payload ||= encoded_payload == '' ? raise(JWT::DecodeError, 'Encoded payload is empty') : parse_and_decode(encoded_payload) + @payload ||= decode_payload end # Sets or returns the encoded payload of the JWT token. @@ -85,6 +85,7 @@ def verify_signature!(algorithm:, key: nil, key_finder: nil) raise ArgumentError, 'Provide either key or key_finder, not both or neither' if key.nil? == key_finder.nil? key ||= key_finder.call(self) + return if valid_signature?(algorithm: algorithm, key: key) raise JWT::VerificationError, 'Signature verification failed' @@ -107,8 +108,31 @@ def valid_signature?(algorithm:, key:) private + def decode_payload + raise JWT::DecodeError, 'Encoded payload is empty' if encoded_payload == '' + + if unecoded_payload? + verify_claims!(crit: ['b64']) + return parse_unencoded(encoded_payload) + end + + parse_and_decode(encoded_payload) + end + + def unecoded_payload? + header['b64'] == false + end + def parse_and_decode(segment) - JWT::JSON.parse(::JWT::Base64.url_decode(segment)) + parse(::JWT::Base64.url_decode(segment)) + end + + def parse_unencoded(segment) + parse(segment) + end + + def parse(segment) + JWT::JSON.parse(segment) rescue ::JSON::ParserError raise JWT::DecodeError, 'Invalid segment encoding' end diff --git a/lib/jwt/error.rb b/lib/jwt/error.rb index d6ea9e91..f17ac537 100644 --- a/lib/jwt/error.rb +++ b/lib/jwt/error.rb @@ -37,6 +37,9 @@ class InvalidAudError < DecodeError; end # The InvalidSubError class is raised when the JWT subject (sub) claim is invalid. class InvalidSubError < DecodeError; end + # The InvalidCritError class is raised when the JWT crit header is invalid. + class InvalidCritError < DecodeError; end + # The InvalidJtiError class is raised when the JWT ID (jti) claim is invalid. class InvalidJtiError < DecodeError; end diff --git a/spec/jwt/claims/crit_spec.rb b/spec/jwt/claims/crit_spec.rb new file mode 100644 index 00000000..6b08c455 --- /dev/null +++ b/spec/jwt/claims/crit_spec.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +RSpec.describe JWT::Claims::Crit do + subject(:verify!) { described_class.new(expected_crits: expected_crits).verify!(context: SpecSupport::Token.new(header: header)) } + let(:expected_crits) { [] } + let(:header) { {} } + + context 'when header is missing' do + it 'raises JWT::InvalidCritError' do + expect { verify! }.to raise_error(JWT::InvalidCritError, 'Crit header missing') + end + end + + context 'when header is not an array' do + let(:header) { { 'crit' => 'not_an_array' } } + + it 'raises JWT::InvalidCritError' do + expect { verify! }.to raise_error(JWT::InvalidCritError, 'Crit header should be an array') + end + end + + context 'when header is an array and not containing the expected value' do + let(:header) { { 'crit' => %w[crit1] } } + let(:expected_crits) { %w[crit2] } + it 'raises an InvalidCritError' do + expect { verify! }.to raise_error(JWT::InvalidCritError, 'Crit header missing expected values: crit2') + end + end + + context 'when header is an array containing exactly the expected values' do + let(:header) { { 'crit' => %w[crit1 crit2] } } + let(:expected_crits) { %w[crit1 crit2] } + it 'does not raise an error' do + expect(verify!).to eq(nil) + end + end + + context 'when header is an array containing at least the expected values' do + let(:header) { { 'crit' => %w[crit1 crit2 crit3] } } + let(:expected_crits) { %w[crit1 crit2] } + it 'does not raise an error' do + expect(verify!).to eq(nil) + end + end +end diff --git a/spec/jwt/encoded_token_spec.rb b/spec/jwt/encoded_token_spec.rb index 77688d10..4cc46fdb 100644 --- a/spec/jwt/encoded_token_spec.rb +++ b/spec/jwt/encoded_token_spec.rb @@ -2,13 +2,15 @@ RSpec.describe JWT::EncodedToken do let(:payload) { { 'pay' => 'load' } } - let(:encoded_token) { JWT.encode(payload, 'secret', 'HS256') } + let(:header) { {} } + let(:encoded_token) { JWT::Token.new(payload: payload, header: header).tap { |t| t.sign!(algorithm: 'HS256', key: 'secret') }.jwt } let(:detached_payload_token) do JWT::Token.new(payload: payload).tap do |t| t.detach_payload! t.sign!(algorithm: 'HS256', key: 'secret') end end + subject(:token) { described_class.new(encoded_token) } describe '#payload' do @@ -28,6 +30,26 @@ end end end + + context 'when payload is not encoded and the b64 crit is enabled' do + subject(:token) { described_class.new(encoded_token) } + let(:encoded_token) { 'eyJhbGciOiJIUzI1NiIsImI2NCI6ZmFsc2UsImNyaXQiOlsiYjY0Il19..signature' } + before { token.encoded_payload = '{"foo": "bar"}' } + + it 'does not raise' do + expect(token.payload).to eq({ 'foo' => 'bar' }) + end + end + + context 'when payload is not encoded and the b64 crit is NOT enabled' do + let(:encoded_token) { 'eyJhbGciOiJIUzI1NiIsImI2NCI6ZmFsc2UsImNyaXQiOlsiYjY0Il19..signature' } + before { token.encoded_payload = '{"foo": "bar"}' } + + it 'raises an error' do + pending 'Need to decide how to handle this case' + expect { token.payload }.to raise_error + end + end end describe '#header' do @@ -99,6 +121,17 @@ expect { token.verify_signature!(algorithm: 'HS256', key: 'key', key_finder: 'finder') }.to raise_error(ArgumentError, 'Provide either key or key_finder, not both or neither') end end + + context 'when payload is not encoded' do + let(:encoded_token) { 'eyJhbGciOiJIUzI1NiIsImI2NCI6ZmFsc2UsImNyaXQiOlsiYjY0Il19..A5dxf2s96_n5FLueVuW1Z_vh161FwXZC4YLPff6dmDY' } + before { token.encoded_payload = '$.02' } + + let(:key) { Base64.urlsafe_decode64('AyM1SysPpbyDfgZld3umj1qzKObwVMkoqQ-EstJQLr_T-1qS0gZH75aKtMN3Yj0iPS4hcgUuTwjAzZr1Z9CAow') } + + it 'does not raise' do + expect(token.verify_signature!(algorithm: 'HS256', key: key)).to eq(nil) + end + end end describe '#verify_claims!' do @@ -150,6 +183,22 @@ end end end + + context 'when header contains crits header' do + let(:header) { { crit: ['b64'] } } + + context 'when expected crits are missing' do + it 'raises an error' do + expect { token.verify_claims!(crit: ['other']) }.to raise_error(JWT::InvalidCritError, 'Crit header missing expected values: other') + end + end + + context 'when expected crits are present' do + it 'passes verification' do + expect { token.verify_claims!(crit: ['b64']) }.not_to raise_error + end + end + end end describe '#valid_claims?' do diff --git a/spec/spec_support/token.rb b/spec/spec_support/token.rb index 8dd7de4b..e076aae1 100644 --- a/spec/spec_support/token.rb +++ b/spec/spec_support/token.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module SpecSupport - Token = Struct.new(:payload, keyword_init: true) + Token = Struct.new(:payload, :header, keyword_init: true) end