From 6263443936440291da5654a7bff00acf50fe7c40 Mon Sep 17 00:00:00 2001 From: Mark van Holsteijn Date: Thu, 22 Mar 2018 20:44:08 +0100 Subject: [PATCH] feat(jwt) add a maximum_expiration config value Support for limiting the expiration period on JWT tokens. In the JWT plugin you can set the property `maximum_expiration` to a positive integer, indicating the maximum number of seconds the `exp` claim in the token may be ahead in the future. From #3331 Signed-off-by: Thibault Charbonnier --- kong/plugins/jwt/handler.lua | 8 ++++ kong/plugins/jwt/jwt_parser.lua | 18 ++++++++ kong/plugins/jwt/migrations/cassandra.lua | 18 ++++++++ kong/plugins/jwt/migrations/postgres.lua | 18 ++++++++ kong/plugins/jwt/schema.lua | 32 ++++++++++++++ spec/03-plugins/17-jwt/03-access_spec.lua | 41 +++++++++++++++++- spec/03-plugins/17-jwt/06-schema_spec.lua | 53 +++++++++++++++++++++++ 7 files changed, 187 insertions(+), 1 deletion(-) create mode 100644 spec/03-plugins/17-jwt/06-schema_spec.lua diff --git a/kong/plugins/jwt/handler.lua b/kong/plugins/jwt/handler.lua index f73ca638c4c..47d8b61cb23 100644 --- a/kong/plugins/jwt/handler.lua +++ b/kong/plugins/jwt/handler.lua @@ -165,6 +165,14 @@ local function do_authentication(conf) return false, {status = 401, message = errors} end + -- Verify the JWT registered claims + if conf.maximum_expiration ~= nil and conf.maximum_expiration > 0 then + local ok, errors = jwt:check_maximum_expiration(conf.maximum_expiration) + if not ok then + return false, {status = 403, message = errors} + end + end + -- Retrieve the consumer local consumer_cache_key = singletons.db.consumers:cache_key(jwt_secret.consumer_id) local consumer, err = singletons.cache:get(consumer_cache_key, nil, diff --git a/kong/plugins/jwt/jwt_parser.lua b/kong/plugins/jwt/jwt_parser.lua index b080ce8abdd..c36573f3d94 100644 --- a/kong/plugins/jwt/jwt_parser.lua +++ b/kong/plugins/jwt/jwt_parser.lua @@ -282,6 +282,24 @@ function _M:verify_registered_claims(claims_to_verify) return errors == nil, errors end +--- Check that the maximum allowed expiration is not reached +-- @param maximum_expiration of the claim +-- @return A Boolean indicating true if the claim has reached the maximum +-- allowed expiration time +-- @return error if any +function _M:check_maximum_expiration(maximum_expiration) + if maximum_expiration <= 0 then + return true + end + + local exp = self.claims["exp"] + if exp == nil or exp - ngx_time() > maximum_expiration then + return false, {exp = "exceeds maximum allowed expiration"} + end + + return true +end + _M.encode = encode_token return _M diff --git a/kong/plugins/jwt/migrations/cassandra.lua b/kong/plugins/jwt/migrations/cassandra.lua index e2aefc7703a..837e5e2ea53 100644 --- a/kong/plugins/jwt/migrations/cassandra.lua +++ b/kong/plugins/jwt/migrations/cassandra.lua @@ -68,4 +68,22 @@ return { end, down = function(_, _, dao) end -- not implemented }, + { + name = "2018-03-15-150000_jwt_maximum_expiration", + up = function(_, _, dao) + for ok, config, update in plugin_config_iterator(dao, "jwt") do + if not ok then + return config + end + if config.maximum_expiration == nil then + config.maximum_expiration = 0 + local _, err = update(config) + if err then + return err + end + end + end + end, + down = function(_, _, dao) end -- not implemented + }, } diff --git a/kong/plugins/jwt/migrations/postgres.lua b/kong/plugins/jwt/migrations/postgres.lua index 822db9fbc8b..082e93a4342 100644 --- a/kong/plugins/jwt/migrations/postgres.lua +++ b/kong/plugins/jwt/migrations/postgres.lua @@ -86,4 +86,22 @@ return { end, down = function(_, _, dao) end -- not implemented }, + { + name = "2018-03-15-150000_jwt_maximum_expiration", + up = function(_, _, dao) + for ok, config, update in plugin_config_iterator(dao, "jwt") do + if not ok then + return config + end + if config.maximum_expiration == nil then + config.maximum_expiration = 0 + local _, err = update(config) + if err then + return err + end + end + end + end, + down = function(_, _, dao) end -- not implemented + }, } diff --git a/kong/plugins/jwt/schema.lua b/kong/plugins/jwt/schema.lua index 89611b78a2d..bb48ae04720 100644 --- a/kong/plugins/jwt/schema.lua +++ b/kong/plugins/jwt/schema.lua @@ -1,4 +1,5 @@ local utils = require "kong.tools.utils" +local Errors = require "kong.dao.errors" local function check_user(anonymous) if anonymous == "" or utils.is_valid_uuid(anonymous) then @@ -8,6 +9,14 @@ local function check_user(anonymous) return false, "the anonymous user must be empty or a valid uuid" end +local function check_positive(v) + if v < 0 then + return false, "should be 0 or greater" + end + + return true +end + return { no_consumer = true, fields = { @@ -18,5 +27,28 @@ return { claims_to_verify = {type = "array", enum = {"exp", "nbf"}}, anonymous = {type = "string", default = "", func = check_user}, run_on_preflight = {type = "boolean", default = true}, + maximum_expiration = {type = "number", default = 0, func = check_positive}, }, + self_check = function(schema, plugin_t, dao, is_update) + if plugin_t.maximum_expiration ~= nil + and plugin_t.maximum_expiration > 0 + then + local has_exp + + if plugin_t.claims_to_verify then + for index, value in ipairs(plugin_t.claims_to_verify) do + if value == "exp" then + has_exp = true + break + end + end + end + + if not has_exp then + return false, Errors.schema "claims_to_verify must contain 'exp' when specifying maximum_expiration" + end + end + + return true + end } diff --git a/spec/03-plugins/17-jwt/03-access_spec.lua b/spec/03-plugins/17-jwt/03-access_spec.lua index affec2312e1..b3a73f9e021 100644 --- a/spec/03-plugins/17-jwt/03-access_spec.lua +++ b/spec/03-plugins/17-jwt/03-access_spec.lua @@ -28,7 +28,7 @@ for _, strategy in helpers.each_strategy() do local routes = {} - for i = 1, 10 do + for i = 1, 11 do routes[i] = bp.routes:insert { hosts = { "jwt" .. i .. ".com" }, } @@ -105,6 +105,12 @@ for _, strategy in helpers.each_strategy() do config = { key_claim_name = "kid" }, }) + plugins:insert({ + name = "jwt", + route_id = routes[11].id, + config = { claims_to_verify = {"nbf", "exp"}, maximum_expiration = 300 }, + }) + plugins:insert({ name = "ctx-checker", route_id = routes[1].id, @@ -254,6 +260,39 @@ for _, strategy in helpers.each_strategy() do local body = assert.res_status(401, res) assert.equal([[{"message":"Unauthorized"}]], body) end) + it("returns 403 if the token exceeds the maximum allowed expiration limit", function() + local payload = { + iss = jwt_secret.key, + exp = os.time() + 3600, + nbf = os.time() - 30 + } + local jwt = jwt_encoder.encode(payload, jwt_secret.secret) + local res = assert(proxy_client:send { + method = "GET", + path = "/request/?jwt=" .. jwt, + headers = { + ["Host"] = "jwt11.com" + } + }) + local body = assert.res_status(403, res) + assert.equal('{"exp":"exceeds maximum allowed expiration"}', body) + end) + it("accepts a JWT token within the maximum allowed expiration limit", function() + local payload = { + iss = jwt_secret.key, + exp = os.time() + 270, + nbf = os.time() - 30 + } + local jwt = jwt_encoder.encode(payload, jwt_secret.secret) + local res = assert(proxy_client:send { + method = "GET", + path = "/request/?jwt=" .. jwt, + headers = { + ["Host"] = "jwt11.com" + } + }) + assert.res_status(200, res) + end) end) describe("HS256", function() diff --git a/spec/03-plugins/17-jwt/06-schema_spec.lua b/spec/03-plugins/17-jwt/06-schema_spec.lua new file mode 100644 index 00000000000..9dbe040b872 --- /dev/null +++ b/spec/03-plugins/17-jwt/06-schema_spec.lua @@ -0,0 +1,53 @@ +local validate_entity = require("kong.dao.schemas_validation").validate_entity +local jwt_schema = require "kong.plugins.jwt.schema" + + +describe("Plugin: jwt (schema)", function() + it("validates 'maximum_expiration'", function() + local ok, err = validate_entity({ + maximum_expiration = 60, + claims_to_verify = { "exp", "nbf" }, + }, jwt_schema) + + assert.is_nil(err) + assert.is_true(ok) + end) + + describe("errors", function() + it("when 'maximum_expiration' is negative", function() + local ok, err = validate_entity({ + maximum_expiration = -1, + claims_to_verify = { "exp", "nbf" }, + }, jwt_schema) + + assert.is_false(ok) + assert.same({ + maximum_expiration = "should be 0 or greater" + }, err) + + local ok, err = validate_entity({ + maximum_expiration = -1, + claims_to_verify = { "nbf" }, + }, jwt_schema) + + assert.is_false(ok) + assert.same({ + maximum_expiration = "should be 0 or greater" + }, err) + end) + + it("when 'maximum_expiration' is specified without 'exp' in 'claims_to_verify'", function() + local ok, err, self_err = validate_entity({ + maximum_expiration = 60, + claims_to_verify = { "nbf" }, + }, jwt_schema) + + assert.is_false(ok) + assert.is_nil(err) + assert.same({ + message = "claims_to_verify must contain 'exp' when specifying maximum_expiration", + schema = true, + }, self_err) + end) + end) +end)