Skip to content

fix(pg): preserve configured TLS servername - #3828

Open
ritiksahni wants to merge 1 commit into
brianc:masterfrom
ritiksahni:fix/ssl-servername
Open

ritiksahni wants to merge 1 commit into
brianc:masterfrom
ritiksahni:fix/ssl-servername

Conversation

@ritiksahni

Copy link
Copy Markdown

Fixes issue #2803

When ssl.servername is provided, Connection#upgradeToSSL initially copies it into the options passed to tls.connect(). For non-IP connection hosts, pg then unconditionally replaces it with the connection host.

This prevents callers from connecting through one hostname while using another hostname for TLS SNI and certificate verification—for example, connecting through an internal proxy while verifying the database's public hostname.

Only derive servername from the connection host when no non-empty value was supplied. Existing behavior is preserved for connections that do not configure it:

  • hostname connections still use the connection host as their TLS server name;
  • IP connections still omit derived SNI and use host for certificate verification;
  • explicitly configured TLS server names are passed through unchanged.

The regression coverage verifies this at two levels:

  1. A focused unit test checks the exact options passed to the secure stream.
  2. A real local TLS handshake verifies that both the server-observed SNI value and the client's certificate-verification hostname use the configured servername.

This continues the earlier fix proposed in #2804 by @gabegorelick and adds the regression coverage requested in that PR.

Validation

  • node packages/pg/test/unit/connection/ssl-tests.js postgres://
  • make -C packages/pg test-unit
  • corepack yarn build
  • corepack yarn lint

All of the above pass.

Keep an explicit ssl.servername instead of replacing it with the connection host. Add option-level and real TLS handshake regression coverage for issue brianc#2803.
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