[WIP] Adds secure_headers & Content-Security-Policy to Classroom

[anglinb]

👋 I was taking a look at Classroom as part of the appsec-review process and, while this PR does not address a specific known vulnerability, adding the secureheaders gem and configuring Content-Security-Policy (CSP) Headers will help mitigate any available client-side attacks and those which may occur in the future.

---

This is a WIP PR b/c I have a few concerns on the best way to integrate secureheaders and handle some issues that came up:

Questions:

• Should we have the CSP config inline in config/application.rb file or should it get broken out into it's own file?
• Are we ok with pinning a fork of the jquery-datetimepicker-rails gem?
  • The latest released version of jquery-datepicker-rails is v2.4.1.0 and it contains an outdated version of jQuery DateTimePicker plugin. This outdated version relies on eval which it's newest release v2.5.4 does not.
  • Options:
    • Explicitly allow unsafe-eval
    • Use a forked version of jquery-datepicker-rails
    • Move to rails-assest.org for management of JS dependencies
• Should we refactor JS inclusion to remove the YouTube script from being included on every page?
  • Right now, the application.js file includes = require_tree ., which pulls in the https://www.youtube.com/iframe_api.
  • This has a minimal performance hit by pulling in all these assets on page load but after the initial load they should be cached. However, from a CSP perspective, whitelisting a very minimal set of resources is ideal and for that reason, I have only added the YouTube sources to the CSP Definition on the unauthed home page.
  • Options:
    • Add YouTube sources to the global CSP definition
    • Refactor application.js to not include all the scripts all the time.
    • Use a nonce or sha256 CSP directive to inline the pages.js file on the homepage.

Next Steps:

If ya'll have a sec to answer these questions, I would be happy to fix up this PR. 👍

/cc @gregose

[tarebyte]

• Should we have the CSP config inline in config/application.rb file or should it get broken out into it's own file?

I would like it to be in it's own file

• Are we ok with pinning a fork of the jquery-datetimepicker-rails gem?

Yes, using the forked version is the easiest path ATM so let's go with that one.

• Should we refactor JS inclusion to remove the YouTube script from being included on every page?

Absolutely 😄

[anglinb]

/cc @oreoshake

[oreoshake]

The csp changes look great 👍 but I can't speak to the rubocop changes or whether anything will break 😄.

[anglinb]

@oreoshake Thanks for taking a look! Good catch on the rubocop changes--I think those should match. It was complaining about the quotes for self in %w['self'] so I tried to disable PercentStringArray

[houndci-bot]

Missing magic comment # frozen_string_literal: true.

[tarebyte]

Thanks @anglinb!!

[anglinb]

@tarebyte np! Just a heads up: There is a CSP error on pretty much every page b/c the pages.js file gets included by the application.js file's require_tree . directive. This will spam the console but otherwise has no effect.

I punted a bit on refactoring js importing b/c its kinda out of scope of just adding CSP so if you're cool with having this error around in the console, then feel free to merge. If not I can try and add a workaround for now. Lmk 😄

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[anglinb]

👋 Friendly bump

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[BenEmdon]

@srinjoym this sounds right up your alley