From 3eeacd93945252bec78c9453bc0e37934476ed70 Mon Sep 17 00:00:00 2001 From: Yuki Iwanaga Date: Tue, 9 Sep 2014 02:22:54 +0900 Subject: [PATCH 1/2] Remove unnecessary secure_compare method --- lib/jwt.rb | 13 +------------ spec/jwt_spec.rb | 17 ----------------- 2 files changed, 1 insertion(+), 29 deletions(-) diff --git a/lib/jwt.rb b/lib/jwt.rb index f5dc7a3d..2e592d8a 100644 --- a/lib/jwt.rb +++ b/lib/jwt.rb @@ -116,7 +116,7 @@ def signature_algorithm_and_key(header, key, &keyfinder) def verify_signature(algo, key, signing_input, signature) begin if ["HS256", "HS384", "HS512"].include?(algo) - raise JWT::DecodeError.new("Signature verification failed") unless secure_compare(signature, sign_hmac(algo, signing_input, key)) + raise JWT::DecodeError.new("Signature verification failed") unless signature == sign_hmac(algo, signing_input, key) elsif ["RS256", "RS384", "RS512"].include?(algo) raise JWT::DecodeError.new("Signature verification failed") unless verify_rsa(algo, key, signing_input, signature) else @@ -129,15 +129,4 @@ def verify_signature(algo, key, signing_input, signature) end end - # From devise - # constant-time comparison algorithm to prevent timing attacks - def secure_compare(a, b) - return false if a.nil? || b.nil? || a.empty? || b.empty? || a.bytesize != b.bytesize - l = a.unpack "C#{a.bytesize}" - - res = 0 - b.each_byte { |byte| res |= byte ^ l.shift } - res == 0 - end - end diff --git a/spec/jwt_spec.rb b/spec/jwt_spec.rb index e81945f4..f900b8b6 100644 --- a/spec/jwt_spec.rb +++ b/spec/jwt_spec.rb @@ -122,23 +122,6 @@ JWT.decode(jwt, secret) end - describe "secure comparison" do - it "returns true if strings are equal" do - expect(JWT.secure_compare("Foo", "Foo")).to be_true - end - - it "returns false if either input is nil or empty" do - [nil, ""].each do |bad| - expect(JWT.secure_compare(bad, "Foo")).to be_false - expect(JWT.secure_compare("Foo", bad)).to be_false - end - end - - it "retuns false if the strings are different" do - expect(JWT.secure_compare("Foo", "Bar")).to be_false - end - end - # no method should leave OpenSSL.errors populated after do expect(OpenSSL.errors).to be_empty From 4a6d677b302a9ce8701871e642d2c9631a55a9ed Mon Sep 17 00:00:00 2001 From: Yuki Iwanaga Date: Tue, 9 Sep 2014 02:27:45 +0900 Subject: [PATCH 2/2] Remove spec: does not use == to compare digests --- spec/jwt_spec.rb | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/spec/jwt_spec.rb b/spec/jwt_spec.rb index f900b8b6..2891e09b 100644 --- a/spec/jwt_spec.rb +++ b/spec/jwt_spec.rb @@ -109,19 +109,6 @@ expect { JWT.decode(jwt, nil, true) }.to raise_error(JWT::DecodeError) end - it "does not use == to compare digests" do - secret = "secret" - jwt = JWT.encode(@payload, secret) - crypto_segment = jwt.split(".").last - - signature = JWT.base64url_decode(crypto_segment) - expect(signature).not_to receive('==') - expect(JWT).to receive(:base64url_decode).with(crypto_segment).once.and_return(signature) - expect(JWT).to receive(:base64url_decode).at_least(:once).and_call_original - - JWT.decode(jwt, secret) - end - # no method should leave OpenSSL.errors populated after do expect(OpenSSL.errors).to be_empty