Skip to content

feat: make rustls cert public#858

Open
shaneutt wants to merge 4 commits intocloudflare:mainfrom
praxis-proxy:certificate-public
Open

feat: make rustls cert public#858
shaneutt wants to merge 4 commits intocloudflare:mainfrom
praxis-proxy:certificate-public

Conversation

@shaneutt
Copy link
Copy Markdown
Contributor

This enables a deeper level of control from implementations utilizing Pingora over certificates.

This is being paired with #726 in our fork right now.

@github-project-automation github-project-automation Bot moved this to Backlog in AI Gateway Apr 14, 2026
@shaneutt shaneutt moved this from Backlog to Review in AI Gateway Apr 14, 2026
@drcaramelsyrup drcaramelsyrup added the enhancement New feature or request label Apr 17, 2026
@johnhurt
Copy link
Copy Markdown
Contributor

@fabian4 @nojima — could you review this when you get a chance?

@nojima
Copy link
Copy Markdown
Contributor

nojima commented Apr 26, 2026

@shaneutt
Could you please explain how you are using raw_cert and cert? Do both of these need to be public? Or would it be sufficient to expose only a few methods, such as borrow_cert()?

@shaneutt
Copy link
Copy Markdown
Contributor Author

@shaneutt Could you please explain how you are using raw_cert and cert? Do both of these need to be public? Or would it be sufficient to expose only a few methods, such as borrow_cert()?

Thank you for your time @nojima!

The primary need is constructing WrappedX509 values from DER bytes outside the pingora-core crate. In Praxis, we cache CA certificate DER bytes at config time and convert them into WrappedX509 values for assignment to HttpPeer.options.ca when configuring per-upstream TLS verification:

upstream_peer.rs L105-L111:

fn ca_from_cached(                                                                                                                                           
    cached: &praxis_tls::CachedCaCerts,                   
) -> Vec<pingora_core::utils::tls::WrappedX509> {                                                                                                            
    cached                                                                                                                                                   
        .der_certs()                                                                                                                                         
        .iter()                                                                                                                                              
        .map(|der| {                                      
            pingora_core::utils::tls::WrappedX509::new(                                                                                                      
                der.clone(),                                                                                                                                 
                pingora_core::utils::tls::parse_x509,
            )                                                                                                                                                
        })                                                
        .collect()
}

Called during upstream peer selection to populate per-cluster CA roots.

So what this PR provides, broken down:

  1. pub_extras makes WrappedX509::new() public (ouroboros gates the constructor on this, not on field visibility)
  2. pub fn parse_x509() is needed as the builder argument to new()
  3. pub fields make the ouroboros-generated borrow_cert() and borrow_raw_cert() accessors public (ouroboros ties accessor visibility to field visibility)

Items 1 and 2 are strictly required for our use case. Item 3 is not something we use today, but it seems generally useful for any downstream that needs to inspect certificate contents after construction. Let me know your thoughts? And again, thank you for your time 🙇

@nojima
Copy link
Copy Markdown
Contributor

nojima commented Apr 28, 2026

@shaneutt
Thank you for the use cases🙏

The approach makes sense, but I have a slight concern about the public API surface. I consider the fact that WrappedX509 is implemented using ouroboros to be an implementation detail, and I'd prefer not to leak it into the public interface. Similarly, parse_x509 doesn't seem like a good candidate for the crate's public API. So, instead of exposing these directly, how about adding a new function to WrappedX509?

impl WrappedX509 {
    pub fn parse(raw_cert: Vec<u8>) -> Self {
        Self::new(raw_cert, parse_x509)
    }
    ...
}

With this function, you can create WrappedX509 instances without exposing new or parse_x509, which should cover your use cases 1 and 2.

As for use case 3, we could support it by adding a new method rather than making the fields public:

    pub fn parsed(&self) -> &X509Certificate<'_> {
        self.borrow_cert()
    }

This approach avoids exposing the various ouroboros-generated methods, leaving us with more flexibility for future design decisions.

That said, I'm hesitant about implementing use case 3 right now. Removing a public function once it's been published is generally difficult. If you don't have an immediate need for it, I think it would be fine to address only use cases 1 and 2 in this PR and leave 3 for later.

podpress and others added 4 commits April 29, 2026 12:51
Adapted from cloudflare#726. Allows full control over the
rustls ServerConfig, including 0-RTT, session resumption, and custom
certificate resolvers.
Signed-off-by: Shane Utt <shaneutt@linux.com>
Signed-off-by: Shane Utt <shaneutt@linux.com>
Signed-off-by: Shane Utt <shaneutt@linux.com>
@shaneutt shaneutt force-pushed the certificate-public branch from 875e4d9 to 514d06b Compare April 29, 2026 17:01
@shaneutt
Copy link
Copy Markdown
Contributor Author

This approach avoids exposing the various ouroboros-generated methods, leaving us with more flexibility for future design decisions.

That said, I'm hesitant about implementing use case 3 right now. Removing a public function once it's been published is generally difficult. If you don't have an immediate need for it, I think it would be fine to address only use cases 1 and 2 in this PR and leave 3 for later.

Sounds good 👍

Switched to that approach, and pushed it up. LMKWYT.

Note: I also included: dependency updates because audit was failing on reqwest; pinned actions to commit SHAs due to security restrictions for our fork. Let me know if you want me to drop anything.

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.

5 participants