Support verification of HS256-signed JWTs

README.md

@@ -69,6 +69,7 @@ config.omniauth :openid_connect, {
 | pkce_verifier                | Specify a custom PKCE verifier code.                                                                                                                          | no       | A random 128-char string      | Proc.new { SecureRandom.hex(64) }                   |
 | pkce_options                 | Specify a custom implementation of the PKCE code challenge/method.                                                                                            | no       | SHA256(code_challenge) in hex | Proc to customise the code challenge generation     |
 | client_options               | A hash of client options detailed in its own section                                                                                                          | yes      |                               |                                                     |
+| jwt_secret_base64 | For HMAC with SHA2 (e.g. HS256) signing algorithms, specify the base64-encoded secret used to sign the JWT token. Defaults to the OAuth2 client secret if not specified. | no | client_options.secret | "bXlzZWNyZXQ=\n"
 
 ### Client Config Options
 

lib/omniauth/strategies/openid_connect.rb

@@ -1,5 +1,6 @@
 # frozen_string_literal: true
 
+require 'base64'
 require 'timeout'
 require 'net/http'
 require 'open-uri'
@@ -36,6 +37,7 @@ class OpenIDConnect # rubocop:disable Metrics/ClassLength
       option :issuer
       option :discovery, false
       option :client_signing_alg
+      option :jwt_secret_base64
       option :client_jwk_signing_key
       option :client_x509_signing_key
       option :scope, [:openid]
@@ -200,13 +202,19 @@ def authorize_uri
       def public_key
         @public_key ||= if options.discovery
                           config.jwks
-                        elsif key_or_secret
-                          key_or_secret
+                        elsif configured_public_key
+                          configured_public_key
                         elsif client_options.jwks_uri
                           fetch_key
                         end
       end
 
+      # Some OpenID providers use the OAuth2 client secret as the shared secret, but
+      # Keycloak uses a separate key that's stored inside the database.
+      def secret
+        base64_decoded_jwt_secret || client_options.secret
+      end
+
       def pkce_authorize_params(verifier)
         # NOTE: see https://tools.ietf.org/html/rfc7636#appendix-A
         {
@@ -221,6 +229,12 @@ def fetch_key
         @fetch_key ||= parse_jwk_key(::OpenIDConnect.http_client.get_content(client_options.jwks_uri))
       end
 
+      def base64_decoded_jwt_secret
+        return unless options.jwt_secret_base64
+
+        Base64.decode64(options.jwt_secret_base64)
+      end
+
       def issuer
         resource = "#{ client_options.scheme }://#{ client_options.host }"
         resource = "#{ resource }:#{ client_options.port }" if client_options.port
@@ -265,8 +279,75 @@ def access_token
         @access_token
       end
 
+      # Unlike ::OpenIDConnect::ResponseObject::IdToken.decode, this
+      # method splits the decoding and verification of JWT into two
+      # steps. First, we decode the JWT without verifying it to
+      # determine the algorithm used to sign. Then, we verify it using
+      # the appropriate public key (e.g. if algorithm is RS256) or
+      # shared secret (e.g. if algorithm is HS256).  This works around a
+      # limitation in the openid_connect gem:
+      # https://github.com/nov/openid_connect/issues/61
       def decode_id_token(id_token)
-        ::OpenIDConnect::ResponseObject::IdToken.decode(id_token, public_key)
+        decoded = JSON::JWT.decode(id_token, :skip_verification)
+        algorithm = decoded.algorithm.to_sym
+
+        validate_client_algorithm!(algorithm)
+
+        keyset =
+          case algorithm
+          when :HS256, :HS384, :HS512
+            secret
+          else
+            public_key
+          end
+
+        decoded.verify!(keyset)
+        ::OpenIDConnect::ResponseObject::IdToken.new(decoded)
+      rescue JSON::JWK::Set::KidNotFound
+        # If the JWT has a key ID (kid), then we know that the set of
+        # keys supplied doesn't contain the one we want, and we're
+        # done. However, if there is no kid, then we try each key
+        # individually to see if one works:
+        # https://github.com/nov/json-jwt/pull/92#issuecomment-824654949
+        raise if decoded&.header&.key?('kid')
+
+        decoded = decode_with_each_key!(id_token, keyset)
+
+        raise unless decoded
+
+        decoded
+      end
+
+      # If client_signing_alg is specified, we check that the returned JWT
+      # matches the expected algorithm. If not, we reject it.
+      def validate_client_algorithm!(algorithm)
+        client_signing_alg = options.client_signing_alg&.to_sym
+
+        return unless client_signing_alg
+        return if algorithm == client_signing_alg
+
+        reason = "Received JWT is signed with #{algorithm}, but client_singing_alg is configured for #{client_signing_alg}"
+        raise CallbackError, error: :invalid_jwt_algorithm, reason: reason, uri: params['error_uri']
+      end
+
+      def decode!(id_token, key)
+        ::OpenIDConnect::ResponseObject::IdToken.decode(id_token, key)
+      end
+
+      def decode_with_each_key!(id_token, keyset)
+        return unless keyset.is_a?(JSON::JWK::Set)
+
+        keyset.each do |key|
+          begin
+            decoded = decode!(id_token, key)
+          rescue JSON::JWS::VerificationFailed, JSON::JWS::UnexpectedAlgorithm, JSON::JWS::UnknownAlgorithm
+            next
+          end
+
+          return decoded if decoded
+        end
+
+        nil
       end
 
       def client_options
@@ -308,18 +389,12 @@ def session
         super
       end
 
-      def key_or_secret
-        @key_or_secret ||=
-          case options.client_signing_alg&.to_sym
-          when :HS256, :HS384, :HS512
-            client_options.secret
-          when :RS256, :RS384, :RS512
-            if options.client_jwk_signing_key
-              parse_jwk_key(options.client_jwk_signing_key)
-            elsif options.client_x509_signing_key
-              parse_x509_key(options.client_x509_signing_key)
-            end
-          end
+      def configured_public_key
+        @configured_public_key ||= if options.client_jwk_signing_key
+                                     parse_jwk_key(options.client_jwk_signing_key)
+                                   elsif options.client_x509_signing_key
+                                     parse_x509_key(options.client_x509_signing_key)
+                                   end
       end
 
       def parse_x509_key(key)

test/lib/omniauth/strategies/openid_connect_test.rb

@@ -269,6 +269,138 @@ def test_callback_phase_with_id_token_and_param_provided_nonce # rubocop:disable
         strategy.callback_phase
       end
 
+      def test_callback_phase_with_id_token_no_kid
+        other_rsa_private = OpenSSL::PKey::RSA.generate(2048)
+
+        key = JSON::JWK.new(private_key)
+        other_key = JSON::JWK.new(other_rsa_private)
+        state = SecureRandom.hex(16)
+        request.stubs(:params).returns('id_token' => jwt.to_s, 'state' => state)
+        request.stubs(:path_info).returns('')
+
+        strategy.options.issuer = issuer
+        strategy.options.client_signing_alg = :RS256
+        strategy.options.client_jwk_signing_key = { 'keys' => [other_key, key] }.to_json
+        strategy.options.response_type = 'id_token'
+
+        strategy.unstub(:user_info)
+        strategy.call!('rack.session' => { 'omniauth.state' => state, 'omniauth.nonce' => nonce })
+        strategy.callback_phase
+      end
+
+      def test_callback_phase_with_id_token_with_kid
+        other_rsa_private = OpenSSL::PKey::RSA.generate(2048)
+
+        key = JSON::JWK.new(private_key)
+        other_key = JSON::JWK.new(other_rsa_private)
+        state = SecureRandom.hex(16)
+        jwt_with_kid = JSON::JWT.new(payload).sign(key, :RS256)
+        request.stubs(:params).returns('id_token' => jwt_with_kid.to_s, 'state' => state)
+        request.stubs(:path_info).returns('')
+
+        strategy.options.issuer = issuer
+        strategy.options.client_signing_alg = :RS256
+        strategy.options.client_jwk_signing_key = { 'keys' => [other_key, key] }.to_json
+        strategy.options.response_type = 'id_token'
+
+        strategy.unstub(:user_info)
+        strategy.call!('rack.session' => { 'omniauth.state' => state, 'omniauth.nonce' => nonce })
+        strategy.callback_phase
+      end
+
+      def test_callback_phase_with_id_token_with_kid_and_no_matching_kid
+        other_rsa_private = OpenSSL::PKey::RSA.generate(2048)
+
+        key = JSON::JWK.new(private_key)
+        other_key = JSON::JWK.new(other_rsa_private)
+        state = SecureRandom.hex(16)
+        jwt_with_kid = JSON::JWT.new(payload).sign(key, :RS256)
+        request.stubs(:params).returns('id_token' => jwt_with_kid.to_s, 'state' => state)
+        request.stubs(:path_info).returns('')
+
+        strategy.options.issuer = issuer
+        strategy.options.client_signing_alg = :RS256
+        # We use private_key here instead of the wrapped key, which contains a kid
+        strategy.options.client_jwk_signing_key = { 'keys' => [other_key, private_key] }.to_json
+        strategy.options.response_type = 'id_token'
+
+        strategy.unstub(:user_info)
+        strategy.call!('rack.session' => { 'omniauth.state' => state, 'omniauth.nonce' => nonce })
+
+        assert_raises JSON::JWK::Set::KidNotFound do
+          strategy.callback_phase
+        end
+      end
+
+      def test_callback_phase_with_id_token_with_hs256
+        state = SecureRandom.hex(16)
+        request.stubs(:params).returns('id_token' => jwt_with_hs256.to_s, 'state' => state)
+        request.stubs(:path_info).returns('')
+
+        strategy.options.issuer = issuer
+        strategy.options.client_options.secret = hmac_secret
+        strategy.options.client_signing_alg = :HS256
+        strategy.options.response_type = 'id_token'
+
+        strategy.unstub(:user_info)
+        strategy.call!('rack.session' => { 'omniauth.state' => state, 'omniauth.nonce' => nonce })
+        strategy.callback_phase
+      end
+
+      def test_callback_phase_with_hs256_base64_jwt_secret
+        state = SecureRandom.hex(16)
+        request.stubs(:params).returns('id_token' => jwt_with_hs256.to_s, 'state' => state)
+        request.stubs(:path_info).returns('')
+
+        strategy.options.issuer = issuer
+        strategy.options.jwt_secret_base64 = Base64.encode64(hmac_secret)
+        strategy.options.response_type = 'id_token'
+
+        strategy.unstub(:user_info)
+        strategy.call!('rack.session' => { 'omniauth.state' => state, 'omniauth.nonce' => nonce })
+        strategy.callback_phase
+      end
+
+      def test_callback_phase_with_mismatched_signing_algorithm
+        state = SecureRandom.hex(16)
+        request.stubs(:params).returns('id_token' => jwt_with_hs512.to_s, 'state' => state)
+        request.stubs(:path_info).returns('')
+
+        strategy.options.issuer = issuer
+        strategy.options.client_options.secret = hmac_secret
+        strategy.options.client_signing_alg = :HS256
+        strategy.options.response_type = 'id_token'
+
+        strategy.unstub(:user_info)
+        strategy.call!('rack.session' => { 'omniauth.state' => state, 'omniauth.nonce' => nonce })
+
+        strategy.expects(:fail!).with(:invalid_jwt_algorithm, is_a(OmniAuth::Strategies::OpenIDConnect::CallbackError))
+        strategy.callback_phase
+      end
+
+      def test_callback_phase_with_id_token_no_matching_key
+        rsa_private = OpenSSL::PKey::RSA.generate(2048)
+        other_rsa_private = OpenSSL::PKey::RSA.generate(2048)
+
+        other_key = JSON::JWK.new(other_rsa_private)
+        token = JSON::JWT.new(payload).sign(rsa_private, :RS256).to_s
+        state = SecureRandom.hex(16)
+        request.stubs(:params).returns('id_token' => token, 'state' => state)
+        request.stubs(:path_info).returns('')
+
+        strategy.options.issuer = issuer
+        strategy.options.client_signing_alg = :RS256
+        strategy.options.client_jwk_signing_key = { 'keys' => [other_key] }.to_json
+        strategy.options.response_type = 'id_token'
+
+        strategy.unstub(:user_info)
+        strategy.call!('rack.session' => { 'omniauth.state' => state, 'omniauth.nonce' => nonce })
+
+        assert_raises JSON::JWK::Set::KidNotFound do
+          strategy.callback_phase
+        end
+      end
+
       def test_callback_phase_with_discovery # rubocop:disable Metrics/AbcSize
         state = SecureRandom.hex(16)
 
@@ -699,7 +831,7 @@ def test_public_key_with_x509
       def test_public_key_with_hmac
         strategy.options.client_options.secret = 'secret'
         strategy.options.client_signing_alg = :HS256
-        assert_equal strategy.options.client_options.secret, strategy.public_key
+        assert_equal strategy.options.client_options.secret, strategy.secret
       end
 
       def test_id_token_auth_hash

test/strategy_test_case.rb

@@ -37,6 +37,18 @@ def jwt
     @jwt ||= JSON::JWT.new(payload).sign(private_key, :RS256)
   end
 
+  def hmac_secret
+    @hmac_secret ||= SecureRandom.hex(16)
+  end
+
+  def jwt_with_hs256
+    @jwt_with_hs256 ||= JSON::JWT.new(payload).sign(hmac_secret, :HS256)
+  end
+
+  def jwt_with_hs512
+    @jwt_with_hs512 ||= JSON::JWT.new(payload).sign(hmac_secret, :HS512)
+  end
+
   def jwks
     @jwks ||= begin
       key = JSON::JWK.new(private_key)