diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e11183c4..dbdb5cad7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/bench.rb b/bench.rb new file mode 100644 index 000000000..5ab918fa7 --- /dev/null +++ b/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 diff --git a/lib/rack/utils.rb b/lib/rack/utils.rb index 7de7a9dff..2b61298ee 100644 --- a/lib/rack/utils.rb +++ b/lib/rack/utils.rb @@ -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