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)

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?

…ept-callbacks

Add verify_cert_key_match() helper (SPKI compare via x509-parser) and call it on the listener and connector custom-verifier paths, which had silently dropped the consistency check along with webpki policy validation. set_certificate_chain_file / set_private_key_file now return Result and error on missing or empty files, matching the OpenSSL backend. Addresses review feedback on cloudflare#833.

Drop the ad-hoc build_tls_ref() helper. Store the TlsRef on TlsStream, populate it in accept()/connect(), and return &TlsRef from get_ssl() so callers can use the standard Ssl trait method. handshake_with_callback now goes through get_ssl() instead of building a TlsRef on demand.

jsulmont · 2026-04-18T07:52:39Z

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?

Instinctively I'd lean towards keeping TslRef defined asa a standalone structure, growing it as needs arise rather than aliasing a rustls type. But that's probably because I don't fully grasp the implication of 2.
In the meantime to address the concrete gaps I've added version() and server_name() to TslRef
@nojima

jsulmont · 2026-04-18T07:57:55Z

Hi @fabian4 @nojima

Thanks for your review and sorry for not getting to this sooner — I've been v. busy with work :/

johnhurt · 2026-04-24T17:34:16Z

@fabian4 @nojima I see there are some new changes here, how does it look to you? (Also thanks for agreeing to look into these as external reviewers 🙏 )

nojima · 2026-04-25T08:42:12Z

@johnhurt @jsulmont
Sorry for the late response.
I've been reading through it bit by bit in the background, but I needed a solid block of time to think about what this change should look like, so I haven't been able to get my review back.
I'm planning to set aside some time this weekend to send my review.

nojima · 2026-04-25T10:07:07Z

@jsulmont
I have a question.
In this PR, a lot of workaround code has been written to ignore validation errors caused by unrecognized critical extensions. Why is this kind of code necessary?

nojima · 2026-04-26T15:02:24Z

@jsulmont @johnhurt
If my understanding is correct, this PR contains two different types of changes:

Support for TlsAcceptCallbacks
Intentional bypassing of the webpki policy

While the first point is the main focus of this PR, the second does not seem related. I believe it would be better to split this into a separate PR, so that we can discuss whether this change introduces any security issues independently.

johnhurt · 2026-04-26T16:27:50Z

Agreed.

jsulmont mentioned this pull request Mar 6, 2026

feat: implement TlsAcceptCallbacks support for rustls backend #834

Open

jsulmont 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

Comment thread pingora-core/src/listeners/tls/rustls/mod.rs Outdated

fabian4 reviewed Mar 30, 2026

View reviewed changes

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

nojima reviewed Apr 1, 2026

View reviewed changes

jsulmont added 5 commits April 18, 2026 08:57

Merge remote-tracking branch 'upstream/main' into feat/rustls-tls-acc…

e5352a9

…ept-callbacks

feat(rustls): expose custom ResolvesServerCert via set_cert_resolver

50e24e1

feat(rustls): add version() and server_name() to TlsRef

abc4842

jsulmont requested review from fabian4 and nojima April 18, 2026 07:58

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

jsulmont commented Apr 18, 2026

Uh oh!

jsulmont commented Apr 18, 2026

Uh oh!

johnhurt commented Apr 24, 2026

Uh oh!

nojima commented Apr 25, 2026

Uh oh!

nojima commented Apr 25, 2026

Uh oh!

nojima commented Apr 26, 2026

Uh oh!

johnhurt commented Apr 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

johnhurt commented Mar 27, 2026 •

edited

Loading

nojima commented Apr 2, 2026 •

edited

Loading