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

Some security flaws in Jellyfin #1885

Closed
teacupx opened this issue Oct 11, 2019 · 7 comments
Closed

Some security flaws in Jellyfin #1885

teacupx opened this issue Oct 11, 2019 · 7 comments
Labels
security The issue is a security issue.

Comments

@teacupx
Copy link

teacupx commented Oct 11, 2019

Hello everyone. I noticed that running a scan towards some public Jellyfin server from https://observatory.mozilla.org reveals a few security flaws. More specifically:

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

I'm not an HTML security expert, but according to Mozilla these are not too difficult to correct (the webpage linked above gives information about how to do it). Could it be done in future versions of Jellyfin?

@iotku
Copy link

iotku commented Oct 11, 2019

Might be worth modifying the documentation for reverse proxies with notes about general security headers.

They're not critically important in most cases, but they're not terrible practice either.

The CSP is the most "difficult" part to deal with as you somewhat have to work out a lot of settings and the rabbit hole goes pretty deep if you want a "perfectly" restricted CSP without breaking functionality and I believe as it stands jellyfin-web does a fair bit of "unsafe" inlines or possibly evals.

For example, my nginx config I have the following in my server directive (updated 10/19/19)

add_header X-Frame-Options "SAMEORIGIN";
add_header X-XSS-Protection "1; mode=block";
add_header X-Content-Type-Options "nosniff";
add_header Content-Security-Policy "default-src https: data: blob:; style-src 'self' 'unsafe-inline'; script-src 'self' 'unsafe-inline' https://www.gstatic.com/cv/js/sender/v1/cast_sender.js; worker-src 'self' blob:; connect-src 'self'; object-src 'none'; frame-ancestors 'self'";

My CSP is pretty lax and could be restricted more for an improved "score", but at a glance it seems functional with jellyfin, a lot more effort would probably have to go in if you have multiple applications on the same subdomain.

@teacupx
Copy link
Author

teacupx commented Oct 12, 2019

Might be worth modifying the documentation for reverse proxies with notes about general security headers.

Definitely something very useful for those of us who don't know where to start, and very easy to do for someone who has the knowledge.

@iotku Thank you very much for sharing your nginx reverse proxy configuration. Do you know, by chance, what would be the equivalent config for Apache?

@JustAMan JustAMan added the security The issue is a security issue. label Oct 15, 2019
@JustAMan
Copy link
Contributor

@iotku would you mind making a PR to https://github.com/jellyfin/jellyfin-docs adding those directives?

This also seems to resonate with #879

@iotku
Copy link

iotku commented Oct 16, 2019

As it stands currently a lot of the benefit of the CSP would be had if we didn't need script-src 'unsafe-inline'.

https://csp.withgoogle.com/docs/why-csp.html

Creating a policy which allows inline scripts (script-src 'unsafe-inline') or allows loading scripts from untrusted domains does not improve security because it doesn't protect the application from XSS. If an application cannot use a safe policy it will not benefit from CSP until all patterns incompatible with CSP are removed; as such, it is prudent to hold off on deploying a policy until the application can adopt strict CSP.

I believe most of that would be on the jellyfin-web side of things or perhaps the react client in the future.

@dkanada
Copy link
Member

dkanada commented Oct 17, 2019

I'm not sure how much work it would be to remove all inline scripts except apploader.js which initializes everything. @iotku if you want to look into that we could integrate the CSP changes into master.

I have also seen issues with the current CSP when running the web client on its own using nginx, but hadn't looked into the required changes yet.

@iotku
Copy link

iotku commented Oct 19, 2019

Most of the onclick elements I hit were on the dashboard (like the shutdown button) and a quick search of jellyfin-web looks like there's some on the setup wizard as well.

I've been fairly successful viewing media/searching etc without hitting any csp blocks outside of the dashboard, but I suspect there might be some I missed. I believe the reports plugin also utilizes inline scrips, though I haven't toyed with all the plugins and again that's on the dashboard.

Of particular usefulness is adding 'report-sample' to script-src while testing as it gives much more detail in the console about what triggered it

@sevenrats
Copy link
Member

much of this has been addressed according to the linked issues.
if parts of this issue still persist, please open new, more specific issues.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
security The issue is a security issue.
Projects
Archived in project
Development

No branches or pull requests

5 participants