Return rustls listener FIPS failures as errors#885
Open
eldryoth wants to merge 1 commit into
Open
Conversation
9987b73 to
49baf28
Compare
Collaborator
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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