Skip to content

Commit 70a5624

Browse files
feanilclaude
andcommitted
fix: block SSRF in SAML metadata URL fetching
Adds `validate_saml_metadata_url()` to third_party_auth utils, which enforces HTTPS and blocks loopback, link-local (including cloud metadata endpoints like 169.254.169.254), and reserved IP addresses. RFC 1918 private ranges are blocked by default and can be opted out via `SAML_METADATA_URL_ALLOW_PRIVATE_IPS = True` for deployments where the SAML IdP lives on the same private network. Calls the validator in `fetch_saml_metadata()` before `requests.get()`, also adds a 30s request timeout and removes the previous non-enforcing HTTP warning. Addresses the platform-side fetch path described in: GHSA-328g-7h4g-r2m9 Note: the primary exploit path (`sync_provider_data` endpoint) now lives in edx-enterprise following the migration documented in docs/decisions/0025-saml-admin-views-in-enterprise-plugin.rst and will need a corresponding fix there. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
1 parent cddc25c commit 70a5624

4 files changed

Lines changed: 142 additions & 4 deletions

File tree

common/djangoapps/third_party_auth/tasks.py

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,10 @@
1616
from common.djangoapps.third_party_auth.models import SAMLConfiguration, SAMLProviderConfig
1717
from common.djangoapps.third_party_auth.utils import (
1818
MetadataParseError,
19+
SAMLMetadataURLError,
1920
create_or_update_bulk_saml_provider_data,
2021
parse_metadata_xml,
22+
validate_saml_metadata_url,
2123
)
2224

2325
log = logging.getLogger(__name__)
@@ -74,10 +76,9 @@ def fetch_saml_metadata():
7476
failure_messages = [] # We return the length of this array for num_failed
7577
for url, entity_ids in url_map.items():
7678
try:
79+
validate_saml_metadata_url(url)
7780
log.info("Fetching %s", url)
78-
if not url.lower().startswith('https'):
79-
log.warning("This SAML metadata URL is not secure! It should use HTTPS. (%s)", url)
80-
response = requests.get(url, verify=True) # May raise HTTPError or SSLError or ConnectionError
81+
response = requests.get(url, verify=True, timeout=30) # May raise HTTPError or SSLError or ConnectionError
8182
response.raise_for_status() # May raise an HTTPError
8283

8384
try:
@@ -96,7 +97,7 @@ def fetch_saml_metadata():
9697
num_updated += 1
9798
else:
9899
log.info(f"→ Updated existing SAMLProviderData. Nothing has changed for entityID {entity_id}")
99-
except (exceptions.SSLError, exceptions.HTTPError, exceptions.RequestException, MetadataParseError) as error:
100+
except (exceptions.SSLError, exceptions.HTTPError, exceptions.RequestException, MetadataParseError, SAMLMetadataURLError) as error:
100101
# Catch and process exception in case of errors during fetching and processing saml metadata.
101102
# Here is a description of each exception.
102103
# SSLError is raised in case of errors caused by SSL (e.g. SSL cer verification failure etc.)

common/djangoapps/third_party_auth/tests/test_utils.py

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,19 +6,23 @@
66
from unittest.mock import MagicMock
77

88
import ddt
9+
import pytest
10+
from django.test import override_settings
911
from lxml import etree
1012

1113
from common.djangoapps.student.tests.factories import UserFactory
1214
from common.djangoapps.third_party_auth.models import SAMLProviderData
1315
from common.djangoapps.third_party_auth.tests.testutil import TestCase
1416
from common.djangoapps.third_party_auth.utils import (
17+
SAMLMetadataURLError,
1518
create_or_update_bulk_saml_provider_data,
1619
get_associated_user_by_email_response,
1720
get_user_from_email,
1821
is_enterprise_customer_user,
1922
is_oauth_provider,
2023
parse_metadata_xml,
2124
user_exists,
25+
validate_saml_metadata_url,
2226
)
2327
from openedx.core.djangolib.testing.utils import skip_unless_lms
2428
from openedx.features.enterprise_support.tests.factories import (
@@ -272,3 +276,60 @@ def test_multiple_keys(self):
272276
entity_id='http://entity1',
273277
).count()
274278
assert count == 2
279+
280+
281+
@ddt.ddt
282+
@skip_unless_lms
283+
class TestValidateSAMLMetadataURL(TestCase):
284+
"""Tests for validate_saml_metadata_url."""
285+
286+
@ddt.data(
287+
'https://idp.example.com/metadata',
288+
'https://1.1.1.1/metadata',
289+
)
290+
def test_valid_urls_pass(self, url):
291+
validate_saml_metadata_url(url) # should not raise
292+
293+
@ddt.data(
294+
('http://idp.example.com/metadata', 'must use HTTPS'),
295+
('ftp://idp.example.com/metadata', 'must use HTTPS'),
296+
('https://', 'no hostname'),
297+
)
298+
@ddt.unpack
299+
def test_invalid_scheme_or_missing_hostname(self, url, expected_fragment):
300+
with pytest.raises(SAMLMetadataURLError, match=expected_fragment):
301+
validate_saml_metadata_url(url)
302+
303+
@ddt.data(
304+
'https://127.0.0.1/metadata', # IPv4 loopback
305+
'https://[::1]/metadata', # IPv6 loopback
306+
'https://169.254.169.254/latest', # AWS metadata endpoint
307+
'https://169.254.0.1/metadata', # other link-local
308+
'https://[fe80::1]/metadata', # IPv6 link-local
309+
'https://240.0.0.1/metadata', # reserved (Class E)
310+
)
311+
def test_always_blocked_regardless_of_setting(self, url):
312+
for allow_private in (False, True):
313+
with override_settings(SAML_METADATA_URL_ALLOW_PRIVATE_IPS=allow_private):
314+
with pytest.raises(SAMLMetadataURLError):
315+
validate_saml_metadata_url(url)
316+
317+
@ddt.data(
318+
'https://10.0.0.1/metadata',
319+
'https://172.16.0.1/metadata',
320+
'https://192.168.1.1/metadata',
321+
'https://[fc00::1]/metadata', # IPv6 unique local
322+
)
323+
def test_private_ip_blocked_by_default(self, url):
324+
with pytest.raises(SAMLMetadataURLError):
325+
validate_saml_metadata_url(url)
326+
327+
@ddt.data(
328+
'https://10.0.0.1/metadata',
329+
'https://172.16.0.1/metadata',
330+
'https://192.168.1.1/metadata',
331+
'https://[fc00::1]/metadata', # IPv6 unique local
332+
)
333+
@override_settings(SAML_METADATA_URL_ALLOW_PRIVATE_IPS=True)
334+
def test_private_ip_allowed_when_setting_enabled(self, url):
335+
validate_saml_metadata_url(url) # should not raise

common/djangoapps/third_party_auth/utils.py

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,12 @@
33
"""
44

55
import datetime
6+
import ipaddress
7+
from urllib.parse import urlparse
68
from zoneinfo import ZoneInfo
79

810
import dateutil.parser
11+
from django.conf import settings
912
from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user
1013
from django.utils.timezone import now
1114
from enterprise.models import EnterpriseCustomerIdentityProvider, EnterpriseCustomerUser
@@ -27,6 +30,67 @@ class MetadataParseError(Exception):
2730
pass # lint-amnesty, pylint: disable=unnecessary-pass
2831

2932

33+
class SAMLMetadataURLError(Exception):
34+
""" A SAML metadata URL failed security validation """
35+
pass # lint-amnesty, pylint: disable=unnecessary-pass
36+
37+
38+
def validate_saml_metadata_url(url):
39+
"""
40+
Validate that a SAML metadata URL is safe to fetch.
41+
42+
Enforces HTTPS and blocks requests to loopback, link-local, and reserved IP
43+
addresses. Link-local specifically covers cloud instance metadata endpoints
44+
(169.254.0.0/16, e.g. the AWS metadata service at 169.254.169.254).
45+
Reserved addresses (e.g. 240.0.0.0/4) are IETF-assigned ranges that are
46+
never routable on real networks.
47+
48+
Private IP ranges (RFC 1918: 10.x, 172.16.x, 192.168.x) are also blocked by
49+
default, since most Open edX deployments fetch SAML metadata from public IdPs.
50+
Operators running in a private network where the SAML IdP has a private IP can
51+
opt out by setting SAML_METADATA_URL_ALLOW_PRIVATE_IPS = True in Django settings.
52+
53+
Limitation: IP address checks only apply to literal IPs in the URL. Hostname-
54+
based URLs are not validated against the IP blocklists. Operators are encouraged
55+
to complement this with network-level egress filtering that blocks outbound
56+
connections from the Open edX server to link-local (169.254.0.0/16) and RFC
57+
1918 private address ranges.
58+
59+
Raises SAMLMetadataURLError if the URL fails validation.
60+
"""
61+
parsed = urlparse(url)
62+
63+
if parsed.scheme != 'https':
64+
raise SAMLMetadataURLError(
65+
f"SAML metadata URL must use HTTPS, got scheme: {parsed.scheme!r}"
66+
)
67+
68+
hostname = parsed.hostname
69+
if not hostname:
70+
raise SAMLMetadataURLError("SAML metadata URL has no hostname")
71+
72+
try:
73+
addr = ipaddress.ip_address(hostname)
74+
except ValueError:
75+
# hostname is a domain name, not a numeric IP literal — pass through.
76+
return
77+
78+
# Loopback, link-local, and reserved ranges are never legitimate SAML IdP
79+
# addresses regardless of deployment topology.
80+
if addr.is_loopback or addr.is_link_local or addr.is_reserved:
81+
raise SAMLMetadataURLError(
82+
f"SAML metadata URL hostname is a forbidden IP address: {addr}"
83+
)
84+
85+
# Private ranges are blocked by default but can be allowed via Django settings
86+
# for deployments where the SAML IdP lives on the same private network.
87+
if addr.is_private and not settings.SAML_METADATA_URL_ALLOW_PRIVATE_IPS:
88+
raise SAMLMetadataURLError(
89+
f"SAML metadata URL hostname is a private IP address: {addr}. "
90+
"Set SAML_METADATA_URL_ALLOW_PRIVATE_IPS = True in Django settings to allow this."
91+
)
92+
93+
3094
def parse_metadata_xml(xml, entity_id):
3195
"""
3296
Given an XML document containing SAML 2.0 metadata, parse it and return a tuple of

openedx/envs/common.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1925,6 +1925,18 @@ def add_optional_apps(optional_apps, installed_apps):
19251925
SOCIAL_AUTH_SAML_SP_PRIVATE_KEY_DICT = {}
19261926
SOCIAL_AUTH_SAML_SP_PUBLIC_CERT_DICT = {}
19271927

1928+
# .. setting_name: SAML_METADATA_URL_ALLOW_PRIVATE_IPS
1929+
# .. setting_default: False
1930+
# .. setting_description: When False (the default), fetching SAML metadata from
1931+
# private IP address ranges (RFC 1918: 10.x, 172.16.x, 192.168.x) is blocked
1932+
# as a defense against SSRF attacks. Set to True only in deployments where the
1933+
# SAML Identity Provider is hosted on the same private network as the Open edX
1934+
# server. Note: loopback (127.x) and link-local (169.254.x) addresses remain
1935+
# blocked regardless of this setting. Operators are also encouraged to enforce
1936+
# network-level egress filtering as a complementary control, particularly to
1937+
# cover hostname-based URLs that are not subject to IP validation.
1938+
SAML_METADATA_URL_ALLOW_PRIVATE_IPS = False
1939+
19281940
########################### django-fernet-fields ###########################
19291941

19301942
FERNET_KEYS = [

0 commit comments

Comments
 (0)