Add hashed secure compare

ext/openssl/ossl.c

@@ -606,29 +606,17 @@ static void Init_ossl_locks(void)
 
 /*
  * call-seq:
- *   OpenSSL.secure_compare(string, string) -> boolean
+ *   OpenSSL.fixed_length_secure_compare(string, string) -> boolean
  *
- * Constant time memory comparison. Inputs must be of equal length, otherwise
- * an error is raised since timing attacks could leak the length of a
- * secret.
+ * Constant time memory comparison for fixed length strings, such as results
+ * of HMAC calculations.
  *
- * Returns +true+ if the strings are identical, +false+ otherwise.
- *
- * For securely comparing user input, it's recommended to use hashing and
- * regularly compare after to prevent an unlikely false positive due to a
- * collision.
- *
- *   user_input = "..."
- *   secret = "..."
- *   hashed_input = OpenSSL::Digest::SHA256.digest(user_input)
- *   hashed_secret = OpenSSL::Digest::SHA256.digest(secret)
- *   OpenSSL.secure_compare(hashed_input, hashed_secret) && user_input == secret
- *
- * Be aware that timing attacks against the hash functions may reveal the
- * length of the secret.
+ * Returns +true+ if the strings are identical, +false+ if they are of the same
+ * length but not identical. If the length is different, +ArgumentError+ is
+ * raised.
  */
 static VALUE
-ossl_crypto_secure_compare(VALUE dummy, VALUE str1, VALUE str2)
+ossl_crypto_fixed_length_secure_compare(VALUE dummy, VALUE str1, VALUE str2)
 {
     const unsigned char *p1 = (const unsigned char *)StringValuePtr(str1);
     const unsigned char *p2 = (const unsigned char *)StringValuePtr(str2);
@@ -1166,7 +1154,7 @@ Init_openssl(void)
      */
     mOSSL = rb_define_module("OpenSSL");
     rb_global_variable(&mOSSL);
-    rb_define_singleton_method(mOSSL, "secure_compare", ossl_crypto_secure_compare, 2);
+    rb_define_singleton_method(mOSSL, "fixed_length_secure_compare", ossl_crypto_fixed_length_secure_compare, 2);
 
     /*
      * OpenSSL ruby extension version

lib/openssl.rb

@@ -20,3 +20,17 @@
 require 'openssl/x509'
 require 'openssl/ssl'
 require 'openssl/pkcs5'
+
+module OpenSSL
+  # call-seq:
+  #   OpenSSL.secure_compare(string, string) -> boolean
+  #
+  # Constant time memory comparison. Inputs are hashed using SHA-256 to mask
+  # the length of the secret. Returns +true+ if the strings are identical,
+  # +false+ otherwise.
+  def self.secure_compare(a, b)
+    hashed_a = OpenSSL::Digest::SHA256.digest(a)
+    hashed_b = OpenSSL::Digest::SHA256.digest(b)
+    OpenSSL.fixed_length_secure_compare(hashed_a, hashed_b) && a == b
+  end
+end

test/test_ossl.rb

@@ -6,28 +6,45 @@
 if defined?(OpenSSL)
 
 class OpenSSL::OSSL < OpenSSL::SSLTestCase
-  def test_secure_compare
-    assert_raises(ArgumentError) { OpenSSL.secure_compare("aaa", "a") }
-    assert_raises(ArgumentError) { OpenSSL.secure_compare("aaa", "aa") }
+  def test_fixed_length_secure_compare
+    assert_raises(ArgumentError) { OpenSSL.fixed_length_secure_compare("aaa", "a") }
+    assert_raises(ArgumentError) { OpenSSL.fixed_length_secure_compare("aaa", "aa") }
 
-    assert OpenSSL.secure_compare("aaa", "aaa")
-    assert OpenSSL.secure_compare(
+    assert OpenSSL.fixed_length_secure_compare("aaa", "aaa")
+    assert OpenSSL.fixed_length_secure_compare(
       OpenSSL::Digest::SHA256.digest("aaa"), OpenSSL::Digest::SHA256.digest("aaa")
     )
 
-    assert_raises(ArgumentError) { OpenSSL.secure_compare("aaa", "aaaa") }
+    assert_raises(ArgumentError) { OpenSSL.fixed_length_secure_compare("aaa", "aaaa") }
+    refute OpenSSL.fixed_length_secure_compare("aaa", "baa")
+    refute OpenSSL.fixed_length_secure_compare("aaa", "aba")
+    refute OpenSSL.fixed_length_secure_compare("aaa", "aab")
+    assert_raises(ArgumentError) { OpenSSL.fixed_length_secure_compare("aaa", "aaab") }
+    assert_raises(ArgumentError) { OpenSSL.fixed_length_secure_compare("aaa", "b") }
+    assert_raises(ArgumentError) { OpenSSL.fixed_length_secure_compare("aaa", "bb") }
+    refute OpenSSL.fixed_length_secure_compare("aaa", "bbb")
+    assert_raises(ArgumentError) { OpenSSL.fixed_length_secure_compare("aaa", "bbbb") }
+  end
+
+  def test_secure_compare
+    refute OpenSSL.secure_compare("aaa", "a")
+    refute OpenSSL.secure_compare("aaa", "aa")
+
+    assert OpenSSL.secure_compare("aaa", "aaa")
+
+    refute OpenSSL.secure_compare("aaa", "aaaa")
     refute OpenSSL.secure_compare("aaa", "baa")
     refute OpenSSL.secure_compare("aaa", "aba")
     refute OpenSSL.secure_compare("aaa", "aab")
-    assert_raises(ArgumentError) { OpenSSL.secure_compare("aaa", "aaab") }
-    assert_raises(ArgumentError) { OpenSSL.secure_compare("aaa", "b") }
-    assert_raises(ArgumentError) { OpenSSL.secure_compare("aaa", "bb") }
+    refute OpenSSL.secure_compare("aaa", "aaab")
+    refute OpenSSL.secure_compare("aaa", "b")
+    refute OpenSSL.secure_compare("aaa", "bb")
     refute OpenSSL.secure_compare("aaa", "bbb")
-    assert_raises(ArgumentError) { OpenSSL.secure_compare("aaa", "bbbb") }
+    refute OpenSSL.secure_compare("aaa", "bbbb")
   end
 
   def test_memcmp_timing
-    # Ensure using secure_compare takes almost exactly the same amount of time to compare two different strings.
+    # Ensure using fixed_length_secure_compare takes almost exactly the same amount of time to compare two different strings.
     # Regular string comparison will short-circuit on the first non-matching character, failing this test.
     # NOTE: this test may be susceptible to noise if the system running the tests is otherwise under load.
     a = "x" * 512_000
@@ -36,9 +53,9 @@ def test_memcmp_timing
     a = "#{a}x"
 
     n = 10_000
-    a_b_time = Benchmark.measure { n.times { OpenSSL.secure_compare(a, b) } }.real
-    a_c_time = Benchmark.measure { n.times { OpenSSL.secure_compare(a, c) } }.real
-    assert_in_delta(a_b_time, a_c_time, 1, "secure_compare timing test failed")
+    a_b_time = Benchmark.measure { n.times { OpenSSL.fixed_length_secure_compare(a, b) } }.real
+    a_c_time = Benchmark.measure { n.times { OpenSSL.fixed_length_secure_compare(a, c) } }.real
+    assert_in_delta(a_b_time, a_c_time, 1, "fixed_length_secure_compare timing test failed")
   end
 end