[Panel] CSP errors - Githubissues

bnomei commented 5 years ago

Describe the bug The panel requires CSP with inline styles and inline eval. this would require a rule to allow inline js eval for the complete site just to make the panel work. please find a more secure way.

the following examples are just for the panel login page. there might be more issues past the login.

Content Security Policy: The page’s settings blocked the loading of a resource at inline (“style-src”).
<body>
<svg aria-hidden="true" style="position:absolute;width:0;height:0" xmlns="http://www.w3.org/2000/svg" overflow="hidden">
solution: just add a class instead, right?

Content Security Policy: The page’s settings blocked the loading of a resource at inline (“script-src”).
<script>window.panel = {"url":"https://DOMAIN/panel","site":"https://DOMAIN","api":"https://DOMAIN/api","csrf":"ade9273632029b912a42a7aa9f54b85d7626a24ddd8b97177d05a7eb94ed3116","translation":"en","debug":true,"search":{"limit":10}}</script>
solution: register a nonce using http header and add a nonce for the script element. https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/script-src#Unsafe_inline_script

To Reproduce Steps to reproduce the behavior: setup some kind of secure CSPs (aka not inline, not eval!) a quick test could be done with my plugin https://github.com/bnomei/kirby3-security-headers please note that i am well aware that in almost all cases the default CSP my plugin provide must be extended. i just want to point out the panel could be made more security friendly in not using inline styles and eval.

Expected behavior I expeced the panel to not use inline styles and eval because its unsave to do so. I understand kirby should not define csp for the fronend itself, but it should at least make the panel save.

Kirby Version 3.x

bnomei commented 5 years ago

furthermore i did not run into the inline scripts error using kirby 3.0.1? so maybe this is new?

bnomei commented 5 years ago

trying to login with no CSP rule for the inline script will fail. this is the error message.

Object { status: "error", exception: "Kirby\\Exception\\InvalidArgumentException", message: "Invalid CSRF token", key: "0", file: "y/config/api/routes/auth.php", line: 31, details: [], code: 400 }
app.js:1:236022

bastianallgeier commented 5 years ago

We had the same setup in the previous versions, so the errors must be the same.

As a first fix I removed the inline style attribute and switched to a class.

Eval isn't used anywhere so that's not a problem.

The inline script tag is still an issue and as you said we can only fix that with a nonce. I'm a bit hesitant to set a default content security policy right now. I don't really know what kind of expectations plugin devs would have. Let's say you create a plugin that loads images from a CDN, you would then also need to change the CSP for the panel.

I was wondering if it made sense to introduce a panel.headers option. This could be a callback that receives the nonce that the panel will use in the template.

return [
  'panel' => [
    'headers' => function ($nonce) {
       // set custom headers here. 
    }
  ]
];

This would offer total freedom to create your own policies for the panel.

Another option would be some global registry for plugins to register "safe" sources and the panel would then take care to set the matching headers.

bnomei commented 5 years ago

ah you are right. the error appeared once i switched my plugin from snippet to route:before hook. the latter is applied to panel as well.

the panel.headers callback seems like a fine idea. but my suggestion would be to make it panel.csp.nonces and return an array. pretty much like my plugins allows setting custom nonces right now. https://github.com/bnomei/kirby3-security-headers/blob/master/index.php#L17

i could just array_merge that callback in my plugins default implementation and thus still keep it "zero config" for default usecase. https://github.com/bnomei/kirby3-security-headers/blob/master/classes/SecurityHeaders.php#L84

bnomei commented 5 years ago

sorry that might have been confusing. what i do not need is a way to set custom headers. i use my plugin to do that and putting it in another callback in the config does not make it much more readable imho.

i only need the array of all nonces the panel needs. so it does not have to be a callback at all. something like kirby()->system()->nonces():array would be totally fine. that array i can use to build the headers.

lukasbestle commented 5 years ago

Also see this thread in the forum.

distantnative commented 5 years ago

I understand @bastianallgeier's hesitation to set a default content security policy. What about:

We generate and set a nonce for the inline script on the tag
We do not set any CSP ourselves
We provide that nonce in kirby()->system()->nonces():array

Then everything is prepared. And those who set a CSP can include the nonce to keep the Panel working.

bastianallgeier commented 5 years ago

I finally fixed this by introducing a new $kirby->nonce() method. The nonce is used for all scripts, stylesheets and the svg. I think it's enough that we have one nonce which changes on every request, but please correct me if I'm wrong.

getkirby / kirby

[Panel] CSP errors #1527