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

Admin access to users server lead to 403 (workaround exist) #1700

Closed
0nebody opened this issue Jun 26, 2020 · 7 comments
Closed

Admin access to users server lead to 403 (workaround exist) #1700

0nebody opened this issue Jun 26, 2020 · 7 comments
Labels

Comments

@0nebody
Copy link

0nebody commented Jun 26, 2020

Bug description

As an admin when I attempt to access another users notebook I receive a 403 error.

Expected behaviour

I expect to be able to access the users notebook.

Actual behaviour

I am redirected to a 403 page.

How to reproduce

  1. Go to admin.
  2. Click on access server for another user.
  3. See error

Your personal set up

This occurs when when proxy.https.type is set to offload and a load balancer is responsible for SSL termination.

[E 2020-06-26 00:19:01.177 JupyterHub auth:280] OAuth POST from https://domain.com/hub/api/oauth2/authorize?client_id=jupyterhub-user-test&redirect_uri=%2Fuser%2Ftest%2Foauth_callback&response_type=code&state=asdf != http://domain.com/hub/api/oauth2/authorize?client_id=jupyterhub-user-test&redirect_uri=%2Fuser%2Ftest%2Foauth_callback&response_type=code&state=asdf
[W 2020-06-26 00:19:01.178 JupyterHub web:1786] 403 POST /hub/api/oauth2/authorize?client_id=jupyterhub-user-test&redirect_uri=%2Fuser%2Ftest%2Foauth_callback&response_type=code&state=asdf (10.23.101.226): Authorization form must be sent from authorization page

The headers sent from the proxy to the container are:

x-forwarded-port=443,80
x-forwarded-proto=https,http

When HTTPS is offloaded the proxy should not attempt to add x-forward headers. Adding --no-x-forward flag to the proxy fixes the issue.

@0nebody 0nebody added the bug label Jun 26, 2020
@welcome
Copy link

welcome bot commented Jun 26, 2020

Thank you for opening your first issue in this project! Engagement like this is essential for open source projects! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please try to follow the issue template as it helps other other community members to contribute more effectively.
welcome
You can meet the other Jovyans by joining our Discourse forum. There is also an intro thread there where you can stop by and say Hi! 👋

Welcome to the Jupyter community! 🎉

@duongnt
Copy link
Contributor

duongnt commented Jun 28, 2020

@0nebody this is discussed in jupyterhub/jupyterhub#2284, you can find some solutions in the comments.

@consideRatio
Copy link
Member

consideRatio commented Oct 8, 2020

In #1813 I've added a configuration option allowing us to influence the ConfigurableHTTPProxy running in the proxy pod which can be used to workaround this temporarily when merged.

I don't understand the HTTP Headers well enough x-forwarded-port to properly evaluate what makes sense in general =/ But, @0nebody you saying that if the CHP proxy receive already TLS terminated HTTPS traffic which is now just HTTP, it should when it proxies it onwards always be configured with --no-x-forward? This is a very easy PR to make using proxy.https.type=offload.

@manics, this relates to our discussion in #1811! I don't understand the HTTP header well enough to really grasp this yet though.

@manics
Copy link
Member

manics commented Oct 8, 2020

This looks like a bug to me. X-Forwarded-Proto indicates the protocol used for the original request, so it doesn't make sense for there to be both https,http
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-Proto

Example:

  1. browser makes a https request to a frontend nginx proxy
  2. nginx forwards that request using http to CHP
  3. CHP forwards that request using http to JupyterHub.

In this situation Nginx should be configured to add a X-Forwarded-Proto: https header, and CHP should be configured to pass it through unchanged. Note that CHP should not automatically pass this header through unless you trust the front-end proxy to correctly set the header (otherwise an attacker can make a HTTP request but spoof the header to make JupyterHub think it's HTTPS).

In the case where CHP is directly receiving traffic:

  1. browser makes a http request to CHP
  2. CHP forwards that request using http to JupyterHub.

CHP should add a X-Forwarded-Proto: http header.

A single HTTP request can only be made with one protocol, so what's causing CHP to pass https,http? Same applies to port, you only talk to one port.

@manics
Copy link
Member

manics commented Oct 15, 2020

The http-proxy library in CHP concatenates headers:
https://github.com/http-party/node-http-proxy/blob/9b96cd725127a024dabebec6c7ea8c807272223d/lib/http-proxy/passes/web-incoming.js#L78-L83

    ['for', 'port', 'proto'].forEach(function(header) {
      req.headers['x-forwarded-' + header] =
        (req.headers['x-forwarded-' + header] || '') +
        (req.headers['x-forwarded-' + header] ? ',' : '') +
        values[header];
    });

This makes sense for the X-Forwarded-For header which indicates a chain of proxies.

I was under the impression that X-Forwarded-Proto should be the original protocol used by the browser i.e. the first proxy. The Mozilla docs only have a header with one value, but they don't explicitly say that you can't have multiple.

Then there's this relevant Tornado PR: tornadoweb/tornado#2162

@minrk
Copy link
Member

minrk commented Nov 10, 2020

The upstream issue is fixed in jupyterhub/jupyterhub#2284 . I think we can get away with not dealing with the peculiarities of multiple forwarded-headers here.

@consideRatio consideRatio changed the title 403 when accessing another users notebook Admin access to users server lead to 403 (workaround exist) Dec 12, 2020
@consideRatio
Copy link
Member

consideRatio commented Jan 16, 2021

As the upstream issue is fixed and we have bumped to version 1.3.0 (or earlier?) that contains that fix, I'll go ahead and close this issue.

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

No branches or pull requests

5 participants