Skip to content

Commit

Permalink
Use faster OpenSSL secure_compare if available
Browse files Browse the repository at this point in the history
Comparing a 16 byte string:
openssl_secure_compare:  9397508.4 i/s
   rack_secure_compare:   515938.0 i/s - 18.21x  (± 0.00) slower
  • Loading branch information
bdewater committed Oct 9, 2020
1 parent 5ce5b2c commit 391ae79
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 6 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -19,6 +19,7 @@ All notable changes to this project will be documented in this file. For info on
- `Rack::Request#[]` and `#[]=` now warn even in non-verbose mode. ([#1277](https://github.com/rack/rack/issues/1277), [@jeremyevans](https://github.com/jeremyevans))
- Decrease default allowed parameter recursion level from 100 to 32. ([#1640](https://github.com/rack/rack/issues/1640), [@jeremyevans](https://github.com/jeremyevans))
- Attempting to parse a multipart response with an empty body now raises Rack::Multipart::EmptyContentError. ([#1603](https://github.com/rack/rack/issues/1603), [@jeremyevans](https://github.com/jeremyevans))
- `Rack::Utils.secure_compare` uses OpenSSL's faster implementation if available. ([#1711](https://github.com/rack/rack/pull/1711), [@bdewater](https://github.com/bdewater))

### Fixed

Expand Down
42 changes: 42 additions & 0 deletions bench.rb
@@ -0,0 +1,42 @@
# frozen_string_literal: true

require "bundler/inline"

gemfile(true) do
source "https://rubygems.org"
gem "benchmark-ips"
gem "openssl", "~> 2.2"
end

require 'benchmark/ips'
require 'openssl'

def rack_secure_compare(a, b)
return false unless a.bytesize == b.bytesize

l = a.unpack("C*")

r, i = 0, -1
b.each_byte { |v| r |= v ^ l[i += 1] }
r == 0
end

def openssl_secure_compare(a, b)
return false unless a.bytesize == b.bytesize

OpenSSL.fixed_length_secure_compare(a, b)
end

Benchmark.ips do |x|
string = "acbdefgh12345678"
x.report("rack_secure_compare") do
rack_secure_compare(string, string)
end

x.report("openssl_secure_compare") do
openssl_secure_compare(string, string)
end


x.compare!
end
20 changes: 14 additions & 6 deletions lib/rack/utils.rb
Expand Up @@ -374,14 +374,22 @@ def get_byte_ranges(http_range, size)
# 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
if defined?(OpenSSL.fixed_length_secure_compare)
def secure_compare(a, b)
return false unless a.bytesize == b.bytesize

l = a.unpack("C*")
OpenSSL.fixed_length_secure_compare(a, b)
end
else
def secure_compare(a, b)
return false unless a.bytesize == b.bytesize

r, i = 0, -1
b.each_byte { |v| r |= v ^ l[i += 1] }
r == 0
l = a.unpack("C*")

r, i = 0, -1
b.each_byte { |v| r |= v ^ l[i += 1] }
r == 0
end
end

# Context allows the use of a compatible middleware at different points
Expand Down

0 comments on commit 391ae79

Please sign in to comment.