fix(runloop): service level tls_verify not overriding global settin…

…g `proxy_ssl_verify ` (#13470)

Should also set_upstream_ssl_verify when `tls_verify` is false, because the
global level config `proxy_ssl_verify` may be `on`.

https://konghq.atlassian.net/browse/FTI-6095

changelog/unreleased/kong/fix-service-tls-verify.yml

@@ -0,0 +1,4 @@
+message: |
+  Fixed an issue where setting `tls_verify` to `false` didn't override the global level `proxy_ssl_verify`.
+type: bugfix
+scope: Core

kong/db/schema/entities/services.lua

@@ -42,7 +42,7 @@ return {
     { read_timeout       = nonzero_timeout { default = 60000 }, },
     { tags               = typedefs.tags },
     { client_certificate = { description = "Certificate to be used as client certificate while TLS handshaking to the upstream server.", type = "foreign", reference = "certificates" }, },
-    { tls_verify         = { description = "Whether to enable verification of upstream server TLS certificate.", type = "boolean", }, },
+    { tls_verify         = { description = "Whether to enable verification of upstream server TLS certificate. If not set, the global level config `proxy_ssl_verify` will be used.", type = "boolean", }, },
     { tls_verify_depth   = { description = "Maximum depth of chain while verifying Upstream server's TLS certificate.", type = "integer", default = null, between = { 0, 64 }, }, },
     { ca_certificates    = { description = "Array of CA Certificate object UUIDs that are used to build the trust store while verifying upstream server's TLS certificate.", type = "array", elements = { type = "string", uuid = true, }, }, },
     { enabled            = { description = "Whether the Service is active. ", type = "boolean", required = true, default = true, }, },

kong/runloop/upstream_ssl.lua

@@ -42,7 +42,7 @@ local function set_service_ssl(ctx)
   end
 
   local tls_verify = service.tls_verify
-  if tls_verify then
+  if tls_verify ~= nil then
     res, err = set_upstream_ssl_verify(tls_verify)
     if not res then
       log(CRIT, "unable to set upstream TLS verification to: ",

spec/02-integration/05-proxy/18-upstream_tls_spec.lua

@@ -121,6 +121,23 @@ local function gen_plugin(route)
   }
 end
 
+local function get_proxy_client(subsystems, stream_port)
+  if subsystems == "http" then
+    return assert(helpers.proxy_client())
+  else
+     return assert(helpers.proxy_client(20000, stream_port))
+  end
+end
+
+local function wait_for_all_config_update(subsystems)
+  local opt = {}
+  if subsystems == "stream" then
+    opt.stream_enabled = true
+    opt.stream_port = 19003
+  end
+
+  helpers.wait_for_all_config_update(opt)
+end
 
 for _, flavor in ipairs({ "traditional", "traditional_compatible", "expressions" }) do
 for _, strategy in helpers.each_strategy() do
@@ -318,24 +335,6 @@ for _, strategy in helpers.each_strategy() do
       end
     end
 
-    local function get_proxy_client(subsystems, stream_port)
-      if subsystems == "http" then
-        return assert(helpers.proxy_client())
-      else
-         return assert(helpers.proxy_client(20000, stream_port))
-      end
-    end
-
-    local function wait_for_all_config_update(subsystems)
-      local opt = {}
-      if subsystems == "stream" then
-        opt.stream_enabled = true
-        opt.stream_port = 19003
-      end
-
-      helpers.wait_for_all_config_update(opt)
-    end
-
     for _, subsystems in pairs({"http", "stream"}) do
     describe(subsystems .. " mutual TLS authentication against upstream with Service object", function()
       describe("no client certificate supplied", function()
@@ -1250,3 +1249,196 @@ for _, strategy in helpers.each_strategy() do
   end)
 end
 end   -- for flavor
+
+for _, flavor in ipairs({ "traditional", "traditional_compatible", "expressions" }) do
+for _, strategy in helpers.each_strategy() do
+  describe("overriding upstream TLS parameters for database [#" .. strategy .. ", flavor = " .. flavor .. "] (nginx_proxy_proxy_ssl_verify: on, nginx_sproxy_proxy_ssl_verify: on)", function()
+    local admin_client
+    local bp
+    local service_tls
+    local tls_service_tls
+    local route_tls_buffered_proxying
+
+    reload_router(flavor)
+
+    lazy_setup(function()
+      bp = helpers.get_db_utils(strategy, {
+        "routes",
+        "services",
+        "certificates",
+        "ca_certificates",
+        "upstreams",
+        "targets",
+      })
+
+      service_tls = assert(bp.services:insert({
+        name = "protected-service",
+        url = "https://example.com:16799/", -- domain name needed for hostname check
+      }))
+
+      assert(bp.routes:insert(gen_route(flavor,{
+        service = { id = service_tls.id, },
+        hosts = { "example.com", },
+        paths = { "/tls", },
+      })))
+
+      route_tls_buffered_proxying = assert(bp.routes:insert(gen_route(flavor,{
+        service = { id = service_tls.id, },
+        hosts = { "example.com", },
+        paths = { "/tls-buffered-proxying", },
+      })))
+
+      -- use pre-function to enable buffered_proxying in order to trigger the
+      -- `ngx.location.capture("/kong_buffered_http")` in `Kong.response()`
+      assert(bp.plugins:insert(gen_plugin(route_tls_buffered_proxying)))
+
+      -- tls
+      tls_service_tls = assert(bp.services:insert({
+        name = "tls-protected-service",
+        url = "tls://example.com:16799", -- domain name needed for hostname check
+      }))
+
+      assert(bp.routes:insert(gen_route(flavor,{
+        service = { id = tls_service_tls.id, },
+        destinations = {
+          {
+            port = 19001,
+          },
+        },
+        protocols = {
+          "tls",
+        },
+      })))
+
+      assert(helpers.start_kong({
+        router_flavor = flavor,
+        database   = strategy,
+        nginx_conf = "spec/fixtures/custom_nginx.template",
+        stream_listen = helpers.get_proxy_ip(false) .. ":19000,"
+                     .. helpers.get_proxy_ip(false) .. ":19001,"
+                     .. helpers.get_proxy_ip(false) .. ":19002,"
+                     .. helpers.get_proxy_ip(false) .. ":19003",
+        nginx_proxy_proxy_ssl_verify = "on",
+        -- An unrelated ca, just used as a placeholder to prevent nginx from reporting errors
+        nginx_proxy_proxy_ssl_trusted_certificate = "../spec/fixtures/kong_clustering_ca.crt",
+        nginx_sproxy_proxy_ssl_verify = "on",
+        nginx_sproxy_proxy_ssl_trusted_certificate = "../spec/fixtures/kong_clustering_ca.crt",
+      }, nil, nil, fixtures))
+
+      admin_client = assert(helpers.admin_client())
+    end)
+
+    lazy_teardown(function()
+      helpers.stop_kong()
+    end)
+
+    for _, subsystems in pairs({"http", "stream"}) do
+    describe(subsystems .. " TLS verification options against upstream", function()
+      describe("tls_verify", function()
+        it("default is on, request is blocked", function()
+          local proxy_client = get_proxy_client(subsystems, 19001)
+          local path
+          if subsystems == "http" then
+            path = "/tls"
+          else
+            path = "/"
+          end
+
+          local res, err = proxy_client:send {
+            path    = path,
+            headers = {
+              ["Host"] = "example.com",
+            }
+          }
+          if subsystems == "http" then
+            local body = assert.res_status(502, res)
+            assert.matches("An invalid response was received from the upstream server", body)
+          else
+            assert.equals("connection reset by peer", err)
+          end
+          assert(proxy_client:close())
+        end)
+
+        -- buffered_proxying
+        if subsystems == "http" then
+          it("default is on, buffered_proxying = true, request is blocked", function()
+            local proxy_client = get_proxy_client(subsystems, 19001)
+            local path = "/tls-buffered-proxying"
+            local res = proxy_client:send {
+              path    = path,
+              headers = {
+                ["Host"] = "example.com",
+              }
+            }
+
+            local body = assert.res_status(502, res)
+            assert.matches("An invalid response was received from the upstream server", body)
+            assert(proxy_client:close())
+          end)
+        end
+
+        it("#db turn it off, request is allowed", function()
+          local service_tls_id
+          if subsystems == "http" then
+            service_tls_id = service_tls.id
+          else
+            service_tls_id = tls_service_tls.id
+          end
+          local res = assert(admin_client:patch("/services/" .. service_tls_id, {
+            body = {
+              tls_verify = false,
+            },
+            headers = { ["Content-Type"] = "application/json" },
+          }))
+
+          assert.res_status(200, res)
+
+          wait_for_all_config_update(subsystems)
+
+          local path
+          if subsystems == "http" then
+            path = "/tls"
+          else
+            path = "/"
+          end
+
+          helpers.wait_until(function()
+            local proxy_client = get_proxy_client(subsystems, 19001)
+            res = proxy_client:send {
+              path    = path,
+              headers = {
+                ["Host"] = "example.com",
+              }
+            }
+            return pcall(function()
+              local body = assert.res_status(200, res)
+              assert.equals("it works", body)
+              assert(proxy_client:close())
+            end)
+          end, 10)
+
+          -- buffered_proxying
+          if subsystems == "http" then
+            helpers.wait_until(function()
+              local proxy_client = get_proxy_client(subsystems, 19001)
+              res = proxy_client:send {
+                path    = "/tls-buffered-proxying",
+                headers = {
+                  ["Host"] = "example.com",
+                }
+              }
+
+              return pcall(function()
+                local body = assert.res_status(200, res)
+                assert.equals("it works", body)
+                assert(proxy_client:close())
+              end)
+            end, 10)
+          end
+        end)
+      end)
+    end)
+    end
+  end)
+end
+end   -- for flavor