Skip to content

Commit 9bcec51

Browse files
committed
fix(security): hold https_only across every redirect hop of the key fetch
1 parent c740c2f commit 9bcec51

3 files changed

Lines changed: 35 additions & 0 deletions

File tree

backend/onyx/auth/jwt.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,11 +55,14 @@ def _fetch_public_key_payload(
5555
else:
5656
# Mirrors the PUT-time check: the configured SSRF level decides
5757
# whether private endpoints are reachable.
58+
# https_only holds across redirect hops, so no hop can downgrade
59+
# the key fetch to plaintext.
5860
response = ssrf_safe_get(
5961
public_key_url,
6062
allow_private_network=allow_private_network,
6163
block_loopback_and_link_local=block_loopback_and_link_local,
6264
block_link_local_only=block_link_local_only,
65+
https_only=True,
6366
)
6467
response.raise_for_status()
6568
except (requests.RequestException, SSRFException, ValueError) as exc:

backend/onyx/utils/url.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -403,6 +403,7 @@ def _make_ssrf_safe_request(
403403
allow_private_network: bool = False,
404404
block_loopback_and_link_local: bool = True,
405405
block_link_local_only: bool = False,
406+
https_only: bool = False,
406407
**kwargs: Any,
407408
) -> requests.Response:
408409
"""
@@ -421,12 +422,18 @@ def _make_ssrf_safe_request(
421422
it to ``block_link_local_only`` so loopback services are reachable while
422423
cloud-metadata stays blocked.
423424
"""
425+
if https_only and urlparse(url).scheme != "https":
426+
raise SSRFException(
427+
f"Invalid URL scheme '{urlparse(url).scheme}'. Only https is allowed."
428+
)
429+
424430
if allow_private_network:
425431
validate_outbound_http_url(
426432
url,
427433
allow_private_network=True,
428434
block_loopback_and_link_local=block_loopback_and_link_local,
429435
block_link_local_only=block_link_local_only,
436+
https_only=https_only,
430437
)
431438
parsed = urlparse(url)
432439
hostname = parsed.hostname or ""
@@ -458,6 +465,7 @@ def ssrf_safe_get(
458465
allow_private_network: bool = False,
459466
block_loopback_and_link_local: bool = True,
460467
block_link_local_only: bool = False,
468+
https_only: bool = False,
461469
**kwargs: Any,
462470
) -> requests.Response:
463471
"""
@@ -493,6 +501,7 @@ def ssrf_safe_get(
493501
allow_private_network=allow_private_network,
494502
block_loopback_and_link_local=block_loopback_and_link_local,
495503
block_link_local_only=block_link_local_only,
504+
https_only=https_only,
496505
**kwargs,
497506
)
498507

@@ -532,6 +541,7 @@ def ssrf_safe_get(
532541
allow_private_network=allow_private_network,
533542
block_loopback_and_link_local=block_loopback_and_link_local,
534543
block_link_local_only=block_link_local_only,
544+
https_only=https_only,
535545
**kwargs,
536546
)
537547

backend/tests/unit/onyx/utils/test_url_ssrf.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -607,3 +607,25 @@ def test_dns_failure_does_not_raise_with_floor(self) -> None:
607607
block_loopback_and_link_local=True,
608608
)
609609
assert validated == "https://internal-only.company.com/"
610+
611+
612+
class TestSsrfSafeGetHttpsOnly:
613+
def test_redirect_cannot_downgrade_to_http(self) -> None:
614+
"""A https_only fetch must refuse a redirect hop to plain http."""
615+
redirect = MagicMock()
616+
redirect.is_redirect = True
617+
redirect.headers = {"Location": "http://cdn.example.com/keys"}
618+
619+
with patch("onyx.utils.url.socket.getaddrinfo") as mock_getaddrinfo:
620+
mock_getaddrinfo.return_value = [(2, 1, 6, "", ("93.184.216.34", 443))]
621+
622+
with patch("onyx.utils.url.requests.Session") as mock_session_cls:
623+
session = mock_session_cls.return_value.__enter__.return_value
624+
session.get.return_value = redirect
625+
626+
with pytest.raises(SSRFException, match="Only https"):
627+
ssrf_safe_get("https://idp.example.com/keys", https_only=True)
628+
629+
def test_rejects_plain_http_upfront(self) -> None:
630+
with pytest.raises(SSRFException, match="Only https"):
631+
ssrf_safe_get("http://idp.example.com/keys", https_only=True)

0 commit comments

Comments
 (0)