kong icon indicating copy to clipboard operation
kong copied to clipboard

nginx variable "merge_slashes" does not work for Kong

Open alokrbl opened this issue 1 year ago • 10 comments

Is there an existing issue for this?

  • [X] I have searched the existing issues

Kong version ($ kong version)

Kong 2.8.3 and Kong 3.1.1

Current Behavior

We have an API which is an OCSP responder that accepts both POST and GET methods. We have an issue with the GET method when the OCSP request URL contains multiple successive slashes.

URL is in this format : 'GET {url}/{url-encoding of base-64 encoding of the DER encoding of the OCSPRequest}'

By default, Kong is merging those multiple slashes.

We referred below mentioned links and added 2 environment variables to disable it ( as value "off") and deployed kong ( tried adding either one at a time and both too)

• KONG_NGINX_HTTP_MERGE_SLASHES ( for http context) • KONG_NGINX_PROXY_MERGE_SLASHES ( for server context)

above env variables are added correctly in /opt/app/var/nginx-kong.conf but looks like kong is not respecting this and sending this with merging the slash.

Doc links Referred: https://docs.konghq.com/gateway/latest/reference/configuration/#nginx-injected-directives-section https://nginx.org/en/docs/http/ngx_http_core_module.html#merge_slashes

Expected Behavior

Slashes used in uri should not be merged.

Steps To Reproduce

use the url with slashes with base64 encoding.

  • received url value: "/ejbca/publicweb/status/ocspMHoweDBRME8wTTAJBgUrDgMCGgUABBQNw3RqyC6xHUNZEnsPwJ74U6x+LgQUoxGE7mYCotQBEo+9BE3//vQ7pDYCFAC5lBCKmOU5fesTX5vnnmwfE5N6oiMwITAfBgkrBgEFBQcwAQIEEgQQz6Ap6eUOHOS8ethetqUC7Q=="

  • sent url value: "/ejbca/publicweb/status/ocsp/MHoweDBRME8wTTAJBgUrDgMCGgUABBQNw3RqyC6xHUNZEnsPwJ74U6x+LgQUoxGE7mYCotQBEo+9BE3/vQ7pDYCFAC5lBCKmOU5fesTX5vnnmwfE5N6oiMwITAfBgkrBgEFBQcwAQIEEgQQz6Ap6eUOHOS8ethetqUC7Q=="

Anything else?

No response

alokrbl avatar Oct 24 '24 18:10 alokrbl

Kong matches the URL and proxies it with the normalized URL which always merges slashes. This is expected for not respecting this nginx directive. Currently, there is no option to bypass it.

Oyami-Srk avatar Oct 25 '24 02:10 Oyami-Srk

Is it possible to url encode the token before it is put into path?

bungle avatar Oct 28 '24 09:10 bungle

I'd like to suggest passing base64 as a part of the URL should be done with URL encoding.

According to RFC2396, slash is a reserved character and must be escaped.

Although HTTP RFC requires that a transparent proxy must not rewrite the path of the request URI, Kong is not a transparent proxy.

Personally, I don't think passing Base64 by URL is a good idea. For example, Base64 is case-sensitive, and HTTP RFC doesn't require any case sensitivity of URLs. Although most software doesn't normalize URLs to lower-case or upper-case, it can be a problem if you want to apply some middleware that unfortunately does that.

I'll close this issue for now. Feel free to reopen it if you have any other concerns.

Oyami-Srk avatar Oct 29 '24 02:10 Oyami-Srk

Hello @Oyami-Srk ,

Could you please explain what is for you "must be escaped" ? Sorry to ask that but did some tests and I can't have it working even when I have the double slashes "//" replaced by their url encoded value "%2F%2F". Result is that Kong replace the url encoded values by a simple slash when forwarding request URL to the upstream server.

Maybe I missed a step or did not correctly understood your answer. Please help to have this topic fixed even when using Kong for OCSP responder. For more details about why we try to make it work with GET method you can have a look on the RFC 6960

Regards

younid avatar Nov 14 '24 18:11 younid

Ok, I thought escaping to %2F%2F should work and sending %2F%2F to upstream, but seems like it didn't. One mitigation is passing base64 by body or parameter after ?. Feel free to reopen this issue if you want. Thanks.

Oyami-Srk avatar Nov 18 '24 08:11 Oyami-Srk

Hello @Oyami-Srk ,

I tried to reopen this issue but I did managed to find any option for that. Could you please tell me how I should do ?

Regards, Youness

younid avatar Nov 19 '24 07:11 younid

I'll reopen this for you. Overall, Kong is not a transparent proxy and will always normalize the URI according to the documentation. But the doc didn't clarify that how percent-encoded triplets of reserved characters are being treated. It could be designed behavior but undocumented. I'm not very sure about that. Could you please take a look into that? @bungle

Oyami-Srk avatar Nov 19 '24 09:11 Oyami-Srk

@younid,

can you show your route and service configuration?

bungle avatar Feb 05 '25 13:02 bungle

At first Kong tried to be transparent regards to urls (https://github.com/Kong/kong/issues/2366#issuecomment-293081846). But then people reported that in some cases it may lead to nasty security issues. E.g. you have configured service with path /sandbox but incoming url is /../../../not-a-sandbox, which lead Kong to proxy to /sandbox/../../../not-a-sandbox. Of course the right way to do it is to normalize the incoming url before concatenating it with the service path. Later on we started to normalize the request paths.

That said, I do not remember why we decided to merge slashes. It is usually ok, but not always. Cloudflare merges slashes too by default (https://developers.cloudflare.com/rules/normalization/how-it-works/#cloudflare-normalization).

I think it would be better to not merge slashes, but this has been there now a quite a while. We could add an option, but should we default to not merge (may be breaking) or keep current one (may require many to change the default). And what is granularity of such setting.

I would love to just use this: https://github.com/bungle/lua-resty-ada

for our url normalization and encoding needs.

It is just that this change may have side effects. When we added kong.request.clear_query_arg, we used libada to do it. There are now reports that Ada encodes spaces as + in query string (which is correct), but some people's upstream cannot handle it and they rather want %20 (which is also correct, but less common, e.g. browsers and many standard libs use +, at least by default). So sometimes even if we try to do something the "correct" way, different things start to happen. Thus it is quite a painful to touch this. Then we are left with adding options and config parameters. Which also has downsides.

bungle avatar Feb 05 '25 13:02 bungle

Hello @bungle,

I don't have any sample to share but there's nothing special in routes configuration. I just need to disable this merge slashes mecanisme using the parameters that Nginx already provides: • KONG_NGINX_HTTP_MERGE_SLASHES ( for http context) • KONG_NGINX_PROXY_MERGE_SLASHES ( for server context)

As you said, it should be kept enabled by default for retro compatibility and just allow people who need to disable it. Doing it lke that should not have any impact on behavior or security because it still activated by default.

Just for example, you can that Apache has its own directive, MergeSlashes, allowing change for this merge slashes and its default value is true. https://httpd.apache.org/docs/2.4/mod/core.html#mergeslashes

Nginx has its own parameter, merge_slashes, with default value at true as well: https://nginx.org/en/docs/http/ngx_http_core_module.html#merge_slashes

Regards

younid avatar Feb 08 '25 09:02 younid