Skip to content

Use postgres-native-tls to connect to the database with TLS #9192

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jul 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -91,11 +91,13 @@ tikv-jemallocator = { version = "=0.6.0", features = ['unprefixed_malloc_on_supp
lettre = { version = "=0.11.7", default-features = false, features = ["file-transport", "smtp-transport", "native-tls", "hostname", "builder"] }
minijinja = "=2.1.0"
mockall = "=0.13.0"
native-tls = "=0.2.12"
oauth2 = "=4.4.2"
object_store = { version = "=0.10.2", features = ["aws"] }
p256 = "=0.13.2"
parking_lot = "=0.12.3"
paste = "=1.0.15"
postgres-native-tls = "=0.5.0"
prometheus = { version = "=0.13.4", default-features = false }
quick-xml = "=0.36.1"
rand = "=0.8.5"
Expand All @@ -113,6 +115,7 @@ tar = "=0.4.41"
tempfile = "=3.10.1"
thiserror = "=1.0.63"
tokio = { version = "=1.39.2", features = ["net", "signal", "io-std", "io-util", "rt-multi-thread", "macros", "process"]}
tokio-postgres = "=0.7.11"
toml = "=0.8.15"
tower = "=0.4.13"
tower-http = { version = "=0.5.2", features = ["add-extension", "fs", "catch-panic", "timeout", "compression-full"] }
Expand Down
8 changes: 5 additions & 3 deletions src/app.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! Application-wide components in a struct accessible from each request

use crate::config;
use crate::db::{connection_url, ConnectionConfig};
use crate::db::{connection_url, make_manager_config, ConnectionConfig};
use std::ops::Deref;
use std::sync::Arc;

Expand Down Expand Up @@ -88,7 +88,8 @@ impl App {
};

let url = connection_url(&config.db, config.db.primary.url.expose_secret());
let manager = AsyncDieselConnectionManager::<AsyncPgConnection>::new(url);
let manager_config = make_manager_config(config.db.enforce_tls);
let manager = AsyncDieselConnectionManager::new_with_config(url, manager_config);

DeadpoolPool::builder(manager)
.runtime(Runtime::Tokio1)
Expand All @@ -108,7 +109,8 @@ impl App {
};

let url = connection_url(&config.db, pool_config.url.expose_secret());
let manager = AsyncDieselConnectionManager::<AsyncPgConnection>::new(url);
let manager_config = make_manager_config(config.db.enforce_tls);
let manager = AsyncDieselConnectionManager::new_with_config(url, manager_config);

let pool = DeadpoolPool::builder(manager)
.runtime(Runtime::Tokio1)
Expand Down
5 changes: 3 additions & 2 deletions src/bin/background-worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

use anyhow::Context;
use crates_io::cloudfront::CloudFront;
use crates_io::db::make_manager_config;
use crates_io::fastly::Fastly;
use crates_io::storage::Storage;
use crates_io::team_repo::TeamRepoImpl;
Expand All @@ -27,7 +28,6 @@
use crates_io_worker::Runner;
use diesel_async::pooled_connection::deadpool::Pool;
use diesel_async::pooled_connection::AsyncDieselConnectionManager;
use diesel_async::AsyncPgConnection;
use reqwest::Client;
use secrecy::ExposeSecret;
use std::sync::Arc;
Expand Down Expand Up @@ -82,7 +82,8 @@
let fastly = Fastly::from_environment(client.clone());
let team_repo = TeamRepoImpl::default();

let manager = AsyncDieselConnectionManager::<AsyncPgConnection>::new(db_url);
let manager_config = make_manager_config(config.db.enforce_tls);
let manager = AsyncDieselConnectionManager::new_with_config(db_url, manager_config);

Check warning on line 86 in src/bin/background-worker.rs

View check run for this annotation

Codecov / codecov/patch

src/bin/background-worker.rs#L85-L86

Added lines #L85 - L86 were not covered by tests
let deadpool = Pool::builder(manager).max_size(10).build().unwrap();

let environment = Environment::builder()
Expand Down
11 changes: 11 additions & 0 deletions src/certs/crunchy.pem
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
-----BEGIN CERTIFICATE-----
MIIBozCCAUqgAwIBAgIJAMDLUd0ypWHdMAoGCCqGSM49BAMDMCUxIzAhBgNVBAMM
GnJqamxwMmxnbWJjanZvamV4YjN3NndsNTVlMB4XDTI0MDYwNTA2MjcyOVoXDTQ0
MDUzMTA2MjcyOVowJTEjMCEGA1UEAwwacmpqbHAybGdtYmNqdm9qZXhiM3c2d2w1
NWUwWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAAS4J3uDAfsWOQFD6XAwpPWOvviY
kCPqyJ37OGMOhA70zvQKOnxTmrKu2p7lsyVrnbCtD4Ve11CouI4iDPeVmK/wo2Mw
YTAPBgNVHRMBAf8EBTADAQH/MA4GA1UdDwEB/wQEAwIBBjAdBgNVHQ4EFgQUy5m8
qXuAIReA7KFV1fKaHLPZo14wHwYDVR0jBBgwFoAUy5m8qXuAIReA7KFV1fKaHLPZ
o14wCgYIKoZIzj0EAwMDRwAwRAIgQfpsO+B96Xse+ushnl+0Abx2tx0F5ac+K0L/
x4uyKP4CIBaCSz+Oa/rG30W2F0VVtJN8guKFvnCMy7Gg/XCGGx8l
-----END CERTIFICATE-----
8 changes: 6 additions & 2 deletions src/certs/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
//! Certificates from <https://letsencrypt.org/certificates/>.

/// Certificate from <https://letsencrypt.org/certificates/>.
pub const ISRG_ROOT_X1: &[u8] = include_bytes!("./isrg-root-x1.pem");

/// Certificate from <https://letsencrypt.org/certificates/>.
pub const ISRG_ROOT_X2: &[u8] = include_bytes!("./isrg-root-x2.pem");

/// crates.io team certificate from <https://crunchybridge.com/>.
pub const CRUNCHY: &[u8] = include_bytes!("./crunchy.pem");
51 changes: 51 additions & 0 deletions src/db.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
use crate::certs::CRUNCHY;
use diesel::{Connection, ConnectionResult, PgConnection, QueryResult};
use diesel_async::pooled_connection::deadpool::{Hook, HookError};
use diesel_async::pooled_connection::ManagerConfig;
use diesel_async::{AsyncPgConnection, RunQueryDsl};
use native_tls::{Certificate, TlsConnector};
use postgres_native_tls::MakeTlsConnector;
use secrecy::ExposeSecret;
use std::time::Duration;
use url::Url;
Expand Down Expand Up @@ -43,6 +47,53 @@ fn maybe_append_url_param(url: &mut Url, key: &str, value: &str) {
}
}

/// Create a new [ManagerConfig] for the database connection pool, which can
/// be used with [diesel_async::pooled_connection::AsyncDieselConnectionManager::new_with_config()].
pub fn make_manager_config(enforce_tls: bool) -> ManagerConfig<AsyncPgConnection> {
let mut manager_config = ManagerConfig::default();
manager_config.custom_setup =
Box::new(move |url| Box::pin(establish_async_connection(url, enforce_tls)));
manager_config
}

/// Establish a new database connection with the given URL.
///
/// Adapted from <https://github.com/weiznich/diesel_async/blob/v0.5.0/examples/postgres/pooled-with-rustls/src/main.rs>.
async fn establish_async_connection(
url: &str,
enforce_tls: bool,
) -> ConnectionResult<AsyncPgConnection> {
use diesel::ConnectionError::BadConnection;

let cert = Certificate::from_pem(CRUNCHY).map_err(|err| BadConnection(err.to_string()))?;

let connector = TlsConnector::builder()
.add_root_certificate(cert)
// On OSX the native TLS stack is complaining about the long validity
// period of the certificate, so if locally we don't enforce TLS
// connections, we also don't enforce the validity of the certificate.
//
// Similarly, on CI the native TLS stack is complaining about the
// certificate being self-signed. On CI we are connecting to a locally
// running database, so we also don't need to enforce the validity of
// the certificate either.
.danger_accept_invalid_certs(!enforce_tls)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I promise I stared at this line for a while to make sure it was correct.

.build()
.map_err(|err| BadConnection(err.to_string()))?;

let connector = MakeTlsConnector::new(connector);
let result = tokio_postgres::connect(url, connector).await;
let (client, conn) = result.map_err(|err| BadConnection(err.to_string()))?;

tokio::spawn(async move {
if let Err(e) = conn.await {
eprintln!("Database connection: {e}");
}
});

AsyncPgConnection::try_from(client).await
}

#[derive(Debug, Clone, Copy)]
pub struct ConnectionConfig {
pub statement_timeout: Duration,
Expand Down
3 changes: 2 additions & 1 deletion src/util/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,8 @@ impl From<EmailError> for BoxedAppError {
}

impl From<diesel_async::pooled_connection::deadpool::PoolError> for BoxedAppError {
fn from(_err: diesel_async::pooled_connection::deadpool::PoolError) -> BoxedAppError {
fn from(err: diesel_async::pooled_connection::deadpool::PoolError) -> BoxedAppError {
error!("Database pool error: {err}");
service_unavailable()
}
}
Expand Down