Skip to content

fix: enforce trusted-endpoint validation for all authentication methods (CWE-918) - #642

Open
tanmaya-panda1 wants to merge 1 commit into
masterfrom
security/cwe-918-validate-trusted-endpoint-all-auth
Open

fix: enforce trusted-endpoint validation for all authentication methods (CWE-918)#642
tanmaya-panda1 wants to merge 1 commit into
masterfrom
security/cwe-918-validate-trusted-endpoint-all-auth

Conversation

@tanmaya-panda1

Copy link
Copy Markdown

Remediation for IcM 842305145 (Glasswing Mythos - July, follow-on wave to Twin Flames).

Issue

_KustoClientBase.validate_endpoint() only invoked well_known_kusto_endpoints.validate_trusted_endpoint() when the token provider was a CloudInfoTokenProvider; otherwise it simply set _endpoint_validated = True and returned.

BasicTokenProvider and CallbackTokenProvider derive from TokenProviderBase directly, not from CloudInfoTokenProvider. As a result these flows skipped trusted-host validation entirely while still sending Authorization: Bearer <token> to whatever host the connection string named:

  • with_aad_user_token_authentication
  • with_aad_application_token_authentication
  • with_token_provider
  • with_async_token_provider

The Java SDK validates unconditionally for every auth type (ClientImpl.validateEndpointAsync), so Python was the outlier here rather than this being an intentional design choice.

Why the obvious fix was not enough

Simply deleting the isinstance gate would have introduced a second problem. Resolving the login endpoint calls CloudSettings.get_cloud_info_for_cluster, which issues a GET to the cluster host itself. Validating only after that call means an untrusted connection string still drives a request to an arbitrary host, which leaves an SSRF primitive in place even though the bearer token is no longer disclosed. It would also have newly required the /v1/rest/auth/metadata endpoint on hosts that callers had explicitly trusted through add_trusted_hosts, breaking custom proxies that do not implement it.

Fix

validate_trusted_endpoint / validate_hostname_is_trusted now accept the login endpoint either as a plain string (unchanged behaviour for existing callers) or as a callable that is resolved lazily. The callable is only invoked once the hostname is already known to appear in at least one cloud allow list.

Consequences:

  • A host that no login endpoint could ever make trusted is rejected with zero network calls.
  • Hosts trusted via add_trusted_hosts or an override policy, and local addresses, are resolved without any cloud metadata lookup.
  • Hosts in a built-in allow list behave exactly as before: metadata is fetched, then the precise per-cloud check runs.

client_base.validate_endpoint() passes the lookup as a lambda and no longer gates on provider type.

Tests

New azure-kusto-data/tests/test_endpoint_validation.py covers all four previously-bypassing auth methods:

  • untrusted host is rejected for token-based and callback-based auth
  • untrusted host is rejected without any network call (the SSRF assertion)
  • an explicitly trusted host does not trigger cloud metadata resolution
  • the login endpoint is resolved only for allow-listed hosts
  • passing a plain string still works

Validation

  • Baseline on master: 133 passed, 11 failed, 3 skipped, 42 errors.
  • With this change: 144 passed (133 + 11 new), with failures and errors identical to baseline. The pre-existing failures are unrelated local environment issues (pandas version assertions and token-provider tests requiring env vars).
  • ruff format --check clean across all 51 files in azure-kusto-data.
  • azure-kusto-ingest needs no separate change: both ingest_client.py and streaming_ingest_client.py delegate to the data-layer KustoClient and inherit this validation.

Known limitation, pre-existing and unchanged in scope

The async client (aio/client.py:174) calls the synchronous validate_endpoint, so metadata resolution still blocks the event loop for allow-listed hosts, and the underlying requests.get has no explicit timeout. This already affected every CloudInfo-based provider before this change. Fixing it properly requires an async CloudSettings path and is better handled as a separate PR.

…ds (CWE-918)

validate_endpoint() only invoked well_known_kusto_endpoints.validate_trusted_endpoint()
when the token provider was a CloudInfoTokenProvider. BasicTokenProvider and
CallbackTokenProvider derive from TokenProviderBase directly, so the flows built by
with_aad_user_token_authentication, with_aad_application_token_authentication,
with_token_provider and with_async_token_provider skipped host validation entirely while
still sending 'Authorization: Bearer <token>' to whatever host the connection string named.
The Java SDK (ClientImpl.validateEndpointAsync) validates unconditionally, so Python was
the outlier.

Removing the gate alone would have introduced a second problem: resolving the login
endpoint calls CloudSettings.get_cloud_info_for_cluster, which issues a GET to the cluster
itself. Validating after that call means an untrusted connection string still drives a
request to an arbitrary host, leaving an SSRF primitive, and it would newly require the
metadata endpoint on hosts that are trusted explicitly via add_trusted_hosts.

The login endpoint is therefore now resolved lazily, and only once the hostname is known to
appear in at least one cloud's allow list. Hosts that no login endpoint could ever make
trusted are rejected without any network call, and hosts trusted via add_trusted_hosts or an
override policy no longer need cloud metadata at all. Passing a plain string is still
supported.

Known limitation, pre-existing and unchanged in scope: the async client calls the
synchronous validate_endpoint, so metadata resolution still blocks the event loop for
allow-listed hosts. That needs an async CloudSettings path and is left as a follow-up.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 41fe4674-b54d-43ae-9bd2-6769c81623e4
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Test Results

    5 files  ± 0      5 suites  ±0   7m 9s ⏱️ - 4m 47s
  335 tests +11    298 ✅ +10   35 💤 ±0   2 ❌ +1 
1 675 runs  +55  1 490 ✅ +46  175 💤 ±0  10 ❌ +9 

For more details on these failures, see this check.

Results for commit 43e8594. ± Comparison against base commit 5a8a712.

@tanmaya-panda1

Copy link
Copy Markdown
Author

CI note for reviewers: the 5 build (3.x) failures on this PR are not caused by this change.

The only failing tests are test_e2e_data.py::TestE2E::test_log_analytics_query and test_log_analytics_query_async, both with AssertionError: assert 'genAIContent' == 'pageViews'. Everything else passes (298 passed, 35 skipped), including the 11 new endpoint-validation tests added here.

Root cause is Azure-side data drift, not code. application_insights_tables() (azure-kusto-data/tests/test_e2e_data.py:82-96) asserts the live workspace returns exactly this hardcoded, alphabetically ordered list:

availabilityResults, browserTimings, customEvents, customMetrics, dependencies,
exceptions, pageViews, performanceCounters, requests, resources, traces

Application Insights has since gained a genAIContent table, which sorts between exceptions and pageViews. At that position the test compares actual genAIContent against expected pageViews, producing exactly the observed error.

Two further points confirming independence from this change:

  1. The query succeeded and returned real rows, so endpoint validation permitted the connection. A regression from this PR would surface as KustoClientInvalidConnectionStringException, not a table-name mismatch.
  2. That test authenticates via with_azure_token_credential (test_e2e_data.py:111), which maps to AzureIdentityTokenCredentialProvider, a CloudInfoTokenProvider subclass. That path was already being validated before this change, so its validation behaviour is untouched here.

The fix belongs in a separate PR that makes the assertion tolerant of new tables rather than requiring an exact ordered match. Happy to open that separately if useful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant