From 27d4a8f5c433536688a325c124e521cda803033c Mon Sep 17 00:00:00 2001 From: xumin Date: Mon, 30 Oct 2023 15:26:08 +0800 Subject: [PATCH] fix(core): definition of cookie name validate Fix #11860 --- .../unreleased/kong/cookie-name-validator.yml | 3 ++ kong/db/schema/entities/upstreams.lua | 2 +- kong/db/schema/typedefs.lua | 8 ++++ kong/tools/utils.lua | 38 ++++++++++++------- spec/01-unit/05-utils_spec.lua | 4 +- .../04-admin_api/07-upstreams_routes_spec.lua | 4 +- 6 files changed, 40 insertions(+), 19 deletions(-) create mode 100644 changelog/unreleased/kong/cookie-name-validator.yml diff --git a/changelog/unreleased/kong/cookie-name-validator.yml b/changelog/unreleased/kong/cookie-name-validator.yml new file mode 100644 index 00000000000..5451b28531a --- /dev/null +++ b/changelog/unreleased/kong/cookie-name-validator.yml @@ -0,0 +1,3 @@ +message: Now cookie names are validated against RFC 6265, which allows more characters than the previous validation. +type: bugfix +scope: Core diff --git a/kong/db/schema/entities/upstreams.lua b/kong/db/schema/entities/upstreams.lua index eed59c788f7..6d3c963411c 100644 --- a/kong/db/schema/entities/upstreams.lua +++ b/kong/db/schema/entities/upstreams.lua @@ -189,7 +189,7 @@ local r = { { hash_fallback = hash_on }, { hash_on_header = typedefs.header_name, }, { hash_fallback_header = typedefs.header_name, }, - { hash_on_cookie = { description = "The cookie name to take the value from as hash input.", type = "string", custom_validator = utils.validate_cookie_name }, }, + { hash_on_cookie = typedefs.cookie_name{ description = "The cookie name to take the value from as hash input."}, }, { hash_on_cookie_path = typedefs.path{ default = "/", }, }, { hash_on_query_arg = simple_param }, { hash_fallback_query_arg = simple_param }, diff --git a/kong/db/schema/typedefs.lua b/kong/db/schema/typedefs.lua index 91c7c710093..3838b10d10b 100644 --- a/kong/db/schema/typedefs.lua +++ b/kong/db/schema/typedefs.lua @@ -331,6 +331,14 @@ typedefs.url = Schema.define { description = "A string representing a URL, such as https://example.com/path/to/resource?q=search." } + +typedefs.cookie_name = Schema.define { + type = "string", + custom_validator = utils.validate_cookie_name, + description = "A string representing an HTTP token defined by RFC 2616." +} + +-- should we also allow all http token for this? typedefs.header_name = Schema.define { type = "string", custom_validator = utils.validate_header_name, diff --git a/kong/tools/utils.lua b/kong/tools/utils.lua index 38e1825ba51..c823c399952 100644 --- a/kong/tools/utils.lua +++ b/kong/tools/utils.lua @@ -601,6 +601,29 @@ _M.format_host = function(p1, p2) end end +local CONTROLS = [[\x00-\x1F\x7F]] +local HIGHBIT = [[\x80-\xFF]] +local SEPARATORS = [==[ \t()<>@,;:\\\"\/?={}\[\]]==] +local HTTP_TOKEN_FORBID_PATTERN = "[".. CONTROLS .. HIGHBIT .. SEPARATORS .. "]" + +--- Validates a token defined by RFC 2616. +-- @param token (string) the string to verify +-- @return the valid token, or `nil+error` +function _M.validate_http_token(token) + if token == nil or token == "" then + return nil, "no token provided" + end + + if not re_match(token, HTTP_TOKEN_FORBID_PATTERN, "jo") then + return token + end + + return nil, "contains one or more invalid characters. ASCII " .. + "control characters (0-31;127), space, tab and the " .. + "characters ()<>@,;:\\\"/?={}[] are not allowed." +end + +-- should we also use validate_http_token for this? --- Validates a header name. -- Checks characters used in a header name to be valid, as per nginx only -- a-z, A-Z, 0-9 and '-' are allowed. @@ -620,22 +643,9 @@ _M.validate_header_name = function(name) end --- Validates a cookie name. --- Checks characters used in a cookie name to be valid --- a-z, A-Z, 0-9, '_' and '-' are allowed. -- @param name (string) the cookie name to verify -- @return the valid cookie name, or `nil+error` -_M.validate_cookie_name = function(name) - if name == nil or name == "" then - return nil, "no cookie name provided" - end - - if re_match(name, "^[a-zA-Z0-9-_]+$", "jo") then - return name - end - - return nil, "bad cookie name '" .. name .. - "', allowed characters are A-Z, a-z, 0-9, '_', and '-'" -end +_M.validate_cookie_name = _M.validate_http_token local validate_labels diff --git a/spec/01-unit/05-utils_spec.lua b/spec/01-unit/05-utils_spec.lua index 12764e67368..58af472e50e 100644 --- a/spec/01-unit/05-utils_spec.lua +++ b/spec/01-unit/05-utils_spec.lua @@ -656,12 +656,12 @@ describe("Utils", function() end end) it("validate_cookie_name() validates cookie names", function() - local header_chars = [[_-0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz]] + local cookie_chars = [[~`|!#$%&'*+-._-^0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz]] for i = 1, 255 do local c = string.char(i) - if string.find(header_chars, c, nil, true) then + if string.find(cookie_chars, c, nil, true) then assert(utils.validate_cookie_name(c) == c, "ascii character '" .. c .. "' (" .. i .. ") should have been allowed") else diff --git a/spec/02-integration/04-admin_api/07-upstreams_routes_spec.lua b/spec/02-integration/04-admin_api/07-upstreams_routes_spec.lua index 025435994d3..a7d5121bf32 100644 --- a/spec/02-integration/04-admin_api/07-upstreams_routes_spec.lua +++ b/spec/02-integration/04-admin_api/07-upstreams_routes_spec.lua @@ -404,7 +404,7 @@ describe("Admin API: #" .. strategy, function() }) body = assert.res_status(400, res) local json = cjson.decode(body) - assert.equals("bad cookie name 'not a <> valid <> cookie name', allowed characters are A-Z, a-z, 0-9, '_', and '-'", + assert.equals([[must not contain invalid characters: ASCII control characters (0-31;127), space, tab and the following characters: ()<>@,;:"/?={}[]. Please refer to RFC 2616]], json.fields.hash_on_cookie) -- Invalid cookie path @@ -437,7 +437,7 @@ describe("Admin API: #" .. strategy, function() }) body = assert.res_status(400, res) local json = cjson.decode(body) - assert.equals("bad cookie name 'not a <> valid <> cookie name', allowed characters are A-Z, a-z, 0-9, '_', and '-'", + assert.equals([[must not contain invalid characters: ASCII control characters (0-31;127), space, tab and the following characters: ()<>@,;:"/?={}[]. Please refer to RFC 2616]], json.fields.hash_on_cookie) -- Invalid cookie path in hash fallback