diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 9ad6c7c..b73e592 100644 --- a/app/controllers/application_controller.rb +++ b/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 diff --git a/app/controllers/concerns/inline_script_hashes.rb b/app/controllers/concerns/inline_script_hashes.rb new file mode 100644 index 0000000..4577292 --- /dev/null +++ b/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 diff --git a/config/initializers/content_security_policy.rb b/config/initializers/content_security_policy.rb index 54f47cf..a99e648 100644 --- a/config/initializers/content_security_policy.rb +++ b/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 diff --git a/config/initializers/uploaded_script_guard.rb b/config/initializers/uploaded_script_guard.rb new file mode 100644 index 0000000..d0badc3 --- /dev/null +++ b/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 diff --git a/test/controllers/cached_responses_test.rb b/test/controllers/cached_responses_test.rb index 8968aa0..888994d 100644 --- a/test/controllers/cached_responses_test.rb +++ b/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| diff --git a/test/integration/content_security_policy_test.rb b/test/integration/content_security_policy_test.rb new file mode 100644 index 0000000..7a3aaf6 --- /dev/null +++ b/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 diff --git a/test/middleware/uploaded_script_guard_test.rb b/test/middleware/uploaded_script_guard_test.rb new file mode 100644 index 0000000..7663c3c --- /dev/null +++ b/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