From 9a26080423e7844d8727327cb1cc053b4719e012 Mon Sep 17 00:00:00 2001 From: ryazhang Date: Fri, 5 Jun 2026 10:42:31 -0400 Subject: [PATCH 1/4] [confidentialledger] Enforce stricter redirect destination policy Restrict redirects followed by the data-plane redirect policy to the original ledger host or one of its subdomains (e.g. an individual node). Redirects to sibling ledgers, parent domains, unrelated hosts, and look-alike suffix domains are now rejected, logged at WARNING level, and never followed or cached. - Add _is_allowed_redirect_target() host/subdomain check (case-insensitive, port-agnostic, fail-safe when a host is missing). - Enforce it in both RedirectCachingPolicy.send and AsyncRedirectCachingPolicy.send by capturing the pristine request URL before any cache rewrite. - Add unit tests covering allowed (same host, subdomains) and disallowed (sibling, parent, unrelated, suffix look-alike) targets, plus caplog assertions for the block warning. - Update CHANGELOG. Fixes ICM 31000000622491. --- .../azure-confidentialledger/CHANGELOG.md | 1 + .../_redirect_caching_policy.py | 58 +++++ .../tests/test_redirect_caching_policy.py | 203 +++++++++++++++--- 3 files changed, 238 insertions(+), 24 deletions(-) diff --git a/sdk/confidentialledger/azure-confidentialledger/CHANGELOG.md b/sdk/confidentialledger/azure-confidentialledger/CHANGELOG.md index e2a3e0e82e87..dc02f7418eae 100644 --- a/sdk/confidentialledger/azure-confidentialledger/CHANGELOG.md +++ b/sdk/confidentialledger/azure-confidentialledger/CHANGELOG.md @@ -5,6 +5,7 @@ ### Features Added - Added redirect URL caching for write operations (POST/PUT/PATCH/DELETE). After the first redirect from the load balancer, subsequent writes go directly to the primary node, significantly reducing latency. +- Enforced a stricter redirect destination policy: redirects are only followed when the target host is the original ledger host or one of its subdomains (e.g. an individual node). Redirects to sibling ledgers, parent domains, unrelated hosts, or look-alike suffix domains are now rejected and never followed or cached. ### Bugs Fixed diff --git a/sdk/confidentialledger/azure-confidentialledger/azure/confidentialledger/_redirect_caching_policy.py b/sdk/confidentialledger/azure-confidentialledger/azure/confidentialledger/_redirect_caching_policy.py index fe1a2214d723..933a58d25941 100644 --- a/sdk/confidentialledger/azure-confidentialledger/azure/confidentialledger/_redirect_caching_policy.py +++ b/sdk/confidentialledger/azure-confidentialledger/azure/confidentialledger/_redirect_caching_policy.py @@ -15,6 +15,15 @@ The cache is invalidated on 5xx responses or transport errors so that a failover is respected on the next write. +Redirect destination policy +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +Redirects are only followed when the target host is the original request host +or one of its subdomains (e.g. an individual ledger node). Redirects to sibling +ledgers, parent domains, unrelated hosts, or look-alike suffix domains are +rejected and never followed or cached. This prevents a misconfigured or +malicious load-balancer from redirecting requests (and their sensitive headers) +to an unintended destination. + Thread-safety ~~~~~~~~~~~~~ * Reads of the cached value are lock-free (CPython GIL guarantees atomic @@ -94,6 +103,29 @@ def _is_redirect(status_code: int) -> bool: return status_code in _REDIRECT_STATUS_CODES +def _is_allowed_redirect_target(original_url: str, target_url: str) -> bool: + """Return whether *target_url* is an allowed redirect destination. + + A redirect is permitted only when the target host is identical to the + original request host or is a subdomain of it. All other targets — sibling + hosts, parent domains, unrelated hosts, and look-alike suffix domains — are + rejected. Comparison is case-insensitive and ignores any port. + + :param str original_url: The URL of the original request. + :param str target_url: The redirect target URL (e.g. from a ``Location`` header). + :return: True if the redirect target is permitted, otherwise False. + :rtype: bool + """ + original_host = (urlparse(original_url).hostname or "").lower() + target_host = (urlparse(target_url).hostname or "").lower() + + # Fail safe: if either host cannot be determined, do not follow the redirect. + if not original_host or not target_host: + return False + + return target_host == original_host or target_host.endswith("." + original_host) + + class RedirectCachingPolicy(HTTPPolicy): """Synchronous redirect policy with write-URL caching. @@ -122,6 +154,10 @@ def send(self, request: PipelineRequest) -> PipelineResponse: method = request.http_request.method.upper() is_write = method in _WRITE_METHODS + # Capture the pristine request host (before any cache rewrite) so that + # redirect targets are validated against the original ledger endpoint. + original_url = request.http_request.url + # For writes, rewrite the URL to the cached primary (if warm). if is_write: cached = self._cache.get() @@ -157,6 +193,15 @@ def send(self, request: PipelineRequest) -> PipelineResponse: if not redirect_url: break + # Enforce the redirect destination policy: only the original host or + # one of its subdomains may be followed. + if not _is_allowed_redirect_target(original_url, redirect_url): + _LOGGER.warning( + "Refusing to follow redirect to disallowed target: %s", + redirect_url, + ) + break + # Only cache for write methods. if is_write: self._cache.set(redirect_url) @@ -205,6 +250,10 @@ async def send(self, request: PipelineRequest) -> PipelineResponse: method = request.http_request.method.upper() is_write = method in _WRITE_METHODS + # Capture the pristine request host (before any cache rewrite) so that + # redirect targets are validated against the original ledger endpoint. + original_url = request.http_request.url + if is_write: cached = self._cache.get() if cached: @@ -237,6 +286,15 @@ async def send(self, request: PipelineRequest) -> PipelineResponse: if not redirect_url: break + # Enforce the redirect destination policy: only the original host or + # one of its subdomains may be followed. + if not _is_allowed_redirect_target(original_url, redirect_url): + _LOGGER.warning( + "Refusing to follow redirect to disallowed target: %s", + redirect_url, + ) + break + if is_write: self._cache.set(redirect_url) _LOGGER.debug("Cached redirect target for writes: %s", redirect_url) diff --git a/sdk/confidentialledger/azure-confidentialledger/tests/test_redirect_caching_policy.py b/sdk/confidentialledger/azure-confidentialledger/tests/test_redirect_caching_policy.py index 4ab2293452c1..7187b1b9bd19 100644 --- a/sdk/confidentialledger/azure-confidentialledger/tests/test_redirect_caching_policy.py +++ b/sdk/confidentialledger/azure-confidentialledger/tests/test_redirect_caching_policy.py @@ -6,6 +6,7 @@ """Unit tests for RedirectCachingPolicy and AsyncRedirectCachingPolicy.""" import asyncio +import logging from unittest.mock import AsyncMock, MagicMock import pytest @@ -14,14 +15,29 @@ AsyncRedirectCachingPolicy, RedirectCachingPolicy, _RedirectUrlCache, + _is_allowed_redirect_target, _rewrite_url, ) +# ── test hosts ─────────────────────────────────────────────────────────────── +# The ledger endpoint (also the load-balancer host) and a valid node subdomain. +LEDGER_URL = "https://test-ledger.confidential-ledger.azure.com/app/transactions" +NODE_URL = "https://node3.test-ledger.confidential-ledger.azure.com/app/transactions" +NODE_BASE = "https://node3.test-ledger.confidential-ledger.azure.com" +NODE_HOST = "node3.test-ledger.confidential-ledger.azure.com" + +# Disallowed redirect destinations. +SIBLING_URL = "https://other-ledger.confidential-ledger.azure.com/app/transactions" +PARENT_URL = "https://confidential-ledger.azure.com/app/transactions" +UNRELATED_URL = "https://evil.example.com/app/transactions" +SUFFIX_LOOKALIKE_URL = "https://test-ledger.confidential-ledger.azure.com.evil.com/app/transactions" + + # ── helpers ────────────────────────────────────────────────────────────────── -def _make_request(method: str = "POST", url: str = "https://lb.example.com/app/transactions"): +def _make_request(method: str = "POST", url: str = LEDGER_URL): """Return a fake PipelineRequest.""" http_request = MagicMock() http_request.method = method @@ -80,6 +96,60 @@ def test_replaces_scheme_and_host(self): assert result == "https://new-host:443/path" +# ── _is_allowed_redirect_target ────────────────────────────────────────────── + + +class TestIsAllowedRedirectTarget: + def test_same_host_allowed(self): + assert _is_allowed_redirect_target(LEDGER_URL, LEDGER_URL) is True + + def test_subdomain_allowed(self): + assert _is_allowed_redirect_target(LEDGER_URL, NODE_URL) is True + assert ( + _is_allowed_redirect_target( + LEDGER_URL, + "https://primary-1.test-ledger.confidential-ledger.azure.com/app", + ) + is True + ) + + def test_same_host_ignores_port(self): + assert ( + _is_allowed_redirect_target( + LEDGER_URL, + "https://test-ledger.confidential-ledger.azure.com:443/app", + ) + is True + ) + + def test_case_insensitive(self): + assert ( + _is_allowed_redirect_target( + LEDGER_URL, + "https://NODE3.Test-Ledger.Confidential-Ledger.Azure.Com/app", + ) + is True + ) + + def test_sibling_host_blocked(self): + assert _is_allowed_redirect_target(LEDGER_URL, SIBLING_URL) is False + + def test_parent_domain_blocked(self): + assert _is_allowed_redirect_target(LEDGER_URL, PARENT_URL) is False + + def test_unrelated_host_blocked(self): + assert _is_allowed_redirect_target(LEDGER_URL, UNRELATED_URL) is False + + def test_suffix_lookalike_blocked(self): + assert _is_allowed_redirect_target(LEDGER_URL, SUFFIX_LOOKALIKE_URL) is False + + def test_missing_target_host_blocked(self): + assert _is_allowed_redirect_target(LEDGER_URL, "/app/transactions") is False + + def test_missing_original_host_blocked(self): + assert _is_allowed_redirect_target("/app/transactions", NODE_URL) is False + + # ── RedirectCachingPolicy (sync) ───────────────────────────────────────────── @@ -92,7 +162,7 @@ def _make_policy(self, responses): return policy def test_cache_populated_on_first_redirect(self): - redirect_resp = _make_response(307, {"Location": "https://primary.example.com/app/transactions"}) + redirect_resp = _make_response(307, {"Location": NODE_URL}) final_resp = _make_response(200) policy = self._make_policy([redirect_resp, final_resp]) @@ -100,40 +170,40 @@ def test_cache_populated_on_first_redirect(self): result = policy.send(request) assert result.http_response.status_code == 200 - assert policy._cache.get() == "https://primary.example.com" + assert policy._cache.get() == NODE_BASE def test_subsequent_write_uses_cached_url(self): # First request: redirect populates cache - redirect_resp = _make_response(307, {"Location": "https://primary.example.com/app/transactions"}) + redirect_resp = _make_response(307, {"Location": NODE_URL}) final_resp1 = _make_response(200) # Second request: should go directly final_resp2 = _make_response(200) policy = self._make_policy([redirect_resp, final_resp1, final_resp2]) policy.send(_make_request("POST")) - req2 = _make_request("POST", "https://lb.example.com/app/transactions") + req2 = _make_request("POST", LEDGER_URL) policy.send(req2) # The request URL should have been rewritten to the cached primary - assert "primary.example.com" in req2.http_request.url + assert NODE_HOST in req2.http_request.url def test_get_never_uses_cache(self): # Warm the cache via a POST redirect - redirect_resp = _make_response(307, {"Location": "https://primary.example.com/app/transactions"}) + redirect_resp = _make_response(307, {"Location": NODE_URL}) final_resp1 = _make_response(200) # GET request final_resp2 = _make_response(200) policy = self._make_policy([redirect_resp, final_resp1, final_resp2]) policy.send(_make_request("POST")) - get_req = _make_request("GET", "https://lb.example.com/app/transactions") + get_req = _make_request("GET", LEDGER_URL) policy.send(get_req) # GET should NOT have been rewritten - assert get_req.http_request.url == "https://lb.example.com/app/transactions" + assert get_req.http_request.url == LEDGER_URL def test_5xx_invalidates_cache(self): - redirect_resp = _make_response(307, {"Location": "https://primary.example.com/app/transactions"}) + redirect_resp = _make_response(307, {"Location": NODE_URL}) final_resp1 = _make_response(200) error_resp = _make_response(503) policy = self._make_policy([redirect_resp, final_resp1, error_resp]) @@ -145,7 +215,7 @@ def test_5xx_invalidates_cache(self): assert policy._cache.get() is None def test_transport_error_invalidates_cache(self): - redirect_resp = _make_response(307, {"Location": "https://primary.example.com/app/transactions"}) + redirect_resp = _make_response(307, {"Location": NODE_URL}) final_resp = _make_response(200) policy = self._make_policy([redirect_resp, final_resp, ConnectionError("connection lost")]) @@ -157,16 +227,16 @@ def test_transport_error_invalidates_cache(self): assert policy._cache.get() is None def test_delete_is_a_write_method(self): - redirect_resp = _make_response(307, {"Location": "https://primary.example.com/app/transactions"}) + redirect_resp = _make_response(307, {"Location": NODE_URL}) final_resp = _make_response(200) policy = self._make_policy([redirect_resp, final_resp]) policy.send(_make_request("DELETE")) - assert policy._cache.get() == "https://primary.example.com" + assert policy._cache.get() == NODE_BASE def test_permit_redirects_false_skips_redirect(self): policy = RedirectCachingPolicy(permit_redirects=False) - redirect_resp = _make_response(307, {"Location": "https://primary.example.com/app/transactions"}) + redirect_resp = _make_response(307, {"Location": NODE_URL}) policy.next = MagicMock() policy.next.send = MagicMock(return_value=redirect_resp) @@ -175,6 +245,48 @@ def test_permit_redirects_false_skips_redirect(self): assert result.http_response.status_code == 307 assert policy._cache.get() is None + @pytest.mark.parametrize( + "target", [SIBLING_URL, PARENT_URL, UNRELATED_URL, SUFFIX_LOOKALIKE_URL] + ) + def test_disallowed_redirect_not_followed(self, target): + redirect_resp = _make_response(307, {"Location": target}) + # Only one downstream call is expected; the redirect must not be followed. + policy = self._make_policy([redirect_resp]) + + result = policy.send(_make_request("POST")) + + # The 307 is returned as-is and the disallowed target is never cached. + assert result.http_response.status_code == 307 + assert policy._cache.get() is None + assert policy.next.send.call_count == 1 + + def test_subdomain_redirect_followed(self): + redirect_resp = _make_response(307, {"Location": NODE_URL}) + final_resp = _make_response(200) + policy = self._make_policy([redirect_resp, final_resp]) + + result = policy.send(_make_request("POST")) + + assert result.http_response.status_code == 200 + assert policy._cache.get() == NODE_BASE + + def test_disallowed_redirect_emits_warning_log(self, caplog): + redirect_resp = _make_response(307, {"Location": UNRELATED_URL}) + policy = self._make_policy([redirect_resp]) + + with caplog.at_level( + logging.WARNING, + logger="azure.confidentialledger._redirect_caching_policy", + ): + policy.send(_make_request("POST")) + + warnings = [r for r in caplog.records if r.levelno == logging.WARNING] + assert any( + "Refusing to follow redirect to disallowed target" in r.message + and UNRELATED_URL in r.message + for r in warnings + ), f"Expected a block warning for {UNRELATED_URL}, got: {[r.message for r in warnings]}" + # ── AsyncRedirectCachingPolicy ─────────────────────────────────────────────── @@ -195,43 +307,43 @@ def _make_policy(self, responses): @pytest.mark.asyncio async def test_cache_populated_on_first_redirect(self): - redirect_resp = _make_response(307, {"Location": "https://primary.example.com/app/transactions"}) + redirect_resp = _make_response(307, {"Location": NODE_URL}) final_resp = _make_response(200) policy = self._make_policy([redirect_resp, final_resp]) result = await policy.send(_make_request("POST")) assert result.http_response.status_code == 200 - assert policy._cache.get() == "https://primary.example.com" + assert policy._cache.get() == NODE_BASE @pytest.mark.asyncio async def test_subsequent_write_uses_cached_url(self): - redirect_resp = _make_response(307, {"Location": "https://primary.example.com/app/transactions"}) + redirect_resp = _make_response(307, {"Location": NODE_URL}) final_resp1 = _make_response(200) final_resp2 = _make_response(200) policy = self._make_policy([redirect_resp, final_resp1, final_resp2]) await policy.send(_make_request("POST")) - req2 = _make_request("POST", "https://lb.example.com/app/transactions") + req2 = _make_request("POST", LEDGER_URL) await policy.send(req2) - assert "primary.example.com" in req2.http_request.url + assert NODE_HOST in req2.http_request.url @pytest.mark.asyncio async def test_get_never_uses_cache(self): - redirect_resp = _make_response(307, {"Location": "https://primary.example.com/app/transactions"}) + redirect_resp = _make_response(307, {"Location": NODE_URL}) final_resp1 = _make_response(200) final_resp2 = _make_response(200) policy = self._make_policy([redirect_resp, final_resp1, final_resp2]) await policy.send(_make_request("POST")) - get_req = _make_request("GET", "https://lb.example.com/app/transactions") + get_req = _make_request("GET", LEDGER_URL) await policy.send(get_req) - assert get_req.http_request.url == "https://lb.example.com/app/transactions" + assert get_req.http_request.url == LEDGER_URL @pytest.mark.asyncio async def test_5xx_invalidates_cache(self): - redirect_resp = _make_response(307, {"Location": "https://primary.example.com/app/transactions"}) + redirect_resp = _make_response(307, {"Location": NODE_URL}) final_resp1 = _make_response(200) error_resp = _make_response(503) policy = self._make_policy([redirect_resp, final_resp1, error_resp]) @@ -244,7 +356,7 @@ async def test_5xx_invalidates_cache(self): @pytest.mark.asyncio async def test_transport_error_invalidates_cache(self): - redirect_resp = _make_response(307, {"Location": "https://primary.example.com/app/transactions"}) + redirect_resp = _make_response(307, {"Location": NODE_URL}) final_resp = _make_response(200) policy = self._make_policy([redirect_resp, final_resp, ConnectionError("connection lost")]) @@ -254,3 +366,46 @@ async def test_transport_error_invalidates_cache(self): with pytest.raises(ConnectionError): await policy.send(_make_request("POST")) assert policy._cache.get() is None + + @pytest.mark.asyncio + @pytest.mark.parametrize( + "target", [SIBLING_URL, PARENT_URL, UNRELATED_URL, SUFFIX_LOOKALIKE_URL] + ) + async def test_disallowed_redirect_not_followed(self, target): + redirect_resp = _make_response(307, {"Location": target}) + policy = self._make_policy([redirect_resp]) + + result = await policy.send(_make_request("POST")) + + assert result.http_response.status_code == 307 + assert policy._cache.get() is None + assert policy.next.send.call_count == 1 + + @pytest.mark.asyncio + async def test_subdomain_redirect_followed(self): + redirect_resp = _make_response(307, {"Location": NODE_URL}) + final_resp = _make_response(200) + policy = self._make_policy([redirect_resp, final_resp]) + + result = await policy.send(_make_request("POST")) + + assert result.http_response.status_code == 200 + assert policy._cache.get() == NODE_BASE + + @pytest.mark.asyncio + async def test_disallowed_redirect_emits_warning_log(self, caplog): + redirect_resp = _make_response(307, {"Location": UNRELATED_URL}) + policy = self._make_policy([redirect_resp]) + + with caplog.at_level( + logging.WARNING, + logger="azure.confidentialledger._redirect_caching_policy", + ): + await policy.send(_make_request("POST")) + + warnings = [r for r in caplog.records if r.levelno == logging.WARNING] + assert any( + "Refusing to follow redirect to disallowed target" in r.message + and UNRELATED_URL in r.message + for r in warnings + ), f"Expected a block warning for {UNRELATED_URL}, got: {[r.message for r in warnings]}" From 3bbe730c802287c9c56a2c62038bccd0bbe40850 Mon Sep 17 00:00:00 2001 From: ryazhang Date: Fri, 5 Jun 2026 10:46:59 -0400 Subject: [PATCH 2/4] [confidentialledger] Set 2.0.0b3 release date in CHANGELOG --- sdk/confidentialledger/azure-confidentialledger/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/confidentialledger/azure-confidentialledger/CHANGELOG.md b/sdk/confidentialledger/azure-confidentialledger/CHANGELOG.md index dc02f7418eae..00d2644c478b 100644 --- a/sdk/confidentialledger/azure-confidentialledger/CHANGELOG.md +++ b/sdk/confidentialledger/azure-confidentialledger/CHANGELOG.md @@ -1,6 +1,6 @@ # Release History -## 2.0.0b3 (Unreleased) +## 2.0.0b3 (2026-06-05) ### Features Added From f02ce92aff2964ec62c9859c4698e5ec7e5b4e86 Mon Sep 17 00:00:00 2001 From: ryazhang Date: Fri, 5 Jun 2026 12:43:44 -0400 Subject: [PATCH 3/4] [confidentialledger] Also require HTTPS and matching port for redirect targets Address review feedback: the redirect destination check validated only the host, so an https->http downgrade or a same-host-different-port target would pass (urlparse(...).hostname drops scheme/port). Because the redirected request reuses the same request object downstream of auth, the bearer token could be sent over cleartext or to an unexpected port. Match the JS/.NET fix: a redirect is now followed only when the target uses HTTPS, has the same effective port (scheme default applied when absent), and the host is the original host or a subdomain. - Add _effective_port() helper (explicit port, else 443/80 by scheme). - Reject non-HTTPS targets and effective-port mismatches in _is_allowed_redirect_target. - Add unit tests for http downgrade and different-port targets (host and subdomain) at both the helper and policy.send levels; extend the sync/async parametrized block lists. - Update CHANGELOG and module docstring. --- .../azure-confidentialledger/CHANGELOG.md | 2 +- .../_redirect_caching_policy.py | 77 ++++++++++++++++--- .../tests/test_redirect_caching_policy.py | 54 ++++++++++++- 3 files changed, 117 insertions(+), 16 deletions(-) diff --git a/sdk/confidentialledger/azure-confidentialledger/CHANGELOG.md b/sdk/confidentialledger/azure-confidentialledger/CHANGELOG.md index 00d2644c478b..645885090a54 100644 --- a/sdk/confidentialledger/azure-confidentialledger/CHANGELOG.md +++ b/sdk/confidentialledger/azure-confidentialledger/CHANGELOG.md @@ -5,7 +5,7 @@ ### Features Added - Added redirect URL caching for write operations (POST/PUT/PATCH/DELETE). After the first redirect from the load balancer, subsequent writes go directly to the primary node, significantly reducing latency. -- Enforced a stricter redirect destination policy: redirects are only followed when the target host is the original ledger host or one of its subdomains (e.g. an individual node). Redirects to sibling ledgers, parent domains, unrelated hosts, or look-alike suffix domains are now rejected and never followed or cached. +- Enforced a stricter redirect destination policy: redirects are only followed when the target uses HTTPS, has the same effective port, and the host is the original ledger host or one of its subdomains (e.g. an individual node). Redirects that downgrade to HTTP, change the port, or point to sibling ledgers, parent domains, unrelated hosts, or look-alike suffix domains are now rejected and never followed or cached. ### Bugs Fixed diff --git a/sdk/confidentialledger/azure-confidentialledger/azure/confidentialledger/_redirect_caching_policy.py b/sdk/confidentialledger/azure-confidentialledger/azure/confidentialledger/_redirect_caching_policy.py index 933a58d25941..ca3f2dcbd61c 100644 --- a/sdk/confidentialledger/azure-confidentialledger/azure/confidentialledger/_redirect_caching_policy.py +++ b/sdk/confidentialledger/azure-confidentialledger/azure/confidentialledger/_redirect_caching_policy.py @@ -17,12 +17,20 @@ Redirect destination policy ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -Redirects are only followed when the target host is the original request host -or one of its subdomains (e.g. an individual ledger node). Redirects to sibling -ledgers, parent domains, unrelated hosts, or look-alike suffix domains are -rejected and never followed or cached. This prevents a misconfigured or -malicious load-balancer from redirecting requests (and their sensitive headers) -to an unintended destination. +A redirect is only followed when **all** of the following hold for the target: + +* the scheme is ``https`` (an ``https`` -> ``http`` downgrade is rejected so the + bearer token can never travel over cleartext); +* the effective port matches the original request's effective port (the scheme + default — 443 for ``https`` — is used when no port is given); and +* the host is the original request host or one of its subdomains (e.g. an + individual ledger node). + +Redirects to sibling ledgers, parent domains, unrelated hosts, look-alike suffix +domains, downgraded (``http``) schemes, or different ports are rejected and never +followed or cached. This prevents a misconfigured or malicious load-balancer from +redirecting requests (and their sensitive headers, including the ``Authorization`` +bearer token which is *not* stripped on redirect) to an unintended destination. Thread-safety ~~~~~~~~~~~~~ @@ -103,26 +111,71 @@ def _is_redirect(status_code: int) -> bool: return status_code in _REDIRECT_STATUS_CODES +def _effective_port(parsed) -> Optional[int]: + """Return the effective port for a parsed URL, applying the scheme default. + + :param parsed: A :func:`urllib.parse.urlparse` result. + :return: The explicit port, or the scheme default (443 for ``https``, + 80 for ``http``), or ``None`` if it cannot be determined. + :rtype: Optional[int] + """ + try: + explicit = parsed.port + except ValueError: + # An invalid (out-of-range) port in the URL. + return None + if explicit is not None: + return explicit + scheme = (parsed.scheme or "").lower() + if scheme == "https": + return 443 + if scheme == "http": + return 80 + return None + + def _is_allowed_redirect_target(original_url: str, target_url: str) -> bool: """Return whether *target_url* is an allowed redirect destination. - A redirect is permitted only when the target host is identical to the - original request host or is a subdomain of it. All other targets — sibling - hosts, parent domains, unrelated hosts, and look-alike suffix domains — are - rejected. Comparison is case-insensitive and ignores any port. + A redirect is permitted only when **all** of these hold: + + * the target scheme is ``https`` (no downgrade — the bearer token must never + travel over cleartext); + * the target's effective port equals the original request's effective port + (the scheme default is used when no port is present); and + * the target host is identical to the original request host or is a subdomain + of it. + + All other targets — sibling hosts, parent domains, unrelated hosts, look-alike + suffix domains, downgraded schemes, and different ports — are rejected. Host + comparison is case-insensitive. :param str original_url: The URL of the original request. :param str target_url: The redirect target URL (e.g. from a ``Location`` header). :return: True if the redirect target is permitted, otherwise False. :rtype: bool """ - original_host = (urlparse(original_url).hostname or "").lower() - target_host = (urlparse(target_url).hostname or "").lower() + original = urlparse(original_url) + target = urlparse(target_url) + + original_host = (original.hostname or "").lower() + target_host = (target.hostname or "").lower() # Fail safe: if either host cannot be determined, do not follow the redirect. if not original_host or not target_host: return False + # Require HTTPS on the target so the bearer token is never sent over cleartext. + if (target.scheme or "").lower() != "https": + return False + + # Require the same effective port (scheme default applied when absent). + original_port = _effective_port(original) + target_port = _effective_port(target) + if original_port is None or target_port is None or original_port != target_port: + return False + + # Require the same host or a subdomain of the original host. return target_host == original_host or target_host.endswith("." + original_host) diff --git a/sdk/confidentialledger/azure-confidentialledger/tests/test_redirect_caching_policy.py b/sdk/confidentialledger/azure-confidentialledger/tests/test_redirect_caching_policy.py index 7187b1b9bd19..b62b6707980f 100644 --- a/sdk/confidentialledger/azure-confidentialledger/tests/test_redirect_caching_policy.py +++ b/sdk/confidentialledger/azure-confidentialledger/tests/test_redirect_caching_policy.py @@ -32,6 +32,11 @@ PARENT_URL = "https://confidential-ledger.azure.com/app/transactions" UNRELATED_URL = "https://evil.example.com/app/transactions" SUFFIX_LOOKALIKE_URL = "https://test-ledger.confidential-ledger.azure.com.evil.com/app/transactions" +# Scheme downgrade and port-mismatch variants of otherwise-valid hosts. +DOWNGRADE_SAME_HOST_URL = "http://test-ledger.confidential-ledger.azure.com/app/transactions" +DOWNGRADE_NODE_URL = "http://node3.test-ledger.confidential-ledger.azure.com/app/transactions" +DIFFERENT_PORT_HOST_URL = "https://test-ledger.confidential-ledger.azure.com:8443/app/transactions" +DIFFERENT_PORT_NODE_URL = "https://node3.test-ledger.confidential-ledger.azure.com:8443/app/transactions" # ── helpers ────────────────────────────────────────────────────────────────── @@ -113,7 +118,9 @@ def test_subdomain_allowed(self): is True ) - def test_same_host_ignores_port(self): + def test_same_host_explicit_default_port_allowed(self): + # Original has an implicit 443; an explicit :443 target is the same + # effective port and must be allowed. assert ( _is_allowed_redirect_target( LEDGER_URL, @@ -122,6 +129,27 @@ def test_same_host_ignores_port(self): is True ) + def test_subdomain_explicit_default_port_allowed(self): + assert ( + _is_allowed_redirect_target( + LEDGER_URL, + "https://node3.test-ledger.confidential-ledger.azure.com:443/app", + ) + is True + ) + + def test_http_downgrade_same_host_blocked(self): + assert _is_allowed_redirect_target(LEDGER_URL, DOWNGRADE_SAME_HOST_URL) is False + + def test_http_downgrade_subdomain_blocked(self): + assert _is_allowed_redirect_target(LEDGER_URL, DOWNGRADE_NODE_URL) is False + + def test_different_port_same_host_blocked(self): + assert _is_allowed_redirect_target(LEDGER_URL, DIFFERENT_PORT_HOST_URL) is False + + def test_different_port_subdomain_blocked(self): + assert _is_allowed_redirect_target(LEDGER_URL, DIFFERENT_PORT_NODE_URL) is False + def test_case_insensitive(self): assert ( _is_allowed_redirect_target( @@ -246,7 +274,17 @@ def test_permit_redirects_false_skips_redirect(self): assert policy._cache.get() is None @pytest.mark.parametrize( - "target", [SIBLING_URL, PARENT_URL, UNRELATED_URL, SUFFIX_LOOKALIKE_URL] + "target", + [ + SIBLING_URL, + PARENT_URL, + UNRELATED_URL, + SUFFIX_LOOKALIKE_URL, + DOWNGRADE_SAME_HOST_URL, + DOWNGRADE_NODE_URL, + DIFFERENT_PORT_HOST_URL, + DIFFERENT_PORT_NODE_URL, + ], ) def test_disallowed_redirect_not_followed(self, target): redirect_resp = _make_response(307, {"Location": target}) @@ -369,7 +407,17 @@ async def test_transport_error_invalidates_cache(self): @pytest.mark.asyncio @pytest.mark.parametrize( - "target", [SIBLING_URL, PARENT_URL, UNRELATED_URL, SUFFIX_LOOKALIKE_URL] + "target", + [ + SIBLING_URL, + PARENT_URL, + UNRELATED_URL, + SUFFIX_LOOKALIKE_URL, + DOWNGRADE_SAME_HOST_URL, + DOWNGRADE_NODE_URL, + DIFFERENT_PORT_HOST_URL, + DIFFERENT_PORT_NODE_URL, + ], ) async def test_disallowed_redirect_not_followed(self, target): redirect_resp = _make_response(307, {"Location": target}) From e29f4e5caf3b734e111e5d71ea02384dca5783cc Mon Sep 17 00:00:00 2001 From: ryazhang Date: Fri, 5 Jun 2026 13:51:28 -0400 Subject: [PATCH 4/4] [confidentialledger] Fix pylint docstring-missing-type for _effective_port --- .../azure/confidentialledger/_redirect_caching_policy.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/sdk/confidentialledger/azure-confidentialledger/azure/confidentialledger/_redirect_caching_policy.py b/sdk/confidentialledger/azure-confidentialledger/azure/confidentialledger/_redirect_caching_policy.py index ca3f2dcbd61c..7fbf8c798fba 100644 --- a/sdk/confidentialledger/azure-confidentialledger/azure/confidentialledger/_redirect_caching_policy.py +++ b/sdk/confidentialledger/azure-confidentialledger/azure/confidentialledger/_redirect_caching_policy.py @@ -44,7 +44,7 @@ import logging import threading from typing import FrozenSet, Optional -from urllib.parse import urlparse, urlunparse +from urllib.parse import ParseResult, urlparse, urlunparse from azure.core.pipeline import PipelineRequest, PipelineResponse from azure.core.pipeline.policies import AsyncHTTPPolicy, HTTPPolicy @@ -111,10 +111,11 @@ def _is_redirect(status_code: int) -> bool: return status_code in _REDIRECT_STATUS_CODES -def _effective_port(parsed) -> Optional[int]: +def _effective_port(parsed: ParseResult) -> Optional[int]: """Return the effective port for a parsed URL, applying the scheme default. :param parsed: A :func:`urllib.parse.urlparse` result. + :type parsed: ~urllib.parse.ParseResult :return: The explicit port, or the scheme default (443 for ``https``, 80 for ``http``), or ``None`` if it cannot be determined. :rtype: Optional[int]