From 94a1866151ba3d32b90e5dbb61ddb8c49c7cd5f6 Mon Sep 17 00:00:00 2001 From: An Tran Date: Tue, 23 Jan 2024 22:16:53 +1000 Subject: [PATCH 1/4] Update lua-resty-http to 0.17.1 --- Dockerfile | 2 +- gateway/Roverfile.lock | 2 +- gateway/apicast-scm-1.rockspec | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Dockerfile b/Dockerfile index 6477ee198..5b16d0535 100644 --- a/Dockerfile +++ b/Dockerfile @@ -44,7 +44,7 @@ ENV PATH="./lua_modules/bin:/usr/local/openresty/luajit/bin/:${PATH}" \ LUA_CPATH="./lua_modules/lib/lua/5.1/?.so;;" \ LD_LIBRARY_PATH="$LD_LIBRARY_PATH:/opt/app-root/lib" -RUN luarocks install --deps-mode=none --tree /usr/local https://luarocks.org/manifests/pintsized/lua-resty-http-0.15-0.src.rock +RUN luarocks install --deps-mode=none --tree /usr/local https://luarocks.org/manifests/pintsized/lua-resty-http-0.17.1-0.src.rock RUN luarocks install --deps-mode=none --tree /usr/local https://luarocks.org/manifests/kikito/router-2.1-0.src.rock RUN luarocks install --deps-mode=none --tree /usr/local https://luarocks.org/manifests/kikito/inspect-3.1.1-0.src.rock RUN luarocks install --deps-mode=none --tree /usr/local https://luarocks.org/manifests/cdbattags/lua-resty-jwt-0.2.0-0.src.rock diff --git a/gateway/Roverfile.lock b/gateway/Roverfile.lock index 81ca5e27a..ef2469e79 100644 --- a/gateway/Roverfile.lock +++ b/gateway/Roverfile.lock @@ -9,7 +9,7 @@ ldoc 1.4.6-2||development liquid 0.2.0-2||production lua-resty-env 0.4.0-1||production lua-resty-execvp 0.1.1-1||production -lua-resty-http 0.15-0||production +lua-resty-http 0.17.1-0||production lua-resty-iputils 0.3.0-2||production lua-resty-jit-uuid 0.0.7-2||production lua-resty-jwt 0.2.0-0||production diff --git a/gateway/apicast-scm-1.rockspec b/gateway/apicast-scm-1.rockspec index 8d7f2edd9..d588476db 100644 --- a/gateway/apicast-scm-1.rockspec +++ b/gateway/apicast-scm-1.rockspec @@ -10,7 +10,7 @@ description = { license = "Apache License 2.0" } dependencies = { - 'lua-resty-http == 0.15', + 'lua-resty-http == 0.17.1', 'inspect', 'lyaml', 'router', From b37186b5cc35d32f0b64c28b8dc8b8db80dd2aea Mon Sep 17 00:00:00 2001 From: An Tran Date: Tue, 23 Jan 2024 22:26:40 +1000 Subject: [PATCH 2/4] Refactor code to use lua-resty-http 0.17.1 --- gateway/src/resty/http/proxy.lua | 192 ++++++++---------- .../src/resty/http_ng/backend/async_resty.lua | 25 ++- gateway/src/resty/resolver/http.lua | 24 ++- spec/resty/resolver/http_spec.lua | 2 +- 4 files changed, 118 insertions(+), 125 deletions(-) diff --git a/gateway/src/resty/http/proxy.lua b/gateway/src/resty/http/proxy.lua index eccdcd4c4..fb88bb79f 100644 --- a/gateway/src/resty/http/proxy.lua +++ b/gateway/src/resty/http/proxy.lua @@ -14,109 +14,6 @@ local function default_port(uri) return uri.port or resty_url.default_port(uri.scheme) end -local function connect_direct(httpc, request) - local uri = request.uri - local host = uri.host - local ip, port = httpc:resolve(host, nil, uri) - -- #TODO: This logic may no longer be needed as of PR#1323 and should be reviewed as part of a refactor - local options = { pool = format('%s:%s', host, port) } - local ok, err = httpc:connect(ip, port or default_port(uri), options) - - if not ok then return nil, err end - - ngx.log(ngx.DEBUG, 'connection to ', host, ':', httpc.port, ' established', - ', reused times: ', httpc:get_reused_times()) - - if uri.scheme == 'https' then - ok, err = httpc:ssl_handshake(nil, host, request.ssl_verify) - if not ok then return nil, err end - end - - -- use correct host header - httpc.host = host - - return httpc -end - -local function _connect_tls_direct(httpc, request, host, port) - - local uri = request.uri - - local ok, err = httpc:ssl_handshake(nil, uri.host, request.ssl_verify) - if not ok then return nil, err end - - return httpc -end - -local function _connect_proxy_https(httpc, request, host, port) - -- When the connection is reused the tunnel is already established, so - -- the second CONNECT request would reach the upstream instead of the proxy. - if httpc:get_reused_times() > 0 then - return httpc, 'already connected' - end - - local uri = request.uri - - local res, err = httpc:request({ - method = 'CONNECT', - path = format('%s:%s', host, port or default_port(uri)), - headers = { - ['Host'] = request.headers.host or format('%s:%s', uri.host, default_port(uri)), - ['Proxy-Authorization'] = request.proxy_auth or '' - } - }) - if not res then return nil, err end - - if res.status < 200 or res.status > 299 then - return nil, "failed to establish a tunnel through a proxy: " .. res.status - end - - res, err = httpc:ssl_handshake(nil, uri.host, request.ssl_verify) - if not res then return nil, err end - - return httpc -end - -local function connect_proxy(httpc, request) - -- target server requires hostname not IP and DNS resolution is left to the proxy itself as specified in the RFC #7231 - -- https://httpwg.org/specs/rfc7231.html#CONNECT - local uri = request.uri - local proxy_uri = request.proxy - local skip_https_connect = request.skip_https_connect - - if proxy_uri.scheme ~= 'http' then - return nil, 'proxy connection supports only http' - else - proxy_uri.port = default_port(proxy_uri) - end - - local port = default_port(uri) - - -- TLS tunnel is verified only once, so we need to reuse connections only for the same Host header - local options = { pool = format('%s:%s:%s:%s', proxy_uri.host, proxy_uri.port, uri.host, port) } - local ok, err = httpc:connect(proxy_uri.host, proxy_uri.port, options) - if not ok then return nil, err end - - ngx.log(ngx.DEBUG, 'connection to ', proxy_uri.host, ':', proxy_uri.port, ' established', - ', pool: ', options.pool, ' reused times: ', httpc:get_reused_times()) - - ngx.log(ngx.DEBUG, 'targeting server ', uri.host, ':', uri.port) - - if uri.scheme == 'http' then - -- http proxy needs absolute URL as the request path - request.path = format('%s://%s:%s%s', uri.scheme, uri.host, uri.port, uri.path or '/') - return httpc - elseif uri.scheme == 'https' and skip_https_connect then - local custom_uri = { scheme = uri.scheme, host = uri.host, port = uri.port, path = request.path } - request.path = url_helper.absolute_url(custom_uri) - return _connect_tls_direct(httpc, request, uri.host, uri.port) - elseif uri.scheme == 'https' then - return _connect_proxy_https(httpc, request, uri.host, uri.port) - else - return nil, 'invalid scheme' - end -end - local function parse_request_uri(request) local uri = request.uri or resty_url.parse(request.url) request.uri = uri @@ -150,15 +47,96 @@ local function connect(request) end local proxy_uri = find_proxy_url(request) + local uri = request.uri + local scheme = uri.scheme + local host = uri.host + local port = default_port(uri) + local skip_https_connect = request.skip_https_connect - request.ssl_verify = request.options and request.options.ssl and request.options.ssl.verify - request.proxy = proxy_uri + -- set ssl_verify: lua-resty-http set ssl_verify to true by default if scheme is https, whereas + -- openresty treat nil as false, so we need to explicitly set ssl_verify to false if nil + local ssl_verify = request.options and request.options.ssl and request.options.ssl.verify or false + local options = { + scheme = scheme, + host = host, + port = port + } + if scheme == 'https' then + options.ssl_server_name = host + options.ssl_verify = ssl_verify + end + + -- Connect via proxy if proxy_uri then - return connect_proxy(httpc, request) + if proxy_uri.scheme ~= 'http' then + return nil, 'proxy connection supports only http' + else + proxy_uri.port = default_port(proxy_uri) + end + + -- Resolve the proxy IP/Port + local proxy_host, proxy_port = httpc:resolve(proxy_uri.host, proxy_uri.port) + local proxy_url = format("%s://%s:%s", proxy_uri.scheme, proxy_host, proxy_port) + local proxy_auth = request.proxy_auth + + if scheme == 'http' then + -- http proxy needs absolute URL as the request path, lua-resty-http 1.17.1 will + -- construct a path_prefix based on host and port so we only set request path here + -- + -- https://github.com/ledgetech/lua-resty-http/blob/master/lib/resty/http_connect.lua#L99 + request.path = uri.path or '/' + options.proxy_opts = { + http_proxy = proxy_url, + http_proxy_authorization = proxy_auth + } + elseif scheme == 'https' and skip_https_connect then + options.scheme = proxy_uri.scheme + options.host = proxy_uri.host + options.port = proxy_uri.port + options.pool = format('%s:%s:%s:%s', proxy_uri.host, proxy_uri.port, host, port) + local custom_uri = { scheme = uri.scheme, host = uri.host, port = uri.port, path = request.path } + request.path = url_helper.absolute_url(custom_uri) + + local ok, err = httpc:connect(options) + if not ok then return nil, err end + + ngx.log(ngx.DEBUG, 'connection to ', proxy_uri.host, ':', proxy_uri.port, ' established', + ', pool: ', httpc.pool, ' reused times: ', httpc:get_reused_times()) + + ngx.log(ngx.DEBUG, 'targeting server ', host, ':', port) + + local ok, err = httpc:ssl_handshake(nil, host, request.ssl_verify) + if not ok then return nil, err end + + return httpc + elseif scheme == 'https' then + options.proxy_opts = { + https_proxy = proxy_url, + https_proxy_authorization = proxy_auth + } + else + return nil, 'invalid scheme' + end + + -- TLS tunnel is verified only once, so we need to reuse connections only for the same Host header + local ok, err = httpc:connect(options) + if not ok then return nil, err end + + ngx.log(ngx.DEBUG, 'connection to ', proxy_uri.host, ':', proxy_uri.port, ' established', + ', pool: ', httpc.pool, ' reused times: ', httpc:get_reused_times()) + ngx.log(ngx.DEBUG, 'targeting server ', host, ':', port) else - return connect_direct(httpc, request) + -- Connect direct + local ok, err = httpc:connect(options) + if not ok then return nil, err end + + ngx.log(ngx.DEBUG, 'connection to ', httpc.host, ':', httpc.port, ' established', + ', pool: ', httpc.pool, ' reused times: ', httpc:get_reused_times()) end + + + return httpc end function _M.env() diff --git a/gateway/src/resty/http_ng/backend/async_resty.lua b/gateway/src/resty/http_ng/backend/async_resty.lua index 36e9bd35a..b41670fbb 100644 --- a/gateway/src/resty/http_ng/backend/async_resty.lua +++ b/gateway/src/resty/http_ng/backend/async_resty.lua @@ -45,22 +45,21 @@ _M.async = function(request) end end - local ok, err = httpc:connect(host, port) + local verify = request.options and request.options.ssl and request.options.ssl.verify + if type(verify) == 'nil' then verify = true end - if not ok then - return response.error(request, err) - end - - if scheme == 'https' then - local verify = request.options and request.options.ssl and request.options.ssl.verify - if type(verify) == 'nil' then verify = true end + local options = { + scheme = scheme, + host = host, + port = port, + ssl_server_name = host, + ssl_verify = verify + } - local session - session, err = httpc:ssl_handshake(false, host, verify) + local ok, err = httpc:connect(options) - if not session then - return response.error(request, err) - end + if not ok then + return response.error(request, err) end local res diff --git a/gateway/src/resty/resolver/http.lua b/gateway/src/resty/resolver/http.lua index 4af497855..c08db8263 100644 --- a/gateway/src/resty/resolver/http.lua +++ b/gateway/src/resty/resolver/http.lua @@ -38,13 +38,29 @@ function _M:resolve(host, port, options) return ip, port end -function _M.connect(self, host, port, ...) - local ip, real_port = self:resolve(host, port) - local ok, err = resty_http.connect(self, ip, real_port, ...) +function _M.connect(self, options, ...) + -- cache the host because we need to resolve host to IP + local host = options.host + local proxy_opts = options.proxy_opts + local proxy = proxy_opts and (proxy_opts.http_proxy or proxy_opts.https_proxy) + local ip, real_port + + -- target server requires hostname not IP and DNS resolution is left to the proxy itself as specified in the RFC #7231 + -- https://httpwg.org/specs/rfc7231.html#CONNECT + -- + -- Therefore, only resolve host IP when not using with proxy + if not proxy then + ip, real_port = self:resolve(options.host, options.port) + options.host = ip + options.port = real_port + end + + local ok, err = resty_http.connect(self, options, ...) if ok then + -- use correct host header self.host = host - self.port = real_port + self.port = options.port end ngx.log(ngx.DEBUG, 'connected to ip:', ip, ' host: ', host, ' port: ', real_port, ' ok: ', ok, ' err: ', err) diff --git a/spec/resty/resolver/http_spec.lua b/spec/resty/resolver/http_spec.lua index 87c43ed20..fadd42330 100644 --- a/spec/resty/resolver/http_spec.lua +++ b/spec/resty/resolver/http_spec.lua @@ -15,7 +15,7 @@ describe('resty.resolver.http', function() local client = _M.new() client:set_timeout(1000) client.resolver.cache:save({ { address = '127.0.0.1', name = 'unknown.', ttl = 1800 } }) - assert(client:connect('unknown', 1984)) + assert(client:connect({scheme="http", host='unknown', port=1984})) assert.equal('unknown', client.host) assert.equal(1984, client.port) end) From 32781dd671922cb70704e95fea7476dcfc2f0653 Mon Sep 17 00:00:00 2001 From: An Tran Date: Tue, 30 Jan 2024 16:17:50 +1000 Subject: [PATCH 3/4] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 781eae3b1..183a21e68 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Docker compose up instead of docker compose run [PR #1442](https://github.com/3scale/APIcast/pull/1442) - Fix integration of upstream connection policy with camel policy [PR #1443](https://github.com/3scale/APIcast/pull/1443) [THREESCALE-10582](https://issues.redhat.com/browse/THREESCALE-10582) +- Upgrade lua-resty-http to 0.17.1 to fix 100 response header are not handled when using `HTTPS_PROXY` [PR #1434](https://github.com/3scale/APIcast/pull/1434) [THREESCALE-10278](https://issues.redhat.com/browse/THREESCALE-10278) ### Added From 9436b44b134a0404a7f88a8d90ee5edc68e30713 Mon Sep 17 00:00:00 2001 From: An Tran Date: Mon, 5 Feb 2024 17:56:55 +1000 Subject: [PATCH 4/4] Move the proxy DNS resolver code into gateway/src/resty/resolver/http.lua To avoid having to call a resolver when used with proxies, all DNS resolution should happen inside the connect() method --- CHANGELOG.md | 1 + gateway/src/resty/http/proxy.lua | 7 ++++--- gateway/src/resty/resolver/http.lua | 17 +++++++++++++++++ 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 183a21e68..9451e8ce4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Docker compose up instead of docker compose run [PR #1442](https://github.com/3scale/APIcast/pull/1442) - Fix integration of upstream connection policy with camel policy [PR #1443](https://github.com/3scale/APIcast/pull/1443) [THREESCALE-10582](https://issues.redhat.com/browse/THREESCALE-10582) + - Upgrade lua-resty-http to 0.17.1 to fix 100 response header are not handled when using `HTTPS_PROXY` [PR #1434](https://github.com/3scale/APIcast/pull/1434) [THREESCALE-10278](https://issues.redhat.com/browse/THREESCALE-10278) ### Added diff --git a/gateway/src/resty/http/proxy.lua b/gateway/src/resty/http/proxy.lua index fb88bb79f..40ea28c88 100644 --- a/gateway/src/resty/http/proxy.lua +++ b/gateway/src/resty/http/proxy.lua @@ -75,12 +75,12 @@ local function connect(request) proxy_uri.port = default_port(proxy_uri) end - -- Resolve the proxy IP/Port - local proxy_host, proxy_port = httpc:resolve(proxy_uri.host, proxy_uri.port) - local proxy_url = format("%s://%s:%s", proxy_uri.scheme, proxy_host, proxy_port) + local proxy_url = format("%s://%s:%s", proxy_uri.scheme, proxy_uri.host, proxy_uri.port) local proxy_auth = request.proxy_auth if scheme == 'http' then + -- Used by http_ng module to send request to 3scale backend through proxy. + -- http proxy needs absolute URL as the request path, lua-resty-http 1.17.1 will -- construct a path_prefix based on host and port so we only set request path here -- @@ -128,6 +128,7 @@ local function connect(request) ngx.log(ngx.DEBUG, 'targeting server ', host, ':', port) else -- Connect direct + -- Mostly used by http_ng module to connect 3scale backend module. local ok, err = httpc:connect(options) if not ok then return nil, err end diff --git a/gateway/src/resty/resolver/http.lua b/gateway/src/resty/resolver/http.lua index c08db8263..4f646ffbc 100644 --- a/gateway/src/resty/resolver/http.lua +++ b/gateway/src/resty/resolver/http.lua @@ -1,6 +1,8 @@ local resty_http = require 'resty.http' local resty_resolver = require 'resty.resolver' local round_robin = require 'resty.balancer.round_robin' +local url_helper = require('resty.url_helper') +local format = string.format local setmetatable = setmetatable @@ -53,6 +55,21 @@ function _M.connect(self, options, ...) ip, real_port = self:resolve(options.host, options.port) options.host = ip options.port = real_port + else + local proxy_uri, err = url_helper.parse_url(proxy) + if not proxy_uri then + return nil, 'invalid proxy: ' .. err + end + + -- Resolve the proxy IP/Port + local proxy_host, proxy_port = self:resolve(proxy_uri.host, proxy_uri.port) + local proxy_url = format("%s://%s:%s", proxy_uri.scheme, proxy_host, proxy_port) + + if proxy_opts.http_proxy then + options.proxy_opts.http_proxy = proxy_url + elseif proxy_opts.https_proxy then + options.proxy_opts.https_proxy = proxy_url + end end local ok, err = resty_http.connect(self, options, ...)