Skip to content

bug: Two rustls crypto providers are linked, so the first TLS handshake panics (in certain cases) #4736

Description

@kirsanium

Summary

Every binary in the workspace compiles rustls with both the ring and aws-lc-rs providers enabled. rustls 0.23 refuses to choose between them, so the first TLS configuration built anywhere in a process panics — unless some earlier code path happened to call CryptoProvider::install_default().

Nothing in the workspace installs a provider at startup. Today we get away with it because the code paths that reach TLS first are ones whose libraries install a provider on our behalf. That is an ordering accident, not a guarantee, and it is already load-bearing for the pAMM state-override stream.

Reproducer

const AMBIGUOUS: &str = "Could not automatically determine the process-level CryptoProvider";

#[test]
fn tls_panics_until_a_crypto_provider_is_installed() {
    // Both providers are linked. Naming each one compiles only because the
    // matching rustls feature is enabled somewhere in the graph, so this is a
    // compile-time assertion as much as a runtime one.
    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());

    // Nothing has chosen between them: no dependency installs a provider at
    // load time, and this binary has opened no connection of its own.
    assert!(
        rustls::crypto::CryptoProvider::get_default().is_none(),
        "a provider was already installed, so this test proves nothing — it must run in a process \
         that has not opened a TLS connection"
    );

    // So the first TLS config built in the process panics. This is the call
    // every rustls client bottoms out in: `tokio_tungstenite::connect_async`
    // reaches it through `tokio-tungstenite/src/tls.rs`, as does any reqwest
    // client, hyper-rustls connector or alloy websocket transport that has not
    // installed a provider first.
    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}"
    );

    // Installing one explicitly is the entire fix, and the same call every
    // affected crate has to make before its first handshake.
    aws_lc_rs
        .install_default()
        .expect("no provider should have been installed yet");
    let _ = rustls::ClientConfig::builder();
}

It asserts, in order, that both providers are linked, that no provider is installed, that rustls::ClientConfig::builder() panics, and that installing one explicitly fixes it.

Where the two providers come from

Provider Enabled via
ring observeasync-nats 0.48 → tokio-rustls/ring, tokio-websockets/ring
aws-lc-rs reqwest__rustls-aws-lc-rs, hyper-rustls/aws-lc-rs, aws-smithy-http-client/rustls-aws-lc

Both reach every binary through feature unification. Confirmed identical for simulator, driver, price-estimation, orderbook and autopilot:

$ cargo tree -p driver -e features | grep -oE 'rustls feature "(ring|aws-lc-rs)"' | sort -u
rustls feature "aws-lc-rs"
rustls feature "ring"

Relevant rustls source — rustls-0.23.41/src/crypto/mod.rs:243-256, from_crate_features() returns None when the features are ambiguous, and the .expect() above panics.

Why it has not bitten us yet

alloy-transport-ws-1.8.3/src/native.rs:138-141 installs aws-lc-rs as it opens a websocket connection:

// Install the default rustls crypto provider if not already set.
// Required since rustls 0.23+ no longer auto-installs a provider.
let _ = rustls::crypto::CryptoProvider::install_default(
    rustls::crypto::aws_lc_rs::default_provider(),
);

The binaries reach that through ethrpc::block_stream::current_block_ws_stream early in startup, which covers everything opened afterwards.

However, I managed to hit it in the process of testing a PR.

The gap

That only happens when the block stream is a websocket. crates/shared/src/current_block.rs:47-58 falls back to HTTP polling when NODE_WS_URL is unset:

match &self.node_ws_url {
    Some(ws_url) => current_block_ws_stream(alloy_provider, ws_url.clone()).await,
    None => current_block_stream(http_url, poll_interval).await,   // no ws, no provider
}

On that path nothing installs a provider. The pAMM state-override stream then becomes the process's first TLS user: it calls tokio_tungstenite::connect_async on a wss:// URL, which reaches ClientConfig::builder() at tokio-tungstenite-0.28.0/src/tls.rs:126 and panics.

The panic surfaces inside a spawned background task, so it kills that task and nothing else. The stream never delivers overrides, overrides_for reports Empty forever, and the driver keeps running with no indication that a feature it was configured for is dead.

Any future crate that opens a TLS connection without going through alloy first inherits the same trap.

Suggested fix

Install a provider once, explicitly, during startup — in observe::init or alongside it, so every binary is covered before anything opens a connection:

let _ = rustls::crypto::aws_lc_rs::default_provider().install_default();

aws-lc-rs matches what alloy already installs, so this changes no behaviour on the paths that work today; it only removes the dependency on ordering. The result is ignored deliberately: install_default returns Err when a provider is already installed, which is the normal case as soon as anything else has opened a connection, and is not a failure.

Worth considering separately: whether ring needs to be in the graph at all. async-nats enables it through default features, so default-features = false plus an explicit feature list on that dependency would remove the ambiguity at the source rather than papering over it.

Final note

Of course I do not think this is a major issue, however I can not pass it by :)

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingtrack:maintenancemaintenance track

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions