Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proxy attempts to normalize encoded slash by issuing http 301 redirect #175

Closed
AndrewDryga opened this Issue Apr 4, 2019 · 6 comments

Comments

Projects
None yet
2 participants
@AndrewDryga
Copy link

AndrewDryga commented Apr 4, 2019

Describe the bug
We have a RabbitMQ management running behind SSO proxy, but we noticed that some pages would not work behind proxy versus using kubectl port forward.

I believe the main reason is that SSO proxy attempts to normalize request path, initially RabbitMQ issues request to: https://foo/api/exchanges/%2F/amq.direct?msg_rates_age=3600&msg_rates_incr=60 (notice /%2F/ in URL which can be accidentally read by proxy as /// and normalized to /).

Then proxy issues a HTTP 301 redirect to /api/exchanges/amq.direct?msg_rates_age=3600&msg_rates_incr=60. Notice that /%2F/ is missing here.

This clearly does not happen when connecting to management UI via port forward.

Expected behavior
Proxy should forward requests as-is without trying to decode and normalize path.

@AndrewDryga

This comment has been minimized.

Copy link
Author

AndrewDryga commented Apr 4, 2019

I think to test it you can setup any page with /%2F/ in path and open it in any browser.

Our upstream service definition is using the default simple type (not rewrite).

@jphines

This comment has been minimized.

Copy link
Contributor

jphines commented Apr 4, 2019

👋 I can confirm this behavior in SSO and have at least validated is not being added by the stdlib itself. I need to do a little more digging before I would feel comfortable triaging this as a "feature" or a "bug".

@AndrewDryga

This comment has been minimized.

Copy link
Author

AndrewDryga commented Apr 4, 2019

Looks more like a bug to me as it breaks our stuff 😃.

@jphines jphines added the wontfix label Apr 4, 2019

@jphines

This comment has been minimized.

Copy link
Contributor

jphines commented Apr 4, 2019

Actually, after looking into the SSO repo a bit more, I couldn't find anything that would be contributing to this behavior.

After more investigation, this is happening from the Go's standard libraries ServeMux implementation. https://golang.org/pkg/net/http/#ServeMux

See a full example here: https://play.golang.org/p/H2Br99jNcwT

And see a more explicit example shows this is coming from the way the Go standard library parses the request, the mux cleans the URL path, and the way the redirect handler parses and re-cleans the url.

Condensed Example: https://play.golang.org/p/Lo5Xh88mZbR

To "fix", we'd have to drop support for Go's standard ServeMux and be very sensitive of our URL handling to ensure that these potentially malformed URLs are proxied correctly.

This might be a potential path in the future, but for now, this seems to be working as intended. I'd suggest opening an issue against golang itself to see if you can update the standard library logic, or against rabbitmq to change these problematic URLs.

@jphines jphines closed this Apr 4, 2019

@AndrewDryga

This comment has been minimized.

Copy link
Author

AndrewDryga commented Apr 4, 2019

@jphines do you have ideas how we could workaround this for time being? Maybe some configuration change would disable that routine for certain paths? We are okay to disable authorization for certain paths as RabbitMQ is password protected by itself.

RabbitMQ development team is known in terms of their "maintenance" and they won't ever consider doing this change. Going trough Go would be a really hard path including we don't have Go developers able to contribute this PR and this project would need to update to the latest Go version even if it gets through.

@AndrewDryga

This comment has been minimized.

Copy link
Author

AndrewDryga commented Apr 4, 2019

Also, golang/go#19481 so go path is not an option too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.