video.js icon indicating copy to clipboard operation
video.js copied to clipboard

Make the JS more CSP friendly

Open udf2457 opened this issue 3 years ago • 11 comments

Description

At present, the js file is not particularly CSP friendly, you receive errors such as:

Content Security Policy: The page's settings blocked the loading of a resource at inline ("default-src").

This is despite having appropriate headers set.

I believe this is the same as an issue I have experienced before with FontAwesome. In that the JS file attempts to make CSS DOM manipulation which violates CSP (unless you use the unsafe flag, which is ill-advised for obvious reasons).

TL;DR, FontAwesome resolved this by providing the option to:

  1. Disable automatic CSS insertion.
  2. Reference the external CSS file explicitly.

(see https://fontawesome.com/v6/docs/web/dig-deeper/security#contentHeader)

They achieve this by:

  1. Adding the option to use a data-auto-add-css="false" flag which disables automatic CSS insertion (e.g. <script src="https://example.com/fontawesome/v6.1.2/js/all.js" data-auto-add-css="false"></script>)
  2. Providing a separate CSS file which can be referenced alongside (e.g. <link href="https://example.com/fontawesome/v6.1.2/css/svg-with-js.css" rel="stylesheet">) i.e. your final code would look like:
      <script src="https://example.com/fontawesome/v6.1.2/js/all.js" data-auto-add-css="false"></script>
      <link href="https://example.com/fontawesome/v6.1.2/css/svg-with-js.css" rel="stylesheet">

I suggest the videojs team consider giving users the similar option (i.e. publish extracted CSS and provide ability to disable JS css insertion)

I have had zero CSP errors with FontAwesome since implementing the above with their library.

Reduced test case

No response

Steps to reproduce

See Main NOte

Errors

No response

What version of Video.js are you using?

7.20.2

Video.js plugins used.

No response

What browser(s) including version(s) does this occur with?

All

What OS(es) and version(s) does this occur with?

All

udf2457 avatar Aug 23 '22 12:08 udf2457

👋 Thanks for opening your first issue here! 👋

If you're reporting a 🐞 bug, please make sure you include steps to reproduce it. We get a lot of issues on this repo, so please be patient and we will get back to you as soon as we can. To help make it easier for us to investigate your issue, please follow the contributing guidelines.

welcome[bot] avatar Aug 23 '22 12:08 welcome[bot]

We do have an option to turn off dynamic style elements by setting window.VIDEOJS_NO_DYNAMIC_STYLE = true. Looks like it's only documented on the skins guide. The dynamic styles are there for better fluid layout and to resize the player based on the intrinsic size of the video. Without it, as long as you size the player properly, it shouldn't be a problem.

As for other CSP issues, it gets a bit more complicated because we need to do things like set blob urls on video elements.

gkatsev avatar Aug 23 '22 15:08 gkatsev

I will try that out and let you know shortly @gkatsev

Also are you aware video.js is not require-trusted-types-for 'script' safe ? (https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/require-trusted-types-for)

Uncaught TypeError: Failed to set the 'innerHTML' property on 'Element': This document requires 'TrustedHTML' assignment.

udf2457 avatar Aug 23 '22 16:08 udf2457

Yeah, there's still a few more uses of innerHTML. Mostly in https://github.com/videojs/video.js/blob/main/src/js/tracks/text-track-settings.js, I believe. No one had the chance to address that yet. Any help would be appreciated.

gkatsev avatar Aug 23 '22 16:08 gkatsev

Thanks @gkatsev , I'm currently chest-deep in web dev at $work, but maybe if I have time after that !

Also, re: window.VIDEOJS_NO_DYNAMIC_STYLE = true that removes some/many CSP errors, but it doesn't prevent your embedded font from trying to load ...

Content Security Policy: The page's settings blocked the loading of a resource at data:application/font-woff;charset=utf-8… downloadable font: font load failed (font-family: "VideoJS" style:normal weight:400 stretch:100 src index:0): content blocked source: data:application/font-woff;charset=utf-

udf2457 avatar Aug 23 '22 16:08 udf2457

The fonts should be in the external CSS file and not set via JS.

gkatsev avatar Aug 23 '22 16:08 gkatsev

Any update on this issue perhaps? Is making the library CSP friendly high on the to-do list?

BasvanH avatar Sep 27 '22 09:09 BasvanH

I'm with @BasvanH here I'm afraid. Its 2022, and CSP friendly is no longer an optional nice to have, similar how having a mobile-friendly responsive website is no longer an optional extra.

Added to that, the major browser developers are increasingly aggressively hardening their security posture. So libraries really need to be ready for the "new normal" where harder stances are taken on CSP etc.

udf2457 avatar Sep 27 '22 10:09 udf2457

Yes, we will be happy to consider specific enhancements to be more compatible with various CSPs. We'll keep this ticket open - the idea of an option to disable dynamic CSS insertion is a good one.

Unfortunately, as @gkatsev said, there are some cases where we will be unable to do anything. For example, worker-src must be set for streaming playback to work.

misteroneill avatar Mar 28 '23 16:03 misteroneill