Headscale: Added an option to set an Access-Control-Allow-Origin resp… #2302

Jisse-Meruma · 2024-12-16T11:07:18Z

add a header to enable Cross-Origin Resource Sharing (CORS) #2301

have read the CONTRIBUTING.md file
raised a GitHub issue or discussed it on the projects chat beforehand
added unit tests
added integration tests
updated documentation if needed
updated CHANGELOG.md

kradalby · 2024-12-18T15:40:38Z

http_Headscale_tutorial.md

@@ -0,0 +1,99 @@
+# Setting Up a Simple Headscale Control Server Using HTTP


I dont think this is the place for this documentation, and we have our own documentation. This looks like internal notes.

You are correct this has been reverted

kradalby · 2024-12-18T15:41:36Z

config-example.yaml

+# - "*" to allow access from any origin (not recommended for sensitive data).
+# - "http://example.com" to only allow access from a specific origin.
+# - "" to disable Cross-Origin Resource Sharing (CORS).
+Access-Control-Allow-Origin: ""


This does not follow the format of the config file and has not been discussed. This isnt really anything we need and people needing this can handle it in Nginx and the likes.

@kradalby

I have modified the code to align with the requested configuration file format.

I did open an accompanying issue for discussion: #2301. I also see mentions of a project chat, could you point me to it?

The idea of this feature is to simplify the Headscale server setup and have everything in one place. Tailscale continues to grow in the browser sector; for example, they recently released their new SSH client.

Our company has just released WebVM 2.0, which is a virtual machine inside the browser. We use this product with Tailscale to establish fast peer-to-peer connections between machines.

With this small change, the Headscale setup for the browser would be much easier and much simpler, especially in combination with your integrated TLS support.

This feature has already been requested in two other issues but without success. Here are their references: #623 #1283 .

This does not follow the format of the config file and has not been discussed. This isnt really anything we need and people needing this can handle it in Nginx and the likes.

FYI expecting people to rely on a reverse proxy to intercept requests and inject CORS is introducing a large amount of user-side complexity that doesn't need to be there. I (as the maintainer of headscale ui) get people confused about this pretty much twice a month and opening issues as a result, even when the troubleshooting steps are listed (twice) on the front page readme.

A simple UI_DOMAIN configuration that optionally sets the appropriate CORS information would solve a lot of issues. I would not suggest allowing * anyway since browsers are planning to block that syntax.

A simple UI_DOMAIN configuration that optionally sets the appropriate CORS information would solve a lot of issues. I would not suggest allowing * anyway since browsers are planning to block that syntax.

We do want to keep the flexibility of using the * available, but if indeed browsers are planning to block this syntax, I will make sure to update the feature to check for this syntax. To avoid any confusion, I would prefer to stick with the normal CORS syntax. This * is indeed another security risk, but this flexibility is also possible by using a reverse proxy. However, if Headscale agrees to not allow *, then I would gladly make another commit with a fix.

Jisse-Meruma · 2025-01-27T14:08:22Z

Hi, I would like to know if there is a possibility to further discuss this feature?

kradalby

I posted some comments, I do not know enough about this topic other than how little I know and that I never do it correctly. I am uncomfortable to add something that has security implications that I do not have the knowledge to say if correct, or will be a problem/need change in the future.

This has been done a lot of times before, and it has currently led to a maintenance burden that is so large that I dont really have time to spend time on PRs like this where I need to figure out what is the correct way (which then will potentially have the recursive effect of having me do this again later).

My main thing is not to be negative, I am not saying no, I am just saying that all of this things needs thought, and I have a lot of other inflight things atm.

kradalby · 2025-01-27T14:43:35Z

config-example.yaml

+# - "*" to allow access from any origin (not recommended for sensitive data).
+# - "http://example.com" to only allow access from a specific origin.
+# - "" to disable Cross-Origin Resource Sharing (CORS).
+access_control_allow_origin: ""


I do not like this being a global setting, maybe it should be under api, or grpc or something.

Hi @kradalby,

Yes I suggest to have an structure like this:

api: cors_allow_origins: [] # as a list

or

cors: allow_origins: [] # as a list

I would value your input on which approach you prefer.

kradalby · 2025-01-27T14:43:57Z

hscontrol/app.go

 func (h *Headscale) createRouter(grpcMux *grpcRuntime.ServeMux) *mux.Router {
 	router := mux.NewRouter()
 	router.Use(prometheusMiddleware)

+	if h.cfg.AccessControlAllowOrigins != "" {
+		router.Use(h.corsHeadersMiddleware)


I do not think this should be added for all endpoints, it should only be needed for the API?

Hi @kradalby,

Thank you for your feedback. I understand your concern about applying this to all endpoints.

I agree that not all endpoints require CORS headers. However, the Tailscale Client does use several non-API routes — specifically /register, /key, /machine, /derp, /verify and /health — to communicate with the Headscale server via HTTP requests. For these endpoints, implementing CORS headers is necessary to allow proper functionality when CORS is enabled. Additionally, the Tailscale SSH web client operates for example in a similar manner.

At the same time, I understand that certain endpoints such as /apple, /windows, and /swagger do not need CORS headers.

I will give you a code draft of our idea on how to implement this in a simple matter.

If you have any questions feel free to ask me for any clarification if needed.

kradalby · 2025-01-27T14:45:03Z

config-example.yaml

@@ -40,6 +40,13 @@ grpc_listen_addr: 127.0.0.1:50443
 # are doing.
 grpc_allow_insecure: false

+# The Access-Control-Allow-Origin header specifies which origins are allowed to access resources.
+# Options:
+# - "*" to allow access from any origin (not recommended for sensitive data).


I dont really like giving people footguns, so I do not think we should allow *, and force people to do domains explicitly.

If it can be a list of domains, it should be a list data structure or something similar.

Hi @kradalby,

Thank you for your input. I completely understand the concern about providing a wildcard * and the importance of specifying domains explicitly.

We can certainly disable the use of * for domain whitelisting and enforce explicit domain declarations. Our initial idea was to implement an "Allow list" where users can specify any allowed domain.

As recommended by MDN: If the server supports clients from multiple origins, it must return the origin for the specific client making the request. Additionally, MDN advises adding the Vary: Origin header to inform clients that the response varies based on the Origin header.

Further information and recommendations can be found https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Origin.

I will show you a code draft to implement this feature soon. Feel free to give any feedback on the idea.

…onse header to enable Cross-Origin Resource Sharing (CORS)

… enable cors for specific endpoints (#2)

Jisse-Meruma · 2025-02-05T14:17:02Z

Hi @kradalby,

I hope this message finds you well.

As per your recent comments, I have made some changes to the code. I'll wait once you have the time available to review the updated code.

Jisse-Meruma · 2025-02-28T09:42:47Z

Hi @kradalby, Would it be possible to continue the discussion of this PR?

kradalby · 2025-02-28T12:25:49Z

Sorry, I have been swamped, I've read up a bit this morning and this is my thoughts:

I think these headers should be limited to the api, that covers the main use case of allowing CORS for the different WebUIs. I do not think we should add it to all the other endpoints, that is quite an exotic use case and while I agree with @routerino that it is bad UX to ask people using the API and WebUI to setup a proxy to have CORS, I do think that this is exotic enough to say that you need to do it in a proxy.

I do not really want to support *, so setting a domain would be required. From what I understand from the spec, you can only have a single origin, not a list, so we should allow the list of a single domain (I might be wrong about this one, please enlighten me).
E.g. headscale server is: myheadscale.net and ui is: admin.myheadscale.net

Jisse-Meruma requested review from ohdearaugustin, nblock, juanfont and kradalby as code owners December 16, 2024 11:07

kradalby reviewed Dec 18, 2024

View reviewed changes

Jisse-Meruma force-pushed the enable-cors-with-config branch 2 times, most recently from e07c42b to dfad6a1 Compare December 18, 2024 16:27

Jisse-Meruma requested a review from kradalby December 24, 2024 12:16

kradalby reviewed Jan 27, 2025

View reviewed changes

Headscale: Added an option to set an Access-Control-Allow-Origin resp…

68ce697

…onse header to enable Cross-Origin Resource Sharing (CORS)

Jisse-Meruma force-pushed the enable-cors-with-config branch from dfad6a1 to 68ce697 Compare February 5, 2025 11:08

Block ability to use '*'. replace origins string to a list to config.…

cbf6a43

… enable cors for specific endpoints (#2)

Jisse-Meruma requested a review from kradalby February 5, 2025 14:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Headscale: Added an option to set an Access-Control-Allow-Origin resp… #2302

Headscale: Added an option to set an Access-Control-Allow-Origin resp… #2302

Jisse-Meruma commented Dec 16, 2024

kradalby Dec 18, 2024

Jisse-Meruma Dec 18, 2024 •

edited

Loading

kradalby Dec 18, 2024

Jisse-Meruma Dec 19, 2024 •

edited

Loading

routerino Dec 23, 2024 •

edited

Loading

Jisse-Meruma Dec 24, 2024

Jisse-Meruma commented Jan 27, 2025

kradalby left a comment

kradalby Jan 27, 2025

Jisse-Meruma Jan 28, 2025

kradalby Jan 27, 2025

Jisse-Meruma Jan 28, 2025 •

edited

Loading

kradalby Jan 27, 2025

Jisse-Meruma Jan 28, 2025

Jisse-Meruma commented Feb 5, 2025

Jisse-Meruma commented Feb 28, 2025

kradalby commented Feb 28, 2025

		@@ -0,0 +1,99 @@
		# Setting Up a Simple Headscale Control Server Using HTTP

Headscale: Added an option to set an Access-Control-Allow-Origin resp… #2302

Are you sure you want to change the base?

Headscale: Added an option to set an Access-Control-Allow-Origin resp… #2302

Conversation

Jisse-Meruma commented Dec 16, 2024

Choose a reason for hiding this comment

Jisse-Meruma Dec 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jisse-Meruma Dec 19, 2024 • edited Loading

Choose a reason for hiding this comment

routerino Dec 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jisse-Meruma commented Jan 27, 2025

kradalby left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jisse-Meruma Jan 28, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jisse-Meruma commented Feb 5, 2025

Jisse-Meruma commented Feb 28, 2025

kradalby commented Feb 28, 2025

Jisse-Meruma Dec 18, 2024 •

edited

Loading

Jisse-Meruma Dec 19, 2024 •

edited

Loading

routerino Dec 23, 2024 •

edited

Loading

Jisse-Meruma Jan 28, 2025 •

edited

Loading