Skip to content

feat: add SNI field to SslDigest for HTTP filter context access#857

Open
nanookclaw wants to merge 2 commits intocloudflare:mainfrom
nanookclaw:feat/add-sni-to-ssl-digest
Open

feat: add SNI field to SslDigest for HTTP filter context access#857
nanookclaw wants to merge 2 commits intocloudflare:mainfrom
nanookclaw:feat/add-sni-to-ssl-digest

Conversation

@nanookclaw
Copy link
Copy Markdown

Summary

Fixes #547

Adds an sni: Option<String> field to SslDigest so that the Server Name Indication can be retrieved from req_filter() callbacks via session.digest().ssl_digest.sni.

The SNI is extracted during TLS handshake and stored alongside existing TLS connection metadata (cipher, version, organization, etc.) in the SslDigest struct.

Changes

  • digest.rs: Add sni: Option<String> field to SslDigest struct and update new() constructor
  • BoringSSL/OpenSSL: Extract SNI via ssl.servername(ssl::NameType::HOST_NAME).map(ToOwned::to_owned) in from_ssl()
  • Rustls: Extract SNI via server.server_name().map(ToOwned::to_owned) from ServerConnection (returns None for client connections)
  • s2n: Extract SNI via conn.server_name().map(ToOwned::to_owned) from the s2n Connection

Usage

After this change, users can access the SNI in their HTTP filter callbacks:

fn req_filter(&self, session: &mut Session) -> Result<bool> {
    if let Some(digest) = session.digest() {
        if let Some(ssl_digest) = &digest.ssl_digest {
            if let Some(sni) = &ssl_digest.sni {
                // Use SNI for business logic alongside the HTTP HOST header
                println!("SNI: {}", sni);
            }
        }
    }
    Ok(false)
}

This directly addresses the use case described in #547 where the SNI is needed alongside the HTTP HOST header in req_filter() but is currently inaccessible.

Testing

  • Compilation verified across all three TLS backends (boringssl/openssl, rustls, s2n)
  • No new dependencies added — only existing TLS crate APIs are used
  • Minimal, non-breaking change: sni is Option<String> so existing code is unaffected

nanookclaw and others added 2 commits April 12, 2026 07:39
Fixes cloudflare#547

Add sni: Option<String> to SslDigest struct so that the SNI can be
retrieved from req_filter() callbacks via session.digest().ssl_digest.sni.

The SNI is extracted during TLS handshake and stored alongside existing
TLS connection metadata (cipher, version, organization, etc.) in the
SslDigest struct.

Supported across all TLS backends:
- BoringSSL/OpenSSL: via ssl.servername(NameType::HOST_NAME)
- Rustls: via session.server_name()
- s2n: via conn.server_name()

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@drcaramelsyrup
Copy link
Copy Markdown
Collaborator

Would 6ebc829 the extensible SslDigest also work for your purposes?

@drcaramelsyrup drcaramelsyrup added the enhancement New feature or request label Apr 17, 2026
@nanookclaw
Copy link
Copy Markdown
Author

Pushed the cert_digest is None test case (test_ssl_digest_cert_digest_none_plain_http). It verifies that cert_digest and sni are independent — a plain-HTTP or no-client-cert connection leaves cert_digest as None while sni can still be set.

@nanookclaw
Copy link
Copy Markdown
Author

Good question. I looked at the extension mechanism from 6ebc829 — the SslDigestExtension::set is pub(crate), so external consumers can read but not write custom data through it. For Pingora's own internal extensions that's fine, but SNI is something the TLS stream implementations need to populate at handshake time, which lives inside the crate.

That said, the extensible approach is the right long-term direction. For the SNI case specifically, a dedicated field is the cleanest fit because:

  1. SNI is a first-class TLS concept (RFC 6066), not custom application data
  2. Every TLS backend exposes it natively (BoringSSL SSL_get_servername, rustls server_name(), s2n get_server_name())
  3. No downcast needed — ssl_digest.sni is immediately usable without type erasure

If the team prefers keeping the struct surface smaller and routing all new fields through the extension, I can rework this to use the extension — but that would require making set public or adding a builder hook.

@nanookclaw
Copy link
Copy Markdown
Author

Thanks for the review! I'll add a wildcard SNI test case to the directory — something like matching and confirming still passes while (missing subdomain) is rejected. Will push shortly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Be able to get the SNI from the HTTP filter context

2 participants