Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(jwt): deny requests that have different tokens in the jwt token search locations. #9946

Merged
merged 5 commits into from
Dec 29, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,16 @@

## Unreleased

### Breaking Changes

#### Plugins

- **JWT**: JWT plugin now denies a request that has different tokens in the jwt token search locations.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- **JWT**: JWT plugin now denies a request that has different tokens in the jwt token search locations.
- **JWT**: JWT plugin now denies a request that has different JWTs in the HTTP request header and Query parameter.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are three search locations, which are querystring(query parameter), header, and cookie.

[#9946](https://github.com/Kong/kong/pull/9946)

### Additions

### Plugins
#### Plugins

- **Zipkin**: Add support to set the durations of Kong phases as span tags
through configuration property `config.phase_duration_flavor`.
Expand All @@ -94,6 +101,8 @@

- **Zipkin**: Fix an issue where the global plugin's sample ratio overrides route-specific.
[#9877](https://github.com/Kong/kong/pull/9877)
- **JWT**: Deny requests that have different tokens in the jwt token search locations. Thanks Jackson 'Che-Chun' Kuo from Latacora for reporting this issue.
[#9946](https://github.com/Kong/kong/pull/9946)
vm-001 marked this conversation as resolved.
Show resolved Hide resolved

### Dependencies

Expand Down
43 changes: 37 additions & 6 deletions kong/plugins/jwt/handler.lua
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ local kong = kong
local type = type
local error = error
local ipairs = ipairs
local pairs = pairs
local tostring = tostring
local re_gmatch = ngx.re.gmatch

Expand All @@ -24,19 +25,30 @@ local JwtHandler = {
-- @param conf Plugin configuration
-- @return token JWT token contained in request (can be a table) or nil
-- @return err
local function retrieve_token(conf)
local function retrieve_tokens(conf)
local token_set = {}
local args = kong.request.get_query()
for _, v in ipairs(conf.uri_param_names) do
if args[v] then
return args[v]
local token = args[v] -- can be a table
if token then
if type(token) == "table" then
for _, t in ipairs(token) do
if t ~= "" then
token_set[t] = true
end
end

elseif token ~= "" then
token_set[token] = true
end
end
end

local var = ngx.var
for _, v in ipairs(conf.cookie_names) do
local cookie = var["cookie_" .. v]
if cookie and cookie ~= "" then
return cookie
token_set[cookie] = true
end
end

Expand All @@ -60,10 +72,29 @@ local function retrieve_token(conf)
end

if m and #m > 0 then
return m[1]
if m[1] ~= "" then
token_set[m[1]] = true
end
end
end
end

local tokens_n = 0
local tokens = {}
for token, _ in pairs(token_set) do
tokens_n = tokens_n + 1
tokens[tokens_n] = token
end

if tokens_n == 0 then
return nil
end

if tokens_n == 1 then
return tokens[1]
end

return tokens
end


Expand Down Expand Up @@ -117,7 +148,7 @@ end


local function do_authentication(conf)
local token, err = retrieve_token(conf)
local token, err = retrieve_tokens(conf)
if err then
return error(err)
end
Expand Down
15 changes: 15 additions & 0 deletions spec/03-plugins/16-jwt/03-access_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,21 @@ for _, strategy in helpers.each_strategy() do
assert.falsy(ok)
assert.match("Code: Unauthenticated", err)
end)

it("reject if multiple different tokens found", function()
PAYLOAD.iss = jwt_secret.key
local jwt = jwt_encoder.encode(PAYLOAD, jwt_secret.secret)
local res = assert(proxy_client:send {
method = "GET",
path = "/request?jwt=" .. jwt,
headers = {
["Authorization"] = "Bearer invalid.jwt.token",
["Host"] = "jwt1.com",
}
})
local body = cjson.decode(assert.res_status(401, res))
assert.same({ message = "Multiple tokens provided" }, body)
end)
end)

describe("HS256", function()
Expand Down