feat(rustls): implement TlsAcceptCallbacks support for rustls backend by jsulmont · Pull Request #833 · cloudflare/pingora

jsulmont · 2026-03-06T12:28:29Z

Summary

TlsSettings::with_callbacks() — previously returned an error ("Certificate callbacks are not supported with feature rustls"). Now accepts a TlsAcceptCallbacks and threads it through to Acceptor and the handshake path.
TlsRef — extended from an empty struct to carry peer certificate chain (Vec<CertificateDer>) and negotiated cipher suite name, with public accessors: peer_certificate_der(), peer_cert_chain_der(), current_cipher_name()
TlsStream::build_tls_ref() — extracts connection state from the underlying rustls session after handshake completes
handshake_with_callback() — now builds a populated TlsRef and passes it to the callback, matching the OpenSSL/BoringSSL path's behavior
Added set_certificate_chain_file() / set_private_key_file() builder methods on TlsSettings for use with the callbacks constructor
Added test_handshake_complete_callback integration test

Motivation

This enables downstream projects to use mTLS peer certificate inspection in post-handshake callbacks when using the rustls feature, which previously only worked with the OpenSSL/BoringSSL backend.

Test plan

cargo check -p pingora-core --features rustls
cargo clippy -p pingora-core --features rustls --tests
cargo fmt -p pingora-core -- --check
cargo test -p pingora-core --features rustls -- server::tests::test_handshake_complete_callback

TlsSettings::with_callbacks() was previously unimplemented and returned an error. This change adds full callback support to the rustls TLS path: - TlsRef now carries peer certificates and cipher suite name - TlsStream::build_tls_ref() extracts session state after handshake - handshake_with_callback() passes populated TlsRef to callbacks - Added set_certificate_chain_file/set_private_key_file setters - Added test_handshake_complete_callback integration test

…nSSL backend with_single_cert() runs webpki validation on the server's own certificate chain, rejecting certs with unrecognized critical extensions. Replace with CertifiedKey::new() + with_cert_resolver() which loads the cert+key without upfront validation, matching how the OpenSSL backend behaves. Also re-exports sign::CertifiedKey, ResolvesServerCert, and ClientHello from pingora-rustls for downstream use.

Add ConnectorOptions::server_cert_verifier to allow users to provide a custom rustls ServerCertVerifier for upstream connections. When set, the connector bypasses webpki for CA loading, server cert validation, and client cert validation — all three of which reject certificates with unrecognized critical extensions. Changes: - ConnectorOptions: add server_cert_verifier field (rustls feature-gated) - TlsConnector: use dangerous().with_custom_certificate_verifier() and with_client_cert_resolver() when custom verifier is provided - CustomServerCertVerifier: generalize delegate from WebPkiServerVerifier to dyn ServerCertVerifier for use with custom verifiers - ProxyServiceBuilder: add connector_options() builder method to override the default ConnectorOptions derived from ServerConf - pingora-rustls: re-export ResolvesClientCert and sign module

…rt paths in build()

johnhurt · 2026-03-06T18:43:31Z

I think this is a great PR, and I would like to accept it. As contributors to this project have (unfortunately) realized, we are slow on reviews and ingestion, and extra slow when things are included that relate to parts of pingora that we don't use internally (like rustls or s2n integration). To help with that, I am hoping that we can find someone or organization that is willing to be a trusted reviewer for rustls-related prs. (Hence the help-wanted label).

If you or someone you know would like to volunteer to take that on, let us know here. We (and the other rustls pingorians) would appreciate it.

cc @hargut

johnhurt · 2026-03-27T17:53:18Z

@fabian4 @nojima — this PR implements TlsAcceptCallbacks for the rustls backend and extends TlsRef with peer cert chain and cipher info. It's the largest of the group but has a real integration test. Note it adds a couple of #[cfg(feature = "rustls")] fields to core structs which we'll review on our side. Would appreciate your review of the rustls-specific parts. (Per #835)

fabian4 · 2026-03-30T12:59:31Z

pingora-core/src/listeners/tls/rustls/mod.rs

+            .key_provider
+            .load_private_key(key)
+            .expect("Failed to load server private key");
+        let certified_key = Arc::new(pingora_rustls::sign::CertifiedKey::new(certs, signing_key));


I think it would be safer to keep an explicit cert/key consistency check here, as we previously did with with_single_cert(...), so that we only relax Web PKI policy validation without also dropping the more basic key/cert match validation.

fabian4 · 2026-03-30T13:04:12Z

pingora-core/src/connectors/tls/rustls/mod.rs

+                    let provider = builder.crypto_provider().clone();
+                    let signing_key = provider
+                        .key_provider
+                        .load_private_key(key)


Also, the cert/key consistency check seems to be skipped here as well.

nojima · 2026-04-01T05:07:54Z

pingora-core/src/listeners/tls/rustls/mod.rs

    }

+    /// Set the path to the certificate chain file (PEM format).
+    pub fn set_certificate_chain_file(&mut self, path: &str) {


The behavior is inconsistent with the corresponding function in the openssl version. The openssl version of this function returns a Result type. For example, it returns an error when a non-existent file is specified. Having rustls version behave the same way would be more convenient for users.

nojima · 2026-04-01T05:08:09Z

pingora-core/src/listeners/tls/rustls/mod.rs

+    }
+
+    /// Set the path to the private key file (PEM format).
+    pub fn set_private_key_file(&mut self, path: &str) {


Same as above

nojima · 2026-04-01T05:13:21Z

pingora-core/src/listeners/tls/rustls/mod.rs

+        assert!(
+            !self.cert_path.is_empty() && !self.key_path.is_empty(),
+            "Certificate and key paths must be set before calling build(). \
+             When using with_callbacks(), call set_certificate_chain_file() \
+             and set_private_key_file() first."
+        );


In the openssl version, it is permitted to omit the certificate and private key at initialization. This kind of setup is useful when you want to return different server certificates depending on information received from the client during the handshake.

nojima · 2026-04-01T05:29:35Z

pingora-core/src/protocols/tls/rustls/server.rs

-
+        // NOTE: certificate_callback is not invoked for rustls. Dynamic cert selection
+        // should use a custom ResolvesServerCert instead.
+        warn!("certificate_callback is not supported with the rustls backend; use ResolvesServerCert for dynamic cert selection");


Can pingora users currently plug in their own implementation of ResolvesServerCert?

nojima · 2026-04-01T05:56:22Z

pingora-core/src/protocols/tls/rustls/server.rs

    {
-        let tls_ref = TlsRef;
+        // Build TlsRef with connection state for the callback
+        let tls_ref = tls_stream.build_tls_ref();


IO already has get_ssl method that returns a Option<&TlsRef>. Rather than creating a new function, you should call this method instead.

if let Some(ref stream) = tls_stream.stream { if let Some(tls_ref) = stream.get_ref().0.get_ssl() { // do something with tls_ref } }

Currently, get_ssl on TlsStream is a default implementation that simply returns None.

pingora/pingora-core/src/protocols/tls/rustls/stream.rs

Lines 230 to 247 in 9a4eee3

impl<T> Ssl for TlsStream<T> {

fn get_ssl_digest(&self) -> Option<Arc<SslDigest>> {

self.ssl_digest()

}

fn selected_alpn_proto(&self) -> Option<ALPN> {

let st = self.tls.stream.as_ref();

if let Some(stream) = st {

let proto = stream.get_ref().1.alpn_protocol();

match proto {

None => None,

Some(raw) => ALPN::from_wire_selected(raw),

}

} else {

None

}

}

}

pingora/pingora-core/src/protocols/mod.rs

Lines 57 to 60 in 9a4eee3

/// Return the TLS info if the connection is over TLS

fn get_ssl(&self) -> Option<&TlsRef> {

None

}

It would be good to provide a proper implementation here.

nojima · 2026-04-02T15:46:42Z

I'm not sure if this is the right place to discuss this, but I'd like to open a discussion about how the rustls version of TlsRef should be defined.

The OpenSSL version of TlsRef is defined as an alias for SslRef, giving comprehensive access to information within the TLS connection. In contrast, the TlsRef in this PR only holds peer_certs and cipher, meaning information such as the TLS version or SNI cannot be retrieved.

Should TlsRef be defined as a standalone struct, as in the current PR's approach, where Pingora populates it with the necessary data? Or
Should TlsRef be defined as an alias for some rustls type (e.g. ServerConnection), similar to how the openssl version aliases SslRef, allowing users to access whatever they need?

jsulmont mentioned this pull request Mar 6, 2026

feat: implement TlsAcceptCallbacks support for rustls backend #834

Open

Jan van Lindt added 4 commits March 6, 2026 16:10

fix(rustls): warn on unsupported certificate_callback and validate ce…

251cd0c

…rt paths in build()

fix(rustls): replace expect() with or_err on peer client key loading

6a09ea3

drcaramelsyrup requested a review from johnhurt March 6, 2026 18:33

drcaramelsyrup added enhancement New feature or request help wanted Extra attention is needed labels Mar 6, 2026

fabian4 reviewed Mar 30, 2026

View reviewed changes

nojima reviewed Apr 1, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(rustls): implement TlsAcceptCallbacks support for rustls backend#833

feat(rustls): implement TlsAcceptCallbacks support for rustls backend#833
jsulmont wants to merge 5 commits intocloudflare:mainfrom
jsulmont:feat/rustls-tls-accept-callbacks

jsulmont commented Mar 6, 2026

Uh oh!

johnhurt commented Mar 6, 2026

Uh oh!

johnhurt commented Mar 27, 2026 •

edited

Loading

Uh oh!

fabian4 Mar 30, 2026 •

edited

Loading

Uh oh!

fabian4 Mar 30, 2026

Uh oh!

nojima Apr 1, 2026

Uh oh!

nojima Apr 1, 2026

Uh oh!

nojima Apr 1, 2026

Uh oh!

nojima Apr 1, 2026

Uh oh!

nojima Apr 1, 2026

Uh oh!

nojima Apr 1, 2026

Uh oh!

nojima commented Apr 2, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

	impl<T> Ssl for TlsStream<T> {
	fn get_ssl_digest(&self) -> Option<Arc<SslDigest>> {
	self.ssl_digest()
	}

	fn selected_alpn_proto(&self) -> Option<ALPN> {
	let st = self.tls.stream.as_ref();
	if let Some(stream) = st {
	let proto = stream.get_ref().1.alpn_protocol();
	match proto {
	None => None,
	Some(raw) => ALPN::from_wire_selected(raw),
	}
	} else {
	None
	}
	}
	}

	/// Return the TLS info if the connection is over TLS
	fn get_ssl(&self) -> Option<&TlsRef> {
	None
	}

Conversation

jsulmont commented Mar 6, 2026

Summary

Motivation

Test plan

Uh oh!

johnhurt commented Mar 6, 2026

Uh oh!

johnhurt commented Mar 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fabian4 Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fabian4 Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

nojima Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

nojima Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

nojima Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

nojima Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

nojima Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

nojima Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

nojima commented Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

johnhurt commented Mar 27, 2026 •

edited

Loading

fabian4 Mar 30, 2026 •

edited

Loading

nojima commented Apr 2, 2026 •

edited

Loading