Skip to content

Commit 0b27115

Browse files
committed
store: Fix TLS regression in diesel-async connection pool
The migration from synchronous diesel (r2d2 + PgConnection) to diesel-async (deadpool + AsyncPgConnection) in the "Make the store async" change inadvertently broke TLS support for the database connection pool. The old code used diesel::r2d2::ConnectionManager<PgConnection> which is backed by libpq (via pq-sys). libpq defaults to sslmode=prefer, meaning it transparently negotiates TLS with the server when available. The new code uses diesel_async::AsyncPgConnection::establish() which internally calls tokio_postgres::connect() with tokio_postgres::NoTls, meaning TLS is never negotiated regardless of the sslmode parameter in the connection URL. This breaks connections to any PostgreSQL server that requires encrypted connections via pg_hba.conf. Fix this by replacing AsyncPgConnection::establish() with a manual tokio_postgres::connect() call using postgres-openssl as the TLS connector (with SslVerifyMode::NONE to match libpq's default prefer behavior), then constructing the AsyncPgConnection via try_from_client_and_connection(). This restores the pre-v0.42.0 behavior where connections are encrypted by default. Note: tokio-postgres does not support sslmode=verify-ca or sslmode=verify-full in its URL parser — only disable, prefer, and require are recognized. Certificate verification would require upstream changes to tokio-postgres. The openssl and postgres-openssl crates were already dependencies of graph-store-postgres (used by the notification listener). Only tokio-postgres was added as a new direct dependency.
1 parent 277f45e commit 0b27115

3 files changed

Lines changed: 55 additions & 6 deletions

File tree

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

store/postgres/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ lru_time_cache = "0.11"
2424
postgres = "0.19.1"
2525
openssl = { version = "0.10.76", features = ["vendored"] }
2626
postgres-openssl = "0.5.2"
27+
tokio-postgres = "0.7"
2728
rand.workspace = true
2829
serde = { workspace = true }
2930
serde_json = { workspace = true }

store/postgres/src/pool/manager.rs

Lines changed: 53 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use deadpool::managed::{Hook, RecycleError, RecycleResult};
77
use diesel::IntoSql;
88

99
use diesel_async::pooled_connection::{PoolError as DieselPoolError, PoolableConnection};
10-
use diesel_async::{AsyncConnection, RunQueryDsl};
10+
use diesel_async::RunQueryDsl;
1111
use graph::env::ENV_VARS;
1212
use graph::prelude::error;
1313
use graph::prelude::AtomicMovingStats;
@@ -17,6 +17,8 @@ use graph::prelude::MetricsRegistry;
1717
use graph::prelude::PoolWaitStats;
1818
use graph::slog::info;
1919
use graph::slog::Logger;
20+
use openssl::ssl::{SslConnector, SslMethod, SslVerifyMode};
21+
use postgres_openssl::MakeTlsConnector;
2022

2123
use std::collections::HashMap;
2224
use std::sync::atomic::AtomicBool;
@@ -100,11 +102,44 @@ impl deadpool::managed::Manager for ConnectionManager {
100102
type Error = DieselPoolError;
101103

102104
async fn create(&self) -> Result<Self::Type, Self::Error> {
103-
let res = diesel_async::AsyncPgConnection::establish(&self.connection_url).await;
104-
if let Err(ref e) = res {
105-
self.handle_error(e);
106-
}
107-
res.map_err(DieselPoolError::ConnectionError)
105+
// diesel-async's AsyncPgConnection::establish() uses
106+
// tokio_postgres::NoTls, which never negotiates TLS. This breaks
107+
// connections to any PostgreSQL server that requires encrypted
108+
// connections via pg_hba.conf.
109+
//
110+
// We use postgres-openssl to provide TLS support, with
111+
// SslVerifyMode::NONE to match libpq's default sslmode=prefer
112+
// behavior: encrypt the connection when the server supports it,
113+
// but don't verify the server certificate. tokio-postgres handles
114+
// the sslmode parameter from the connection URL to decide whether
115+
// TLS is used (disable/prefer/require); we configure how the
116+
// handshake behaves.
117+
//
118+
// Note: tokio-postgres does not support sslmode=verify-ca or
119+
// sslmode=verify-full. Certificate verification would require
120+
// upstream support in the tokio-postgres URL parser.
121+
let tls = make_tls_connector().map_err(|e| {
122+
DieselPoolError::ConnectionError(diesel::ConnectionError::BadConnection(
123+
e.to_string(),
124+
))
125+
})?;
126+
127+
let (client, conn) =
128+
tokio_postgres::connect(&self.connection_url, tls)
129+
.await
130+
.map_err(|e| {
131+
self.handle_error(&e);
132+
DieselPoolError::ConnectionError(
133+
diesel::ConnectionError::BadConnection(e.to_string()),
134+
)
135+
})?;
136+
137+
diesel_async::AsyncPgConnection::try_from_client_and_connection(client, conn)
138+
.await
139+
.map_err(|e| {
140+
self.handle_error(&e);
141+
DieselPoolError::ConnectionError(e)
142+
})
108143
}
109144

110145
async fn recycle(
@@ -249,6 +284,18 @@ impl StateTracker {
249284
}
250285
}
251286

287+
/// Build an OpenSSL TLS connector for PostgreSQL connections.
288+
///
289+
/// Uses `SslVerifyMode::NONE` (encrypt without certificate verification),
290+
/// matching libpq's behavior for sslmode=prefer and sslmode=require.
291+
/// tokio-postgres handles the sslmode URL parameter to decide whether TLS
292+
/// is actually used (disable/prefer/require).
293+
fn make_tls_connector() -> Result<MakeTlsConnector, openssl::error::ErrorStack> {
294+
let mut builder = SslConnector::builder(SslMethod::tls())?;
295+
builder.set_verify(SslVerifyMode::NONE);
296+
Ok(MakeTlsConnector::new(builder.build()))
297+
}
298+
252299
fn brief_error_msg(error: &dyn std::error::Error) -> String {
253300
// For 'Connection refused' errors, Postgres includes the IP and
254301
// port number in the error message. We want to suppress that and

0 commit comments

Comments
 (0)