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

REVEL CSRF: Same origin mismatch. #83

Closed
ptman opened this issue Sep 27, 2018 · 14 comments
Closed

REVEL CSRF: Same origin mismatch. #83

ptman opened this issue Sep 27, 2018 · 14 comments
Assignees

Comments

@ptman
Copy link
Contributor

ptman commented Sep 27, 2018

I'm not sure if it's because of running behind a proxy or not, but with some logging just after

isSameOrigin := sameOrigin(c.Request.URL, referer)
:

sameorigin=false referer=https://dev.carecode.net/ url=/

Clearly the test will fail with those inputs:

	return u1.Scheme == u2.Scheme && u1.Host == u2.Host

Something isn't working. I see that the logic is a bit different from cbonello/revel-csrf

@ptman
Copy link
Contributor Author

ptman commented Oct 2, 2018

Is something stripping out the Host -header? Logging c.Request.Header.Get("Host") gives me nothing. But firefox claims the header is set.

@ptman
Copy link
Contributor Author

ptman commented Oct 16, 2018

I'm still getting this:

Server Error:
Forbidden: REVEL CSRF: Same origin mismatch.

I prepared a test revel project: https://github.com/ptman/revel-csrf-test , please guide me to fix it

@notzippy notzippy mentioned this issue Oct 18, 2018
@notzippy notzippy self-assigned this Oct 18, 2018
@notzippy
Copy link
Contributor

One other thing with your test. The form value posted must be csrftoken (not _csrftoken) and you need to have a referrer page to start from. So you need to start at page A, have a link to page B (which has the form) and final post the result to page C and you should be good to go

@ptman
Copy link
Contributor Author

ptman commented Oct 19, 2018

A -> B -> C sounds nonsensical. Why couldn't the landing page include a form that has csrf-protection? You don't need to have referer & token to show the form (GET), only to process the form (POST).

@golddranks
Copy link

Hi, sorry to butt in. I just tested the code on the development branch. The "request URL fixing" ( https://github.com/revel/modules/blob/develop/csrf/app/csrf.go#L66 ) that adds http:// or https:// to the request URL breaks some things: it breaks relative URLs, changing stuff like /login/ to http:///login/. Because the URL is dispatched at this point, this doesn't have a direct effect to routing, but for example, when doing redirects, it makes the location header behave oddly with relative URLs, stuff like Location: http:///

@notzippy
Copy link
Contributor

Hi @golddranks the issue you seem to be having is that the hostname is missing from the request, then it would compact in that manner, the missing hostname may be due to revel running behind a proxy. Is that your setup ?

@golddranks
Copy link

I have revel running inside a container locally and behind a load balancer in AWS. Both have the same behaviour: revel.Controller.Request.URL is relative.

I tried to run revel directly without container on localhost, but it still behaves the same.

@golddranks
Copy link

golddranks commented Oct 23, 2018

Looking at the code (EDIT, here: https://github.com/revel/revel/blob/master/server_adapter_go.go#L241 ), I don't find that behaviour surprising: https://stackoverflow.com/questions/23151827/how-to-get-url-in-http-request

So it seems that the current way of "fixing" the URL is broken.

@notzippy
Copy link
Contributor

Updated PR #91 , specifically this section which should cover most cases where the hostname is unknown. @golddranks let me know if this is still an issue

@ptman
Copy link
Contributor Author

ptman commented Oct 25, 2018

@notzippy Much better, thank you.

@golddranks Does it work behind a proxy for you?

@ptman
Copy link
Contributor Author

ptman commented Nov 1, 2018

Tests seem to be failing for me. And also running behind the proxy started by revel run.

DEBUG 13:26:23    app   csrf.go:122: Using                                    request host=0.0.0.0:44917 cookie domain=  path=/login/password request url host= namespace=App\\  ip=127.0.0.1 method=GET action=App.LoginPassword
DEBUG 13:26:23    app   csrf.go:161: getFullRequestURL                          ip=127.0.0.1 path=/login/password method=GET action=App.LoginPassword namespace=App\\ requesturl=http://0.0.0.0:44917

@ptman
Copy link
Contributor Author

ptman commented Nov 1, 2018

Do I have to rewrite all tests in order to keep the csrf filter happy? https://github.com/ptman/revel-csrf-test/commit/f069d3d0d600e67f42927cc429462fff890f88a3

@ptman
Copy link
Contributor Author

ptman commented Nov 1, 2018

Maybe add helpers that take care of the csrf-specific bits during tests?

@notzippy
Copy link
Contributor

notzippy commented Nov 4, 2018

Will create a new issue on that

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

No branches or pull requests

3 participants