diff --git a/connectors/dataverse_connector.py b/connectors/dataverse_connector.py index ff3351b8..1529188b 100644 --- a/connectors/dataverse_connector.py +++ b/connectors/dataverse_connector.py @@ -21,6 +21,7 @@ augment_low_id_like_for_persist, ) +from .microsoft_identity import assert_allowlisted_microsoft_token_url from .url_guard import ( build_pinned_httpx_client, merge_host_pins, @@ -75,6 +76,7 @@ def _dataverse_token(target: dict[str, Any]) -> str | None: auth.get("token_url") or f"https://login.microsoftonline.com/{tenant_id}/oauth2/v2.0/token" ) + assert_allowlisted_microsoft_token_url(token_url) # SSRF guard (#1232 / #832 / #1552): custom token_url receives client_secret — # validate + pin peer IPs (no DNS rebinding on the secret-bearing request). token_kwargs: dict[str, Any] = { diff --git a/connectors/microsoft_identity.py b/connectors/microsoft_identity.py new file mode 100644 index 00000000..c92d1cfc --- /dev/null +++ b/connectors/microsoft_identity.py @@ -0,0 +1,33 @@ +"""Microsoft identity (Azure AD) token endpoint host allowlist (#2007). + +HubSpot uses a fixed vendor API host before attaching secrets (#1607). Power BI and +Dataverse POST client secrets to ``auth.token_url``; IP pinning alone does not block +a public attacker host. Only ``login.microsoftonline.com`` is allowlisted today. +""" + +from __future__ import annotations + +from urllib.parse import urlparse + +_MICROSOFT_TOKEN_HOST_TAG = "#2007" +_ALLOWLISTED_LOGIN_HOST = "login.microsoftonline.com" + + +def assert_allowlisted_microsoft_token_url(url: str) -> None: + """Refuse client-credential token exchange to non-Microsoft login hosts.""" + parsed = urlparse((url or "").strip()) + if parsed.scheme.lower() != "https": + raise ValueError( + "auth.token_url must use https:// and host " + f"{_ALLOWLISTED_LOGIN_HOST!r} ({_MICROSOFT_TOKEN_HOST_TAG})." + ) + host = (parsed.hostname or "").lower() + if host != _ALLOWLISTED_LOGIN_HOST: + raise ValueError( + f"auth.token_url host {host!r} is not allowlisted; expected " + f"{_ALLOWLISTED_LOGIN_HOST!r} ({_MICROSOFT_TOKEN_HOST_TAG})." + ) + if parsed.username or parsed.password: + raise ValueError( + f"auth.token_url must not include userinfo ({_MICROSOFT_TOKEN_HOST_TAG})." + ) diff --git a/connectors/powerbi_connector.py b/connectors/powerbi_connector.py index e3892e7b..1f507547 100644 --- a/connectors/powerbi_connector.py +++ b/connectors/powerbi_connector.py @@ -21,6 +21,7 @@ augment_low_id_like_for_persist, ) +from .microsoft_identity import assert_allowlisted_microsoft_token_url from .url_guard import ( build_pinned_httpx_client, merge_host_pins, @@ -61,6 +62,7 @@ def _get_access_token(target: dict[str, Any]) -> str | None: token_url = auth.get("token_url") or _AZURE_TOKEN_URL_TMPL.format( tenant_id=tenant_id ) + assert_allowlisted_microsoft_token_url(token_url) # SSRF guard (#832 / #1552): custom token_url receives client_secret via POST — # validate + pin peer IPs (no DNS rebinding on the secret-bearing request). token_kwargs: dict[str, Any] = { diff --git a/connectors/rest_connector.py b/connectors/rest_connector.py index 97831496..de6bdec9 100644 --- a/connectors/rest_connector.py +++ b/connectors/rest_connector.py @@ -90,6 +90,15 @@ def _auth_loads_secrets_from_env(auth: dict[str, Any]) -> bool: return False +def _target_loads_secrets_from_env(target: dict[str, Any]) -> bool: + """True when loader resolved target credentials from env (#2006).""" + if (target.get("pass_from_env") or target.get("password_from_env") or "").strip(): + return True + if (target.get("user_from_env") or "").strip(): + return True + return False + + def _validate_rest_env_var_names(auth: dict[str, Any]) -> None: for key, label in ( ("token_from_env", "auth.token_from_env"), @@ -105,6 +114,22 @@ def _validate_rest_env_var_names(auth: dict[str, Any]) -> None: ) +def _validate_target_env_var_names(target: dict[str, Any]) -> None: + for key, label in ( + ("pass_from_env", "pass_from_env"), + ("password_from_env", "password_from_env"), + ("user_from_env", "user_from_env"), + ): + env_name = (target.get(key) or "").strip() + if not env_name: + continue + if not any(env_name.startswith(prefix) for prefix in _REST_ENV_VAR_PREFIXES): + raise ValueError( + f"{label} must use one of the prefixes " + f"{', '.join(_REST_ENV_VAR_PREFIXES)} ({_REST_AUTH_HOST_TAG})." + ) + + def _credential_endpoint_urls(target: dict[str, Any]) -> list[tuple[str, str]]: urls: list[tuple[str, str]] = [] base = (target.get("base_url") or target.get("url") or "").strip() @@ -160,9 +185,12 @@ def _rest_auth_will_attach_credentials(target: dict[str, Any]) -> bool: def _assert_rest_credential_hosts_allowlisted(target: dict[str, Any]) -> None: """Refuse Bearer/client_secret unless each endpoint host is allowlisted (#1977).""" auth = target.get("auth") or {} - uses_env = _auth_loads_secrets_from_env(auth) + uses_env = _auth_loads_secrets_from_env(auth) or _target_loads_secrets_from_env( + target + ) if uses_env: _validate_rest_env_var_names(auth) + _validate_target_env_var_names(target) explicit = _collect_explicit_allowed_hosts(target) if uses_env and not explicit: raise ValueError( diff --git a/tests/test_powerbi_dataverse_http_surfaces.py b/tests/test_powerbi_dataverse_http_surfaces.py index 8ba037b2..e9514c36 100644 --- a/tests/test_powerbi_dataverse_http_surfaces.py +++ b/tests/test_powerbi_dataverse_http_surfaces.py @@ -5,8 +5,12 @@ import httpx import pytest -from connectors.dataverse_connector import DataverseConnector -from connectors.powerbi_connector import PowerBIConnector +from connectors.dataverse_connector import DataverseConnector, _dataverse_token +from connectors.powerbi_connector import ( + PowerBIConnector, + _AZURE_TOKEN_URL_TMPL, + _get_access_token, +) def _mk_scanner(): @@ -27,6 +31,79 @@ def _oauth_token_response_ok() -> MagicMock: return m +@patch("connectors.powerbi_connector.pinned_httpx_request") +def test_powerbi_default_template_token_url_posts_to_microsoft_login( + mock_pinned_req, +) -> None: + """#2007: default Azure AD URL from template must pass vendor host allowlist.""" + mock_pinned_req.return_value = _oauth_token_response_ok() + tenant_id = "11111111-2222-3333-4444-555555555555" + _get_access_token( + { + "tenant_id": tenant_id, + "client_id": "cid", + "client_secret": "sec", + } + ) + mock_pinned_req.assert_called_once() + assert mock_pinned_req.call_args[0][1] == _AZURE_TOKEN_URL_TMPL.format( + tenant_id=tenant_id + ) + + +@patch("connectors.powerbi_connector.pinned_httpx_request") +def test_powerbi_rejects_attacker_token_url_before_client_secret_post( + mock_pinned_req, +) -> None: + with pytest.raises(ValueError, match="#2007"): + _get_access_token( + { + "tenant_id": "t", + "client_id": "cid", + "client_secret": "sec", + "auth": {"token_url": "https://attacker.example/oauth/token"}, + } + ) + mock_pinned_req.assert_not_called() + + +@patch("connectors.dataverse_connector.pinned_httpx_request") +def test_dataverse_default_template_token_url_posts_to_microsoft_login( + mock_pinned_req, +) -> None: + mock_pinned_req.return_value = _oauth_token_response_ok() + tenant_id = "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee" + _dataverse_token( + { + "org_url": "https://org.crm.dynamics.com", + "tenant_id": tenant_id, + "client_id": "cid", + "client_secret": "sec", + } + ) + mock_pinned_req.assert_called_once() + assert mock_pinned_req.call_args[0][1] == ( + f"https://login.microsoftonline.com/{tenant_id}/oauth2/v2.0/token" + ) + + +@patch("connectors.dataverse_connector.pinned_httpx_request") +def test_dataverse_rejects_attacker_token_url_before_client_secret_post( + mock_pinned_req, +) -> None: + with pytest.raises(ValueError, match="#2007"): + _dataverse_token( + { + "org_url": "https://org.crm.dynamics.com", + "tenant_id": "t", + "client_id": "cid", + "client_secret": "sec", + "auth": {"token_url": "https://attacker.example/oauth/token"}, + } + ) + mock_pinned_req.assert_not_called() + + @patch("connectors.dataverse_connector.build_pinned_httpx_client") @patch("connectors.dataverse_connector.pinned_httpx_request") def test_dataverse_run_save_failure_on_entity_definitions_http_error( diff --git a/tests/test_rest_connector_pass_from_env_loader.py b/tests/test_rest_connector_pass_from_env_loader.py new file mode 100644 index 00000000..b217b6a2 --- /dev/null +++ b/tests/test_rest_connector_pass_from_env_loader.py @@ -0,0 +1,88 @@ +"""REST pass_from_env egress (#2006): loader → connector path (not hand-built targets).""" + +from __future__ import annotations + +from pathlib import Path +from types import SimpleNamespace + +import httpx +import pytest + +from config.loader import load_config +from connectors.rest_connector import _build_auth + + +def _write_and_load_rest_target(tmp_path: Path, *extra_target_lines: str) -> dict: + extra = "" + if extra_target_lines: + extra = "\n".join(f" {line}" for line in extra_target_lines) + "\n" + body = f"""targets: + - name: rest-env-exfil + type: api + base_url: https://attacker.example + paths: ["/json"] + user: labuser +{extra}report: + output_dir: . +sqlite_path: audit_loader_test.db +""" + cfg_path = tmp_path / "config.yaml" + cfg_path.write_text(body, encoding="utf-8") + return load_config(cfg_path)["targets"][0] + + +def test_pass_from_env_via_loader_requires_explicit_allowed_hosts( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Regression: env password must not use attacker base_url as implicit allowlist (#2006).""" + monkeypatch.setenv("API_REST_EXFIL_PASS", "stolen-value") + target = _write_and_load_rest_target(tmp_path, "pass_from_env: API_REST_EXFIL_PASS") + assert target["pass"] == "stolen-value" + assert target.get("pass_from_env") == "API_REST_EXFIL_PASS" + client = SimpleNamespace(headers={}, auth=None) + with pytest.raises(ValueError, match="allowed_hosts.*#1977"): + _build_auth(client, target) + assert client.auth is None + + +def test_pass_from_env_via_loader_rejects_non_allowlisted_base_host( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.setenv("API_REST_EXFIL_PASS", "stolen-value") + target = _write_and_load_rest_target( + tmp_path, + "pass_from_env: API_REST_EXFIL_PASS", + 'allowed_hosts: ["api.trusted.example"]', + ) + client = SimpleNamespace(headers={}, auth=None) + with pytest.raises(ValueError, match="base_url host.*#1977"): + _build_auth(client, target) + + +def test_pass_from_env_via_loader_rejects_disallowed_env_name_prefix( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.setenv("LAB_PG_PASSWORD", "db-secret") + target = _write_and_load_rest_target( + tmp_path, + "pass_from_env: LAB_PG_PASSWORD", + 'allowed_hosts: ["attacker.example"]', + ) + client = SimpleNamespace(headers={}, auth=None) + with pytest.raises(ValueError, match="pass_from_env.*#1977"): + _build_auth(client, target) + + +def test_pass_from_env_via_loader_allows_explicit_allowlisted_host( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.setenv("API_REST_OK_PASS", "inline-secret") + target = _write_and_load_rest_target( + tmp_path, + "pass_from_env: API_REST_OK_PASS", + 'allowed_hosts: ["attacker.example"]', + ) + client = SimpleNamespace(headers={}, auth=None) + _build_auth(client, target) + expected = httpx.BasicAuth("labuser", "inline-secret") + assert client.auth._auth_header == expected._auth_header diff --git a/tests/test_ssrf_guard.py b/tests/test_ssrf_guard.py index 0b022eac..41353cdd 100644 --- a/tests/test_ssrf_guard.py +++ b/tests/test_ssrf_guard.py @@ -422,7 +422,7 @@ def test_powerbi_token_url_guarded() -> None: if not _HTTPX_AVAILABLE: pytest.skip("httpx not installed") - with pytest.raises(ValueError, match="#832"): + with pytest.raises(ValueError, match="#2007|#832"): _get_access_token( { "tenant_id": "t",