mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-10-06 23:35:08 +00:00
fix(web): origin guard accepts an https page behind a TLS proxy; log only the site
- A portless Host now matches the default port of either the browser's scheme or Flask's, so nginx terminating TLS in front of a plain-http upstream (Origin https://pi.example -> 443, Flask sees http -> 80) no longer refuses every legitimate write. A non-default port still has to match exactly. - The refusal log records only scheme://host[:port] of Origin/Referer, never a Referer's path or query (which can carry tokens), and repr()s the path. - Docs: forward $http_host, not $host (nginx's $host drops the port). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
+4
-2
@@ -38,8 +38,10 @@ accepts both, but the store flags the old spelling as deprecated
|
|||||||
dashboard or userscript on another host) can no longer call the mutating
|
dashboard or userscript on another host) can no longer call the mutating
|
||||||
API; call it server-side instead. Anyone posting a form body to
|
API; call it server-side instead. Anyone posting a form body to
|
||||||
`system/action` must switch to JSON. Behind a reverse proxy, forward the
|
`system/action` must switch to JSON. Behind a reverse proxy, forward the
|
||||||
original `Host` (`proxy_set_header Host $host;`); `X-Forwarded-Host` is
|
original `Host`, port included (`proxy_set_header Host $http_host;`;
|
||||||
not trusted.
|
nginx's `$host` drops the port); `X-Forwarded-Host` is not trusted. A
|
||||||
|
TLS-terminating proxy needs nothing more: a portless `Host` matches an
|
||||||
|
`https://` page.
|
||||||
|
|
||||||
### Fixes
|
### Fixes
|
||||||
|
|
||||||
|
|||||||
@@ -26,8 +26,8 @@ driving the Pi through a LAN user's browser. Scripts, curl, Home Assistant and
|
|||||||
the MQTT bridge send neither header and are unaffected. A browser page on
|
the MQTT bridge send neither header and are unaffected. A browser page on
|
||||||
another origin (a dashboard you host elsewhere, say) can no longer call the
|
another origin (a dashboard you host elsewhere, say) can no longer call the
|
||||||
API; call it server-side instead. Behind a reverse proxy, pass the original
|
API; call it server-side instead. Behind a reverse proxy, pass the original
|
||||||
`Host` through (nginx: `proxy_set_header Host $host;`) -- `X-Forwarded-Host`
|
`Host` through, port included (nginx: `proxy_set_header Host $http_host;`;
|
||||||
is not read.
|
`$host` drops the port) -- `X-Forwarded-Host` is not read.
|
||||||
|
|
||||||
## Table of Contents
|
## Table of Contents
|
||||||
|
|
||||||
|
|||||||
@@ -419,7 +419,8 @@ The API blueprint (`web_interface/blueprints/api_v3/`) is registered at
|
|||||||
(403 `CROSS_SITE_REQUEST`), so use the interface from its own address.
|
(403 `CROSS_SITE_REQUEST`), so use the interface from its own address.
|
||||||
- Scripts, curl, Home Assistant and the MQTT bridge send no such header and
|
- Scripts, curl, Home Assistant and the MQTT bridge send no such header and
|
||||||
keep working. Behind a reverse proxy, forward the original `Host` header
|
keep working. Behind a reverse proxy, forward the original `Host` header
|
||||||
(nginx: `proxy_set_header Host $host;`).
|
with its port (nginx: `proxy_set_header Host $http_host;` -- `$host`
|
||||||
|
drops the port).
|
||||||
|
|
||||||
**Best Practices:**
|
**Best Practices:**
|
||||||
1. Run on a private network (not exposed to internet)
|
1. Run on a private network (not exposed to internet)
|
||||||
|
|||||||
@@ -90,6 +90,57 @@ def test_the_same_host_on_another_port_is_another_site(probe):
|
|||||||
assert resp.status_code == 403
|
assert resp.status_code == 403
|
||||||
|
|
||||||
|
|
||||||
|
def test_an_https_page_behind_a_tls_terminating_proxy_passes(probe):
|
||||||
|
# nginx terminates TLS and forwards a portless Host to the plain-http
|
||||||
|
# upstream: the browser's Origin is https (443), Flask sees http (80).
|
||||||
|
resp = probe.post('/change', headers={
|
||||||
|
'Host': 'pi.example', 'Origin': 'https://pi.example'})
|
||||||
|
assert resp.status_code == 200
|
||||||
|
resp = probe.post('/change', headers={
|
||||||
|
'Host': 'pi.example', 'Referer': 'https://pi.example/v3'})
|
||||||
|
assert resp.status_code == 200
|
||||||
|
|
||||||
|
|
||||||
|
def test_a_portless_host_still_refuses_a_nondefault_port(probe):
|
||||||
|
# Only the standard port of either scheme counts as "no port".
|
||||||
|
for origin in ('https://pi.example:8443', 'http://pi.example:5000',
|
||||||
|
'http://pi.example:443', 'https://evil.example'):
|
||||||
|
resp = probe.post('/change', headers={
|
||||||
|
'Host': 'pi.example', 'Origin': origin})
|
||||||
|
assert resp.status_code == 403, origin
|
||||||
|
|
||||||
|
|
||||||
|
def test_an_explicit_host_port_must_match_exactly(probe):
|
||||||
|
# A Host with a port (the proxy forwards $http_host) is compared as is.
|
||||||
|
assert probe.post('/change', headers={
|
||||||
|
'Host': 'pi.example:8443',
|
||||||
|
'Origin': 'https://pi.example:8443'}).status_code == 200
|
||||||
|
assert probe.post('/change', headers={
|
||||||
|
'Host': 'pi.example:8443',
|
||||||
|
'Origin': 'https://pi.example'}).status_code == 403
|
||||||
|
|
||||||
|
|
||||||
|
def test_a_refusal_logs_only_the_site_never_the_referer_path(probe, caplog):
|
||||||
|
# A Referer's path and query can carry tokens.
|
||||||
|
with caplog.at_level('WARNING', logger='web_interface.origin_guard'):
|
||||||
|
resp = probe.post('/change', headers={
|
||||||
|
'Referer': EVIL + '/page?token=s3cret#frag'})
|
||||||
|
assert resp.status_code == 403
|
||||||
|
logged = caplog.text
|
||||||
|
assert 'evil.example' in logged
|
||||||
|
assert 's3cret' not in logged
|
||||||
|
assert '/page' not in logged
|
||||||
|
|
||||||
|
|
||||||
|
def test_a_refusal_log_cannot_be_forged_with_newlines(probe, caplog):
|
||||||
|
with caplog.at_level('WARNING', logger='web_interface.origin_guard'):
|
||||||
|
probe.post('/change%0D%0AFAKE', headers={'Origin': EVIL})
|
||||||
|
assert len(caplog.records) == 1
|
||||||
|
message = caplog.records[0].getMessage()
|
||||||
|
assert '\n' not in message and '\r' not in message
|
||||||
|
assert 'FAKE' in message # the path was logged, escaped
|
||||||
|
|
||||||
|
|
||||||
def test_no_origin_and_no_referer_passes(probe):
|
def test_no_origin_and_no_referer_passes(probe):
|
||||||
# curl, Home Assistant, the MQTT bridge: not a browser.
|
# curl, Home Assistant, the MQTT bridge: not a browser.
|
||||||
assert probe.post('/change').status_code == 200
|
assert probe.post('/change').status_code == 200
|
||||||
|
|||||||
@@ -29,14 +29,18 @@ it follows whatever name or address the user typed: ``ledpi.local:5000``,
|
|||||||
portal's port 80 -> 5000 redirect keeps the Host the browser sent, and the
|
portal's port 80 -> 5000 redirect keeps the Host the browser sent, and the
|
||||||
setup page's fetches go back to that same host).
|
setup page's fetches go back to that same host).
|
||||||
|
|
||||||
The scheme is deliberately not compared, only host and port (with each side's
|
The scheme is deliberately not compared, only host and port. The claimed
|
||||||
default port filled in from its own scheme). A TLS-terminating reverse proxy
|
value's default port comes from its own scheme. A ``Host`` without a port
|
||||||
that passes ``Host`` through makes the browser say ``https://pi.example`` while
|
means "the default port of whatever scheme the browser used", and that scheme
|
||||||
Flask sees ``http``; an attacker cannot use that gap, because to match they
|
is not always the one Flask sees: a TLS-terminating reverse proxy makes the
|
||||||
would need to serve a page from this same host and port. The app does not use
|
browser say ``https://pi.example`` (443) while Flask sees ``http`` (80). So a
|
||||||
``ProxyFix`` and so does not trust ``X-Forwarded-Host``: a proxy that rewrites
|
portless ``Host`` accepts either default. An attacker cannot use that gap,
|
||||||
``Host`` to the upstream address (nginx's default ``proxy_pass`` does) must
|
because to match they would need to serve a page from this same host on its
|
||||||
be configured to pass the original one (``proxy_set_header Host $host;``).
|
standard port. The app does not use ``ProxyFix`` and so does not trust
|
||||||
|
``X-Forwarded-Host`` or ``X-Forwarded-Proto``: a proxy that rewrites ``Host``
|
||||||
|
to the upstream address (nginx's default ``proxy_pass`` does) must be
|
||||||
|
configured to pass the original one, port included
|
||||||
|
(``proxy_set_header Host $http_host;`` -- nginx's ``$host`` drops the port).
|
||||||
|
|
||||||
Not covered: DNS rebinding (an attacker's hostname re-pointed at the Pi is
|
Not covered: DNS rebinding (an attacker's hostname re-pointed at the Pi is
|
||||||
"same origin" to the browser), and anyone who can reach the port directly.
|
"same origin" to the browser), and anyone who can reach the port directly.
|
||||||
@@ -55,14 +59,14 @@ STATE_CHANGING_METHODS = frozenset({'POST', 'PUT', 'PATCH', 'DELETE'})
|
|||||||
_DEFAULT_PORTS = {'http': 80, 'https': 443}
|
_DEFAULT_PORTS = {'http': 80, 'https': 443}
|
||||||
|
|
||||||
|
|
||||||
def _host_port(scheme: str, netloc: str):
|
def _authority(netloc: str):
|
||||||
"""``(hostname, port)`` for a URL's authority, or None if it has none.
|
"""``(hostname, port)`` for an authority; port is None when it has none.
|
||||||
|
|
||||||
Lower-cases the host and fills in the scheme's default port, so
|
Lower-cases the host and drops a trailing dot, so ``Pi.local.`` and
|
||||||
``http://Pi.local`` and a ``Host: pi.local:80`` header compare equal.
|
``pi.local`` compare equal. None if the authority is unreadable.
|
||||||
"""
|
"""
|
||||||
try:
|
try:
|
||||||
parts = urlsplit(f'{scheme}://{netloc}')
|
parts = urlsplit(f'//{netloc}')
|
||||||
hostname = parts.hostname
|
hostname = parts.hostname
|
||||||
port = parts.port
|
port = parts.port
|
||||||
except ValueError:
|
except ValueError:
|
||||||
@@ -70,25 +74,63 @@ def _host_port(scheme: str, netloc: str):
|
|||||||
return None
|
return None
|
||||||
if not hostname:
|
if not hostname:
|
||||||
return None
|
return None
|
||||||
if port is None:
|
|
||||||
port = _DEFAULT_PORTS.get(scheme.lower())
|
|
||||||
return hostname.lower().rstrip('.'), port
|
return hostname.lower().rstrip('.'), port
|
||||||
|
|
||||||
|
|
||||||
def _url_host_port(url: str):
|
def _url_host_port(url: str):
|
||||||
"""``(hostname, port)`` for an Origin or Referer value, or None."""
|
"""``(hostname, port, default_port)`` for an Origin or Referer, or None.
|
||||||
|
|
||||||
|
``port`` is the explicit port or, failing that, the URL scheme's default,
|
||||||
|
which is also returned as ``default_port``.
|
||||||
|
"""
|
||||||
try:
|
try:
|
||||||
parts = urlsplit(url.strip())
|
parts = urlsplit(url.strip())
|
||||||
except ValueError:
|
except ValueError:
|
||||||
return None
|
return None
|
||||||
if parts.scheme.lower() not in _DEFAULT_PORTS or not parts.netloc:
|
scheme = parts.scheme.lower()
|
||||||
|
if scheme not in _DEFAULT_PORTS or not parts.netloc:
|
||||||
return None
|
return None
|
||||||
return _host_port(parts.scheme.lower(), parts.netloc.rsplit('@', 1)[-1])
|
authority = _authority(parts.netloc.rsplit('@', 1)[-1])
|
||||||
|
if authority is None:
|
||||||
|
return None
|
||||||
|
hostname, port = authority
|
||||||
|
default_port = _DEFAULT_PORTS[scheme]
|
||||||
|
return hostname, default_port if port is None else port, default_port
|
||||||
|
|
||||||
|
|
||||||
def _request_host_port():
|
def _names_this_server(claimed) -> bool:
|
||||||
"""``(hostname, port)`` this request was addressed to, per its Host."""
|
"""Whether a claimed ``(hostname, port, default_port)`` is this request's
|
||||||
return _host_port(request.scheme, request.host)
|
own ``Host``."""
|
||||||
|
own = _authority(request.host)
|
||||||
|
if own is None:
|
||||||
|
return False
|
||||||
|
hostname, port = own
|
||||||
|
claimed_host, claimed_port, claimed_default = claimed
|
||||||
|
if claimed_host != hostname:
|
||||||
|
return False
|
||||||
|
if port is not None:
|
||||||
|
return claimed_port == port
|
||||||
|
# A portless Host is the default port of the scheme the browser used.
|
||||||
|
# Behind a TLS-terminating proxy that is https/443 while Flask sees
|
||||||
|
# http/80, so accept the default of either scheme.
|
||||||
|
return claimed_port in (claimed_default,
|
||||||
|
_DEFAULT_PORTS.get(request.scheme))
|
||||||
|
|
||||||
|
|
||||||
|
def _loggable(value: str) -> str:
|
||||||
|
"""Just the ``scheme://host[:port]`` of an Origin/Referer, for the log.
|
||||||
|
|
||||||
|
A Referer's path and query can carry tokens or other private data, and
|
||||||
|
only the site matters when reading a refusal.
|
||||||
|
"""
|
||||||
|
try:
|
||||||
|
parts = urlsplit(value.strip())
|
||||||
|
netloc = parts.netloc.rsplit('@', 1)[-1]
|
||||||
|
except ValueError:
|
||||||
|
return '<unreadable>'
|
||||||
|
if not parts.scheme or not netloc:
|
||||||
|
return '<unreadable>'
|
||||||
|
return f'{parts.scheme}://{netloc}'
|
||||||
|
|
||||||
|
|
||||||
def check_request_origin():
|
def check_request_origin():
|
||||||
@@ -115,7 +157,7 @@ def check_request_origin():
|
|||||||
claimed = _url_host_port(value)
|
claimed = _url_host_port(value)
|
||||||
if claimed is None:
|
if claimed is None:
|
||||||
return f'{header} header is not a valid http(s) URL'
|
return f'{header} header is not a valid http(s) URL'
|
||||||
if claimed != _request_host_port():
|
if not _names_this_server(claimed):
|
||||||
# The claimed value is attacker-chosen: the hook logs it, but the
|
# The claimed value is attacker-chosen: the hook logs it, but the
|
||||||
# reason (echoed in the 403 body) never repeats it.
|
# reason (echoed in the 403 body) never repeats it.
|
||||||
return header + ' names a different host than this interface'
|
return header + ' names a different host than this interface'
|
||||||
@@ -130,10 +172,14 @@ def init_app(app: Flask) -> None:
|
|||||||
reason = check_request_origin()
|
reason = check_request_origin()
|
||||||
if reason is None:
|
if reason is None:
|
||||||
return None
|
return None
|
||||||
logger.warning("Refused cross-site %s %s: %s (Origin=%r, Referer=%r)",
|
# Only the site each header names, never a Referer's path or query
|
||||||
|
# (which can carry tokens); %r keeps CR/LF from forging log lines.
|
||||||
|
origin = request.headers.get('Origin')
|
||||||
|
referer = request.headers.get('Referer')
|
||||||
|
logger.warning("Refused cross-site %s %r: %s (Origin=%r, Referer=%r)",
|
||||||
request.method, request.path, reason,
|
request.method, request.path, reason,
|
||||||
request.headers.get('Origin'),
|
None if origin is None else _loggable(origin),
|
||||||
request.headers.get('Referer'))
|
None if referer is None else _loggable(referer))
|
||||||
return jsonify({
|
return jsonify({
|
||||||
'status': 'error',
|
'status': 'error',
|
||||||
'error_code': 'CROSS_SITE_REQUEST',
|
'error_code': 'CROSS_SITE_REQUEST',
|
||||||
|
|||||||
Reference in New Issue
Block a user