feat(key-auth) validate the configured headernames

Adds validation of header names (was completely absent) due to
nginx/openresty config the '_' is also considered an invalid
character.

Fix Kong#2013

Signed-off-by: Thibault Charbonnier <[email protected]>

kong/plugins/key-auth/schema.lua

@@ -1,24 +1,52 @@
 local utils = require "kong.tools.utils"
 
+
 local function check_user(anonymous)
   if anonymous == "" or utils.is_valid_uuid(anonymous) then
     return true
   end
-  
+
   return false, "the anonymous user must be empty or a valid uuid"
 end
 
+
+local function check_keys(keys)
+  for _, key in ipairs(keys) do
+    local res, err = utils.validate_header_name(key, false)
+
+    if not res then
+      return false, "'" .. key .. "' is illegal: " .. err
+    end
+  end
+
+  return true
+end
+
+
 local function default_key_names(t)
   if not t.key_names then
-    return {"apikey"}
+    return { "apikey" }
   end
 end
 
+
 return {
   no_consumer = true,
   fields = {
-    key_names = {required = true, type = "array", default = default_key_names},
-    hide_credentials = {type = "boolean", default = false},
-    anonymous = {type = "string", default = "", func = check_user},
+    key_names = {
+      required = true,
+      type = "array",
+      default = default_key_names,
+      func = check_keys,
+    },
+    hide_credentials = {
+      type = "boolean",
+      default = false,
+    },
+    anonymous = {
+      type = "string",
+      default = "",
+      func = check_user,
+    },
   }
 }

kong/tools/utils.lua

@@ -29,6 +29,7 @@ local gsub       = string.gsub
 local split      = pl_stringx.split
 local strip      = pl_stringx.strip
 local re_find    = ngx.re.find
+local re_match   = ngx.re.match
 
 ffi.cdef[[
 typedef unsigned char u_char;
@@ -583,4 +584,22 @@ _M.format_host = function(p1, p2)
   end
 end
 
+--- 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.
+-- @param name (string) the header name to verify
+-- @return the valid header name, or `nil+error`
+_M.validate_header_name = function(name)
+  if name == nil or name == "" then
+    return nil, "no header name provided"
+  end
+
+  if re_match(name, "^[a-zA-Z0-9-]+$", "jo") then
+    return name
+  end
+
+  return nil, "bad header name '" .. name ..
+              "', allowed characters are A-Z, a-z, 0-9 and '-'"
+end
+
 return _M

spec/01-unit/04-utils_spec.lua

@@ -458,5 +458,20 @@ describe("Utils", function()
       end)
     end)
   end)
-
+
+  it("validate_header_name() validates header names", function()
+    local header_chars = [[-0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz]]
+
+    for i = 1, 255 do
+      local c = string.char(i)
+
+      if string.find(header_chars, c, nil, true) then
+        assert(utils.validate_header_name(c) == c,
+          "ascii character '" .. c .. "' (" .. i .. ") should have been allowed")
+      else
+        assert(utils.validate_header_name(c) == nil,
+          "ascii character " .. i .. " should not have been allowed")
+      end
+    end
+  end)
 end)

spec/03-plugins/02-key-auth/01-api_spec.lua

@@ -5,12 +5,21 @@ describe("Plugin: key-auth (API)", function()
   local consumer
   local admin_client
   setup(function()
-    assert(helpers.start_kong())
-    admin_client = helpers.admin_client()
-
+    assert(helpers.dao.apis:insert {
+      name = "keyauth1",
+      upstream_url = "http://mockbin.com",
+      hosts = { "keyauth1.test" },
+    })
+    assert(helpers.dao.apis:insert {
+      name = "keyauth2",
+      upstream_url = "http://mockbin.com",
+      hosts = { "keyauth2.test" },
+    })
     consumer = assert(helpers.dao.consumers:insert {
       username = "bob"
     })
+    assert(helpers.start_kong())
+    admin_client = helpers.admin_client()
   end)
   teardown(function()
     if admin_client then admin_client:close() end
@@ -254,4 +263,46 @@ describe("Plugin: key-auth (API)", function()
       end)
     end)
   end)
+  describe("/apis/:api/plugins", function()
+    it("fails with invalid key_names", function()
+      local key_name = "hello\\world"
+      local res = assert(admin_client:send {
+        method = "POST",
+        path = "/apis/keyauth1/plugins",
+        body = {
+          name = "key-auth",
+          config = {
+            key_names = {key_name},
+          },
+        },
+        headers = {
+          ["Content-Type"] = "application/json"
+        }
+      })
+      assert.response(res).has.status(400)
+      local body = assert.response(res).has.jsonbody()
+      assert.equal("'hello\\world' is illegal: bad header name " ..
+                   "'hello\\world', allowed characters are A-Z, a-z, 0-9 " ..
+                   "and '-'", body["config.key_names"])
+    end)
+    it("succeeds with valid key_names", function()
+      local key_name = "hello-world"
+      local res = assert(admin_client:send {
+        method = "POST",
+        path = "/apis/keyauth2/plugins",
+        body = {
+          name = "key-auth",
+          config = {
+            key_names = {key_name},
+          },
+        },
+        headers = {
+          ["Content-Type"] = "application/json"
+        }
+      })
+      assert.response(res).has.status(201)
+      local body = assert.response(res).has.jsonbody()
+      assert.equal(key_name, body.config.key_names[1])
+    end)
+  end)
 end)