feat: add Content-Security-Policy Header, allows setting frame-ancestor domains and moved font local

[Ruakij]

Description

I have added Content-Security-Policy Header in a very moderate mode not to break any current features.
Through Content-Security-Policy we can allow specific origins or "Frame-Ancestors" to embed jellyseer as e.g. iframe.

https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/frame-ancestors

Currently, correct usage in an iframe is blocked by strict or lax setting of SameSite on the session- and, if enabled, the csrf-cookie, which results in the cookie not getting send in a request which originates from a cross-origin.

jellyseerr/server/index.ts

Lines 163 to 175 in edfd804

	const sessionRespository = getRepository(Session);
	server.use(
	'/api',
	session({
	secret: settings.clientId,
	resave: false,
	saveUninitialized: false,
	cookie: {
	maxAge: 1000 * 60 * 60 * 24 * 30,
	httpOnly: true,
	sameSite: settings.main.csrfProtection ? 'strict' : 'lax',
	secure: 'auto',
	},

This setting is incompatible with CSRF-Protection as its not possible to have the protection without SameSite strict.

I am unsure if we should complexity remove the SameSite attribute or leave it with the condition when Frame-Ancestor Domains is unset like i have done currently.

As my setup of Content-Security-Policy also blocked the google-fonts request, i simply moved it to local with the additional benefit of speed and privacy.
If you rather want to, we can remove most of the Content-Security-Policy not to hinder future features, but its a good thing for security usually to block anything we dont expect like loading external javascript.

Screenshot (if UI-related)

The setting is a space-separated string of domains.

When the browser blocks the request, it looks something like this:

And here when having added the domain:

To-Dos

• Successful build pnpm build
• Translation keys pnpm i18n:extract
• Database migration (if required)

Issues Fixed or Closed

• Fixes Authentification error Jellyseerr iFrame in Jellyfin #455

[ThomasVicot]

Hi,
I tried to install this version via the to do,
pnpm build
pnpm i18n:extract
pnpm start
In ‘General Settings’ I don't have the new feature ‘Frame-Ancestor Domains’.
Have I forgotten a step?

[Ruakij]

Hi, I tried to install this version via the to do, pnpm build pnpm i18n:extract pnpm start In ‘General Settings’ I don't have the new feature ‘Frame-Ancestor Domains’. Have I forgotten a step?

Ah, i think you already had an existing configuration and then the new setting is missing and wont be displayed.. wasnt aware there isnt a migration for that.
I'll recheck the code. In the meantime you can probably just add the entry "cspFrameAncestorDomains": "" under main in settings.json

[Ruakij]

@ThomasVicot I have checked, the setting doesnt require a migration, if the field doesnt appear, you did something wrong.
All you should have to do is checkout the branch and pnpm dev for the development build.

[Fallenbagel]

Hi,
I tried to install this version via the to do,
pnpm build
pnpm i18n:extract
pnpm start
In ‘General Settings’ I don't have the new feature ‘Frame-Ancestor Domains’.
Have I forgotten a step?

The todo is not the steps to checkout prs. Thats for the PR authors todos...

to checkout prs you do:
http://stackoverflow.com/questions/27567846/ddg#30584951

[github-actions]

This pull request has merge conflicts. Please resolve the conflicts so the PR can be successfully reviewed and merged.

[gauthier-th]

I am unsure if we should complexity remove the SameSite attribute or leave it with the condition when Frame-Ancestor Domains is unset like i have done currently.

IMHO I think it's ok the way you've done it. @M0NsTeRRR may have a more stronger opinion on the subject.

[gauthier-th]

Shouldn't you specify here that this is a space-separated list of domains?

[Ruakij]

There are more possibilities to setting this than just space separated as this is directly put into the header.
But maybe we could link to the specification: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/frame-ancestors

[gauthier-th]

Oh ok. That's a good idea yes

[gauthier-th]

Why this change?

[Ruakij]

Before, the text-box wasnt visibly disabled apart from not being able to interact with it, so this makes it a bit more visible

[gauthier-th]

Did you check that this does not have any other unexpected behavior? It could have undesired effects on some text fields. and other fields too*

We usually do styling using TailwindCSS, inside the class attribute of the elements

[gauthier-th]

Maybe you could add a newline after the imports?

[M0NsTeRRR]

As my setup of Content-Security-Policy also blocked the google-fonts request, i simply moved it to local with the additional benefit of speed and privacy.

Fully agree

If you rather want to, we can remove most of the Content-Security-Policy not to hinder future features, but its a good thing for security usually to block anything we dont expect like loading external javascript.

Fully agree

This setting is incompatible with CSRF-Protection as its not possible to have the protection without SameSite strict.

Not exactly. CSRF protection shouldn't affect the CSP header, but it does in this case because it isn't configured properly. CSRF protection is meant to prevent logged-in users from unknowingly executing actions (such as via a JavaScript fetch when they visit a webpage). Ideally, CSRF protection should only apply to form pages and shouldn't impact iframes, as iframes are used only for displaying data not form. If we don't want to disable CSRF for CSP (we should do that), we need to fix the CSRF system in this way.

[Ruakij]

If we don't want to disable CSRF for CSP (we should do that), we need to fix the CSRF system in this way.

Yeah i worded that too broadly, CSP in general is fine, but CSP ancestor-domains/iframes in combination with SameSite is incompatible.
SameSite is just very restrictive and doesnt allow any domain exceptions.

So the CSRF cookie would simply need SameSite none.
We should probably explicitly set SameSite none for all cookies going forward as we should be able to rely on CSP ancestor-domains preventing clickjacking (which could afaik. otherwise bypass CSRF protected forms with iframes).

So i guess we can change

jellyseerr/server/index.ts

Lines 157 to 174 in 257cab3

	if (settings.main.csrfProtection) {
	server.use(
	csurf({
	cookie: {
	httpOnly: true,
	sameSite: true,
	secure: !dev,
	},
	})
	);
	server.use((req, res, next) => {
	res.cookie('XSRF-TOKEN', req.csrfToken(), {
	sameSite: true,
	secure: !dev,
	});
	next();
	});
	}

sameSite: true to 'none'?

Another thing should probably then be to be able to bypass CSRF protection with API-keys to have read/write API, but protected for user-actions, but thats for the future.

[M0NsTeRRR]

So the CSRF cookie would simply need SameSite none.

Jellyseerr is a stateless application that uses signed double submit cookies to prevent CSRF attacks. According to the proposed standard, we should set the SameSite attribute to strict or lax, though lax might not cover all of our cases. This mitigation is suggested for page that need authentication for cookie based session management, but it’s not as simple as turning off SameSite (which we want to avoid, as it would disable CSRF protection). If we want to iframe non action pages, we can disable CSRF for those pages, as it's not necessary.

Another thing should probably then be to be able to bypass CSRF protection with API-keys to have read/write API, but protected for user-actions, but thats for the future.

Yes

[Ruakij]

Yeah i havent thought this through fully.
If we want the application to be useable (at least for user-actions like requesting), we cannot use Cookie-based CSRF protection which relies on SameSite as on other mode than none, it doesnt work through iframes.
iframe navigations arent top-level navigations.

So maybe using the signed-token approach via header or somewhere an attacker cannot obtain it and/or get the browser to submit it via Cross-Origin.

I think for now we can implement this PRs iframe-support with disabling the current cookie-based CSRF-protection and then gradually move towards a more compatible system?

[M0NsTeRRR]

If we want the application to be useable (at least for user-actions like requesting), we cannot use Cookie-based CSRF protection

As I mentioned before, CSRF protection should be disabled for non-action requests. By 'action,' I mean any operation that updates the application, like POST, PATCH, or DELETE requests. GET requests don't require CSRF protection, which would allow us to use iframes without encountering this issue. Regarding action within an iframe, I’m not clear on the objective here. It would be more effective to rebuild the form you need and use a future API (in a feature request) for this purpose.

So maybe using the signed-token approach via header or somewhere an attacker cannot obtain it and/or get the browser to submit it via Cross-Origin.

There is no API to read initial page HTTP response header.

I think for now we can implement this PRs iframe-support with disabling the current cookie-based CSRF-protection and then gradually move towards a more compatible system?

That’s up to the maintainer, but in my opinion, I disagree. Adding a 'feature' (iframe) that disables security, potentially allowing unauthorized admin access via an HTTP POST request on the create user endpoint, could lead to serious issues. I previously submitted a CVE for network equipment that had this vulnerability (about 20 lines of code) and was rated as a 8.8 CVE.

[Fallenbagel]

That’s up to the maintainer, but in my opinion, I disagree. Adding a 'feature' (iframe) that disables security, potentially allowing unauthorized admin access via an HTTP POST request on the create user endpoint, could lead to serious issues. I previously submitted a CVE for network equipment that had this vulnerability (about 20 lines of code) and was rated as a 8.8 CVE.

As the maintainer, I would like to avoid security vulnerabilities/CVEs as much as possible, so I'm with @M0NsTeRRR on this.

[github-actions]

This pull request has merge conflicts. Please resolve the conflicts so the PR can be successfully reviewed and merged.