fix(clustering): hybrid mode should accept special characters used in…

… forward proxy password (#13457)

Hybrid mode connections not working if the forward proxy password contains
special character(`#`). Even after the `proxy_server` is url encoded, it still
doesn't work and reports a "407 Proxy Authentication Required" error.

This fix tries to address this issue. But, keep in mind that even with this fix,
`proxy_server` still needs to be url-encoded.

changelog/unreleased/kong/fix-clustering-forward-proxy-authentication.yml

@@ -0,0 +1,3 @@
+message: Fixed an issue where hybrid mode not working if the forward proxy password contains special character(#). Note that the `proxy_server` configuration parameter still needs to be url-encoded.
+type: bugfix
+scope: Clustering

kong.conf.default

@@ -199,8 +199,8 @@
                                  # stack traces to help improve Kong.
 
 
-#proxy_server =                  # Proxy server defined as a URL. Kong will only use this
-                                 # option if a component is explicitly configured
+#proxy_server =                  # Proxy server defined as an encoded URL. Kong will only
+                                 # use this option if a component is explicitly configured
                                  # to use a proxy.
 
 
@@ -2167,7 +2167,7 @@
 #   separate namespaces in the `"<name>/<key>"` format.
 #   For using these functions with non-namespaced keys, the Nginx template needs
 #   a `shm_kv *` entry, which can be defined using `nginx_wasm_shm_kv`.
-# - `nginx_wasm_wasmtime_<flag>`: Injects `flag <flag>` into the `wasmtime {}` 
+# - `nginx_wasm_wasmtime_<flag>`: Injects `flag <flag>` into the `wasmtime {}`
 #   block, allowing various Wasmtime-specific flags to be set.
 # - `nginx_<http|proxy>_<directive>`: Injects `<directive>` into the
 #   `http {}` or `server {}` blocks, as specified in the Nginx injected directives

kong/clustering/utils.lua

@@ -9,6 +9,7 @@ local type = type
 local table_insert = table.insert
 local table_concat = table.concat
 local encode_base64 = ngx.encode_base64
+local unescape_uri = ngx.unescape_uri
 local worker_id = ngx.worker.id
 local fmt = string.format
 
@@ -43,14 +44,14 @@ local function parse_proxy_url(proxy_server)
       -- the connection details is statically rendered in nginx template
 
     else -- http
-      ret.proxy_url = fmt("%s://%s:%s", parsed.scheme, parsed.host, parsed.port or 443)
+      ret.proxy_url = fmt("%s://%s:%s", parsed.scheme, unescape_uri(parsed.host), parsed.port or 443)
       ret.scheme = parsed.scheme
-      ret.host = parsed.host
+      ret.host = unescape_uri(parsed.host)
       ret.port = parsed.port
     end
 
     if parsed.user and parsed.password then
-      ret.proxy_authorization = "Basic " .. encode_base64(parsed.user  .. ":" .. parsed.password)
+      ret.proxy_authorization = "Basic " .. encode_base64(unescape_uri(parsed.user)  .. ":" .. unescape_uri(parsed.password))
     end
   end
 

spec/01-unit/03-conf_loader_spec.lua

@@ -1173,6 +1173,18 @@ describe("Configuration loader", function()
           assert.is_nil(errors)
           assert.is_table(conf)
 
+          local conf, _, errors = conf_loader(nil, {
+            proxy_server = "http://😉.tld",
+          })
+          assert.is_nil(errors)
+          assert.is_table(conf)
+
+          local conf, _, errors = conf_loader(nil, {
+            proxy_server = "http://%F0%9F%98%89.tld",
+          })
+          assert.is_nil(errors)
+          assert.is_table(conf)
+
           local conf, _, errors = conf_loader(nil, {
             proxy_server = "://localhost:2333",
           })
@@ -1198,6 +1210,18 @@ describe("Configuration loader", function()
           })
           assert.contains("fragments, query strings or parameters are meaningless in proxy configuration", errors)
           assert.is_nil(conf)
+
+          local conf, _, errors = conf_loader(nil, {
+            proxy_server = "http://user:password%23@localhost:2333",
+          })
+          assert.is_nil(errors)
+          assert.is_table(conf)
+
+          local conf, _, errors = conf_loader(nil, {
+            proxy_server = "http://user:password#@localhost:2333",
+          })
+          assert.contains("fragments, query strings or parameters are meaningless in proxy configuration", errors)
+          assert.is_nil(conf)
         end)
 
         it("doesn't allow cluster_use_proxy on CP but allows on DP", function()

spec/02-integration/09-hybrid_mode/10-forward-proxy_spec.lua

@@ -34,7 +34,7 @@ local fixtures = {
 
       content_by_lua_block {
         require("spec.fixtures.forward-proxy-server").connect({
-          basic_auth = ngx.encode_base64("test:konghq"),
+          basic_auth = ngx.encode_base64("test:konghq#"),
         })
       }
     }
@@ -49,15 +49,15 @@ local proxy_configs = {
     proxy_server_ssl_verify = "off",
   },
   ["https off auth on"] = {
-    proxy_server = "http://test:konghq@127.0.0.1:16796",
+    proxy_server = "http://test:konghq%23@127.0.0.1:16796",
     proxy_server_ssl_verify = "off",
   },
   ["https on auth off"] = {
     proxy_server = "https://127.0.0.1:16799",
     proxy_server_ssl_verify = "off",
   },
   ["https on auth on"] = {
-    proxy_server = "https://test:konghq@127.0.0.1:16798",
+    proxy_server = "https://test:konghq%23@127.0.0.1:16798",
     proxy_server_ssl_verify = "off",
   },
   ["https on auth off verify on"] = {