diff --git a/CHANGELOG.md b/CHANGELOG.md index 75d28dd..e3ef2f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ ## Ruby CSS Parser CHANGELOG ### Unreleased +* `Parser#load_uri!` accepts an `integrity:` option (Subresource Integrity, https://www.w3.org/TR/SRI/) to verify a fetched remote stylesheet before it is parsed ### Version 3.0.0 * Harden read_remote_file, use `allow_local_network: true` and `allow_file_uris: true` to bypass diff --git a/Gemfile.lock b/Gemfile.lock index f4c369e..077ebe0 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -3,6 +3,7 @@ PATH specs: css_parser (3.0.0) addressable + base64 ssrf_filter (~> 1.5) GEM @@ -11,6 +12,7 @@ GEM addressable (2.8.8) public_suffix (>= 2.0.2, < 8.0) ast (2.4.3) + base64 (0.2.0) benchmark-ips (2.14.0) bump (0.10.0) json (2.18.1) diff --git a/README.md b/README.md index 7a34d29..e31b196 100644 --- a/README.md +++ b/README.md @@ -71,6 +71,39 @@ content_rule.offset #=> 0..21 ``` +# Subresource Integrity + +`Parser#load_uri!` accepts an `integrity:` option that verifies a fetched remote stylesheet +against a [Subresource Integrity](https://www.w3.org/TR/SRI/) value before it's parsed -- the +same value an HTML `` attribute carries. + +```Ruby +parser.load_uri!( + 'http://example.com/styles/style.css', + integrity: 'sha384-oqVuAfXRKap7fdgcCY5uykM6+R9GqQ8K/uxy9rx7HNQlGYl1kPzQho1wx4JwY8wC' +) +``` + +When the fetched body doesn't match, `CssParser::IntegrityError` (a subclass of +`CssParser::RemoteFileError`, so existing `rescue RemoteFileError` code is unaffected) is +raised if `io_exceptions` is enabled, or nothing is loaded otherwise. + +`integrity:` also accepts several space-separated values, exactly like the HTML attribute +does. When more than one hash algorithm is present, only the strongest one is checked +(sha512 > sha384 > sha256) and every weaker value is ignored; multiple values for that same +strongest algorithm are treated as alternatives -- matching any one of them is enough (useful +during a stylesheet rotation, when a CDN may still serve the old version for a while): + +```Ruby +parser.load_uri!( + 'http://example.com/styles/style.css', + integrity: 'sha256-Br6tO8uuFyBAw2O0eUNdXVyuS/POLb5jpHxXaxIq6Q0= sha384-0gCPKBW0n+VzQzZu5gzP+YMxy9QTLyn1y/O/TMvLTpVajzRKAx6d7TiPB5W7DnDn' +) +# only the sha384 value is actually checked here; the sha256 one is present +# (e.g. for browsers/tools that only understand sha256) but ignored by this +# library since a stronger algorithm is also listed. +``` + # Testing ```Bash diff --git a/css_parser.gemspec b/css_parser.gemspec index ef1eb0c..2d0697b 100644 --- a/css_parser.gemspec +++ b/css_parser.gemspec @@ -19,5 +19,6 @@ Gem::Specification.new name, CssParser::VERSION do |s| s.metadata['rubygems_mfa_required'] = 'true' s.add_dependency 'addressable' + s.add_dependency 'base64' s.add_dependency 'ssrf_filter', '~> 1.5' end diff --git a/lib/css_parser/parser.rb b/lib/css_parser/parser.rb index c3289f8..ca5161b 100644 --- a/lib/css_parser/parser.rb +++ b/lib/css_parser/parser.rb @@ -1,11 +1,20 @@ # frozen_string_literal: true require 'strscan' +require 'digest' +require 'base64' module CssParser # Exception class used for any errors encountered while downloading remote files. class RemoteFileError < IOError; end + # Exception class used when a fetched remote file fails Subresource Integrity + # verification (see the `:integrity` option on `Parser#load_uri!`). A subclass of + # `RemoteFileError` so existing `rescue RemoteFileError` callers are unaffected; + # callers that want to distinguish an integrity failure from other fetch failures + # (404, SSRF rejection, timeout, etc.) can rescue this class specifically. + class IntegrityError < RemoteFileError; end + # Exception class used if a request is made to load a CSS file more than once. class CircularReferenceError < StandardError; end @@ -19,6 +28,9 @@ class CircularReferenceError < StandardError; end # [io_exceptions] Throw an exception if a link can not be found. Boolean, default is true. # [allow_local_network] Permit http(s) fetches against loopback / private / link-local / cloud-metadata addresses. Boolean, default is false. When false (the default), outbound HTTP requests are routed through ssrf_filter, which resolves the host and rejects unsafe IP ranges. Set to true only when the destination is known to be safe (e.g. local fixture servers in tests). Independent of allow_file_uris. # [allow_file_uris] Permit file:// URIs via load_uri!. Boolean, default is false. When false (the default), a caller that passes a file:// URI to load_uri! — directly or via a CSS @import resolved against a file:// base_uri — is refused, closing the local-file-disclosure vector when the URI is influenced by user input. load_file! is unaffected: it is the explicit local-file API and takes a caller-supplied path. Independent of allow_local_network. + # + # load_uri! also accepts a per-call :integrity option (see its documentation) for + # verifying a remote stylesheet against a Subresource Integrity value before it is parsed. class Parser USER_AGENT = "Ruby CSS Parser/#{CssParser::VERSION} (https://github.com/premailer/css_parser)".freeze RULESET_TOKENIZER_RX = /\s+|\\{2,}|\\?[{}\s"]|[()]|.[^\s"{}()\\]*/.freeze @@ -37,6 +49,18 @@ class Parser # was GHSA-9pmc-p236-855h. REMOTE_ALLOWED_SCHEMES = %w[http https].freeze + # Subresource Integrity hash algorithms this library can verify, mapped to their + # Digest class, ordered strongest first. A single structure (rather than a + # separate priority list and digest-class lookup) so the two can't drift out of + # sync. Mirrors the SRI spec's "agility" rule (https://www.w3.org/TR/SRI/#agility): + # when a caller-supplied `integrity` value lists more than one algorithm, only + # the strongest one present is checked. + INTEGRITY_ALGORITHMS = { + 'sha512' => Digest::SHA512, + 'sha384' => Digest::SHA384, + 'sha256' => Digest::SHA256 + }.freeze + # Array of CSS files that have been loaded. attr_reader :loaded_uris @@ -492,7 +516,13 @@ def parse_block_into_rule_sets!(block, options = {}) # :nodoc: # # You can also pass in file://test.css # - # See add_block! for options. + # See add_block! for options. In addition to those, :integrity accepts a + # Subresource Integrity value (https://www.w3.org/TR/SRI/) -- e.g. the value of an + # HTML attribute -- and, for http(s) URIs, verifies the + # fetched response body against it before the CSS is parsed. When the digest does not + # match, CssParser::IntegrityError (a subclass of RemoteFileError) is + # raised if io_exceptions is enabled, otherwise nothing is loaded. Ignored for + # file:// URIs. # # Deprecated: originally accepted three params: `uri`, `base_uri` and `media_types` def load_uri!(uri, options = {}, deprecated = nil) @@ -538,7 +568,7 @@ def load_uri!(uri, options = {}, deprecated = nil) end read_local_file(uri) else - src_and_charset, = read_remote_file(uri) # skip charset + src_and_charset, = read_remote_file(uri, integrity: opts[:integrity]) # skip charset src_and_charset end @@ -677,10 +707,14 @@ def read_local_file(uri) # :nodoc: # is still validated on every redirect hop, so cross-scheme # redirect to `file://` (the original GHSA-9pmc-p236-855h sink) # remains closed even on this opt-in path. + # + # `integrity:`, when given, is verified against the raw response body + # (before charset decoding, matching Subresource Integrity semantics) + # -- see `integrity_matches?` and `load_uri!`'s documentation. #-- # TODO: add option to fail silently or throw and exception on a 404 #++ - def read_remote_file(uri) # :nodoc: + def read_remote_file(uri, integrity: nil) # :nodoc: uri = Addressable::URI.parse(uri.to_s) unless circular_reference_check(uri.to_s) @@ -711,11 +745,27 @@ def read_remote_file(uri) # :nodoc: return '', nil end + if integrity && !integrity_matches?(res.body, integrity) + raise IntegrityError, uri.to_s if @options[:io_exceptions] + + return nil, nil + end + charset = res.respond_to?(:charset) ? res.encoding : 'utf-8' src = res.body src.encode!('UTF-8', charset) if charset [src, charset] + rescue IntegrityError + # Let the IntegrityError raised above propagate with its specific + # class intact, rather than being downgraded to a generic + # RemoteFileError by the catch-all below. Other RemoteFileErrors + # raised within this method (e.g. from fetch_via_net_http on a + # cross-scheme redirect) are intentionally still caught by the + # catch-all: it discards their (potentially redirect-target-scoped) + # message in favor of this method's own `uri`, which several + # existing tests depend on. + raise rescue raise RemoteFileError, uri.to_s if @options[:io_exceptions] @@ -723,6 +773,29 @@ def read_remote_file(uri) # :nodoc: end end + # Verifies +body+ (raw response bytes, not yet charset-decoded) against a + # Subresource Integrity value -- a single `-` token, or + # several whitespace-separated tokens (https://www.w3.org/TR/SRI/#the-integrity-attribute). + # Tokens using an algorithm this library doesn't recognize are ignored; per the spec's + # "agility" rule, when multiple recognized algorithms are present only the strongest one + # is checked. A value containing no recognized algorithm is treated as unverifiable and + # matches by default, rather than failing every fetch whenever a caller passes a stronger + # or newer algorithm than this library currently supports. + def integrity_matches?(body, integrity) # :nodoc: + candidates = integrity.to_s.split.filter_map do |token| + algorithm, value = token.split('-', 2) + [algorithm, value] if algorithm && value && INTEGRITY_ALGORITHMS.key?(algorithm) + end + return true if candidates.empty? + + algorithms_present = candidates.map(&:first) + algorithm = INTEGRITY_ALGORITHMS.each_key.find { |a| algorithms_present.include?(a) } + expected_values = candidates.select { |a, _v| a == algorithm }.map { |_a, v| v } + digest_class = INTEGRITY_ALGORITHMS.fetch(algorithm) + + expected_values.include?(Base64.strict_encode64(digest_class.digest(body))) + end + # Net::HTTP path used only when `allow_local_network: true`. Validates # the URI scheme on every redirect hop so a `Location: file://...` # cannot be followed even on this opt-in code path. diff --git a/test/test_css_parser_integrity.rb b/test/test_css_parser_integrity.rb new file mode 100644 index 0000000..750aa27 --- /dev/null +++ b/test/test_css_parser_integrity.rb @@ -0,0 +1,147 @@ +# frozen_string_literal: true + +require_relative 'test_helper' + +# Tests for `Parser#load_uri!`'s `:integrity` option, which verifies a +# fetched remote stylesheet against a Subresource Integrity value +# (https://www.w3.org/TR/SRI/) before it is parsed -- the same mechanism +# an HTML `` attribute describes. +class CssParserIntegrityTests < Minitest::Test + include CssParser + include WEBrick + + PORT = 12_011 + + def setup + @www_root = File.expand_path('fixtures', __dir__) + @fixture_file = File.expand_path('fixtures/simple.css', __dir__) + @fixture_body = File.binread(@fixture_file) + @uri_base = "http://127.0.0.1:#{PORT}" + + # `:integrity` verification only applies on the remote-fetch path, so + # these tests use `allow_local_network: true` against a loopback + # fixture server rather than mocking the HTTP layer -- matching the + # approach `test_allow_local_network_opt_in_permits_loopback` uses in + # test_css_parser_ssrf.rb. + @server_thread = Thread.new do + s = WEBrick::HTTPServer.new( + Port: PORT, BindAddress: '127.0.0.1', DocumentRoot: @www_root, + Logger: Log.new(nil, BasicLog::FATAL), AccessLog: [] + ) + begin + s.start + ensure + s.shutdown + end + end + + sleep 1 + end + + def teardown + @server_thread.kill + @server_thread.join(5) + @server_thread = nil + end + + def cp + Parser.new(allow_local_network: true) + end + + def sha(algorithm, body = @fixture_body) + digest_class = {'sha256' => Digest::SHA256, 'sha384' => Digest::SHA384, 'sha512' => Digest::SHA512}.fetch(algorithm) + "#{algorithm}-#{Base64.strict_encode64(digest_class.digest(body))}" + end + + def test_load_uri_without_integrity_option_is_unaffected + cp.load_uri!("#{@uri_base}/simple.css") + # no-op: reaching here without an exception is the assertion. + end + + def test_matching_sha384_integrity_loads_normally + parser = cp + parser.load_uri!("#{@uri_base}/simple.css", integrity: sha('sha384')) + assert_equal 'margin: 0px;', parser.find_by_selector('p').join(' ') + end + + def test_matching_sha256_integrity_loads_normally + parser = cp + parser.load_uri!("#{@uri_base}/simple.css", integrity: sha('sha256')) + assert_equal 'margin: 0px;', parser.find_by_selector('p').join(' ') + end + + def test_matching_sha512_integrity_loads_normally + parser = cp + parser.load_uri!("#{@uri_base}/simple.css", integrity: sha('sha512')) + assert_equal 'margin: 0px;', parser.find_by_selector('p').join(' ') + end + + def test_mismatched_integrity_is_refused + tampered = "#{sha('sha384')[0, 15]}not-the-real-digest-at-all==" + assert_raises(CssParser::IntegrityError) do + cp.load_uri!("#{@uri_base}/simple.css", integrity: tampered) + end + end + + def test_mismatched_integrity_raises_a_subclass_of_remote_file_error + # CssParser::IntegrityError must remain catchable by existing + # `rescue RemoteFileError` callers. + tampered = "#{sha('sha384')[0, 15]}not-the-real-digest-at-all==" + assert_raises(CssParser::RemoteFileError) do + cp.load_uri!("#{@uri_base}/simple.css", integrity: tampered) + end + end + + def test_mismatched_integrity_without_io_exceptions_loads_nothing + parser = Parser.new(allow_local_network: true, io_exceptions: false) + tampered = "#{sha('sha384')[0, 15]}not-the-real-digest-at-all==" + parser.load_uri!("#{@uri_base}/simple.css", integrity: tampered) + assert_empty parser.find_by_selector('p') + end + + def test_strongest_algorithm_wins_when_multiple_present_and_it_fails + # A correct sha256 paired with a wrong sha512 must fail -- the spec's + # "agility" rule means only the strongest present algorithm (sha512 + # here) is authoritative, so a right-but-weaker value must not mask + # a wrong-but-stronger one. + value = "#{sha('sha256')} sha512-#{Base64.strict_encode64('not the real digest')}" + assert_raises(CssParser::IntegrityError) do + cp.load_uri!("#{@uri_base}/simple.css", integrity: value) + end + end + + def test_strongest_algorithm_wins_when_multiple_present_and_it_passes + # Mirror of the above: a wrong sha256 paired with a correct sha512 + # must still pass, since sha512 is the one actually checked. + wrong_sha256 = "sha256-#{Base64.strict_encode64('not the real digest')}" + value = "#{wrong_sha256} #{sha('sha512')}" + parser = cp + parser.load_uri!("#{@uri_base}/simple.css", integrity: value) + assert_equal 'margin: 0px;', parser.find_by_selector('p').join(' ') + end + + def test_multiple_values_for_the_same_algorithm_accepts_any_match + # The spec allows several acceptable digests for the same algorithm + # (e.g. during a stylesheet rotation) -- any match should pass. + other_value = "sha384-#{Base64.strict_encode64('some other build of the file')}" + value = "#{other_value} #{sha('sha384')}" + parser = cp + parser.load_uri!("#{@uri_base}/simple.css", integrity: value) + assert_equal 'margin: 0px;', parser.find_by_selector('p').join(' ') + end + + def test_unrecognized_algorithm_only_value_is_unverifiable_and_passes + # md5 is not in INTEGRITY_ALGORITHMS. A value naming only an + # unsupported algorithm can't be checked either way, so it's treated + # as unverifiable rather than failing every such fetch. + parser = cp + parser.load_uri!("#{@uri_base}/simple.css", integrity: 'md5-deadbeef==') + assert_equal 'margin: 0px;', parser.find_by_selector('p').join(' ') + end + + def test_blank_integrity_option_is_unaffected + parser = cp + parser.load_uri!("#{@uri_base}/simple.css", integrity: '') + assert_equal 'margin: 0px;', parser.find_by_selector('p').join(' ') + end +end