From: cremno@...
Date: 2014-07-29T19:10:55+00:00
Subject: [ruby-core:64121] [ruby-trunk - Feature #10098] [PATCH] Timing-safe string comparison for OpenSSL::HMAC

Issue #10098 has been updated by cremno phobia.


I don't like the proposed method name.

`tsafe`? It should be `timingsafe`. Saving some keystrokes isn't worth it to have a less obvious name. Even C programmers chose a longer name!

I'm also not happy about `eql?` either. Then people expect it to work like `String#eql?`. Same argument requirements/conversion, same result, and it's even mentioned in the proposed docs (���similarly��� is vague)! But it doesn't. For example:

```ruby
a = "\xFF".force_encoding(Encoding::CP437)
b = "\xFF".force_encoding(Encoding::CP850)
p a.eql?(b)  # => false
p a.tsafe_eql?(b)  # => true
```

If something is going to be added to String, then encodings need to be considered. Or:

```ruby
require 'fiddle'
a = "\xFF"
(b = Fiddle::Pointer.malloc(1))[0] = 0xff
p a.eql?(b)  # => false
p a.tsafe_eql?(b)  # => true
```


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

* 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)
tsafe_inline.patch (3.51 KB)


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