Skip to content

Commit

Permalink
fix(cors) improve regex matching behavior
Browse files Browse the repository at this point in the history
This reverts a regression introduced in #3872, in which regexes were
being forcibly anchored and matched against normalized domains, leading
to a breaking change with regard to the 0.x behavior.

In 0.x, regexes such as `(.*[.])?foo\.test` would accept subdomain
entries (but were subject to bug #3832 as it would also accept
`foo.test.evil.test`); in 1.0.2, the latter is not accepted, but the
regular uses failed as well, because regexes were translated to
`^(.*[.])?foo\.test$` but were then matched against the normalized
domains in a way that always included the port.

With this commit, both configured origins and the input header value are
normalized so that default ports are not an issue for both regex and
non-regex cases. We verify if each configured origin is a regex or not,
then:

* for non-regex entries, we do plain equality matching against the normalized
  domain
* For regex entries,
  * if the regex contains `:`, we do an anchored match against the normalized
    domain
  * otherwise, we do an anchored match against the host component only
    (to account for the 0.x behavior where ports were not considered)

Matching domains with regexes must be done with care (for starters, note
that dots must be escaped), so we recommend using plain-text full-domain
matching whenever possible.

This change in behavior is arguably a breaking change, but it is a
compromise between a workable behavior and backwards compatibility. Good
domain-matching regexes such as `(.*[.])?foo\.test` will remain matching
against the host component as intended (without being subject to #3832).
Naive regexes such as `.foo.test` will stop "working", but these were
vulnerable to #3832 anyway. In particular, thorough regexes such as
`^https?://(.*[.])?foo\\.test(:(80|90))?$` that performed their own
anchoring remain working as well.

From #4261

Signed-off-by: Thibault Charbonnier <thibaultcha@me.com>
  • Loading branch information
hishamhm authored and thibaultcha committed Jan 30, 2019
1 parent 34b88d7 commit 8213f97
Show file tree
Hide file tree
Showing 2 changed files with 344 additions and 27 deletions.
88 changes: 64 additions & 24 deletions kong/plugins/cors/handler.lua
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ local url = require "socket.url"

local kong = kong
local re_find = ngx.re.find
local find = string.find
local concat = table.concat
local tostring = tostring
local ipairs = ipairs
Expand Down Expand Up @@ -32,21 +33,24 @@ local normalized_req_domains = lrucache.new(10e3)
local function normalize_origin(domain)
local parsed_obj = assert(url.parse(domain))
if not parsed_obj.host then
return domain
return {
domain = domain,
host = domain,
}
end

local port = parsed_obj.port
if not port and parsed_obj.scheme then
if parsed_obj.scheme == "http" then
port = 80

elseif parsed_obj.scheme == "https" then
port = 443
end
if (parsed_obj.scheme == "http" and port == "80")
or (parsed_obj.scheme == "https" and port == "443") then
port = nil
end

return (parsed_obj.scheme and parsed_obj.scheme .. "://" or "") ..
parsed_obj.host .. (port and ":" .. port or "")
return {
domain = (parsed_obj.scheme and parsed_obj.scheme .. "://" or "") ..
parsed_obj.host ..
(port and ":" .. port or ""),
host = parsed_obj.host,
}
end


Expand Down Expand Up @@ -83,15 +87,40 @@ local function configure_origin(conf)

local req_origin = kong.request.get_header("origin")
if req_origin then
local normalized_domains = config_cache[conf]
if not normalized_domains then
normalized_domains = {}

for _, domain in ipairs(conf.origins) do
table.insert(normalized_domains, normalize_origin(domain))
local cached_domains = config_cache[conf]
if not cached_domains then
cached_domains = {}

for _, entry in ipairs(conf.origins) do
local domain
local maybe_regex, _, err = re_find(entry, "[^A-Za-z0-9.:/-]", "jo")
if err then
kong.log.err("could not inspect origin for type: ", err)
end

if maybe_regex then
-- Kong 0.x did not anchor regexes:
-- Perform adjustments to support regexes
-- explicitly anchored by the user.
if entry:sub(-1) ~= "$" then
entry = entry .. "$"
end

if entry:sub(1, 1) == "^" then
entry = entry:sub(2)
end

domain = { regex = entry }

else
domain = normalize_origin(entry)
end

domain.by_host = not find(entry, ":", 1, true)
table.insert(cached_domains, domain)
end

config_cache[conf] = normalized_domains
config_cache[conf] = cached_domains
end

local normalized_req_origin = normalized_req_domains:get(req_origin)
Expand All @@ -100,20 +129,31 @@ local function configure_origin(conf)
normalized_req_domains:set(req_origin, normalized_req_origin)
end

for _, normalized_domain in ipairs(normalized_domains) do
local from, _, err = re_find(normalized_req_origin,
normalized_domain .. "$", "ajo")
if err then
kong.log.err("could not search for domain: ", err)
for _, cached_domain in ipairs(cached_domains) do
local found, _, err

if cached_domain.regex then
local subject = cached_domain.by_host
and normalized_req_origin.host
or normalized_req_origin.domain

found, _, err = re_find(subject, cached_domain.regex, "ajo")
if err then
kong.log.err("could not search for domain: ", err)
end

else
found = (normalized_req_origin.domain == cached_domain.domain)
end

if from then
set_header("Access-Control-Allow-Origin", req_origin)
if found then
set_header("Access-Control-Allow-Origin", normalized_req_origin.domain)
set_header("Vary", "Origin")
return false
end
end
end

return false
end

Expand Down
Loading

0 comments on commit 8213f97

Please sign in to comment.