fix: build with a TLS backend so sslmode=require works (#6) - #7
Open
pcallewaert wants to merge 1 commit into
Open
fix: build with a TLS backend so sslmode=require works (#6)#7pcallewaert wants to merge 1 commit into
pcallewaert wants to merge 1 commit into
Conversation
The sqlx dependency set `default-features = false` and never re-enabled a TLS feature, so sqlx-core compiled in its no-TLS stub and every published binary returned `Error::Tls` on any upgrade attempt. This affected the macOS archives, the .deb/.rpm packages and `cargo install pgmon` just as much as the musl tarball reported in nbari#6. It also failed quietly: sqlx defaults sslmode to `prefer`, which downgrades to plaintext instead of erroring, so a server that only accepts TLS looked like a connection refusal rather than a missing feature. Select the backend through pgmon features rather than hardcoding it on the sqlx line, so the version string reports what is actually compiled in: tls-rustls-ring (default) rustls + ring, Mozilla roots bundled tls-rustls-ring-native-roots rustls + ring, host OS trust store rustls+ring over native-tls because the musl targets would otherwise need a vendored OpenSSL, and over aws-lc-rs because that needs cmake at build time. Bundled roots keep the static musl releases self-contained; private CAs are supplied per connection with sslrootcert=. Report the backend in `--version` and `check-config` so a binary's TLS capability is answerable without a server to try it against, and fail the build in build.rs unless exactly one backend is enabled — no test can catch this configuration, so it is pinned there instead. Cargo.lock is regenerated; CI builds with --locked. Fixes nbari#6 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #6.
Root cause
Cargo.tomlsetdefault-features = falseonsqlxand never re-enabled a TLS feature:sqlx 0.9 gates all TLS behind
tls-native-tls/tls-rustls*. With none enabled,sqlx-corecompiles in a stub that returnsError::Tlson any upgrade attempt — confirmed byCargo.lock, which contained no rustls, native-tls or openssl entries at all.Two notes beyond what the issue reported:
.deb/.rpmandcargo install pgmonwere equally affected. The musl tarball is just where it was hit.sslmodetoPrefer, which downgrades to plaintext rather than erroring, so ahostssl-only server looks like a connection refusal rather than a missing feature.No connection code needed changing —
PgConnectOptions::from_stralready parsessslmode/sslrootcert/sslcert/sslkey,build_pool_keyalready keys the pool on them, andclassify_connect_erroralready handlessqlx::Error::Tls. Only the backend was absent.Changes
The fix — a
[features]table selecting the backend:tls-rustls-ring(default)webpki-rootstls-rustls-ring-native-rootsRouted through pgmon features rather than hardcoded on the
sqlxline so the version string reports what is actually compiled in instead of a constant that can drift.rustls+ring over
tls-native-tls(the musl targets would need a vendored OpenSSL) and overtls-rustls-aws-lc-rs(needscmakeat build time). Bundled roots keep the static musl releases self-contained; private CAs are supplied per connection withsslrootcert=.Visible TLS status — a new
src/tls.rsreports backend and root store.--versiongains aTLS:line andcheck-configaTLSsection, so a binary's capability is answerable without a server:Build guard —
build.rsfails the build unless exactly one backend feature is enabled. No test can catch this configuration, so it is pinned there instead.Cargo.lock — regenerated. Additions only (147 lines, no unrelated version churn), which matters because CI builds with
--locked.Docs — a
TLS / SSLREADME section (sslmode examples, theprefer-downgrades-silently warning, root store trade-off, how to check a binary) plus a CHANGELOG entry.Verification
cargo clippy --all-targetsclean, 153 tests pass, and--no-default-featuresnow fails with the guard's message rather than silently producing another TLS-less binary.End-to-end against PostgreSQL 16 with
ssl=on, both binaries connecting withsslmode=require, measured server-side viapg_stat_ssl:ssl=true version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384, live TUIWorth noting from that test: the old binary surfaces no error banner, just zeros — the failure is quieter than the issue suggests.
Built for
x86_64-unknown-linux-muslin a container mirroring the release workflow (musl-tools+--locked). One caveat: that container also neededprotobuf-compilerforpg_query, which the GitHub runner image provides — a gap in my local harness, not a change in this branch, but worth watching on the first CI run.The version is left at 0.7.1; users only get this once a new tag is cut.
🤖 Generated with Claude Code