[ruby-core:64112] [ruby-trunk - Feature #10098] [PATCH] Timing-safe string comparison for OpenSSL::HMAC

[nobu@ruby-lang.org]

Issue #10098 has been updated by Nobuyoshi Nakada.


Seems correct.

I made a benchmark for it:

~~~ruby
# bm-tsafe_eql.rb: [Feature #10098]

require 'benchmark'
a = "x"*1024_000
b = a+"y"
c = "y"+a
a << "x"
n = (ARGV.shift || 100000).to_i
Benchmark.bm(15) {|bm|
  bm.report("a==b") {n.times{a==b}}
  bm.report("a==c") {n.times{a==c}}
  bm.report("a.tsafe_eql?(b)") {n.times{a.tsafe_eql?(b)}}
  bm.report("a.tsafe_eql?(c)") {n.times{a.tsafe_eql?(c)}}
}
~~~

It seems quite slow.

               |      user|    system|     total|       real
---------------|----------|----------|----------|-----------
a==b           |  4.660000|  0.000000|  4.660000|(  4.672629)
a==c           |  0.010000|  0.000000|  0.010000|(  0.007154)
a.tsafe_eql?(b)| 34.330000|  0.020000| 34.350000|( 34.366759)
a.tsafe_eql?(c)| 34.450000|  0.020000| 34.470000|( 34.480267)

With the following patch,

               |      user|    system|     total|       real
---------------|----------|----------|----------|-----------
a==b           |  4.660000|  0.000000|  4.660000|(  4.666683)
a==c           |  0.000000|  0.000000|  0.000000|(  0.006657)
a.tsafe_eql?(b)|  7.660000|  0.010000|  7.670000|(  7.662584)
a.tsafe_eql?(c)|  7.660000|  0.000000|  7.660000|(  7.671189)

~~~ruby
diff --git a/string.c b/string.c
index 5c2852f..90865c0 100644
--- a/string.c
+++ b/string.c
@@ -2504,7 +2504,7 @@ static VALUE
 rb_str_tsafe_eql(VALUE str1, VALUE str2)
 {
     long len, idx;
-    char result;
+    VALUE result;
     const char *buf1, *buf2;
 
     str2 = StringValue(str2);
@@ -2516,7 +2516,13 @@ rb_str_tsafe_eql(VALUE str1, VALUE str2)
     buf2 = RSTRING_PTR(str2);
 
     result = 0;
-    for (idx = 0; idx < len; idx++) {
+    idx = 0;
+    if (UNALIGNED_WORD_ACCESS || !((VALUE)buf1 % sizeof(VALUE)) && !((VALUE)buf2 % sizeof(VALUE))) {
+	for (; idx < len; idx += sizeof(VALUE)) {
+	    result |= *(const VALUE *)(buf1+idx) ^ *(const VALUE *)(buf2+idx);
+	}
+    }
+    for (; idx < len; idx++) {
         result |= buf1[idx] ^ buf2[idx];
     }
 
~~~




----------------------------------------
Feature #10098: [PATCH] Timing-safe string comparison for OpenSSL::HMAC
https://bugs.ruby-lang.org/issues/10098#change-48123

* Author: Matt U
* Status: Open
* Priority: Normal
* Assignee: 
* Category: ext/openssl
* Target version: next minor
----------------------------------------
I could be totally wrong, but it seems the standard library doesn't provide a reliable way of comparing hashes in constant-time.

* The docs for `OpenSSL::HMAC` encourage the use of `Digest#to_s` (see: http://ruby-doc.org/stdlib-2.1.0/libdoc/openssl/rdoc/OpenSSL/HMAC.html#method-c-new )
* Ruby's string comparison uses memcmp, which isn't timing safe (see: http://rxr.whitequark.org/mri/source/string.c#2382 )

With this patch I propose to add an additional method, `OpenSSL::HMAC#verify`, which takes a binary string with a digest and compares it against the computed hash.


---Files--------------------------------
hmac-timing.patch (2.5 KB)
hmac-timing.patch (2.48 KB)
tsafe_eql.patch (2.48 KB)


-- 
https://bugs.ruby-lang.org/