Merge pull request #365 from basecamp/enforce-script-policy

Enforce a script Content-Security-Policy

app/controllers/application_controller.rb

@@ -1,4 +1,4 @@
 class ApplicationController < ActionController::Base
-  include AllowBrowser, Authentication, Authorization, BlockBannedRequests, SetCurrentRequest, SetPlatform, TrackedRoomVisit, VersionHeaders, CachedResponses
+  include AllowBrowser, Authentication, Authorization, BlockBannedRequests, SetCurrentRequest, SetPlatform, TrackedRoomVisit, VersionHeaders, CachedResponses, InlineScriptHashes
   include Turbo::Streams::Broadcasts, Turbo::Streams::StreamName
 end

app/controllers/concerns/inline_script_hashes.rb

@@ -0,0 +1,50 @@
+# Allows the scripts the importmap helper emits and nothing else: the inline
+# importmap and its entry-point module by content hash, and the asset
+# directory its URLs point into. Not the whole origin, because uploads are
+# served from this origin too. The hashes change only when the importmap does
+# (a deploy), so cached HTML stays valid, unlike a per-request nonce.
+module InlineScriptHashes
+  extend ActiveSupport::Concern
+
+  ENTRY_POINT_SCRIPT = %(import "application")
+
+  included do
+    # With the policy turned off there's nothing to add to, so leave the
+    # request without one rather than starting an empty policy
+    content_security_policy if: -> { request.content_security_policy } do |policy|
+      if sources = policy.directives["script-src"]
+        importmap = Rails.application.importmap.to_json(resolver: helpers)
+        policy.directives["script-src"] = (sources - [ "'self'" ]) | script_locations(importmap) | script_hashes(importmap, ENTRY_POINT_SCRIPT)
+        policy.worker_src "#{request.host_with_port}#{service_worker_path(format: :js)}"
+      end
+    end
+  end
+
+  private
+    # The importmap is resolved by Rails' asset helpers, so its URLs already
+    # carry any relative root and asset host, however the host is written:
+    # with or without a scheme, protocol-relative, a %d set, or a proc. Each
+    # is allowed up to its asset directory, the last one in its path, since an
+    # asset host's own path may contain one too; a URL outside it is allowed
+    # exactly.
+    #
+    # This origin's own sources leave out the scheme, so they match the page's
+    # scheme even behind a proxy that terminates TLS without saying so.
+    #
+    # A path only limits the first request: browsers check just the origin
+    # after a redirect. Nothing under the asset directory redirects.
+    def script_locations(importmap)
+      assets = File.join(config.relative_url_root.to_s, Rails.application.config.assets.prefix, "")
+
+      JSON.parse(importmap)["imports"].values.map do |url|
+        url = URI.join(request.base_url, url)
+        origin = url.origin == request.base_url ? request.host_with_port : url.origin
+        directory = url.path[0, url.path.rindex(assets) + assets.length] if url.path.include?(assets)
+        "#{origin}#{directory || url.path}"
+      end.uniq
+    end
+
+    def script_hashes(*scripts)
+      scripts.map { |script| "'sha256-#{Digest::SHA256.base64digest(script)}'" }
+    end
+end

config/initializers/content_security_policy.rb

@@ -1,25 +1,21 @@
-# Be sure to restart your server when you modify this file.
-
-# Define an application-wide content security policy.
-# See the Securing Rails Applications Guide for more information:
-# https://guides.rubyonrails.org/security.html#content-security-policy-header
-
-# Rails.application.configure do
-#   config.content_security_policy do |policy|
-#     policy.default_src :self, :https
-#     policy.font_src    :self, :https, :data
-#     policy.img_src     :self, :https, :data
-#     policy.object_src  :none
-#     policy.script_src  :self, :https
-#     policy.style_src   :self, :https
-#     # Specify URI for violation reports
-#     # policy.report_uri "/csp-violation-report-endpoint"
-#   end
+# Enforcing baseline policy. Application pages narrow script-src from this
+# origin to its asset directory, since uploads are served from this origin
+# too, and allow the two inline scripts the importmap helper emits by hash
+# (InlineScriptHashes). There is no per-request nonce, so rendered HTML stays
+# cacheable. 'self' remains for responses outside ApplicationController, none
+# of which render user content.
 #
-#   # Generate session nonces for permitted importmap and inline scripts
-#   config.content_security_policy_nonce_generator = ->(request) { request.session.id.to_s }
-#   config.content_security_policy_nonce_directives = %w(script-src)
+# Nothing else is restricted: images, media, fonts, styles (including an
+# account's custom styles) and connections behave as before.
 #
-#   # Report violations without enforcing the policy.
-#   # config.content_security_policy_report_only = true
-# end
+# DISABLE_CONTENT_SECURITY_POLICY=true is the escape hatch for an install whose
+# proxy or customization injects scripts the policy would block.
+unless ENV["DISABLE_CONTENT_SECURITY_POLICY"] == "true"
+  Rails.application.configure do
+    config.content_security_policy do |policy|
+      policy.script_src :self
+      policy.object_src :none
+      policy.base_uri   :self
+    end
+  end
+end

config/initializers/uploaded_script_guard.rb

@@ -0,0 +1,34 @@
+# Uploaded files are served from this origin. Application pages only allow
+# scripts from the asset directory (InlineScriptHashes), so an upload can't
+# load as a script whatever type it's served with. This guard is the second
+# layer: browsers accept sixteen spellings of a JavaScript type, and Disk URLs
+# sign the type they were minted with, so instead of refusing a list of types,
+# anything Active Storage serves that isn't plainly media goes out as a
+# download, whatever its URL says. A multi-range response keeps its
+# multipart/byteranges type and boundary, which no browser runs as a script.
+# Only reads are touched: uploading (direct upload JSON, Disk PUT) answers in
+# its own types.
+#
+# A cache that stored an upload under a script type before this shipped can
+# still serve it that way until the entry expires (an hour for Disk URLs, and
+# never for blob proxy URLs, which Rails caches as immutable). The policy's
+# path restriction is what keeps such a copy from running.
+class UploadedScriptGuard
+  SERVABLE = %r{\A(image|video|audio)/|\A(application/pdf|application/octet-stream|text/plain|multipart/byteranges)\z}
+
+  def initialize(app)
+    @app = app
+  end
+
+  def call(env)
+    status, headers, body = @app.call(env)
+
+    if env["REQUEST_METHOD"].in?(%w[ GET HEAD ]) && env["PATH_INFO"].start_with?("#{ActiveStorage.routes_prefix}/") && (type = headers["content-type"])
+      headers["content-type"] = ActiveStorage.binary_content_type unless type.split(";").first.strip.downcase.match?(SERVABLE)
+    end
+
+    [ status, headers, body ]
+  end
+end
+
+Rails.application.config.middleware.use UploadedScriptGuard

test/controllers/cached_responses_test.rb

@@ -26,6 +26,18 @@ class CachedResponsesTest < ActionDispatch::IntegrationTest
     ActionController::Base.perform_caching = @previous_caching
   end
 
+  test "a reused page carries the same enforcing policy, with no nonce" do
+    get room_messages_url(@room)
+    first = response.headers["Content-Security-Policy"]
+    assert_match %r{script-src [^/ ]+/assets/ 'sha256-}, first
+    assert_no_match "nonce-", first
+
+    MessagesController.any_instance.expects(:find_paged_messages).never
+    get room_messages_url(@room)
+    assert_response :success
+    assert_equal first, response.headers["Content-Security-Policy"]
+  end
+
   test "all four read actions reuse completed pages" do
     urls = [ room_url(@room), room_messages_url(@room), user_sidebar_url(:me), searches_url(q: "hello") ]
     urls.each do |url|

test/integration/content_security_policy_test.rb

@@ -0,0 +1,187 @@
+require "test_helper"
+
+class ContentSecurityPolicyTest < ActionDispatch::IntegrationTest
+  PAGE_PATH = "/"
+
+  setup { sign_in :david }
+
+  test "enforcing policy allows exactly the inline scripts the page carries" do
+    get root_url
+    follow_redirect! while response.redirect?
+
+    policy = response.headers["Content-Security-Policy"]
+    assert policy, "expected an enforcing Content-Security-Policy header"
+    assert_nil response.headers["Content-Security-Policy-Report-Only"]
+    assert_match "object-src 'none'", policy
+    assert_match "base-uri 'self'", policy
+    assert_no_match "unsafe-inline", policy
+    assert_no_match "nonce-", policy
+    assert_equal "www.example.com/service-worker.js", policy[/worker-src ([^;]+)/, 1]
+
+    script_src = policy[/script-src ([^;]+)/, 1].split
+    assert_not_includes script_src, "'self'"
+    assert_includes script_src, "www.example.com/assets/"
+    inline = Nokogiri::HTML5(response.body).css("script").reject { |s| s["src"] || s["type"] == "text/template" }
+    assert_operator inline.size, :>=, 2
+    inline.each do |script|
+      assert_includes script_src, "'sha256-#{Digest::SHA256.base64digest(script.text)}'", "inline #{script["type"]} script not allowed by hash"
+    end
+  end
+
+  test "scripts may load from a configured asset host, however it's written" do
+    {
+      "https://cdn.example.test" => %r{\Ahttps://cdn\.example\.test\z},
+      "cdn.example.test" => %r{\Ahttp://cdn\.example\.test\z},
+      "//cdn.example.test" => %r{\Ahttp://cdn\.example\.test\z},
+      "https://assets%d.example.test" => %r{\Ahttps://assets[0-3]\.example\.test\z},
+      ->(source) { "https://one.example.test" } => %r{\Ahttps://one\.example\.test\z},
+      ->(source, request) { "https://#{request.host}.example.test" } => %r{\Ahttps://www\.example\.com\.example\.test\z}
+    }.each do |asset_host, expected_origin|
+      with_asset_host(asset_host) do
+        get PAGE_PATH
+        follow_redirect! while response.redirect?
+        assert_response :success
+
+        script_src = response.headers["Content-Security-Policy"][/script-src ([^;]+)/, 1].split
+        page = Nokogiri::HTML5(response.body)
+        urls = JSON.parse(page.at_css("script[type=importmap]").text)["imports"].values +
+          page.css("link[rel=modulepreload]").map { it["href"] } + page.css("script[src]").map { it["src"] }
+        origins = urls.map { URI.join(request.base_url, it).origin }.uniq - [ request.base_url ]
+
+        assert_not_empty origins, asset_host.inspect
+        origins.each { assert_match expected_origin, it, asset_host.inspect }
+        assert_scripts_allowed_by_path script_src, urls.map { URI.join(request.base_url, it).to_s }, asset_host.inspect
+        assert_includes script_src, "'sha256-#{Digest::SHA256.base64digest(page.at_css("script[type=importmap]").text)}'", asset_host.inspect
+      end
+    end
+  end
+
+  test "an asset host whose path contains the asset directory is limited to the directory it serves" do
+    with_asset_host("https://cdn.example.test/assets") do
+      get PAGE_PATH
+      follow_redirect! while response.redirect?
+
+      script_src = response.headers["Content-Security-Policy"][/script-src ([^;]+)/, 1].split
+      assert_includes script_src, "https://cdn.example.test/assets/assets/"
+      assert_not allowed_by_path?(script_src, "https://cdn.example.test/assets/rails/active_storage/blobs/proxy/x/upload.js")
+    end
+  end
+
+  test "the sign-in page is limited to the asset directory too" do
+    cookies.delete(:session_token)
+    get new_session_url
+    assert_response :success
+
+    script_src = response.headers["Content-Security-Policy"][/script-src ([^;]+)/, 1].split
+    assert_not_includes script_src, "'self'"
+    assert_includes script_src, "www.example.com/assets/"
+  end
+
+  test "scripts are limited to the asset directory under a relative root" do
+    with_relative_url_root("/chat") do
+      get PAGE_PATH
+      follow_redirect! while response.redirect?
+      assert_response :success
+
+      script_src = response.headers["Content-Security-Policy"][/script-src ([^;]+)/, 1].split
+      page = Nokogiri::HTML5(response.body)
+      urls = JSON.parse(page.at_css("script[type=importmap]").text)["imports"].values + page.css("link[rel=modulepreload]").map { it["href"] }
+
+      assert urls.all? { it.start_with?("/chat/assets/") }, urls.inspect
+      assert_includes script_src, "www.example.com/chat/assets/"
+      assert_scripts_allowed_by_path script_src, urls.map { URI.join(request.base_url, it).to_s }
+    end
+  end
+
+  test "this origin's sources keep a non-default port and take the page's scheme" do
+    host! "www.example.com:8443"
+    get PAGE_PATH
+    follow_redirect! while response.redirect?
+
+    policy = response.headers["Content-Security-Policy"]
+    assert_includes policy[/script-src ([^;]+)/, 1].split, "www.example.com:8443/assets/"
+    assert_equal "www.example.com:8443/service-worker.js", policy[/worker-src ([^;]+)/, 1]
+  end
+
+  test "uploads can't load as scripts" do
+    get PAGE_PATH
+    follow_redirect! while response.redirect?
+
+    script_src = response.headers["Content-Security-Policy"][/script-src ([^;]+)/, 1].split
+    blob = ActiveStorage::Blob.create_and_upload!(io: StringIO.new("alert(1)"), filename: "upload.js", content_type: "text/javascript")
+
+    disk_url = ActiveStorage::Current.set(url_options: { protocol: "http", host: "www.example.com" }) { blob.url }
+
+    [ rails_blob_url(blob), rails_blob_url(blob, disposition: :inline), rails_storage_proxy_url(blob), disk_url ].each do |url|
+      assert_not allowed_by_path?(script_src, url), "#{url} allowed by #{script_src.inspect}"
+    end
+  end
+
+  test "with the policy turned off, pages send none" do
+    without_policy do
+      get PAGE_PATH
+      follow_redirect! while response.redirect?
+
+      assert_response :success
+      assert_nil response.headers["Content-Security-Policy"]
+    end
+  end
+
+  test "the escape hatch boots without a policy" do
+    output = IO.popen({ "DISABLE_CONTENT_SECURITY_POLICY" => "true" },
+      [ "bin/rails", "runner", "print Rails.application.config.content_security_policy.inspect" ], chdir: Rails.root, err: File::NULL, &:read)
+
+    assert_equal "nil", output
+  end
+
+  test "uploaded files are guarded on their way out" do
+    assert_includes Rails.application.middleware.map(&:klass), UploadedScriptGuard
+  end
+
+  private
+    # What DISABLE_CONTENT_SECURITY_POLICY=true leaves at boot: no application policy
+    def without_policy
+      previous = Rails.application.env_config.delete("action_dispatch.content_security_policy")
+      yield
+    ensure
+      Rails.application.env_config["action_dispatch.content_security_policy"] = previous
+    end
+
+    # Every page script is allowed by a source with a path, never by a bare origin
+    def assert_scripts_allowed_by_path(script_src, urls, message = nil)
+      assert_empty script_src.grep(%r{\A([a-z]+://)?[^/']+/?\z}), message
+      urls.each { assert allowed_by_path?(script_src, it), "#{it} not allowed by #{script_src.inspect} #{message}" }
+    end
+
+    # CSP source matching on a first request: 'self' and a bare origin match any
+    # path, a path ending in / matches as a prefix, any other path exactly. A
+    # source without a scheme takes the page's.
+    def allowed_by_path?(script_src, url)
+      url = URI(url)
+      script_src.any? do |source|
+        unless source.start_with?("'") && source != "'self'"
+          source = URI(source == "'self'" ? request.base_url : (source.include?("://") ? source : "#{request.scheme}://#{source}"))
+        end
+        source.is_a?(URI) && source.origin == url.origin &&
+          (source.path.empty? || (source.path.end_with?("/") ? url.path.start_with?(source.path) : url.path == source.path))
+      end
+    end
+
+    def with_relative_url_root(root)
+      previous, ApplicationController.config.relative_url_root = ApplicationController.config.relative_url_root, root
+      Rails.application.importmap.send(:clear_cache)
+      yield
+    ensure
+      ApplicationController.config.relative_url_root = previous
+      Rails.application.importmap.send(:clear_cache)
+    end
+
+    def with_asset_host(asset_host)
+      previous, ApplicationController.asset_host = ApplicationController.asset_host, asset_host
+      Rails.application.importmap.send(:clear_cache)
+      yield
+    ensure
+      ApplicationController.asset_host = previous
+      Rails.application.importmap.send(:clear_cache)
+    end
+end

test/middleware/uploaded_script_guard_test.rb

@@ -0,0 +1,42 @@
+require "test_helper"
+
+class UploadedScriptGuardTest < ActiveSupport::TestCase
+  # Every JavaScript MIME type essence in the MIME Sniffing Standard, plus
+  # parameters and casing a signed Disk URL might carry.
+  SCRIPT_TYPES = %w[
+    application/ecmascript application/javascript application/x-ecmascript application/x-javascript
+    text/ecmascript text/javascript text/javascript1.0 text/javascript1.1 text/javascript1.2 text/javascript1.3
+    text/javascript1.4 text/javascript1.5 text/jscript text/livescript text/x-ecmascript text/x-javascript
+  ] + [ "Text/JavaScript; charset=utf-8" ]
+
+  test "uploads never leave with a script type, whatever the URL says" do
+    SCRIPT_TYPES.each do |type|
+      assert_equal ActiveStorage.binary_content_type, served_type(type, "#{ActiveStorage.routes_prefix}/disk/key/probe.js"), type
+    end
+  end
+
+  test "media and downloads keep their type" do
+    %w[ image/png video/mp4 audio/mpeg application/pdf text/plain ].each do |type|
+      assert_equal type, served_type(type, "#{ActiveStorage.routes_prefix}/disk/key/file"), type
+    end
+  end
+
+  test "multi-range responses keep their boundary" do
+    type = "multipart/byteranges; boundary=AaB03x"
+    assert_equal type, served_type(type, "#{ActiveStorage.routes_prefix}/blobs/proxy/key/movie.mp4")
+  end
+
+  test "direct upload responses keep their JSON type" do
+    assert_equal "application/json", served_type("application/json", "#{ActiveStorage.routes_prefix}/direct_uploads", method: "POST")
+  end
+
+  test "application responses outside uploads are untouched" do
+    assert_equal "text/javascript", served_type("text/javascript", "/assets/application-abc123.js")
+  end
+
+  private
+    def served_type(type, path, method: "GET")
+      app = ->(env) { [ 200, { "content-type" => type }, [ "" ] ] }
+      UploadedScriptGuard.new(app).call(Rack::MockRequest.env_for(path, method: method))[1]["content-type"]
+    end
+end