ckeditor / ckeditor5

Powerful rich text editor framework with a modular architecture, modern integrations, and features like collaborative editing.
https://ckeditor.com/ckeditor-5
Other
9.62k stars 3.7k forks source link

Support CSP #335

Closed Reinmar closed 5 years ago

Reinmar commented 8 years ago

CSP has been a serious problem for CKEditor 4 (https://dev.ckeditor.com/ticket/8584) due to some old decisions that's been made (mainly due to legacy browsers support).

In case of CKEditor 5 I don't expect to see any problems, but we should test for it. AFAICS, it should be possible to simply enforce some CSP settings using a <meta> tag:

<meta http-equiv="Content-Security-Policy" content="default-src https://cdn.example.net; child-src 'none'; object-src 'none'">

If not, then let's create a simple test env (a Node.js HTTP server and a sample).

Reinmar commented 7 years ago

I asked @pomek to validate that CKE5 works with CSP. We'll need a small change in our manual tests server.

pomek commented 7 years ago

CKE5 will work if CSP will contain following data:

<meta http-equiv="Content-Security-Policy" content="default-src http://localhost:8125; child-src 'none'; object-src 'none'; style-src 'self' 'unsafe-inline'; font-src data; img-src 'self' data">

for http://localhost:8125/ckeditor5-enter/tests/manual/enter.html.

Since at this moment I'm working on dev-tools for CKE5, I've checked whether the dev-tools will be working with CSP. I had to add additional domains to CSP:

<meta http-equiv="Content-Security-Policy" content="default-src http://localhost:8125; child-src 'none'; object-src 'none'; style-src 'self' 'unsafe-inline' https://fonts.googleapis.com; font-src data https://fonts.gstatic.com; img-src 'self' data">

for http://localhost:8125/ckeditor5-engine/tests/dev-utils/manual/graphicalmodelrepresentation.html.

Unfortunately, the URL above won't work because I haven't pushed my changes to a remote server.

fredck commented 7 years ago

@pomek, instead of showing which CSP tag must be used, are you able to list here which CSP features will not work with CKEditor 5?

pomek commented 7 years ago

To be honest, I don't know what do you would like to see. Could you provide an example which helps me understand your request?

fredck commented 7 years ago

If you check the Content Security Policy Reference, you'll find the "Directive Reference" where all CSP features are listed. Our goal should be to document the CSP support and say that "CKEditor 5 is compatible with CSP, except when the following directives are used: ...".

If we don't have any limitation, then even better. We just need to confirm that.

Reinmar commented 7 years ago

Also, I think that part of this task should be enabling common CSP rules by default in our tests.

I'm not sure how well supported are those <meta> tags, so I'd rather propose sending proper headers to the browser. We should be able to do that equally easy with our manual tests server.

pomek commented 7 years ago

As for today, CKEditor 5 is compatible with the following CSP rules:

And doesn't work with the following rules:

but, the editor can work if will be configured with additional rules:

{
    "style-src": "'unsafe-inline'",
    "font-src": "data:",
}

I was not be able to check the following rules:

pomek commented 7 years ago

I'm not sure how well supported are those tags, so I'd rather propose sending proper headers to the browser. We should be able to do that equally easy with our manual tests server.

=> https://github.com/ckeditor/ckeditor5-dev/issues/87

wwalc commented 7 years ago

Just to simplify things a bit...

The most strict CSP setting

The most strict CSP settings with which CKEditor would still run is as follows:

(I assumed that CKEditor is loaded from the same host ('self')).

Optional Image support

Additionally, the following CSP must be set in order to use images in the editing area:

Most likely the data: protocol will need to be allowed to support pasting images from clipboard.

Explanation

default-src

default-src 'none' - CSP can be set to block everything by default (yay).

script-src

script-src 'self' - allow the website to load JavaScript file with CKEditor from the same host. In fact, it can be further reduced to load just one script with bundled CKEditor using nonces, e.g.:

// CSP rule
script-src 'nonce-2726c7f26c';
// Loading CKEditor:
<script src="build/js/app.bundle.js?0.8.0" nonce="2726c7f26c"></script>

style-src

This one is tricky. As long as we do not allow users to use inline styles in CKEditor there should be no need to allow unsafe inline styles. However it looks like currently SVG icons require it.

'self' is used to load a CSS file with CKEditor "skin" (if one creates a bundle with separate .js and .css files, like I do it here). Again nonces can be used to allow just specific files.

'unsafe-inline' unfortunately it is required due to SVG icons - at least when merged into app.js they have inline styles, e.g. style="fill:#3a3a3a;". What's funny is that even when CSP blocks these styles, SVG icons are rendered. I did not find any significant differences between the look of SVG icons, but I did not zoom them. For me this is something to check.

img-src

Thanks to using SVG icons, there is no need for CKEditor to require enabling images, unless someone wants to allow users to insert them into CKEditor.

I have no idea why font-src was specified by @pomek above. maybe I missed something but I did not notice any errors in the dev console when using a bundled version of CKEditor with settings described above.


Note: the following check was maded with a CKEditor bundle generated with Webpack 2, with and without minification performed by UglifyJS.

pomek commented 7 years ago

Let me clarify one thing - because it was a long time ago, I don't remember WHY I mentioned about font-src. Maybe I was checking the editor using an invalid bundle or I forgot to switch to incognito mode.

Reinmar commented 7 years ago

Let me clarify one thing - because it was a long time ago, I don't remember WHY I mentioned about font-src. Maybe I was checking the editor using an invalid bundle or I forgot to switch to incognito mode.

Didn't you mention it because of your tool for printing out the model structure?

pomek commented 7 years ago

Yes, it uses Inconsolata font from Google Fonts.

fredck commented 7 years ago

What's funny is that even when CSP blocks these styles, SVG icons are rendered. I did not find any significant differences between the look of SVG icons, but I did not zoom them. For me this is something to check.

Do you have a confirmation that the styles have been blocked? I have the impression that CSP blocks some styles (the "unsafe" ones), not all of them.

Comandeer commented 7 years ago

@fredck according to the specification:

The styles will be blocked unless every policy allows inline style, either implicitly by not specifying a style-src (or default-src) directive, or explicitly, by whitelisting "unsafe-inline", a nonce-source or a hash-source that matches the inline block.

"unsafe" in unsafe-inline refers to the fact that inline styles/script are unsafe.

Actually SVG could be rendered without fill styles as they're just defining color of the graphic, not the graphic itself.

fredck commented 7 years ago

Thanks for the confirmation @Comandeer... I always had this doubt.

jhuesos commented 7 years ago

One of the most important and useful rules when adding a Content Security Policy (CSP) is to disable inline scripts. If we add the unsafe-inline, the effectiveness of any policy will be significantly reduced. So this is not a solution.

In 1-3 years time, when everybody will have a Content Security Policy, this will make CKEditor totally unusable. This is the new standard for security in the industry. I have the feeling that this issue is not being considered as important as it should be. And I say this because there have been issues opened for years about this issue and I was kind of hoping that CKEditor v5 would solve these problems since it was my understanding that it was re-written pretty much from scratch.

In our case, in my company, we will be migrating away from CKEditor because we must have a CSP. We don't have another option. Security comes first even over functionality.

wwalc commented 7 years ago

@jhuesos - not sure if you read my post or just quickly scanned it.

One of the most important and useful rules when adding a Content Security Policy (CSP) is to disable inline scripts. If we add the unsafe-inline, the effectiveness of any policy will be significantly reduced. So this is not a solution.

CKEditor 5 does not need unsafe-inline for scripts as I wrote above. The only need so far is unsafe-inline for inline styles, and this is something that both @Comandeer and me pointed out as something worth investigating. It's quite obvious that anything that starts with unsafe- should be avoided. There have been many examples in the past of XSS attacks via CSS, so I completely agree with you.

In 1-3 years time, when everybody will have a Content Security Policy

Agree.

this will make CKEditor totally unusable

This ticket is all about it. We want to support as strict CSP as possible, if you have any comments on https://github.com/ckeditor/ckeditor5-dev/issues/87#issuecomment-285042436 please feel free to add them there.

And I say this because there have been issues opened for years about this issue and I was kind of hoping that CKEditor v5 would solve these problems since it was my understanding that it was re-written pretty much from scratch.

This is exactly what is going to happen. CKEditor 5 will support CSP. I'm sorry if you got confused by our discussion.

In our case, in my company, we will be migrating away from CKEditor because we must have a CSP. We don't have another option. Security comes first even over functionality.

You can safely stay with CKEditor 5 :)

jhuesos commented 7 years ago

@wwalc yes, I misread it. I thought it was proposed to set unsafe-inline for scripts. Apologize. I need to improve my reading skills apparently. You were right, I scanned the message.

For us, unsafe-inline for styles is okay. I will take a look to the linked issue and provide my feedback

erictabet commented 6 years ago

Sorry to bump this year-old discussion but in an environment where unsafe-inline for styles is not okay, would you have any workaround or suggestions for us to be able to use CKEditor 5, please?

Reinmar commented 6 years ago

Did you check, @erictabet, that setting style-src: 'self' breaks something? According to @wwalc's comment it may work fine:

'unsafe-inline' unfortunately it is required due to SVG icons - at least when merged into app.js they have inline styles, e.g. style="fill:#3a3a3a;". What's funny is that even when CSP blocks these styles, SVG icons are rendered. I did not find any significant differences between the look of SVG icons, but I did not zoom them. For me this is something to check.

erictabet commented 6 years ago

@Reinmar thanks for speedy reply. Yes we did and it does break the editor, e.g. in Chrome where it says "Refused to apply inline style" from addStyles.js line 322.

Reinmar commented 6 years ago

Out of curiosity, I removed all fill="#[^"]+" attributes from our icons and this is how the editor looks now:

image

Could we live without this attribute, @dkonopka @oleq?

BTW, it looks like we need to get back to https://github.com/ckeditor/ckeditor5-dev/issues/87.

Reinmar commented 6 years ago

@Reinmar thanks for speedy reply. Yes we did and it does break the editor, e.g. in Chrome where it says "Refused to apply inline style" from addStyles.js line 322.

addStyles.js ;| Could you post a screenshot of this error?

erictabet commented 6 years ago

ckeditor

Reinmar commented 6 years ago

OK, it comes from webpack's style-loader which we use by default:

image

I tested this by adding this to one of the manual tests:

<head>
    <meta http-equiv="Content-Security-Policy" content="default-src http://localhost:8125; child-src 'none'; object-src 'none'; style-src 'self'; font-src data; img-src 'self' data">
</head>

I guess that to get compatible with the style-src 'self' rule we need to configure webpack export CSS to a separate file. It's describe how to do that here, @erictabet: https://docs.ckeditor.com/ckeditor5/latest/builds/guides/integration/advanced-setup.html#option-extracting-css

I wonder if this is going to remove all errors... Checking this now.

Reinmar commented 6 years ago

Yup, everything seems to work fine when webpack is configured to extract styles. If I haven't misconfigured something (no unsafe-inline needed) this screenshot proves it:

image

erictabet commented 6 years ago

Ok very well. Thanks a lot, We'll go through the document to rebuild/extract CSS (unless of course there is a build we could download with CSS extracted out...)

oleq commented 6 years ago

Could we live without this attribute, @dkonopka @oleq?

This is odd. I remove those fill attributes and it works for me.

image

Reinmar commented 6 years ago

In normal editors too? Maybe the toolbar in this manual test is styled in a certain way. Although, it's more probable that I did a mistake when testing it.

So, if we can remove these attributes, I'd do that. Although, to close this ticket we need to make sure we're testing the editor with a proper CSP rules. We have two options:

Reinmar commented 5 years ago

I'd like to finish this effort. What we're missing:

dkonopka commented 5 years ago

☝️ fill attributes in svg files are removed - See: https://github.com/ckeditor/ckeditor5-theme-lark/pull/207

chandanch commented 3 years ago

I'm getting this error: Refused to execute inline event handler because it violates the following Content Security Policy directive My current script src values:

'unsafe-hashes' 'nonce-EDNnf03nceIOfn39fn3e9h3sdfa' 'nonce-EDNnf03nceIOfn39fn3e9h3sdrtyr' 'self' https://*.intercom.io https://*.intercomcdn.com https://*.walkme.com http://*.cloudflare.com 

react-ckeditor-component version: 1.1.0

A workaround would be to set unsafe-inline within script-src but that would the react-app under some risk.

Any other way to mitigate this issue?

oleq commented 3 years ago

@chandanch react-ckeditor-component is a 3rd-party React integration and unfortunately, we cannot help you out (by the way, check out the official integration maintained by the CKEditor 5 team). I'd suggest contacting the maintainer of the package for help.

fliespl commented 3 years ago

Ok very well. Thanks a lot, We'll go through the document to rebuild/extract CSS (unless of course there is a build we could download with CSS extracted out...)

Did you ever found such build?

@erictabet

erictabet commented 3 years ago

Hello @fliespl , Nope. I recall someone from my team here used the doc to extract the CSS at the time. Then we had our issue resolved in the following update.

fliespl commented 3 years ago

@erictabet I am not even sure if CSS extraction is even needed now... I have just built ckeditor5 (without change - normal symfony encore build) and it seems to be working with Strict CSP out of the box - probably am missing something :)