Use activesupport implementation and add tests

Author: hieuk09

lib/slack/events/request.rb

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-require 'openssl'
+require 'slack/utils/security'
 
 module Slack
   module Events
@@ -62,7 +62,7 @@ def valid?
         signature_basestring = [version, timestamp, body].join(':')
         hex_hash = OpenSSL::HMAC.hexdigest(digest, signing_secret, signature_basestring)
         computed_signature = [version, hex_hash].join('=')
-        secure_compare(computed_signature, signature)
+        Utils::Security.secure_compare(computed_signature, signature)
       end
 
       # Validates the request signature and its expiration.
@@ -72,14 +72,6 @@ def verify!
 
         true
       end
-
-      private
-
-      def secure_compare(computed_signature, signature)
-        return false if computed_signature.bytesize != signature.bytesize
-
-        OpenSSL.fixed_length_secure_compare(computed_signature, signature)
-      end
     end
   end
 end

lib/slack/utils/security.rb

@@ -0,0 +1,45 @@
+# frozen_string_literal: true
+require 'openssl'
+
+# This module is copied from activesupport
+# https://github.com/rails/rails/blob/3235827585d87661942c91bc81f64f56d710f0b2/activesupport/lib/active_support/security_utils.rb
+module Slack
+  module Utils
+    # rubocop:disable Naming/MethodParameterName
+    module Security
+      # Constant time string comparison, for fixed length strings.
+      #
+      # The values compared should be of fixed length, such as strings
+      # that have already been processed by HMAC. Raises in case of length mismatch.
+
+      if defined?(OpenSSL.fixed_length_secure_compare)
+        def fixed_length_secure_compare(a, b)
+          OpenSSL.fixed_length_secure_compare(a, b)
+        end
+      else
+        def fixed_length_secure_compare(a, b)
+          raise ArgumentError, 'string length mismatch.' unless a.bytesize == b.bytesize
+
+          l = a.unpack "C#{a.bytesize}"
+
+          res = 0
+          b.each_byte { |byte| res |= byte ^ l.shift }
+          res.zero?
+        end
+      end
+      module_function :fixed_length_secure_compare
+
+      # Secure string comparison for strings of variable length.
+      #
+      # While a timing attack would not be able to discern the content of
+      # a secret compared via secure_compare, it is possible to determine
+      # the secret length. This should be considered when using secure_compare
+      # to compare weak, short secrets to user input.
+      def secure_compare(a, b)
+        a.bytesize == b.bytesize && fixed_length_secure_compare(a, b)
+      end
+      module_function :secure_compare
+    end
+    # rubocop:enable Naming/MethodParameterName
+  end
+end

spec/slack/utils/security_spec.rb

@@ -0,0 +1,29 @@
+# frozen_string_literal: true
+
+require 'slack/utils/security'
+
+RSpec.describe Slack::Utils::Security do
+  describe '.secure_compare' do
+    it 'performs string comparison correctly' do
+      expect(described_class.secure_compare('a', 'a')).to be true
+      expect(described_class.secure_compare('a', 'b')).to be false
+    end
+
+    it 'returns false on bytesize mismatch' do
+      expect(described_class.secure_compare('a', "\u{ff41}")).to be false
+    end
+  end
+
+  describe '.fixed_length_secure_compare' do
+    it 'performs string comparison correctly' do
+      expect(described_class.fixed_length_secure_compare('a', 'a')).to be true
+      expect(described_class.fixed_length_secure_compare('a', 'b')).to be false
+    end
+
+    it 'raises ArgumentError on length mismatch' do
+      expect do
+        described_class.fixed_length_secure_compare('a', 'ab')
+      end.to raise_error(ArgumentError, 'inputs must be of equal length')
+    end
+  end
+end