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

Address an issue where a toxiproxy can be used to bypass the Same-Origin Policy in web browsers #184

Merged
merged 2 commits into from
Jun 26, 2017

Conversation

JackMc
Copy link
Contributor

@JackMc JackMc commented Jun 22, 2017

This PR provides a temporary fix to an issue where proxies can be created from the browser.

@JackMc JackMc requested a review from jpittis June 22, 2017 18:14
@JackMc
Copy link
Contributor Author

JackMc commented Jun 22, 2017

Reference for most user agents starting with Mozilla:
screen shot 2017-06-22 at 2 15 30 pm

(Note that the Presto engine is not used in modern browsers)

Copy link
Contributor

@jpittis jpittis left a 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/") {
Copy link
Contributor

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?

Copy link
Contributor Author

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/

Copy link
Contributor Author

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

@jpittis jpittis requested a review from sirupsen June 22, 2017 19:16
Copy link
Contributor

@sirupsen sirupsen left a 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.

@JackMc JackMc merged commit a5f75b4 into master Jun 26, 2017
@EiNSTeiN-
Copy link

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.

@EiNSTeiN- EiNSTeiN- deleted the temp-address-security-issue branch June 27, 2017 13:38
@xthexder
Copy link
Contributor

xthexder commented Jul 1, 2018

I'm curious about what particular scenario this is trying to prevent?
Access from the broswer is nice to have, and by design in the case of this dashboard PR #218

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.
Even if you could create a proxy to access another site, you wouldn't have access to the browser's session still.

Maybe there's some scenario I'm missing? If this is still important we can add some CSRF token stuff to the dashboard PR.

@EiNSTeiN-
Copy link

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.

@JackMc
Copy link
Contributor Author

JackMc commented Jul 1, 2018

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.

@xthexder
Copy link
Contributor

xthexder commented Jul 1, 2018

In that example, how does evil.com access localhost:8474? The very same same-origin policy would block it. Either toxiproxy has to be accessible through evil.com, or the malicious code has to be running on localhost. There's also nothing stopping evil.com from hosting its own proxy via nginx or any other method. I'm still failing to come up with a particular scenario that is a problem.

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.

@EiNSTeiN-
Copy link

The very same same-origin policy would block it.

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)

@xthexder
Copy link
Contributor

xthexder commented Jul 1, 2018

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.

@nsidhaye
Copy link

nsidhaye commented Apr 5, 2021

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.

Invoke-RestMethod http://localhost:8474/proxies

gives following error

Invoke-RestMethod : User agent not allowed
At line:1 char:1
+ Invoke-RestMethod http://localhost:8474/proxies
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : InvalidOperation: (System.Net.HttpWebRequest:HttpWebRequest) [Invoke-RestMethod], WebExc
   eption
    + FullyQualifiedErrorId : WebCmdletWebResponseException,Microsoft.PowerShell.Commands.InvokeRestMethodCommand

One need to pass empty user-agent then only you able to see response.

Invoke-RestMethod -UserAgent "" http://localhost:8474/proxies

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants