pyca / bcrypt

Modern(-ish) password hashing for your software and your servers
Apache License 2.0
1.25k stars 168 forks source link

New release results in bcrypt break #677

Closed Stefanhg closed 6 months ago

Stefanhg commented 11 months ago

Hello,

Our system sadly relies on tip of bcrypt and in this case the new release breaks when you do import bcrypt Can anyone fix this? I don't know what this package does except that it is used in some dependable packages.

Stefanhg commented 11 months ago

After more digging it is actually not our systems but the package Paramiko with bcrypt>=3.2 https://github.com/paramiko/paramiko/blob/main/setup.py But i would still assume that import bcrypt is expected to work.

alex commented 11 months ago

Can you explain what "breaks" means?

Stefanhg commented 11 months ago

Reproduce:

  1. python -m venv venv
  2. venv\Scripts\activate
  3. python
  4. import bcrypt

Actual result:

>>> import bcrypt
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\Users\shg\venv\lib\site-packages\bcrypt\__init__.py", line 13, in <module>
    from ._bcrypt import (
ModuleNotFoundError: No module named 'bcrypt._bcrypt'
Stefanhg commented 11 months ago

The 4.0.1 release includes the following files:__about__.py, __init__.py, _bcrypt.pyd, _bcrypt.pyi, py.typed The 4.1.0 release includes the following files: __init__.py, __init.pyi, _bcrypt.cp37-win_amd64.pyd, py.typed

Seems like something has gone wrong when the release has happened. Some files are missing.

strickvl commented 11 months ago

+1 have encountered breaking changes with this release. (Am installing bcrypt through passlib).

alex commented 11 months ago

I'm not able to reproduce:

(tempenv-3ce71814330fc) ~ ❯❯❯ pip install bcrypt
Collecting bcrypt
  Downloading bcrypt-4.1.0-cp37-abi3-macosx_13_0_universal2.whl.metadata (9.2 kB)
Downloading bcrypt-4.1.0-cp37-abi3-macosx_13_0_universal2.whl (528 kB)
   ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 528.2/528.2 kB 5.5 MB/s eta 0:00:00
Installing collected packages: bcrypt
Successfully installed bcrypt-4.1.0
(tempenv-3ce71814330fc) ~ ❯❯❯ python3 -c "import bcrypt"
(tempenv-3ce71814330fc) ~ ❯❯❯ 

Can you provide more steps to reproduce?

Stefanhg commented 11 months ago

Maybe it is a Windows-specific thing? If you go into the folder where bcrypt is installed. Do you have the same files as me: __init__.py, __init.pyi, _bcrypt.cp37-win_amd64.pyd, py.typed except the _bcrypt.cp37-win_amd64.pyd file

Stefanhg commented 11 months ago

On WSL I can import bcrypt just fine. I don't have a non-WSL Linux available.

joein commented 11 months ago

I use bcrypt with passlib I updated bcrypt to 4.1.0 today and it has broken my service

I run my service in a docker container from docker image python:3.11-slim With these dependencies:

passlib==1.7.4
bcrypt==4.0.1

The following code works:

from passlib.context import CryptContext

pwd_context = CryptContext(schemes=["bcrypt"], deprecated="auto")
pwd_context.verify('123', pwd_context.hash('123'))  # True

but with such dependencies

passlib==1.7.4
bcrypt==4.1.0

it crashes and traceback is:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/venv/lib/python3.11/site-packages/passlib/context.py", line 2258, in hash
    return record.hash(secret, **kwds)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/venv/lib/python3.11/site-packages/passlib/utils/handlers.py", line 779, in hash
    self.checksum = self._calc_checksum(secret)
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/venv/lib/python3.11/site-packages/passlib/handlers/bcrypt.py", line 591, in _calc_checksum
    self._stub_requires_backend()
  File "/venv/lib/python3.11/site-packages/passlib/utils/handlers.py", line 2254, in _stub_requires_backend
    cls.set_backend()
  File "/venv/lib/python3.11/site-packages/passlib/utils/handlers.py", line 2156, in set_backend
    return owner.set_backend(name, dryrun=dryrun)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/venv/lib/python3.11/site-packages/passlib/utils/handlers.py", line 2163, in set_backend
    return cls.set_backend(name, dryrun=dryrun)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/venv/lib/python3.11/site-packages/passlib/utils/handlers.py", line 2188, in set_backend
    cls._set_backend(name, dryrun)
  File "/venv/lib/python3.11/site-packages/passlib/utils/handlers.py", line 2311, in _set_backend
    super(SubclassBackendMixin, cls)._set_backend(name, dryrun)
  File "/venv/lib/python3.11/site-packages/passlib/utils/handlers.py", line 2224, in _set_backend
    ok = loader(**kwds)
         ^^^^^^^^^^^^^^
  File "/venv/lib/python3.11/site-packages/passlib/handlers/bcrypt.py", line 745, in _load_backend_mixin
    return mixin_cls._finalize_backend_mixin(name, dryrun)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/venv/lib/python3.11/site-packages/passlib/handlers/bcrypt.py", line 403, in _finalize_backend_mixin
    result = safe_verify("test", test_hash_20)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/venv/lib/python3.11/site-packages/passlib/handlers/bcrypt.py", line 303, in safe_verify
    return verify(secret, hash)
           ^^^^^^^^^^^^^^^^^^^^
  File "/venv/lib/python3.11/site-packages/passlib/utils/handlers.py", line 792, in verify
    return consteq(self._calc_checksum(secret), chk)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/venv/lib/python3.11/site-packages/passlib/handlers/bcrypt.py", line 762, in _calc_checksum_raw
    hash = _pybcrypt.hashpw(secret, config)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: argument 'salt': 'str' object cannot be converted to 'PyBytes'
strickvl commented 11 months ago

I get the same error here https://github.com/zenml-io/zenml/actions/runs/7007131635/job/19060516311#step:4:2556

alex commented 11 months ago

Hmm, I don't see how this could ever have worked: https://github.com/pyca/bcrypt/blob/4.0.1/src/bcrypt/__init__.py#L72-L74 (that's what it looks like in the previous release)

guilhermeyoshida commented 11 months ago

Same error here

PopeyeTheSailorsCat commented 11 months ago

Same error with passlib[bcrypt]==1.7.4 Build with --no-cache and everything just stopped working

f1shl commented 11 months ago

Same error for me, using paramiko.

joein commented 11 months ago

@alex

https://github.com/pyca/bcrypt/blob/4.0.1/src/bcrypt/__init__.py#L72-L74

I send strings to passlib context object, not directly to bcrypt. I haven't dug deep into the codebase, but I found this https://foss.heptapod.net/python-libs/passlib/-/blob/branch/stable/passlib/handlers/bcrypt.py#L646 , so encoding happens inside passlib before it is sent to bcrypt

reaperhulk commented 11 months ago

We have yanked the 4.1.0 release so people experiencing issues should pip install -U bcrypt and it will re-install 4.0.1. If you have a local wheel cache and just type pip install bcrypt it may still install 4.1.0, so make sure you use -U to have it hit PyPI.

We're looking at the Windows bug, but the passlib issue is actually an issue with passlib's heuristic detection failing because we added __version__ to the shared object. We'll work around this in 4.1.1 since while it isn't our bug it certainly is our problem 😄

kgriffs commented 11 months ago

Thanks @reaperhulk ! This broke one of our apps at work as well; appreciate the rapid response.

cvubrugier commented 11 months ago

Thank you @reaperhulk for your prompt reply. The pipeline of an application I maintain at work was broken since bcrypt release 4.1.0 (the unit tests failed). I see that our build pipeline is back to normal.

domq commented 11 months ago

Same error using the test script below. passlib's self-test logic appears to choke on the salt parameter having changed expected types inbetween 4.0.1 and 4.1.0.

from passlib.handlers.bcrypt import bcrypt

settings = {'ident': '2b', 'rounds': 12, 'salt': 'Aejoo1theicoquo9waeT6O'}

hasher = bcrypt.using(settings)

print(hasher.hash('passw0rd'))

Resulting stack trace:

Traceback (most recent call last):
  File "/Users/quatrava/Dev/ops/go-epfl/go-ops/ansible-deps-cache/python-libs/lib/python/site-packages/passlib/utils/handlers.py", line 2163, in set_backend
    return cls.set_backend(name, dryrun=dryrun)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/quatrava/Dev/ops/go-epfl/go-ops/ansible-deps-cache/python-libs/lib/python/site-packages/passlib/utils/handlers.py", line 2188, in set_backend
    cls._set_backend(name, dryrun)
  File "/Users/quatrava/Dev/ops/go-epfl/go-ops/ansible-deps-cache/python-libs/lib/python/site-packages/passlib/utils/handlers.py", line 2311, in _set_backend
    super(SubclassBackendMixin, cls)._set_backend(name, dryrun)
  File "/Users/quatrava/Dev/ops/go-epfl/go-ops/ansible-deps-cache/python-libs/lib/python/site-packages/passlib/utils/handlers.py", line 2224, in _set_backend
    ok = loader(**kwds)
         ^^^^^^^^^^^^^^
  File "/Users/quatrava/Dev/ops/go-epfl/go-ops/ansible-deps-cache/python-libs/lib/python/site-packages/passlib/handlers/bcrypt.py", line 745, in _load_backend_mixin
    return mixin_cls._finalize_backend_mixin(name, dryrun)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/quatrava/Dev/ops/go-epfl/go-ops/ansible-deps-cache/python-libs/lib/python/site-packages/passlib/handlers/bcrypt.py", line 403, in _finalize_backend_mixin
    result = safe_verify("test", test_hash_20)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/quatrava/Dev/ops/go-epfl/go-ops/ansible-deps-cache/python-libs/lib/python/site-packages/passlib/handlers/bcrypt.py", line 303, in safe_verify
    return verify(secret, hash)
           ^^^^^^^^^^^^^^^^^^^^
  File "/Users/quatrava/Dev/ops/go-epfl/go-ops/ansible-deps-cache/python-libs/lib/python/site-packages/passlib/utils/handlers.py", line 792, in verify
    return consteq(self._calc_checksum(secret), chk)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/quatrava/Dev/ops/go-epfl/go-ops/ansible-deps-cache/python-libs/lib/python/site-packages/passlib/handlers/bcrypt.py", line 764, in _calc_checksum_raw
    hash = _pybcrypt.hashpw(secret, config)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: argument 'salt': 'str' object cannot be converted to 'PyBytes'

Thanks for scrubbing this release from PyPI.

Stefanhg commented 11 months ago

Thank you @reaperhulk for pulling back the release!

gitta-jfrog commented 11 months ago

Hi @reaperhulk

Can you add a reason for the "data-yanked" field of version 4.1.0? Unfortunately, following the pulling back of 4.1.0, Artifactory failed to detect this version as yanked as the value is empty and continued to serve it as the latest version.

It will be highly appreciated if that change is possible. Thanks!

alex commented 11 months ago

I've gone ahead and done so.

(It was a bit fraught, because the only way to do so was to un-yank for a second. Hopefully this didn't disturb anyone!)