Merge pull request #1392 from 3scale/backport-2.13-THREESCALE-9009-fix-oidc-issuer-verification

Backport 2.13 threescale 9009 fix OIDC issuer verification

Author: eguzki

.circleci/config.yml

@@ -17,9 +17,9 @@ commands:
     steps:
       - restore_cache:
           keys:
-          - apicast-rocks-{{ arch }}-{{ checksum "gateway/Roverfile.lock" }}
-          - apicast-rocks-{{ arch }}-{{ .Branch }}
-          - apicast-rocks-{{ arch }}-master
+          - apicast-rocks-v2.13-{{ arch }}-{{ checksum "gateway/Roverfile.lock" }}
+          - apicast-rocks-v2.13-{{ arch }}-{{ .Branch }}
+          - apicast-rocks-v2.13-{{ arch }}-master
 
   restore-perl-cache:
     steps:
@@ -106,12 +106,13 @@ executors:
     docker:
     - image: docker:stable
     environment:
+      COMPOSE_TLS_VERSION: "TLSv1_2"
       DOCKER_COMPOSE_VERSION: "1.16.1"
 
   openresty:
     working_directory: /opt/app-root/apicast
     docker:
-      - image: quay.io/3scale/apicast-ci:openresty-1.19.3
+      - image: quay.io/3scale/apicast-ci:openresty-1.19.3-pr1381
       - image: redis:3.2.8-alpine
     environment:
       TEST_NGINX_BINARY: openresty
@@ -170,7 +171,7 @@ jobs:
       - restore-lua-cache
       - run: make lua_modules
       - save_cache:
-          key: apicast-rocks-{{ arch }}-{{ checksum "gateway/Roverfile.lock" }}
+          key: apicast-rocks-v2.13-{{ arch }}-{{ checksum "gateway/Roverfile.lock" }}
           <<: *lua-cache-paths
       - persist_to_workspace:
           root: .

CHANGELOG.md

@@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ## [Unreleased]
 
+## [3.13.2] 2023-02-21
+
+### Fixed
+
+- Fixed: OIDC jwt key verification [PR #1392](https://github.com/3scale/APIcast/pull/1392) [THREESCALE-9009](https://issues.redhat.com/browse/THREESCALE-9009)
+
+## [3.13.0] 2023-02-07
+
 ### Fixed
 
 - Fixed NGINX filters policy error [PR #1339](https://github.com/3scale/APIcast/pull/1339) [THREESCALE-7349](https://issues.redhat.com/browse/THREESCALE-7349)
@@ -18,6 +26,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ### Added
 
+## [3.12.2] 2023-02-21
+
+ - Fixed: OIDC jwt key verification [PR #1391](https://github.com/3scale/APIcast/pull/1391) [THREESCALE-9009](https://issues.redhat.com/browse/THREESCALE-9009)
+
 ## [3.12.0] 2022-07-07
 
 ### Fixed
@@ -965,3 +977,5 @@ Apart from the changes mentioned in this section, this version also includes the
 [3.10.0]: https://github.com/3scale/apicast/compare/v3.10.0-beta1..v3.10.0
 [3.11.0]: https://github.com/3scale/apicast/compare/v3.10.0..v3.11.0
 [3.12.0]: https://github.com/3scale/apicast/compare/v3.11.0..v3.12.0
+[3.12.2]: https://github.com/3scale/apicast/compare/v3.12.0..v3.12.2
+[3.13.0]: https://github.com/3scale/apicast/compare/v3.12.0..v3.13.0

gateway/src/apicast/oauth/oidc.lua

@@ -105,7 +105,7 @@ end
 
 local function find_jwk(jwt, keys)
   local jwk = keys and keys[jwt.header.kid]
-  if jwk then return jwk end
+  return jwk
 end
 
 -- Parses the token - in this case we assume it's a JWT token
@@ -185,8 +185,13 @@ function _M:verify(jwt, cache_key)
   -- Find jwk with matching kid for current JWT in request
   local jwk_obj = find_jwk(jwt, self.keys)
 
+  if jwk_obj == nil then
+    ngx.log(ngx.ERR, "[jwt] failed verification for kid: ", jwt.header.kid)
+    return false, '[jwk] not found, token might belong to a different realm'
+  end
+
   local pubkey = jwk_obj.pem
-  -- Check the jwk for the alg field and if not present skip the validation as it is 
+  -- Check the jwk for the alg field and if not present skip the validation as it is
   -- OPTIONAL according to https://www.rfc-editor.org/rfc/rfc7517#section-4.4
   if jwk_obj.alg and jwk_obj.alg ~= jwt.header.alg then
     return false, '[jwt] alg mismatch'

gateway/src/apicast/version.lua

@@ -1 +1 @@
-return "latest"
+return "3.13.2"

spec/oauth/oidc_spec.lua

@@ -292,6 +292,42 @@ describe('OIDC', function()
       assert(credentials, err)
     end)
 
+    it('token was signed by a different key', function()
+      local oidc = _M.new(oidc_config)
+      local access_token = jwt:sign(rsa.private, {
+        header = { typ = 'JWT', alg = 'RS256', kid = 'otherkid' },
+        payload = {
+          iss = oidc_config.issuer,
+          aud = 'notused',
+          azp = 'ce3b2e5e',
+          sub = 'someone',
+          exp = ngx.now() + 10,
+        },
+      })
+
+      local credentials, _, _, err = oidc:transform_credentials({ access_token = access_token })
+
+      assert.match('[jwk] not found, token might belong to a different realm', err, nil, true)
+    end)
+
+    it('token was signed by a different issuer', function()
+      local oidc = _M.new(oidc_config)
+      local access_token = jwt:sign(rsa.private, {
+        header = { typ = 'JWT', alg = 'RS256', kid = 'somekid' },
+        payload = {
+          iss = 'other_issuer',
+          aud = 'notused',
+          azp = 'ce3b2e5e',
+          sub = 'someone',
+          exp = ngx.now() + 10,
+        },
+      })
+
+      local credentials, _, _, err = oidc:transform_credentials({ access_token = access_token })
+
+      assert.match('Claim \'iss\' (\'other_issuer\') returned failure', err, nil, true)
+    end)
+
     describe('getting client_id from any JWT claim', function()
 
       before_each(function()

t/apicast-oidc.t

@@ -313,7 +313,7 @@ my $jwt = encode_jwt(payload => {
 "Authorization: Bearer $jwt"
 --- no_error_log
 
-=== TEST 7: JWT verification fails when jwk.alg exists AND does not match jwt.header.alg 
+=== TEST 7: JWT verification fails when jwk.alg exists AND does not match jwt.header.alg
 (see THREESCALE-8249 for steps to generate tampered JWT. rsa.pub from fixtures used to sign)
 --- configuration env eval
 use JSON qw(to_json);
@@ -362,3 +362,81 @@ my $jwt = 'eyJraWQiOiJzb21la2lkIiwiYWxnIjoiSFMyNTYifQ.'.
 "Authorization: Bearer $jwt"
 --- error_log
 [jwt] alg mismatch
+
+=== TEST 8: Token was signed by a different key
+--- configuration env eval
+use JSON qw(to_json);
+
+to_json({
+  services => [{
+    id => 42,
+    backend_version => 'oauth',
+    backend_authentication_type => 'provider_key',
+    backend_authentication_value => 'fookey',
+    proxy => {
+        authentication_method => 'oidc',
+        oidc_issuer_endpoint => 'https://example.com/auth/realms/a',
+        api_backend => "http://test:$TEST_NGINX_SERVER_PORT/",
+        proxy_rules => [
+          { pattern => '/', http_method => 'GET', metric_system_name => 'hits', delta => 1  }
+        ]
+    }
+  }],
+  oidc => [{
+    issuer => 'https://example.com/auth/realms/a',
+    config => { id_token_signing_alg_values_supported => [ 'RS256' ] },
+    keys => { somekid => { pem => $::public_key, alg => 'RS256' } },
+  }]
+});
+--- request: GET /test
+--- error_code: 403
+--- more_headers eval
+use Crypt::JWT qw(encode_jwt);
+my $jwt = encode_jwt(payload => {
+  aud => 'something',
+  azp => 'appid',
+  sub => 'someone',
+  iss => 'https://example.com/auth/realms/b',
+  exp => time + 3600 }, key => \$::private_key, alg => 'RS256', extra_headers => { kid => 'otherkid' });
+"Authorization: Bearer $jwt"
+--- error_log
+[jwk] not found, token might belong to a different realm
+
+=== TEST 9: Token was signed by a different issuer
+--- configuration env eval
+use JSON qw(to_json);
+
+to_json({
+  services => [{
+    id => 42,
+    backend_version => 'oauth',
+    backend_authentication_type => 'provider_key',
+    backend_authentication_value => 'fookey',
+    proxy => {
+        authentication_method => 'oidc',
+        oidc_issuer_endpoint => 'https://example.com/auth/realms/apicast',
+        api_backend => "http://test:$TEST_NGINX_SERVER_PORT/",
+        proxy_rules => [
+          { pattern => '/', http_method => 'GET', metric_system_name => 'hits', delta => 1  }
+        ]
+    }
+  }],
+  oidc => [{
+    issuer => 'https://example.com/auth/realms/apicast',
+    config => { id_token_signing_alg_values_supported => [ 'RS256' ] },
+    keys => { somekid => { pem => $::public_key, alg => 'RS256' } },
+  }]
+});
+--- request: GET /test
+--- error_code: 403
+--- more_headers eval
+use Crypt::JWT qw(encode_jwt);
+my $jwt = encode_jwt(payload => {
+  aud => 'something',
+  azp => 'appid',
+  sub => 'someone',
+  iss => 'unexpected_issuer',
+  exp => time + 3600 }, key => \$::private_key, alg => 'RS256', extra_headers => { kid => 'somekid' });
+"Authorization: Bearer $jwt"
+--- error_log eval
+[ qr/Claim 'iss' \('unexpected_issuer'\) returned failure/ ]