Support HTTP Upgrade request for stealer. #928
Assignees
Labels
enhancement
New feature or request
Comments
bors bot
pushed a commit
that referenced
this issue
Jan 22, 2023
…led. (#973) Issue: #928 Depends on [test-images #14](metalbear-co/test-images#14). Implements a pass-through mechanism to handle HTTP/1 upgrade requests from a client. ## Motivation Currently, if you run mirrord with steal traffic, and HTTP filter enabled, we end up not serving HTTP upgrade requests appropriately. Our implementation of [`Service`](https://docs.rs/hyper/1.0.0-rc.2/hyper/service/trait.Service.html) would either: - Forward the HTTP request to the local app (if a filter matched), send back a response and **not** upgrade the connection, or; - Pass the request to its original destination, get a response back from it, and, again, **not** upgrade the connection. This would prevent the user from handling websockets (our main motivator behind this feature), that start up as HTTP requests. ## Implementation We're manually handling connection upgrades, by first checking if an HTTP request contains the "Upgrade" header, and if that's the case, then we: - Current `unmacthed_request` flow: 1. Create a connection to the original destination (let's call this the `agent_original` connection); 2. Forward the HTTP request to it, awaiting its response; 3. Send the response back through our hyper connection (the `agent_client` connection we handle in hyper); - The new steps: 4. Send the `agent_original` connection to the `filter_task` (through the `upgrade_tx` channel); 5. Retrieve the `agent_client` connection from hyper, as a normal `TcpStream`; 6. Copy whatever bytes were leftover from these connections (bytes that were read, but not processed by hyper); 7. Use [`copy_bidirectional`](https://docs.rs/tokio/latest/tokio/io/fn.copy_bidirectional.html) to keep copying from one connection to the other. ### Tests The feature is being tested by `test_websocket_upgrade`, which is very similar to the `test_complete_passthrough` test. The main difference is that we use [`tokio-tugstenite`](https://docs.rs/tokio-tungstenite/latest/tokio_tungstenite/) to create a websocket connection (instead of a normal `TcpStream`). ## Alternatives ### Give a [`DuplexStream`](https://docs.rs/tokio/latest/tokio/io/struct.DuplexStream.html) to hyper We could feed hyper a `DuplexStream` rather than the actual `TcpStream` connection, and deal with the upgrade ourselves, by parsing the bytes into an `Header`, checking for the "Upgrade: " header, or even use the [`Response`] as a way to identify that this was an upgrade request. I've explored this approach, but this seemed a bit too much, especially when @t4lz asked me about "why not use the `unmatched_request`?" ### Use [`hyper::upgrade::on`] hyper has a proper handler for dealing with HTTP upgrade requests, where you give it a [`Request`](https://docs.rs/hyper/1.0.0-rc.2/hyper/struct.Request.html) that contains an [`OnUpgrade`](https://docs.rs/hyper/1.0.0-rc.2/hyper/upgrade/struct.OnUpgrade.html) extension, and it'll give you back its connection (`agent_client`) and the leftover bytes. I could not make this approach work, as we need to give the request to [`SendRequest`](https://docs.rs/hyper/1.0.0-rc.2/hyper/client/conn/http1/struct.SendRequest.html) by value, making it impossible to also feed it to the `upgrade::on` function. This was the case because we need to poll the connection (`agent_original`) in a separate task, thus requiring us to also move the request into it. The request cannot be copied/cloned, as it contains a `Receiver` channel for the `OnUpgrade` feature, and if you try to manually copy it, the upgrade is never triggered, because we're dealing with different channels. Co-authored-by: meowjesty <[email protected]>
bors bot
pushed a commit
that referenced
this issue
Jan 23, 2023
…led. (#973) Issue: #928 Depends on [test-images #14](metalbear-co/test-images#14). Implements a pass-through mechanism to handle HTTP/1 upgrade requests from a client. ## Motivation Currently, if you run mirrord with steal traffic, and HTTP filter enabled, we end up not serving HTTP upgrade requests appropriately. Our implementation of [`Service`](https://docs.rs/hyper/1.0.0-rc.2/hyper/service/trait.Service.html) would either: - Forward the HTTP request to the local app (if a filter matched), send back a response and **not** upgrade the connection, or; - Pass the request to its original destination, get a response back from it, and, again, **not** upgrade the connection. This would prevent the user from handling websockets (our main motivator behind this feature), that start up as HTTP requests. ## Implementation We're manually handling connection upgrades, by first checking if an HTTP request contains the "Upgrade" header, and if that's the case, then we: - Current `unmacthed_request` flow: 1. Create a connection to the original destination (let's call this the `agent_original` connection); 2. Forward the HTTP request to it, awaiting its response; 3. Send the response back through our hyper connection (the `agent_client` connection we handle in hyper); - The new steps: 4. Send the `agent_original` connection to the `filter_task` (through the `upgrade_tx` channel); 5. Retrieve the `agent_client` connection from hyper, as a normal `TcpStream`; 6. Copy whatever bytes were leftover from these connections (bytes that were read, but not processed by hyper); 7. Use [`copy_bidirectional`](https://docs.rs/tokio/latest/tokio/io/fn.copy_bidirectional.html) to keep copying from one connection to the other. ### Tests The feature is being tested by `test_websocket_upgrade`, which is very similar to the `test_complete_passthrough` test. The main difference is that we use [`tokio-tugstenite`](https://docs.rs/tokio-tungstenite/latest/tokio_tungstenite/) to create a websocket connection (instead of a normal `TcpStream`). ## Alternatives ### Give a [`DuplexStream`](https://docs.rs/tokio/latest/tokio/io/struct.DuplexStream.html) to hyper We could feed hyper a `DuplexStream` rather than the actual `TcpStream` connection, and deal with the upgrade ourselves, by parsing the bytes into an `Header`, checking for the "Upgrade: " header, or even use the [`Response`] as a way to identify that this was an upgrade request. I've explored this approach, but this seemed a bit too much, especially when @t4lz asked me about "why not use the `unmatched_request`?" ### Use [`hyper::upgrade::on`] hyper has a proper handler for dealing with HTTP upgrade requests, where you give it a [`Request`](https://docs.rs/hyper/1.0.0-rc.2/hyper/struct.Request.html) that contains an [`OnUpgrade`](https://docs.rs/hyper/1.0.0-rc.2/hyper/upgrade/struct.OnUpgrade.html) extension, and it'll give you back its connection (`agent_client`) and the leftover bytes. I could not make this approach work, as we need to give the request to [`SendRequest`](https://docs.rs/hyper/1.0.0-rc.2/hyper/client/conn/http1/struct.SendRequest.html) by value, making it impossible to also feed it to the `upgrade::on` function. This was the case because we need to poll the connection (`agent_original`) in a separate task, thus requiring us to also move the request into it. The request cannot be copied/cloned, as it contains a `Receiver` channel for the `OnUpgrade` feature, and if you try to manually copy it, the upgrade is never triggered, because we're dealing with different channels. Co-authored-by: meowjesty <[email protected]>
bors bot
pushed a commit
that referenced
this issue
Jan 23, 2023
…led. (#973) Issue: #928 Depends on [test-images #14](metalbear-co/test-images#14). Implements a pass-through mechanism to handle HTTP/1 upgrade requests from a client. ## Motivation Currently, if you run mirrord with steal traffic, and HTTP filter enabled, we end up not serving HTTP upgrade requests appropriately. Our implementation of [`Service`](https://docs.rs/hyper/1.0.0-rc.2/hyper/service/trait.Service.html) would either: - Forward the HTTP request to the local app (if a filter matched), send back a response and **not** upgrade the connection, or; - Pass the request to its original destination, get a response back from it, and, again, **not** upgrade the connection. This would prevent the user from handling websockets (our main motivator behind this feature), that start up as HTTP requests. ## Implementation We're manually handling connection upgrades, by first checking if an HTTP request contains the "Upgrade" header, and if that's the case, then we: - Current `unmacthed_request` flow: 1. Create a connection to the original destination (let's call this the `agent_original` connection); 2. Forward the HTTP request to it, awaiting its response; 3. Send the response back through our hyper connection (the `agent_client` connection we handle in hyper); - The new steps: 4. Send the `agent_original` connection to the `filter_task` (through the `upgrade_tx` channel); 5. Retrieve the `agent_client` connection from hyper, as a normal `TcpStream`; 6. Copy whatever bytes were leftover from these connections (bytes that were read, but not processed by hyper); 7. Use [`copy_bidirectional`](https://docs.rs/tokio/latest/tokio/io/fn.copy_bidirectional.html) to keep copying from one connection to the other. ### Tests The feature is being tested by `test_websocket_upgrade`, which is very similar to the `test_complete_passthrough` test. The main difference is that we use [`tokio-tugstenite`](https://docs.rs/tokio-tungstenite/latest/tokio_tungstenite/) to create a websocket connection (instead of a normal `TcpStream`). ## Alternatives ### Give a [`DuplexStream`](https://docs.rs/tokio/latest/tokio/io/struct.DuplexStream.html) to hyper We could feed hyper a `DuplexStream` rather than the actual `TcpStream` connection, and deal with the upgrade ourselves, by parsing the bytes into an `Header`, checking for the "Upgrade: " header, or even use the [`Response`] as a way to identify that this was an upgrade request. I've explored this approach, but this seemed a bit too much, especially when @t4lz asked me about "why not use the `unmatched_request`?" ### Use [`hyper::upgrade::on`] hyper has a proper handler for dealing with HTTP upgrade requests, where you give it a [`Request`](https://docs.rs/hyper/1.0.0-rc.2/hyper/struct.Request.html) that contains an [`OnUpgrade`](https://docs.rs/hyper/1.0.0-rc.2/hyper/upgrade/struct.OnUpgrade.html) extension, and it'll give you back its connection (`agent_client`) and the leftover bytes. I could not make this approach work, as we need to give the request to [`SendRequest`](https://docs.rs/hyper/1.0.0-rc.2/hyper/client/conn/http1/struct.SendRequest.html) by value, making it impossible to also feed it to the `upgrade::on` function. This was the case because we need to poll the connection (`agent_original`) in a separate task, thus requiring us to also move the request into it. The request cannot be copied/cloned, as it contains a `Receiver` channel for the `OnUpgrade` feature, and if you try to manually copy it, the upgrade is never triggered, because we're dealing with different channels. Co-authored-by: meowjesty <[email protected]>
|
Should be working as of #973 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The HTTP traffic filter doesn't currently support handling of
Connection: upgraderequests.We need this mostly for
Upgrade: websocket.The text was updated successfully, but these errors were encountered: