Skip to content

Commit 4eec3e8

Browse files
tests: restore cert reloading assertions and fix TLS test helpers
Replace TODO stubs with working implementations: - peer_certificate_der(): raw tokio-rustls handshake to inspect peer certificates. reqwest's TlsInfo::peer_certificate() only works with the native-tls backend, returning None with rustls — so we drop down to tokio_rustls::TlsConnector directly where ServerConnection::peer_certificates() always works. - cert_file_to_der(): parse PEM cert files to DER for comparison. - make_http_tls(): now honors TestTlsConfig (builds hyper-rustls connector from the client config that trusts the test CA). - make_ws_tls(): uses rustls::StreamOwned for synchronous TLS WebSocket connections. Cert reloading test assertions in both environmentd and balancerd are now fully restored — no remaining TODO stubs. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
1 parent 59bf89a commit 4eec3e8

File tree

4 files changed

+64
-67
lines changed

4 files changed

+64
-67
lines changed

src/balancerd/tests/server.rs

Lines changed: 10 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -253,40 +253,28 @@ async fn test_balancer() {
253253
.send()
254254
.await
255255
.unwrap();
256-
// TODO(SEC-219): peer certificate inspection not available with rustls postgres connector
257-
// let tlsinfo = resp.extensions().get::<reqwest::tls::TlsInfo>().unwrap();
258-
// let resp_x509 = X509::from_der(tlsinfo.peer_certificate().unwrap()).unwrap();
259-
// let server_x509 = X509::from_pem(&std::fs::read(&server_cert).unwrap()).unwrap();
260-
// assert_eq!(resp_x509, server_x509);
261256
assert_contains!(resp.text().await.unwrap(), "12234");
257+
let server_cert_der = test_util::cert_file_to_der(&server_cert);
258+
let tls_cfg = TestTlsConfig::with_ca(&ca.ca_cert_path());
259+
let peer_der = test_util::peer_certificate_der(balancer_https_listen, &tls_cfg).await;
260+
assert_eq!(peer_der, server_cert_der);
262261

263262
// Generate new certs. Install only the key, reload, and make sure the old cert is still in
264263
// use.
265264
let (next_cert, next_key) = ca
266265
.request_cert("next", vec![IpAddr::V4(Ipv4Addr::LOCALHOST)])
267266
.unwrap();
268-
// TODO(SEC-219): peer certificate inspection not available with rustls postgres connector
269-
// let next_x509 = X509::from_pem(&std::fs::read(&next_cert).unwrap()).unwrap();
270-
// assert_ne!(next_x509, server_x509);
267+
let next_cert_der = test_util::cert_file_to_der(&next_cert);
268+
assert_ne!(next_cert_der, server_cert_der);
271269
std::fs::copy(next_key, &server_key).unwrap();
272270
let (tx, rx) = oneshot::channel();
273271
reload_tx.try_send(Some(tx)).unwrap();
274272
let res = rx.await.unwrap();
275273
assert_err!(res);
276274

277275
// We should still be on the old cert because now the cert and key mismatch.
278-
let resp = client
279-
.post(&https_url)
280-
.header("Content-Type", "application/json")
281-
.basic_auth(frontegg_user, Some(&frontegg_password))
282-
.body(body)
283-
.send()
284-
.await
285-
.unwrap();
286-
// TODO(SEC-219): peer certificate inspection not available with rustls postgres connector
287-
// let tlsinfo = resp.extensions().get::<reqwest::tls::TlsInfo>().unwrap();
288-
// let resp_x509 = X509::from_der(tlsinfo.peer_certificate().unwrap()).unwrap();
289-
// assert_eq!(resp_x509, server_x509);
276+
let peer_der = test_util::peer_certificate_der(balancer_https_listen, &tls_cfg).await;
277+
assert_eq!(peer_der, server_cert_der);
290278

291279
// Now move the cert too. Reloading should succeed and the response should have the new
292280
// cert.
@@ -295,18 +283,8 @@ async fn test_balancer() {
295283
reload_tx.try_send(Some(tx)).unwrap();
296284
let res = rx.await.unwrap();
297285
assert_ok!(res);
298-
let resp = client
299-
.post(&https_url)
300-
.header("Content-Type", "application/json")
301-
.basic_auth(frontegg_user, Some(&frontegg_password))
302-
.body(body)
303-
.send()
304-
.await
305-
.unwrap();
306-
// TODO(SEC-219): peer certificate inspection not available with rustls postgres connector
307-
// let tlsinfo = resp.extensions().get::<reqwest::tls::TlsInfo>().unwrap();
308-
// let resp_x509 = X509::from_der(tlsinfo.peer_certificate().unwrap()).unwrap();
309-
// assert_eq!(resp_x509, next_x509);
286+
let peer_der = test_util::peer_certificate_der(balancer_https_listen, &tls_cfg).await;
287+
assert_eq!(peer_der, next_cert_der);
310288

311289
if !is_multi_tenant_resolver {
312290
continue;

src/environmentd/src/test_util.rs

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1693,6 +1693,39 @@ pub fn make_pg_tls(config: TestTlsConfig) -> MakeRustlsConnect {
16931693
MakeRustlsConnect::new((*config.build_rustls_client_config()).clone())
16941694
}
16951695

1696+
/// Performs a TLS handshake to `addr` and returns the peer's leaf certificate
1697+
/// in DER encoding.
1698+
///
1699+
/// We use a raw `tokio_rustls` connection instead of reqwest because
1700+
/// `reqwest::tls::TlsInfo::peer_certificate()` only returns the peer cert
1701+
/// when reqwest is built with the `native-tls` backend. With the `rustls-tls`
1702+
/// backend it always returns `None` — a known reqwest limitation. By dropping
1703+
/// down to `tokio_rustls` directly we can call
1704+
/// `ServerConnection::peer_certificates()` which always works.
1705+
pub async fn peer_certificate_der(
1706+
addr: std::net::SocketAddr,
1707+
tls_config: &TestTlsConfig,
1708+
) -> Vec<u8> {
1709+
use tokio_rustls::TlsConnector;
1710+
1711+
let connector = TlsConnector::from(tls_config.build_rustls_client_config());
1712+
let server_name = rustls::pki_types::ServerName::IpAddress(addr.ip().into());
1713+
let tcp = tokio::net::TcpStream::connect(addr).await.unwrap();
1714+
let tls = connector.connect(server_name, tcp).await.unwrap();
1715+
let (_, session) = tls.get_ref();
1716+
let certs = session.peer_certificates().expect("peer certificates");
1717+
certs[0].as_ref().to_vec()
1718+
}
1719+
1720+
/// Reads a PEM certificate file and returns the first certificate as DER bytes.
1721+
pub fn cert_file_to_der(path: &Path) -> Vec<u8> {
1722+
let pem = fs::read(path).unwrap();
1723+
let certs: Vec<CertificateDer> = rustls_pemfile::certs(&mut &*pem)
1724+
.collect::<Result<_, _>>()
1725+
.unwrap();
1726+
certs[0].as_ref().to_vec()
1727+
}
1728+
16961729
/// Configuration for test TLS connections, replacing the old
16971730
/// `FnOnce(&mut SslConnectorBuilder)` closure pattern.
16981731
#[derive(Clone, Debug)]

src/environmentd/tests/auth.rs

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -69,25 +69,25 @@ use uuid::Uuid;
6969
// without increasing test time.
7070
const EXPIRES_IN_SECS: u64 = 20;
7171

72-
// TODO(SEC-219): migrate make_http_tls to rustls (hyper-rustls).
73-
// For now, HTTP test paths using this function will panic at runtime.
74-
fn make_http_tls(_config: &TestTlsConfig) -> hyper_rustls::HttpsConnector<HttpConnector> {
72+
fn make_http_tls(config: &TestTlsConfig) -> hyper_rustls::HttpsConnector<HttpConnector> {
73+
let tls_config: rustls::ClientConfig = (*config.build_rustls_client_config()).clone();
7574
let mut http = HttpConnector::new();
7675
http.enforce_http(false);
7776
hyper_rustls::HttpsConnectorBuilder::new()
78-
.with_native_roots()
79-
.unwrap()
77+
.with_tls_config(tls_config)
8078
.https_or_http()
8179
.enable_http2()
8280
.wrap_connector(http)
8381
}
8482

85-
// TODO(SEC-219): migrate make_ws_tls to rustls.
86-
// For now, WebSocket test paths using this function will connect via plain TCP.
87-
fn make_ws_tls(uri: &Uri, _config: &TestTlsConfig) -> impl Read + Write {
88-
// Plain TCP — TLS WS tests will fail at the protocol level until
89-
// this is migrated to tokio-rustls or rustls-connector.
90-
TcpStream::connect(format!("{}:{}", uri.host().unwrap(), uri.port().unwrap())).unwrap()
83+
fn make_ws_tls(uri: &Uri, config: &TestTlsConfig) -> impl Read + Write {
84+
let tls_config = config.build_rustls_client_config();
85+
let server_name = rustls::pki_types::ServerName::try_from(uri.host().unwrap().to_string())
86+
.expect("valid server name");
87+
let conn = rustls::ClientConnection::new(tls_config, server_name).unwrap();
88+
let tcp =
89+
TcpStream::connect(format!("{}:{}", uri.host().unwrap(), uri.port().unwrap())).unwrap();
90+
rustls::StreamOwned::new(conn, tcp)
9191
}
9292

9393
// Use two error types because some tests need to retry certain errors because

src/environmentd/tests/server.rs

Lines changed: 10 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -4737,35 +4737,29 @@ async fn test_cert_reloading() {
47374737
.send()
47384738
.await
47394739
.unwrap();
4740-
// TODO(SEC-219): certificate comparison needs rustls-native peer cert API.
4741-
// Previously we compared X509::from_der(tlsinfo.peer_certificate()) against the
4742-
// server cert file. With rustls, peer_certificate() is not available on reqwest TlsInfo.
4743-
let _tlsinfo = resp.extensions().get::<reqwest::tls::TlsInfo>().unwrap();
47444740
assert_contains!(resp.text().await.unwrap(), "12234");
4741+
let server_cert_der = test_util::cert_file_to_der(&server_cert);
4742+
let tls_cfg = TestTlsConfig::with_ca(&ca.ca_cert_path());
4743+
let peer_der = test_util::peer_certificate_der(envd_server.http_local_addr(), &tls_cfg).await;
4744+
assert_eq!(peer_der, server_cert_der);
47454745
check_pgwire(&conn_str, &ca.ca_cert_path()).await;
47464746

47474747
// Generate new certs. Install only the key, reload, and make sure the old cert is still in
47484748
// use.
47494749
let (next_cert, next_key) = ca
47504750
.request_cert("next", vec![IpAddr::V4(Ipv4Addr::LOCALHOST)])
47514751
.unwrap();
4752+
let next_cert_der = test_util::cert_file_to_der(&next_cert);
4753+
assert_ne!(next_cert_der, server_cert_der);
47524754
std::fs::copy(next_key, &server_key).unwrap();
47534755
let (tx, rx) = oneshot::channel();
47544756
reload_tx.try_send(Some(tx)).unwrap();
47554757
let res = rx.await.unwrap();
47564758
assert_err!(res);
47574759

47584760
// We should still be on the old cert because now the cert and key mismatch.
4759-
let resp = client
4760-
.post(&https_url)
4761-
.header("Content-Type", "application/json")
4762-
.basic_auth(frontegg_user, Some(&frontegg_password))
4763-
.body(body)
4764-
.send()
4765-
.await
4766-
.unwrap();
4767-
// TODO(SEC-219): certificate comparison needs rustls-native peer cert API.
4768-
let _tlsinfo = resp.extensions().get::<reqwest::tls::TlsInfo>().unwrap();
4761+
let peer_der = test_util::peer_certificate_der(envd_server.http_local_addr(), &tls_cfg).await;
4762+
assert_eq!(peer_der, server_cert_der);
47694763
check_pgwire(&conn_str, &ca.ca_cert_path()).await;
47704764

47714765
// Now move the cert too. Reloading should succeed and the response should have the new
@@ -4775,16 +4769,8 @@ async fn test_cert_reloading() {
47754769
reload_tx.try_send(Some(tx)).unwrap();
47764770
let res = rx.await.unwrap();
47774771
assert_ok!(res);
4778-
let resp = client
4779-
.post(&https_url)
4780-
.header("Content-Type", "application/json")
4781-
.basic_auth(frontegg_user, Some(&frontegg_password))
4782-
.body(body)
4783-
.send()
4784-
.await
4785-
.unwrap();
4786-
// TODO(SEC-219): certificate comparison needs rustls-native peer cert API.
4787-
let _tlsinfo = resp.extensions().get::<reqwest::tls::TlsInfo>().unwrap();
4772+
let peer_der = test_util::peer_certificate_der(envd_server.http_local_addr(), &tls_cfg).await;
4773+
assert_eq!(peer_der, next_cert_der);
47884774
check_pgwire(&conn_str, &ca.ca_cert_path()).await;
47894775
}
47904776

0 commit comments

Comments
 (0)