fix(core): definition of cookie name validate

Fix #11860

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

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 },

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,

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

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

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