mirror of
https://github.com/maziggy/bambuddy.git
synced 2026-10-09 23:45:40 +02:00
Revert " fix(spoolman): allow LAN Spoolman in SSRF guard"
This reverts commit 4416fd4577.
This commit is contained in:
@@ -15,42 +15,26 @@ from urllib.parse import urlparse
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
_CLOUD_METADATA_IPS = frozenset(
|
||||
{
|
||||
# AWS / GCP / Azure / Oracle / DigitalOcean IMDS
|
||||
ipaddress.ip_address("169.254.169.254"),
|
||||
# Alibaba Cloud metadata
|
||||
ipaddress.ip_address("100.100.100.200"),
|
||||
# AWS IMDS IPv6
|
||||
ipaddress.ip_address("fd00:ec2::254"),
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
def assert_safe_spoolman_url(url: str) -> None:
|
||||
"""Raise ValueError if *url* should be blocked as an SSRF risk.
|
||||
|
||||
Bambuddy is typically deployed on a home LAN alongside Spoolman, so
|
||||
loopback (127.0.0.1) and RFC-1918 private ranges (192.168.x.x, 10.x.x.x,
|
||||
172.16-31.x) must be permitted — they are THE normal Spoolman topology.
|
||||
This guard therefore targets the genuinely dangerous cases only.
|
||||
|
||||
Checks performed:
|
||||
- Scheme must be http or https (no file://, gopher://, dict://, etc.).
|
||||
- Scheme must be http or https.
|
||||
- Numeric-encoded IP addresses in decimal (e.g. ``2130706433``) or hex
|
||||
(e.g. ``0x7f000001``) are rejected. Python's ``ipaddress`` module raises
|
||||
``ValueError`` for these forms so they would otherwise bypass the
|
||||
explicit-IP block below, but libc (and browsers) resolve them as valid
|
||||
IPv4 addresses.
|
||||
- Cloud provider metadata endpoints (169.254.169.254, 100.100.100.200,
|
||||
fd00:ec2::254) are blocked — the classic SSRF credential-exfil target.
|
||||
- Multicast (224.0.0.0/4, ff00::/8) and unspecified (0.0.0.0, ::) addresses
|
||||
are blocked — pointless as a destination and suggests misuse.
|
||||
- IPv4-mapped IPv6 addresses (::ffff:x.x.x.x) are unwrapped so they cannot
|
||||
bypass the checks above.
|
||||
(e.g. ``0x7f000001``) are rejected — Python's ``ipaddress`` module raises
|
||||
``ValueError`` for these forms so they would otherwise bypass the IP-range
|
||||
guard, but libc resolves them as valid IPv4 addresses.
|
||||
- Bare numeric IP hosts in loopback (127.x, ::1), link-local (169.254.x,
|
||||
fe80::), private (RFC-1918), multicast (224.x, ff::/8), or unspecified
|
||||
(0.0.0.0, ::) ranges are rejected.
|
||||
- IPv4-mapped IPv6 addresses (::ffff:x.x.x.x) are unwrapped to their IPv4
|
||||
equivalent and subject to the same checks.
|
||||
|
||||
Hostname-based addresses ("localhost", "spoolman.lan", "internal.corp")
|
||||
are out of scope — DNS resolution is deliberately not performed here.
|
||||
Hostname-based addresses ("localhost", "internal.corp") require DNS resolution
|
||||
and are outside the scope of this guard — they are mitigated by network-level
|
||||
controls in the deployment environment. "localhost" is intentionally *not*
|
||||
blocked here because running Spoolman on the same host is a common and
|
||||
supported topology.
|
||||
"""
|
||||
parsed = urlparse(url)
|
||||
if parsed.scheme.lower() not in ("http", "https"):
|
||||
@@ -59,8 +43,9 @@ def assert_safe_spoolman_url(url: str) -> None:
|
||||
hostname = (parsed.hostname or "").lower()
|
||||
|
||||
# Reject decimal- and hex-encoded IPs (e.g. http://2130706433/ or
|
||||
# http://0x7f000001/). These slip past ipaddress.ip_address() but libc
|
||||
# (and browsers) parse them as IPv4 — an obvious bypass if not caught.
|
||||
# http://0x7f000001/). Python's ipaddress.ip_address() raises ValueError for
|
||||
# these forms so they slip past the except-clause below, but the C library
|
||||
# (and browsers) parse them as valid IPv4 addresses.
|
||||
if re.match(r"^(0x[0-9a-f]+|[0-9]+)$", hostname, re.I):
|
||||
raise ValueError("Spoolman URL must not use numeric-encoded IP addresses; use standard dotted-decimal notation")
|
||||
|
||||
@@ -70,17 +55,22 @@ def assert_safe_spoolman_url(url: str) -> None:
|
||||
# Not a bare IP — hostname-based addresses are out of scope.
|
||||
return
|
||||
|
||||
# Unwrap IPv4-mapped IPv6 (::ffff:169.254.169.254 etc.) so attackers can't
|
||||
# encode a blocked IPv4 into an IPv6 literal to bypass the check.
|
||||
# Unwrap IPv4-mapped IPv6 (::ffff:169.254.x.x etc.) so their IPv4
|
||||
# properties are evaluated correctly.
|
||||
effective: ipaddress.IPv4Address | ipaddress.IPv6Address = addr
|
||||
if isinstance(addr, ipaddress.IPv6Address) and addr.ipv4_mapped is not None:
|
||||
effective = addr.ipv4_mapped
|
||||
|
||||
if effective in _CLOUD_METADATA_IPS:
|
||||
raise ValueError("Spoolman URL must not point to a cloud metadata endpoint")
|
||||
|
||||
if effective.is_multicast or effective.is_unspecified:
|
||||
raise ValueError("Spoolman URL must not point to a multicast or unspecified address")
|
||||
if (
|
||||
effective.is_loopback
|
||||
or effective.is_link_local
|
||||
or effective.is_private
|
||||
or effective.is_multicast
|
||||
or effective.is_unspecified
|
||||
):
|
||||
raise ValueError(
|
||||
"Spoolman URL must not point to a private, loopback, link-local, multicast, or unspecified address"
|
||||
)
|
||||
|
||||
|
||||
_COLOR_HEX_RE = re.compile(r"^[0-9A-Fa-f]{6}$")
|
||||
|
||||
@@ -758,16 +758,14 @@ class TestSpoolmanInventoryInputValidation:
|
||||
"file:///etc/passwd",
|
||||
"gopher://127.0.0.1:70/",
|
||||
"dict://internal.corp/",
|
||||
"javascript:alert(1)",
|
||||
"http://169.254.169.254/latest/meta-data/", # AWS IMDS
|
||||
"http://100.100.100.200/", # Alibaba Cloud metadata
|
||||
"http://[fd00:ec2::254]/", # AWS IMDS IPv6
|
||||
"http://169.254.169.254/latest/meta-data/", # AWS IMDS (link-local)
|
||||
"http://[::1]:7912/", # IPv6 loopback
|
||||
"http://0.0.0.0/", # unspecified
|
||||
"javascript:alert(1)",
|
||||
"http://224.0.0.1/", # IPv4 multicast
|
||||
"http://[ff02::1]/", # IPv6 multicast
|
||||
"http://127.1.2.3/", # 127.x.x.x loopback range
|
||||
"http://[::ffff:169.254.169.254]/", # IPv4-mapped IPv6 IMDS bypass
|
||||
"http://2130706433/", # decimal-encoded 127.0.0.1
|
||||
"http://0x7f000001/", # hex-encoded 127.0.0.1
|
||||
],
|
||||
)
|
||||
async def test_ssrf_blocked_schemes_and_addresses(
|
||||
@@ -777,10 +775,7 @@ class TestSpoolmanInventoryInputValidation:
|
||||
mock_spoolman_client,
|
||||
evil_url: str,
|
||||
):
|
||||
"""SSRF: dangerous schemes, cloud metadata IPs, multicast, unspecified,
|
||||
and numeric-encoded IPs must be rejected with 400. Loopback and
|
||||
RFC-1918 private ranges are allowed — they are legitimate Spoolman
|
||||
topologies for self-hosted Bambuddy deployments."""
|
||||
"""SSRF: any Spoolman URL that is not http(s) must be rejected with 400."""
|
||||
from backend.app.models.settings import Settings
|
||||
|
||||
db_session.add(Settings(key="spoolman_enabled", value="true"))
|
||||
@@ -792,36 +787,6 @@ class TestSpoolmanInventoryInputValidation:
|
||||
f"Expected 400 for SSRF URL {evil_url!r} but got {response.status_code}: {response.json()}"
|
||||
)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
@pytest.mark.parametrize(
|
||||
"lan_url",
|
||||
[
|
||||
"http://127.0.0.1:7912/", # loopback
|
||||
"http://[::1]:7912/", # IPv6 loopback
|
||||
"http://192.168.1.50:7912/", # RFC-1918 /16
|
||||
"http://10.0.0.5:7912/", # RFC-1918 /8
|
||||
"http://172.20.0.3:7912/", # RFC-1918 /12
|
||||
],
|
||||
)
|
||||
async def test_ssrf_allows_lan_spoolman_topologies(
|
||||
self,
|
||||
async_client: AsyncClient,
|
||||
db_session,
|
||||
mock_spoolman_client,
|
||||
lan_url: str,
|
||||
):
|
||||
"""Regression: Bambuddy's normal deployment is LAN-local Spoolman.
|
||||
Loopback and RFC-1918 private addresses must NOT be rejected as SSRF."""
|
||||
from backend.app.models.settings import Settings
|
||||
|
||||
db_session.add(Settings(key="spoolman_enabled", value="true"))
|
||||
db_session.add(Settings(key="spoolman_url", value=lan_url))
|
||||
await db_session.commit()
|
||||
|
||||
response = await async_client.get("/api/v1/spoolman/inventory/spools")
|
||||
assert response.status_code != 400, f"LAN URL {lan_url!r} was incorrectly blocked as SSRF: {response.json()}"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_create_rejects_storage_location_too_long(
|
||||
@@ -1317,7 +1282,9 @@ class TestSpoolmanInventorySSRFSpoolBuddyPath:
|
||||
[
|
||||
"file:///etc/passwd",
|
||||
"http://169.254.169.254/latest/meta-data/", # AWS IMDS
|
||||
"http://[::1]:7912/", # IPv6 loopback
|
||||
"http://0.0.0.0/", # unspecified
|
||||
"http://10.0.0.1/", # RFC-1918 private
|
||||
"http://[::ffff:169.254.169.254]/", # IPv4-mapped IMDS bypass
|
||||
],
|
||||
)
|
||||
@@ -1356,6 +1323,7 @@ class TestSpoolmanInventorySSRFSpoolBuddyPath:
|
||||
"evil_url",
|
||||
[
|
||||
"http://169.254.169.254/latest/meta-data/", # AWS IMDS
|
||||
"http://10.0.0.1/", # RFC-1918 private
|
||||
"http://[::ffff:169.254.169.254]/", # IPv4-mapped IMDS bypass
|
||||
],
|
||||
)
|
||||
@@ -1414,6 +1382,7 @@ class TestSpoolmanInventorySSRFSpoolBuddyPath:
|
||||
"evil_url",
|
||||
[
|
||||
"http://169.254.169.254/latest/meta-data/", # AWS IMDS
|
||||
"http://10.0.0.1/", # RFC-1918 private
|
||||
],
|
||||
)
|
||||
async def test_scale_update_weight_with_ssrf_url_degrades_gracefully(
|
||||
|
||||
@@ -452,62 +452,22 @@ class TestSpoolmanClient:
|
||||
|
||||
|
||||
class TestInitSpoolmanClientSSRFGuard:
|
||||
"""init_spoolman_client must reject genuinely unsafe URLs before creating a client.
|
||||
|
||||
Scope: cloud metadata endpoints, multicast, unspecified, non-http(s) schemes,
|
||||
and numeric-encoded IP bypasses. Loopback and RFC-1918 private ranges are
|
||||
explicitly allowed — Bambuddy's primary deployment is LAN-local Spoolman.
|
||||
"""
|
||||
"""init_spoolman_client must reject unsafe URLs before creating a client."""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_cloud_metadata_raises_value_error(self):
|
||||
with pytest.raises(ValueError, match="cloud metadata"):
|
||||
async def test_private_ip_raises_value_error(self):
|
||||
with pytest.raises(ValueError, match="private|loopback|link-local|multicast|unspecified"):
|
||||
await init_spoolman_client("http://10.0.0.1/")
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_link_local_raises_value_error(self):
|
||||
with pytest.raises(ValueError):
|
||||
await init_spoolman_client("http://169.254.169.254/latest/meta-data/")
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_multicast_raises_value_error(self):
|
||||
with pytest.raises(ValueError, match="multicast|unspecified"):
|
||||
await init_spoolman_client("http://224.0.0.1/")
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_unspecified_raises_value_error(self):
|
||||
with pytest.raises(ValueError, match="multicast|unspecified"):
|
||||
await init_spoolman_client("http://0.0.0.0/")
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_numeric_encoded_ip_raises_value_error(self):
|
||||
# decimal-encoded 127.0.0.1 — libc resolves these but ipaddress doesn't
|
||||
with pytest.raises(ValueError, match="numeric-encoded"):
|
||||
await init_spoolman_client("http://2130706433/")
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_non_http_scheme_raises_value_error(self):
|
||||
with pytest.raises(ValueError, match="http or https"):
|
||||
await init_spoolman_client("file:///etc/passwd")
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_private_ip_is_allowed(self):
|
||||
"""Regression: RFC-1918 private addresses are the normal LAN topology."""
|
||||
mock_instance = AsyncMock()
|
||||
with (
|
||||
patch("backend.app.services.spoolman._spoolman_client", None),
|
||||
patch("backend.app.services.spoolman.SpoolmanClient", return_value=mock_instance) as mock_cls,
|
||||
):
|
||||
client = await init_spoolman_client("http://192.168.1.50:7912/")
|
||||
mock_cls.assert_called_once_with("http://192.168.1.50:7912/")
|
||||
assert client is mock_instance
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_loopback_ip_is_allowed(self):
|
||||
"""Regression: same-host Spoolman via loopback is a supported topology."""
|
||||
mock_instance = AsyncMock()
|
||||
with (
|
||||
patch("backend.app.services.spoolman._spoolman_client", None),
|
||||
patch("backend.app.services.spoolman.SpoolmanClient", return_value=mock_instance) as mock_cls,
|
||||
):
|
||||
client = await init_spoolman_client("http://127.0.0.1:7912/")
|
||||
mock_cls.assert_called_once_with("http://127.0.0.1:7912/")
|
||||
assert client is mock_instance
|
||||
async def test_loopback_ip_raises_value_error(self):
|
||||
with pytest.raises(ValueError):
|
||||
await init_spoolman_client("http://127.0.0.1:7912/")
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_localhost_hostname_is_allowed(self):
|
||||
|
||||
File diff suppressed because one or more lines are too long
+1
-1
@@ -26,7 +26,7 @@
|
||||
|
||||
<!-- Splash screens for iOS -->
|
||||
<link rel="apple-touch-startup-image" href="/img/android-chrome-512x512.png" />
|
||||
<script type="module" crossorigin src="/assets/index-DwiMX8U1.js"></script>
|
||||
<script type="module" crossorigin src="/assets/index-C8M6qM08.js"></script>
|
||||
<link rel="stylesheet" crossorigin href="/assets/index-CgXLDG6B.css">
|
||||
</head>
|
||||
<body>
|
||||
|
||||
Reference in New Issue
Block a user