headscale icon indicating copy to clipboard operation
headscale copied to clipboard

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

Open Jisse-Meruma opened this issue 1 year ago • 7 comments

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

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

Jisse-Meruma avatar Dec 16 '24 11:12 Jisse-Meruma

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

Jisse-Meruma avatar Jan 27 '25 14:01 Jisse-Meruma

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 avatar Feb 05 '25 14:02 Jisse-Meruma

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

Jisse-Meruma avatar Feb 28 '25 09:02 Jisse-Meruma

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

kradalby avatar Feb 28 '25 12:02 kradalby

Hi @kradalby,

As of writing, the current commit no longer supports the use of * for the Access-Control-Allow-Origin header, as it could pose a security risk. You can now only set a specific origin. However, considering that multiple clients might connect to the server, I have implemented the flexibility to specify multiple origins.

The code loops through the list of allowed origins and checks if the request origin is in the list. If it is, the corresponding header is added to enable Cross-Origin Resource Sharing (CORS) for that specific origin. This technique is a standard approach when dealing with a list of Allowed origins, as documented here: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Origin.

I understand your concerns about enabling CORS on sensitive routes and the potential security risks involved. Given that we no longer allow the use of * for the Access-Control-Allow-Origin header, administrators can now specify only particular URLs to permit connections to Headscale via the web.

Even if a rouge website was allowed to communicate with Headscale through the web, it would still require to go to the mandatory security layers to fully register a node. Additionally, Tailscale is beginning to implement web clients that can already connect to the official control server with CORS enabled.

To add an extra layer of security, we can consider allowing CORS on sensitive endpoints only for origins that use the HTTPS protocol. This ensures that the data is transmitted securely and reduces the risk associated with allowing CORS on less secure origins.

However, if this approach is not preferred, I am more than willing to make adjustments to enable CORS only for the WEBUI API.

Thank you for your understanding and please let me know if you have any further suggestions or concerns. Best regards, Jisse Meruma

Jisse-Meruma avatar Mar 04 '25 10:03 Jisse-Meruma

Hi @kradalby, I know you are quite busy. But still I would like to know if there is a change we could move this forward. I would like to know your opinion of my latest suggestion so that I can make a code draft for you.

Jisse-Meruma avatar Mar 24 '25 10:03 Jisse-Meruma

I agree with @Jisse-Meruma that this is a sensible feature to add.

This PR seems to be ready, or very close to ready barring a final code review. Is there anything we can do to help @kradalby?

scarytom avatar Apr 24 '25 14:04 scarytom