diff --git a/azure-functions-durable/CHANGELOG.md b/azure-functions-durable/CHANGELOG.md index 5bea7251..f443337c 100644 --- a/azure-functions-durable/CHANGELOG.md +++ b/azure-functions-durable/CHANGELOG.md @@ -14,6 +14,15 @@ the synchronous client into synchronous functions and the asynchronous client into coroutine functions. Both clients support scheduled-task and history-export APIs without an async-to-sync bridge. +FIXED + +- Prevented Durable HTTP calls from forwarding managed identity tokens, +authorization headers, cookies, proxy credentials, or function keys to +cross-origin redirect and polling targets. Function keys are now removed from +every `202 Accepted` poll, including same-origin polls, because the initial +function-level key may not authorize the status endpoint. Direct client +invocation of the internal HTTP polling orchestrator is now rejected. + ## 2.0.0b1 First preview (beta) release of `azure-functions-durable` 2.x — a ground-up diff --git a/azure-functions-durable/azure/durable_functions/http/builtin.py b/azure-functions-durable/azure/durable_functions/http/builtin.py index add02175..b921e772 100644 --- a/azure-functions-durable/azure/durable_functions/http/builtin.py +++ b/azure-functions-durable/azure/durable_functions/http/builtin.py @@ -46,6 +46,15 @@ # usable ``Retry-After`` header. _DEFAULT_POLL_INTERVAL_SECONDS = 1 +_HTTP_SCHEMES = frozenset(("http", "https")) +_CROSS_ORIGIN_SENSITIVE_HEADERS = frozenset(( + "authorization", + "cookie", + "proxy-authorization", + "x-functions-key", +)) +_POLL_SENSITIVE_HEADERS = frozenset(("x-functions-key",)) + # Process-wide credential cache. ``DefaultAzureCredential`` is safe to reuse and # caches tokens internally, so a single worker-local instance avoids repeating # credential-chain discovery and token acquisition on every durable HTTP @@ -79,6 +88,76 @@ def _acquire_bearer_token(resource: str) -> str: return _get_credential().get_token(scope).token +def _http_origin(uri: str) -> Optional[tuple[str, str, int]]: + """Return a normalized HTTP origin, or ``None`` for an invalid URL.""" + parsed = urlparse(uri) + scheme = parsed.scheme.lower() + if scheme not in _HTTP_SCHEMES or parsed.hostname is None: + return None + try: + port = parsed.port + except ValueError: + return None + if port is None: + port = 443 if scheme == "https" else 80 + return scheme, parsed.hostname.lower(), port + + +def _is_same_origin(first_uri: str, second_uri: str) -> bool: + """Return whether two absolute HTTP URLs have the same origin.""" + first_origin = _http_origin(first_uri) + return first_origin is not None and first_origin == _http_origin(second_uri) + + +def _without_headers( + headers: dict[str, str], + excluded_names: frozenset[str]) -> dict[str, str]: + """Copy headers except for case-insensitively excluded names.""" + return { + name: value + for name, value in headers.items() + if name.lower() not in excluded_names + } + + +class _SecureRedirectHandler(urllib.request.HTTPRedirectHandler): + """Follow only HTTP(S) redirects without leaking cross-origin secrets.""" + + def redirect_request( + self, + req: urllib.request.Request, + fp: Any, + code: int, + msg: str, + headers: Any, + newurl: str) -> Optional[urllib.request.Request]: + redirected = super().redirect_request( + req, fp, code, msg, headers, newurl) + if redirected is None: + return None + + redirected_url = redirected.full_url + if _http_origin(redirected_url) is None: + raise urllib.error.URLError( + f"Refusing redirect to non-HTTP(S) URL {redirected_url!r}.") + + if _is_same_origin(req.full_url, redirected_url): + return redirected + + for header_map in ( + redirected.headers, + redirected.unredirected_hdrs): + for name in list(header_map): + if name.lower() in _CROSS_ORIGIN_SENSITIVE_HEADERS: + del header_map[name] + return redirected + + +def _open_http_request(req: urllib.request.Request) -> Any: + """Open a request using the credential-safe redirect policy.""" + return urllib.request.build_opener(_SecureRedirectHandler()).open(req) + + def builtin_http_activity(input: dict[str, Any]) -> dict[str, Any]: """Execute a single HTTP request and return the response payload. @@ -105,9 +184,9 @@ def builtin_http_activity(input: dict[str, Any]) -> dict[str, Any]: # Durable HTTP only ever means http(s); reject other schemes (file://, # ftp://, ...) that urlopen would otherwise honor, closing off local-file # reads / SSRF to non-HTTP endpoints from orchestration-supplied URLs. - if urlparse(str(uri)).scheme.lower() not in ("http", "https"): + if _http_origin(str(uri)) is None: raise ValueError( - "call_http only supports http/https URLs; " + "call_http requires an absolute http/https URL; " f"got {uri!r}.") content = request.get("content") headers: dict[str, str] = dict(request.get("headers") or {}) @@ -125,7 +204,7 @@ def builtin_http_activity(input: dict[str, Any]) -> dict[str, Any]: req = urllib.request.Request(url=uri, data=data, method=method, headers=headers) try: - with urllib.request.urlopen(req) as resp: # noqa: S310 - user-supplied URL is the feature + with _open_http_request(req) as resp: status = int(resp.status) resp_headers = {k: v for k, v in resp.headers.items()} body = resp.read().decode("utf-8", errors="replace") @@ -202,6 +281,11 @@ def builtin_http_poll_orchestrator(context: Any) -> Generator[Any, Any, DurableH is reconstructed type-safely (required under strict typing, which will not build a custom type from a bare JSON object). """ + if context.parent_instance_id is None: + raise PermissionError( + f"{BUILTIN_HTTP_POLL_ORCHESTRATOR_NAME} can only run as a " + "sub-orchestration.") + request: dict[str, Any] = context.get_input() or {} response: dict[str, Any] = yield context.call_activity( BUILTIN_HTTP_ACTIVITY_NAME, request) @@ -221,20 +305,29 @@ def builtin_http_poll_orchestrator(context: Any) -> Generator[Any, Any, DurableH # against the current request URI so the next poll targets an absolute # http(s) URL (the built-in activity rejects non-absolute URIs). location = urljoin(current_uri, location) + if _http_origin(location) is None: + raise ValueError( + "Durable HTTP polling requires an absolute http/https " + f"Location URL; got {location!r}.") now = context.current_utc_datetime delay = _retry_after_seconds(headers, now) fire_at = now + timedelta(seconds=delay) yield context.create_timer(fire_at) + same_origin = _is_same_origin(current_uri, location) poll_request: dict[str, Any] = {"method": "GET", "uri": location} - # Preserve auth for the polling requests. if request.get("headers") is not None: - poll_request["headers"] = request["headers"] - if request.get("tokenSource") is not None: + excluded_headers = _POLL_SENSITIVE_HEADERS + if not same_origin: + excluded_headers = _CROSS_ORIGIN_SENSITIVE_HEADERS + poll_request["headers"] = _without_headers( + dict(request["headers"]), excluded_headers) + if same_origin and request.get("tokenSource") is not None: poll_request["tokenSource"] = request["tokenSource"] current_uri = location + request = poll_request response = yield context.call_activity(BUILTIN_HTTP_ACTIVITY_NAME, poll_request) return DurableHttpResponse.from_json(response) diff --git a/tests/azure-functions-durable/test_http_builtin_compat.py b/tests/azure-functions-durable/test_http_builtin_compat.py index 9d747a1d..2130d694 100644 --- a/tests/azure-functions-durable/test_http_builtin_compat.py +++ b/tests/azure-functions-durable/test_http_builtin_compat.py @@ -4,12 +4,15 @@ from datetime import datetime, timedelta, timezone from types import SimpleNamespace from unittest.mock import MagicMock, patch +import urllib.error +import urllib.request import pytest from azure.durable_functions.http.builtin import ( BUILTIN_HTTP_ACTIVITY_NAME, BUILTIN_HTTP_POLL_ORCHESTRATOR_NAME, + _SecureRedirectHandler, _retry_after_seconds, builtin_http_activity, builtin_http_poll_orchestrator, @@ -97,7 +100,7 @@ def _fake_urlopen_response(status, headers, body): def test_activity_executes_request(): fake_resp = _fake_urlopen_response(200, {"Content-Type": "application/json"}, "ok") - with patch("azure.durable_functions.http.builtin.urllib.request.urlopen", + with patch("azure.durable_functions.http.builtin._open_http_request", return_value=fake_resp): result = builtin_http_activity({"method": "GET", "uri": "http://example.com"}) assert result["status_code"] == 200 @@ -129,7 +132,7 @@ def _capture(req): fake_credential = MagicMock() fake_credential.get_token.return_value = SimpleNamespace(token="THE_TOKEN") with patch("azure.durable_functions.http.builtin._cached_credential", None), \ - patch("azure.durable_functions.http.builtin.urllib.request.urlopen", + patch("azure.durable_functions.http.builtin._open_http_request", side_effect=_capture), \ patch("azure.identity.DefaultAzureCredential", return_value=fake_credential): @@ -155,7 +158,7 @@ def _ok(req): return _fake_urlopen_response(200, {}, "ok") with patch("azure.durable_functions.http.builtin._cached_credential", None), \ - patch("azure.durable_functions.http.builtin.urllib.request.urlopen", + patch("azure.durable_functions.http.builtin._open_http_request", side_effect=_ok), \ patch("azure.identity.DefaultAzureCredential", return_value=fake_credential) as credential_ctor: @@ -172,11 +175,82 @@ def _ok(req): assert fake_credential.get_token.call_count == 2 +@pytest.mark.parametrize("target", [ + "https://other.example/next", + "http://example.com/next", +]) +def test_redirect_strips_credentials_when_origin_changes(target): + request = urllib.request.Request( + "https://example.com/start", + headers={ + "Authorization": "******", + "Cookie": "session=secret", + "Proxy-Authorization": "Basic proxy-secret", + "x-functions-key": "function-secret", + "X-Custom": "preserved", + }) + + redirected = _SecureRedirectHandler().redirect_request( + request, None, 302, "Found", {}, target) + + assert redirected is not None + redirected_headers = { + name.lower(): value + for name, value in redirected.headers.items() + } + assert "authorization" not in redirected_headers + assert "cookie" not in redirected_headers + assert "proxy-authorization" not in redirected_headers + assert "x-functions-key" not in redirected_headers + assert redirected_headers["x-custom"] == "preserved" + + +def test_redirect_preserves_credentials_for_same_origin(): + request = urllib.request.Request( + "https://example.com:443/start", + headers={ + "Authorization": "******", + "Cookie": "session=secret", + "x-functions-key": "function-secret", + }) + + redirected = _SecureRedirectHandler().redirect_request( + request, None, 302, "Found", {}, "https://example.com/next") + + assert redirected is not None + redirected_headers = { + name.lower(): value + for name, value in redirected.headers.items() + } + assert redirected_headers["authorization"] == "******" + assert redirected_headers["cookie"] == "session=secret" + assert redirected_headers["x-functions-key"] == "function-secret" + + +def test_redirect_rejects_non_http_scheme(): + request = urllib.request.Request("https://example.com/start") + + with pytest.raises(urllib.error.URLError, match="non-HTTP"): + _SecureRedirectHandler().redirect_request( + request, None, 302, "Found", {}, "ftp://example.com/next") + + +def test_non_redirectable_response_preserves_http_error(): + request = urllib.request.Request( + "https://example.com/start", method="PUT") + + with pytest.raises(urllib.error.HTTPError) as raised: + _SecureRedirectHandler().redirect_request( + request, None, 302, "Found", {}, "ftp://example.com/next") + + assert raised.value.code == 302 + + # --------------------------------------------------------------------------- # Built-in poll orchestrator # --------------------------------------------------------------------------- -def _fake_orchestration_context(request): +def _fake_orchestration_context(request, parent_instance_id="parent"): activity_calls = [] def call_activity(name, inp): @@ -191,6 +265,7 @@ def create_timer(fire_at): call_activity=call_activity, create_timer=create_timer, current_utc_datetime=datetime(2020, 1, 1, tzinfo=timezone.utc), + parent_instance_id=parent_instance_id, _activity_calls=activity_calls, ) return ctx @@ -208,8 +283,12 @@ def test_poll_orchestrator_returns_non_202_immediately(): def test_poll_orchestrator_polls_until_complete(): - request = {"method": "GET", "uri": "http://x", - "headers": {"h": "v"}, "tokenSource": {"resource": "r"}} + request = { + "method": "GET", + "uri": "http://x/start", + "headers": {"h": "v", "x-functions-key": "secret"}, + "tokenSource": {"resource": "r"}, + } ctx = _fake_orchestration_context(request) gen = builtin_http_poll_orchestrator(ctx) @@ -219,7 +298,7 @@ def test_poll_orchestrator_polls_until_complete(): # A 202 with a Location + Retry-After schedules a durable timer. timer = gen.send({ "status_code": 202, - "headers": {"Location": "http://poll", "Retry-After": "5"}, + "headers": {"Location": "/poll", "Retry-After": "5"}, "content": None, }) assert timer[0] == "timer" @@ -231,7 +310,7 @@ def test_poll_orchestrator_polls_until_complete(): assert poll_name == BUILTIN_HTTP_ACTIVITY_NAME assert poll_input == { "method": "GET", - "uri": "http://poll", + "uri": "http://x/poll", "headers": {"h": "v"}, "tokenSource": {"resource": "r"}, } @@ -243,6 +322,61 @@ def test_poll_orchestrator_polls_until_complete(): assert stop.value.value.content == "done" +def test_poll_orchestrator_does_not_forward_cross_origin_credentials(): + request = { + "method": "GET", + "uri": "https://example.com/start", + "headers": { + "Authorization": "******", + "Cookie": "session=secret", + "Proxy-Authorization": "Basic proxy-secret", + "x-functions-key": "function-secret", + "X-Custom": "preserved", + }, + "tokenSource": {"resource": "https://management.azure.com"}, + } + ctx = _fake_orchestration_context(request) + gen = builtin_http_poll_orchestrator(ctx) + + assert next(gen) == ("activity_task", 1) + gen.send({ + "status_code": 202, + "headers": {"Location": "https://other.example/operations/1"}, + "content": None, + }) + assert gen.send(None) == ("activity_task", 2) + assert ctx._activity_calls[1][1] == { + "method": "GET", + "uri": "https://other.example/operations/1", + "headers": {"X-Custom": "preserved"}, + } + + # Credentials removed at the trust boundary must not reappear on a later + # same-origin poll. + gen.send({ + "status_code": 202, + "headers": {"Location": "/operations/2"}, + "content": None, + }) + assert gen.send(None) == ("activity_task", 3) + assert ctx._activity_calls[2][1] == { + "method": "GET", + "uri": "https://other.example/operations/2", + "headers": {"X-Custom": "preserved"}, + } + + +def test_poll_orchestrator_rejects_top_level_invocation(): + ctx = _fake_orchestration_context( + {"method": "GET", "uri": "https://example.com"}, + parent_instance_id=None) + gen = builtin_http_poll_orchestrator(ctx) + + with pytest.raises(PermissionError, match="sub-orchestration"): + next(gen) + assert ctx._activity_calls == [] + + def test_poll_orchestrator_stops_when_202_has_no_location(): ctx = _fake_orchestration_context({"method": "GET", "uri": "http://x"}) gen = builtin_http_poll_orchestrator(ctx)