diff --git a/app/controllers/concerns/inline_script_hashes.rb b/app/controllers/concerns/inline_script_hashes.rb index bf2761e..efc7fde 100644 --- a/app/controllers/concerns/inline_script_hashes.rb +++ b/app/controllers/concerns/inline_script_hashes.rb @@ -1,7 +1,8 @@ -# Allows the scripts the importmap helper emits: the inline importmap and its -# entry-point module by content hash, and the origins its URLs point to. The -# hashes change only when the importmap does (a deploy), so cached HTML stays -# valid, unlike a per-request nonce. +# 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 @@ -13,19 +14,27 @@ module InlineScriptHashes 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 | script_origins(importmap) | script_hashes(importmap, ENTRY_POINT_SCRIPT) + policy.directives["script-src"] = (sources - [ "'self'" ]) | script_locations(importmap) | script_hashes(importmap, ENTRY_POINT_SCRIPT) + policy.worker_src service_worker_url(format: :js) end end end private # The importmap is resolved by Rails' asset helpers, so its URLs already - # carry any configured asset host, however it's written: with or without a - # scheme, protocol-relative, a %d set, or a proc. - def script_origins(importmap) - JSON.parse(importmap)["imports"].values.filter_map do |url| - origin = URI.join(request.base_url, url).origin - origin unless origin == request.base_url + # 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; one outside it is allowed exactly. + # + # 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) + directory = url.path[0, url.path.index(assets) + assets.length] if url.path.include?(assets) + "#{url.origin}#{directory || url.path}" end.uniq end diff --git a/config/initializers/content_security_policy.rb b/config/initializers/content_security_policy.rb index e343526..a99e648 100644 --- a/config/initializers/content_security_policy.rb +++ b/config/initializers/content_security_policy.rb @@ -1,6 +1,9 @@ -# Enforcing baseline policy. Scripts run only from this origin; the two inline -# scripts the importmap helper emits are allowed by hash (InlineScriptHashes), -# so there is no per-request nonce and rendered HTML stays cacheable. +# 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. # # Nothing else is restricted: images, media, fonts, styles (including an # account's custom styles) and connections behave as before. diff --git a/config/initializers/uploaded_script_guard.rb b/config/initializers/uploaded_script_guard.rb index 97eb00c..d0badc3 100644 --- a/config/initializers/uploaded_script_guard.rb +++ b/config/initializers/uploaded_script_guard.rb @@ -1,20 +1,18 @@ -# Uploaded files are served from this origin, so under script-src 'self' an -# upload served with any JavaScript MIME type would run as one of our scripts. -# Browsers accept sixteen spellings of that type, and Disk URLs sign the type -# they were minted with, so a list of types to refuse can't cover old links. -# Instead, anything Active Storage serves that isn't plainly media goes out as -# a download, whatever its URL says. A multi-range response keeps its +# 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. +# Only reads are touched: uploading (direct upload JSON, Disk PUT) answers in +# its own types. # -# Accepted on upgrade: a cache that stored an upload under a script type -# before this guard shipped can still serve it as one until the entry expires. -# For Disk URLs that's up to an hour (config/initializers/active_storage.rb). -# Blob proxy URLs are cached publicly and as immutable for a hundred years -# (Rails' http_cache_forever), so a copy a browser or a shared cache took that -# way doesn't expire. Revalidation doesn't help, since a 304 carries no -# content type. +# 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} diff --git a/test/controllers/cached_responses_test.rb b/test/controllers/cached_responses_test.rb index fa420f4..26efc07 100644 --- a/test/controllers/cached_responses_test.rb +++ b/test/controllers/cached_responses_test.rb @@ -29,7 +29,7 @@ class CachedResponsesTest < ActionDispatch::IntegrationTest 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 "script-src 'self' 'sha256-", first + assert_match %r{script-src http://[^/ ]+/assets/ 'sha256-}, first assert_no_match "nonce-", first MessagesController.any_instance.expects(:find_paged_messages).never diff --git a/test/integration/content_security_policy_test.rb b/test/integration/content_security_policy_test.rb index 3f0eac7..67cf66c 100644 --- a/test/integration/content_security_policy_test.rb +++ b/test/integration/content_security_policy_test.rb @@ -16,8 +16,11 @@ class ContentSecurityPolicyTest < ActionDispatch::IntegrationTest assert_match "base-uri 'self'", policy assert_no_match "unsafe-inline", policy assert_no_match "nonce-", policy + assert_equal "http://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, "http://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| @@ -46,15 +49,43 @@ class ContentSecurityPolicyTest < ActionDispatch::IntegrationTest origins = urls.map { URI.join(request.base_url, it).origin }.uniq - [ request.base_url ] assert_not_empty origins, asset_host.inspect - origins.each do |origin| - assert_match expected_origin, origin, asset_host.inspect - assert_includes script_src, origin, asset_host.inspect - end + 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 "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, "http://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 "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 @@ -85,6 +116,32 @@ class ContentSecurityPolicyTest < ActionDispatch::IntegrationTest 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 + def allowed_by_path?(script_src, url) + url = URI(url) + script_src.any? do |source| + source = URI(source == "'self'" ? request.base_url : source) if source == "'self'" || source.match?(%r{\A[a-z]+://}) + 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)