Possible bug introduced in 4.7 #173

alvaroabascar · 2021-01-15T17:22:11Z

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.

The text was updated successfully, but these errors were encountered:

jamescooke · 2021-01-21T21:44:34Z

@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 · 2021-01-22T13:35:30Z

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 · 2021-01-22T13:56:02Z

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:

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.
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 · 2021-01-22T13:59:17Z

@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 · 2021-01-22T15:31:43Z

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 · 2021-01-22T17:16:25Z

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 · 2021-01-22T17:35:06Z

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 · 2021-01-26T12:23:53Z

@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 · 2021-02-04T00:06:32Z

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

nitishxp · 2021-02-12T07:49:30Z

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

Computing the blinding factor and its inverse was done in a thread-unsafe manner. Locking the computation & update of the blinding factors, and passing these around in frame- and stack-bound data, solves this.

sybrenstuvel · 2021-02-14T11:28:49Z

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 · 2021-02-14T21:40:33Z

@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 · 2021-02-15T11:53:20Z

@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 · 2021-02-15T18:37:40Z

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?

Computing the blinding factor and its inverse was done in a thread-unsafe manner. Locking the computation & update of the blinding factors, and passing these around in frame- and stack-bound data, solves this. This fixes part of the issues reported in #173, but there is more going on in that particular report.

sybrenstuvel · 2021-02-15T20:46:44Z

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 · 2021-02-16T15:49:35Z

@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 · 2021-02-24T10:36:13Z

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 · 2021-02-24T15:06:56Z

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

🙌🏻

- Update from 4.0 to 4.8 - Update of rootfile - Changelog - Switch to [Poetry](https://python-poetry.org/) for dependency and release management. - Compatibility with Python 3.10. - Chain exceptions using `raise new_exception from old_exception` ([#157](sybrenstuvel/python-rsa#157)) - Added marker file for PEP 561. This will allow type checking tools in dependent projects to use type annotations from Python-RSA ([#136](sybrenstuvel/python-rsa#136)). - Use the Chinese Remainder Theorem when decrypting with a private key. This makes decryption 2-4x faster ([#163](sybrenstuvel/python-rsa#163)). - Fix picking/unpickling issue introduced in 4.7 ([#173](sybrenstuvel/python-rsa#173)) - Fix threading issue introduced in 4.7 ([#173](sybrenstuvel/python-rsa#173)) - Fix [#165](sybrenstuvel/python-rsa#165): CVE-2020-25658 - Bleichenbacher-style timing oracle in PKCS#1 v1.5 decryption code - Add padding length check as described by PKCS#1 v1.5 (Fixes [#164](sybrenstuvel/python-rsa#164)) - Reuse of blinding factors to speed up blinding operations. Fixes [#162](sybrenstuvel/python-rsa#162). - Declare & test support for Python 3.9 Version 4.4 and 4.6 are almost a re-tagged release of version 4.2. It requires Python 3.5+. To avoid older Python installations from trying to upgrade to RSA 4.4, this is now made explicit in the `python_requires` argument in `setup.py`. There was a mistake releasing 4.4 as "3.5+ only", which made it necessary to retag 4.4 as 4.6 as well. No functional changes compared to version 4.2. Version 4.3 and 4.5 are almost a re-tagged release of version 4.0. It is the last to support Python 2.7. This is now made explicit in the `python_requires` argument in `setup.py`. Python 3.4 is not supported by this release. There was a mistake releasing 4.4 as "3.5+ only", which made it necessary to retag 4.3 as 4.5 as well. Two security fixes have also been backported, so 4.3 = 4.0 + these two fixes. - Choose blinding factor relatively prime to N. Thanks Christian Heimes for pointing this out. - Reject cyphertexts (when decrypting) and signatures (when verifying) that have been modified by prepending zero bytes. This resolves CVE-2020-13757. Thanks Carnil for pointing this out. - Rolled back the switch to Poetry, and reverted back to using Pipenv + setup.py for dependency management. There apparently is an issue no-binary installs of packages build with Poetry. This fixes [#148](sybrenstuvel/python-rsa#148) - Limited SHA3 support to those Python versions (3.6+) that support it natively. The third-party library that adds support for this to Python 3.5 is a binary package, and thus breaks the pure-Python nature of Python-RSA. This should fix [#147](sybrenstuvel/python-rsa#147). - Added support for Python 3.8. - Dropped support for Python 2 and 3.4. - Added type annotations to the source code. This will make Python-RSA easier to use in your IDE, and allows better type checking. - Added static type checking via [MyPy](http://mypy-lang.org/). - Fix [#129](sybrenstuvel/python-rsa#129) Installing from source gives UnicodeDecodeError. - Switched to using [Poetry](https://poetry.eustace.io/) for package management. - Added support for SHA3 hashing: SHA3-256, SHA3-384, SHA3-512. This is natively supported by Python 3.6+ and supported via a third-party library on Python 3.5. - Choose blinding factor relatively prime to N. Thanks Christian Heimes for pointing this out. - Reject cyphertexts (when decrypting) and signatures (when verifying) that have been modified by prepending zero bytes. This resolves CVE-2020-13757. Thanks Adelapie for pointing this out. Signed-off-by: Adolf Belka <adolf.belka@ipfire.org> Reviewed-by: Peter Müller <peter.mueller@ipfire.org>

busunkim96 mentioned this issue Jan 15, 2021

Getting "Invalid JWT Signature" after upgrading to rsa==4.7 googleapis/google-auth-library-python#667

Closed

fcollman mentioned this issue Feb 3, 2021

invalid_grant googleapis/python-storage#369

Closed

busunkim96 mentioned this issue Feb 19, 2021

Call AbstractKey.__init__() in __setstate__ #178

Merged

sybrenstuvel closed this as completed in eb6ddb6 Feb 24, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Possible bug introduced in 4.7 #173

Possible bug introduced in 4.7 #173

alvaroabascar commented Jan 15, 2021 •

edited

Loading

jamescooke commented Jan 21, 2021

sybrenstuvel commented Jan 22, 2021 •

edited

Loading

jamescooke commented Jan 22, 2021

jamescooke commented Jan 22, 2021

GiardEmilien commented Jan 22, 2021 •

edited

Loading

sybrenstuvel commented Jan 22, 2021

sybrenstuvel commented Jan 22, 2021

jamescooke commented Jan 26, 2021

fcollman commented Feb 4, 2021

nitishxp commented Feb 12, 2021

sybrenstuvel commented Feb 14, 2021

jamescooke commented Feb 14, 2021

jamescooke commented Feb 15, 2021

sybrenstuvel commented Feb 15, 2021

sybrenstuvel commented Feb 15, 2021

jamescooke commented Feb 16, 2021

sybrenstuvel commented Feb 24, 2021

jamescooke commented Feb 24, 2021

Possible bug introduced in 4.7 #173

Possible bug introduced in 4.7 #173

Comments

alvaroabascar commented Jan 15, 2021 • edited Loading

jamescooke commented Jan 21, 2021

sybrenstuvel commented Jan 22, 2021 • edited Loading

jamescooke commented Jan 22, 2021

jamescooke commented Jan 22, 2021

GiardEmilien commented Jan 22, 2021 • edited Loading

sybrenstuvel commented Jan 22, 2021

sybrenstuvel commented Jan 22, 2021

jamescooke commented Jan 26, 2021

fcollman commented Feb 4, 2021

nitishxp commented Feb 12, 2021

sybrenstuvel commented Feb 14, 2021

jamescooke commented Feb 14, 2021

jamescooke commented Feb 15, 2021

sybrenstuvel commented Feb 15, 2021

sybrenstuvel commented Feb 15, 2021

jamescooke commented Feb 16, 2021

sybrenstuvel commented Feb 24, 2021

jamescooke commented Feb 24, 2021

alvaroabascar commented Jan 15, 2021 •

edited

Loading

sybrenstuvel commented Jan 22, 2021 •

edited

Loading

GiardEmilien commented Jan 22, 2021 •

edited

Loading