fix(observe): install aws-lc-rs rustls crypto provider at startup - #4879
Tyagiquamar wants to merge 1 commit into
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
|
Our CI is green and the code is running in prod, could you share a reproduction? Otherwise we can't validate this fix actually fixes anything |
|
Thank you @jmg-duarte for the review. You are 100% correct — project CI and production binaries ( Environment & Reproduction
I have read the CLA Document and I hereby sign the CLA |
|
Hi @jmg-duarte! The reproduction scenario occurs when |
|
rechek |
|
I have read the CLA Document and I hereby sign the CLA |
|
@jmg-duarte here is a reproduction that does not depend on the pAMM stream or a live node. It only needs a process that has linked both rustls providers and has not opened TLS yet. Both const AMBIGUOUS: &str = "Could not automatically determine the process-level CryptoProvider";
#[test]
fn tls_panics_until_a_crypto_provider_is_installed() {
let ring = rustls::crypto::ring::default_provider();
let aws_lc_rs = rustls::crypto::aws_lc_rs::default_provider();
assert!(!ring.cipher_suites.is_empty());
assert!(!aws_lc_rs.cipher_suites.is_empty());
assert!(
rustls::crypto::CryptoProvider::get_default().is_none(),
"a provider was already installed; this test must run before any TLS handshake"
);
let hook = std::panic::take_hook();
std::panic::set_hook(Box::new(|_| {}));
let built = std::panic::catch_unwind(rustls::ClientConfig::builder);
std::panic::set_hook(hook);
let Err(panic) = built else {
panic!("ClientConfig::builder() succeeded, so the providers are no longer ambiguous")
};
let message = panic
.downcast_ref::<String>()
.map(String::as_str)
.or_else(|| panic.downcast_ref::<&str>().copied())
.unwrap_or("<non-string panic payload>");
assert!(
message.contains(AMBIGUOUS),
"panicked for an unrelated reason: {message}"
);
aws_lc_rs
.install_default()
.expect("no provider should have been installed yet");
let _ = rustls::ClientConfig::builder();
}How this shows up in the binaries: if Prod can stay green when the block stream is a websocket, because alloy installs Full write-up is in #4736. |
|
This pull request has been marked as stale because it has been inactive a while. Please update this pull request or it will be automatically closed. |
|
Keeping this PR active. As detailed in the reproduction test case above, registering the default crypto provider during startup prevents runtime panics when multiple TLS crypto providers are linked in the build graph. Ready for maintainer review whenever convenient. |
|
@jmg-duarte Friendly ping to see if you had a chance to check the reproduction test case above. Please let me know if any further details or adjustments are needed. Thanks! |
Root Cause
rustls 0.23 panics when no default crypto provider is installed at process startup and both ring and aws-lc-rs providers are linked into the dependency graph via feature unification.
Fix
Installed aws-lc-rs as default provider in observe::initialize(), and added rustls with the aws_lc_rs feature to crates/observe/Cargo.toml.
Fixes #4736