Merge pull request #17369 from rails/secure_compare

Secure compare

actionpack/lib/action_controller/metal/request_forgery_protection.rb

@@ -1,5 +1,6 @@
 require 'rack/session/abstract/id'
 require 'action_controller/metal/exceptions'
+require 'active_support/security_utils'
 
 module ActionController #:nodoc:
   class InvalidAuthenticityToken < ActionControllerError #:nodoc:
@@ -305,8 +306,7 @@ def valid_authenticity_token?(session, encoded_masked_token)
       end
 
       def compare_with_real_token(token, session)
-        # Borrow a constant-time comparison from Rack
-        Rack::Utils.secure_compare(token, real_csrf_token(session))
+        ActiveSupport::SecurityUtils.secure_compare(token, real_csrf_token(session))
       end
 
       def real_csrf_token(session)

activesupport/lib/active_support/message_verifier.rb

@@ -1,5 +1,6 @@
 require 'base64'
 require 'active_support/core_ext/object/blank'
+require 'active_support/security_utils'
 
 module ActiveSupport
   # +MessageVerifier+ makes it easy to generate and verify messages which are
@@ -37,7 +38,7 @@ def verify(signed_message)
       raise InvalidSignature if signed_message.blank?
 
       data, digest = signed_message.split("--")
-      if data.present? && digest.present? && secure_compare(digest, generate_digest(data))
+      if data.present? && digest.present? && ActiveSupport::SecurityUtils.secure_compare(digest, generate_digest(data))
         begin
           @serializer.load(::Base64.strict_decode64(data))
         rescue ArgumentError => argument_error
@@ -55,17 +56,6 @@ def generate(value)
     end
 
     private
-      # constant-time comparison algorithm to prevent timing attacks
-      def secure_compare(a, b)
-        return false unless a.bytesize == b.bytesize
-
-        l = a.unpack "C#{a.bytesize}"
-
-        res = 0
-        b.each_byte { |byte| res |= byte ^ l.shift }
-        res == 0
-      end
-
       def generate_digest(data)
         require 'openssl' unless defined?(OpenSSL)
         OpenSSL::HMAC.hexdigest(OpenSSL::Digest.const_get(@digest).new, @secret, data)

activesupport/lib/active_support/security_utils.rb

@@ -0,0 +1,20 @@
+module ActiveSupport
+  module SecurityUtils
+    # Constant time string comparison.
+    #
+    # The values compared should be of fixed length, such as strings
+    # that have already been processed by HMAC. This should not be used
+    # on variable length plaintext strings because it could leak length info
+    # via timing attacks.
+    def secure_compare(a, b)
+      return false unless a.bytesize == b.bytesize
+
+      l = a.unpack "C#{a.bytesize}"
+
+      res = 0
+      b.each_byte { |byte| res |= byte ^ l.shift }
+      res == 0
+    end
+    module_function :secure_compare
+  end
+end

activesupport/test/security_utils_test.rb

@@ -0,0 +1,9 @@
+require 'abstract_unit'
+require 'active_support/security_utils'
+
+class SecurityUtilsTest < ActiveSupport::TestCase
+  def test_secure_compare_should_perform_string_comparison
+    assert ActiveSupport::SecurityUtils.secure_compare('a', 'a')
+    assert !ActiveSupport::SecurityUtils.secure_compare('a', 'b')
+  end
+end