From 4a04b360ac93dd5d0a305fee227e459979ee9503 Mon Sep 17 00:00:00 2001 From: Pablo Polvorin Date: Fri, 30 Aug 2024 17:03:04 -0300 Subject: [PATCH] do not replace host header --- .../rust/ockam/ockam_api/src/http_auth/mod.rs | 68 +++------- .../ockam_api/src/nodes/models/portal.rs | 12 +- .../tcp_inlets/background_node_client.rs | 6 +- .../nodes/service/tcp_inlets/inlets_trait.rs | 2 +- .../service/tcp_inlets/node_manager_worker.rs | 14 +- .../src/incoming_services/commands.rs | 2 +- .../ockam_command/src/http/inlet/create.rs | 120 ++++++++++-------- .../ockam_command/src/tcp/inlet/create.rs | 2 +- 8 files changed, 103 insertions(+), 123 deletions(-) diff --git a/implementations/rust/ockam/ockam_api/src/http_auth/mod.rs b/implementations/rust/ockam/ockam_api/src/http_auth/mod.rs index 181ff00c549..83e0140213a 100644 --- a/implementations/rust/ockam/ockam_api/src/http_auth/mod.rs +++ b/implementations/rust/ockam/ockam_api/src/http_auth/mod.rs @@ -28,49 +28,39 @@ struct HttpAuthInterceptorState { struct HttpAuthInterceptor { state: Arc>, token_refresher: TokenLeaseRefresher, - upstream: String, } impl HttpAuthInterceptor { - fn new(token_refresher: TokenLeaseRefresher, upstream: String) -> Self { + fn new(token_refresher: TokenLeaseRefresher) -> Self { let state = HttpAuthInterceptorState { state: RequestState::ParsingHeader(None), }; Self { state: Arc::new(Mutex::new(state)), token_refresher, - upstream, } } } pub struct HttpAuthInterceptorFactory { token_refresher: TokenLeaseRefresher, - upstream: String, } impl HttpAuthInterceptorFactory { - pub fn new(token_refresher: TokenLeaseRefresher, upstream: String) -> Self { - Self { - token_refresher, - upstream, - } + pub fn new(token_refresher: TokenLeaseRefresher) -> Self { + Self { token_refresher } } } impl PortalInterceptorFactory for HttpAuthInterceptorFactory { fn create(&self) -> Arc { - Arc::new(HttpAuthInterceptor::new( - self.token_refresher.clone(), - self.upstream.clone(), - )) + Arc::new(HttpAuthInterceptor::new(self.token_refresher.clone())) } } fn attach_auth_token_and_serialize_into( req: &httparse::Request, token: &str, - upstream: &str, buffer: &mut Vec, ) { debug!("Serializing http req header"); @@ -84,9 +74,8 @@ fn attach_auth_token_and_serialize_into( .unwrap(); write!(buffer, "Authorization: Token {}\r\n", token).unwrap(); - write!(buffer, "Host: {}\r\n", upstream).unwrap(); for h in &*req.headers { - if !(h.name.eq_ignore_ascii_case("HOST") || h.name.eq_ignore_ascii_case("Authorization")) { + if !h.name.eq_ignore_ascii_case("Authorization") { write!(buffer, "{}: ", h.name).unwrap(); buffer.extend_from_slice(h.value); buffer.extend_from_slice(b"\r\n"); @@ -124,15 +113,10 @@ fn body_state(method: &str, headers: &[Header]) -> ockam_core::Result ockam_core::Result> { + * data is received in chunks, and there is no warranty on what we get on each: + * incomplete requests, multiple requests, etc. + */ + fn process_http_buffer(&mut self, buf: &[u8], token: &str) -> ockam_core::Result> { let mut acc = Vec::with_capacity(buf.len()); let mut cursor = buf; loop { @@ -162,7 +146,7 @@ impl RequestState { } Ok(httparse::Status::Complete(body_offset)) => { cursor = &cursor[body_offset - prev_size..]; - attach_auth_token_and_serialize_into(&req, token, upstream, &mut acc); + attach_auth_token_and_serialize_into(&req, token, &mut acc); *self = body_state(req.method.unwrap(), req.headers)?; } Err(e) => { @@ -269,11 +253,9 @@ impl PortalInterceptor for HttpAuthInterceptor { if token.is_none() { error!("No authorization token available"); } - let out = guard.state.process_http_buffer( - buffer, - &token.unwrap_or_default(), - &self.upstream, - )?; + let out = guard + .state + .process_http_buffer(buffer, &token.unwrap_or_default())?; Ok(Some(out)) } } @@ -293,11 +275,9 @@ Transfer-Encoding: gzip, chunked\r\n\r\n\ const TOKEN: &str = "SAMPLE-TOKEN"; - const UPSTREAM: &str = "upstream.com:6565"; - const EXPECTED: &str = "POST / HTTP/1.1\r\n\ Authorization: Token SAMPLE-TOKEN\r\n\ -Host: upstream.com:6565\r\n\ +Host: www.example.com\r\n\ User-Agent: Mozilla/5.0\r\n\ Accept-Encoding: gzip, deflate, br\r\n\ Transfer-Encoding: gzip, chunked\r\n\r\n\ @@ -313,9 +293,7 @@ Transfer-Encoding: gzip, chunked\r\n\r\n\ let mut result = Vec::new(); let mut request_state = RequestState::ParsingHeader(None); for chunk in data.chunks(size) { - let data_out = request_state - .process_http_buffer(chunk, TOKEN, UPSTREAM) - .unwrap(); + let data_out = request_state.process_http_buffer(chunk, TOKEN).unwrap(); result.extend_from_slice(&data_out); } assert_eq!( @@ -336,11 +314,11 @@ field1=value1&field2=value2"; let expected_r = format!( "POST /test HTTP/1.1\r\n\ Authorization: Token {}\r\n\ -Host: {}\r\n\ +Host: foo.example\r\n\ Content-Type: application/x-www-form-urlencoded\r\n\ Content-Length: 27\r\n\r\n\ field1=value1&field2=value2", - TOKEN, UPSTREAM + TOKEN ); let data = [req.as_bytes(), req.as_bytes()].concat(); @@ -350,9 +328,7 @@ field1=value1&field2=value2", let mut result = Vec::new(); let mut request_state = RequestState::ParsingHeader(None); for chunk in data.chunks(size) { - let data_out = request_state - .process_http_buffer(chunk, TOKEN, UPSTREAM) - .unwrap(); + let data_out = request_state.process_http_buffer(chunk, TOKEN).unwrap(); result.extend_from_slice(&data_out); } assert_eq!( @@ -371,8 +347,8 @@ field1=value1&field2=value2", data.extend_from_slice(req.as_bytes()); let mut expected = format!( - "GET /home/user/example.txt HTTP/1.1\r\nAuthorization: Token {}\r\nHost: {}\r\n\r\n", - TOKEN, UPSTREAM + "GET /home/user/example.txt HTTP/1.1\r\nAuthorization: Token {}\r\n\r\n", + TOKEN ); expected = expected.clone() + &expected; @@ -380,9 +356,7 @@ field1=value1&field2=value2", let mut result = Vec::new(); let mut request_state = RequestState::ParsingHeader(None); for chunk in data.chunks(size) { - let data_out = request_state - .process_http_buffer(chunk, TOKEN, UPSTREAM) - .unwrap(); + let data_out = request_state.process_http_buffer(chunk, TOKEN).unwrap(); result.extend_from_slice(&data_out); } assert_eq!(String::from_utf8(result).unwrap(), expected); diff --git a/implementations/rust/ockam/ockam_api/src/nodes/models/portal.rs b/implementations/rust/ockam/ockam_api/src/nodes/models/portal.rs index dde982638a1..6a36eb01fc7 100644 --- a/implementations/rust/ockam/ockam_api/src/nodes/models/portal.rs +++ b/implementations/rust/ockam/ockam_api/src/nodes/models/portal.rs @@ -53,10 +53,10 @@ pub struct CreateInlet { /// TCP won't be used to transfer data between the Inlet and the Outlet. #[n(10)] pub disable_tcp_fallback: bool, - /// If present, it instruct the creation of HTTP inlet, that is, TCP inlet + + /// If enabled it instruct the creation of HTTP inlet, that is, TCP inlet + /// a http interceptor that modify incoming requests, adding to them an Authorization /// token. - #[n(11)] pub http_upstream: Option, + #[n(11)] pub is_http_auth_inlet: bool, } impl CreateInlet { @@ -68,7 +68,7 @@ impl CreateInlet { wait_connection: bool, enable_udp_puncture: bool, disable_tcp_fallback: bool, - http_upstream: Option, + is_http_auth_inlet: bool, ) -> Self { Self { listen_addr: listen, @@ -81,7 +81,7 @@ impl CreateInlet { secure_channel_identifier: None, enable_udp_puncture, disable_tcp_fallback, - http_upstream, + is_http_auth_inlet, } } @@ -94,7 +94,7 @@ impl CreateInlet { wait_connection: bool, enable_udp_puncture: bool, disable_tcp_fallback: bool, - http_upstream: Option, + is_http_auth_inlet: bool, ) -> Self { Self { listen_addr: listen, @@ -107,7 +107,7 @@ impl CreateInlet { secure_channel_identifier: None, enable_udp_puncture, disable_tcp_fallback, - http_upstream, + is_http_auth_inlet, } } diff --git a/implementations/rust/ockam/ockam_api/src/nodes/service/tcp_inlets/background_node_client.rs b/implementations/rust/ockam/ockam_api/src/nodes/service/tcp_inlets/background_node_client.rs index 20b76bb8e57..d5d7180da25 100644 --- a/implementations/rust/ockam/ockam_api/src/nodes/service/tcp_inlets/background_node_client.rs +++ b/implementations/rust/ockam/ockam_api/src/nodes/service/tcp_inlets/background_node_client.rs @@ -27,7 +27,7 @@ impl Inlets for BackgroundNodeClient { secure_channel_identifier: &Option, enable_udp_puncture: bool, disable_tcp_fallback: bool, - http_upstream: Option, + is_http_auth_inlet: bool, ) -> miette::Result> { let request = { let via_project = outlet_addr.matches(0, &[ProjectProto::CODE.into()]); @@ -39,7 +39,7 @@ impl Inlets for BackgroundNodeClient { wait_connection, enable_udp_puncture, disable_tcp_fallback, - http_upstream, + is_http_auth_inlet, ) } else { CreateInlet::to_node( @@ -50,7 +50,7 @@ impl Inlets for BackgroundNodeClient { wait_connection, enable_udp_puncture, disable_tcp_fallback, - http_upstream, + is_http_auth_inlet, ) }; if let Some(e) = policy_expression.as_ref() { diff --git a/implementations/rust/ockam/ockam_api/src/nodes/service/tcp_inlets/inlets_trait.rs b/implementations/rust/ockam/ockam_api/src/nodes/service/tcp_inlets/inlets_trait.rs index 81601e4edea..b1c95fd5f99 100644 --- a/implementations/rust/ockam/ockam_api/src/nodes/service/tcp_inlets/inlets_trait.rs +++ b/implementations/rust/ockam/ockam_api/src/nodes/service/tcp_inlets/inlets_trait.rs @@ -25,7 +25,7 @@ pub trait Inlets { secure_channel_identifier: &Option, enable_udp_puncture: bool, disable_tcp_fallback: bool, - http_upstream: Option, + is_http_auth_inlet: bool, ) -> miette::Result>; async fn show_inlet(&self, ctx: &Context, alias: &str) -> miette::Result>; diff --git a/implementations/rust/ockam/ockam_api/src/nodes/service/tcp_inlets/node_manager_worker.rs b/implementations/rust/ockam/ockam_api/src/nodes/service/tcp_inlets/node_manager_worker.rs index 7d62f105830..8fc791288ee 100644 --- a/implementations/rust/ockam/ockam_api/src/nodes/service/tcp_inlets/node_manager_worker.rs +++ b/implementations/rust/ockam/ockam_api/src/nodes/service/tcp_inlets/node_manager_worker.rs @@ -4,7 +4,6 @@ use ockam::{route, Address, Result}; use ockam_core::api::{Error, Response}; use ockam_core::AllowAll; use ockam_node::Context; -use ockam_transport_core::HostnamePort; use ockam_transport_tcp::PortalInletInterceptor; use crate::http_auth::HttpAuthInterceptorFactory; @@ -35,13 +34,12 @@ impl NodeManagerWorker { secure_channel_identifier, enable_udp_puncture, disable_tcp_fallback, - http_upstream, + is_http_auth_inlet, } = create_inlet; - // PABLO - let prefix_route = if let Some(upstream) = http_upstream { + let prefix_route = if is_http_auth_inlet { let interceptor_address = self - .create_http_auth_interceptor(ctx, upstream, &alias) + .create_http_auth_interceptor(ctx, &alias) .await .map_err(|e| { Response::bad_request_no_request(&format!( @@ -80,14 +78,10 @@ impl NodeManagerWorker { async fn create_http_auth_interceptor( &self, ctx: &Context, - upstream: HostnamePort, inlet_alias: &String, ) -> Result { let token_refresher = TokenLeaseRefresher::new(ctx, self.node_manager.clone()).await?; - let http_interceptor_factory = Arc::new(HttpAuthInterceptorFactory::new( - token_refresher, - format!("{}:{}", upstream.hostname(), upstream.port()).to_string(), - )); + let http_interceptor_factory = Arc::new(HttpAuthInterceptorFactory::new(token_refresher)); let interceptor_address: Address = (inlet_alias.to_owned() + "_http_intercetor").into(); //TODO: incoming and outgoing access control? diff --git a/implementations/rust/ockam/ockam_app_lib/src/incoming_services/commands.rs b/implementations/rust/ockam/ockam_app_lib/src/incoming_services/commands.rs index 06d6b459e64..0281999f2b4 100644 --- a/implementations/rust/ockam/ockam_app_lib/src/incoming_services/commands.rs +++ b/implementations/rust/ockam/ockam_app_lib/src/incoming_services/commands.rs @@ -226,7 +226,7 @@ impl AppState { &None, false, false, - None, + false, ) .await .map_err(|err| { diff --git a/implementations/rust/ockam/ockam_command/src/http/inlet/create.rs b/implementations/rust/ockam/ockam_command/src/http/inlet/create.rs index 86094204b42..4d9d005c0cc 100644 --- a/implementations/rust/ockam/ockam_command/src/http/inlet/create.rs +++ b/implementations/rust/ockam/ockam_command/src/http/inlet/create.rs @@ -50,21 +50,6 @@ pub struct CreateCommand { #[arg(long, display_order = 900, id = "SOCKET_ADDRESS", hide_default_value = true, default_value_t = default_from_addr(), value_parser = hostname_parser)] pub from: HostnamePort, - /// Route to a TCP Outlet or the name of the TCP Outlet service you want to connect to. - /// - /// If you are connecting to a local node, you can provide the route as `/node/n/service/outlet`. - /// - /// If you are connecting to a remote node through a relay in the Orchestrator you can either - /// provide the full route to the TCP Outlet as `/project/myproject/service/forward_to_myrelay/secure/api/service/outlet`, - /// or just the name of the service as `outlet` or `/service/outlet`. - /// If you are passing just the service name, consider using `--via` to specify the - /// relay name (e.g. `ockam http-inlet create --to outlet --via myrelay`). - /// - /// If not provided connection to the backend server will be made directly from this node. For that, - /// a tcp outlet will be created. - #[arg(long, display_order = 900, id = "ROUTE")] - pub to: Option, - /// Name of the relay that this TCP Inlet will use to connect to the TCP Outlet. /// /// Use this flag when you are using `--to` to specify the service name of a TCP Outlet @@ -132,6 +117,28 @@ pub struct CreateCommand { )] pub no_tcp_fallback: bool, + #[clap(flatten)] + pub destination_group: DestinationGroup, +} + +#[derive(Debug, Clone, clap::Args)] +#[group(required = true, multiple = false)] +pub struct DestinationGroup { + /// Route to a TCP Outlet or the name of the TCP Outlet service you want to connect to. + /// + /// If you are connecting to a local node, you can provide the route as `/node/n/service/outlet`. + /// + /// If you are connecting to a remote node through a relay in the Orchestrator you can either + /// provide the full route to the TCP Outlet as `/project/myproject/service/forward_to_myrelay/secure/api/service/outlet`, + /// or just the name of the service as `outlet` or `/service/outlet`. + /// If you are passing just the service name, consider using `--via` to specify the + /// relay name (e.g. `ockam http-inlet create --to outlet --via myrelay`). + /// + /// If not provided connection to the backend server will be made directly from this node. For that, + /// a tcp outlet will be created. + #[arg(long, display_order = 900, id = "ROUTE")] + pub to: Option, + /// Upstream address #[arg( long, @@ -139,7 +146,7 @@ pub struct CreateCommand { id = "UPSTREAM_ADDRESS_URL", value_parser(NonEmptyStringValueParser::new()) )] - pub upstream_server: String, + pub upstream_server: Option, } pub(crate) fn default_from_addr() -> HostnamePort { @@ -164,44 +171,43 @@ impl Command for CreateCommand { cmd.timeout.timeout.map(|t| node.set_timeout_mut(t)); - let parsed_upstream = Url::parse(&cmd.upstream_server).into_diagnostic()?; - if parsed_upstream.scheme().ne("http") && parsed_upstream.scheme().ne("https") { - return Err(miette!( - "Invalid upstream schema {}, must be http or https", - cmd.upstream_server - ) - .into()); - } - if parsed_upstream.path().ne("/") { - return Err(miette!( - "Invalid upstream schema {}, must not contain a path", - cmd.upstream_server - ) - .into()); - } - if parsed_upstream.query().is_some() { - return Err(miette!( - "Invalid upstream schema {}, must not contain query fragment", - cmd.upstream_server - ) - .into()); - } - let upstream_host = parsed_upstream - .host_str() - .ok_or(miette!("could not read domain from upstream url"))?; - let upstream_port = parsed_upstream - .port_or_known_default() - .ok_or(miette!("could not read port from upstream url"))?; - let upstream_hostport = HostnamePort::new(upstream_host, upstream_port); - - let to = if let Some(t) = cmd.to() { - t - } else { + // It's either a 'upstream_server' argument, in that case an outlet is created implicitly here, + // or a 'to' argument, in which case we just point the inlet to that outlet + let to = if let Some(upstream) = &cmd.destination_group.upstream_server { + let parsed_upstream = Url::parse(upstream).into_diagnostic()?; + if parsed_upstream.scheme().ne("http") && parsed_upstream.scheme().ne("https") { + return Err(miette!( + "Invalid upstream schema {}, must be http or https", + upstream + ) + .into()); + } + if parsed_upstream.path().ne("/") { + return Err(miette!( + "Invalid upstream schema {}, must not contain a path", + upstream + ) + .into()); + } + if parsed_upstream.query().is_some() { + return Err(miette!( + "Invalid upstream schema {}, must not contain query fragment", + upstream + ) + .into()); + } + let upstream_host = parsed_upstream + .host_str() + .ok_or(miette!("could not read domain from upstream url"))?; + let upstream_port = parsed_upstream + .port_or_known_default() + .ok_or(miette!("could not read port from upstream url"))?; + let upstream_hostport = HostnamePort::new(upstream_host, upstream_port); let upstream_tls = parsed_upstream.scheme() == "https"; if let Some(pb) = &opts.terminal.progress_bar() { pb.set_message(format!( "Creating a new TCP Outlet to {}...\n", - color_primary(&cmd.upstream_server) + color_primary(upstream) )); } let outlet_status = node @@ -217,6 +223,8 @@ impl Command for CreateCommand { let mut multiaddr = outlet_status.worker_address().into_diagnostic()?; multiaddr.push_front(Secure::new("api")).unwrap(); multiaddr + } else { + cmd.to().unwrap() }; let tostr = to.to_string(); @@ -243,7 +251,7 @@ impl Command for CreateCommand { &cmd.secure_channel_identifier(&opts.state).await?, cmd.udp, cmd.no_tcp_fallback, - Some(upstream_hostport.clone()), + true, ) .await?; @@ -317,7 +325,10 @@ impl Command for CreateCommand { impl CreateCommand { fn to(&self) -> Option { - self.to.as_ref().map(|t| MultiAddr::from_str(t).unwrap()) + self.destination_group + .to + .as_ref() + .map(|t| MultiAddr::from_str(t).unwrap()) } async fn secure_channel_identifier( @@ -357,8 +368,9 @@ impl CreateCommand { .await .into_diagnostic()?; port_is_free_guard(&from)?; - if let Some(t) = self.to { - self.to = Some(Self::parse_arg_to(&opts.state, t, self.via.as_ref()).await?); + if let Some(t) = self.destination_group.to { + self.destination_group.to = + Some(Self::parse_arg_to(&opts.state, t, self.via.as_ref()).await?); if self .to() .unwrap() diff --git a/implementations/rust/ockam/ockam_command/src/tcp/inlet/create.rs b/implementations/rust/ockam/ockam_command/src/tcp/inlet/create.rs index 032f98bfa2f..2446a9a456e 100644 --- a/implementations/rust/ockam/ockam_command/src/tcp/inlet/create.rs +++ b/implementations/rust/ockam/ockam_command/src/tcp/inlet/create.rs @@ -174,7 +174,7 @@ impl Command for CreateCommand { &cmd.secure_channel_identifier(&opts.state).await?, cmd.udp, cmd.no_tcp_fallback, - None, + false, ) .await?;