Possible bug introduced in 4.7

[alvaroabascar]

Hi! I am a a dvc user. This library connects to Google Cloud Storage, and it can use a google service key to authenticate. When authenticating in this way, the google-auth library fails with {"error":"invalid_grant","error_description":"Invalid JWT Signature."}. This happens with rsa==4.7 but not with with rsa==4.6. After some research, we found that restoring the 4.6 version of the rsa/key.py file solves the problem.

[jamescooke]

@alvaroabascar Thanks for raising this. I've got a similar issue - we're using Luigi and communicating with GCS, however, I've got a full stack trace:

  File "/usr/local/lib/python3.6/multiprocessing/pool.py", line 119, in worker
    result = (True, func(*args, **kwds))
  File "/usr/local/lib/python3.6/multiprocessing/pool.py", line 44, in mapstar
    return list(map(*args))
  File "/usr/local/lib/python3.6/site-packages/luigi/contrib/gcs.py", line 258, in _forward_args_to_put
    return self.put(**kwargs)
  File "/usr/local/lib/python3.6/site-packages/luigi/contrib/gcs.py", line 255, in put
    self._do_put(media, dest_path)
  File "/usr/local/lib/python3.6/site-packages/luigi/contrib/gcs.py", line 173, in _do_put
    status, response = request.next_chunk()
  File "/usr/local/lib/python3.6/site-packages/googleapiclient/_helpers.py", line 134, in positional_wrapper
    return wrapped(*args, **kwargs)
  File "/usr/local/lib/python3.6/site-packages/googleapiclient/http.py", line 993, in next_chunk
    headers=start_headers,
  File "/usr/local/lib/python3.6/site-packages/googleapiclient/http.py", line 177, in _retry_request
    resp, content = http.request(uri, method, *args, **kwargs)
  File "/usr/local/lib/python3.6/site-packages/google_auth_httplib2.py", line 190, in request
    self._request, method, uri, request_headers)
  File "/usr/local/lib/python3.6/site-packages/google/auth/credentials.py", line 133, in before_request
    self.refresh(request)
  File "/usr/local/lib/python3.6/site-packages/google/oauth2/service_account.py", line 360, in refresh
    assertion = self._make_authorization_grant_assertion()
  File "/usr/local/lib/python3.6/site-packages/google/oauth2/service_account.py", line 354, in _make_authorization_grant_assertion
    token = jwt.encode(self._signer, payload)
  File "/usr/local/lib/python3.6/site-packages/google/auth/jwt.py", line 112, in encode
    signature = signer.sign(signing_input)
  File "/usr/local/lib/python3.6/site-packages/google/auth/crypt/_python_rsa.py", line 136, in sign
    return rsa.pkcs1.sign(message, self._key, "SHA-256")
  File "/usr/local/lib/python3.6/site-packages/rsa/pkcs1.py", line 331, in sign
    return sign_hash(msg_hash, priv_key, hash_method)
  File "/usr/local/lib/python3.6/site-packages/rsa/pkcs1.py", line 306, in sign_hash
    encrypted = priv_key.blinded_encrypt(payload)
  File "/usr/local/lib/python3.6/site-packages/rsa/key.py", line 463, in blinded_encrypt
    blinded = self.blind(message)  # blind before encrypting
  File "/usr/local/lib/python3.6/site-packages/rsa/key.py", line 165, in blind
    self._update_blinding_factor()
  File "/usr/local/lib/python3.6/site-packages/rsa/key.py", line 190, in _update_blinding_factor
    if self.blindfac < 0:
AttributeError: blindfac

Reverting to rsa==4.6 fixes the issue.

Looking at the diff (version-4.6...version-4.7) I can see that the blindfac attribute was introduced to key.py in this commit 06ec1ea - my guess is that it's not initialised correctly.

[sybrenstuvel]

This looks like a problem the initalisation of AbstractKey.blindfac indeed. It is initialised in AbstractKey.__init__() though, so I don't really see how this could cause issues. I have tried this code, which hits the if self.blindfac < 0 line. It works fine, though, so I can't reproduce the error here.

#!/usr/bin/env python

from google.auth import crypt
from google.auth import jwt

private_key = open('privatekey.pem').read()
signer = crypt.RSASigner.from_string(private_key)
payload = {'some': 'payload'}
jwt.encode(signer, payload)

This was tested with Python-RSA 4.7 and Google-Auth 1.24.0.

I want to get this fixed, but to do that in a non-hackish way, I need to be able to reproduce the issue myself first.

[jamescooke]

Hi @sybrenstuvel - I had a look at that blindfac patch and I also couldn't see why it's not initialised. My two guesses were:

1. That something in Google auth was initialising the key in a strange way - that's why I posted this comment: Getting "Invalid JWT Signature" after upgrading to rsa==4.7 googleapis/google-auth-library-python#667 (comment) - however I think your code above shows that there are no issues with the google auth invocation.

2. That it might be something to do with running inside multiprocessing subprocesses (see the start of the stack trace). We're getting this error when we use Luigi's GCS put_multiple() method with num_processes=20: https://github.com/spotify/luigi/blob/2.8.13/luigi/contrib/gcs.py#L277-L281 . The failure only happens in one test that creates a GCS directory with more than 1,000 objects (so we use put_multiple() for speed). All other calls to GCS are successful (more than 250 other tests pass).

[jamescooke]

@alvaroabascar Could you confirm if DVC is using threads or subprocesses in the call that you originally reported failing? That might help confirm / deny my guess number 2 above 🙏🏻

[GiardEmilien]

Hi @jamescooke, we always see this error when we make a call (in our case to big query with google-cloud-bigquery) inside a Thread, so I think number 2 is the right guess.

[sybrenstuvel]

Excellent, that explains why I couldn't reproduce it with a single thread. Still, I don't see how multithreading could cause this exact error, as the __init__() function should have been called before calling anything else on the instance. If I add a mutex to avoid threading issues, could you (@jamescooke or @GiardEmilien) try out a test version of the library to see if that fixes things?

[sybrenstuvel]

hmmm even with this code I cannot reproduce the problem:

#!/usr/bin/env python

import multiprocessing
import sys

from google.auth import crypt
from google.auth import jwt

private_key = open('privatekey.pem').read()
signer = crypt.RSASigner.from_string(private_key)
payload = {'some': 'payload'}


def doit():
  try:
    jwt.encode(signer, payload)
  except:
    sys.excepthook(*sys.exc_info())


if __name__ == '__main__':
  procs = []
  for _ in range(1000):
    proc = multiprocessing.Process(target=doit, args=())
    procs.append(proc)

  for proc in procs:
    proc.start()

  for proc in procs:
    proc.join()

I still wouldn't mind having a way to reproduce the problem myself.

[jamescooke]

@sybrenstuvel I agree it would be great to reproduce locally and simply.

could you (...) try out a test version of the library to see if that fixes things?

Happy to try! 😊 👍🏻

[fcollman]

I have reason to believe the issue might be with multithreading and using multiple keys.

[nitishxp]

Yes I also faced this issue in bigquery when doing a multithread call.

[sybrenstuvel]

The code below seems to reproduce the issue. It at least shows a threading-related issue, so let's hope it's the same one :)

@jamescooke (or anyone else in a position to test) could you try this version of the library, to see if it fixes things for you? rsa-4.7.1.dev0-source-and-wheel.zip

#!/usr/bin/env python

import threading
import sys

import rsa

private_key_data = open('privatekey.pem').read()
priv_key = rsa.PrivateKey.load_pkcs1(private_key_data)
pub_key = rsa.PublicKey(priv_key.n, priv_key.e)

payload = b'this is some test payload'

main_thread_ident = threading.get_ident()
exception_mutex = threading.Lock()
abort_everything = threading.Event()

def doit(thread_idx: int):
  try:
    if abort_everything.is_set():
      return

    # sign/verify
    signature = rsa.sign(payload, priv_key, hash_method='SHA-256')
    rsa.verify(payload, signature, pub_key)

    if abort_everything.is_set():
      return

    # encrypt/decrypt
    crypto = rsa.encrypt(payload, pub_key)
    rsa.decrypt(crypto, priv_key)

    print(f"OK in thread {thread_idx}")
  except:
    if threading.get_ident() == main_thread_ident:
      raise
    with exception_mutex:
      abort_everything.set()
      print(f"Exception in thread {thread_idx}:")
      sys.excepthook(*sys.exc_info())


if __name__ == '__main__':
  doit(-1)
  doit(-1)
  doit(-1)
  print("Single thread is fine")

  threads = []
  for thread_idx in range(100):
    thread = threading.Thread(target=doit, args=(thread_idx, ))
    threads.append(thread)

  for thread in threads:
    thread.start()

  for thread in threads:
    thread.join()

[jamescooke]

@sybrenstuvel This looks good. I've run the code above and had it fail with Python 3.8.6 and rsa==4.7. When I install the 4.7.1 wheel, the example passes.

I'll be able to test the wheel on the work project where I found the bug on Tuesday and will ping back here.

[jamescooke]

@sybrenstuvel Good news is that I got to test this earlier than I thought, bad news is that I've got a new error 😞

Traceback (most recent call last):
  File "/usr/local/lib/python3.6/multiprocessing/pool.py", line 119, in worker
    result = (True, func(*args, **kwds))
  File "/usr/local/lib/python3.6/multiprocessing/pool.py", line 44, in mapstar
    return list(map(*args))
  File "/usr/local/lib/python3.6/site-packages/luigi/contrib/gcs.py", line 257, in _forward_args_to_put
    return self.put(**kwargs)
  File "/usr/local/lib/python3.6/site-packages/luigi/contrib/gcs.py", line 254, in put
    self._do_put(media, dest_path)
  File "/usr/local/lib/python3.6/site-packages/luigi/contrib/gcs.py", line 172, in _do_put
    status, response = request.next_chunk()
  File "/usr/local/lib/python3.6/site-packages/googleapiclient/_helpers.py", line 134, in positional_wrapper
    return wrapped(*args, **kwargs)
  File "/usr/local/lib/python3.6/site-packages/googleapiclient/http.py", line 993, in next_chunk
    headers=start_headers,
  File "/usr/local/lib/python3.6/site-packages/googleapiclient/http.py", line 177, in _retry_request
    resp, content = http.request(uri, method, *args, **kwargs)
  File "/usr/local/lib/python3.6/site-packages/google_auth_httplib2.py", line 190, in request
    self._request, method, uri, request_headers)
  File "/usr/local/lib/python3.6/site-packages/google/auth/credentials.py", line 133, in before_request
    self.refresh(request)
  File "/usr/local/lib/python3.6/site-packages/google/oauth2/service_account.py", line 360, in refresh
    assertion = self._make_authorization_grant_assertion()
  File "/usr/local/lib/python3.6/site-packages/google/oauth2/service_account.py", line 354, in _make_authorization_grant_assertion
    token = jwt.encode(self._signer, payload)
  File "/usr/local/lib/python3.6/site-packages/google/auth/jwt.py", line 112, in encode
    signature = signer.sign(signing_input)
  File "/usr/local/lib/python3.6/site-packages/google/auth/crypt/_python_rsa.py", line 136, in sign
    return rsa.pkcs1.sign(message, self._key, "SHA-256")
  File "/usr/local/lib/python3.6/site-packages/rsa/pkcs1.py", line 331, in sign
    return sign_hash(msg_hash, priv_key, hash_method)
  File "/usr/local/lib/python3.6/site-packages/rsa/pkcs1.py", line 306, in sign_hash
    encrypted = priv_key.blinded_encrypt(payload)
  File "/usr/local/lib/python3.6/site-packages/rsa/key.py", line 477, in blinded_encrypt
    blinded, blindfac_inverse = self.blind(message)
  File "/usr/local/lib/python3.6/site-packages/rsa/key.py", line 167, in blind
    blindfac, blindfac_inverse = self._update_blinding_factor()
  File "/usr/local/lib/python3.6/site-packages/rsa/key.py", line 203, in _update_blinding_factor
    with self.mutex:
AttributeError: mutex

[sybrenstuvel]

This really must be related to how the key objects are created. This wouldn't happen if they'd just use the regular AbstractKey.__init__() function, so I wouldn't be surprised if they use a different object altogether that just happens to have n and e properties.

Can you help me figure out how to reproduce this on my machine?

[sybrenstuvel]

I have released the threading fix as version 4.7.1. All that's now left of this issue is the Google Cloud Service weirdness.

[jamescooke]

@sybrenstuvel Thanks for pushing the threading fix so quickly 🙌🏻

Can you help me figure out how to reproduce this on my machine?

I had a look at the Google GCS code when this bug first came up, but nothing jumped out at me as strange about the init calls. However, there could be some monkey patching or tweaks happening elsewhere in the code base.

The only way I know how to reproduce this easily is with Luigi and a GCS account - let me know if you want me to put together a small example of that, maybe as a gist?

[sybrenstuvel]

Thanks @busunkim96 for providing the answer to this riddle. It turns out the pickling/unpickling functions didn't take the new properties (key.mutex, key.blindfac, and key.blindfac_inverse) into account. #178 fixed that, which will be in a release soon.

[jamescooke]

Thanks for doing a release quickly @sybrenstuvel - can confirm it's working with rsa==4.7.2 👍🏻

🙌🏻