Skip to content

Commit

Permalink
fix(router) ignore SNI matcher when request protocol is not https.
Browse files Browse the repository at this point in the history
…fixes #6425
  • Loading branch information
dndx committed Oct 27, 2020
1 parent c4441f5 commit 674d55a
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 2 deletions.
8 changes: 6 additions & 2 deletions kong/router.lua
Original file line number Diff line number Diff line change
Expand Up @@ -545,7 +545,6 @@ local function marshall_route(r)

-- snis


if snis then
if type(snis) ~= "table" then
return nil, "snis field must be a table"
Expand Down Expand Up @@ -974,7 +973,7 @@ do

[MATCH_RULES.SNI] = function(route_t, ctx)
local sni = route_t.snis[ctx.sni]
if sni then
if sni or ctx.req_scheme == "http" then
ctx.matches.sni = ctx.sni
return true
end
Expand Down Expand Up @@ -1236,6 +1235,10 @@ function _M.new(routes)
return r1.max_uri_length > r2.max_uri_length
end

--if #r1.route.protocols ~= #r2.route.protocols then
-- return #r1.route.protocols < #r2.route.protocols
--end

if r1.route.created_at ~= nil and r2.route.created_at ~= nil then
return r1.route.created_at < r2.route.created_at
end
Expand Down Expand Up @@ -1354,6 +1357,7 @@ function _M.new(routes)
ctx.req_method = req_method
ctx.req_uri = req_uri
ctx.req_host = req_host
ctx.req_scheme = req_scheme
ctx.req_headers = req_headers
ctx.src_ip = src_ip or ""
ctx.src_port = src_port or ""
Expand Down
59 changes: 59 additions & 0 deletions spec/01-unit/08-router_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -3305,16 +3305,75 @@ describe("Router", function()
snis = { "www.example.org" }
}
},
-- see #6425
{
service = service,
route = {
hosts = {
"sni.example.com",
},
protocols = {
"http", "https",
},
snis = {
"sni.example.com",
},
},
},
{
service = service,
route = {
hosts = {
"sni.example.com",
},
protocols = {
"http",
},
},
},
}

local use_case_ignore_sni = {
-- see #6425
{
service = service,
route = {
hosts = {
"sni.example.com",
},
protocols = {
"http", "https",
},
snis = {
"sni.example.com",
},
},
},
}

local router = assert(Router.new(use_case))
local router_ignore_sni = assert(Router.new(use_case_ignore_sni))

it("[sni]", function()
local match_t = router.select(nil, nil, nil, "tcp", nil, nil, nil, nil,
"www.example.org")
assert.truthy(match_t)
assert.same(use_case[1].route, match_t.route)
end)

it("[sni] is ignored for http request but does not shadow `protocols={'http'}` only routes", function()
local match_t = router_ignore_sni.select(nil, nil, "sni.example.com",
"http", nil, nil, nil, nil,
nil)
assert.truthy(match_t)
assert.same(use_case_ignore_sni[1].route, match_t.route)

match_t = router.select(nil, nil, "sni.example.com",
"http", nil, nil, nil, nil,
nil)
assert.truthy(match_t)
assert.same(use_case[3].route, match_t.route)
end)
end)


Expand Down
22 changes: 22 additions & 0 deletions spec/02-integration/05-proxy/06-ssl_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,13 @@ for _, strategy in helpers.each_strategy() do
service = service2,
}

bp.routes:insert {
protocols = { "https" },
hosts = { "sni.example.com" },
snis = { "sni.example.com" },
service = service2,
}

local service4 = bp.services:insert {
name = "api-3",
protocol = helpers.mock_upstream_ssl_protocol,
Expand Down Expand Up @@ -338,6 +345,21 @@ for _, strategy in helpers.each_strategy() do
assert.same({ message = "Please use HTTPS protocol" }, json)
assert.contains("Upgrade", res.headers.connection)
assert.equal("TLS/1.2, HTTP/1.1", res.headers.upgrade)

-- SNI case, see #6425
res = assert(proxy_client:send {
method = "GET",
path = "/",
headers = {
["Host"] = "sni.example.com",
}
})

body = assert.res_status(426, res)
json = cjson.decode(body)
assert.same({ message = "Please use HTTPS protocol" }, json)
assert.contains("Upgrade", res.headers.connection)
assert.equal("TLS/1.2, HTTP/1.1", res.headers.upgrade)
end)

it("returns 301 when route has https_redirect_status_code set to 301", function()
Expand Down

0 comments on commit 674d55a

Please sign in to comment.