Backport: Avoid using the same digest across calls (#697) (#701)

Avoid using the same digest across calls (#697)

* Avoid using the same digest across calls

JWT appears to reuse these JWA instances across threads, which
can lead to them stepping on each other via the shared
OpenSSL::Digest instance. This causes decoding to fail
verification, likely because the digest contains an amalgam of
data from the different threads.

This patch creates a new OpenSSL::Digest for each use, avoiding
the threading issue.

Note that the HMAC JWA already calls OpenSSL::HMAC.digest,
avoiding the shared state, and the others do not use digest.

The original code does not fail on CRuby most likely because only
one thread at a time can be calculating a digest against a given
OpenSSL::Digest instance, due to the VM lock.

Fixes #696

Addresses the issue reported in jruby/jruby#8504 by @mohamedhafez

* Add #697 to changelog

* Modify Rsa digest name test for new structure

The @digest instance variable now contains the name to the digest
to be used.

See #697

* Add test for concurrent encode/decode using ECDSA

This is adapted from the script in #696 and provides a test for
the ECDSA part of the fix in #697.

* Fixes for Rubocop

Co-authored-by: Charles Oliver Nutter <headius@headius.com>

Author: anakinj

lib/jwt/jwa/ecdsa.rb

@@ -8,23 +8,23 @@ class Ecdsa
 
       def initialize(alg, digest)
         @alg = alg
-        @digest = OpenSSL::Digest.new(digest)
+        @digest = digest
       end
 
       def sign(data:, signing_key:)
         curve_definition = curve_by_name(signing_key.group.curve_name)
         key_algorithm = curve_definition[:algorithm]
         raise IncorrectAlgorithm, "payload algorithm is #{alg} but #{key_algorithm} signing key was provided" if alg != key_algorithm
 
-        asn1_to_raw(signing_key.dsa_sign_asn1(digest.digest(data)), signing_key)
+        asn1_to_raw(signing_key.dsa_sign_asn1(OpenSSL::Digest.new(digest).digest(data)), signing_key)
       end
 
       def verify(data:, signature:, verification_key:)
         curve_definition = curve_by_name(verification_key.group.curve_name)
         key_algorithm = curve_definition[:algorithm]
         raise IncorrectAlgorithm, "payload algorithm is #{alg} but #{key_algorithm} verification key was provided" if alg != key_algorithm
 
-        verification_key.dsa_verify_asn1(digest.digest(data), raw_to_asn1(signature, verification_key))
+        verification_key.dsa_verify_asn1(OpenSSL::Digest.new(digest).digest(data), raw_to_asn1(signature, verification_key))
       rescue OpenSSL::PKey::PKeyError
         raise JWT::VerificationError, 'Signature verification raised'
       end

lib/jwt/jwa/rsa.rb

@@ -8,17 +8,17 @@ class Rsa
 
       def initialize(alg)
         @alg = alg
-        @digest = OpenSSL::Digest.new(alg.sub('RS', 'SHA'))
+        @digest = alg.sub('RS', 'SHA')
       end
 
       def sign(data:, signing_key:)
         raise_sign_error!("The given key is a #{signing_key.class}. It has to be an OpenSSL::PKey::RSA instance") unless signing_key.is_a?(OpenSSL::PKey::RSA)
 
-        signing_key.sign(digest, data)
+        signing_key.sign(OpenSSL::Digest.new(digest), data)
       end
 
       def verify(data:, signature:, verification_key:)
-        verification_key.verify(digest, signature, data)
+        verification_key.verify(OpenSSL::Digest.new(digest), signature, data)
       rescue OpenSSL::PKey::PKeyError
         raise JWT::VerificationError, 'Signature verification raised'
       end

spec/jwt/concurrent_encode_decode_spec.rb

@@ -0,0 +1,29 @@
+# frozen_string_literal: true
+
+RSpec.describe JWT::JWA::Ecdsa do
+  context 'used across threads for encoding and decoding' do
+    it 'successfully encodes, decodes, and verifies' do
+      threads = 10.times.map do
+        Thread.new do
+          public_key_pem = "-----BEGIN PUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEcKuFOqoNEN+TXylz4MVAWREa9yA8\npOF9QgGchnAy6Ad4P7yCpk+R3wCGTDLfNboYqUmbK5Hd9uHszf+EMTi22g==\n-----END PUBLIC KEY-----\n"
+          private_key_pem = "-----BEGIN PRIVATE KEY-----\nMIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQgiF/iNuQem/yQyd16\nc9shf2Y9vMycOU7g6W6LTmkyj1ehRANCAARwq4U6qg0Q35NfKXPgxUBZERr3IDyk\n4X1CAZyGcDLoB3g/vIKmT5HfAIZMMt81uhipSZsrkd324ezN/4QxOLba\n-----END PRIVATE KEY-----\n"
+          full_pem = private_key_pem + public_key_pem
+          curve = OpenSSL::PKey.read(full_pem)
+          public_key = OpenSSL::PKey::EC.new(public_key_pem)
+
+          10.times do
+            input_payload = { 'aud' => 'https://fcm.googleapis.com', 'exp' => (Time.now.to_i + 600), 'sub' => 'mailto:example@example.com' }
+            input_header = { 'typ' => 'JWT', 'alg' => 'ES256' }
+            token = JWT.encode(input_payload, curve, 'ES256', input_header)
+
+            output_payload, output_header = JWT.decode(token, public_key, true, { algorithm: 'ES256', verify_expiration: true })
+            expect(output_payload).to eq input_payload
+            expect(output_header).to eq input_header
+          end
+        end
+      end
+
+      threads.each(&:join)
+    end
+  end
+end

spec/jwt/jwa/rsa_spec.rb

@@ -8,7 +8,7 @@
   describe '#initialize' do
     it 'initializes with the correct algorithm and digest' do
       expect(rsa_instance.instance_variable_get(:@alg)).to eq('RS256')
-      expect(rsa_instance.send(:digest).name).to eq('SHA256')
+      expect(rsa_instance.send(:digest)).to eq('SHA256')
     end
   end