jitsi / jitsi-meet

Jitsi Meet - Secure, Simple and Scalable Video Conferences that you use as a standalone app or embed in your web application.
https://jitsi.org/meet
Apache License 2.0
22.89k stars 6.69k forks source link

Low mozilla observatory score (CSP, XSS...) #6887

Closed 532910 closed 4 months ago

532910 commented 4 years ago

https://observatory.mozilla.org/analyze/meet.jit.si Grade C-

Content Security Policy (CSP) header not implemented X-Frame-Options (XFO) header not implemented X-XSS-Protection header not implemented

saghul commented 4 years ago

Ping @aaronkvanmeerten

532910 commented 4 years ago

I suppose this is not exactly meet.jit.si instance issue, but lack of HTTP or <meta> headers.

luixxiul commented 4 years ago

It seems to me there is a couple of options:

  1. Implement everything to make the rank A, no matter how highly it is possible to break existing instances ... implementing CSP without caution would break the working sites
  2. Add nothing on config files + on README.md add link to a handbook page which explains how to harden server securities ...
  3. Add a couple of things to make the rank higher (even if it would be not A) and also avoid breakage.

IMHO implementing CSP frame-anscestors / X-Frame-Options by default should be avoided as it will break the existing instances on iframe, so my take would be the third one to improve stuff moderately. Setting up a guide on handbook should also be appreciated.

532910 commented 4 years ago
  1. I don't think that ignoring CPS it's a good idea. (What securities do you mean?)
  2. I think we should start with rules that don't break anything. Not for rank, but for the start. Minimal rules that works for me gives only C+ grade instead of current C- https://github.com/jitsi/jitsi-meet-electron/issues/358#issuecomment-636266478
  3. At the end we should implement everything to make the rank A. Not breaking everything, but correcting the code.
saghul commented 4 years ago

There are things that we won’t the able to “correct” AFAIK if one wants to have the external API enabled, since it uses an iframe... but I’ll be glad to be wrong!

532910 commented 4 years ago

frame-src/frame-ancestors isn't the only policy and there are a lot of other that could be tightened.

luixxiul commented 4 years ago

I'm really not against making that grade A. I'm concerned that without thorough tests over as many platforms as possible instances might break critically, which leads to UX disasters and a flood of feedback that you saw April. That is why I'm inclined to the opinion that security measures would be integrated moderately and gradually, and radical changes be avoided as well, considering the balance between security and UX.

Instead documentations can be radical here :-D

532910 commented 4 years ago

moderately and gradually

Sure!

But this is about current issues that should be solved to increase grade. For example style-src and script-src requires unsafe-inline.

stale[bot] commented 4 years ago

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

532910 commented 4 years ago

still not fixed

stale[bot] commented 3 years ago

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

532910 commented 3 years ago

The grade is still C-

I believe that the bot shouldn't decide whether to fix it or not!

stale[bot] commented 3 years ago

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

532910 commented 3 years ago

The grade is F now!

I believe that the bot shouldn't decide whether to fix it or not!

stale[bot] commented 2 years ago

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

532910 commented 2 years ago

The grade is still F

jbg commented 2 years ago

It's quite easy to get this up to B without too much risk of breakage:

If the user disables iFrame API access for their stack, we also add X-Frame-Options: SAMEORIGIN and the frame-ancestors directive in the CSP. These would probably not be realistic on meet.jit.si, since the ability to put it in an <iframe> is a feature, so that might cap the grade lower.

Getting the grade above B would require code changes to allow further restricting the CSP. Jitsi Meet makes use of both inline scripts (which could be made more secure using nonces, allowing the removal of unsafe-inline from the CSP) and eval() (use of which needs to be removed in order for unsafe-eval to be removed from the CSP). I'm sure PRs would be welcomed to improve things in this area.

saghul commented 2 years ago

I haven't checked in the last few months, but one challenge with unsafe-eval is/was that removing it also makes WASM not work, which we depend on for virtual backgrounds, E2EE and more stuff.

jbg commented 2 years ago

Yeah, I think that is still the case (from memory it may depend on how you load the WASM blob though). Having the CSP with unsafe-eval is still better than no CSP though; it still allows you to restrict base-uri, restrict scripts loaded from remote sources, etc. You could also safely deploy Content-Security-Policy-Report-Only with a report-uri and pretty quickly get an idea of whether a given policy will cause problems or not.

stale[bot] commented 2 years ago

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

532910 commented 2 years ago

The grade is still F

532910 commented 1 year ago

The grade is still F!!

github-actions[bot] commented 6 months ago

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

532910 commented 6 months ago

It's C now!

github-actions[bot] commented 4 months ago

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