feat(connect): add --server-name flag for tunneled connections #678
Conversation
Allows specifying the server name sent in the TDS LOGIN7 packet separately from the dial address. Fixes connections through SSH tunnels or proxies to Azure SQL where the server validates hostname. Refs: microsoft#576
|
@microsoft-github-policy-service agree |
dlevy-msft-sql
added
enhancement
There was a problem hiding this comment.
Pull request overview
Adds support for overriding the server name used during TDS login (and related connection metadata) so sqlcmd can connect through tunnels/proxies (e.g., 127.0.0.1:11433) while authenticating as the real Azure SQL hostname.
Changes:
- Introduces
ConnectSettings.ServerNameOverrideand applies it when building the connection string. - Adds a
proxyDialerto rewrite the actual dial target while keeping the overridden server name for login. - Adds unit tests covering the new connection string behavior and dialer behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/sqlcmd/connect.go | Adds ServerNameOverride and applies it during connection string construction. |
| pkg/sqlcmd/sqlcmd.go | Attempts to attach a custom dialer when ServerNameOverride is set. |
| pkg/sqlcmd/dialer.go | Implements a dialer that rewrites dial host/port while reporting an overridden hostname. |
| pkg/sqlcmd/dialer_test.go | Adds unit tests for proxyDialer. |
| pkg/sqlcmd/sqlcmd_test.go | Extends connection string tests to cover ServerNameOverride. |
| serverName: overrideName, | ||
| targetHost: serverName, | ||
| targetPort: targetPort, | ||
| } |
There was a problem hiding this comment.
When ServerNameOverride is set, the dial rewrite is only applied if connector can be type-asserted to *mssql.Connector. If that assertion ever fails (e.g., for non-mssql.Connector implementations returned by token/AAD connectors), ServerNameOverride will still change the connection string host but the dial target won’t be rewritten back to the original -S host/port, which breaks tunneled/proxy scenarios.
Consider either (1) ensuring GetTokenBasedConnection returns a *mssql.Connector (so this always works), (2) supporting dialer injection for the token connector type, or (3) returning an explicit error when the dialer can’t be applied while ServerNameOverride is set.
| } | |
| } | |
| } else { | |
| // When ServerNameOverride is set we must be able to inject a dialer; | |
| // otherwise the connection string host is changed without rewriting | |
| // the actual dial target, which breaks tunneled/proxy scenarios. | |
| return localizer.Errorf("Server name override is not supported with the current authentication method") |
| dialer *net.Dialer | ||
| } | ||
|
|
||
| func (d *proxyDialer) DialContext(ctx context.Context, network, addr string) (net.Conn, error) { | ||
| if d.dialer == nil { | ||
| d.dialer = &net.Dialer{} | ||
| } |
There was a problem hiding this comment.
proxyDialer lazily initializes d.dialer with an unsynchronized write. If DialContext is called concurrently on the same proxyDialer instance (which is plausible when a driver.Connector is used by database/sql), this introduces a data race.
Safer options: initialize dialer eagerly (remove the nil check), store it as a non-pointer net.Dialer value, or guard initialization with sync.Once/a mutex.
| dialer *net.Dialer | |
| } | |
| func (d *proxyDialer) DialContext(ctx context.Context, network, addr string) (net.Conn, error) { | |
| if d.dialer == nil { | |
| d.dialer = &net.Dialer{} | |
| } | |
| dialer net.Dialer | |
| } | |
| func (d *proxyDialer) DialContext(ctx context.Context, network, addr string) (net.Conn, error) { |
| func (connect ConnectSettings) useServerNameOverride(protocol string, serverName string) bool { | ||
| if connect.ServerNameOverride == "" { | ||
| return false | ||
| } | ||
| if protocol == "np" || strings.HasPrefix(serverName, `\\`) { | ||
| return false | ||
| } | ||
| return true |
There was a problem hiding this comment.
useServerNameOverride introduces logic to skip applying the override for named pipes (protocol == "np" or \\...), but there’s no test exercising the override+named-pipe combination. A regression here would be easy to miss since ConnectionString() behavior changes based on both ServerName and ServerNameOverride.
Add a unit test in TestConnectionStringFromSqlCmd (or a new test) that sets ServerName to a named-pipe value and also sets ServerNameOverride, asserting that the resulting connection string is unchanged (override ignored).
2 participants
Problem
When connecting to Azure SQL through an SSH tunnel or proxy (e.g.,
127.0.0.1:11433), connections fail because SQL Server validates the hostname in the TDS LOGIN7 packet. The server receives127.0.0.1as the server name and rejects it with "Cannot open server '127.0.0.1' requested by the login".Current workaround requires adding
/etc/hostsentries mapping the real server name to 127.0.0.1.Fixes #576
Solution
Add a
--server-nameflag that specifies the server name to send in the login packet, separate from the dial address (-S).# Connect via tunnel on localhost:11433, authenticate as the real server sqlcmd -S 127.0.0.1,11433 --server-name myserver.database.windows.net \ -U myuser -P mypass -N -CImplementation
ServerNameOverridefield toConnectSettings-Shost/port