Valid upgrade path detection requires dual /

[Ongy]

Describe the bug

When we setup a reverse proxy with a subpath and remove the prefix on the server side, the wstunnel server portion gets confused because it get's the path /events, which is not valid.

After looking at the code a bit, I found https://github.com/erebe/wstunnel/blob/18db0a73c0e09846fd9cdd7ccad2f07121f5ce2e/src/tunnel/transport/websocket.rs#L238 which compositos the path //events when there is no prefix set.

This allowed the implementation, to rely on having at least 2 slashes in the path, even if they are only 1 path separator. https://github.com/erebe/wstunnel/blob/18db0a73c0e09846fd9cdd7ccad2f07121f5ce2e/src/tunnel/server/utils.rs#L71 checks for a slash past the first byte.

To Reproduce

Run wstunnel server side behind a reverse proxy that strips the path. E.g. we run different versions (a 9.x and a 10.x for different clients) and provide -P /wstunnel and -P /wstunnel-10 to the respective clients. The result is //wstunnel/events on the server. The reverse proxy rewrites this to /events by dropping /wstunnel, as path segment.

Expected behavior

The server component should allow clients that reach it with /events. Since it is equivalent to //events as path.

Your wstunnel setup

Paste your logs of wstunnel, started with --log-lvl=DEBUG, and with the command line used

• client wss://... --http-upgrade-path-prefix /{{.Project}}/aqualine/ --local-to-remote http://0.0.0.0:1212 --connection-min-idle "10" --http-headers-file /shared/header
• server ws:0.0.0.0:8080

Desktop (please complete the following information):

• OS: [e.g. iOS]
• Version [e.g. 22]

Additional context Getting it started with the debug flags is a bit hard in our setup, but I hope pointing to the code helps here. I can likely provide a PR to fix this myself soon™

I think the client should retain the doubling behaviour, to not break old systems, but the server should be a bit more permissive/not require an empty path segment, but allow path normalization along the way.

[erebe]

Hello,

By default, if the user does not specify anything, the path prefix is v1 so /v1/events.

  -P, --http-upgrade-path-prefix <HTTP_UPGRADE_PATH_PREFIX>
          Use a specific prefix that will show up in the http path during the upgrade request.
          Useful if you need to route requests server side but don't have vhosts
          When using mTLS this option overrides the default behavior of using the common name of the
          client's certificate. This will likely result in the wstunnel server rejecting the connection.

          [env: WSTUNNEL_HTTP_UPGRADE_PATH_PREFIX=]
------->  [default: v1]     <----------

So you only need your reverse proxy to rewrite the url from //events to /whatever/events to solve your issue. There just must be always a path before the /events.

Out of curiosity, what kind of service are you providing to your clients that you are in need of wstunnel ?

[Ongy]

Ahh, I tried to find it in code and thought it was empty. Ok, that explains part of it.

Yea, we currently have <identifier>/aqualine as prefix, and use that identifier as hint for authentication. Aqualine being an internal codename for the feature. I.e. I consider the entire prefix in the "domain" of the reverse proxy.

This leads to a request on /project/aqualine/events that gets rewritten to /events. The workaround is to set the prefix with this in mind, so it ends on a /, which leads to /project/aqualine//events => //events which is odd.

Your pointer to the default being v1 makes sense. But I prefer my mental model over yours :o

We are using it to tunnel grpc(http2) connections through middleboxes we don't control, which do not support anything newer than http1.

[Ongy]

My mental model being, I think the entire specified prefix should be possible to strip.

Though judging from other features, it's partially intended to be used in tandem with the prefix filter on the frontend to use it as symmetric secret.

yea, if you don't like the proposed change, I can just do a different rewrite on the server.

[erebe]

indeed it is supposed to work as a shared secret between client and server, so it is mandatory when restriction is on.

I will go with letting you change the rewrite rule of your reverse-proxy 😉

[erebe]

In anycase, thank you for trying to debug your issue and understand the codebase 🙏