-
Notifications
You must be signed in to change notification settings - Fork 454
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
Address an issue where a toxiproxy can be used to bypass the Same-Origin Policy in web browsers #184
Conversation
…gin Policy in web browsers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you add a quick test for the StopBrowsersMiddleware
?
@@ -46,6 +47,16 @@ func (server *ApiServer) PopulateConfig(filename string) { | |||
} | |||
} | |||
|
|||
func StopBrowsersMiddleware(h http.Handler) http.Handler { | |||
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |||
if strings.HasPrefix(r.UserAgent(), "Mozilla/") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is matching against browser user agents that simple?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to MDN, all the common browsers' user agents start with Mozilla/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
http://webaim.org/blog/user-agent-string-history/ User agents have a sad history of pretending to be each other
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this the standard way to solve such issues? This seems kind of.. awkward? No concerns otherwise.
The standard way to solve this issue would be to require clients to provide a custom HTTP header. This can't be done by browsers (it requires CORS, which wouldn't be allowed in this case) so it would prove that the request comes from a client that isn't a browser. |
I'm curious about what particular scenario this is trying to prevent? You can only create and connect to proxies from the same machine running toxiproxy, which means there would have to be malicious code running on localhost (or I guess the test CI server). Toxiproxy shouldn't be running in production environments, and even if it was, it definitely shouldn't be publicly accessible. Maybe there's some scenario I'm missing? If this is still important we can add some CSRF token stuff to the dashboard PR. |
Same-Origin policy is one of the cornerstone of browser security. Browsers enforce that a website example.com can only read data from itself, not from google.com. For obvious reasons bypassing the same-origin policy would be a major problem. Toxiproxy doesn't know which domain name it is served from, so IIRC this PR aimed at preventing browsers from connecting to it. Without this, a dev running a local toxiproxy visiting a random evil.com site could get its proxy configuration modified by evil.com to access any local resources that the developer has access to on the local network. |
Hi @xthexder! Thanks for the comment. As @EiNSTeiN- said, this was introduced to stop violations of the same origin policy on developer machines through abuse of Toxiproxy. You're correct that this could be fixed by introducing a CSRF token, but the problem there is that it would require modifications to several third party clients and a breaking change. Therefore, we decided to go with this approach at least in the short term. |
In that example, how does If we decide this needs to be kept, we can check CSRF only if the request is coming from a browser, which doesn't break client compatibility. |
DNS Rebinding can be used when the server does not validate the domain name used to connect to it (which is the case for toxiproxy AFAIK) |
Ah yeah, that's the loophole I was looking for. I guess browser access is staying off until an auth token of some sort is added. Maybe in the next major version rev we can fix this properly with updates to the clients. |
Already commented on #219 while adding here so users can able to see some work-around. It's not only browser but if I use powershell rest command then also I need to override user-agent.
gives following error
One need to pass empty user-agent then only you able to see response.
|
This PR provides a temporary fix to an issue where proxies can be created from the browser.