THREESCALE-11944 Verify CA certificate when sending request via proxy

CHANGELOG.md

@@ -23,6 +23,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - Use new cpu.requests formula from Kubernetes. [PR #1595](https://github.com/3scale/APIcast/pull/1595) [THREESCALE-15465](https://redhat.atlassian.net/browse/THREESCALE-15465)
 - Fix batcher policy fails silently when configured with string values instead of integers. [PR #1597](https://github.com/3scale/APIcast/pull/1597) [THREESCALE-15547](https://redhat.atlassian.net/browse/THREESCALE-15547)
 - Unify timeout options between http clients library [PR #1600](https://github.com/3scale/APIcast/pull/1600)
+- Certificate is never verified in API request for https backend via proxy [PR #1573](https://github.com/3scale/APIcast/pull/1568) [THREESCALE-11944](https://redhat.atlassian.net/browse/THREESCALE-11944)
 
 ### Added
 - Update APIcast schema manifest [PR #1550](https://github.com/3scale/APIcast/pull/1550)

gateway/src/apicast/http_proxy.lua

@@ -150,7 +150,8 @@ local function forward_https_request(proxy_uri, uri, proxy_opts)
         body = body,
         proxy_uri = proxy_uri,
         timeout = opts.upstream_connection_opts,  -- Extract timeouts to top level
-        proxy_options = opts
+        options = opts.options,
+        proxy_options = opts,
     }
 
     local httpc, err = http_proxy.new(request)
@@ -238,7 +239,7 @@ function _M.request(upstream, proxy_uri)
             skip_https_connect = upstream.skip_https_connect,
             request_unbuffered = upstream.request_unbuffered,
             upstream_connection_opts = upstream.upstream_connection_opts,
-            upstream_ssl = upstream.upstream_ssl
+            options = upstream.options
         }
 
         forward_https_request(proxy_uri, uri, proxy_opts)

gateway/src/apicast/upstream.lua

@@ -233,10 +233,12 @@ 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
+        self.options = {
+          ssl = {
+            verify = context.upstream_verify,
+            client_cert = context.upstream_certificate,
+            client_priv_key = context.upstream_key
+          }
         }
         http_proxy.request(self, proxy_uri)
     else

gateway/src/resty/http/proxy.lua

@@ -71,9 +71,9 @@ local function connect(request)
     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
+        if request.options and request.options.ssl then
+          options.ssl_client_cert = request.options.ssl.client_cert
+          options.ssl_client_priv_key = request.options.ssl.client_priv_key
         end
     end
 
@@ -116,7 +116,7 @@ local function connect(request)
 
             ngx.log(ngx.DEBUG, 'targeting server ', host, ':', port)
 
-            local ok, err = httpc:ssl_handshake(nil, host, request.ssl_verify)
+            local ok, err = httpc:ssl_handshake(nil, host, ssl_verify)
             if not ok then return nil, err end
 
             return httpc

spec/resty/http/proxy_spec.lua

@@ -196,4 +196,121 @@ describe('resty.http.proxy', function()
           end)
       end)
     end)
+
+    describe('options.ssl', function()
+        local http, httpc_mock, connect_spy
+
+        local function create_httpc_mock()
+            return {
+                set_timeouts = function() end,
+                connect = function() return true end,
+                request = function() return { status = 200, read_body = function() return 'ok' end } end,
+                close = function() end,
+                set_keepalive = function() end,
+                pool = 'test',
+                get_reused_times = function() return 0 end,
+                host = 'example.com',
+                port = 443
+            }
+        end
+
+        before_each(function()
+            http = require('resty.resolver.http')
+            httpc_mock = create_httpc_mock()
+            connect_spy = spy.new(function() return true end)
+            httpc_mock.connect = connect_spy
+            stub(http, 'new', function() return httpc_mock end)
+        end)
+
+        it('propagates options.ssl.verify to the connect ssl_verify option (used by http_ng)', function()
+            local request = {
+                url = 'https://upstream:8091/request',
+                method = 'GET',
+                options = { ssl = { verify = true } }
+            }
+
+            assert(_M.new(request))
+
+            local connect_options = connect_spy.calls[1].vals[2]
+            assert.is_true(connect_options.ssl_verify)
+        end)
+
+        it('propagates options.ssl.client_cert/client_priv_key set by the forward-proxy path', function()
+            local cert, key = 'cert-data', 'key-data'
+            local request = {
+                url = 'https://upstream:8091/request',
+                method = 'GET',
+                options = { ssl = { verify = true, client_cert = cert, client_priv_key = key } }
+            }
+
+            assert(_M.new(request))
+
+            local connect_options = connect_spy.calls[1].vals[2]
+            assert.same(cert, connect_options.ssl_client_cert)
+            assert.same(key, connect_options.ssl_client_priv_key)
+        end)
+
+        it('defaults ssl_verify to false when options.ssl is not set', function()
+            local request = { url = 'https://upstream:8091/request', method = 'GET' }
+
+            assert(_M.new(request))
+
+            local connect_options = connect_spy.calls[1].vals[2]
+            assert.is_false(connect_options.ssl_verify)
+        end)
+    end)
+
+    describe('skip_https_connect', function()
+        local http, httpc_mock, ssl_handshake_spy
+
+        local function create_httpc_mock()
+            return {
+                set_timeouts = function() end,
+                connect = function() return true end,
+                ssl_handshake = function() return true end,
+                request = function() return { status = 200, read_body = function() return 'ok' end } end,
+                close = function() end,
+                set_keepalive = function() end,
+                pool = 'test',
+                get_reused_times = function() return 0 end,
+                host = 'example.com',
+                port = 443
+            }
+        end
+
+        before_each(function()
+            http = require('resty.resolver.http')
+            httpc_mock = create_httpc_mock()
+            ssl_handshake_spy = spy.new(function() return true end)
+            httpc_mock.ssl_handshake = ssl_handshake_spy
+            stub(http, 'new', function() return httpc_mock end)
+        end)
+
+        it('passes options.ssl.verify to ssl_handshake, not the unset request.ssl_verify', function()
+            local request = {
+                url = 'https://upstream:8091/request',
+                method = 'GET',
+                proxy_uri = { scheme = 'http', host = 'proxy', port = 8080 },
+                proxy_options = { skip_https_connect = true },
+                options = { ssl = { verify = true } }
+            }
+
+            assert(_M.new(request))
+
+            assert.spy(ssl_handshake_spy).was_called_with(match.is_table(), nil, 'upstream', true)
+        end)
+
+        it('defaults ssl_verify to false in ssl_handshake when options.ssl is not set', function()
+            local request = {
+                url = 'https://upstream:8091/request',
+                method = 'GET',
+                proxy_uri = { scheme = 'http', host = 'proxy', port = 8080 },
+                proxy_options = { skip_https_connect = true }
+            }
+
+            assert(_M.new(request))
+
+            assert.spy(ssl_handshake_spy).was_called_with(match.is_table(), nil, 'upstream', false)
+        end)
+    end)
 end)