Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[THREESCALE-5105] Adding support for mtls when using proxy policy #1499

Merged
merged 4 commits into from
Oct 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

- Fixed Conditional policy evaluating incorrectly: second policy in policy chain that implement export() always triggers [PR #1485](https://github.com/3scale/APIcast/pull/1485) [THREESCALE-9320](https://issues.redhat.com/browse/THREESCALE-9320)
- Fix APIcast using stale configuration for deleted products [PR #1488](https://github.com/3scale/APIcast/pull/1488) [THREESCALE-10130](https://issues.redhat.com/browse/THREESCALE-10130)
- Fixed Mutual TLS between APIcast and the Backend API fails when using a Forward Proxy [PR #1499](https://github.com/3scale/APIcast/pull/1499) [THREESCALE-5105](https://issues.redhat.com/browse/THREESCALE-5105)

### Added

Expand Down
7 changes: 3 additions & 4 deletions gateway/src/apicast/http_proxy.lua
Original file line number Diff line number Diff line change
Expand Up @@ -143,9 +143,7 @@ local function forward_https_request(proxy_uri, uri, proxy_opts)
path = format('%s%s%s', ngx.var.uri, ngx.var.is_args, ngx.var.query_string or ''),
body = body,
proxy_uri = proxy_uri,
proxy_auth = opts.proxy_auth,
upstream_connection_opts = opts.upstream_connection_opts,
skip_https_connect = opts.skip_https_connect
proxy_options = opts
}

local httpc, err = http_proxy.new(request)
Expand Down Expand Up @@ -226,7 +224,8 @@ function _M.request(upstream, proxy_uri)
proxy_auth = proxy_auth,
skip_https_connect = upstream.skip_https_connect,
request_unbuffered = upstream.request_unbuffered,
upstream_connection_opts = upstream.upstream_connection_opts
upstream_connection_opts = upstream.upstream_connection_opts,
upstream_ssl = upstream.upstream_ssl
}

forward_https_request(proxy_uri, uri, proxy_opts)
Expand Down
39 changes: 38 additions & 1 deletion gateway/src/apicast/policy/apicast/apicast.lua
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
local setmetatable = setmetatable
local assert = assert
local table_insert = table.insert
local base = require "resty.core.base"
local get_request = base.get_request
local tls = require 'resty.tls'

local user_agent = require('apicast.user_agent')

Expand Down Expand Up @@ -156,6 +159,40 @@
}
end

_M.balancer = balancer.call
function _M:balancer(context)
-- All of this happens on balancer because this is subrequest inside APICAst
--to @upstream, so the request need to be the one that connects to the
--upstreamssl_client_raw_cert0
local r = get_request()
if not r then
ngx.log(ngx.WARN, "Invalid request")
return

Check warning on line 169 in gateway/src/apicast/policy/apicast/apicast.lua

View check run for this annotation

Codecov / codecov/patch

gateway/src/apicast/policy/apicast/apicast.lua#L168-L169

Added lines #L168 - L169 were not covered by tests
end

if context.upstream_certificate and context.upstream_key then
local ok, err = tls.set_upstream_cert_and_key(context.upstream_certificate, context.upstream_key)
if ok ~= nil then
ngx.log(ngx.ERR, "Certificate cannot be set correctly, err: ", err)

Check warning on line 175 in gateway/src/apicast/policy/apicast/apicast.lua

View check run for this annotation

Codecov / codecov/patch

gateway/src/apicast/policy/apicast/apicast.lua#L175

Added line #L175 was not covered by tests
end
end

if context.upstream_verify then
local ok, err = tls.set_upstream_ssl_verify(true, 1)
if ok ~= nil then
ngx.log(ngx.WARN, "Cannot verify SSL upstream connection, err: ", err)

Check warning on line 182 in gateway/src/apicast/policy/apicast/apicast.lua

View check run for this annotation

Codecov / codecov/patch

gateway/src/apicast/policy/apicast/apicast.lua#L182

Added line #L182 was not covered by tests
end

if not context.upstream_ca_store then
ngx.log(ngx.WARN, "Set verify without including CA certificates")

Check warning on line 186 in gateway/src/apicast/policy/apicast/apicast.lua

View check run for this annotation

Codecov / codecov/patch

gateway/src/apicast/policy/apicast/apicast.lua#L186

Added line #L186 was not covered by tests
end

ok, err = tls.set_upstream_ca_cert(context.upstream_ca_store)
if ok ~= nil then
ngx.log(ngx.WARN, "Cannot set a valid trusted CA store, err: ", err)

Check warning on line 191 in gateway/src/apicast/policy/apicast/apicast.lua

View check run for this annotation

Codecov / codecov/patch

gateway/src/apicast/policy/apicast/apicast.lua#L191

Added line #L191 was not covered by tests
end
end

balancer:call(context)
end

return _M
2 changes: 2 additions & 0 deletions gateway/src/apicast/policy/upstream_mtls/Readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,5 @@ When using http forms and file upload
This policy overwrites `APICAST_PROXY_HTTPS_CERTIFICATE_KEY` and
`APICAST_PROXY_HTTPS_CERTIFICATE` values and it uses the certificates set by
the policy, so those environment variables will have no effect.

This policy is not compatible with Camel proxy.
67 changes: 5 additions & 62 deletions gateway/src/apicast/policy/upstream_mtls/upstream_mtls.lua
Original file line number Diff line number Diff line change
@@ -1,28 +1,14 @@
-- This policy enables MTLS with the upstream API endpoint

local ssl = require('ngx.ssl')
local ffi = require "ffi"
local base = require "resty.core.base"
local data_url = require('resty.data_url')
local util = require 'apicast.util'

local C = ffi.C
local get_request = base.get_request
local pairs = pairs

local X509_STORE = require('resty.openssl.x509.store')
local X509 = require('resty.openssl.x509')

ffi.cdef([[
int ngx_http_apicast_ffi_set_proxy_cert_key(
ngx_http_request_t *r, void *cdata_chain, void *cdata_key);
int ngx_http_apicast_ffi_set_proxy_ca_cert(
ngx_http_request_t *r, void *cdata_ca);
int ngx_http_apicast_ffi_set_ssl_verify(
ngx_http_request_t *r, int verify, int verify_deph);
]])


local policy = require('apicast.policy')
local _M = policy.new('mtls', "builtin")

Expand Down Expand Up @@ -104,54 +90,11 @@ function _M.new(config)
return self
end


-- Set the certs for the upstream connection. Need to receive the pointers from
-- parse_* functions.
--- Public function to be able to unittest this.
function _M.set_certs(r, cert, key)
local val = C.ngx_http_apicast_ffi_set_proxy_cert_key(r, cert, key)
if val ~= ngx.OK then
ngx.log(ngx.ERR, "Certificate cannot be set correctly")
end
end

function _M.set_ca_cert(r, store)
local val = C.ngx_http_apicast_ffi_set_proxy_ca_cert(r, store)
if val ~= ngx.OK then
ngx.log(ngx.WARN, "Cannot set a valid trusted CA store")
return
end
end

-- All of this happens on balancer because this is subrequest inside APICAst
--to @upstream, so the request need to be the one that connects to the
--upstream0
function _M:balancer(context)
local r = get_request()
if not r then
ngx.log(ngx.WARN, "Invalid request")
return
end

if self.cert and self.cert_key then
self.set_certs(r, self.cert, self.cert_key)
end

if not self.verify then
return
end

local val = C.ngx_http_apicast_ffi_set_ssl_verify(r, ffi.new("int", 1), ffi.new("int", 1))
if val ~= ngx.OK then
ngx.log(ngx.WARN, "Cannot verify SSL upstream connection")
end

if not self.ca_store then
ngx.log(ngx.WARN, "Set verify without including CA certificates")
return
end

self.set_ca_cert(r, self.ca_store)
function _M:access(context)
context.upstream_certificate = self.cert
context.upstream_key = self.cert_key
context.upstream_verify = self.verify
context.upstream_ca_store = self.ca_store
end

return _M
5 changes: 5 additions & 0 deletions gateway/src/apicast/upstream.lua
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,11 @@ function _M:call(context)

self.request_unbuffered = context.request_unbuffered
self.upstream_connection_opts = context.upstream_connection_opts
self.upstream_ssl = {
ssl_verify = context.upstream_verify,
ssl_client_cert = context.upstream_certificate,
ssl_client_priv_key = context.upstream_key
}
http_proxy.request(self, proxy_uri)
else
local err = self:rewrite_request()
Expand Down
13 changes: 9 additions & 4 deletions gateway/src/resty/http/proxy.lua
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,10 @@
local function connect(request)
request = request or { }
local httpc = http.new()
local proxy_options = request.proxy_options or {}

if request.upstream_connection_opts then
local con_opts = request.upstream_connection_opts
if proxy_options.upstream_connection_opts then
local con_opts = request.proxy_options.upstream_connection_opts

Check warning on line 38 in gateway/src/resty/http/proxy.lua

View check run for this annotation

Codecov / codecov/patch

gateway/src/resty/http/proxy.lua#L38

Added line #L38 was not covered by tests
ngx.log(ngx.DEBUG, 'setting timeouts (secs), connect_timeout: ', con_opts.connect_timeout,
' send_timeout: ', con_opts.send_timeout, ' read_timeout: ', con_opts.read_timeout)
-- lua-resty-http uses nginx API for lua sockets
Expand All @@ -51,7 +52,7 @@
local scheme = uri.scheme
local host = uri.host
local port = default_port(uri)
local skip_https_connect = request.skip_https_connect
local skip_https_connect = proxy_options.skip_https_connect

-- 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
Expand All @@ -68,6 +69,10 @@
if scheme == 'https' then
options.ssl_server_name = host
options.ssl_verify = ssl_verify
if proxy_options.upstream_ssl then
options.ssl_client_cert = proxy_options.upstream_ssl.ssl_client_cert
options.ssl_client_priv_key = proxy_options.upstream_ssl.ssl_client_priv_key

Check warning on line 74 in gateway/src/resty/http/proxy.lua

View check run for this annotation

Codecov / codecov/patch

gateway/src/resty/http/proxy.lua#L73-L74

Added lines #L73 - L74 were not covered by tests
end
end

-- Connect via proxy
Expand All @@ -79,7 +84,7 @@
end

local proxy_url = format("%s://%s:%s", proxy_uri.scheme, proxy_uri.host, proxy_uri.port)
local proxy_auth = request.proxy_auth
local proxy_auth = proxy_options.proxy_auth

if scheme == 'http' then
-- Used by http_ng module to send request to 3scale backend through proxy.
Expand Down
91 changes: 91 additions & 0 deletions gateway/src/resty/tls.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
local base = require "resty.core.base"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please ignore codecov report. I will move tls.lua to apicast-nginx-module in the future commit.

Do we plan to move this file (and content) to the apicast-nginx-module??

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I'm planing to replace https://github.com/3scale/apicast-nginx-module/blob/master/lib/resty/mtls.lua with the content of this tls.lua file. This makes it easier to test for patches, memory leaks...etc.


local type = type
local tostring = tostring

local get_request = base.get_request
local ffi = require "ffi"
local C = ffi.C

local _M = {}

local NGX_OK = ngx.OK

local ngx_http_apicast_ffi_set_proxy_cert_key;
local ngx_http_apicast_ffi_set_proxy_ca_cert;

Check warning on line 15 in gateway/src/resty/tls.lua

View check run for this annotation

Codecov / codecov/patch

gateway/src/resty/tls.lua#L15

Added line #L15 was not covered by tests
local ngx_http_apicast_ffi_set_ssl_verify

ffi.cdef([[
int ngx_http_apicast_ffi_set_proxy_cert_key(
ngx_http_request_t *r, void *cdata_chain, void *cdata_key);
int ngx_http_apicast_ffi_set_proxy_ca_cert(
ngx_http_request_t *r, void *cdata_ca);
int ngx_http_apicast_ffi_set_ssl_verify(
ngx_http_request_t *r, int verify, int verify_deph);
]])

ngx_http_apicast_ffi_set_proxy_cert_key = C.ngx_http_apicast_ffi_set_proxy_cert_key
ngx_http_apicast_ffi_set_proxy_ca_cert = C.ngx_http_apicast_ffi_set_proxy_ca_cert
ngx_http_apicast_ffi_set_ssl_verify = C.ngx_http_apicast_ffi_set_ssl_verify

-- Set the certs for the upstream connection. Need to receive the pointers from
-- parse_* functions.
function _M.set_upstream_cert_and_key(cert, key)
local r = get_request()
if not r then
error("no request found")

Check warning on line 36 in gateway/src/resty/tls.lua

View check run for this annotation

Codecov / codecov/patch

gateway/src/resty/tls.lua#L36

Added line #L36 was not covered by tests
end

if not cert or not key then
return nil, "cert and key must not be nil"

Check warning on line 40 in gateway/src/resty/tls.lua

View check run for this annotation

Codecov / codecov/patch

gateway/src/resty/tls.lua#L40

Added line #L40 was not covered by tests
end

local ret = ngx_http_apicast_ffi_set_proxy_cert_key(r, cert, key)
if ret ~= NGX_OK then
return nil, "error while setting upstream client certificate and key"
end
end

-- Set the trusted store for the upstream connection.
function _M.set_upstream_ca_cert(store)
local r = get_request()
if not r then
error("no request found")

Check warning on line 53 in gateway/src/resty/tls.lua

View check run for this annotation

Codecov / codecov/patch

gateway/src/resty/tls.lua#L53

Added line #L53 was not covered by tests
end

if not store then
return nil, "trusted store must not be nil"

Check warning on line 57 in gateway/src/resty/tls.lua

View check run for this annotation

Codecov / codecov/patch

gateway/src/resty/tls.lua#L57

Added line #L57 was not covered by tests
end

local ret = ngx_http_apicast_ffi_set_proxy_ca_cert(r, store)
if ret ~= NGX_OK then
return nil, "error while setting upstream trusted CA store"
end
end

-- Verify upstream connection
function _M.set_upstream_ssl_verify(verify, verify_deph)
local r = get_request()
if not r then
error("no request found")

Check warning on line 70 in gateway/src/resty/tls.lua

View check run for this annotation

Codecov / codecov/patch

gateway/src/resty/tls.lua#L70

Added line #L70 was not covered by tests
end

if type(verify) ~= 'boolean' then
return nil, "verify expects a boolean but found " .. type(verify)

Check warning on line 74 in gateway/src/resty/tls.lua

View check run for this annotation

Codecov / codecov/patch

gateway/src/resty/tls.lua#L74

Added line #L74 was not covered by tests
end

if type(verify_deph) ~= 'number' then
return nil, "verify depth expects a number but found " .. type(verify)

Check warning on line 78 in gateway/src/resty/tls.lua

View check run for this annotation

Codecov / codecov/patch

gateway/src/resty/tls.lua#L78

Added line #L78 was not covered by tests
end

if verify_deph < 0 then
return nil, "verify_deph expects a non-negative interger but found" .. tostring(verify_deph)

Check warning on line 82 in gateway/src/resty/tls.lua

View check run for this annotation

Codecov / codecov/patch

gateway/src/resty/tls.lua#L82

Added line #L82 was not covered by tests
end

local val = ngx_http_apicast_ffi_set_ssl_verify(r, verify, verify_deph)
if val ~= NGX_OK then
return nil, "error while setting upstream verify"
end
end

return _M
Loading
Loading