ssl_mode=require/verify-ca/verify-full silently falls back to plaintext (deadpool Config.ssl_mode never set)

[aesslinger]

Summary

Discovered during a post-merge sanity pass on main after #35/#37/#40/#41/#42 landed (not a regression from any of those — confirmed via git log -p that this line has never been present since the file was first staged in #3).

build_pool in src/client.rs builds a deadpool_postgres::Config (cfg) and, when needs_tls(params) is true, wires in a TLS-capable connector (MakeRustlsConnect via build_tls_connector) — but it never calls cfg.ssl_mode(...). deadpool_postgres::Config::ssl_mode is Option<tokio_postgres::config::SslMode>, and when left None, the underlying tokio_postgres::Config this deadpool config produces defaults its own ssl_mode to SslMode::Prefer (confirmed in tokio-postgres-0.7.18/src/config.rs:255).

SslMode::Prefer means: attempt TLS, but silently fall back to plaintext if the server doesn't offer it. So ssl_mode=require/verify-ca/verify-full currently:

• Build the right rustls ClientConfig/verifier (this part is correct, and is exactly what fix: support client certificate authentication (mTLS) #35/fix: fold TLS params into pool cache key #37/fix: verify-ca incorrectly enforces hostname verification #40 fixed and tested).
• But never actually tell tokio_postgres's connection negotiation to require TLS at the protocol level.

Reproduction

Confirmed live against a local PostgreSQL 16 instance with ssl = off:

cargo build
echo '{"jsonrpc":"2.0","id":1,"method":"test_connection","params":{"params":{"host":"127.0.0.1","port":54320,"username":"postgres","password":"password","database":"testdb","ssl_mode":"require"}}}' \
  | ./target/debug/postgresql-plugin

Expected: connection should fail (require means TLS is mandatory; the server has no SSL to offer).
Actual: {"id":1,"jsonrpc":"2.0","result":null} — succeeds over plaintext, silently.

This defeats the entire purpose of ssl_mode=require: a user who explicitly opts into "TLS or nothing" gets an unencrypted connection with no error or warning if the server can't/won't negotiate TLS — a security-relevant gap, not just a correctness one.

Fix

In build_pool (src/client.rs), set cfg.ssl_mode(...) based on params.ssl_mode, mapping this plugin's string values to tokio_postgres::config::SslMode:

• "disable" → SslMode::Disable
• "allow" / unset/other → SslMode::Prefer (current default behavior, so this stays correct for those modes)
• "prefer" → SslMode::Prefer
• "require" / "verify-ca" / "verify-full" → SslMode::Require (chain/hostname enforcement is handled separately, at the rustls-connector layer, exactly as it is now — only the protocol-level "was TLS actually negotiated" guarantee is missing)

This mirrors the builtin driver's own mapping in build_postgres_configurations (src-tauri/src/pool_manager.rs): disable→Disable, allow|prefer→Prefer, require|verify-ca|verify-full→Require.

Test plan suggestion

Add a live-DB test (tests/live_db.rs) against a non-SSL Postgres instance (matches this repo's existing demo-db/CI fixture, which run without SSL) asserting that test_connection with ssl_mode=require returns an error, not success — this is exactly the kind of behavior a live handshake is needed to prove; the existing unit tests in client_tests.rs only cover connector construction, not negotiation.

[aesslinger]

Implementation plan

Root cause confirmed

build_pool in src/client.rs never calls cfg.ssl_mode(...) on the deadpool_postgres::Config. Left as None, the underlying tokio_postgres::Config defaults to SslMode::Prefer (confirmed in tokio-postgres-0.7.18/src/config.rs:255) — negotiate TLS if offered, silently accept plaintext otherwise. So require/verify-ca/verify-full build the right rustls verifier but never enforce TLS at the protocol level.

Fix

Add a small pure helper, resolve_ssl_mode(ssl_mode: Option<&str>) -> Option<SslMode>, matching the builtin's exact mapping in build_postgres_configurations (src-tauri/src/pool_manager.rs):

• "disable" → Some(SslMode::Disable)
• "allow" / "prefer" → Some(SslMode::Prefer) (no behavior change — this is already tokio_postgres's default)
• "require" / "verify-ca" / "verify-full" → Some(SslMode::Require) — this is the actual fix: forces the protocol-level handshake to fail closed instead of silently downgrading. Certificate/hostname verification stays exactly as-is (handled separately in build_tls_connector).
• unset/unknown → None (no behavior change)

Wire cfg.ssl_mode = resolve_ssl_mode(params.ssl_mode.as_deref()) into build_pool, right after cfg.manager = Some(...).

TDD approach (same discipline as #34/#36/#38/#39)

1. Unit tests first (src/client_tests.rs): every case of the mapping table above, calling resolve_ssl_mode directly. Confirmed these fail to compile before the function exists (Red), then implement until green.
2. Live-DB regression test (tests/live_db.rs) — the test that actually proves the original bug, since the unit test above only proves the mapping function is correct, not that a real handshake enforces it: spawn the plugin, call test_connection with ssl_mode=require against CI's postgres:16 fixture container (confirmed via .github/workflows/ci.yml — no SSL configured there, matching my local repro), assert the response is an error, not success. Confirmed this fails against current main and will be checked green after the fix.

Verification

cargo test --lib --bins, cargo test --test live_db (against a live non-SSL Postgres), cargo clippy --all-targets -- -D warnings, cargo fmt --all -- --check, plus re-running the original repro command from this issue to confirm it now errors instead of returning null.

Branch: fix/ssl-mode-require-plaintext-fallback, off current main (includes #24/#35/#37/#40/#41/#42).

[aesslinger]

Fixed in #45 — added resolve_ssl_mode mapping this plugin's ssl_mode strings to deadpool_postgres::SslMode, wired into build_pool. Proved the fix with a live-database regression test against a real non-SSL PostgreSQL instance (fails before the fix, passes after), plus manual smoke tests against a real SSL-enabled instance with a self-signed cert to confirm no regression in the modes this change doesn't touch.