Rewrite signature verification using regexps and StringScanner (#29133)

app/lib/signature_parser.rb

@@ -11,43 +11,30 @@ class SignatureParser
   # In addition, ignore a `Signature ` string prefix that was added by old versions
   # of `node-http-signatures`
 
-  class SignatureParamsParser < Parslet::Parser
+  TOKEN_RE = /[0-9a-zA-Z!#$%&'*+.^_`|~-]+/
-    rule(:token)         { match("[0-9a-zA-Z!#$%&'*+.^_`|~-]").repeat(1).as(:token) }
+  # qdtext and quoted_pair are not exactly according to spec but meh
-    rule(:quoted_string) { str('"') >> (qdtext | quoted_pair).repeat.as(:quoted_string) >> str('"') }
+  QUOTED_STRING_RE = /"([^\\"]|(\\.))*"/
-    # qdtext and quoted_pair are not exactly according to spec but meh
+  PARAM_RE = /(?<key>#{TOKEN_RE})\s*=\s*((?<value>#{TOKEN_RE})|(?<quoted_value>#{QUOTED_STRING_RE}))/
-    rule(:qdtext)        { match('[^\\\\"]') }
-    rule(:quoted_pair)   { str('\\') >> any }
-    rule(:bws)           { match('\s').repeat }
-    rule(:param)         { (token.as(:key) >> bws >> str('=') >> bws >> (token | quoted_string).as(:value)).as(:param) }
-    rule(:comma)         { bws >> str(',') >> bws }
-    # Old versions of node-http-signature add an incorrect "Signature " prefix to the header
-    rule(:buggy_prefix)  { str('Signature ') }
-    rule(:params)        { buggy_prefix.maybe >> (param >> (comma >> param).repeat).as(:params) }
-    root(:params)
-  end
-
-  class SignatureParamsTransformer < Parslet::Transform
-    rule(params: subtree(:param)) do
-      (param.is_a?(Array) ? param : [param]).each_with_object({}) { |(key, value), hash| hash[key] = value }
-    end
-
-    rule(param: { key: simple(:key), value: simple(:val) }) do
-      [key, val]
-    end
-
-    rule(quoted_string: simple(:string)) do
-      string.to_s
-    end
-
-    rule(token: simple(:string)) do
-      string.to_s
-    end
-  end
 
   def self.parse(raw_signature)
-    tree = SignatureParamsParser.new.parse(raw_signature)
+    # Old versions of node-http-signature add an incorrect "Signature " prefix to the header
-    SignatureParamsTransformer.new.apply(tree)
+    raw_signature = raw_signature.delete_prefix('Signature ')
-  rescue Parslet::ParseFailed
+
+    params = {}
+    scanner = StringScanner.new(raw_signature)
+
+    # Use `skip` instead of `scan` as we only care about the subgroups
+    while scanner.skip(PARAM_RE)
+      # This is not actually correct with regards to quoted pairs, but it's consistent
+      # with our previous implementation, and good enough in practice.
+      params[scanner[:key]] = scanner[:value] || scanner[:quoted_value][1...-1]
+
+      scanner.skip(/\s*/)
+      return params if scanner.eos?
+
+      raise ParsingError unless scanner.skip(/\s*,\s*/)
+    end
+
     raise ParsingError
   end
 end

spec/lib/signature_parser_spec.rb

@@ -10,12 +10,12 @@ RSpec.describe SignatureParser do
       let(:header) do
         # This example signature string deliberately mixes uneven spacing
         # and quoting styles to ensure everything is covered
-        'keyId = "https://remote.domain/users/bob#main-key",algorithm=  rsa-sha256 ,  headers="host date digest (request-target)",signature="gmhMjgMROGElJU3fpehV2acD5kMHeELi8EFP2UPHOdQ54H0r55AxIpji+J3lPe+N2qSb/4H1KXIh6f0lRu8TGSsu12OQmg5hiO8VA9flcA/mh9Lpk+qwlQZIPRqKP9xUEfqD+Z7ti5wPzDKrWAUK/7FIqWgcT/mlqB1R1MGkpMFc/q4CIs2OSNiWgA4K+Kp21oQxzC2kUuYob04gAZ7cyE/FTia5t08uv6lVYFdRsn4XNPn1MsHgFBwBMRG79ng3SyhoG4PrqBEi5q2IdLq3zfre/M6He3wlCpyO2VJNdGVoTIzeZ0Zz8jUscPV3XtWUchpGclLGSaKaq/JyNZeiYQ=="' # rubocop:disable Layout/LineLength
+        'keyId = "https://remote.domain/users/bob#main-key,",algorithm=  rsa-sha256 ,  headers="host date digest (request-target)",signature="gmhMjgMROGElJU3fpehV2acD5kMHeELi8EFP2UPHOdQ54H0r55AxIpji+J3lPe+N2qSb/4H1KXIh6f0lRu8TGSsu12OQmg5hiO8VA9flcA/mh9Lpk+qwlQZIPRqKP9xUEfqD+Z7ti5wPzDKrWAUK/7FIqWgcT/mlqB1R1MGkpMFc/q4CIs2OSNiWgA4K+Kp21oQxzC2kUuYob04gAZ7cyE/FTia5t08uv6lVYFdRsn4XNPn1MsHgFBwBMRG79ng3SyhoG4PrqBEi5q2IdLq3zfre/M6He3wlCpyO2VJNdGVoTIzeZ0Zz8jUscPV3XtWUchpGclLGSaKaq/JyNZeiYQ=="' # rubocop:disable Layout/LineLength
       end
 
       it 'correctly parses the header' do
         expect(subject).to eq({
-          'keyId' => 'https://remote.domain/users/bob#main-key',
+          'keyId' => 'https://remote.domain/users/bob#main-key,',
           'algorithm' => 'rsa-sha256',
           'headers' => 'host date digest (request-target)',
           'signature' => 'gmhMjgMROGElJU3fpehV2acD5kMHeELi8EFP2UPHOdQ54H0r55AxIpji+J3lPe+N2qSb/4H1KXIh6f0lRu8TGSsu12OQmg5hiO8VA9flcA/mh9Lpk+qwlQZIPRqKP9xUEfqD+Z7ti5wPzDKrWAUK/7FIqWgcT/mlqB1R1MGkpMFc/q4CIs2OSNiWgA4K+Kp21oQxzC2kUuYob04gAZ7cyE/FTia5t08uv6lVYFdRsn4XNPn1MsHgFBwBMRG79ng3SyhoG4PrqBEi5q2IdLq3zfre/M6He3wlCpyO2VJNdGVoTIzeZ0Zz8jUscPV3XtWUchpGclLGSaKaq/JyNZeiYQ==', # rubocop:disable Layout/LineLength