wombatu-kun opened a new pull request, #16541:
URL: https://github.com/apache/iceberg/pull/16541

   Closes #16465.
   
   ## Summary
   
   `AzureKeyManagementClient` built its Key Vault `KeyClient` from an arbitrary 
`azure.keyvault.url` and authenticated using *ambient* Azure credentials — when 
no `adls.token-credential-provider` was configured, the credential chain fell 
back to `DefaultAzureCredentialBuilder().build()` (managed identity / 
environment / Azure CLI). A malicious or mis-scoped catalog configuration could 
therefore redirect Key Vault authentication traffic to an attacker-controlled 
endpoint and exfiltrate the client's ambient bearer token, as reported in 
#16465.
   
   Following the direction suggested on the issue, the Key Vault client now 
authenticates only with a credential supplied through configuration (a 
catalog-vended `adls.token`, or an explicitly configured 
`adls.token-credential-provider`) and no longer silently falls back to ambient 
`DefaultAzureCredential`. When no such credential is provided it fails fast 
with a `ValidationException`. With this change a malicious catalog can at most 
recover a credential it already issued, rather than the client's ambient 
identity.
   
   ## What changed
   
   `AzureProperties` gains `keyVaultTokenCredential()`, which returns the 
catalog-/operator-supplied credential and is empty when only ambient 
credentials would be available; the inline `TokenCredential` builder previously 
used for ADLS is extracted into a shared helper and reused. 
`AzureKeyManagementClient` now resolves its credential through this method and 
throws a `ValidationException` when none is configured. ADLS storage credential 
behavior is unchanged — this is a Key Vault–only change.
   
   Note: the supplied token must be scoped for Key Vault (for the public cloud, 
`https://vault.azure.net`). `adls.token` is reused as the carrier rather than 
introducing a new property; a dedicated `azure.keyvault.token` could be added 
as a follow-up if preferred.
   
   ## Tests
   
   `TestAzureProperties` adds unit coverage for `keyVaultTokenCredential()`: 
resolution from `adls.token`, resolution from a custom token-credential 
provider, and — critically — that it is empty (no ambient fallback) when 
nothing is configured. The integration `TestAzureKeyManagementClient` now 
asserts both sides of the new contract against a live vault: the old URL-only 
configuration fails with `ValidationException`, and wrap/unwrap works when a 
Key Vault-scoped token is provided via `adls.token`.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to