Skip to content

Return rustls listener FIPS failures as errors#885

Open
eldryoth wants to merge 1 commit into
cloudflare:mainfrom
eldryoth:rustls-listener-fips-result
Open

Return rustls listener FIPS failures as errors#885
eldryoth wants to merge 1 commit into
cloudflare:mainfrom
eldryoth:rustls-listener-fips-result

Conversation

@eldryoth
Copy link
Copy Markdown

@eldryoth eldryoth commented May 22, 2026

Summary

This PR makes rustls listener construction able to report configuration failures through Pingora's normal Result path instead of requiring downstream users to panic when a listener policy cannot be satisfied.

It adds a fallible TlsSettings::try_build() method for rustls listeners and uses it when Pingora builds listener transport stacks. The existing build() method remains available for compatibility and delegates to try_build().unwrap(), so existing direct callers keep the previous behavior.

The PR also exposes a small set of rustls server configuration hooks needed by applications that must select a specific rustls crypto provider or verify the resulting listener configuration.

Motivation

rustls 0.23 requires an explicit CryptoProvider. Some downstream applications need to use a provider selected by policy rather than Pingora's default ring provider. One example is a deployment profile that builds rustls with the AWS-LC FIPS-capable provider and must fail closed if the final ServerConfig does not report fips().

Before this change, Pingora's rustls listener path only exposed a panicking build() API. That makes policy enforcement awkward for applications that need startup to fail cleanly with structured context. The only practical downstream option was to keep a final panic assertion after normal provider and TLS policy checks.

This PR moves that failure into Pingora's normal error path.

What Changed

  • Added TlsSettings::try_build() -> pingora_error::Result<Acceptor> for rustls listeners.
  • Kept TlsSettings::build() -> Acceptor for source compatibility.
  • Changed listener stack construction to call try_build() and propagate the error.
  • Added rustls listener configuration hooks for:
    • explicit CryptoProvider
    • explicit ALPN protocol list
    • TLS 1.2 / TLS 1.3 minimum protocol selection
    • cipher suite selection
    • key exchange group selection
    • certificate resolver support
    • requiring the final ServerConfig to report FIPS mode
  • Re-exported the needed rustls server/provider types from pingora-rustls.
  • Added a regression test proving thatTitle:

Compatibility

This is intended to be low risk for existing Pingora users:

  • Existing TlsSettings::intermediate(cert, key) behavior is preserved.
  • Existing TlsSettings::build() remains available.
  • If no custom provider is configured, the rustls listener still installs/uses the default ring provider as before.
  • Existing callers that do not opt into set_require_fips(true) are not subject to the new FIPS check.
  • The new failure propagation only affects Pingora's own listener-stack build path by turning configuration failures into normal Result errors instead of panics.

Why this is useful downstream

Downstream applications that have compliance-driven TLS requirements can now express those requirements through Pingora's rustls listener configuration and let Pingora fail startup cleanly if the resulting rustls::ServerConfig does not satisfy them.

This does not make Pingora itself enforce FIPS policy globally. It only provides the hooks and error propagation needed for applications to opt into that policy.

Testing

  • cargo fmt
  • cargo check -p pingora-core --features rustls
  • cargo test -p pingora-core --features rustls try_build_returns_error_when_required_fips_is_not_reported

@eldryoth eldryoth force-pushed the rustls-listener-fips-result branch from 9987b73 to 49baf28 Compare May 22, 2026 16:53
@drcaramelsyrup drcaramelsyrup added the enhancement New feature or request label May 22, 2026
@drcaramelsyrup
Copy link
Copy Markdown
Collaborator

@nojima @fabian4 , if you have time, would you be able to review this PR?

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.

2 participants