What the developer wrote
Authored by yyforyongyu
sqldb/v2: enforce require ssl mode
Apply the RequireSSL config knob when opening Postgres stores
so it cannot be silently ignored.
Before this change, sqldb/v2 exposed PostgresConfig.RequireSSL
but still opened cfg.Dsn verbatim, which meant RequireSSL=true
was a no-op. A caller could set RequireSSL=true together with a
DSN such as sslmode=disable and still establish a non-TLS
connection.
This is a v2-only API contract bug, not a v1 regression in DSN
handling: v1 never offered a separate RequireSSL flag and always
left TLS policy entirely up to the DSN. The fix rewrites the DSN
to use sslmode=require when needed, while preserving stricter
modes such as verify-ca and verify-full.
The bug was easy to miss because no test asserted that the boolean
flag changed the effective DSN or overrode an insecure sslmode.
Keep the focused DSN rewrite test because it proves the contract
without needing a live Postgres instance.
What changed, and why it matters
This commit fixes a bug in LND's new database library (sqldb/v2) where a 'require SSL' setting was silently ignored. A user could turn on RequireSSL=true, but if their database connection string also said sslmode=disable, the connection would still be made without encryption. The fix now rewrites the connection string to enforce TLS when RequireSSL is true, unless an even stricter TLS mode is already set. It also adds tests to prove the behavior.
Users of sqldb/v2 who set RequireSSL=true should upgrade to this commit or a release containing it, and verify that their Postgres connections are using TLS. Operators should audit connection strings that previously combined RequireSSL=true with sslmode=disable or other non-TLS modes. No action is needed for v1 users, which did not expose RequireSSL.
Security signals we found
TLS/SSL enforcement bypass due to ignored configuration flag
API contract bug where boolean security setting had no effect
Potential plaintext database connections despite explicit RequireSSL=true
Fix includes regression test for DSN rewriting behavior
Evidence from the diff
In sqldb/v2, PostgresConfig.RequireSSL was exposed but NewPostgresStore opened cfg.Dsn verbatim, making RequireSSL a no-op. The patch adds ensureRequiredSSLMode(), which parses the DSN and sets sslmode=require when RequireSSL is true, while preserving existing require/verify-ca/verify-full modes. It also copies the config to avoid mutating the caller’s value and adds unit tests for the DSN rewrite logic.
Changed components
sqldb/v2/postgres.gosqldb/v2/PostgresConfig.RequireSSLsqldb/v2/NewPostgresStoreInspect captured patch +103 / −4
diff --git a/sqldb/v2/postgres.go b/sqldb/v2/postgres.go
index bd4e291..d6b4912 100644
--- a/sqldb/v2/postgres.go
+++ b/sqldb/v2/postgres.go
@@ -43,6 +43,14 @@ var (
_ DB = (*PostgresStore)(nil)
)
+// sslModesRequiringTLS lists sslmode values that already enforce TLS and
+// therefore do not need to be rewritten when RequireSSL is set.
+var sslModesRequiringTLS = map[string]struct{}{
+ "require": {},
+ "verify-ca": {},
+ "verify-full": {},
+}
+
// replacePasswordInDSN takes a DSN string and returns it with the password
// replaced by "***".
func replacePasswordInDSN(dsn string) (string, error) {
@@ -84,6 +92,28 @@ func getDatabaseNameFromDSN(dsn string) (string, error) {
return path.Base(u.Path), nil
}
+// ensureRequiredSSLMode rewrites the DSN to require TLS when requested.
+func ensureRequiredSSLMode(dsn string, requireSSL bool) (string, error) {
+ if !requireSSL {
+ return dsn, nil
+ }
+
+ u, err := url.Parse(dsn)
+ if err != nil {
+ return "", err
+ }
+
+ query := u.Query()
+ sslMode := query.Get("sslmode")
+ if _, ok := sslModesRequiringTLS[sslMode]; !ok {
+ query.Set("sslmode", "require")
+ }
+
+ u.RawQuery = query.Encode()
+
+ return u.String(), nil
+}
+
// PostgresStore is a database store implementation that uses a Postgres
// backend.
type PostgresStore struct {
@@ -95,13 +125,29 @@ type PostgresStore struct {
// NewPostgresStore creates a new store that is backed by a Postgres database
// backend.
func NewPostgresStore(cfg *PostgresConfig) (*PostgresStore, error) {
- sanitizedDSN, err := replacePasswordInDSN(cfg.Dsn)
+ if cfg == nil {
+ return nil, fmt.Errorf("postgres config is required")
+ }
+
+ // Copy the caller config so we can enforce RequireSSL on the DSN
+ // without mutating a config value that may be reused elsewhere.
+ effectiveCfg := *cfg
+
+ effectiveDSN, err := ensureRequiredSSLMode(
+ effectiveCfg.Dsn, effectiveCfg.RequireSSL,
+ )
+ if err != nil {
+ return nil, err
+ }
+ effectiveCfg.Dsn = effectiveDSN
+
+ sanitizedDSN, err := replacePasswordInDSN(effectiveCfg.Dsn)
if err != nil {
return nil, err
}
log.Infof("Using SQL database '%s'", sanitizedDSN)
- db, err := sql.Open("pgx", cfg.Dsn)
+ db, err := sql.Open("pgx", effectiveCfg.Dsn)
if err != nil {
return nil, err
}
@@ -148,11 +194,11 @@ func NewPostgresStore(cfg *PostgresConfig) (*PostgresStore, error) {
db.SetConnMaxIdleTime(connMaxIdleTime)
return &PostgresStore{
- cfg: cfg,
+ cfg: &effectiveCfg,
BaseDB: &BaseDB{
DB: db,
BackendType: BackendTypePostgres,
- SkipMigrations: cfg.SkipMigrations,
+ SkipMigrations: effectiveCfg.SkipMigrations,
},
}, nil
}
diff --git a/sqldb/v2/postgres_internal_test.go b/sqldb/v2/postgres_internal_test.go
new file mode 100644
index 0000000..408a2a3
--- /dev/null
+++ b/sqldb/v2/postgres_internal_test.go
@@ -0,0 +1,53 @@
+package sqldb
+
+import (
+ "testing"
+
+ "github.com/stretchr/testify/require"
+)
+
+// TestEnsureRequiredSSLMode verifies that the Postgres DSN is upgraded to a
+// TLS-enforcing sslmode when requested.
+func TestEnsureRequiredSSLMode(t *testing.T) {
+ t.Parallel()
+
+ testCases := []struct {
+ name string
+ dsn string
+ requireSSL bool
+ expected string
+ }{
+ {
+ name: "ssl disabled",
+ dsn: "postgres://user:pass@localhost/db?sslmode=disable",
+ requireSSL: true,
+ expected: "postgres://user:pass@localhost/db?sslmode=require",
+ },
+ {
+ name: "ssl not requested",
+ dsn: "postgres://user:pass@localhost/db?sslmode=disable",
+ requireSSL: false,
+ expected: "postgres://user:pass@localhost/db?sslmode=disable",
+ },
+ {
+ name: "strict mode preserved",
+ dsn: "postgres://user:pass@localhost/db?sslmode=verify-full",
+ requireSSL: true,
+ expected: "postgres://user:pass@localhost/db?sslmode=verify-full",
+ },
+ }
+
+ for _, testCase := range testCases {
+ testCase := testCase
+
+ t.Run(testCase.name, func(t *testing.T) {
+ t.Parallel()
+
+ result, err := ensureRequiredSSLMode(
+ testCase.dsn, testCase.requireSSL,
+ )
+ require.NoError(t, err)
+ require.Equal(t, testCase.expected, result)
+ })
+ }
+}
Why this scored 60/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.