diff --git a/changelog/unreleased/kong/subsystems_do_not_share_router_schemas.yml b/changelog/unreleased/kong/subsystems_do_not_share_router_schemas.yml new file mode 100644 index 00000000000..07a40e62f25 --- /dev/null +++ b/changelog/unreleased/kong/subsystems_do_not_share_router_schemas.yml @@ -0,0 +1,6 @@ +message: | + Expressions route in `http` and `stream` subsystem now have stricter validation. + Previously they share the same validation schema which means admin can configure expressions + route using fields like `http.path` even for stream routes. This is no longer allowed. +type: bugfix +scope: Core diff --git a/kong/db/schema/entities/routes.lua b/kong/db/schema/entities/routes.lua index 5c98e3931b3..0ff3943ddce 100644 --- a/kong/db/schema/entities/routes.lua +++ b/kong/db/schema/entities/routes.lua @@ -3,30 +3,22 @@ local router = require("resty.router.router") local deprecation = require("kong.deprecation") local validate_route -local has_paths do - local isempty = require("table.isempty") - local CACHED_SCHEMA = require("kong.router.atc").schema + local get_schema = require("kong.router.atc").schema local get_expression = require("kong.router.compat").get_expression - local type = type - -- works with both `traditional_compatiable` and `expressions` routes` validate_route = function(entity) + local schema = get_schema(entity.protocols) local exp = entity.expression or get_expression(entity) - local ok, err = router.validate(CACHED_SCHEMA, exp) + local ok, err = router.validate(schema, exp) if not ok then return nil, "Router Expression failed validation: " .. err end return true end - - has_paths = function(entity) - local paths = entity.paths - return type(paths) == "table" and not isempty(paths) - end end local kong_router_flavor = kong and kong.configuration and kong.configuration.router_flavor @@ -73,15 +65,8 @@ if kong_router_flavor == "expressions" then entity_checks = { { custom_entity_check = { - field_sources = { "expression", "id", }, - fn = function(entity) - local ok, err = validate_route(entity) - if not ok then - return nil, err - end - - return true - end, + field_sources = { "expression", "id", "protocols", }, + fn = validate_route, } }, }, } @@ -126,17 +111,7 @@ else table.insert(entity_checks, { custom_entity_check = { run_with_missing_fields = true, - field_sources = { "id", "paths", }, - fn = function(entity) - if has_paths(entity) then - local ok, err = validate_route(entity) - if not ok then - return nil, err - end - end - - return true - end, + fn = validate_route, }} ) end diff --git a/kong/router/atc.lua b/kong/router/atc.lua index 17f9f48752b..df8b7c636ce 100644 --- a/kong/router/atc.lua +++ b/kong/router/atc.lua @@ -55,8 +55,10 @@ local values_buf = buffer.new(64) local CACHED_SCHEMA +local HTTP_SCHEMA +local STREAM_SCHEMA do - local FIELDS = { + local HTTP_FIELDS = { ["String"] = {"net.protocol", "tls.sni", "http.method", "http.host", @@ -66,21 +68,39 @@ do }, ["Int"] = {"net.port", - "net.src.port", "net.dst.port", + }, + } + + local STREAM_FIELDS = { + + ["String"] = {"net.protocol", "tls.sni", + }, + + ["Int"] = {"net.src.port", "net.dst.port", }, ["IpAddr"] = {"net.src.ip", "net.dst.ip", }, } - CACHED_SCHEMA = schema.new() + local function generate_schema(fields) + local s = schema.new() - for typ, fields in pairs(FIELDS) do - for _, v in ipairs(fields) do - assert(CACHED_SCHEMA:add_field(v, typ)) + for t, f in pairs(fields) do + for _, v in ipairs(f) do + assert(s:add_field(v, t)) + end end + + return s end + -- used by validation + HTTP_SCHEMA = generate_schema(HTTP_FIELDS) + STREAM_SCHEMA = generate_schema(STREAM_FIELDS) + + -- used by running router + CACHED_SCHEMA = is_http and HTTP_SCHEMA or STREAM_SCHEMA end @@ -871,7 +891,26 @@ function _M._set_ngx(mock_ngx) end -_M.schema = CACHED_SCHEMA +do + local protocol_to_schema = { + http = HTTP_SCHEMA, + https = HTTP_SCHEMA, + grpc = HTTP_SCHEMA, + grpcs = HTTP_SCHEMA, + + tcp = STREAM_SCHEMA, + udp = STREAM_SCHEMA, + tls = STREAM_SCHEMA, + + tls_passthrough = STREAM_SCHEMA, + } + + -- for db schema validation + function _M.schema(protocols) + return assert(protocol_to_schema[protocols[1]]) + end +end + _M.LOGICAL_OR = LOGICAL_OR _M.LOGICAL_AND = LOGICAL_AND diff --git a/spec/01-unit/01-db/01-schema/06-routes_spec.lua b/spec/01-unit/01-db/01-schema/06-routes_spec.lua index 7146043dbdb..f4ef090ce0f 100644 --- a/spec/01-unit/01-db/01-schema/06-routes_spec.lua +++ b/spec/01-unit/01-db/01-schema/06-routes_spec.lua @@ -1329,7 +1329,7 @@ describe("routes schema (flavor = traditional_compatible)", function() reload_flavor("traditional_compatible") setup_global_env() - it("validates a valid route", function() + it("validates a valid http route", function() local route = { id = a_valid_uuid, name = "my_route", @@ -1351,6 +1351,21 @@ describe("routes schema (flavor = traditional_compatible)", function() assert.falsy(route.strip_path) end) + it("validates a valid stream route", function() + local route = { + id = a_valid_uuid, + name = "my_route", + protocols = { "tcp" }, + sources = { { ip = "1.2.3.4", port = 80 } }, + service = { id = another_uuid }, + } + route = Routes:process_auto_fields(route, "insert") + assert.truthy(route.created_at) + assert.truthy(route.updated_at) + assert.same(route.created_at, route.updated_at) + assert.truthy(Routes:validate(route)) + end) + it("fails when path is invalid", function() local route = { id = a_valid_uuid, @@ -1370,6 +1385,23 @@ describe("routes schema (flavor = traditional_compatible)", function() assert.falsy(errs["@entity"]) end) + it("fails when ip address is invalid", function() + local route = { + id = a_valid_uuid, + name = "my_route", + protocols = { "tcp" }, + sources = { { ip = "x.x.x.x", port = 80 } }, + service = { id = another_uuid }, + } + route = Routes:process_auto_fields(route, "insert") + local ok, errs = Routes:validate_insert(route) + assert.falsy(ok) + assert.truthy(errs["sources"]) + + -- verified by `schema/typedefs.lua` + assert.falsy(errs["@entity"]) + end) + it("won't fail when rust.regex update to 1.8", function() local route = { id = a_valid_uuid, @@ -1384,3 +1416,98 @@ describe("routes schema (flavor = traditional_compatible)", function() assert.is_nil(errs) end) end) + + +describe("routes schema (flavor = expressions)", function() + local a_valid_uuid = "cbb297c0-a956-486d-ad1d-f9b42df9465a" + local another_uuid = "64a8670b-900f-44e7-a900-6ec7ef5aa4d3" + + reload_flavor("expressions") + setup_global_env() + + it("validates a valid http route", function() + local route = { + id = a_valid_uuid, + name = "my_route", + protocols = { "http" }, + expression = [[http.method == "GET" && http.host == "example.com" && http.path == "/ovo"]], + priority = 100, + strip_path = false, + preserve_host = true, + service = { id = another_uuid }, + } + route = Routes:process_auto_fields(route, "insert") + assert.truthy(route.created_at) + assert.truthy(route.updated_at) + assert.same(route.created_at, route.updated_at) + assert.truthy(Routes:validate(route)) + assert.falsy(route.strip_path) + end) + + it("validates a valid stream route", function() + local route = { + id = a_valid_uuid, + name = "my_route", + protocols = { "tcp" }, + expression = [[net.src.ip == 1.2.3.4 && net.src.port == 80]], + priority = 100, + service = { id = another_uuid }, + } + route = Routes:process_auto_fields(route, "insert") + assert.truthy(route.created_at) + assert.truthy(route.updated_at) + assert.same(route.created_at, route.updated_at) + assert.truthy(Routes:validate(route)) + end) + + it("fails when path is invalid", function() + local route = { + id = a_valid_uuid, + name = "my_route", + protocols = { "http" }, + expression = [[http.method == "GET" && http.path ~ "/[abc/*/user$"]], + priority = 100, + service = { id = another_uuid }, + } + route = Routes:process_auto_fields(route, "insert") + local ok, errs = Routes:validate_insert(route) + assert.falsy(ok) + + -- verified by `schema/typedefs.lua` + assert.truthy(errs["@entity"]) + end) + + it("fails when ip address is invalid", function() + local route = { + id = a_valid_uuid, + name = "my_route", + protocols = { "tcp" }, + expression = [[net.src.ip in 1.2.3.4/16 && net.src.port == 80]], + priority = 100, + service = { id = another_uuid }, + } + route = Routes:process_auto_fields(route, "insert") + local ok, errs = Routes:validate_insert(route) + assert.falsy(ok) + + -- verified by `schema/typedefs.lua` + assert.truthy(errs["@entity"]) + end) + + it("fails if http route's field appears in stream route", function() + local route = { + id = a_valid_uuid, + name = "my_route", + protocols = { "tcp" }, + expression = [[http.method == "GET" && net.src.ip == 1.2.3.4 && net.src.port == 80]], + priority = 100, + service = { id = another_uuid }, + } + route = Routes:process_auto_fields(route, "insert") + local ok, errs = Routes:validate_insert(route) + assert.falsy(ok) + + -- verified by `schema/typedefs.lua` + assert.truthy(errs["@entity"]) + end) +end)