diff --git a/crates/starknet_transaction_prover/src/server.rs b/crates/starknet_transaction_prover/src/server.rs index 38e0f9abda3..32d36e18826 100644 --- a/crates/starknet_transaction_prover/src/server.rs +++ b/crates/starknet_transaction_prover/src/server.rs @@ -47,6 +47,8 @@ pub const OHTTP_JSONRPSEE_BODY_BUILDER: fn(Full) -> HttpBody = HttpBody:: /// than to the OHTTP ciphertext envelope. `MapRequestBodyLayer`/`MapResponseBodyLayer` keep /// `HttpBody` on both sides of OHTTP to satisfy its symmetric-body bound; `HttpBody::new` is a /// zero-cost wrapper, so non-OHTTP requests still stream through unbuffered. +/// - `RequestSpanLayer` sits BELOW `OhttpLayer` so it spans the decapsulated inner request with a +/// fresh, envelope-unlinkable id (see `request_span`). macro_rules! prover_http_middleware { ($cors_layer:expr, $ohttp_layer:expr $(,)?) => { ServiceBuilder::new() @@ -55,6 +57,7 @@ macro_rules! prover_http_middleware { .option_layer($cors_layer) .layer(MapRequestBodyLayer::new(HttpBody::new)) .option_layer($ohttp_layer) + .layer(RequestSpanLayer) .layer(MapResponseBodyLayer::new(HttpBody::new)) .layer(CompressionLayer::new()) }; @@ -70,12 +73,14 @@ pub mod middleware_test_utils; #[cfg(test)] pub mod mock_rpc; pub mod request_log; +pub mod request_span; pub mod rpc_api; pub mod rpc_impl; pub mod tls; pub use health::{HealthLayer, HEALTH_PATH}; pub use request_log::{RequestLogLayer, REQUEST_ID_HEADER}; +pub use request_span::RequestSpanLayer; #[cfg(test)] mod rpc_spec_test; diff --git a/crates/starknet_transaction_prover/src/server/ohttp_integration_test.rs b/crates/starknet_transaction_prover/src/server/ohttp_integration_test.rs index d09e5080dc5..ebe2f5582f0 100644 --- a/crates/starknet_transaction_prover/src/server/ohttp_integration_test.rs +++ b/crates/starknet_transaction_prover/src/server/ohttp_integration_test.rs @@ -1,8 +1,7 @@ //! Integration tests for the sequencer's OHTTP wiring. These tests exercise //! the `tower_ohttp::OhttpLayer` with `jsonrpsee::server::HttpBody` as the -//! response body type and the same middleware stack used in production -//! (`OhttpLayer` outermost, `CompressionLayer` between OHTTP and the inner -//! service). +//! response body type, and — where the layer order matters — the production +//! middleware chain itself via `prover_http_middleware!`. //! //! The body-type-agnostic layer behavior (method/path/status/content-type //! preservation, error paths, body size limits, passthrough) is covered by @@ -16,9 +15,9 @@ use std::io::Read; use flate2::read::GzDecoder; use http::header; -use http_body_util::{BodyExt, Full}; +use http_body_util::BodyExt; use jsonrpsee::server::HttpBody; -use tower::{BoxError, Layer, Service}; +use tower::{BoxError, Layer, Service, ServiceBuilder}; use tower_http::compression::CompressionLayer; use tower_http::cors::CorsLayer; use tower_http::map_request_body::MapRequestBodyLayer; @@ -30,15 +29,13 @@ use tower_ohttp::test_utils::{ }; use tower_ohttp::OhttpLayer; +use crate::server::request_log::{RequestLogLayer, REQUEST_ID_HEADER}; +use crate::server::request_span::RequestSpanLayer; +use crate::server::{HealthLayer, OHTTP_JSONRPSEE_BODY_BUILDER}; + const DEFAULT_BODY_LIMIT: usize = 102_400; const KEY_CACHE_SECS: u64 = 3600; -/// Body builder for jsonrpsee's `HttpBody`. Returned as a `fn` pointer to -/// give `OhttpLayer` a sized, `Copy` closure type without an `as` cast. -fn body_builder() -> fn(Full) -> HttpBody { - HttpBody::new -} - /// Echo service with jsonrpsee's `HttpBody` on both sides — matches the /// layer's new symmetric-body inner service bound. async fn jsonrpsee_echo_service( @@ -67,8 +64,12 @@ fn ohttp_http_request(encapsulated: Vec) -> http::Request { #[tokio::test] async fn ohttp_round_trip_with_jsonrpsee_body() { let gateway = test_gateway(); - let layer = - OhttpLayer::new(gateway.clone(), DEFAULT_BODY_LIMIT, KEY_CACHE_SECS, body_builder()); + let layer = OhttpLayer::new( + gateway.clone(), + DEFAULT_BODY_LIMIT, + KEY_CACHE_SECS, + OHTTP_JSONRPSEE_BODY_BUILDER, + ); let mut svc = layer.layer(tower::service_fn(jsonrpsee_echo_service)); let json_body = br#"{"jsonrpc":"2.0","method":"starknet_specVersion","id":1}"#; @@ -85,24 +86,21 @@ async fn ohttp_round_trip_with_jsonrpsee_body() { assert_eq!(decapsulated.body, json_body); } -/// Verify the full production `ServiceBuilder` chain compresses the *inner* -/// JSON-RPC response and leaves the *outer* OHTTP envelope uncompressed. -/// Mirrors the exact chain in `server.rs`/`tls.rs`, so any drift in layer -/// order or a missing `MapResponseBodyLayer` will break this test. +/// Verify the production middleware chain compresses the *inner* JSON-RPC +/// response and leaves the *outer* OHTTP envelope uncompressed. Runs the +/// actual `prover_http_middleware!` chain, so any layer reorder that breaks +/// compress-then-encrypt fails here. #[tokio::test] async fn production_chain_compresses_inner_not_outer() { let gateway = test_gateway(); - let ohttp_layer = - OhttpLayer::new(gateway.clone(), DEFAULT_BODY_LIMIT, KEY_CACHE_SECS, body_builder()); - - // Replicates the production ServiceBuilder chain from `server.rs`/`tls.rs`. - // Must be kept in sync with those files. - let mut svc = tower::ServiceBuilder::new() - .option_layer(None::) - .layer(MapRequestBodyLayer::new(HttpBody::new)) - .option_layer(Some(ohttp_layer)) - .layer(MapResponseBodyLayer::new(HttpBody::new)) - .layer(CompressionLayer::new()) + let ohttp_layer = OhttpLayer::new( + gateway.clone(), + DEFAULT_BODY_LIMIT, + KEY_CACHE_SECS, + OHTTP_JSONRPSEE_BODY_BUILDER, + ); + + let mut svc = prover_http_middleware!(None::, Some(ohttp_layer)) .service(tower::service_fn(jsonrpsee_echo_service)); // Body must be large enough for gzip to actually compress. @@ -158,8 +156,12 @@ async fn production_chain_compresses_inner_not_outer() { #[tokio::test] async fn non_ohttp_request_passes_through_jsonrpsee() { let gateway = test_gateway(); - let layer = - OhttpLayer::new(gateway.clone(), DEFAULT_BODY_LIMIT, KEY_CACHE_SECS, body_builder()); + let layer = OhttpLayer::new( + gateway.clone(), + DEFAULT_BODY_LIMIT, + KEY_CACHE_SECS, + OHTTP_JSONRPSEE_BODY_BUILDER, + ); let mut svc = layer.layer(tower::service_fn(jsonrpsee_echo_service)); let json_body = br#"{"jsonrpc":"2.0","method":"starknet_specVersion","id":1}"#; @@ -176,3 +178,73 @@ async fn non_ohttp_request_passes_through_jsonrpsee() { let body = response.into_body().collect().await.unwrap().to_bytes(); assert_eq!(body.as_ref(), json_body); } + +/// End-to-end OHTTP unlinkability: the request-id echoed on the OUTER +/// (relay-visible) response must differ from the fresh id bound to the +/// decapsulated inner dispatch, and the client-supplied inner id must be +/// discarded — so no shared key links the relay's view to the gateway's. +/// Runs the actual `prover_http_middleware!` chain, so a layer reorder that +/// reverts either property fails here. +#[tokio::test] +async fn ohttp_inner_request_id_unlinkable_from_envelope() { + let gateway = test_gateway(); + let ohttp_layer = OhttpLayer::new( + gateway.clone(), + DEFAULT_BODY_LIMIT, + KEY_CACHE_SECS, + OHTTP_JSONRPSEE_BODY_BUILDER, + ); + + // Inner service echoes the request-id it observes into the response body. + let echo_id = tower::service_fn(|req: http::Request| async move { + let id = req.headers().get(REQUEST_ID_HEADER).map_or("", |v| v.to_str().unwrap()); + Ok::<_, BoxError>( + http::Response::builder() + .status(http::StatusCode::OK) + .body(HttpBody::from(id.as_bytes().to_vec())) + .unwrap(), + ) + }); + + let mut svc = prover_http_middleware!(None::, Some(ohttp_layer)).service(echo_id); + + // The envelope carries a client-chosen inner id that must be discarded. + let (encapsulated, client_response) = encapsulate_bhttp_request( + &gateway, + "POST", + "/", + b"", + &[("x-request-id", b"inner-client-id")], + ); + + // The outer envelope request carries the relay-visible id. + let mut outer_request = ohttp_http_request(encapsulated); + outer_request + .headers_mut() + .insert(REQUEST_ID_HEADER, http::HeaderValue::from_static("envelope-relay-id")); + + let response = svc.call(outer_request).await.unwrap(); + + // The outer (relay-visible) response echoes the envelope id. + let envelope_id = + response.headers().get(REQUEST_ID_HEADER).unwrap().to_str().unwrap().to_owned(); + assert_eq!(envelope_id, "envelope-relay-id"); + + let encrypted_body = response.into_body().collect().await.unwrap().to_bytes(); + let decapsulated = decapsulate_bhttp_response(client_response, &encrypted_body); + assert_eq!(decapsulated.status, 200); + let inner_id = String::from_utf8(decapsulated.body).expect("utf8 inner id"); + + assert_ne!(inner_id, envelope_id, "inner id must not equal the relay-visible envelope id"); + assert_ne!(inner_id, "inner-client-id", "client-supplied inner id must be discarded"); + assert!( + uuid::Uuid::parse_str(&inner_id).is_ok(), + "inner id must be a fresh UUID, got {inner_id:?}" + ); + // No id is set on the inner *response*, so nothing — neither the envelope + // id nor the fresh content id — leaks into the encrypted reply's headers. + assert!( + decapsulated.bhttp_message.header().get(b"x-request-id").is_none(), + "inner OHTTP response must not carry an x-request-id header" + ); +} diff --git a/crates/starknet_transaction_prover/src/server/request_span.rs b/crates/starknet_transaction_prover/src/server/request_span.rs new file mode 100644 index 00000000000..413a1d205bf --- /dev/null +++ b/crates/starknet_transaction_prover/src/server/request_span.rs @@ -0,0 +1,80 @@ +//! tower middleware that binds an `http_request` tracing span over the +//! downstream dispatch. It sits BELOW the OHTTP layer, so it sees the +//! decapsulated inner request (or a plaintext pass-through), and picks the id: +//! +//! - **plaintext** — reuse the `x-request-id` the outer layer already assigned; +//! - **OHTTP-decapsulated** ([`tower_ohttp::Decapsulated`]) — mint a fresh id (any client-supplied +//! inner id was already stripped at decapsulation). The relay never observes it, so the +//! relay-visible envelope id and this content-log id share no join key. Note the residual: at low +//! traffic volume, timestamp proximity in persisted logs still permits probabilistic correlation +//! — an OHTTP traffic-analysis property id separation cannot eliminate. +//! +//! See [`super::request_log`] for why the envelope and content ids are kept +//! separate (OHTTP unlinkability). + +use std::task::{Context, Poll}; + +use http::{Request, Response}; +use tower::{Layer, Service}; +use tower_ohttp::Decapsulated; +use tracing::instrument::Instrumented; +use tracing::{info_span, Instrument}; + +use crate::server::request_log::{ + extract_or_generate_request_id, + new_request_id, + request_id_header_value, + REQUEST_ID_HEADER, +}; + +#[cfg(test)] +#[path = "request_span_test.rs"] +mod request_span_test; + +/// tower [`Layer`] producing [`RequestSpanService`]. +#[derive(Clone, Copy, Default)] +pub struct RequestSpanLayer; + +impl Layer for RequestSpanLayer { + type Service = RequestSpanService; + + fn layer(&self, inner: S) -> Self::Service { + RequestSpanService { inner } + } +} + +#[derive(Clone)] +pub struct RequestSpanService { + inner: S, +} + +impl Service> for RequestSpanService +where + S: Service, Response = Response>, +{ + type Response = Response; + type Error = S::Error; + type Future = Instrumented; + + fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll> { + self.inner.poll_ready(cx) + } + + fn call(&mut self, mut request: Request) -> Self::Future { + let request_id = if request.extensions().get::().is_some() { + // Fresh id, distinct from the relay-visible envelope id (OHTTP + // unlinkability). Inserted so downstream readers see the id the + // span carries. + let fresh_id = new_request_id(); + request.headers_mut().insert(REQUEST_ID_HEADER, request_id_header_value(&fresh_id)); + fresh_id + } else { + // Re-derives, via the shared validator, the exact id + // `RequestLogLayer` already assigned — the header is left + // untouched. This also keeps the layer correct standalone, + // e.g. in unit tests without `RequestLogLayer` upstream. + extract_or_generate_request_id(&request) + }; + self.inner.call(request).instrument(info_span!("http_request", request_id = %request_id)) + } +} diff --git a/crates/starknet_transaction_prover/src/server/request_span_test.rs b/crates/starknet_transaction_prover/src/server/request_span_test.rs new file mode 100644 index 00000000000..f800cde5d5c --- /dev/null +++ b/crates/starknet_transaction_prover/src/server/request_span_test.rs @@ -0,0 +1,70 @@ +use bytes::Bytes; +use http::{Method, Request}; +use http_body_util::Full; +use jsonrpsee::server::HttpBody; +use tower::{Layer, ServiceExt}; +use tower_ohttp::Decapsulated; + +use crate::server::middleware_test_utils::{echo_request_id_service, read_body_and_headers}; +use crate::server::request_log::{RequestLogLayer, REQUEST_ID_HEADER}; +use crate::server::request_span::RequestSpanLayer; + +#[tokio::test] +async fn plaintext_reuses_inbound_request_id() { + let request = Request::builder() + .method(Method::POST) + .uri("/") + .header(REQUEST_ID_HEADER, "reused-xyz") + .body(HttpBody::new(Full::new(Bytes::new()))) + .expect("static body is infallible"); + + let response = + RequestSpanLayer.layer(echo_request_id_service()).oneshot(request).await.unwrap(); + + let (body, _headers) = read_body_and_headers(response).await; + assert_eq!(body, "reused-xyz"); +} + +#[tokio::test] +async fn decapsulated_gets_fresh_id_discarding_inbound() { + let mut request = Request::builder() + .method(Method::POST) + .uri("/") + .header(REQUEST_ID_HEADER, "envelope-abc") + .body(HttpBody::new(Full::new(Bytes::new()))) + .expect("static body is infallible"); + request.extensions_mut().insert(Decapsulated); + + let response = + RequestSpanLayer.layer(echo_request_id_service()).oneshot(request).await.unwrap(); + + let (id, _headers) = read_body_and_headers(response).await; + assert_ne!(id, "envelope-abc", "must discard the client-supplied inner id"); + assert!(uuid::Uuid::parse_str(&id).is_ok(), "must mint a fresh UUID, got {id:?}"); +} + +/// The cross-layer plaintext contract: with `RequestLogLayer` (outer) stacked +/// over `RequestSpanLayer` (inner) and no inbound id, the id the outer layer +/// generates and echoes on the response must be the same id the inner layer +/// binds for the handler — one shared id end-to-end. +#[tokio::test] +async fn plaintext_log_and_span_layers_share_generated_id() { + let request = Request::builder() + .method(Method::POST) + .uri("/") + .body(HttpBody::new(Full::new(Bytes::new()))) + .expect("static body is infallible"); + + let svc = RequestLogLayer.layer(RequestSpanLayer.layer(echo_request_id_service())); + let response = svc.oneshot(request).await.unwrap(); + + let (handler_id, headers) = read_body_and_headers(response).await; + let echoed_id = + headers.get(REQUEST_ID_HEADER).expect("response carries the id").to_str().unwrap(); + + assert_eq!(echoed_id, handler_id, "echoed response id must equal the id the handler saw"); + assert!( + uuid::Uuid::parse_str(&handler_id).is_ok(), + "generated id must be a UUID, got {handler_id:?}" + ); +} diff --git a/crates/starknet_transaction_prover/src/server/tls.rs b/crates/starknet_transaction_prover/src/server/tls.rs index 67c737ec9ac..c28f597c390 100644 --- a/crates/starknet_transaction_prover/src/server/tls.rs +++ b/crates/starknet_transaction_prover/src/server/tls.rs @@ -27,7 +27,7 @@ use tower_http::map_request_body::MapRequestBodyLayer; use tower_http::map_response_body::MapResponseBodyLayer; use tracing::warn; -use crate::server::{HealthLayer, OhttpJsonrpseeLayer, RequestLogLayer}; +use crate::server::{HealthLayer, OhttpJsonrpseeLayer, RequestLogLayer, RequestSpanLayer}; /// Maximum time allowed for a TLS handshake before the connection is dropped. const TLS_HANDSHAKE_TIMEOUT: Duration = Duration::from_secs(10); diff --git a/crates/tower_ohttp/src/bhttp_codec.rs b/crates/tower_ohttp/src/bhttp_codec.rs index 8261e158761..851eeb0bbb6 100644 --- a/crates/tower_ohttp/src/bhttp_codec.rs +++ b/crates/tower_ohttp/src/bhttp_codec.rs @@ -11,6 +11,12 @@ use tracing::debug; use crate::errors::OhttpError; use crate::OHTTP_RESPONSE_CONTENT_TYPE; +/// Correlation-id header stripped from decapsulated inner requests. A +/// client-chosen id inside the encrypted envelope would otherwise reach +/// gateway logs as a stable, attacker-chosen join key, undermining OHTTP +/// unlinkability; downstream layers mint their own id instead. +const REQUEST_ID_HEADER: &[u8] = b"x-request-id"; + /// Rebuild a standard `http::Request>` from a parsed Binary HTTP /// message. /// @@ -18,7 +24,9 @@ use crate::OHTTP_RESPONSE_CONTENT_TYPE; /// missing either is rejected with `OhttpError::InvalidFormat` rather than /// silently defaulted. All BHTTP header fields are forwarded to the inner /// request — this includes `content-type`, `accept-encoding`, and anything -/// else the client specified inside the encrypted envelope. +/// else the client specified inside the encrypted envelope — except +/// `content-length` (recomputed from the body) and `x-request-id` (stripped; +/// see `REQUEST_ID_HEADER`). pub fn rebuild_request( bhttp_message: &bhttp::Message, ) -> Result>, OhttpError> { @@ -50,7 +58,9 @@ pub fn rebuild_request( // CompressionLayer to compress the response before OHTTP encryption. // Skip Content-Length — we set it from the body length above. for field in bhttp_message.header().fields() { - if field.name().eq_ignore_ascii_case(b"content-length") { + if field.name().eq_ignore_ascii_case(b"content-length") + || field.name().eq_ignore_ascii_case(REQUEST_ID_HEADER) + { continue; } builder = builder.header(field.name(), field.value()); diff --git a/crates/tower_ohttp/src/layer.rs b/crates/tower_ohttp/src/layer.rs index 762066a62fe..15e9deee6bc 100644 --- a/crates/tower_ohttp/src/layer.rs +++ b/crates/tower_ohttp/src/layer.rs @@ -34,7 +34,7 @@ use tracing::debug; use crate::bhttp_codec::{encapsulate_response, rebuild_request}; use crate::errors::OhttpError; use crate::gateway::OhttpGateway; -use crate::{OHTTP_KEYS_PATH, OHTTP_REQUEST_CONTENT_TYPE}; +use crate::{Decapsulated, OHTTP_KEYS_PATH, OHTTP_REQUEST_CONTENT_TYPE}; /// Shared runtime state for the OHTTP gateway. struct OhttpState { @@ -194,7 +194,11 @@ where OhttpError::InvalidFormat("Invalid Binary HTTP message") })?; - let inner_request = rebuild_request(&bhttp_message)?.map(build_body); + let mut inner_request = rebuild_request(&bhttp_message)?.map(build_body); + // Mark the request so downstream layers can tell decapsulated + // traffic apart from plaintext pass-through (e.g. to assign a + // fresh, envelope-unlinkable request id). + inner_request.extensions_mut().insert(Decapsulated); inner.oneshot(inner_request).await.map_err(|error| { debug!("Inner service error after successful OHTTP decapsulation: {error:?}"); @@ -267,6 +271,38 @@ mod tests { assert_eq!(response.body, body); } + #[tokio::test] + async fn decapsulation_strips_client_supplied_request_id() { + // A client-chosen x-request-id inside the encrypted envelope must never + // reach the inner service — it would land in gateway logs as a stable, + // attacker-chosen join key (see `Decapsulated` in lib.rs). + let layer = test_layer(); + let svc = layer.layer(tower::service_fn( + |request: http::Request>| async move { + let saw_request_id = + if request.headers().contains_key("x-request-id") { "true" } else { "false" }; + Ok::<_, tower::BoxError>( + http::Response::builder() + .status(http::StatusCode::OK) + .header("x-echo-saw-request-id", saw_request_id) + .body(http_body_util::Full::new(bytes::Bytes::new())) + .unwrap(), + ) + }, + )); + let mut harness = TestHarness { gateway: test_gateway(), svc }; + + let response = harness + .ohttp_round_trip("POST", "/", b"", &[("x-request-id", b"client-inner-id")]) + .await; + + assert_eq!(response.status, 200); + assert_eq!( + response.bhttp_message.header().get(b"x-echo-saw-request-id").unwrap(), + b"false" + ); + } + #[tokio::test] async fn non_post_method_round_trip() { // GET /health encapsulated in OHTTP must reach the inner service as GET. diff --git a/crates/tower_ohttp/src/lib.rs b/crates/tower_ohttp/src/lib.rs index ce67590edde..31069ca33f0 100644 --- a/crates/tower_ohttp/src/lib.rs +++ b/crates/tower_ohttp/src/lib.rs @@ -34,6 +34,20 @@ pub use errors::OhttpError; pub use gateway::OhttpGateway; pub use layer::{OhttpLayer, OhttpService}; +/// Marker inserted into the extensions of a request rebuilt from a decapsulated +/// OHTTP envelope, before it is forwarded to the inner service. Downstream +/// layers can check for it (`request.extensions().get::()`) to +/// distinguish envelope-decapsulated traffic from plaintext pass-through — +/// the two are otherwise indistinguishable once the inner request is rebuilt. +/// +/// Layers may key privacy decisions on this marker (e.g. minting a request id +/// unlinkable to the envelope). Extensions are silently dropped by any +/// intermediate layer that rebuilds the request, so the marker is not the last +/// line of defense: `x-request-id` is already stripped at decapsulation, so a +/// lost marker costs log ergonomics, never unlinkability. +#[derive(Clone, Copy, Debug)] +pub struct Decapsulated; + pub(crate) const OHTTP_REQUEST_CONTENT_TYPE: &str = "message/ohttp-req"; pub(crate) const OHTTP_RESPONSE_CONTENT_TYPE: &str = "message/ohttp-res"; pub(crate) const OHTTP_KEYS_PATH: &str = "/ohttp-keys";