google / martian

Martian is a library for building custom HTTP/S proxies
Apache License 2.0
2k stars 247 forks source link

proxy: provide support for WebSockets with MITM #31

Open admtnnr opened 9 years ago

admtnnr commented 9 years ago

Some prelimary investigation reveals several problems with supporting WebSockets, both secure and insecure. When using a proxy, browsers will send a CONNECT request to the destination when attempting to open a WebSocket, regardless of whether it is secure (ws:// or wss://).

Martian assumes CONNECT requests are requests to upgrade to TLS for HTTPS. In the non MITM case, Martian will blindly make a TCP connection by default for CONNECT requests which means that both secure and insecure WebSocket requests work.

Everything starts to break when MITM is enabled. Here's how:

To start, we'll look at an example request that is made to the proxy when attempting to open a WebSocket to echo.websocket.org.

CONNECT echo.websocket.org:80 HTTP/1.1
Host: echo.websocket.org:80
Proxy-Connection: keep-alive
Content-Length: 0
User-Agent: ...

Insecure WebSockets with MITM

Martian will wrap the connection with a TLS connection after sending a 200 OK response to the client. In the case of an insecure WebSocket request, the connection will then have a normal HTTP request sent to it for the WebSocket handshake. This fails with a TLS handshake error: tls: first record does not look like a TLS handshake.

How to Fix

This is the tricky case. We actually need to inspect the first couple bytes of traffic from the connection to be able to make a guess whether the connection is TLS or something else. The current idea is to inspect the bytes for:

GET https://echo.websocket.org/?encoding=text HTTP/1.1
Host: echo.websocket.org
Upgrade: websocket
Connection: Upgrade
Content-Length: 0
Origin: http://www.websocket.org
Sec-Websocket-Extensions: permessage-deflate
Sec-Websocket-Key: 5/02dRzDuskoI7ZWhnvL1w==
Sec-Websocket-Version: 13

The TLS handshake succeeds in the secure WebSockets case, but fails with the following response to the prior request. This is because Martian will remove the Upgrade header (because it is listed in the Connection header) and thus prevent the downstream server from recognizing it as a WebSocket request.

HTTP/1.1 400 WebSocket Upgrade Failure
Content-Length: 77
Access-Control-Allow-Credentials: true
Access-Control-Allow-Headers: content-type
Access-Control-Allow-Headers: authorization
Access-Control-Allow-Headers: x-websocket-extensions
Access-Control-Allow-Headers: x-websocket-version
Access-Control-Allow-Headers: x-websocket-protocol
Access-Control-Allow-Origin: http://www.websocket.org
Content-Type: text/html
Date: Tue, 24 Nov 2015 21:59:21 GMT

How to Fix

I expect this case will be slightly easier to handle with a custom modifier that checks for the Upgrade header, and if it finds websocket specified, will issue context.Hijack() and stitch the connections together.

context.Hijack()

The idea is that session.Context provides a Hijack() method similar to ResponseWriter.Hijack() that will return net.Conn, bufio.ReadWriter, error.

This will allow a modifier aware of WebSockets to take control of the connection while allowing modifiers before it to run.

A sub modification system (such as modifiers specifically designed to manipulate Websocket messages) could be created to further enhance the system.

To deal with the default case that is currently broken, we can create a modifier that will by default take any request with WebSocket headers and hijack the connection and stitch the two connections together reusing behavior similar to the CONNECT tunnel, send a 101 Switching Protocols response, and then waiting until the connection is closed.

berkant commented 6 years ago

It would be perfect if this were implemented.

trickpattyFH20 commented 5 years ago

@bramhaghosh @admtnnr Is this feature currently being worked on?

bramhaghosh commented 5 years ago

It is not, and it's not likely to be soon. It's very low priority.

On Tue, Dec 11, 2018, 9:51 AM Patrick <notifications@github.com wrote:

@bramhaghosh https://github.com/bramhaghosh @admtnnr https://github.com/admtnnr Is this feature currently being worked on?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/google/martian/issues/31#issuecomment-446296833, or mute the thread https://github.com/notifications/unsubscribe-auth/AADI-VOSGvhPGaoPspLctz0qzBud3_cRks5u3_CvgaJpZM4FxZUr .

trickpattyFH20 commented 5 years ago

I'm following the "how to fix" section described above, and helping to work on a custom modifier with @fcjr. The upgrade header is checked for, and then the connection hijacked, but I'm having trouble with the "stitch the connections together" part

I think it should be something like make anet.dial to the original req.url, but after that I'm unclear how to read and write data between the original connection and the new connection.

Does anyone have any tips on how to achieve this? @admtnnr @bramhaghosh

vtolstov commented 4 years ago

gentle ping on this issue, ow to workaround absent websocket wss support with mitm proxy

nobody5050 commented 3 years ago

going to bump this

bramhaghosh commented 3 years ago

This is a low priority issue. However, I'm happy to review and merge any pull-requests that add websockets mitm.

On Sat, Oct 10, 2020 at 12:50 PM nobody5050 notifications@github.com wrote:

going to bump this

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/google/martian/issues/31#issuecomment-706578188, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAMR6KOMOJDGVQIOV7YT5DSKCGGJANCNFSM4BOFSUVQ .

LukasPukenis commented 3 years ago

@trickpattyFH20 @fcjr did you work out a solution?

berkant commented 3 years ago

Hello! Does Martian support MITM'ing WebSockets connections yet?

bramhaghosh commented 3 years ago

No it does not

On Mon, Feb 1, 2021, 7:21 AM Berkant Ipek notifications@github.com wrote:

Hello! Does Martian support MITM'ing WebSockets connections yet?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/google/martian/issues/31#issuecomment-770816884, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAMR6KDCFGKVYT37ARLCHTS42MFPANCNFSM4BOFSUVQ .

abdullah-lt commented 3 years ago

Does anyone have a workaround to support WebSocket with MITM

mmatczuk commented 1 year ago

If anyone is interested I just submitted a PR to our fork implementing protocol upgrades. This enables running WS with MITM.

https://github.com/saucelabs/martian/pull/22