-
Notifications
You must be signed in to change notification settings - Fork 355
Gatekeeper mangles/truncates proxied request, even when encoded #528
Description
Gatekeeper 9.0.2 is mangling requests it proxies in certain circumstances. Specifically, when put in front of Jenkins, it causes Jenkins' reverse proxy validator to fail.
I do not know why Jenkins performs the test the way it does, but it has been around for years and is not going to change. The browser (via AJAX) hits an endpoint. The endpoint redirects the request to another endpoint URL that includes the URL-encoded value of the "Referrer" header it received. The second endpoint validates that the Referrer matches the expected value and returns an error if it does not.
Thus Gatekeeper receives a request header like:
GET /administrativeMonitor/hudson.diagnosis.ReverseProxySetupMonitor/testForReverseProxySetup/https%3A%2F%2Fjenkins.example.com%2Fmanage/
Note the %3A%2F%2F which decdes to :// (colon slash slash).
However, Gatekeeper decodes the request and squishes the double slash into one in the log:
keycloak-gatekeeper/middleware.go:90 client request {..."method": "GET", "
path": "/administrativeMonitor/hudson.diagnosis.ReverseProxySetupMonitor/testForReverseProxySetup/https:/jenkins.example.com/manage/"}
and by the time Jenkins receives it, Jenkins parses the request as
/administrativeMonitor/hudson.diagnosis.ReverseProxySetupMonitor/testForReverseProxySetup/https:
Everything after the colon has been dropped.
Gatekeeper should not be altering the request. Any processing it needs to do on the request to parse it, validate it, apply security rules, etc., should be done on a copy of the request string. The original request string should be forwarded if the request is allowed.
I am guessing that https://github.com/keycloak/keycloak-gatekeeper/blob/d7f64e71bea70dc4023dbd927983962c4e255ac5/middleware.go#L51 is likely the source of the problem.
I do not understand the point of normalizing the URL in the request to begin with, since it can lead to problems like what we see here with Jenkins, but at the very least I think this line replacing the RawPath with the normalized Path should be removed. Likewise, the RawPath and RequestURI should not be replaced with the parsed Path at the end of the function here https://github.com/keycloak/keycloak-gatekeeper/blob/d7f64e71bea70dc4023dbd927983962c4e255ac5/middleware.go#L65-L66