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

Bug 1622372 - Require CSRF token on all proxied requests #461

Merged
merged 1 commit into from Aug 27, 2018

Conversation

spadgett
Copy link
Member

  • Require a CSRF token on all proxied requests. This prevents loading
    content hosted from a pod under the console domain by clicking on a
    link that uses the console proxy. Previously, it was not required for
    GET requests.
  • Do not forward the X-CSRFToken header through the proxy.
  • Set Content-Security-Policy: default-src 'none' in the proxied
    response to prevent scripts from running in proxied content.

In order to support the CSRF token for WebSockets, this adds an
x-csrf-token query parameter when headers can't be set. It also updates
the console to check the Origin header when present since Referer is
not set for WebSockets.

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1622372

/assign @liggitt

* Require a CSRF token on all proxied requests. This prevents loading
  content hosted from a pod under the console domain by clicking on a
  link that uses the console proxy. Previously, it was not required for
  GET requests.
* Do not forward the X-CSRFToken header through the proxy.
* Set `Content-Security-Policy: default-src 'none'` in the proxied
  response to prevent scripts from running in proxied content.

In order to support the CSRF token for WebSockets, this adds an
`x-csrf-token` query parameter when headers can't be set. It also updates
the console to check the `Origin` header when present since `Referer` is
not set for WebSockets.

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1622372
@openshift-ci-robot openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 27, 2018
@liggitt
Copy link

liggitt commented Aug 27, 2018

lgtm

@spadgett spadgett changed the title [WIP] Bug 1622372 - Require CSRF token on all proxied requests Bug 1622372 - Require CSRF token on all proxied requests Aug 27, 2018
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 27, 2018
@spadgett spadgett added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. lgtm Indicates that a PR is ready to be merged. and removed do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Aug 27, 2018
@openshift-merge-robot openshift-merge-robot merged commit d566668 into openshift:master Aug 27, 2018
@spadgett spadgett deleted the proxy branch August 27, 2018 19:50
@SSshahan
Copy link
Contributor

SSshahan commented Sep 6, 2018

@spadgett one quick question: how does we including csrf-token into proxied requests to perform GET requests? It can get application content with web console proxy from terminal like: curl -k -X GET https://<master>/api/v1/namespaces/hasha-pro1/services/ruby-ex:8080-tcp/proxy/ --header "Authorization: Bearer l22bZp9lMQ5Vr6PpfuHIjqu-tvzWLr4dyMjcPNahWVI".
I tried with CSRF token on the admin console but get nothing output: http://pastebin.test.redhat.com/641927. The CSRF token copied from Storage/Cookie value on the admin console develop windows. I am not clear about it yet, could you point me how to make it works? Thanks!

@SSshahan
Copy link
Contributor

@spadgett bump, in case you miss the above question. Thank you.

@spadgett
Copy link
Member Author

spadgett commented Sep 10, 2018

@SSshahan You need to include both the X-CSRFToken and Authorization headers in the request. I don't see Authorization in the pastebin. You should try curl with -v for verbose output.

edit: This should actually be a cookie not the authorization header... (Getting my consoles mixed up.)

@SSshahan
Copy link
Contributor

@spadgett thanks, but get 401 Unauthorized error with user and X-CSRFToken setting. The X-CSRFToken comes from the csrf-token value in the Storage/Cookie by enter F12. http://pastebin.test.redhat.com/644276. Any other places with the wrong thing?

@spadgett
Copy link
Member Author

Right, it's expected since the request doesn't have the two cookies. You should include the csrf-token and openshift-session-token cookies in the request.

I'd recommend using "Copy -> Copy as cURL" from the Network tab of Chrome developer tools.

@SSshahan
Copy link
Contributor

@spadgett It works finally, copy as cURL is a good approach. Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants