Skip to content
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

Content-Security-Policy #305

Open
bamboleeeero-bamboleeeero opened this issue Nov 29, 2016 · 40 comments
Open

Content-Security-Policy #305

bamboleeeero-bamboleeeero opened this issue Nov 29, 2016 · 40 comments
Labels
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented topic/ui Change the appearance of the Gitea UI type/enhancement An improvement of existing functionality

Comments

@bamboleeeero-bamboleeeero
Copy link

bamboleeeero-bamboleeeero commented Nov 29, 2016

There seems to be a bunch of inline scripts or style rules such as this that don't play nice with CSP. These should be replaced by a CSS class.


Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

@lunny lunny added type/enhancement An improvement of existing functionality topic/ui Change the appearance of the Gitea UI labels Nov 29, 2016
@tboerger tboerger added this to the 1.x.x milestone Nov 29, 2016
@bkcsoft
Copy link
Member

bkcsoft commented Dec 2, 2016

Yes, that should be a class, not inline style :)

@Eriner
Copy link
Contributor

Eriner commented Dec 11, 2016

As a note: report-uri is a CSP directive that informs the browser where to report violations. Something to consider is providing/setting this directive and ingesting it into gitea to make it available to administrators.

@denji
Copy link
Contributor

denji commented Dec 17, 2016

@Bwko
Copy link
Member

Bwko commented Dec 18, 2016

I'm currently implementing a Content-Security-Policy but we need style-src: unsafe-inline & script-src: unsafe-eval for semantic ui & jquery datetimepicker.
@bamboleeeero-bamboleeeero any advice on this?

@tboerger
Copy link
Member

We should get rid of all inline crap.

@Bwko
Copy link
Member

Bwko commented Dec 18, 2016

@tboerger I'm almost done with that (except for labels) but for tooltips semantic ui needs unsafe-inline 😕

@tboerger
Copy link
Member

Even tooltips can be perfectly done with data attributes and unobtrusive js.

@hasufell
Copy link

hasufell commented Jun 6, 2018

any news?

@clarfonthey
Copy link
Contributor

Personally, I would appreciate at least adding a content-security-policy header to Gitea, slowly making it more strict as changes are made. At least things like form-action and frame-ancestors would be nice, which are doable now.

I'd be willing to work on this within the next few days if someone more-acquainted with the codebase isn't up to it!

@toth-dev
Copy link

I'm adding this CSP with nginx:

default-src 'none'; base-uri 'none'; manifest-src 'self'; img-src 'self' data:; font-src 'self' data:; style-src 'self' 'unsafe-inline'; script-src 'self' 'unsafe-eval' 'unsafe-inline'; connect-src 'self'; form-action 'self'

Everything seems to work with this.

@hasufell
Copy link

@totpet interesting, I tried that too and it seems to work so far, but afais mozilla observatory reports this as -20 score, so it seems to be too lax.

@clarfonthey
Copy link
Contributor

As I said, things like frame-ancestors and form-action are really important parts of CSP that we could add for existing gitea. Adding unsafe-inline and unsafe-eval to JavaScript are really bad though, and that pretty much negates everything else.

@toth-dev
Copy link

My policy broke the embedded pdf viewer, because it is loaded as an iframe, and frame-src falls back to default-src, adding this fixes the problem: frame-src 'self'; frame-ancestors 'self'

(frame-ancestors only needs 'default' inside vendor/plugins/pdfjs/web/, for other locations it can be set to 'none')

@stale
Copy link

stale bot commented Jan 21, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 weeks. Thank you for your contributions.

@stale stale bot added the issue/stale label Jan 21, 2019
@techknowlogick techknowlogick added the issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented label Jan 23, 2019
@Oreolek
Copy link

Oreolek commented Jul 7, 2019

Any news on this? Keep in mind the "Gitea behind a reverse proxy" case where Gitea won't be able to change CSP header dynamically.

@noplanman

This comment was marked as off-topic.

@MetroWind
Copy link

MetroWind commented Nov 20, 2019

New user here. I’d also love to see this sorted out. Beside inlines, I think there are also some evals, because I had to add unsafe-eval to my CSP header for that activity matrix thingy in the dashboard page to show up.

@hasufell
Copy link

@MetroWind to which header?

@dpertin
Copy link

dpertin commented Mar 30, 2020

I used the Mozilla Laboratory to generate the following CSP:

default-src 'none'; connect-src 'self'; font-src 'self' data:; form-action 'self'; img-src 'self' https://ga-beacon.appspot.com https://raw.githubusercontent.com https://secure.gravatar.com https://sourcethemes.com; script-src 'self' 'unsafe-eval' 'unsafe-inline'; style-src 'self' 'unsafe-inline'; worker-src 'self';

Everything seems to work with nginx, but the Google CSP Evaluator notifies a high-priority issue due to the use of 'unsafe-inline' in script-src. I'd love to see all those inline scripts either externalized, or whitelisted by supporting nonces as proposed here.

@SuperSandro2000
Copy link
Contributor

img-src 'self' https://ga-beacon.appspot.com https://raw.githubusercontent.com https://secure.gravatar.com https://sourcethemes.com;

This should probably be more like:
img-src 'self' https://raw.githubusercontent.com https://secure.gravatar.com;

@tomposmiko

This comment was marked as off-topic.

@alexanderadam
Copy link

Can we count on fixing this issue ever?
I'm wondering if it will ever happen or we should plan with not fixing it ever.

@tomposmiko most free softwares don't have dedicated developers who are able to make a living by working on them. Meaning, that you can not rely that a feature or a bugfix will be implemented on one point. It is also important that you understand, that this is not specific to gitea, however.

On the plus side, you can usually implement those things by yourself or pay someone to do it.
If I'm not mistaken, this issue wasn't important enough for anyone here, to pay anything, however.
And it wasn't important enough to anyone to invest their valuable time fixing this issue neither.

Now it should be clear, that it would need clairvoyant abilities to answer your question properly. Because this is not a commercial product that can afford ensuring due dates for features or bugfixes reliably.

@SuperSandro2000
Copy link
Contributor

that a feature or a bugfix will be implemented on one point.

Not that it is different if your paying for software 😆

@Crocmagnon
Copy link

Could we at least make gitea serve its own CSP? IMO it's the responsibility of the app to know what it needs to load and from where. Even if we're not going to implement a safe CSP in the first iteration, I'd suggest starting by providing one and then refining it when we clean the inlines and evals.

An even easier first step might be to provide a CSP in the documentation that admins can set on their proxies and after some time implementing it in the code.

@ix5
Copy link

ix5 commented Feb 23, 2022

Just as a current (2022, gitea 1.16.1) reference point, I have the following policy implemented and haven't had any (gitea-related) pings on my report-uri:

Content-Security-Policy-Report-Only: "default-src 'self'; connect-src 'self'; font-src 'self' data:; form-action 'self'; img-src 'self' https: data:; object-src 'self'; script-src 'self' 'unsafe-eval' 'unsafe-inline'; style-src 'self' 'unsafe-inline'; worker-src 'self';"

Edit: Amended with below comment (which I can confirm):
Content-Security-Policy-Report-Only "default-src 'self'; connect-src 'self'; font-src 'self' data:; form-action 'self'; img-src 'self' https: data:; manifest-src 'self' data:; object-src 'self'; script-src 'self' 'unsafe-eval' 'unsafe-inline'; style-src 'self' 'unsafe-inline'; worker-src 'self';"

Admins who would like to tighten that policy up (esp. img-src https) should be free to do so, but I think this is a good baseline.

@tepozoa
Copy link

tepozoa commented Mar 30, 2022

  • Gitea 1.16.5
  • Chrome 100.0.4896.60

Generally following the above, I ran into Chrome refusing to load a manifest-src base64 encoded element as it wasn't explicitly allowed to use data: and was inheriting default-src attributes. Adding manifest-src 'self' data:; onto the above solved that Chrome error.

@silverwind
Copy link
Member

silverwind commented Apr 26, 2023

script-src: unsafe-inline will probably required indefinitely. There are techniques that can only work with inline script like synchronously altering DOM during rendering to avoid content pop-in, so I see it as a requirement for a ideal experience. I made an attempt to remove inline script in #21429, but it was not well received as it's quite a hack.

We should put out a CSP recommendation in the docs as a guidline on how to set up the most strict CSP possible for gitea to work, but I don't see anything else to be done here.

@clarfonthey
Copy link
Contributor

I'm not 100% sure of the details, but surely simply hashing the critical JS would be a solution here? CSP supports this, and it means you'd be able to say that these scripts could be run without anything else being allowed. The hash might change between releases, but I don't imagine that being difficult for folks to deal with if they really care that much over using unsafe-inline.

IMHO the most important aspect of this would be to have similar docs recommending a CSP header for gitea, or having gitea output one itself, even if it does include unsafe-inline. Could this issue be kept open until that is finished?

@silverwind silverwind reopened this Apr 26, 2023
@silverwind
Copy link
Member

Guess we can keep open for docs.

hashing the critical JS

What technique is that? Any more info?

@clarfonthey
Copy link
Contributor

MDN has you covered: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/script-src

Essentially, you can add sha256-(hash) to the list of allowed sources in the CSP header.

@silverwind
Copy link
Member

silverwind commented Apr 26, 2023

Hmm I guess we could set a static nonce on all our inline scripts and CSP could be recommended to use that nonce. It won't be a hash as it needs to work across upgrades.

@jotoho
Copy link

jotoho commented Apr 26, 2023

If Gitea emitted the CSP-header itself, it could dynamically calculate the hashes of the inline scripts it sends, couldn't it?

I wouldn't be surprised if browsers or the standardization committees implement additional measures (in the future if they haven't already) to prevent usage of nonces that are used more than once.

@tepozoa
Copy link

tepozoa commented Apr 26, 2023

If you're running other resource paths on your frontend and need CSP policies to accommodate their needs, having Gitea generate a full header could be problematic. I found this issue discussing the spec outlining that multiple headers are not to be sent by a server though, but if they are they are evaluated as individuals and not a combined directive.

Given what I read in the above, a toggle in app.ini to allow an admin to disable Gitea from generating it's own CSP header has to be present, as the Gitea header may conflict with what they're running in their reverse proxy layer.

@silverwind
Copy link
Member

Yes, own CSP header is problematic when reverse proxies "Add" instead of "Replace" the header, which I think is the most common configuration. So I think we should just put out a recommendation in docs initially.

Also, I do not see inline scripts as the grave security problem that some sites make it out to be. If everything is first-party, a inline script is equivalent to a url-sourced script in terms of security. I would not recommend running third-party scripts on Gitea anyways.

@clarfonthey
Copy link
Contributor

The main benefit of disallowing inline scripts is to proactively prevent XSS and other vulnerabilities from working even if someone manages to get a script tag in the DOM. I agree that the threat model is probably overprotective in some cases, but it's not just about whether there's an external script or not.

@silverwind
Copy link
Member

silverwind commented Apr 26, 2023

Reading MDN nonce it does seem something that would be easy to support, just generate a random number per HTML request and add it to all resource tags (inline or not).

If we set CSP ourselves in gitea, it will need extensive configurability so users can include 3rd party resources and the like, e.g. a format in config like {"script-src": ["https://example.com"]} to define additional directives.

@nicheosala
Copy link

I'm trying to remove inline JS scripts from Gitea, so that we can avoid script-src: 'unsafe-inline' in CSP.

My main problem is the inline script in templates/base/head_script.tmpl: does anyone have a any idea how to move it to a separate JS file? It is obtained mixing Go templates and Javascript.

I'm not an expert on Go templates and can't find a solution to the above problem. The other inline scripts are not many and they do not mix templates and Javascript. I can list them using VS Code (don't consider the ones that start with <script src, they are fine):

inline_scripts

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Feb 9, 2024

I'm trying to remove inline JS scripts from Gitea, so that we can avoid script-src: 'unsafe-inline' in CSP.

My main problem is the inline script in templates/base/head_script.tmpl: does anyone have a any idea how to move it to a separate JS file? It is obtained mixing Go templates and Javascript.

As one of the people who do a lot of Gitea frontend refactoring works, I would say that avoiding inline <script> is not possible at the moment -- at least, very difficult. Actually head_script.tmpl is the easiest one, it could be replaced by some data attribute tags. But, the real blockers are some other <script> blocks which heavily depend on the JS code in page.

@nicheosala
Copy link

Thank you @wxiaoguang. I think that any reduction of inline scripts is useful for the security of Gitea, although this is an objective often overlooked in favor of others. Maybe I'm particularly sensitive, since the cybersecurity professor just taught us how to carry out XSS attacks 😂

I'll try to study the Gitea code better (and the Go language and its templates) and help out.

@silverwind
Copy link
Member

silverwind commented Mar 23, 2024

Inline scripts can be exempted from CSP by adding a nonce

Also, I think there is nothing preventing us from serving a 1st-party CSP. Load balancers can always override or extend it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented topic/ui Change the appearance of the Gitea UI type/enhancement An improvement of existing functionality
Projects
None yet
Development

No branches or pull requests