-
Notifications
You must be signed in to change notification settings - Fork 2.2k
ipn/ipnlocal: [serve] Trim mountPoint prefix from proxy path #7334
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
164bfed to
fb5f1b7
Compare
fb5f1b7 to
418bbe8
Compare
418bbe8 to
b53e770
Compare
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>
b53e770 to
1da6a8b
Compare
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>
…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>
|
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. |
|
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 You could also set up Caddy (or similar) and use our TCP forwarding features to have complete control over web requests. |
|
Could you add support for adding an optional path to the That way I could do something similar to the following, and still support it: 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. |
|
The second backend also listens on |
|
My bad, I meant to write |
|
OK, thanks. I'll take a look at what it would look like and take it into consideration. Thanks for reaching out! |
|
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! 🚀 |
|
Will do, In the meantime, definitely consider wiring up Caddy via: $ tailscale serve tls-terminated-tcp:443 tcp://localhost:80 |
|
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? Edit: He's on an mac M1, and I'm on mac i9, if it matters. |
|
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 thanks for working on this so quickly! 🚀 Just waiting for my app store to show the update, and I'll try it out. ⌛ |
|
Sure thing! We also have builds available on our packages server: |
|
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! |
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
/fooand request to/foo/bar/baz, we leak themountPoint/fooas part of the requestURL's path.
This fix makes removed the
mountPointprefix from the path soproxied 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