diff --git a/kong/pdk/request.lua b/kong/pdk/request.lua index 353908d337d..edb56d91a32 100644 --- a/kong/pdk/request.lua +++ b/kong/pdk/request.lua @@ -16,6 +16,7 @@ local find = string.find local lower = string.lower local type = type local error = error +local pcall = pcall local tonumber = tonumber local check_phase = phase_checker.check local check_not_phase = phase_checker.check_not @@ -285,6 +286,54 @@ local function new(self) end + --- + -- Returns the prefix path component of the request's URL that Kong stripped + -- before proxying to upstream. It also checks if `X-Forwarded-Prefix` comes + -- from a trusted source, and uses it as is when given. The value is returned + -- as a Lua string. + -- + -- In general, this function should be called after Kong has ran its router, + -- as the Kong router may strip the prefix of the request path. That stripped + -- path will become the return value of this function, unless there was already + -- a trusted `X-Forwarded-Prefix` header in the request. + -- + -- Whether this function considers `X-Forwarded-Prefix` or not depends on + -- several Kong configuration parameters: + -- + -- * [trusted\_ips](https://getkong.org/docs/latest/configuration/#trusted_ips) + -- * [real\_ip\_header](https://getkong.org/docs/latest/configuration/#real_ip_header) + -- * [real\_ip\_recursive](https://getkong.org/docs/latest/configuration/#real_ip_recursive) + -- + -- **Note**: we do not currently do any normalization on the request path prefix + -- except return `"/"` on empty prefix. + -- + -- @function kong.request.get_forwarded_prefix + -- @phases access, header_filter, body_filter, log, admin_api + -- @treturn string the forwarded path prefix + -- @usage + -- kong.request.get_forwarded_prefix() -- /prefix + function _REQUEST.get_forwarded_prefix() + check_phase(PHASES.request) + + if self.ip.is_trusted(self.client.get_ip()) then + local prefix = _REQUEST.get_header(X_FORWARDED_PREFIX) + if prefix then + return prefix + end + end + + local ok, prefix = pcall(function() + return ngx.var.upstream_x_forwarded_prefix + end) + + if not ok or not prefix then + return "/" + end + + return prefix == "" and "/" or prefix + end + + --- -- Returns the HTTP version used by the client in the request as a Lua -- number, returning values such as `1`, `1.1`, `2.0`, or `nil` for diff --git a/kong/router.lua b/kong/router.lua index 57535103115..1361e6e9c2a 100644 --- a/kong/router.lua +++ b/kong/router.lua @@ -846,9 +846,14 @@ do ctx.matches.uri = uri_t.value if m.uri_postfix then + ctx.matches.uri_prefix = sub(ctx.req_uri, 1, -(#m.uri_postfix + 1)) + -- remove the uri_postfix group - m[#m] = nil + m[#m] = nil m.uri_postfix = nil + + else + ctx.matches.uri_prefix = "/" end if uri_t.has_captures then @@ -860,6 +865,7 @@ do end -- plain or prefix match from the index + ctx.matches.uri_prefix = sub(ctx.req_uri, 1, #uri_t.value) ctx.matches.uri_postfix = sub(ctx.req_uri, #uri_t.value + 1) ctx.matches.uri = uri_t.value @@ -882,9 +888,14 @@ do ctx.matches.uri = uri_t.value if m.uri_postfix then + ctx.matches.uri_prefix = sub(ctx.req_uri, 1, -(#m.uri_postfix + 1)) + -- remove the uri_postfix group - m[#m] = nil + m[#m] = nil m.uri_postfix = nil + + else + ctx.matches.uri_prefix = "/" end if uri_t.has_captures then @@ -898,6 +909,7 @@ do -- plain or prefix match (not from the index) local from, to = find(ctx.req_uri, uri_t.value, nil, true) if from == 1 then + ctx.matches.uri_prefix = sub(ctx.req_uri, 1, to) ctx.matches.uri_postfix = sub(ctx.req_uri, to + 1) ctx.matches.uri = uri_t.value @@ -1560,9 +1572,13 @@ function _M.new(routes) matched_route.route = routes_by_id[matched_route.route.id].route end + local request_prefix + -- Path construction if matched_route.type == "http" then + request_prefix = matched_route.strip_uri and matches.uri_prefix or "/" + -- if we do not have a path-match, then the postfix is simply the -- incoming path, without the initial slash local request_postfix = matches.uri_postfix or sub(req_uri, 2, -1) @@ -1651,6 +1667,7 @@ function _M.new(routes) upstream_scheme = upstream_url_t.scheme, upstream_uri = upstream_uri, upstream_host = upstream_host, + prefix = request_prefix, matches = { uri_captures = matches.uri_captures, uri = matches.uri, diff --git a/kong/runloop/handler.lua b/kong/runloop/handler.lua index e8909201ca6..9bae94fa597 100644 --- a/kong/runloop/handler.lua +++ b/kong/runloop/handler.lua @@ -1112,6 +1112,7 @@ return { local forwarded_host local forwarded_port local forwarded_path + local forwarded_prefix -- X-Forwarded-* Headers Parsing -- @@ -1128,6 +1129,7 @@ return { forwarded_host = var.http_x_forwarded_host or host forwarded_port = var.http_x_forwarded_port or port forwarded_path = var.http_x_forwarded_path + forwarded_prefix = var.http_x_forwarded_prefix else forwarded_proto = scheme @@ -1143,6 +1145,14 @@ return { end end + if not forwarded_prefix then + forwarded_prefix = match_t.prefix + + if not forwarded_prefix or forwarded_prefix == "" then + forwarded_prefix = "/" + end + end + local protocols = route.protocols if (protocols and protocols.https and not protocols.http and forwarded_proto ~= "https") @@ -1231,6 +1241,7 @@ return { var.upstream_x_forwarded_host = forwarded_host var.upstream_x_forwarded_port = forwarded_port var.upstream_x_forwarded_path = forwarded_path + var.upstream_x_forwarded_prefix = forwarded_prefix -- At this point, the router and `balancer_setup_stage1` have been -- executed; detect requests that need to be redirected from `proxy_pass` diff --git a/spec/01-unit/08-router_spec.lua b/spec/01-unit/08-router_spec.lua index ca3ad620b4e..03937943303 100644 --- a/spec/01-unit/08-router_spec.lua +++ b/spec/01-unit/08-router_spec.lua @@ -2498,7 +2498,7 @@ describe("Router", function() assert.equal("/endel%C3%B8st", match_t.upstream_uri) end) - describe("stripped paths", function() + describe("stripped paths #strip", function() local router local use_case_routes = { { @@ -2516,6 +2516,7 @@ describe("Router", function() id = uuid(), methods = { "POST" }, paths = { "/my-route", "/this-route" }, + strip_path = false, }, }, } @@ -2530,6 +2531,7 @@ describe("Router", function() router._set_ngx(_ngx) local match_t = router.exec() assert.same(use_case_routes[1].route, match_t.route) + assert.equal("/my-route", match_t.prefix) assert.equal("/hello/world", match_t.upstream_uri) end) @@ -2538,6 +2540,7 @@ describe("Router", function() router._set_ngx(_ngx) local match_t = router.exec() assert.same(use_case_routes[1].route, match_t.route) + assert.equal("/my-route", match_t.prefix) assert.equal("/", match_t.upstream_uri) end) @@ -2547,6 +2550,7 @@ describe("Router", function() router._set_ngx(_ngx) local match_t = router.exec() assert.same(use_case_routes[2].route, match_t.route) + assert.equal("/", match_t.prefix) assert.equal("/my-route/hello/world", match_t.upstream_uri) end) @@ -2568,6 +2572,7 @@ describe("Router", function() router._set_ngx(_ngx) local match_t = router.exec() assert.same(use_case_routes[1].route, match_t.route) + assert.equal("/", match_t.prefix) assert.equal("/my-route/hello/world", match_t.upstream_uri) end) @@ -2576,12 +2581,14 @@ describe("Router", function() router._set_ngx(_ngx) local match_t = router.exec() assert.same(use_case_routes[1].route, match_t.route) + assert.equal("/my-route", match_t.prefix) assert.equal("/", match_t.upstream_uri) _ngx = mock_ngx("GET", "/my-route", { host = "domain.org" }) router._set_ngx(_ngx) match_t = router.exec() assert.same(use_case_routes[1].route, match_t.route) + assert.equal("/my-route", match_t.prefix) assert.equal("/", match_t.upstream_uri) end) @@ -2590,24 +2597,28 @@ describe("Router", function() router._set_ngx(_ngx) local match_t = router.exec() assert.same(use_case_routes[1].route, match_t.route) + assert.equal("/my-route", match_t.prefix) assert.equal("/", match_t.upstream_uri) _ngx = mock_ngx("GET", "/this-route", { host = "domain.org" }) router._set_ngx(_ngx) match_t = router.exec() assert.same(use_case_routes[1].route, match_t.route) + assert.equal("/this-route", match_t.prefix) assert.equal("/", match_t.upstream_uri) _ngx = mock_ngx("GET", "/my-route", { host = "domain.org" }) router._set_ngx(_ngx) match_t = router.exec() assert.same(use_case_routes[1].route, match_t.route) + assert.equal("/my-route", match_t.prefix) assert.equal("/", match_t.upstream_uri) _ngx = mock_ngx("GET", "/this-route", { host = "domain.org" }) router._set_ngx(_ngx) match_t = router.exec() assert.same(use_case_routes[1].route, match_t.route) + assert.equal("/this-route", match_t.prefix) assert.equal("/", match_t.upstream_uri) end) @@ -2627,6 +2638,7 @@ describe("Router", function() router._set_ngx(_ngx) local match_t = router.exec() assert.same(use_case_routes[1].route, match_t.route) + assert.equal("/endel%C3%B8st", match_t.prefix) assert.equal("/", match_t.upstream_uri) end) @@ -2646,6 +2658,7 @@ describe("Router", function() { host = "domain.org" }) router._set_ngx(_ngx) local match_t = router.exec() + assert.equal("/users/123/profile", match_t.prefix) assert.equal("/hello/world", match_t.upstream_uri) end) @@ -2665,6 +2678,7 @@ describe("Router", function() { host = "domain.org" }) router._set_ngx(_ngx) local match_t = router.exec() + assert.equal("/users/123/profile", match_t.prefix) assert.equal("/hello/world", match_t.upstream_uri) end) end) diff --git a/spec/02-integration/05-proxy/03-upstream_headers_spec.lua b/spec/02-integration/05-proxy/03-upstream_headers_spec.lua index e7e55868209..cc21c2978f3 100644 --- a/spec/02-integration/05-proxy/03-upstream_headers_spec.lua +++ b/spec/02-integration/05-proxy/03-upstream_headers_spec.lua @@ -303,6 +303,25 @@ for _, strategy in helpers.each_strategy() do end) end) + describe("X-Forwarded-Prefix", function() + it("should be added if not present in request", function() + local headers = request_headers { + ["Host"] = "headers-inspect.com", + } + + assert.equal("/", headers["x-forwarded-prefix"]) + end) + + it("should be replaced if present in request", function() + local headers = request_headers { + ["Host"] = "headers-inspect.com", + ["X-Forwarded-Prefix"] = "/replaced", + } + + assert.equal("/", headers["x-forwarded-prefix"]) + end) + end) + describe("with the downstream host preserved", function() it("should be added if not present in request while preserving the downstream host", function() local headers = request_headers { @@ -316,6 +335,7 @@ for _, strategy in helpers.each_strategy() do assert.equal("preserved.com", headers["x-forwarded-host"]) assert.equal(helpers.get_proxy_port(false), tonumber(headers["x-forwarded-port"])) assert.equal("/", headers["x-forwarded-path"]) + assert.equal("/", headers["x-forwarded-prefix"]) end) it("should be added if present in request while preserving the downstream host", function() @@ -335,6 +355,7 @@ for _, strategy in helpers.each_strategy() do assert.equal("preserved.com", headers["x-forwarded-host"]) assert.equal(helpers.get_proxy_port(false), tonumber(headers["x-forwarded-port"])) assert.equal("/", headers["x-forwarded-path"]) + assert.equal("/", headers["x-forwarded-prefix"]) end) end) @@ -353,6 +374,7 @@ for _, strategy in helpers.each_strategy() do assert.equal("headers-inspect.com", headers["x-forwarded-host"]) assert.equal(helpers.get_proxy_port(false), tonumber(headers["x-forwarded-port"])) assert.equal("/", headers["x-forwarded-path"]) + assert.equal("/", headers["x-forwarded-prefix"]) end) it("if present in request while discarding the downstream host", function() @@ -374,6 +396,7 @@ for _, strategy in helpers.each_strategy() do assert.equal("headers-inspect.com", headers["x-forwarded-host"]) assert.equal(helpers.get_proxy_port(false), tonumber(headers["x-forwarded-port"])) assert.equal("/", headers["x-forwarded-path"]) + assert.equal("/", headers["x-forwarded-prefix"]) end) end) @@ -505,6 +528,25 @@ for _, strategy in helpers.each_strategy() do end) end) + describe("X-Forwarded-Prefix", function() + it("should be added if not present in request", function() + local headers = request_headers { + ["Host"] = "headers-inspect.com", + } + + assert.equal("/", headers["x-forwarded-prefix"]) + end) + + it("should be forwarded if present in request", function() + local headers = request_headers { + ["Host"] = "headers-inspect.com", + ["X-Forwarded-Prefix"] = "/original-path", + } + + assert.equal("/original-path", headers["x-forwarded-prefix"]) + end) + end) + end) describe("(using the non-trusted configuration values)", function() @@ -632,6 +674,25 @@ for _, strategy in helpers.each_strategy() do assert.equal("/", headers["x-forwarded-path"]) end) end) + + describe("X-Forwarded-Prefix", function() + it("should be added if not present in request", function() + local headers = request_headers { + ["Host"] = "headers-inspect.com", + } + + assert.equal("/", headers["x-forwarded-prefix"]) + end) + + it("should be replaced if present in request", function() + local headers = request_headers { + ["Host"] = "headers-inspect.com", + ["X-Forwarded-Prefix"] = "/untrusted", + } + + assert.equal("/", headers["x-forwarded-prefix"]) + end) + end) end) describe("(using the recursive trusted configuration values)", function() diff --git a/t/01-pdk/04-request/00-phase_checks.t b/t/01-pdk/04-request/00-phase_checks.t index f2f7765af8d..e993f975ac8 100644 --- a/t/01-pdk/04-request/00-phase_checks.t +++ b/t/01-pdk/04-request/00-phase_checks.t @@ -107,6 +107,17 @@ qq{ body_filter = true, log = true, admin_api = true, + }, { + method = "get_forwarded_prefix", + args = {}, + init_worker = false, + certificate = "pending", + rewrite = true, + access = true, + header_filter = true, + body_filter = true, + log = true, + admin_api = true, }, { method = "get_http_version", args = {}, diff --git a/t/01-pdk/04-request/19-get_forwarded_prefix.t b/t/01-pdk/04-request/19-get_forwarded_prefix.t new file mode 100644 index 00000000000..e12d4f87453 --- /dev/null +++ b/t/01-pdk/04-request/19-get_forwarded_prefix.t @@ -0,0 +1,147 @@ +use strict; +use warnings FATAL => 'all'; +use Test::Nginx::Socket::Lua; +use t::Util; + +$ENV{TEST_NGINX_CERT_DIR} ||= File::Spec->catdir(server_root(), '..', 'certs'); +$ENV{TEST_NGINX_NXSOCK} ||= html_dir(); + +plan tests => repeat_each() * (blocks() * 3); + +run_tests(); + +__DATA__ + +=== TEST 1: request.get_forwarded_prefix() considers X-Forwarded-Prefix when trusted +--- http_config eval: $t::Util::HttpConfig +--- config + location /t { + access_by_lua_block { + local PDK = require "kong.pdk" + local pdk = PDK.new({ trusted_ips = { "0.0.0.0/0", "::/0" } }) + + ngx.say("prefix: ", pdk.request.get_forwarded_prefix()) + ngx.say("type: ", type(pdk.request.get_forwarded_prefix())) + } + } +--- request +GET /t/request-path +--- more_headers +X-Forwarded-Prefix: /trusted +--- response_body +prefix: /trusted +type: string +--- no_error_log +[error] + + + +=== TEST 2: request.get_forwarded_prefix() doesn't considers X-Forwarded-Prefix when not trusted +--- http_config eval: $t::Util::HttpConfig +--- config + location /t { + access_by_lua_block { + local PDK = require "kong.pdk" + local pdk = PDK.new() + + ngx.say("prefix: ", pdk.request.get_forwarded_prefix()) + } + } +--- request +GET /t/request-path +--- more_headers +X-Forwarded-Prefix: / +--- response_body +prefix: / +--- no_error_log +[error] + + + +=== TEST 3: request.get_forwarded_prefix() considers first X-Forwarded-Prefix if multiple when trusted +--- http_config eval: $t::Util::HttpConfig +--- config + location = /t { + access_by_lua_block { + local PDK = require "kong.pdk" + local pdk = PDK.new({ trusted_ips = { "0.0.0.0/0", "::/0" } }) + + ngx.say("prefix: ", pdk.request.get_forwarded_prefix()) + } + } +--- request +GET /t +--- more_headers +X-Forwarded-Prefix: /first +X-Forwarded-Prefix: /second +--- response_body +prefix: /first +--- no_error_log +[error] + + + +=== TEST 4: request.get_forwarded_prefix() doesn't considers any X-Forwarded-Prefix headers when not trusted +--- http_config eval: $t::Util::HttpConfig +--- config + location /t { + access_by_lua_block { + local PDK = require "kong.pdk" + local pdk = PDK.new() + + ngx.say("prefix: ", pdk.request.get_forwarded_prefix()) + } + } +--- request +GET /t/request-uri +--- more_headers +X-Forwarded-Prefix: /first +X-Forwarded-Prefix: /second +--- response_body +prefix: / +--- no_error_log +[error] + + + +=== TEST 5: request.get_forwarded_prefix() removes query and fragment when not trusted +--- http_config eval: $t::Util::HttpConfig +--- config + location /t { + access_by_lua_block { + local PDK = require "kong.pdk" + local pdk = PDK.new() + + ngx.say("prefix: ", pdk.request.get_forwarded_prefix()) + } + } +--- request +GET /t/request-uri?query&field=value#here +--- more_headers +X-Forwarded-Prefix: /first +--- response_body +prefix: / +--- no_error_log +[error] + + + +=== TEST 6: request.get_forwarded_prefix() does not remove query and fragment when trusted +--- http_config eval: $t::Util::HttpConfig +--- config + location /t { + access_by_lua_block { + local PDK = require "kong.pdk" + local pdk = PDK.new({ trusted_ips = { "0.0.0.0/0", "::/0" } }) + + ngx.say("prefix: ", pdk.request.get_forwarded_prefix()) + } + } +--- request +GET /t/request-uri?query&field=value#here +--- more_headers +X-Forwarded-Prefix: /first?query&field=value#here +--- response_body +prefix: /first?query&field=value#here +--- no_error_log +[error]