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 +} 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 {