From 26920e4e464f6b77d908b129c928b78e92bc51e2 Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Wed, 12 Aug 2026 15:45:18 +0545 Subject: [PATCH 1/2] feat: resolve names through a resolver the host application installs The client dials through nginx's `resolver` directive and knows nothing else, so a host application that owns a resolver cannot make this client agree with the rest of its outbound traffic. In APISIX every cosocket goes through `core.resolver`, which reads /etc/hosts, the dns_resolver config and the search domains; this client saw none of it, so a name that works on every other client fails here. `set_resolver(fn)` installs that resolver once. Names go through it on both entry points before anything crosses into C, which is where the resolution has to happen: the resolver is a Lua module whose lookup yields, so C cannot call it synchronously. The substitution keeps the four invariants that matter: - The name is resolved before the pool key is derived from the address, so two names on two addresses keep separate pools. - The Host header keeps the name, with the port whenever C would have written one, and the SNI keeps it too, so a certificate is still judged against the name rather than the address. - An IP literal short-circuits, so the common case costs nothing. - A resolution failure comes back as a connect error. `resolver` on a single call overrides the installed one, and `false` opts that call out. --- README.md | 27 ++ lib/resty/ngx_http_ffi_client.lua | 154 +++++++++- t/016-resolver-hook.t | 465 ++++++++++++++++++++++++++++++ 3 files changed, 641 insertions(+), 5 deletions(-) create mode 100644 t/016-resolver-hook.t diff --git a/README.md b/README.md index a20b8b6..d88adc7 100644 --- a/README.md +++ b/README.md @@ -75,6 +75,33 @@ very large responses. - A request header may be given as an array too: each value is sent as its own header line, so a response table can be passed straight back. +### Name resolution + +C dials through nginx's `resolver` directive and knows nothing else, so a host +application that owns a resolver installs it here: + +```lua +client.set_resolver(function (host) + return ip, err -- nil plus an error surfaces as a connect error +end) +``` + +Every host that is not already an IP literal goes through it, on both entry +points, before anything crosses into C. The name is resolved first so the +address is what the connect and the keepalive pool key are built from: two +names on two addresses never share a pooled connection. What the peer sees +keeps the name: the `Host` header carries it, with the port whenever the client +would have written one, and the SNI is the name, so the certificate is still +judged against it. A caller-set `Host` or `ssl_server_name` still wins. + +`set_resolver(nil)` removes it, and `resolver = ` on a single +`request_uri` or `connect` overrides the installed one for that call. +`resolver = false` opts that call out and leaves the name to nginx. + +In APISIX this is one call at init with `core.resolver.parse_domain`, which is +what makes this client agree with every cosocket in the gateway about +`/etc/hosts`, `dns_resolver` and the search domains (#45). + ### Request validation The method and every header name must be an RFC 9110 `token`, a header value diff --git a/lib/resty/ngx_http_ffi_client.lua b/lib/resty/ngx_http_ffi_client.lua index 6c3963f..173f029 100644 --- a/lib/resty/ngx_http_ffi_client.lua +++ b/lib/resty/ngx_http_ffi_client.lua @@ -15,6 +15,8 @@ local pairs = pairs local setmetatable = setmetatable local string_lower = string.lower local string_gsub = string.gsub +local string_find = string.find +local string_match = string.match local table_concat = table.concat local ngx_encode_args = ngx.encode_args local co_yield = coroutine._yield @@ -169,6 +171,111 @@ local function set_str(dst, value) end +-- ===== name resolution ===== +-- +-- C dials through nginx's `resolver` directive and knows nothing else. A host +-- application that owns a resolver installs it here, and the name is resolved +-- on this side before anything crosses the FFI boundary: the address is what +-- the pool key and the connect are built from, while the Host header and the +-- SNI keep the name. + + +local resolver_hook + + +-- A hostname cannot hold a ':', so a colon means an IPv6 literal, bracketed or +-- not. The IPv4 literal is four decimal octets. +local function is_ip_literal(host) + if string_find(host, ":", 1, true) then + return true + end + + local a, b, c, d = string_match(host, "^(%d+)%.(%d+)%.(%d+)%.(%d+)$") + if not a then + return false + end + + return tonumber(a) < 256 and tonumber(b) < 256 + and tonumber(c) < 256 and tonumber(d) < 256 +end + + +-- The address to dial. The name comes back unchanged when no resolver is +-- installed or it is already an address. +local function resolve_host(host, override) + local resolver = override + + if resolver == nil then + resolver = resolver_hook + end + + if not resolver or is_ip_literal(host) then + return host + end + + if type(resolver) ~= "function" then + return nil, "resolver must be a function" + end + + local ip, err = resolver(host) + + if type(ip) ~= "string" or ip == "" then + return nil, host .. " could not be resolved (" + .. (err or "no address") .. ")" + end + + return ip +end + + +-- What C would have written from the name, so resolving changes the address +-- dialled and nothing the peer sees. +local function host_header_value(host, port) + if port == 80 then + return host + end + + return host .. ":" .. port +end + + +-- The caller's table is left alone: the copy carries the name C can no longer +-- derive from the address. A caller-set Host wins, as it does everywhere else. +local function with_host_header(headers, value) + local out = {} + + if headers ~= nil then + if type(headers) ~= "table" then + return headers + end + + for key, header_value in pairs(headers) do + if type(key) == "string" and string_lower(key) == "host" then + return headers + end + + out[key] = header_value + end + end + + out["Host"] = value + + return out +end + + +-- set_resolver(fn) installs the resolver every non-IP host goes through; +-- set_resolver(nil) removes it. fn(host) returns an address, or nil and an +-- error that surfaces as a connect error. +function _M.set_resolver(fn) + if fn ~= nil and type(fn) ~= "function" then + error("resolver must be a function or nil", 2) + end + + resolver_hook = fn +end + + -- Host, Connection and Content-Length reach C and feed the request prologue. -- Transfer-Encoding stays out: this client always frames the body with a -- Content-Length, and both on the wire is the request-smuggling shape (#29). @@ -343,6 +450,20 @@ function _M.request_uri(opts) return nil, "ssl_trusted_certificate must be a string" end + -- resolved after the SNI has taken the name and before the pool key is + -- derived from the address: the certificate is still judged against the + -- name, and two names on two addresses keep separate pools + local host, resolve_err = resolve_host(opts.host, opts.resolver) + if not host then + return nil, resolve_err + end + + local headers = opts.headers + if host ~= opts.host then + headers = with_host_header(headers, + host_header_value(opts.host, port)) + end + local method = opts.method or "GET" local path = opts.path or "/" local body = opts.body @@ -379,7 +500,7 @@ function _M.request_uri(opts) end local header_arr, header_count, header_refs, header_err = - build_headers(opts.headers) + build_headers(headers) if header_err then return nil, header_err @@ -389,7 +510,7 @@ function _M.request_uri(opts) local resp = resp_t() local refs = { set_str(req.scheme, scheme), - set_str(req.host, opts.host), + set_str(req.host, host), set_str(req.method, method), set_str(req.path, path), set_str(req.body, body), @@ -550,7 +671,7 @@ function client.connect(self, opts, port_arg) error("no request found", 2) end - local host, port, scheme, pool, pool_size + local host, port, scheme, pool, pool_size, resolver local ssl_verify, ssl_server_name, ssl_trusted_certificate if type(opts) == "table" then @@ -559,6 +680,7 @@ function client.connect(self, opts, port_arg) scheme = opts.scheme or "http" pool = opts.pool pool_size = opts.pool_size + resolver = opts.resolver ssl_verify = opts.ssl_verify ssl_server_name = opts.ssl_server_name ssl_trusted_certificate = opts.ssl_trusted_certificate @@ -615,6 +737,23 @@ function client.connect(self, opts, port_arg) end end + -- the name is what the SNI and the Host header keep; the address is what + -- the pool key and the connect are built from + local name = host + local resolve_err + + host, resolve_err = resolve_host(name, resolver) + if not host then + return nil, resolve_err + end + + if host ~= name then + self._host_header = host_header_value(name, port) + + else + self._host_header = nil + end + if self._op == nil then local op = c_new(r) if op == nil then @@ -632,7 +771,7 @@ function client.connect(self, opts, port_arg) set_str(cp.scheme, scheme), set_str(cp.host, host), set_str(cp.keepalive_pool, pool), - set_str(cp.ssl_server_name, ssl and (ssl_server_name or host) or nil), + set_str(cp.ssl_server_name, ssl and (ssl_server_name or name) or nil), set_str(cp.ssl_trusted_certificate, ssl and ssl_trusted_certificate or nil), } @@ -807,8 +946,13 @@ function client.request(self, params) body = tostring(body) end + local headers = params.headers + if self._host_header then + headers = with_host_header(headers, self._host_header) + end + local header_arr, header_count, header_refs, header_err = - build_headers(params.headers) + build_headers(headers) if header_err then return nil, header_err diff --git a/t/016-resolver-hook.t b/t/016-resolver-hook.t new file mode 100644 index 0000000..0c1f3c4 --- /dev/null +++ b/t/016-resolver-hook.t @@ -0,0 +1,465 @@ +# The host application's resolver. None of these blocks configure nginx's +# `resolver`, so a name that reaches the wire at all proves the hook resolved +# it, and the Host header and the SNI prove the name survived the substitution. + +use Test::Nginx::Socket -Base; + +repeat_each(1); +plan tests => repeat_each() * (blocks() * 3); + +our $HttpConfig = qq{ + lua_package_path "$ENV{TEST_NGINX_LUA_PACKAGE_PATH};;"; +}; + +our $TlsConfig = qq{ + lua_package_path "$ENV{TEST_NGINX_LUA_PACKAGE_PATH};;"; + + server { + listen 127.0.0.1:12899 ssl; + ssl_certificate \$TEST_NGINX_SERVER_ROOT/html/tls.crt; + ssl_certificate_key \$TEST_NGINX_SERVER_ROOT/html/tls.key; + + location /sni { + content_by_lua_block { + local body = (ngx.var.ssl_server_name or "none") .. "\\n" + ngx.header["Content-Length"] = #body + ngx.print(body) + } + } + } +}; + +our $UserFiles = <<'_EOC_'; +>>> tls.crt +-----BEGIN CERTIFICATE----- +MIIDKjCCAhKgAwIBAgIUMpoLE+76X2UdJn+TC1k80Qr6O0MwDQYJKoZIhvcNAQEL +BQAwFTETMBEGA1UEAwwKdGVzdC5sb2NhbDAgFw0yNjA3MTUwODU2MDRaGA8yMTI2 +MDYyMTA4NTYwNFowFTETMBEGA1UEAwwKdGVzdC5sb2NhbDCCASIwDQYJKoZIhvcN +AQEBBQADggEPADCCAQoCggEBAMWo0WdpxTqswzXDnhsRpqby32slAR4BmzjXECvo +LHK1Oza+/8SCAo9r61HXl5fyy6cCll75MzEmZkLJy8vwmvCh5qTBBDt68eq6RbGH +FRlCNhjup2KvsXoUWfmOQC72PMYKg/faY8RoV40P9x/W57JvT5WSCf6O3/anre8g +ywokpH3QokPRdIEg3OE+OTAMcDV43mISXjWJWAtpJU71T/i0Iyxy5IBU01qXAjEe +WIy8fQLzPoK4WqiHH6jII9bB6ebDrWl5SW2CmrStb5lnMuf8OqXXomXBzTcfC3cu +CAREN2a3uZiAPryG4paTBD/3+dmyLDmjEmeFV2CQCSJ9bQkCAwEAAaNwMG4wHQYD +VR0OBBYEFIZZdTC230/zMtPUjW1CpizXxWGGMB8GA1UdIwQYMBaAFIZZdTC230/z +MtPUjW1CpizXxWGGMA8GA1UdEwEB/wQFMAMBAf8wGwYDVR0RBBQwEoIKdGVzdC5s +b2NhbIcEfwAAATANBgkqhkiG9w0BAQsFAAOCAQEAYv+q+iaGoJG1qStGFk/Ojhpa +aLXti7QV53aCaUpNbiKiCxtIC1ZqpaQAW+G5eM2dWC+7TCrqcMJuJjjp5c9mwolR +WHMqinfNUe/LCa3DsJvqYEkr1Dmy+iPabfP6Ni3UnWMehehTvX86jd4GjR7mTj6/ +PC2D6x5gy+SFQSh+OMvtMkmYM91s21UIds5z9PhXDVzolH3kP3zaqtu646F1btIK +g5sdzjzunwEd5Av6hzaqYNu/ysbz6NiVcTOj8cA3RaktwvgBmWE2mRim2BO5bAXf +JE9Vloa+pp9bLzcwlTaU2zIZb2SadrMbNaNdh8lJ/iX5H3pSaNODNTThmw6BWA== +-----END CERTIFICATE----- +>>> tls.key +-----BEGIN PRIVATE KEY----- +MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQDFqNFnacU6rMM1 +w54bEaam8t9rJQEeAZs41xAr6CxytTs2vv/EggKPa+tR15eX8sunApZe+TMxJmZC +ycvL8JrwoeakwQQ7evHqukWxhxUZQjYY7qdir7F6FFn5jkAu9jzGCoP32mPEaFeN +D/cf1ueyb0+Vkgn+jt/2p63vIMsKJKR90KJD0XSBINzhPjkwDHA1eN5iEl41iVgL +aSVO9U/4tCMscuSAVNNalwIxHliMvH0C8z6CuFqohx+oyCPWwenmw61peUltgpq0 +rW+ZZzLn/Dql16Jlwc03Hwt3LggERDdmt7mYgD68huKWkwQ/9/nZsiw5oxJnhVdg +kAkifW0JAgMBAAECggEAIAgKG1ygKjCKGAHh8tgK7j4op6fhBPhUq8Lqa3seDN7C +wE32i+VXvd9KzMIH3odpqmB4dt6ihaIH62XhYWTV7w4Fnwhqg6saXiQenDTcXfIF +a0ftl0gKllKK/C6pxxJ/acaVeUqKZW9VVNZUAXRlqtxwBLicZwTHVaT5wmlJjhR2 +LA0+zqup5J4SxDAk9+p1SgykxhEPP/JkAyAtD39jeQ8uJWx/Rnq3SDLseGUs8qUM +sgHsa5PSHxEmnuM7/LOkO3v6EbkAQsy8hcRdBrXt5DUr1LSk7cmXQlODkQ1klpIU +lAbd+snLarh4+bHzbCfY2PoYy/uqYx7xmBc+FHsE8wKBgQDjj0d3b7KYX2jJWkuT +5o2HD9koRSTrhvuM9/fsYCfJ24QDkUF45OlSd61z7Q9BcfSp8KLuIVbnFOSP4Gim +ZZqOQu0wZffaBWVeXwBpCWNXO/8xuufPXIGaezH4IXZN/uIZWGY5PHHPgACLYSnf +6RXoTTMRCmyWwLip/8l1fCZYQwKBgQDeXOMtHttRaqXCgZY2vkJFd2MqYx+TzREK +1Nzna3vSrsMqvWrwxaONQQK6hphtrfxaB8DPhMWl+0bsrgrjB/7aLDDj7BxUeZPt +/trXRKE+rVMfXKAewvM1NO9to91pDjOOamkG7ih8gWfR4jE6qE0ONvCFwoDS6hQq +XsRUuFTmwwKBgQC4y7Eoz/+D9+8bnQVFLXR/WyJproUF888yMmkWfxuwtGBnmT1H +FPZZbzDftIKwDf+3ReC6az6sV+4o3P9/KYGyx6zgod3+ImWolpO5uNMAk4tw8iyv +25qwPh1dOKdfPX6VQJF7J5fw/yzyA0zDNgEBbjfrPcDjR8xu2Xbbvp9RCwKBgFG+ ++jFTP7N9rnSEKVH0ve5FxqoFiM1QPSyrNo7JH9tDLjKfMhpTvh2mwbcK1iy0IqqC +YSqpF/Q+HUPTc+MkxFc2mb6gxYV0sKJ058Tt0Q12sLE93wuQBdMQo9i9vh7p/qAj +lHrcwPuMozswmYKD7tgD8IZsC+n97e3pqumuXl/7AoGAV88zb7UorfzYmilI12dR +dMEmcapTrGz0WTFrAnkMPOMo4haBswySqePfrG5oC/xa77Dw6/LOD8b1pmupeKvx +YrErqu4suZWhAa1m2pblIxAM3CA3l5ZZR8rF4w0Iau1OoUHDEATOTYETiitKuvFp +Ye/gbwb0N+OIZRCBROm7A70= +-----END PRIVATE KEY----- +_EOC_ + +no_long_string(); +run_tests(); + +__DATA__ + +=== TEST 1: the installed resolver answers for the one shot +--- http_config eval: $::HttpConfig +--- user_files eval: $::UserFiles +--- config + location /echo { + content_by_lua_block { + local body = ngx.var.http_host .. "\n" + ngx.header["Content-Length"] = #body + ngx.print(body) + } + } + + location /t { + content_by_lua_block { + local client = require "resty.ngx_http_ffi_client" + client.set_resolver(function (host) + return "127.0.0.1" + end) + + local res, err = client.request_uri({ + host = "test.local", + port = ngx.var.server_port, + path = "/echo", + timeout = 1000, + }) + + if not res then + ngx.say(err) + return + end + + -- the address was dialled, the name stayed in the Host header + ngx.print(res.body == "test.local:" .. ngx.var.server_port .. "\n") + } + } +--- request +GET /t +--- response_body chomp +true +--- no_error_log +[error] + + + +=== TEST 2: an IP literal never reaches the resolver +--- http_config eval: $::HttpConfig +--- user_files eval: $::UserFiles +--- config + location /echo { + content_by_lua_block { + local body = ngx.var.http_host .. "\n" + ngx.header["Content-Length"] = #body + ngx.print(body) + } + } + + location /t { + content_by_lua_block { + local client = require "resty.ngx_http_ffi_client" + local calls = 0 + + client.set_resolver(function (host) + calls = calls + 1 + return "127.0.0.1" + end) + + local res, err = client.request_uri({ + host = "127.0.0.1", + port = ngx.var.server_port, + path = "/echo", + timeout = 1000, + }) + + if not res then + ngx.say(err) + return + end + + ngx.say("calls: ", calls) + ngx.print(res.body == "127.0.0.1:" .. ngx.var.server_port .. "\n") + } + } +--- request +GET /t +--- response_body chomp +calls: 0 +true +--- no_error_log +[error] + + + +=== TEST 3: a resolver failure is a connect error +--- http_config eval: $::HttpConfig +--- user_files eval: $::UserFiles +--- config + location /t { + content_by_lua_block { + local client = require "resty.ngx_http_ffi_client" + client.set_resolver(function (host) + return nil, "no answer" + end) + + local res, err = client.request_uri({ + host = "test.local", + port = ngx.var.server_port, + path = "/echo", + timeout = 1000, + }) + + ngx.say(res == nil) + ngx.say(err) + } + } +--- request +GET /t +--- response_body +true +test.local could not be resolved (no answer) +--- no_error_log +[error] + + + +=== TEST 4: a caller Host still wins over the resolved name +--- http_config eval: $::HttpConfig +--- user_files eval: $::UserFiles +--- config + location /echo { + content_by_lua_block { + local body = ngx.var.http_host .. "\n" + ngx.header["Content-Length"] = #body + ngx.print(body) + } + } + + location /t { + content_by_lua_block { + local client = require "resty.ngx_http_ffi_client" + client.set_resolver(function (host) + return "127.0.0.1" + end) + + local res, err = client.request_uri({ + host = "test.local", + port = ngx.var.server_port, + path = "/echo", + headers = { ["Host"] = "example.test" }, + timeout = 1000, + }) + + if not res then + ngx.say(err) + return + end + + ngx.print(res.body) + } + } +--- request +GET /t +--- response_body +example.test +--- no_error_log +[error] + + + +=== TEST 5: resolver = false on the call leaves the name to nginx +--- http_config eval: $::HttpConfig +--- user_files eval: $::UserFiles +--- config + location /t { + content_by_lua_block { + local client = require "resty.ngx_http_ffi_client" + client.set_resolver(function (host) + return "127.0.0.1" + end) + + local res, err = client.request_uri({ + host = "test.local", + port = ngx.var.server_port, + path = "/echo", + resolver = false, + timeout = 1000, + }) + + ngx.say(res == nil) + ngx.say(err) + } + } +--- request +GET /t +--- response_body +true +no resolver defined to resolve "test.local" +--- no_error_log +[error] + + + +=== TEST 6: the stateful connect resolves and keeps the name +--- http_config eval: $::HttpConfig +--- user_files eval: $::UserFiles +--- config + location /echo { + content_by_lua_block { + local body = ngx.var.http_host .. "\n" + ngx.header["Content-Length"] = #body + ngx.print(body) + } + } + + location /t { + content_by_lua_block { + local client = require "resty.ngx_http_ffi_client" + client.set_resolver(function (host) + return "127.0.0.1" + end) + + local httpc = client.new() + httpc:set_timeout(1000) + + assert(httpc:connect({ + host = "test.local", + port = ngx.var.server_port, + })) + + local res = assert(httpc:request({ path = "/echo" })) + local body = assert(res:read_body()) + httpc:close() + + ngx.print(body == "test.local:" .. ngx.var.server_port .. "\n") + } + } +--- request +GET /t +--- response_body chomp +true +--- no_error_log +[error] + + + +=== TEST 7: the SNI keeps the name, so verification is against the name +--- http_config eval: $::TlsConfig +--- user_files eval: $::UserFiles +--- config + location /t { + content_by_lua_block { + local client = require "resty.ngx_http_ffi_client" + client.set_resolver(function (host) + return "127.0.0.1" + end) + + local res, err = client.request_uri({ + scheme = "https", + host = "test.local", + port = 12899, + path = "/sni", + ssl_verify = true, + ssl_trusted_certificate = + "$TEST_NGINX_SERVER_ROOT/html/tls.crt", + timeout = 2000, + }) + + if not res then + ngx.say(err) + return + end + + ngx.print(res.body) + } + } +--- request +GET /t +--- response_body +test.local +--- no_error_log +[error] + + + +=== TEST 8: two names on two addresses do not share a pooled connection +--- http_config eval: $::HttpConfig +--- user_files eval: $::UserFiles +--- config + location /echo { + content_by_lua_block { + local body = "hello\n" + ngx.header["Content-Length"] = #body + ngx.print(body) + } + } + + location /t { + content_by_lua_block { + local client = require "resty.ngx_http_ffi_client" + local addresses = { + ["one.test"] = "127.0.0.1", + ["two.test"] = "127.0.0.2", + ["also-one.test"] = "127.0.0.1", + } + + client.set_resolver(function (host) + return addresses[host] + end) + + local function call(host) + local httpc = client.new() + httpc:set_timeout(1000) + + assert(httpc:connect({ + host = host, + port = ngx.var.server_port, + pool_size = 4, + })) + + local reused = httpc:get_reused_times() + local res = assert(httpc:request({ path = "/echo" })) + assert(res:read_body()) + assert(httpc:set_keepalive()) + + return reused + end + + call("one.test") + -- a different address, so a pool of its own + ngx.say("two.test: ", call("two.test")) + -- the same address, so the pooled connection comes back + ngx.say("also-one.test: ", call("also-one.test")) + } + } +--- request +GET /t +--- response_body +two.test: 0 +also-one.test: 1 +--- no_error_log +[error] + + + +=== TEST 9: set_resolver refuses anything but a function or nil +--- http_config eval: $::HttpConfig +--- user_files eval: $::UserFiles +--- config + location /t { + content_by_lua_block { + local client = require "resty.ngx_http_ffi_client" + + local ok, err = pcall(client.set_resolver, "resolver") + ngx.say(ok) + ngx.say(err:match("resolver must be a function or nil")) + + -- nil removes it: the name is nginx's to resolve again + client.set_resolver(function (host) return "127.0.0.1" end) + client.set_resolver(nil) + + local res, connect_err = client.request_uri({ + host = "test.local", + port = ngx.var.server_port, + path = "/echo", + timeout = 1000, + }) + + ngx.say(connect_err) + } + } +--- request +GET /t +--- response_body +false +resolver must be a function or nil +no resolver defined to resolve "test.local" +--- no_error_log +[error] From 1256f8e2026013985e419d63a6ebaa62a16e7e35 Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Thu, 13 Aug 2026 14:09:23 +0545 Subject: [PATCH 2/2] fix: hold the resolved name to the host rule and to the live connection Three follow-ups from review. A resolver made the name skip the host validation C does. The name lands in a Host header, and a header value legally holds a space, so a host that used to come back as `invalid host` reached the peer as a malformed Host and drew a 400. CR and LF were never injectable, since a header value refuses every byte below 0x20, but the contract slipped all the same: the name is now held to the same VCHAR rule C applies before the resolver is asked about it. `connect` on a still-connected object is refused by C with the first connection intact, so the default Host is now committed only once the connect is accepted. It named the rejected destination before, and a request on the surviving connection carried it. The per-call resolver override had no test to hold it, only the opt-out. --- lib/resty/ngx_http_ffi_client.lua | 18 +- t/016-resolver-hook.t | 330 ++++++++++++++++++++++++++++++ 2 files changed, 344 insertions(+), 4 deletions(-) diff --git a/lib/resty/ngx_http_ffi_client.lua b/lib/resty/ngx_http_ffi_client.lua index 173f029..a33171b 100644 --- a/lib/resty/ngx_http_ffi_client.lua +++ b/lib/resty/ngx_http_ffi_client.lua @@ -217,6 +217,14 @@ local function resolve_host(host, override) return nil, "resolver must be a function" end + -- C judges the host it is given, which after this is the address, so the + -- name is held to the same rule here: what C calls a VCHAR string, no + -- space, no control character. A resolver never launders an invalid host + -- into a Host header, where a space would be legal. + if string_find(host, "[%z\1-\32\127]") then + return nil, "invalid host" + end + local ip, err = resolver(host) if type(ip) ~= "string" or ip == "" then @@ -747,11 +755,12 @@ function client.connect(self, opts, port_arg) return nil, resolve_err end + -- held until the connect is accepted: a second connect on a live object is + -- refused with the first connection intact, and that connection keeps the + -- Host it was opened with + local host_header if host ~= name then - self._host_header = host_header_value(name, port) - - else - self._host_header = nil + host_header = host_header_value(name, port) end if self._op == nil then @@ -801,6 +810,7 @@ function client.connect(self, opts, port_arg) end self._connected = true + self._host_header = host_header return 1 end diff --git a/t/016-resolver-hook.t b/t/016-resolver-hook.t index 0c1f3c4..700c7c9 100644 --- a/t/016-resolver-hook.t +++ b/t/016-resolver-hook.t @@ -11,6 +11,21 @@ our $HttpConfig = qq{ lua_package_path "$ENV{TEST_NGINX_LUA_PACKAGE_PATH};;"; }; +our $InitConfig = qq{ + lua_package_path "$ENV{TEST_NGINX_LUA_PACKAGE_PATH};;"; + + init_by_lua_block { + local client = require "resty.ngx_http_ffi_client" + + -- installed once at init, as a host application would, and yielding + -- the way a Lua DNS client does + client.set_resolver(function (host) + ngx.sleep(0.001) + return "127.0.0.1" + end) + } +}; + our $TlsConfig = qq{ lua_package_path "$ENV{TEST_NGINX_LUA_PACKAGE_PATH};;"; @@ -463,3 +478,318 @@ resolver must be a function or nil no resolver defined to resolve "test.local" --- no_error_log [error] + + + +=== TEST 10: a resolver installed at init reaches the worker +--- http_config eval: $::InitConfig +--- user_files eval: $::UserFiles +--- config + location /echo { + content_by_lua_block { + local body = ngx.var.http_host .. "\n" + ngx.header["Content-Length"] = #body + ngx.print(body) + } + } + + location /t { + content_by_lua_block { + local client = require "resty.ngx_http_ffi_client" + local res, err = client.request_uri({ + host = "test.local", + port = ngx.var.server_port, + path = "/echo", + timeout = 1000, + }) + + if not res then + ngx.say(err) + return + end + + ngx.print(res.body == "test.local:" .. ngx.var.server_port .. "\n") + } + } +--- request +GET /t +--- response_body chomp +true +--- no_error_log +[error] + + + +=== TEST 11: the stateful object takes the same resolver, yields and all +--- http_config eval: $::InitConfig +--- user_files eval: $::UserFiles +--- config + location /echo { + content_by_lua_block { + local body = ngx.var.http_host .. "\n" + ngx.header["Content-Length"] = #body + ngx.print(body) + } + } + + location /t { + content_by_lua_block { + local client = require "resty.ngx_http_ffi_client" + local httpc = client.new() + httpc:set_timeout(1000) + + assert(httpc:connect({ + host = "test.local", + port = ngx.var.server_port, + })) + + local res = assert(httpc:request({ path = "/echo" })) + local body = assert(res:read_body()) + httpc:close() + + ngx.print(body == "test.local:" .. ngx.var.server_port .. "\n") + } + } +--- request +GET /t +--- response_body chomp +true +--- no_error_log +[error] + + + +=== TEST 12: an address the resolver answers with may be IPv6 +--- http_config eval: $::HttpConfig +--- user_files eval: $::UserFiles +--- config + listen [::1]:$TEST_NGINX_SERVER_PORT; + + location /echo { + content_by_lua_block { + local body = ngx.var.http_host .. "\n" + ngx.header["Content-Length"] = #body + ngx.print(body) + } + } + + location /t { + content_by_lua_block { + local client = require "resty.ngx_http_ffi_client" + client.set_resolver(function (host) + return "::1" + end) + + local res, err = client.request_uri({ + host = "test.local", + port = ngx.var.server_port, + path = "/echo", + timeout = 1000, + }) + + if not res then + ngx.say(err) + return + end + + ngx.print(res.body == "test.local:" .. ngx.var.server_port .. "\n") + } + } +--- request +GET /t +--- response_body chomp +true +--- no_error_log +[error] + + + +=== TEST 13: a name the resolver would answer for is still held to the host rule +--- http_config eval: $::HttpConfig +--- user_files eval: $::UserFiles +--- config + location /t { + content_by_lua_block { + local client = require "resty.ngx_http_ffi_client" + client.set_resolver(function (host) + return "127.0.0.1" + end) + + -- a space is legal in a header value, so a resolved name must be + -- refused here rather than reaching the peer as a malformed Host + local res, err = client.request_uri({ + host = "127.0.0.1 evil", + port = ngx.var.server_port, + path = "/", + timeout = 1000, + }) + + ngx.say(res and ("unexpected " .. res.status) or err) + + local httpc = client.new() + httpc:set_timeout(1000) + + local ok, connect_err = httpc:connect({ + host = "127.0.0.1 evil", + port = ngx.var.server_port, + }) + + ngx.say(ok == nil) + ngx.say(connect_err) + } + } +--- request +GET /t +--- response_body +invalid host +true +invalid host +--- no_error_log +[error] + + + +=== TEST 14: a CRLF name never reaches the wire +--- http_config eval: $::HttpConfig +--- user_files eval: $::UserFiles +--- config + location /t { + content_by_lua_block { + local client = require "resty.ngx_http_ffi_client" + client.set_resolver(function (host) + return "127.0.0.1" + end) + + for _, host in ipairs({ "evil\r\nX-Injected: 1", + "evil\r\nX-Injected 1" }) do + local res, err = client.request_uri({ + host = host, + port = ngx.var.server_port, + path = "/", + timeout = 1000, + }) + + ngx.say(res and ("unexpected " .. res.status) or err) + end + } + } +--- request +GET /t +--- response_body +invalid host +invalid host +--- no_error_log +[error] + + + +=== TEST 15: a per-call resolver overrides the installed one +--- http_config eval: $::HttpConfig +--- user_files eval: $::UserFiles +--- config + listen 127.0.0.2:$TEST_NGINX_SERVER_PORT; + + location /echo { + content_by_lua_block { + local body = ngx.var.server_addr .. " " .. ngx.var.http_host .. "\n" + ngx.header["Content-Length"] = #body + ngx.print(body) + } + } + + location /t { + content_by_lua_block { + local client = require "resty.ngx_http_ffi_client" + client.set_resolver(function (host) + return "127.0.0.1" + end) + + local function per_call(host) + return "127.0.0.2" + end + + local res = assert(client.request_uri({ + host = "test.local", + port = ngx.var.server_port, + path = "/echo", + resolver = per_call, + timeout = 1000, + })) + + local httpc = client.new() + httpc:set_timeout(1000) + + assert(httpc:connect({ + host = "test.local", + port = ngx.var.server_port, + resolver = per_call, + })) + + local stateful = assert(httpc:request({ path = "/echo" })) + local body = assert(stateful:read_body()) + httpc:close() + + -- the per-call address was dialled on both entry points + local want = "127.0.0.2 test.local:" .. ngx.var.server_port .. "\n" + ngx.say(res.body == want) + ngx.print(body == want) + } + } +--- request +GET /t +--- response_body chomp +true +true +--- no_error_log +[error] + + + +=== TEST 16: a refused reconnect leaves the live connection its own Host +--- http_config eval: $::HttpConfig +--- user_files eval: $::UserFiles +--- config + location /echo { + content_by_lua_block { + local body = ngx.var.http_host .. "\n" + ngx.header["Content-Length"] = #body + ngx.print(body) + } + } + + location /t { + content_by_lua_block { + local client = require "resty.ngx_http_ffi_client" + client.set_resolver(function (host) + return "127.0.0.1" + end) + + local httpc = client.new() + httpc:set_timeout(1000) + + assert(httpc:connect({ + host = "127.0.0.1", + port = ngx.var.server_port, + })) + + -- refused, and the connection above is still the live one + local ok, err = httpc:connect({ + host = "test.local", + port = ngx.var.server_port, + }) + ngx.say(ok == nil, " ", err) + + local res = assert(httpc:request({ path = "/echo" })) + local body = assert(res:read_body()) + httpc:close() + + ngx.print(body == "127.0.0.1:" .. ngx.var.server_port .. "\n") + } + } +--- request +GET /t +--- response_body chomp +true already connected +true +--- no_error_log +[error]