Skip to content

Conversation

@shayne
Copy link
Contributor

@shayne shayne commented Feb 21, 2023

This change trims the mountPoint from the request URL path before
sending the request to the reverse proxy.

Today if you mount a proxy at /foo and request to
/foo/bar/baz, we leak the mountPoint /foo as part of the request
URL's path.

This fix makes removed the mountPoint prefix from the path so
proxied services receive requests as if they were running at the root
(/) path.

This could be an issue if the app generates URLs (in HTML or otherwise)
and assumes /path. In this case, those URLs will 404.

With that, I still think we should trim by default and not leak the
mountPoint (specific to Tailscale) into whatever app is hosted.
If it causes an issue with URL generation, I'd suggest looking at configuring
an app-specific path prefix or running Caddy as a more advanced
solution.

Fixes: #6571

Signed-off-by: Shayne Sweeney shayne@tailscale.com

@shayne shayne force-pushed the shayne/serve_request_path branch from 164bfed to fb5f1b7 Compare February 21, 2023 02:50
@shayne shayne requested review from bradfitz and maisem February 21, 2023 02:51
@shayne shayne self-assigned this Feb 21, 2023
@shayne shayne changed the title ipn/ipnlocal: [server] Trim mountPoint prefix from proxy path ipn/ipnlocal: [serve] Trim mountPoint prefix from proxy path Feb 21, 2023
@shayne shayne force-pushed the shayne/serve_request_path branch from fb5f1b7 to 418bbe8 Compare February 21, 2023 02:57
@shayne shayne force-pushed the shayne/serve_request_path branch from 418bbe8 to b53e770 Compare March 20, 2023 21:27
This change trims the mountPoint from the request URL path before
sending the request to the reverse proxy.

Today if you mount a proxy at `/foo` and request to
`/foo/bar/baz`, we leak the `mountPoint` `/foo` as part of the request
URL's path.

This fix makes removed the `mountPoint` prefix from the path so
proxied services receive requests as if they were running at the root
(`/`) path.

This could be an issue if the app generates URLs (in HTML or otherwise)
and assumes `/path`. In this case, those URLs will 404.

With that, I still think we should trim by default and not leak the
`mountPoint` (specific to Tailscale) into whatever app is hosted.
If it causes an issue with URL generation, I'd suggest looking at configuring
an app-specific path prefix or running Caddy as a more advanced
solution.

Fixes: #6571

Signed-off-by: Shayne Sweeney <shayne@tailscale.com>
@shayne shayne force-pushed the shayne/serve_request_path branch from b53e770 to 1da6a8b Compare March 20, 2023 21:29
@shayne shayne merged commit 7908b6d into main Mar 27, 2023
@shayne shayne deleted the shayne/serve_request_path branch March 27, 2023 14:11
shayne added a commit that referenced this pull request Mar 28, 2023
This change trims the mountPoint from the request URL path before
sending the request to the reverse proxy.

Today if you mount a proxy at `/foo` and request to
`/foo/bar/baz`, we leak the `mountPoint` `/foo` as part of the request
URL's path.

This fix makes removed the `mountPoint` prefix from the path so
proxied services receive requests as if they were running at the root
(`/`) path.

This could be an issue if the app generates URLs (in HTML or otherwise)
and assumes `/path`. In this case, those URLs will 404.

With that, I still think we should trim by default and not leak the
`mountPoint` (specific to Tailscale) into whatever app is hosted.
If it causes an issue with URL generation, I'd suggest looking at configuring
an app-specific path prefix or running Caddy as a more advanced
solution.

Fixes: #6571

Signed-off-by: Shayne Sweeney <shayne@tailscale.com>
darksip pushed a commit to darksip/tailscale that referenced this pull request Apr 3, 2023
…le#7334)

This change trims the mountPoint from the request URL path before
sending the request to the reverse proxy.

Today if you mount a proxy at `/foo` and request to
`/foo/bar/baz`, we leak the `mountPoint` `/foo` as part of the request
URL's path.

This fix makes removed the `mountPoint` prefix from the path so
proxied services receive requests as if they were running at the root
(`/`) path.

This could be an issue if the app generates URLs (in HTML or otherwise)
and assumes `/path`. In this case, those URLs will 404.

With that, I still think we should trim by default and not leak the
`mountPoint` (specific to Tailscale) into whatever app is hosted.
If it causes an issue with URL generation, I'd suggest looking at configuring
an app-specific path prefix or running Caddy as a more advanced
solution.

Fixes: tailscale#6571

Signed-off-by: Shayne Sweeney <shayne@tailscale.com>
@Multiply
Copy link

Multiply commented Apr 4, 2023

How do we get the previous functionality back? I'd prefer to keep forwarding the real path to my local service, when I use funnel.

@shayne
Copy link
Contributor Author

shayne commented Apr 4, 2023

Sorry, you're having trouble. It's a tricky decision, but we decided it's better not to leak the mount-point to the backend. Imagine the case where you mount the same app at two different mount points or if you're using a project that doesn't support the path prefix.

There are a couple of options. You can use / as your mount point. This setup will forward all paths to your reverse proxy.

You could also set up Caddy (or similar) and use our TCP forwarding features to have complete control over web requests.

@Multiply
Copy link

Multiply commented Apr 5, 2023

Could you add support for adding an optional path to the <source>?

That way I could do something similar to the following, and still support it:

tailscale serve https:443 / http://127.0.0.1:3000
tailscale serve https:443 /api/v1/graphql http://127.0.0.1:4000/api/v1/graphql

It's explicit that I want this behaviour for the second backend, as it's expecting that path to be hit.

Edit: Updated the first port, as I had copied the wrong line.

@shayne
Copy link
Contributor Author

shayne commented Apr 5, 2023

The second backend also listens on 127.0.0.1:4000, in which case the first serve command would satisfy all paths.

@Multiply
Copy link

Multiply commented Apr 5, 2023

My bad, I meant to write :3000 for the first one. Bad copypaste.

@shayne
Copy link
Contributor Author

shayne commented Apr 5, 2023

OK, thanks. I'll take a look at what it would look like and take it into consideration. Thanks for reaching out!

@Multiply
Copy link

Multiply commented Apr 5, 2023

Thanks a lot @shayne - It would be super beneficial to our team at least.

Do let me know if you want an alpha tester or 7! 🚀

@shayne
Copy link
Contributor Author

shayne commented Apr 5, 2023

Will do, In the meantime, definitely consider wiring up Caddy via:

$ tailscale serve tls-terminated-tcp:443 tcp://localhost:80

@Multiply
Copy link

Multiply commented Apr 5, 2023

Slightly off-topic, hope it's okay.

Do you know why my colleague can access his own funnel from his own machine (tls on port 443), and I can't access my own?
It worked at some point prior to rebooting my own machine, but no longer. Nothing binds to 443 on my machine, but IPNExtension does on his.

Edit: He's on an mac M1, and I'm on mac i9, if it matters.

@shayne
Copy link
Contributor Author

shayne commented Apr 5, 2023

I worked on this at #6930

macOS sandbox can be annoying. We should move specifics onto that issue. I'm curious if macOS versions are the same and if you can bind any other process to 443, even netcat.

@shayne
Copy link
Contributor Author

shayne commented Apr 5, 2023

@Multiply see #7743

@Multiply
Copy link

Multiply commented Apr 6, 2023

@shayne thanks for working on this so quickly! 🚀

Just waiting for my app store to show the update, and I'll try it out. ⌛

@shayne
Copy link
Contributor Author

shayne commented Apr 6, 2023

Sure thing! We also have builds available on our packages server:
https://pkgs.tailscale.com/stable/#macos

@larryqiann
Copy link

Sorry to reopen this, but I'm wondering whether this works the way I think it is working.

I have a service running at localhost:3080. When I do sudo tailscale serve --bg --set-path /path 3080, I get 404 errors when I go to my-tailnet.ts.net/path, but when i do sudo tailscale serve --bg 3080 and go to my-tailnet.ts.net, everything works fine.

My understanding is that after this change, the /path part is removed silently from the URL when being passed to the app. Is this a misunderstanding of how this works? How can I achieve this functionality?

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FR: Support stripping HTTP request paths from funnel proxy routes

4 participants