Return rustls listener FIPS failures as errors#885
Conversation
9987b73 to
49baf28
Compare
| tls: self | ||
| .tls | ||
| .take() | ||
| .map(|tls| tls.try_build().map(Arc::new)) |
There was a problem hiding this comment.
Do the other TLS backends need try_build() too? This path can use their TlsSettings as well.
|
I may be wrong, but for this listener FIPS work the digest in |
|
Good catch @fabian4. The common listener stack can be built with any enabled TLS backend, so calling try_build() there means each backend TlsSettings needs that method. I added try_build() for the OpenSSL/BoringSSL and s2n listener settings, preserving their existing build() behavior. You are right @53v3n3d4. The ServerConfig could be built with the selected rustls provider, but the SslDigest certificate fingerprint still used pingora_rustls::hash_certificate(), which used ring directly. I changed the rustls listener acceptor to capture a SHA-256 hash implementation from the final ServerConfig crypto provider and pass it into the server TlsStream digest path. For require_fips(true), listener construction now fails closed if no SHA-256 provider is available. CI note: the nightly-only failure appears unrelated to this PR. The failing target is
The memory-cache target passes locally, and the relevant checks for this PR pass:
The same CI matrix appears to pass on Rust 1.84.0 and 1.91.1; only nightly reports the unrelated |
|
@eldryoth, a question when you build for fips this commit, did you check the tree if the ring is not being included? I see that default crypto provider not being gated. I am no reviewer, only read this because I am started using pingora with rustls/aws lc rs. |
|
Good question. I checked this, and this PR does not make Today The default provider path is intentionally left as-is for compatibility: if no provider is configured, Pingora keeps using the existing default ring behavior. For FIPS users, the intended path is to explicitly set the AWS-LC / FIPS provider and set So if we want instead “no ring in the dependency tree at all”, this PR is not sufficient. That would need a larger follow-up to make For us in our project we can already choose the rustls/AWS-LC FIPS provider and validate it before startup, but the final rustls listener But release builds use Our side that wires this policy into Pingora is here: It sets the selected rustls crypto provider and then calls |
|
Thanks for reply and good project. I didn't know the fips in ServerConfig but remembered me the |
Summary
This PR makes rustls listener construction able to report configuration failures through Pingora's normal
Resultpath 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 existingbuild()method remains available for compatibility and delegates totry_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 defaultringprovider. One example is a deployment profile that builds rustls with the AWS-LC FIPS-capable provider and must fail closed if the finalServerConfigdoes not reportfips().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
TlsSettings::try_build() -> pingora_error::Result<Acceptor>for rustls listeners.TlsSettings::build() -> Acceptorfor source compatibility.try_build()and propagate the error.CryptoProviderServerConfigto report FIPS modepingora-rustls.Compatibility
This is intended to be low risk for existing Pingora users:
TlsSettings::intermediate(cert, key)behavior is preserved.TlsSettings::build()remains available.set_require_fips(true)are not subject to the new FIPS check.Resulterrors 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::ServerConfigdoes 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 fmtcargo check -p pingora-core --features rustlscargo test -p pingora-core --features rustls try_build_returns_error_when_required_fips_is_not_reported