From 01b81a51e0079e62581b97a550cac6ef60d552a7 Mon Sep 17 00:00:00 2001 From: "Joshua M. Clulow" Date: Sun, 16 Apr 2023 22:49:02 -0700 Subject: [PATCH 1/2] include address of remote peer in RequestInfo --- dropshot/src/handler.rs | 19 +++++++++++-------- dropshot/src/server.rs | 4 +++- dropshot/src/websocket.rs | 4 +++- 3 files changed, 17 insertions(+), 10 deletions(-) diff --git a/dropshot/src/handler.rs b/dropshot/src/handler.rs index 38adc2ad9..1ed0f6c28 100644 --- a/dropshot/src/handler.rs +++ b/dropshot/src/handler.rs @@ -96,25 +96,24 @@ pub struct RequestInfo { uri: http::Uri, version: http::Version, headers: http::HeaderMap, + remote_addr: std::net::SocketAddr, } -impl From<&hyper::Request> for RequestInfo { - fn from(request: &hyper::Request) -> Self { +impl RequestInfo { + pub(crate) fn new( + request: &hyper::Request, + remote_addr: std::net::SocketAddr, + ) -> Self { RequestInfo { method: request.method().clone(), uri: request.uri().clone(), version: request.version(), headers: request.headers().clone(), + remote_addr, } } } -impl From> for RequestInfo { - fn from(request: hyper::Request) -> Self { - Self::from(&request) - } -} - impl RequestInfo { pub fn method(&self) -> &http::Method { &self.method @@ -132,6 +131,10 @@ impl RequestInfo { &self.headers } + pub fn remote_addr(&self) -> &std::net::SocketAddr { + &self.remote_addr + } + /// Returns a reference to the `RequestInfo` itself /// /// This is provided for source compatibility. In previous versions of diff --git a/dropshot/src/server.rs b/dropshot/src/server.rs index da7c649eb..f2326e2ee 100644 --- a/dropshot/src/server.rs +++ b/dropshot/src/server.rs @@ -714,6 +714,7 @@ async fn http_request_handle_wrap( request, &request_id, request_log.new(o!()), + remote_addr, ) .await; @@ -773,6 +774,7 @@ async fn http_request_handle( request: Request, request_id: &str, request_log: Logger, + remote_addr: std::net::SocketAddr, ) -> Result, HttpError> { // TODO-hardening: is it correct to (and do we correctly) read the entire // request body even if we decide it's too large and are going to send a 400 @@ -786,7 +788,7 @@ async fn http_request_handle( server.router.lookup_route(&method, uri.path().into())?; let rqctx = RequestContext { server: Arc::clone(&server), - request: RequestInfo::from(&request), + request: RequestInfo::new(&request, remote_addr), path_variables: lookup_result.variables, body_content_type: lookup_result.body_content_type, request_id: request_id.to_string(), diff --git a/dropshot/src/websocket.rs b/dropshot/src/websocket.rs index eeddfcf22..0963ef22f 100644 --- a/dropshot/src/websocket.rs +++ b/dropshot/src/websocket.rs @@ -316,6 +316,8 @@ mod tests { .header(http::header::SEC_WEBSOCKET_KEY, "aGFjayB0aGUgcGxhbmV0IQ==") .body(Body::empty()) .unwrap(); + let remote_addr = + SocketAddr::new(IpAddr::V6(Ipv6Addr::LOCALHOST), 12345); let rqctx = RequestContext { server: Arc::new(DropshotState { private: (), @@ -332,7 +334,7 @@ mod tests { ), tls_acceptor: None, }), - request: RequestInfo::from(&request), + request: RequestInfo::new(&request, remote_addr), path_variables: Default::default(), body_content_type: Default::default(), request_id: "".to_string(), From 0409422412927e32285cf90395c9825dea764378 Mon Sep 17 00:00:00 2001 From: "Joshua M. Clulow" Date: Mon, 17 Apr 2023 20:11:41 -0700 Subject: [PATCH 2/2] XXX feedback from dap --- dropshot/src/handler.rs | 4 ++-- dropshot/tests/test_demo.rs | 47 +++++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/dropshot/src/handler.rs b/dropshot/src/handler.rs index 1ed0f6c28..98af28f61 100644 --- a/dropshot/src/handler.rs +++ b/dropshot/src/handler.rs @@ -131,8 +131,8 @@ impl RequestInfo { &self.headers } - pub fn remote_addr(&self) -> &std::net::SocketAddr { - &self.remote_addr + pub fn remote_addr(&self) -> std::net::SocketAddr { + self.remote_addr } /// Returns a reference to the `RequestInfo` itself diff --git a/dropshot/tests/test_demo.rs b/dropshot/tests/test_demo.rs index 41702b9cb..7d9052e3f 100644 --- a/dropshot/tests/test_demo.rs +++ b/dropshot/tests/test_demo.rs @@ -83,6 +83,7 @@ fn demo_api() -> ApiDescription { api.register(demo_handler_307_temporary_redirect).unwrap(); api.register(demo_handler_websocket).unwrap(); api.register(demo_handler_request_compat).unwrap(); + api.register(demo_handler_request_addresses).unwrap(); // We don't need to exhaustively test these cases, as they're tested by unit // tests. @@ -982,6 +983,35 @@ async fn test_request_compat() { testctx.teardown().await; } +#[tokio::test] +async fn test_request_remote_addr() { + let api = demo_api(); + let testctx = common::test_setup("test_request_remote_addr", api); + let laddr = testctx.server.local_addr(); + let mut response = testctx + .client_testctx + .make_request( + Method::GET, + "/testing/request_addresses", + None as Option<()>, + StatusCode::OK, + ) + .await + .expect("expected success"); + let json: Vec = read_json(&mut response).await; + assert_eq!(json.len(), 4); + // Confirm the local address and port seen from inside the server matches + // the one that we got from the test context: + assert_eq!(json[0], laddr.ip().to_string()); + assert_eq!(json[1], laddr.port().to_string()); + // There does not appear to be an easy way to determine which port was + // used for the outbound request we make here, but we at least know that + // it must not be the same as the server listen port: + assert_eq!(json[2], laddr.ip().to_string()); + assert_ne!(json[3], laddr.port().to_string()); + testctx.teardown().await; +} + // Demo handler functions type RequestCtx = RequestContext; @@ -1316,6 +1346,23 @@ async fn demo_handler_request_compat( http_echo(&value) } +#[endpoint { + method = GET, + path = "/testing/request_addresses", +}] +async fn demo_handler_request_addresses( + rqctx: RequestCtx, +) -> Result, HttpError> { + let laddr = rqctx.server.local_addr; + let raddr = rqctx.request.remote_addr(); + http_echo(&vec![ + laddr.ip().to_string(), + laddr.port().to_string(), + raddr.ip().to_string(), + raddr.port().to_string(), + ]) +} + fn http_echo(t: &T) -> Result, HttpError> { Ok(Response::builder() .header(http::header::CONTENT_TYPE, CONTENT_TYPE_JSON)