add constant time comparison for signatures (#119)

googleapis · Oct 10, 2018 · ee67b7f · ee67b7f
1 parent f0ac641
commit ee67b7f
Show file tree

Hide file tree

Showing 5 changed files with 40 additions and 30 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,6 @@
+## 0.11.0 (2018-10-08)
+* Add constant time comparison for oauth signatures.
+
 ## 0.10.0 (2018-09-21)
 * Add UnexpectedStatusError class for http status errors that are not handled.
 

diff --git a/lib/signet/oauth_1/server.rb b/lib/signet/oauth_1/server.rb
@@ -13,7 +13,6 @@
 #    limitations under the License.
 #
 require 'faraday'
-
 require 'stringio'
 require 'addressable/uri'
 require 'signet'
@@ -57,6 +56,13 @@ def initialize(options={})
            instance_variable_set("@#{attr}", options[attr])
         end
       end
+
+      # Constant time string comparison.
+      def safe_equals?(a, b)
+        check = a.bytesize ^ b.bytesize
+        a.bytes.zip(b.bytes) { |x, y| check |= x ^ y.to_i }
+        check == 0
+      end
 
       ##
       # Determine if the supplied nonce/timestamp pair is valid by calling
@@ -285,7 +291,7 @@ def authenticate_temporary_credential_request(options={})
           client_credential_secret,
           nil
         )
-        if(computed_signature == auth_hash['oauth_signature'])
+        if safe_equals?(computed_signature, auth_hash['oauth_signature'])
           if(auth_hash.fetch('oauth_callback', 'oob').empty?)
             'oob'
           else
@@ -363,7 +369,7 @@ def authenticate_token_credential_request(options={})
           temporary_credential.secret
         )
 
-        if(computed_signature == auth_hash['oauth_signature'])
+        if safe_equals?(computed_signature, auth_hash['oauth_signature'])
           {:client_credential=>client_credential,
             :temporary_credential=>temporary_credential,
             :realm=>auth_hash['realm']
@@ -490,7 +496,7 @@ def authenticate_resource_request(options={})
           token_credential_secret
         )
 
-        if(computed_signature == auth_hash['oauth_signature'])
+        if safe_equals?(computed_signature, auth_hash['oauth_signature'])
           {:client_credential=>client_credential,
            :token_credential=>token_credential,
            :realm=>auth_hash['realm']

diff --git a/lib/signet/oauth_2.rb b/lib/signet/oauth_2.rb
@@ -142,7 +142,7 @@ def self.generate_bearer_authorization_header(
     # @return [String] The authorization URI to redirect the user to.
     def self.generate_authorization_uri(authorization_uri, parameters={})
       for key, value in parameters
-        parameters.delete(key) if value == nil
+        parameters.delete(key) if value.nil?
       end
       parsed_uri = Addressable::URI.parse(authorization_uri).dup
       query_values = parsed_uri.query_values || {}

diff --git a/lib/signet/oauth_2/client.rb b/lib/signet/oauth_2/client.rb
@@ -89,7 +89,7 @@ class Client
       #   )
       #
       # @see Signet::OAuth2::Client#update!
-      def initialize(options={})
+      def initialize options={}
         @authorization_uri    = nil
         @token_credential_uri = nil
         @client_id            = nil
@@ -734,7 +734,7 @@ def id_token=(new_id_token)
       #   omitted.
       #
       # @return [String] The decoded ID token.
-      def decoded_id_token(public_key=nil, options = {}, &keyfinder)
+      def decoded_id_token public_key=nil, options = {}, &keyfinder
         options[:algorithm] ||= signing_algorithm
         verify = !!(public_key || keyfinder)
         payload, _header = JWT.decode(self.id_token, public_key, verify, options, &keyfinder)
@@ -767,12 +767,13 @@ def expires_in
       #
       # @param [String, Integer, nil] new_expires_in
       #   The access token lifetime.
-      def expires_in=(new_expires_in)
-        if new_expires_in != nil
+      def expires_in= new_expires_in
+        if !new_expires_in.nil?
           @issued_at = Time.now
           @expires_at = @issued_at + new_expires_in.to_i
         else
-          @expires_at, @issued_at = nil, nil
+          @expires_at = nil
+          @issued_at = nil
         end
       end
 
@@ -781,7 +782,7 @@ def expires_in=(new_expires_in)
       #
       # @return [Time, nil] The access token issuance time.
       def issued_at
-        return @issued_at
+        @issued_at
       end
 
       ##
@@ -982,7 +983,7 @@ def generate_access_token_request(options={})
       end
 
       def fetch_access_token(options={})
-        if self.token_credential_uri == nil
+        if self.token_credential_uri.nil?
           raise ArgumentError, 'Missing token endpoint URI.'
         end
 

diff --git a/lib/signet/version.rb b/lib/signet/version.rb
@@ -17,7 +17,7 @@
   module Signet
     module VERSION
       MAJOR = 0
-      MINOR = 10
+      MINOR = 11
       TINY  = 0
       PRE   = nil
 
@@ -58,30 +58,30 @@ def self.warn_on_old_ruby_version
       # @private
       #
       def self.warn_unsupported_ruby cur_version, recommended_version
-        "WARNING: You are running Ruby #{cur_version}, which has reached" + 
-        " end-of-life and is no longer supported by Ruby Core.\n" + 
-        'Signet works best on supported versions of' + 
-        ' Ruby. It is strongly recommended that you upgrade to Ruby' + 
-        " #{recommended_version} or later. \n" +
-        'See https://www.ruby-lang.org/en/downloads/branches/ for more' +
-        " info on the Ruby maintenance schedule.\n" + 
-        'To suppress this message, set the' + 
-        ' GOOGLE_CLOUD_SUPPRESS_RUBY_WARNINGS environment variable.'
+        "WARNING: You are running Ruby #{cur_version}, which has reached" \
+          " end-of-life and is no longer supported by Ruby Core.\n" \
+          'Signet works best on supported versions of' \
+          ' Ruby. It is strongly recommended that you upgrade to Ruby' \
+          " #{recommended_version} or later. \n" \
+          'See https://www.ruby-lang.org/en/downloads/branches/ for more' \
+          " info on the Ruby maintenance schedule.\n" \
+          'To suppress this message, set the' \
+          ' GOOGLE_CLOUD_SUPPRESS_RUBY_WARNINGS environment variable.'
       end
 
       ##
       # Print a warning for a supported but nearing EOL version of Ruby
       # @private
       #
       def self.warn_nonrecommended_ruby cur_version, recommended_version
-        "WARNING: You are running Ruby #{cur_version}, which is nearing" + 
-        " end-of-life.\n" + 
-        'Signet works best on supported versions of' +
-        " Ruby. Consider upgrading to Ruby #{recommended_version} or later.\n" +
-        'See https://www.ruby-lang.org/en/downloads/branches/ for more' +
-        " info on the Ruby maintenance schedule.\n" +
-        'To suppress this message, set the' + 
-        ' GOOGLE_CLOUD_SUPPRESS_RUBY_WARNINGS environment variable.'
+        "WARNING: You are running Ruby #{cur_version}, which is nearing" \
+          " end-of-life.\n" \
+          'Signet works best on supported versions of' \
+          " Ruby. Consider upgrading to Ruby #{recommended_version} or later.\n" \
+          'See https://www.ruby-lang.org/en/downloads/branches/ for more' \
+          " info on the Ruby maintenance schedule.\n" \
+          'To suppress this message, set the' \
+          ' GOOGLE_CLOUD_SUPPRESS_RUBY_WARNINGS environment variable.'
       end
     end
   end