From b0447272e746c2de709f42f42a04c70709cec956 Mon Sep 17 00:00:00 2001 From: Eguzki Astiz Lezaun Date: Thu, 16 Feb 2023 12:53:32 +0100 Subject: [PATCH 1/7] oidc: sanity check on the jwk which prevents tokens from foreign realms to be verified --- gateway/src/apicast/oauth/oidc.lua | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/gateway/src/apicast/oauth/oidc.lua b/gateway/src/apicast/oauth/oidc.lua index 60591b2d7..e8d6ab98e 100644 --- a/gateway/src/apicast/oauth/oidc.lua +++ b/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,14 @@ 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 + return false, '[jwk] jwk not found, token might not belong to right realm' + end + local pubkey = jwk_obj.pem - if jwk_obj.alg ~= jwt.header.alg then + -- 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' end From 1d251d112590e8d7fcbed260b255ad5e1b5d3e4c Mon Sep 17 00:00:00 2001 From: Eguzki Astiz Lezaun Date: Thu, 16 Feb 2023 17:28:16 +0100 Subject: [PATCH 2/7] oidc: test when key not found --- t/apicast-oidc.t | 106 +++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 102 insertions(+), 4 deletions(-) diff --git a/t/apicast-oidc.t b/t/apicast-oidc.t index ada1469eb..2c3b2e0fb 100644 --- a/t/apicast-oidc.t +++ b/t/apicast-oidc.t @@ -264,9 +264,7 @@ my $jwt = encode_jwt(payload => { --- no_error_log [error] - - -=== TEST 2: JWT verification fails when no alg is present in the jwk to match against jwt.header.alg +=== TEST 6: JWT verification does not fail when no alg is present in the jwk to match against jwt.header.alg --- configuration env eval use JSON qw(to_json); @@ -303,7 +301,7 @@ to_json({ } } --- request: GET /test ---- error_code: 403 +--- error_code: 200 --- more_headers eval use Crypt::JWT qw(encode_jwt); my $jwt = encode_jwt(payload => { @@ -313,5 +311,105 @@ my $jwt = encode_jwt(payload => { iss => 'https://example.com/auth/realms/apicast', exp => time + 3600 }, key => \$::private_key, alg => 'RS256', extra_headers => { kid => 'somekid' }); "Authorization: Bearer $jwt" +--- no_error_log + +=== 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); + +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', 'HS256' ] }, + keys => { somekid => { pem => $::public_key, alg => 'RS256' } }, + }] +}); +--- upstream + location /test { + echo "yes"; + } +--- backend + location = /transactions/oauth_authrep.xml { + content_by_lua_block { + local expected = "provider_key=fookey&service_id=42&usage%5Bhits%5D=1&app_id=appid" + require('luassert').same(ngx.decode_args(expected), ngx.req.get_uri_args(0)) + } + } +--- request: GET /test +--- error_code: 403 +--- more_headers eval +use Crypt::JWT qw(encode_jwt); +my $jwt = 'eyJraWQiOiJzb21la2lkIiwiYWxnIjoiSFMyNTYifQ.'. +'eyJleHAiOjcxNzA1MzE2NDMwLCJhenAiOiJhcHBpZCIsInN1YiI6In'. +'NvbWVvbmUiLCJhdWQiOiJzb21ldGhpbmciLCJpc3MiOiJodHRwczov'. +'L2V4YW1wbGUuY29tL2F1dGgvcmVhbG1zL2FwaWNhc3QifQ.1rFq5QN'. +'b99W6aqQjsx7GJGLDpdkDLI6-huZLzMAmxGQ'; +"Authorization: Bearer $jwt" --- error_log [jwt] alg mismatch + +=== TEST 8: Key not available +(Happens when the token belongs to a different realm) +--- 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' } }, + }] +}); +--- upstream + location /test { + echo "yes"; + } +--- backend + location = /transactions/oauth_authrep.xml { + content_by_lua_block { + local expected = "provider_key=fookey&service_id=42&usage%5Bhits%5D=1&app_id=appid" + require('luassert').same(ngx.decode_args(expected), ngx.req.get_uri_args(0)) + } + } +--- 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] jwk not found From b3a723205daea36ce8b121165b6109b4f2120c65 Mon Sep 17 00:00:00 2001 From: Eguzki Astiz Lezaun Date: Thu, 16 Feb 2023 18:19:40 +0100 Subject: [PATCH 3/7] oidc: enhance test when key not found --- t/apicast-oidc.t | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/t/apicast-oidc.t b/t/apicast-oidc.t index 2c3b2e0fb..3370a9735 100644 --- a/t/apicast-oidc.t +++ b/t/apicast-oidc.t @@ -389,17 +389,6 @@ to_json({ keys => { somekid => { pem => $::public_key, alg => 'RS256' } }, }] }); ---- upstream - location /test { - echo "yes"; - } ---- backend - location = /transactions/oauth_authrep.xml { - content_by_lua_block { - local expected = "provider_key=fookey&service_id=42&usage%5Bhits%5D=1&app_id=appid" - require('luassert').same(ngx.decode_args(expected), ngx.req.get_uri_args(0)) - } - } --- request: GET /test --- error_code: 403 --- more_headers eval From f05d17cf99c24651ff4339f07deae3010dfb0dce Mon Sep 17 00:00:00 2001 From: Eguzki Astiz Lezaun Date: Fri, 17 Feb 2023 10:59:50 +0100 Subject: [PATCH 4/7] changelog updated --- CHANGELOG.md | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ecb8cdeb8..3bf6c7cf4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,39 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Fixed +- Fixed: OIDC jwt key verification [PR #1389](https://github.com/3scale/APIcast/pull/1389) [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) +- Fix to avoid uninitialized variables when request URI is too large [PR #1340](https://github.com/3scale/APIcast/pull/1340) [THREESCALE-7906](https://issues.redhat.com/browse/THREESCALE-7906) +- Fixed issue where request path is stripped for proxied https requests [PR #1342](https://github.com/3scale/APIcast/pull/1342) [THREESCALE-8426](https://issues.redhat.com/browse/THREESCALE-8426) +- Bumped liquid-lua to version 0.2.0-2 [PR #1369](https://github.com/3scale/APIcast/pull/1369) - includes: [THREESCALE-8483](https://issues.redhat.com/browse/THREESCALE-8483) and [THREESCALE-8484](https://issues.redhat.com/browse/THREESCALE-8484) +- Fixed: APIcast could not retrieve the latest version of the proxy config [PR #1370](https://github.com/3scale/APIcast/pull/1370) [THREESCALE-8485](https://issues.redhat.com/browse/THREESCALE-8485) +- Fixed: JWKs without alg field cause the JWT validation process to fail [PR #1371](https://github.com/3scale/APIcast/pull/1371) [THREESCALE-8601](https://issues.redhat.com/browse/THREESCALE-8601) + +### Added + +- Updated policy list [PR #1374](https://github.com/3scale/APIcast/pull/1374) + +## [3.12.0] 2022-07-07 + +### Fixed + +- Fixed warning messages [PR #1318](https://github.com/3scale/APIcast/pull/1318) [THREESCALE-7906](https://issues.redhat.com/browse/THREESCALE-7906) +- Fixed dirty context [PR #1328](https://github.com/3scale/APIcast/pull/1328) [THREESCALE-8000](https://issues.redhat.com/browse/THREESCALE-8000) [THREESCALE-8007](https://issues.redhat.com/browse/THREESCALE-8007) +- Fixed jwk alg confusion [PR #1329](https://github.com/3scale/APIcast/pull/1329) [THREESCALE-8249](https://issues.redhat.com/browse/THREESCALE-8249) +- Fixed issue with resolving target server hostnames to IP when using CONNECT method [PR #1323](https://github.com/3scale/APIcast/pull/1323) [THREESCALE-7967](https://issues.redhat.com/browse/THREESCALE-7967) +- Fixed issue with resolving target server hostnames to IPs when forwarding requests through http/s proxy [PR #1323](https://github.com/3scale/APIcast/pull/1323) [THREESCALE-7967](https://issues.redhat.com/browse/THREESCALE-7967) +- Fixed dirty context [PR #1328](https://github.com/3scale/APIcast/pull/1328) [THREESCALE-8000](https://issues.redhat.com/browse/THREESCALE-8000) [THREESCALE-8007](https://issues.redhat.com/browse/THREESCALE-8007) [THREESCALE-8252](https://issues.redhat.com/browse/THREESCALE-8252) +- Fixed dirty context (part 2 of PR #1328) when tls termination policy is in the policy chain [PR #1333](https://github.com/3scale/APIcast/pull/1333) + +## [3.11.0] 2022-02-17 + +### Fixed + - Fixed hostname_rewrite incompatibility with Routing Policy [PR #1263](https://github.com/3scale/APIcast/pull/1263) [THREESCALE-6723](https://issues.redhat.com/browse/THREESCALE-6723) - Fixed issues with URI when using Routing Policy [PR #1245](https://github.com/3scale/APIcast/pull/1245) [THREESCALE-6410](https://issues.redhat.com/browse/THREESCALE-6410) - Fixed typo on TLS jsonschema [PR #1260](https://github.com/3scale/APIcast/pull/1260) [THREESCALE-6390](https://issues.redhat.com/browse/THREESCALE-6390) From f071581cefc871cc8522085d75d909a0f06c3201 Mon Sep 17 00:00:00 2001 From: Eguzki Astiz Lezaun Date: Fri, 17 Feb 2023 11:46:59 +0100 Subject: [PATCH 5/7] oidc: unittest when oidc pubkey not found --- spec/oauth/oidc_spec.lua | 42 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/spec/oauth/oidc_spec.lua b/spec/oauth/oidc_spec.lua index f65f22276..04eea5626 100644 --- a/spec/oauth/oidc_spec.lua +++ b/spec/oauth/oidc_spec.lua @@ -45,6 +45,11 @@ describe('OIDC', function() config = { id_token_signing_alg_values_supported = { 'RS256', 'HS256' } }, keys = { somekid = { pem = rsa.pub, alg = 'RS256' } }, } + local oidc_config_no_alg = { + issuer = 'https://example.com/auth/realms/apicast', + config = { id_token_signing_alg_values_supported = { 'RS256', 'HS256' } }, + keys = { somekid = { pem = rsa.pub } }, + } before_each(function() jwt_validators.set_system_clock(function() return 0 end) end) @@ -268,6 +273,43 @@ describe('OIDC', function() assert(credentials, err) end) + it('validation passes when jwk.alg does not exist', function() + local oidc = _M.new(oidc_config_no_alg) + local access_token = jwt:sign(rsa.private, { + header = { typ = 'JWT', alg = 'RS256', kid = 'somekid' }, + payload = { + iss = oidc_config.issuer, + aud = 'notused', + azp = 'ce3b2e5e', + sub = 'someone', + nbf = 0, + exp = ngx.now() + 10, + typ = 'Bearer' + }, + }) + + local credentials, _, _, err = oidc:transform_credentials({ access_token = access_token }) + assert(credentials, err) + end) + + it('token key id not found', 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', err, nil, true) + end) + describe('getting client_id from any JWT claim', function() before_each(function() From 64f221c64f0b0c32c9dc18dcdac0de51b6837d1e Mon Sep 17 00:00:00 2001 From: Eguzki Astiz Lezaun Date: Tue, 21 Feb 2023 11:25:54 +0100 Subject: [PATCH 6/7] oidc: add test for issuer verification --- gateway/src/apicast/oauth/oidc.lua | 3 +- spec/oauth/oidc_spec.lua | 22 +++++++++++++-- t/apicast-oidc.t | 44 ++++++++++++++++++++++++++++-- 3 files changed, 63 insertions(+), 6 deletions(-) diff --git a/gateway/src/apicast/oauth/oidc.lua b/gateway/src/apicast/oauth/oidc.lua index e8d6ab98e..52edb8e4c 100644 --- a/gateway/src/apicast/oauth/oidc.lua +++ b/gateway/src/apicast/oauth/oidc.lua @@ -186,7 +186,8 @@ function _M:verify(jwt, cache_key) local jwk_obj = find_jwk(jwt, self.keys) if jwk_obj == nil then - return false, '[jwk] jwk not found, token might not belong to right realm' + 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 diff --git a/spec/oauth/oidc_spec.lua b/spec/oauth/oidc_spec.lua index 04eea5626..7e576d640 100644 --- a/spec/oauth/oidc_spec.lua +++ b/spec/oauth/oidc_spec.lua @@ -292,7 +292,7 @@ describe('OIDC', function() assert(credentials, err) end) - it('token key id not found', function() + 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' }, @@ -307,7 +307,25 @@ describe('OIDC', function() local credentials, _, _, err = oidc:transform_credentials({ access_token = access_token }) - assert.match('jwk not found', err, nil, true) + 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() diff --git a/t/apicast-oidc.t b/t/apicast-oidc.t index 3370a9735..6c8aab812 100644 --- a/t/apicast-oidc.t +++ b/t/apicast-oidc.t @@ -363,8 +363,7 @@ my $jwt = 'eyJraWQiOiJzb21la2lkIiwiYWxnIjoiSFMyNTYifQ.'. --- error_log [jwt] alg mismatch -=== TEST 8: Key not available -(Happens when the token belongs to a different realm) +=== TEST 8: Token was signed by a different key --- configuration env eval use JSON qw(to_json); @@ -401,4 +400,43 @@ my $jwt = encode_jwt(payload => { exp => time + 3600 }, key => \$::private_key, alg => 'RS256', extra_headers => { kid => 'otherkid' }); "Authorization: Bearer $jwt" --- error_log -[jwk] jwk not found +[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/ ] From 91c0b467a03298e45b601204aaf457d04e1bfb9e Mon Sep 17 00:00:00 2001 From: Eguzki Astiz Lezaun Date: Tue, 21 Feb 2023 16:06:38 +0100 Subject: [PATCH 7/7] CHANGELOG updated for 3.12.2 --- CHANGELOG.md | 23 ++++------------------- 1 file changed, 4 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3bf6c7cf4..47e152466 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,28 +5,11 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). - -## [3.11.0] 2021-09-03 ## [Unreleased] -### Fixed - -- Fixed: OIDC jwt key verification [PR #1389](https://github.com/3scale/APIcast/pull/1389) [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) -- Fix to avoid uninitialized variables when request URI is too large [PR #1340](https://github.com/3scale/APIcast/pull/1340) [THREESCALE-7906](https://issues.redhat.com/browse/THREESCALE-7906) -- Fixed issue where request path is stripped for proxied https requests [PR #1342](https://github.com/3scale/APIcast/pull/1342) [THREESCALE-8426](https://issues.redhat.com/browse/THREESCALE-8426) -- Bumped liquid-lua to version 0.2.0-2 [PR #1369](https://github.com/3scale/APIcast/pull/1369) - includes: [THREESCALE-8483](https://issues.redhat.com/browse/THREESCALE-8483) and [THREESCALE-8484](https://issues.redhat.com/browse/THREESCALE-8484) -- Fixed: APIcast could not retrieve the latest version of the proxy config [PR #1370](https://github.com/3scale/APIcast/pull/1370) [THREESCALE-8485](https://issues.redhat.com/browse/THREESCALE-8485) -- Fixed: JWKs without alg field cause the JWT validation process to fail [PR #1371](https://github.com/3scale/APIcast/pull/1371) [THREESCALE-8601](https://issues.redhat.com/browse/THREESCALE-8601) - -### Added +## [3.12.2] 2023-02-21 -- Updated policy list [PR #1374](https://github.com/3scale/APIcast/pull/1374) +- 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 @@ -981,3 +964,5 @@ Apart from the changes mentioned in this section, this version also includes the [3.10.0-beta1]: https://github.com/3scale/apicast/compare/v3.10.0-alpha2..v3.10.0-beta1 [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