Skip to content

Commit d107cd1

Browse files
degenarodegenaro
authored
Merge commit from fork
* fix: url validation Signed-off-by: degenaro <lou.degenaro@gmail.com> * fix: improve security test Signed-off-by: degenaro <lou.degenaro@gmail.com> --------- Signed-off-by: degenaro <lou.degenaro@gmail.com>
1 parent e92bde9 commit d107cd1

2 files changed

Lines changed: 289 additions & 10 deletions

File tree

‎tests/trestle/core/remote/cache_security_test.py‎

Lines changed: 205 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -708,4 +708,209 @@ def mock_getaddrinfo(hostname, port):
708708
validator.validate_url('https://example.com/path')
709709

710710

711+
class TestSSRFBypassVulnerabilities:
712+
"""Test fixes for SSRF bypass vulnerabilities (GHSA-h47f-gmjp-m7rr)."""
713+
714+
def test_blocks_ipv4_mapped_ipv6_cloud_metadata(self, tmp_path: pathlib.Path, monkeypatch) -> None:
715+
"""Test that IPv4-mapped IPv6 cloud metadata addresses are blocked."""
716+
test_utils.ensure_trestle_config_dir(tmp_path)
717+
718+
# Mock DNS resolution to return IPv4-mapped IPv6 address
719+
def mock_getaddrinfo(hostname, port):
720+
# Return ::ffff:169.254.169.254 (IPv4-mapped IPv6 for AWS metadata)
721+
return [(socket.AF_INET6, socket.SOCK_STREAM, 6, '', ('::ffff:169.254.169.254', 443, 0, 0))]
722+
723+
monkeypatch.setattr(socket, 'getaddrinfo', mock_getaddrinfo)
724+
725+
# Should block IPv4-mapped IPv6 cloud metadata address
726+
# The metadata endpoint check catches it first with "cloud metadata endpoints" message
727+
with pytest.raises(TrestleError, match='cloud metadata endpoints'):
728+
HTTPSFetcher(tmp_path, 'https://[::ffff:169.254.169.254]/latest/meta-data/')
729+
730+
def test_blocks_ipv4_mapped_ipv6_loopback(self, tmp_path: pathlib.Path, monkeypatch) -> None:
731+
"""Test that IPv4-mapped IPv6 loopback addresses are blocked."""
732+
test_utils.ensure_trestle_config_dir(tmp_path)
733+
734+
# Mock DNS resolution to return IPv4-mapped IPv6 loopback
735+
def mock_getaddrinfo(hostname, port):
736+
# Return ::ffff:127.0.0.1 (IPv4-mapped IPv6 for loopback)
737+
return [(socket.AF_INET6, socket.SOCK_STREAM, 6, '', ('::ffff:127.0.0.1', 443, 0, 0))]
738+
739+
monkeypatch.setattr(socket, 'getaddrinfo', mock_getaddrinfo)
740+
741+
# Should block IPv4-mapped IPv6 loopback
742+
with pytest.raises(TrestleError, match='127.0.0.0/8'):
743+
HTTPSFetcher(tmp_path, 'https://[::ffff:127.0.0.1]/admin')
744+
745+
def test_blocks_ipv4_mapped_ipv6_rfc1918(self, tmp_path: pathlib.Path, monkeypatch) -> None:
746+
"""Test that IPv4-mapped IPv6 RFC 1918 addresses are blocked when configured."""
747+
test_utils.ensure_trestle_config_dir(tmp_path)
748+
monkeypatch.setenv('TRESTLE_BLOCK_PRIVATE_IPS', 'true')
749+
750+
# Mock DNS resolution to return IPv4-mapped IPv6 private address
751+
def mock_getaddrinfo(hostname, port):
752+
# Return ::ffff:10.0.0.1 (IPv4-mapped IPv6 for RFC 1918)
753+
return [(socket.AF_INET6, socket.SOCK_STREAM, 6, '', ('::ffff:10.0.0.1', 443, 0, 0))]
754+
755+
monkeypatch.setattr(socket, 'getaddrinfo', mock_getaddrinfo)
756+
757+
# Should block IPv4-mapped IPv6 private address when TRESTLE_BLOCK_PRIVATE_IPS is set
758+
with pytest.raises(TrestleError, match='10.0.0.0/8'):
759+
HTTPSFetcher(tmp_path, 'https://[::ffff:10.0.0.1]/admin')
760+
761+
def test_blocks_zero_address_ipv4(self, tmp_path: pathlib.Path, monkeypatch) -> None:
762+
"""Test that 0.0.0.0 is blocked (reaches localhost on Linux)."""
763+
test_utils.ensure_trestle_config_dir(tmp_path)
764+
765+
# Mock DNS resolution to return 0.0.0.0
766+
def mock_getaddrinfo(hostname, port):
767+
return [(socket.AF_INET, socket.SOCK_STREAM, 6, '', ('0.0.0.0', 443))] # noqa: S104 - intentional test for blocked address
768+
769+
monkeypatch.setattr(socket, 'getaddrinfo', mock_getaddrinfo)
770+
771+
# Should block 0.0.0.0
772+
with pytest.raises(TrestleError, match='0.0.0.0/8'):
773+
HTTPSFetcher(tmp_path, 'https://0.0.0.0/admin')
774+
775+
def test_blocks_zero_address_ipv6(self, tmp_path: pathlib.Path, monkeypatch) -> None:
776+
"""Test that :: (IPv6 unspecified) is blocked."""
777+
test_utils.ensure_trestle_config_dir(tmp_path)
778+
779+
# Mock DNS resolution to return ::
780+
def mock_getaddrinfo(hostname, port):
781+
return [(socket.AF_INET6, socket.SOCK_STREAM, 6, '', ('::', 443, 0, 0))]
782+
783+
monkeypatch.setattr(socket, 'getaddrinfo', mock_getaddrinfo)
784+
785+
# Should block ::
786+
with pytest.raises(TrestleError, match='::/128'):
787+
HTTPSFetcher(tmp_path, 'https://[::]/admin')
788+
789+
def test_metadata_endpoint_canonicalization(self) -> None:
790+
"""Test that metadata endpoint check canonicalizes IPv4-mapped IPv6 addresses.
791+
792+
Note: This test directly calls a private method (_check_metadata_endpoints) to verify
793+
the canonicalization logic in isolation. While this creates coupling to implementation
794+
details, it's necessary to test this specific security-critical path without requiring
795+
full DNS resolution setup.
796+
"""
797+
from trestle.core.remote.security import URLSecurityValidator
798+
799+
validator = URLSecurityValidator()
800+
801+
# Test that bracketed IPv4-mapped IPv6 literal is canonicalized and blocked
802+
with pytest.raises(TrestleError, match='cloud metadata endpoints'):
803+
# This should be canonicalized to 169.254.169.254 and blocked
804+
validator._check_metadata_endpoints('::ffff:169.254.169.254')
805+
806+
def test_canonicalize_ip_method(self) -> None:
807+
"""Test the _canonicalize_ip method directly."""
808+
import ipaddress
809+
from trestle.core.remote.security import URLSecurityValidator
810+
811+
validator = URLSecurityValidator()
812+
813+
# Test IPv4-mapped IPv6 canonicalization
814+
ipv6_mapped = ipaddress.ip_address('::ffff:169.254.169.254')
815+
canonical = validator._canonicalize_ip(ipv6_mapped)
816+
assert isinstance(canonical, ipaddress.IPv4Address)
817+
assert str(canonical) == '169.254.169.254'
818+
819+
# Test regular IPv6 is unchanged
820+
ipv6_regular = ipaddress.ip_address('2001:db8::1')
821+
canonical = validator._canonicalize_ip(ipv6_regular)
822+
assert isinstance(canonical, ipaddress.IPv6Address)
823+
assert str(canonical) == '2001:db8::1'
824+
825+
# Test IPv4 is unchanged
826+
ipv4 = ipaddress.ip_address('192.0.2.1')
827+
canonical = validator._canonicalize_ip(ipv4)
828+
assert isinstance(canonical, ipaddress.IPv4Address)
829+
assert str(canonical) == '192.0.2.1'
830+
831+
def test_version_check_prevents_type_error(self, tmp_path: pathlib.Path, monkeypatch) -> None:
832+
"""Test that version checking prevents TypeError when comparing IPv4 and IPv6."""
833+
test_utils.ensure_trestle_config_dir(tmp_path)
834+
835+
# Mock DNS to return both IPv4 and IPv6 addresses
836+
def mock_getaddrinfo(hostname, port):
837+
return [
838+
(socket.AF_INET, socket.SOCK_STREAM, 6, '', ('127.0.0.1', 443)),
839+
(socket.AF_INET6, socket.SOCK_STREAM, 6, '', ('::1', 443, 0, 0)),
840+
]
841+
842+
monkeypatch.setattr(socket, 'getaddrinfo', mock_getaddrinfo)
843+
844+
# Should block both without TypeError
845+
with pytest.raises(TrestleError, match='127.0.0.0/8|::1/128'):
846+
HTTPSFetcher(tmp_path, 'https://localhost/admin')
847+
848+
def test_sftp_blocks_ipv4_mapped_ipv6_cloud_metadata(self, tmp_path: pathlib.Path, monkeypatch) -> None:
849+
"""Test that SFTPFetcher also blocks IPv4-mapped IPv6 cloud metadata."""
850+
test_utils.ensure_trestle_config_dir(tmp_path)
851+
852+
# Mock DNS resolution to return IPv4-mapped IPv6 address
853+
def mock_getaddrinfo(hostname, port):
854+
return [(socket.AF_INET6, socket.SOCK_STREAM, 6, '', ('::ffff:169.254.169.254', 22, 0, 0))]
855+
856+
monkeypatch.setattr(socket, 'getaddrinfo', mock_getaddrinfo)
857+
858+
# Should block IPv4-mapped IPv6 cloud metadata address
859+
# The metadata endpoint check catches it first with "cloud metadata endpoints" message
860+
with pytest.raises(TrestleError, match='cloud metadata endpoints'):
861+
SFTPFetcher(tmp_path, 'sftp://[::ffff:169.254.169.254]/data')
862+
863+
def test_sftp_blocks_zero_address(self, tmp_path: pathlib.Path, monkeypatch) -> None:
864+
"""Test that SFTPFetcher blocks 0.0.0.0."""
865+
test_utils.ensure_trestle_config_dir(tmp_path)
866+
867+
# Mock DNS resolution to return 0.0.0.0
868+
def mock_getaddrinfo(hostname, port):
869+
return [(socket.AF_INET, socket.SOCK_STREAM, 6, '', ('0.0.0.0', 22))] # noqa: S104 - intentional test for blocked address
870+
871+
monkeypatch.setattr(socket, 'getaddrinfo', mock_getaddrinfo)
872+
873+
# Should block 0.0.0.0
874+
with pytest.raises(TrestleError, match='0.0.0.0/8'):
875+
SFTPFetcher(tmp_path, 'sftp://0.0.0.0/data')
876+
877+
def test_sftp_blocks_ipv4_mapped_ipv6_rfc1918(self, tmp_path: pathlib.Path, monkeypatch) -> None:
878+
"""Test that SFTPFetcher blocks IPv4-mapped IPv6 RFC 1918 addresses when configured.
879+
880+
This mirrors test_blocks_ipv4_mapped_ipv6_rfc1918 for SFTP to ensure BYPASS-4
881+
from the advisory is covered for both HTTPSFetcher and SFTPFetcher.
882+
"""
883+
test_utils.ensure_trestle_config_dir(tmp_path)
884+
monkeypatch.setenv('TRESTLE_BLOCK_PRIVATE_IPS', 'true')
885+
886+
# Mock DNS resolution to return IPv4-mapped IPv6 private address
887+
def mock_getaddrinfo(hostname, port):
888+
# Return ::ffff:10.0.0.1 (IPv4-mapped IPv6 for RFC 1918)
889+
return [(socket.AF_INET6, socket.SOCK_STREAM, 6, '', ('::ffff:10.0.0.1', 22, 0, 0))]
890+
891+
monkeypatch.setattr(socket, 'getaddrinfo', mock_getaddrinfo)
892+
893+
# Should block IPv4-mapped IPv6 private address when TRESTLE_BLOCK_PRIVATE_IPS is set
894+
with pytest.raises(TrestleError, match='10.0.0.0/8'):
895+
SFTPFetcher(tmp_path, 'sftp://[::ffff:10.0.0.1]/data')
896+
897+
def test_multiple_ipv4_mapped_addresses(self, tmp_path: pathlib.Path, monkeypatch) -> None:
898+
"""Test handling of multiple IPv4-mapped IPv6 addresses in DNS response."""
899+
test_utils.ensure_trestle_config_dir(tmp_path)
900+
901+
# Mock DNS to return multiple IPv4-mapped IPv6 addresses
902+
def mock_getaddrinfo(hostname, port):
903+
return [
904+
(socket.AF_INET6, socket.SOCK_STREAM, 6, '', ('::ffff:169.254.169.254', 443, 0, 0)),
905+
(socket.AF_INET6, socket.SOCK_STREAM, 6, '', ('::ffff:127.0.0.1', 443, 0, 0)),
906+
]
907+
908+
monkeypatch.setattr(socket, 'getaddrinfo', mock_getaddrinfo)
909+
910+
# Should block on first blocked address (169.254.0.0/16)
911+
# Using | pattern for robustness - either network match indicates proper blocking
912+
with pytest.raises(TrestleError, match='169.254.0.0/16|127.0.0.0/8'):
913+
HTTPSFetcher(tmp_path, 'https://evil.example.com/data')
914+
915+
711916
# Made with Bob

‎trestle/core/remote/security.py‎

Lines changed: 84 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,12 @@ def get_block_private_ips_config() -> bool:
5050
# Always blocked - zero legitimate use for OSCAL fetching
5151
# These ranges are blocked regardless of configuration
5252
ALWAYS_BLOCKED_NETWORKS = [
53-
ipaddress.ip_network('127.0.0.0/8'), # Loopback
53+
ipaddress.ip_network('127.0.0.0/8'), # IPv4 loopback
5454
ipaddress.ip_network('::1/128'), # IPv6 loopback
55-
ipaddress.ip_network('169.254.0.0/16'), # Link-local (includes metadata endpoints)
55+
ipaddress.ip_network('169.254.0.0/16'), # IPv4 link-local (includes metadata endpoints)
5656
ipaddress.ip_network('fe80::/10'), # IPv6 link-local
57+
ipaddress.ip_network('0.0.0.0/8'), # IPv4 "this network", reaches localhost on Linux
58+
ipaddress.ip_network('::/128'), # IPv6 unspecified address
5759
]
5860

5961
# RFC 1918 private ranges - optionally blocked based on configuration
@@ -119,11 +121,31 @@ def validate_url(self, url: str) -> None:
119121

120122
for ip_str in ip_addresses:
121123
ip_addr = self._parse_ip_address(ip_str, hostname)
124+
# Canonicalize IPv4-mapped IPv6 addresses before validation
125+
ip_addr = self._canonicalize_ip(ip_addr)
122126
self._check_blocked_networks(ip_addr, hostname)
123127
self._check_private_networks(ip_addr, hostname)
124128

125129
self._check_suspicious_ports(parsed, url)
126130

131+
def _canonicalize_ip(
132+
self, ip_addr: ipaddress.IPv4Address | ipaddress.IPv6Address
133+
) -> ipaddress.IPv4Address | ipaddress.IPv6Address:
134+
"""Canonicalize IPv4-mapped IPv6 addresses to their IPv4 form.
135+
136+
IPv4-mapped IPv6 addresses (::ffff:a.b.c.d) are converted to their canonical
137+
IPv4 form to ensure consistent validation against IPv4 network ranges.
138+
139+
Args:
140+
ip_addr: The IP address to canonicalize
141+
142+
Returns:
143+
The canonical IP address (IPv4 if it was IPv4-mapped IPv6, otherwise unchanged)
144+
"""
145+
if isinstance(ip_addr, ipaddress.IPv6Address) and ip_addr.ipv4_mapped is not None:
146+
return ip_addr.ipv4_mapped
147+
return ip_addr
148+
127149
def _parse_and_validate_url(self, url: str) -> parse.ParseResult:
128150
"""Parse and validate basic URL structure."""
129151
try:
@@ -140,8 +162,26 @@ def _parse_and_validate_url(self, url: str) -> parse.ParseResult:
140162
return parsed
141163

142164
def _check_metadata_endpoints(self, hostname: str) -> None:
143-
"""Check if hostname is a blocked metadata endpoint."""
144-
if hostname in METADATA_HOSTNAMES:
165+
"""Check if hostname is a blocked metadata endpoint.
166+
167+
Canonicalizes bracketed IPv6 literal hostnames to handle IPv4-mapped
168+
IPv6 addresses (e.g., [::ffff:169.254.169.254]) before checking.
169+
170+
Note: This method is for URL-literal hostnames only (i.e., when the hostname
171+
in the URL itself is an IPv6 literal). DNS-resolved IPs are canonicalized
172+
upstream in validate_url via _canonicalize_ip before hostname-level checks.
173+
"""
174+
# Canonicalize bracketed IPv6 literal hostnames
175+
canonical = hostname.strip('[]')
176+
try:
177+
canonical_ip = ipaddress.ip_address(canonical)
178+
if isinstance(canonical_ip, ipaddress.IPv6Address) and canonical_ip.ipv4_mapped:
179+
canonical = str(canonical_ip.ipv4_mapped)
180+
except ValueError:
181+
# Not an IP literal, use original hostname
182+
canonical = hostname
183+
184+
if canonical in METADATA_HOSTNAMES:
145185
raise TrestleError(
146186
f'Access to cloud metadata endpoints is not allowed: {hostname}. '
147187
'This is a security restriction to prevent SSRF attacks.'
@@ -177,9 +217,21 @@ def _parse_ip_address(self, ip_str: str, hostname: str) -> ipaddress.IPv4Address
177217
raise TrestleError(f'Invalid IP address {ip_str} for hostname {hostname}: {e}') from e
178218

179219
def _check_blocked_networks(self, ip_addr: ipaddress.IPv4Address | ipaddress.IPv6Address, hostname: str) -> None:
180-
"""Check if IP is in always-blocked networks (Tier 1)."""
220+
"""Check if IP is in always-blocked networks (Tier 1).
221+
222+
Note: ip_addr should already be canonicalized by validate_url before calling this method.
223+
"""
224+
# Defensive check: ensure IPv4-mapped IPv6 addresses are canonicalized
225+
# This protects against future refactoring that might skip canonicalization
226+
if isinstance(ip_addr, ipaddress.IPv6Address) and ip_addr.ipv4_mapped is not None:
227+
raise TrestleError(
228+
f'Internal error: IP address {ip_addr} should have been canonicalized before network checks. '
229+
'IPv4-mapped IPv6 addresses must be converted to IPv4 form.'
230+
)
231+
181232
for network in ALWAYS_BLOCKED_NETWORKS:
182-
if ip_addr in network:
233+
# Only check if IP version matches network version to avoid TypeError
234+
if ip_addr.version == network.version and ip_addr in network:
183235
raise TrestleError(
184236
f'Access to {network} addresses is blocked: {hostname} resolves to {ip_addr}. '
185237
f'This range includes loopback, link-local, and cloud metadata endpoints. '
@@ -194,9 +246,20 @@ def _check_private_networks(self, ip_addr: ipaddress.IPv4Address | ipaddress.IPv
194246
self._warn_private_ip(ip_addr, hostname)
195247

196248
def _block_private_ip(self, ip_addr: ipaddress.IPv4Address | ipaddress.IPv6Address, hostname: str) -> None:
197-
"""Block access to private IP addresses when configured."""
249+
"""Block access to private IP addresses when configured.
250+
251+
Note: ip_addr should already be canonicalized by validate_url before calling this method.
252+
"""
253+
# Defensive check: ensure IPv4-mapped IPv6 addresses are canonicalized
254+
if isinstance(ip_addr, ipaddress.IPv6Address) and ip_addr.ipv4_mapped is not None:
255+
raise TrestleError(
256+
f'Internal error: IP address {ip_addr} should have been canonicalized before network checks. '
257+
'IPv4-mapped IPv6 addresses must be converted to IPv4 form.'
258+
)
259+
198260
for network in PRIVATE_IP_NETWORKS:
199-
if ip_addr in network:
261+
# Only check if IP version matches network version to avoid TypeError
262+
if ip_addr.version == network.version and ip_addr in network:
200263
raise TrestleError(
201264
f'Access to private IP addresses is blocked: {hostname} resolves to {ip_addr} '
202265
f'which is in private network {network}. '
@@ -205,9 +268,20 @@ def _block_private_ip(self, ip_addr: ipaddress.IPv4Address | ipaddress.IPv6Addre
205268
)
206269

207270
def _warn_private_ip(self, ip_addr: ipaddress.IPv4Address | ipaddress.IPv6Address, hostname: str) -> None:
208-
"""Log warning when accessing private IP addresses."""
271+
"""Log warning when accessing private IP addresses.
272+
273+
Note: ip_addr should already be canonicalized by validate_url before calling this method.
274+
"""
275+
# Defensive check: ensure IPv4-mapped IPv6 addresses are canonicalized
276+
if isinstance(ip_addr, ipaddress.IPv6Address) and ip_addr.ipv4_mapped is not None:
277+
raise TrestleError(
278+
f'Internal error: IP address {ip_addr} should have been canonicalized before network checks. '
279+
'IPv4-mapped IPv6 addresses must be converted to IPv4 form.'
280+
)
281+
209282
for network in PRIVATE_IP_NETWORKS:
210-
if ip_addr in network:
283+
# Only check if IP version matches network version to avoid TypeError
284+
if ip_addr.version == network.version and ip_addr in network:
211285
logger.warning(
212286
f'Accessing private IP address: {hostname} resolves to {ip_addr} in network {network}. '
213287
f'This is allowed by default to support private GitLab/internal OSCAL repositories. '

0 commit comments

Comments
 (0)