From 4d6aa4c54cd975b4e8ee26805f875806b7782b7d Mon Sep 17 00:00:00 2001 From: Kiran Muddukrishna Date: Tue, 10 Nov 2026 15:08:07 +1100 Subject: [PATCH 1/2] fix(dbconn): verify every host of a multi-host RDS connection string The RDS branch of configureTLS set up only the primary attempt: with no sslmode it dropped every failover host, and with verify-full a fallback verified against system roots, which lack the RDS bundle. --- docs/testing.md | 2 +- internal/cli/cli.go | 2 +- pkg/dbconn/ca_tls.go | 69 +-------------- pkg/dbconn/dbconn.go | 63 ++++++-------- pkg/dbconn/rds.go | 11 --- pkg/dbconn/rds_test.go | 9 -- pkg/dbconn/rds_tls.go | 95 ++++++++++++++++++++ pkg/dbconn/rds_tls_test.go | 174 +++++++++++++++++++++++++++++++++++++ pkg/dbconn/tls_attempts.go | 101 +++++++++++++++++++++ 9 files changed, 403 insertions(+), 123 deletions(-) create mode 100644 pkg/dbconn/rds_tls.go create mode 100644 pkg/dbconn/rds_tls_test.go create mode 100644 pkg/dbconn/tls_attempts.go diff --git a/docs/testing.md b/docs/testing.md index a1889dc2..25f25252 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -466,7 +466,7 @@ same package cover the rest of it. | CLI grammar / config | [internal/cli](../internal/cli/cli_test.go) | | Pool config, bounded session timeouts | [pkg/dbconn](../pkg/dbconn/pool_config_test.go), [integration](../pkg/dbconn/dbconn_integration_test.go) | | Retry classification and behavior | [pkg/dbconn/retry_test.go](../pkg/dbconn/retry_test.go) | -| RDS/Aurora TLS (unit) | [pkg/dbconn/rds_test.go](../pkg/dbconn/rds_test.go) | +| RDS/Aurora TLS (unit), every host of a multi-host URL included | [pkg/dbconn/rds_test.go](../pkg/dbconn/rds_test.go), [multi-host (unit)](../pkg/dbconn/rds_tls_test.go) | | Verify-full TLS against a live TLS-only server; a CA bundle never falls back to plaintext against a server that accepts both (`TestTLSIntegrationNoPlaintextFallback`, `StartPostgresTLSOrPlaintext`) | [pkg/dbconn/tls_integration_test.go](../pkg/dbconn/tls_integration_test.go), [CA pinning (unit)](../pkg/dbconn/ca_tls_test.go) | | Targeted blocker termination | [pkg/dbconn/dbconn_integration_test.go](../pkg/dbconn/dbconn_integration_test.go) | | Test harness self-checks | [internal/testutil](../internal/testutil/postgres_test.go) | diff --git a/internal/cli/cli.go b/internal/cli/cli.go index 0e2fb8c9..85b3211e 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -46,7 +46,7 @@ func New(version string) *CLI { // database, so every entry point carries the same bounded session defaults. type DBFlags struct { URL string `help:"PostgreSQL connection URL or key=value DSN." env:"PGSPRITE_URL" required:""` - CACert string `help:"CA bundle path for verify-full TLS. Overrides the URL's sslmode: every network host is verified against the bundle, with no plaintext or unverified fallback; refused with sslmode=disable. RDS/Aurora endpoints verify with the embedded bundle automatically." env:"PGSPRITE_CA_CERT" type:"existingfile"` + CACert string `help:"CA bundle path for verify-full TLS. Overrides the URL's sslmode: every network host is verified against the bundle, with no plaintext or unverified fallback; refused with sslmode=disable. RDS/Aurora endpoints verify with the embedded bundle automatically, every host of a multi-host URL included." env:"PGSPRITE_CA_CERT" type:"existingfile"` LockTimeout time.Duration `help:"Session lock_timeout applied to every statement. At least 1ms; 0 uses the default." default:"3s"` StatementTimeout time.Duration `help:"Session statement_timeout applied to every statement. At least 1ms; 0 uses the default." default:"30s"` Debug bool `help:"Log statement-level tracing and lifecycle diagnostics to stderr."` diff --git a/pkg/dbconn/ca_tls.go b/pkg/dbconn/ca_tls.go index 4ca26256..977eebf5 100644 --- a/pkg/dbconn/ca_tls.go +++ b/pkg/dbconn/ca_tls.go @@ -1,10 +1,8 @@ package dbconn import ( - "crypto/tls" "errors" - "github.com/jackc/pgx/v5/pgconn" "github.com/jackc/pgx/v5/pgxpool" ) @@ -13,76 +11,17 @@ import ( // encrypted session and the mode asks for none, and neither is safe to guess. var ErrCACertWithSSLModeDisable = errors.New("a CA bundle (--ca-cert, PGSPRITE_CA_CERT, or Config.CACertPath) requires TLS but the connection string selects sslmode=disable; remove sslmode=disable or the CA bundle") -// hostPort identifies one dial target of a possibly multi-host connection. -type hostPort struct { - host string - port uint16 -} - // pinToCABundle makes every network connection attempt of pc verify-full -// against the CA bundle at caCertPath, each against its own host name. -// -// pgx expresses sslmode=prefer (the default) and sslmode=allow as extra -// fallback attempts per host: an unencrypted one, or an encrypted one that -// skips verification. Keeping those would let a server that fails the -// verified handshake — a certificate from another CA, or a man in the middle -// — push the pool onto a connection that is unencrypted or unauthenticated. -// Each distinct host and port keeps exactly one attempt, so multi-host -// failover stays while every attempt is verified. Each attempt keeps the -// client certificate and ALPN protocols pgx parsed for it (sslcert / sslkey, -// sslnegotiation=direct), since they authenticate the client, not the server. -// -// A unix-domain socket attempt keeps pgx's nil TLS config: PostgreSQL does -// not speak TLS over a socket, libpq ignores sslmode for one, and the socket -// never leaves the host, so there is no server to verify. +// against the CA bundle at cfg.CACertPath, each against its own host name, with +// no unencrypted or unverified fallback (see pinEveryAttempt). func pinToCABundle(pc *pgxpool.Config, cfg Config) error { if dsnSSLMode(cfg.URL) == "disable" { return ErrCACertWithSSLModeDisable } - cc := pc.ConnConfig - verified, err := caTLSConfig(cfg.CACertPath, cc.Host) + roots, err := loadCABundle(cfg.CACertPath) if err != nil { return err } - - // The client-side settings pgx parsed for each target, from the first - // attempt at it that carries TLS: sslmode=allow tries plaintext first. - parsed := map[hostPort]*tls.Config{} - note := func(target hostPort, c *tls.Config) { - if c != nil && parsed[target] == nil { - parsed[target] = c - } - } - note(hostPort{cc.Host, cc.Port}, cc.TLSConfig) - for _, fb := range cc.Fallbacks { - note(hostPort{fb.Host, fb.Port}, fb.TLSConfig) - } - attempt := func(target hostPort) *tls.Config { - if network, _ := pgconn.NetworkAddress(target.host, target.port); network == "unix" { - return nil - } - c := verified.Clone() - c.ServerName = target.host - if p := parsed[target]; p != nil { - c.Certificates = p.Certificates - c.GetClientCertificate = p.GetClientCertificate - c.NextProtos = p.NextProtos - } - return c - } - - primary := hostPort{cc.Host, cc.Port} - cc.TLSConfig = attempt(primary) - seen := map[hostPort]bool{primary: true} - fallbacks := make([]*pgconn.FallbackConfig, 0, len(cc.Fallbacks)) - for _, fb := range cc.Fallbacks { - target := hostPort{fb.Host, fb.Port} - if seen[target] { - continue - } - seen[target] = true - fallbacks = append(fallbacks, &pgconn.FallbackConfig{Host: fb.Host, Port: fb.Port, TLSConfig: attempt(target)}) - } - cc.Fallbacks = fallbacks + pinEveryAttempt(&pc.ConnConfig.Config, roots) return nil } diff --git a/pkg/dbconn/dbconn.go b/pkg/dbconn/dbconn.go index 784579e4..3864112e 100644 --- a/pkg/dbconn/dbconn.go +++ b/pkg/dbconn/dbconn.go @@ -64,9 +64,12 @@ type Config struct { // socket. A URL that selects sslmode=disable alongside it is refused with // ErrCACertWithSSLModeDisable. Unset, RDS/Aurora endpoints in // the partitions the embedded bundle covers are auto-verified with it - // (see rds.go); an RDS/Aurora endpoint in any other partition (GovCloud) - // refuses to open with ErrRDSBundleDoesNotCoverHost unless its - // connection string selects an sslmode that needs no roots. + // (see rds.go), each host of a multi-host URL against its own name; an + // RDS/Aurora endpoint in any other partition (GovCloud) refuses to open + // with ErrRDSBundleDoesNotCoverHost unless its connection string selects + // an sslmode that needs no roots, and a URL with no sslmode that lists an + // RDS/Aurora endpoint beside another host refuses with + // ErrMixedRDSAndOtherHosts. CACertPath string // Pool sizing and lifecycle. Zero values keep pgxpool's defaults. @@ -363,9 +366,8 @@ func (l slogTraceLogger) Log(ctx context.Context, level tracelog.LogLevel, msg s l.logger.Log(ctx, slogLevel, msg, attrs...) } -// caTLSConfig builds a verify-full TLS config trusting only the given CA -// bundle, verifying the server certificate against host. -func caTLSConfig(caCertPath, host string) (*tls.Config, error) { +// loadCABundle reads the CA bundle at caCertPath into a cert pool. +func loadCABundle(caCertPath string) (*x509.CertPool, error) { pem, err := os.ReadFile(caCertPath) if err != nil { return nil, fmt.Errorf("read CA bundle: %w", err) @@ -374,11 +376,7 @@ func caTLSConfig(caCertPath, host string) (*tls.Config, error) { if !roots.AppendCertsFromPEM(pem) { return nil, fmt.Errorf("CA bundle %s contains no usable certificates", caCertPath) } - return &tls.Config{ - RootCAs: roots, - ServerName: host, - MinVersion: tls.VersionTLS12, - }, nil + return roots, nil } // configureTLS decides the pool's TLS setup: an explicit CA bundle wins; @@ -386,40 +384,33 @@ func caTLSConfig(caCertPath, host string) (*tls.Config, error) { // global bundle (see rds.go); everything else keeps whatever the connection // string asked for. // -// An RDS/Aurora endpoint the embedded bundle cannot verify never falls -// through to pgx's default, which is encrypted-but-unauthenticated with a -// plaintext fallback: without an sslmode of its own, or with one that asks -// for verification the bundle cannot supply, it is an error naming the -// remedy. An explicit sslmode that needs no roots is honored on every -// RDS/Aurora endpoint alike. +// Every host of a multi-host connection string is judged, not just the +// first. An RDS/Aurora endpoint the embedded bundle cannot verify never +// falls through to pgx's default, which is encrypted-but-unauthenticated +// with a plaintext fallback: without an sslmode of its own, or with one that +// asks for verification the bundle cannot supply, it is an error naming the +// host and the remedy, and so is a host beside an RDS/Aurora endpoint that +// is not one when no sslmode is named. An explicit sslmode that needs no +// roots is honored on every attempt alike. func configureTLS(pc *pgxpool.Config, cfg Config) error { - host := pc.ConnConfig.Host + cc := &pc.ConnConfig.Config switch { case cfg.CACertPath != "": return pinToCABundle(pc, cfg) - case !IsRDSHost(host): - // Not a managed endpoint: the connection string's choice stands. + case !listsRDSHost(cc): + // No managed endpoint: the connection string's choice stands. + return nil case dsnNamesSSLMode(cfg.URL): - // The caller chose an sslmode; honor it — but when verification + // The caller chose an sslmode; honor it — but where verification // was requested without a root bundle, supply the embedded RDS // roots, which are not in system trust stores. - if !requestsVerificationWithoutRoots(pc.ConnConfig.TLSConfig) { - return nil - } - if !RDSBundleCovers(host) { - return uncoveredRDSHostError(host) - } - pc.ConnConfig.TLSConfig.RootCAs = rdsRootPool() - case !RDSBundleCovers(host): - return uncoveredRDSHostError(host) + return supplyRDSRoots(cc) default: - // No explicit sslmode on an RDS/Aurora endpoint: auto-enable - // verify-full with the embedded bundle, and drop the plaintext - // fallbacks the default sslmode would otherwise allow. - pc.ConnConfig.TLSConfig = rdsTLSConfig(host) - pc.ConnConfig.Fallbacks = nil + // No explicit sslmode: auto-enable verify-full with the embedded + // bundle on every host, dropping the plaintext fallbacks the + // default sslmode would otherwise allow. + return pinToRDSBundle(cc) } - return nil } // requestsVerificationWithoutRoots reports whether a parsed TLS config asks diff --git a/pkg/dbconn/rds.go b/pkg/dbconn/rds.go index 7b0d74f5..98a6e174 100644 --- a/pkg/dbconn/rds.go +++ b/pkg/dbconn/rds.go @@ -1,7 +1,6 @@ package dbconn import ( - "crypto/tls" "crypto/x509" _ "embed" "errors" @@ -69,13 +68,3 @@ func rdsRootPool() *x509.CertPool { } return pool } - -// rdsTLSConfig returns a verify-full TLS config for an RDS/Aurora host using -// the embedded global bundle. -func rdsTLSConfig(host string) *tls.Config { - return &tls.Config{ - RootCAs: rdsRootPool(), - ServerName: host, - MinVersion: tls.VersionTLS12, - } -} diff --git a/pkg/dbconn/rds_test.go b/pkg/dbconn/rds_test.go index 70811d8c..0d947865 100644 --- a/pkg/dbconn/rds_test.go +++ b/pkg/dbconn/rds_test.go @@ -1,7 +1,6 @@ package dbconn import ( - "crypto/tls" "crypto/x509" "encoding/pem" "os" @@ -98,14 +97,6 @@ func TestEmbeddedRDSBundleCoversOnlyCommercialRegions(t *testing.T) { } } -func TestRDSTLSConfig(t *testing.T) { - tc := rdsTLSConfig("mydb.abc123.us-east-1.rds.amazonaws.com") - assert.Equal(t, "mydb.abc123.us-east-1.rds.amazonaws.com", tc.ServerName) - assert.NotNil(t, tc.RootCAs) - assert.Equal(t, uint16(tls.VersionTLS12), tc.MinVersion) - assert.False(t, tc.InsecureSkipVerify) -} - func TestConfigureTLS(t *testing.T) { const rdsURL = "postgres://user@mydb.abc123.us-east-1.rds.amazonaws.com:5432/app" const govCloudURL = "postgres://user@mydb.abc123.us-gov-west-1.rds.amazonaws.com:5432/app" diff --git a/pkg/dbconn/rds_tls.go b/pkg/dbconn/rds_tls.go new file mode 100644 index 00000000..b722da0d --- /dev/null +++ b/pkg/dbconn/rds_tls.go @@ -0,0 +1,95 @@ +package dbconn + +import ( + "crypto/tls" + "crypto/x509" + "errors" + "fmt" + "slices" + + "github.com/jackc/pgx/v5/pgconn" +) + +// ErrMixedRDSAndOtherHosts reports a connection string that names no sslmode +// and lists an RDS/Aurora endpoint alongside a host that is not one. The +// embedded RDS bundle verifies only the endpoint, and leaving the other host +// on pgx's default would let it fall back to an unverified or plaintext +// session, so neither is chosen silently. +var ErrMixedRDSAndOtherHosts = errors.New("the connection string lists an RDS/Aurora endpoint alongside a host that is not one, so the embedded RDS CA bundle cannot verify every host") + +// listsRDSHost reports whether any network host of cc is an RDS/Aurora +// endpoint. +func listsRDSHost(cc *pgconn.Config) bool { + return slices.ContainsFunc(networkHosts(cc), IsRDSHost) +} + +// pinToRDSBundle auto-enables verify-full with the embedded RDS bundle on a +// connection string that names no sslmode: every network host keeps one +// verified attempt (see pinEveryAttempt), so multi-host failover survives. +// +// Every network host must be an RDS/Aurora endpoint the bundle covers; the +// first that is not is refused by name, before anything is changed. +func pinToRDSBundle(cc *pgconn.Config) error { + for _, host := range networkHosts(cc) { + if !IsRDSHost(host) { + return mixedRDSHostsError(host) + } + if !RDSBundleCovers(host) { + return uncoveredRDSHostError(host) + } + } + pinEveryAttempt(cc, rdsRootPool()) + return nil +} + +// supplyRDSRoots honors the caller's explicit sslmode on every attempt, but +// gives each RDS/Aurora attempt that asks for verification without roots +// the embedded RDS roots, which are not in system trust stores. Other +// attempts — modes that need no roots, attempts with roots of their own, +// hosts that are not RDS/Aurora endpoints — are left as pgx parsed them. +// +// An RDS/Aurora attempt asking for verification in a partition the bundle +// does not cover is refused by name rather than left to fail its handshake. +func supplyRDSRoots(cc *pgconn.Config) error { + roots := rdsRootPool() + primary, err := withRDSRoots(cc.Host, cc.TLSConfig, roots) + if err != nil { + return err + } + cc.TLSConfig = primary + for _, fb := range cc.Fallbacks { + attempt, err := withRDSRoots(fb.Host, fb.TLSConfig, roots) + if err != nil { + return err + } + fb.TLSConfig = attempt + } + return nil +} + +// withRDSRoots returns the TLS config for one attempt at host: c itself, +// or, when host is an RDS/Aurora endpoint and c asks for verification +// without roots, a copy of c verifying against roots. +func withRDSRoots(host string, c *tls.Config, roots *x509.CertPool) (*tls.Config, error) { + if !IsRDSHost(host) { + return c, nil + } + if !requestsVerificationWithoutRoots(c) { + return c, nil + } + if !RDSBundleCovers(host) { + return nil, uncoveredRDSHostError(host) + } + c = c.Clone() + c.RootCAs = roots + return c, nil +} + +// mixedRDSHostsError names the host the embedded bundle cannot verify beside +// an RDS/Aurora endpoint and the two ways to verify every host. +func mixedRDSHostsError(host string) error { + return fmt.Errorf("configure TLS for %s: %w; set CACertPath to a CA bundle that verifies every host, "+ + "or choose an sslmode in the connection string (sslmode=verify-full verifies RDS/Aurora endpoints "+ + "against the embedded bundle and other hosts against the system roots)", + host, ErrMixedRDSAndOtherHosts) +} diff --git a/pkg/dbconn/rds_tls_test.go b/pkg/dbconn/rds_tls_test.go new file mode 100644 index 00000000..e8fd6060 --- /dev/null +++ b/pkg/dbconn/rds_tls_test.go @@ -0,0 +1,174 @@ +package dbconn + +import ( + "crypto/tls" + "testing" + + "github.com/jackc/pgx/v5/pgxpool" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const ( + rdsWriterHost = "writer.abc123.us-east-1.rds.amazonaws.com" + rdsReplicaHost = "replica.abc123.us-west-2.rds.amazonaws.com" + govCloudHost = "db.abc123.us-gov-west-1.rds.amazonaws.com" +) + +// parseForTLS parses dsn the way buildPoolConfig does and runs configureTLS +// on it without a CA bundle. +func parseForTLS(t *testing.T, dsn string) (*pgxpool.Config, error) { + t.Helper() + pc, err := pgxpool.ParseConfig(dsn) + require.NoError(t, err) + return pc, configureTLS(pc, Config{URL: dsn}) +} + +// TestConfigureTLSRDSAutoTLSKeepsEveryHost proves a multi-host URL of +// RDS/Aurora endpoints with no sslmode keeps its failover hosts, each with +// exactly one attempt that verifies its own host name against the embedded +// bundle; the plaintext attempts pgx's default sslmode adds are gone. +func TestConfigureTLSRDSAutoTLSKeepsEveryHost(t *testing.T) { + pc, err := parseForTLS(t, "postgres://user@"+rdsWriterHost+":5432,"+rdsReplicaHost+":5433/app") + require.NoError(t, err) + + cc := pc.ConnConfig + require.Len(t, cc.Fallbacks, 1, "the replica stays in the failover list, once") + assert.Equal(t, rdsWriterHost, cc.Host) + assert.Equal(t, uint16(5432), cc.Port) + assert.Equal(t, rdsReplicaHost, cc.Fallbacks[0].Host) + assert.Equal(t, uint16(5433), cc.Fallbacks[0].Port) + assertVerifiesAgainstRDSBundle(t, cc.TLSConfig, rdsWriterHost) + assertVerifiesAgainstRDSBundle(t, cc.Fallbacks[0].TLSConfig, rdsReplicaHost) + assert.Equal(t, uint16(tls.VersionTLS12), cc.Fallbacks[0].TLSConfig.MinVersion) +} + +// TestConfigureTLSRDSAutoTLSKeepsClientSettings proves auto-TLS replaces +// only how the server is verified: every attempt still presents the client +// certificate from sslcert / sslkey and offers the ALPN protocol +// sslnegotiation=direct needs. +func TestConfigureTLSRDSAutoTLSKeepsClientSettings(t *testing.T) { + certPath, keyPath := writeClientCert(t, t.TempDir()) + pc, err := parseForTLS(t, "host="+rdsWriterHost+","+rdsReplicaHost+" user=user dbname=app sslnegotiation=direct sslcert="+certPath+" sslkey="+keyPath) + require.NoError(t, err) + + cc := pc.ConnConfig + require.Len(t, cc.Fallbacks, 1) + for _, c := range []*tls.Config{cc.TLSConfig, cc.Fallbacks[0].TLSConfig} { + require.NotNil(t, c) + assert.Len(t, c.Certificates, 1, "attempt for %s presents the client certificate", c.ServerName) + assert.Equal(t, []string{"postgresql"}, c.NextProtos, "attempt for %s offers the direct-negotiation protocol", c.ServerName) + } +} + +// TestConfigureTLSRDSAutoTLSLeavesUnixSocketsLocal proves a unix-domain +// socket beside an RDS/Aurora endpoint keeps pgx's nil TLS config: +// PostgreSQL offers no TLS over a socket, and the socket never leaves the +// host. +func TestConfigureTLSRDSAutoTLSLeavesUnixSocketsLocal(t *testing.T) { + pc, err := parseForTLS(t, "host="+rdsWriterHost+",/var/run/postgresql user=user dbname=app") + require.NoError(t, err) + + cc := pc.ConnConfig + assertVerifiesAgainstRDSBundle(t, cc.TLSConfig, rdsWriterHost) + require.Len(t, cc.Fallbacks, 1) + assert.Equal(t, "/var/run/postgresql", cc.Fallbacks[0].Host) + assert.Nil(t, cc.Fallbacks[0].TLSConfig, "a socket attempt carries no TLS") +} + +// TestConfigureTLSRDSAutoTLSRefusesMixedHosts proves auto-TLS fails closed +// when the URL lists an RDS/Aurora endpoint beside a host the embedded +// bundle cannot verify, whichever comes first: verifying every host would +// fail the other one's handshake, and leaving it on pgx's default would +// fall back to an unverified or plaintext session. The error names the +// host. +func TestConfigureTLSRDSAutoTLSRefusesMixedHosts(t *testing.T) { + t.Run("RDS/Aurora primary with a self-managed fallback", func(t *testing.T) { + _, err := parseForTLS(t, "host="+rdsWriterHost+",db2.example user=user dbname=app") + require.ErrorIs(t, err, ErrMixedRDSAndOtherHosts) + assert.ErrorContains(t, err, "db2.example") + }) + t.Run("self-managed primary with an RDS/Aurora fallback", func(t *testing.T) { + _, err := parseForTLS(t, "host=db1.example,"+rdsReplicaHost+" user=user dbname=app") + require.ErrorIs(t, err, ErrMixedRDSAndOtherHosts) + assert.ErrorContains(t, err, "db1.example") + }) + t.Run("RDS/Aurora fallback in a partition the embedded bundle does not cover", func(t *testing.T) { + _, err := parseForTLS(t, "host="+rdsWriterHost+","+govCloudHost+" user=user dbname=app") + require.ErrorIs(t, err, ErrRDSBundleDoesNotCoverHost) + assert.ErrorContains(t, err, govCloudHost) + }) +} + +// TestConfigureTLSRDSVerifyFullSuppliesRootsToEveryAttempt proves an +// explicit sslmode=verify-full without sslrootcert gets the embedded RDS +// roots on every RDS/Aurora attempt, not just the primary: system trust +// stores do not carry them, so a failover host would fail its handshake. +func TestConfigureTLSRDSVerifyFullSuppliesRootsToEveryAttempt(t *testing.T) { + pc, err := parseForTLS(t, "postgres://user@"+rdsWriterHost+":5432,"+rdsReplicaHost+":5433/app?sslmode=verify-full") + require.NoError(t, err) + + cc := pc.ConnConfig + require.Len(t, cc.Fallbacks, 1, "verify-full has one attempt per host") + assertVerifiesAgainstRDSBundle(t, cc.TLSConfig, rdsWriterHost) + assertVerifiesAgainstRDSBundle(t, cc.Fallbacks[0].TLSConfig, rdsReplicaHost) +} + +// TestConfigureTLSRDSVerifyFullLeavesOtherHostsOnSystemRoots proves the +// explicit verify-full path supplies roots per host: a self-managed host +// beside an RDS/Aurora endpoint keeps verifying against the system roots, +// as the caller asked, while the endpoint gets the embedded bundle. +func TestConfigureTLSRDSVerifyFullLeavesOtherHostsOnSystemRoots(t *testing.T) { + pc, err := parseForTLS(t, "host=db1.example,"+rdsReplicaHost+" user=user dbname=app sslmode=verify-full") + require.NoError(t, err) + + cc := pc.ConnConfig + require.NotNil(t, cc.TLSConfig) + assert.Equal(t, "db1.example", cc.TLSConfig.ServerName) + assert.Nil(t, cc.TLSConfig.RootCAs, "nil roots verify against the system trust store") + require.Len(t, cc.Fallbacks, 1) + assertVerifiesAgainstRDSBundle(t, cc.Fallbacks[0].TLSConfig, rdsReplicaHost) +} + +// TestConfigureTLSRDSVerifyFullRefusesUncoveredFallback proves a fallback +// in a partition the embedded bundle does not cover is refused under +// verify-full without roots, naming that host, rather than left to fail +// its handshake with an x509 error that does not say why. +func TestConfigureTLSRDSVerifyFullRefusesUncoveredFallback(t *testing.T) { + _, err := parseForTLS(t, "host="+rdsWriterHost+","+govCloudHost+" user=user dbname=app sslmode=verify-full") + require.ErrorIs(t, err, ErrRDSBundleDoesNotCoverHost) + assert.ErrorContains(t, err, govCloudHost) +} + +// TestConfigureTLSRDSExplicitModeWithoutVerificationIsHonored proves an +// explicit sslmode that needs no roots is left as the caller chose on every +// attempt: prefer keeps its unverified and plaintext attempts per host. +func TestConfigureTLSRDSExplicitModeWithoutVerificationIsHonored(t *testing.T) { + pc, err := parseForTLS(t, "host="+rdsWriterHost+","+govCloudHost+" user=user dbname=app sslmode=prefer") + require.NoError(t, err) + + cc := pc.ConnConfig + require.NotNil(t, cc.TLSConfig) + assert.True(t, cc.TLSConfig.InsecureSkipVerify) + assert.Nil(t, cc.TLSConfig.RootCAs) + gotHosts := []string{} + gotPlaintext := []bool{} + for _, fb := range cc.Fallbacks { + gotHosts = append(gotHosts, fb.Host) + gotPlaintext = append(gotPlaintext, fb.TLSConfig == nil) + } + // pgx's prefer: encrypted-unverified, then plaintext, for each host. + assert.Equal(t, []string{rdsWriterHost, govCloudHost, govCloudHost}, gotHosts) + assert.Equal(t, []bool{true, false, true}, gotPlaintext) +} + +// assertVerifiesAgainstRDSBundle checks c is a verify-full attempt for host +// against exactly the embedded RDS roots. +func assertVerifiesAgainstRDSBundle(t *testing.T, c *tls.Config, host string) { + t.Helper() + require.NotNil(t, c, "attempt for %s must not be plaintext", host) + assert.Equal(t, host, c.ServerName) + require.NotNil(t, c.RootCAs, "attempt for %s must carry roots", host) + assert.True(t, c.RootCAs.Equal(rdsRootPool()), "attempt for %s must verify against the embedded RDS bundle", host) + assert.False(t, c.InsecureSkipVerify, "attempt for %s must verify the certificate", host) +} diff --git a/pkg/dbconn/tls_attempts.go b/pkg/dbconn/tls_attempts.go new file mode 100644 index 00000000..f49f7e60 --- /dev/null +++ b/pkg/dbconn/tls_attempts.go @@ -0,0 +1,101 @@ +package dbconn + +import ( + "crypto/tls" + "crypto/x509" + + "github.com/jackc/pgx/v5/pgconn" +) + +// hostPort identifies one dial target of a possibly multi-host connection. +type hostPort struct { + host string + port uint16 +} + +// isUnixSocket reports whether target is a unix-domain socket, decided the +// way pgx decides it when it dials. +func isUnixSocket(target hostPort) bool { + network, _ := pgconn.NetworkAddress(target.host, target.port) + return network == "unix" +} + +// pinEveryAttempt makes every network connection attempt of cc verify-full +// against roots, each against its own host name. +// +// pgx expresses sslmode=prefer (the default) and sslmode=allow as extra +// fallback attempts per host: an unencrypted one, or an encrypted one that +// skips verification. Keeping those would let a server that fails the +// verified handshake — a certificate from another CA, or a man in the middle +// — push the pool onto a connection that is unencrypted or unauthenticated. +// Each distinct host and port keeps exactly one attempt, so multi-host +// failover stays while every attempt is verified. Each attempt keeps the +// client certificate and ALPN protocols pgx parsed for it (sslcert / sslkey, +// sslnegotiation=direct), since they authenticate the client, not the server. +// +// A unix-domain socket attempt keeps pgx's nil TLS config: PostgreSQL does +// not speak TLS over a socket, libpq ignores sslmode for one, and the socket +// never leaves the host, so there is no server to verify. +func pinEveryAttempt(cc *pgconn.Config, roots *x509.CertPool) { + // The client-side settings pgx parsed for each target, from the first + // attempt at it that carries TLS: sslmode=allow tries plaintext first. + parsed := map[hostPort]*tls.Config{} + note := func(target hostPort, c *tls.Config) { + if c != nil && parsed[target] == nil { + parsed[target] = c + } + } + note(hostPort{cc.Host, cc.Port}, cc.TLSConfig) + for _, fb := range cc.Fallbacks { + note(hostPort{fb.Host, fb.Port}, fb.TLSConfig) + } + attempt := func(target hostPort) *tls.Config { + if isUnixSocket(target) { + return nil + } + c := &tls.Config{ + RootCAs: roots, + ServerName: target.host, + MinVersion: tls.VersionTLS12, + } + if p := parsed[target]; p != nil { + c.Certificates = p.Certificates + c.GetClientCertificate = p.GetClientCertificate + c.NextProtos = p.NextProtos + } + return c + } + + primary := hostPort{cc.Host, cc.Port} + cc.TLSConfig = attempt(primary) + seen := map[hostPort]bool{primary: true} + var fallbacks []*pgconn.FallbackConfig + for _, fb := range cc.Fallbacks { + target := hostPort{fb.Host, fb.Port} + if seen[target] { + continue + } + seen[target] = true + fallbacks = append(fallbacks, &pgconn.FallbackConfig{Host: fb.Host, Port: fb.Port, TLSConfig: attempt(target)}) + } + cc.Fallbacks = fallbacks +} + +// networkHosts lists the host of every network attempt of cc in dial order, +// each once; unix-domain sockets are left out. +func networkHosts(cc *pgconn.Config) []string { + var hosts []string + seen := map[string]bool{} + add := func(target hostPort) { + if isUnixSocket(target) || seen[target.host] { + return + } + seen[target.host] = true + hosts = append(hosts, target.host) + } + add(hostPort{cc.Host, cc.Port}) + for _, fb := range cc.Fallbacks { + add(hostPort{fb.Host, fb.Port}) + } + return hosts +} From 0f0b780471b6576e5e3dc736ebe55d4309019171 Mon Sep 17 00:00:00 2001 From: Kiran Muddukrishna Date: Wed, 11 Nov 2026 11:13:59 +1100 Subject: [PATCH 2/2] dbconn: prove multi-host TLS failover through a live handshake With the first host refused, the pool must fail over to the second host and verify it, and must refuse an untrusted second host rather than reach it over plaintext. --- pkg/dbconn/tls_integration_test.go | 52 ++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/pkg/dbconn/tls_integration_test.go b/pkg/dbconn/tls_integration_test.go index 1417df75..b25997f7 100644 --- a/pkg/dbconn/tls_integration_test.go +++ b/pkg/dbconn/tls_integration_test.go @@ -1,6 +1,8 @@ package dbconn_test import ( + "net" + neturl "net/url" "strings" "testing" @@ -100,6 +102,56 @@ func TestTLSIntegrationNoPlaintextFallback(t *testing.T) { }) } +// TestTLSIntegrationFailsOverToAVerifiedHost proves the per-host attempts +// pinEveryAttempt builds survive pgx's real dial and handshake: with the +// first host unreachable, the pool fails over to the second host and +// verifies it, and a second host whose certificate the CA did not sign is +// refused rather than reached over plaintext. The server accepts plaintext +// too, so a fallback that skipped verification would connect. +func TestTLSIntegrationFailsOverToAVerifiedHost(t *testing.T) { + pg := testutil.StartPostgresTLSOrPlaintext(t) + url := withUnreachableFirstHost(t, pg.URL) + + t.Run("trusted CA connects to the second host over TLS", func(t *testing.T) { + pool, err := dbconn.NewPool(t.Context(), dbconn.Config{ + URL: url, + CACertPath: pg.CACertPath, + }) + require.NoError(t, err, "the pool must fail over past the unreachable host") + t.Cleanup(pool.Close) + + var sslInUse bool + require.NoError(t, pool.QueryRow(t.Context(), + "SELECT ssl FROM pg_stat_ssl WHERE pid = pg_backend_pid()").Scan(&sslInUse)) + assert.True(t, sslInUse, "the failed-over session must be TLS-encrypted") + }) + + t.Run("untrusted CA is refused on the second host", func(t *testing.T) { + _, err := dbconn.NewPool(t.Context(), dbconn.Config{ + URL: url, + CACertPath: pg.UntrustedCACertPath, + }) + require.Error(t, err, "failover must not reach the second host unverified or over plaintext") + assert.Contains(t, err.Error(), "certificate", + "failure must be a certificate verification error, got: %v", err) + }) +} + +// withUnreachableFirstHost puts a closed local port in front of the host of +// a connection URL, so pgx's first attempt is refused and it fails over. +func withUnreachableFirstHost(t *testing.T, raw string) string { + t.Helper() + l, err := new(net.ListenConfig).Listen(t.Context(), "tcp", "127.0.0.1:0") + require.NoError(t, err) + closed := l.Addr().String() + require.NoError(t, l.Close()) + + u, err := neturl.Parse(raw) + require.NoError(t, err) + u.Host = closed + "," + u.Host + return u.String() +} + // withParam appends a query parameter to a connection URL regardless of // whether it already carries parameters. func withParam(url, param string) string {