Skip to content
Permalink
Browse files Browse the repository at this point in the history
Merge pull request from GHSA-gcv9-6737-pjqw
validate proxied paths starting with /
  • Loading branch information
manics committed Jan 24, 2022
2 parents 8610a3b + 61bad15 commit fd31930
Show file tree
Hide file tree
Showing 3 changed files with 97 additions and 48 deletions.
22 changes: 22 additions & 0 deletions CHANGELOG.md
@@ -1,5 +1,27 @@
## 3.2

### 3.2.1 - 2022-01-24

3.2.1 is a security release, fixing a vulnerability [GHSA-gcv9-6737-pjqw](https://github.com/jupyterhub/jupyter-server-proxy/security/advisories/GHSA-gcv9-6737-pjqw) where `allowed_hosts` were not validated correctly.

## Maintenance and upkeep improvements

- Modernize docs without making changes to its content [#313](https://github.com/jupyterhub/jupyter-server-proxy/pull/313) ([@consideRatio](https://github.com/consideRatio))
- Remove no longer needed logic involving six [#312](https://github.com/jupyterhub/jupyter-server-proxy/pull/312) ([@consideRatio](https://github.com/consideRatio))
- Update language, from master to main [#311](https://github.com/jupyterhub/jupyter-server-proxy/pull/311) ([@consideRatio](https://github.com/consideRatio))

## Other merged PRs

- Remove empty JupyterLab style [#314](https://github.com/jupyterhub/jupyter-server-proxy/pull/314) ([@bollwyvl](https://github.com/bollwyvl))
- ci: avoid triggering ci twice on pre-commit.ci/dependabot prs [#310](https://github.com/jupyterhub/jupyter-server-proxy/pull/310) ([@consideRatio](https://github.com/consideRatio))

## Contributors to this release

([GitHub contributors page for this release](https://github.com/jupyterhub/jupyter-server-proxy/graphs/contributors?from=2021-11-29&to=2022-01-19&type=c))

[@bollwyvl](https://github.com/search?q=repo%3Ajupyterhub%2Fjupyter-server-proxy+involves%3Abollwyvl+updated%3A2021-11-29..2022-01-19&type=Issues) | [@consideRatio](https://github.com/search?q=repo%3Ajupyterhub%2Fjupyter-server-proxy+involves%3AconsideRatio+updated%3A2021-11-29..2022-01-19&type=Issues) | [@welcome](https://github.com/search?q=repo%3Ajupyterhub%2Fjupyter-server-proxy+involves%3Awelcome+updated%3A2021-11-29..2022-01-19&type=Issues) | [@yuvipanda](https://github.com/search?q=repo%3Ajupyterhub%2Fjupyter-server-proxy+involves%3Ayuvipanda+updated%3A2021-11-29..2022-01-19&type=Issues)


### 3.2.0 - 2021-11-29

#### New features added
Expand Down
105 changes: 57 additions & 48 deletions jupyter_server_proxy/handlers.py
Expand Up @@ -209,12 +209,16 @@ def _get_context_path(self, host, port):
return url_path_join(self.base_url, 'proxy', host_and_port)

def get_client_uri(self, protocol, host, port, proxied_path):
context_path = self._get_context_path(host, port)
if self.absolute_url:
context_path = self._get_context_path(host, port)
client_path = url_path_join(context_path, proxied_path)
else:
client_path = proxied_path

# ensure client_path always starts with '/'
if not client_path.startswith("/"):
client_path = "/" + client_path

# Quote spaces, åäö and such, but only enough to send a valid web
# request onwards. To do this, we mark the RFC 3986 specs' "reserved"
# and "un-reserved" characters as safe that won't need quoting. The
Expand All @@ -228,7 +232,7 @@ def get_client_uri(self, protocol, host, port, proxied_path):
protocol=protocol,
host=host,
port=port,
path=client_path
path=client_path,
)
if self.request.query:
client_uri += '?' + self.request.query
Expand Down Expand Up @@ -297,13 +301,14 @@ async def proxy(self, host, port, proxied_path):
client = httpclient.AsyncHTTPClient()

req = self._build_proxy_request(host, port, proxied_path, body)
self.log.debug(f"Proxying request to {req.url}")

try:
# Here, "response" is a tornado.httpclient.HTTPResponse object.
response = await client.fetch(req, raise_error=False)
except httpclient.HTTPError as err:
# We need to capture the timeout error even with raise_error=False,
# because it only affects the HTTPError raised when a non-200 response
# because it only affects the HTTPError raised when a non-200 response
# code is used, instead of suppressing all errors.
# Ref: https://www.tornadoweb.org/en/stable/httpclient.html#tornado.httpclient.AsyncHTTPClient.fetch
if err.code == 599:
Expand All @@ -324,7 +329,7 @@ async def proxy(self, host, port, proxied_path):
else:
# Represent the original response as a RewritableResponse object.
original_response = RewritableResponse(orig_response=response)

# The function (or list of functions) which should be applied to modify the
# response.
rewrite_response = self.rewrite_response
Expand Down Expand Up @@ -688,53 +693,57 @@ def options(self, path):
def setup_handlers(web_app, serverproxy_config):
host_allowlist = serverproxy_config.host_allowlist
rewrite_response = serverproxy_config.non_service_rewrite_response
web_app.add_handlers('.*', [
(
url_path_join(
web_app.settings['base_url'],
r'/proxy/([^/]*):(\d+)(.*)',
web_app.add_handlers(
".*",
[
(
url_path_join(
web_app.settings["base_url"],
r"/proxy/([^/:@]+):(\d+)(/.*|)",
),
RemoteProxyHandler,
{
"absolute_url": False,
"host_allowlist": host_allowlist,
"rewrite_response": rewrite_response,
},
),
RemoteProxyHandler,
{
'absolute_url': False,
'host_allowlist': host_allowlist,
'rewrite_response': rewrite_response,
}
),
(
url_path_join(
web_app.settings['base_url'],
r'/proxy/absolute/([^/]*):(\d+)(.*)',
(
url_path_join(
web_app.settings["base_url"],
r"/proxy/absolute/([^/:@]+):(\d+)(/.*|)",
),
RemoteProxyHandler,
{
"absolute_url": True,
"host_allowlist": host_allowlist,
"rewrite_response": rewrite_response,
},
),
RemoteProxyHandler,
{
'absolute_url': True,
'host_allowlist': host_allowlist,
'rewrite_response': rewrite_response,
}
),
(
url_path_join(
web_app.settings['base_url'],
r'/proxy/(\d+)(.*)',
(
url_path_join(
web_app.settings["base_url"],
r"/proxy/(\d+)(/.*|)",
),
LocalProxyHandler,
{
"absolute_url": False,
"rewrite_response": rewrite_response,
},
),
LocalProxyHandler,
{
'absolute_url': False,
'rewrite_response': rewrite_response,
},
),
(
url_path_join(
web_app.settings['base_url'],
r'/proxy/absolute/(\d+)(.*)',
(
url_path_join(
web_app.settings["base_url"],
r"/proxy/absolute/(\d+)(/.*|)",
),
LocalProxyHandler,
{
"absolute_url": True,
"rewrite_response": rewrite_response,
},
),
LocalProxyHandler,
{
'absolute_url': True,
'rewrite_response': rewrite_response,
},
),
])
],
)


# vim: set et ts=4 sw=4:
18 changes: 18 additions & 0 deletions tests/test_proxies.py
Expand Up @@ -291,3 +291,21 @@ async def _websocket_subprotocols():
def test_server_proxy_websocket_subprotocols(event_loop):
event_loop.run_until_complete(_websocket_subprotocols())

@pytest.mark.parametrize(
"proxy_path, status",
[
("127.0.0.1", 404),
("127.0.0.1/path", 404),
("127.0.0.1@192.168.1.1", 404),
("127.0.0.1@192.168.1.1/path", 404),
("user:pass@host:123/foo", 403),
("user:pass@host/foo", 404),
("absolute/127.0.0.1:123@192.168.1.1/path", 404),
]
)
def test_bad_server_proxy_url(proxy_path, status):
r = request_get(PORT, f"/proxy/{proxy_path}", TOKEN)
assert r.code == status
if status >= 400:
# request should not have been proxied
assert 'X-ProxyContextPath' not in r.headers

0 comments on commit fd31930

Please sign in to comment.