CXH-2349: harden account hostname resolution - #59
Conversation
Match azure/gcp host suffixes on a DNS label boundary so look-alike domains (e.g. evilazuredatabricks.net) no longer resolve to the Azure account host. Drop the unused defaultHost const and document the AWS fall-through as deliberate.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| // label boundary. Plain strings.HasSuffix would let evilazuredatabricks.net | ||
| // match azuredatabricks.net; requiring the leading dot prevents that. | ||
| func hostMatches(hostname, suffix string) bool { | ||
| return hostname == suffix || strings.HasSuffix(hostname, "."+suffix) |
There was a problem hiding this comment.
🟡 Suggestion: hostMatches is case-sensitive, but DNS hostnames are not — a config value like MyOrg.AzureDatabricks.net (or a trailing-dot FQDN) falls through to the default branch and yields accounts.MyOrg.AzureDatabricks.net instead of the canonical Azure account host. The failure is fail-safe rather than a spoofing bypass, but since this function's job is host classification it's worth normalising first, e.g. hostname = strings.ToLower(strings.TrimSuffix(hostname, ".")) in GetAccountHostname before matching.
| default: | ||
| // AWS and unknown hosts have no canonical account suffix to normalise to, | ||
| // so prefix the host as given. | ||
| return "accounts." + hostname |
There was a problem hiding this comment.
🟡 Suggestion: The comment is inaccurate for AWS — AWS does have a canonical account host (accounts.cloud.databricks.com), which is exactly the defaultHost constant this PR deletes. Real AWS workspace hostnames look like dbc-1234abcd.cloud.databricks.com, so this branch produces accounts.dbc-1234abcd.cloud.databricks.com, which is not a valid account endpoint (the added test encodes this as "aws falls through unnormalised"). This is pre-existing behaviour and users can work around it with --account-hostname, so not blocking, but consider keeping defaultHost = "cloud.databricks.com" and adding a third hostMatches(hostname, defaultHost) case for symmetry with Azure/GCP; at minimum reword the comment so it doesn't claim AWS has no canonical suffix.
Connector PR Review: CXH-2349: harden account hostname resolutionBlocking Issues: 0 | Suggestions: 0 | Threads Resolved: 0 Review SummaryThe full PR diff was scanned for security and correctness; the new commit touches only Security IssuesNone found. Correctness IssuesNone found. SuggestionsNone. |
| // label boundary. Plain strings.HasSuffix would let evilazuredatabricks.net | ||
| // match azuredatabricks.net; requiring the leading dot prevents that. | ||
| func hostMatches(hostname, suffix string) bool { | ||
| return hostname == suffix || strings.HasSuffix(hostname, "."+suffix) |
There was a problem hiding this comment.
fyi / out of scope: this is still case-sensitive — MyOrg.AzureDatabricks.NET falls through instead of hitting azure. pre-existing under HasSuffix, not blocking this PR.
| // label boundary. Plain strings.HasSuffix would let evilazuredatabricks.net | ||
| // match azuredatabricks.net; requiring the leading dot prevents that. | ||
| func hostMatches(hostname, suffix string) bool { | ||
| return hostname == suffix || strings.HasSuffix(hostname, "."+suffix) |
There was a problem hiding this comment.
fyi / out of scope: trailing FQDN dot (….azuredatabricks.net.) also misses. atypical input, not blocking.
| return "accounts." + gcpHost | ||
| default: | ||
| // AWS and unknown hosts have no canonical account suffix to normalise to, | ||
| // so prefix the host as given. |
There was a problem hiding this comment.
nit: AWS does have a canonical account host (accounts.cloud.databricks.com). maybe rephrase — something like workspace AWS hosts stay prefixed as-is on purpose, rather than "no canonical suffix"?
Lowercase and trim a trailing root dot in GetAccountHostname so mixed-case or dotted Azure/GCP hosts resolve to the right account host instead of falling through. Restore the AWS defaultHost so workspace hosts like dbc-1234.cloud.databricks.com resolve to accounts.cloud.databricks.com rather than an invalid accounts.dbc-1234... endpoint.
5 participants
The Databricks connector no longer mistakes a look-alike domain for the real Azure or GCP host when it works out the account address, closing a spoofing gap. Behaviour is unchanged for legitimate hostnames.