Skip to content

Consider the future of passlib and password hashing/upgrading #15454

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
miketheman opened this issue Feb 22, 2024 · 8 comments
Open

Consider the future of passlib and password hashing/upgrading #15454

miketheman opened this issue Feb 22, 2024 · 8 comments
Labels
dependencies Pull requests that update a dependency file needs discussion a product management/policy issue maintainers and users should discuss security Security-related issues and pull requests

Comments

@miketheman
Copy link
Member

We use passlib in warehouse as part of our user account management service:

self.hasher = CryptContext(
schemes=[
"argon2",
"bcrypt_sha256",
"bcrypt",
"django_bcrypt",
"unix_disabled",
],
deprecated=["auto"],
truncate_error=True,
# Argon 2 Configuration
argon2__memory_cost=1024,
argon2__parallelism=6,
argon2__time_cost=6,
)

The TL;DR of what this does is allows user password hash algorithms to evolve over time, and as users log in with their passwords they are confirmed and replaced with the newer (presumably more secure) hash algorithms, preventing the user from needing to reset a password only to get the latest and greatest algorithm.

passlib hasher docs can be found here: https://passlib.readthedocs.io/en/stable/lib/passlib.hash.html

The most recent release of passlib was in 2020, and raises warnings for using crpyt, which will turn into breakages under Python 3.13, so this is not yet a blocker, it's something we should consider long before it becomes one.

Here's an issue for maintenance status that has yet to be resolved, either by nominating new maintainers, or some other resolution.

In the interim another contender has emerged - pwdlib (author launch blog post), which appears to have argon2 and bcrypt support.

So in theory, we could leverage pwdlib and continue to leverage the upgradability-behavior, however we'd still need to account for folks that have yet to log in in modern times and the lack of older algo support in pwdlib.
pwdlib also does not yet support disable() or is_enabled() which we use today, but could be replaced by using the boolean flag User.is_active or such.

Alternately, since it's not urgent yet, we can continue to observe the evolving space around passlib and hope that a new maintenance team arises before it becomes a severe issue.


Some SQL counting:

warehouse=> SELECT
  CASE
    WHEN password LIKE '$argon2%' THEN 'argon2'
    WHEN password LIKE '$bcrypt-sha256%' THEN 'bcrypt_sha256'
    WHEN password LIKE '$2b$%' THEN 'bcrypt'
    WHEN password LIKE 'bcrypt$%' THEN 'django_bcrypt'
    WHEN password LIKE 'spammer' THEN 'disabled'
    WHEN password LIKE '!' THEN 'disabled'
    ELSE 'other'
  END AS hash_type,
  COUNT(*)
FROM users
GROUP BY hash_type
ORDER BY COUNT(*) DESC;
   hash_type   | count
---------------+--------
 argon2        | 671847
 bcrypt_sha256 |  76452
 disabled      |  28087
 django_bcrypt |  10930
(4 rows)
@miketheman miketheman added needs discussion a product management/policy issue maintainers and users should discuss security Security-related issues and pull requests labels Feb 22, 2024
@JacobCoffee
Copy link
Member

Some relevant things: canonical/cloud-init#4791

@frankie567
Copy link

Hey, pwdlib maintainer here 👋

I understand this is not a critical priority for warehouse at the moment, but if you need specific features and/or algorithms so you can replace passlib, I would be glad to discuss it 🙂

@miketheman
Copy link
Member Author

Hi @frankie567 ! Thanks for asking.

We currently use passlib "lightly" in warehouse.
I linked above the to CryptContext in use - specifically the algos still in use are listed there - I don't think pwdlib has support yet for the algos we have in use today.

We also use the verify_and_update() function, it does most of the heavy lifting:

# Actually check our hash, optionally getting a new hash for it if
# we should upgrade our saved hashed.
try:
ok, new_hash = self.hasher.verify_and_update(password, user.password)
except passlib.exc.PasswordValueError:
ok = False

Other functions we use are hash(), verify() which I think pwdlib supports, disable(), and is_enabled() which I don't think are supported yet.

@frankie567
Copy link

Thank you for those details, Mike 👍

I confirm pwdlib supports verify_and_update, hash and verify.

I'll have a look at the algorithms you use. Regarding the enable/disable feature, I'll consider it, even though currently I believe this is something that should be handled at user's level (with a flag like you suggest) rather than at password's level.

@miketheman
Copy link
Member Author

miketheman commented Jan 9, 2025

There's drop-in a fork here: https://pypi.org/project/libpass/ - it appears to be sufficient.

A diff:

diff --git a/requirements/main.in b/requirements/main.in
index c80c84e57..21215c60f 100644
--- a/requirements/main.in
+++ b/requirements/main.in
@@ -25,6 +25,7 @@ humanize
 itsdangerous
 Jinja2>=2.8
 kombu[sqs]>=5.4,<5.5  # https://github.com/jazzband/pip-tools/issues/1577
+libpass  # maintained fork of `passlib`
 limits
 linehaul
 lxml
@@ -38,7 +39,6 @@ packaging>=24.2
 packaging_legacy
 paginate>=0.5.2
 paginate_sqlalchemy
-passlib>=1.6.4
 platformdirs>=4.2
 premailer
 psycopg[c]
diff --git a/requirements/main.txt b/requirements/main.txt
index eb6f266fa..207aac998 100644
--- a/requirements/main.txt
+++ b/requirements/main.txt
@@ -994,6 +994,14 @@ lazy-object-proxy==1.10.0 \
     --hash=sha256:edb45bb8278574710e68a6b021599a10ce730d156e5b254941754a9cc0b17d03 \
     --hash=sha256:fec03caabbc6b59ea4a638bee5fce7117be8e99a4103d9d5ad77f15d6f81020c
     # via openapi-spec-validator
+legacycrypt==0.3 \
+    --hash=sha256:b5e373506ccb442f8d715e29fa75f53a11bbec3ca0d7b63445f4dbb656555218 \
+    --hash=sha256:e76e7fd25666a451428b20d5afbbecf3654565b2e11511b53226be955c4d2292
+    # via libpass
+libpass==1.8.1 \
+    --hash=sha256:9eef494740e3bc1455935cba21f91d2e0a8dac1ae9980bd171fbacd25517a9ba \
+    --hash=sha256:a4a4a936ddb8a886ca6d2c37859bff1f6de25f57eb74466d01454a7fb6c5ce1b
+    # via -r requirements/main.in
 limits==3.14.1 \
     --hash=sha256:051aca02da56e6932599a25cb8e70543959294f5d587d57bcd7e38df234e697b \
     --hash=sha256:cad16a9b3cf3924e27da48e78bdab33ef312ecb7194fdb50e509cc8111c8d0bb
@@ -1552,10 +1560,6 @@ parse==1.20.2 \
     --hash=sha256:967095588cb802add9177d0c0b6133b5ba33b1ea9007ca800e526f42a85af558 \
     --hash=sha256:b41d604d16503c79d81af5165155c0b20f6c8d6c559efa66b4b695c3e5a0a0ce
     # via openapi-core
-passlib==1.7.4 \
-    --hash=sha256:aa6bca462b8d8bda89c70b382f0c298a20b5560af6cbfa2dce410c0a2fb669f1 \
-    --hash=sha256:defd50f72b65c5402ab2c573830a6978e5f202ad0d984793c8dde2c4152ebe04
-    # via -r requirements/main.in
 pastedeploy==3.1.0 \
     --hash=sha256:76388ad53a661448d436df28c798063108f70e994ddc749540d733cdbd1b38cf \
     --hash=sha256:9ddbaf152f8095438a9fe81f82c78a6714b92ae8e066bed418b6a7ff6a095a95

And it looks like there's work underway to remove the need for legacycrypt that hasn't landed just yet.

@ewdurbin
Copy link
Member

Researched while working on #17447. It appears that our use-case isn't impacted by the deprecation.

I added at test in 32677e1 that succeeds before/after updating to Python3.13.

@ewdurbin
Copy link
Member

Note on why the removal of crypt in 3.13 did not break us: passlib has a fallback in that event that appears to not impact any of the algorithms we are using 🤷🏼

@miketheman miketheman added the dependencies Pull requests that update a dependency file label Jan 22, 2025
@notypecheck
Copy link

@miketheman Hey 👋

Hashing schemes you're using generally shouldn't be affected by crypt removal since you already have optional dependencies such as bcrypt and argon2-cffi installed.
Generally passlib would first try to use those, and if they're not available - fallback to a built-in implementation or try to use OS crypt API, but I decided to drop builtin and os_crypt implementations where possible.
By the way, do you have tests for all 4 password hashing schemes there?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file needs discussion a product management/policy issue maintainers and users should discuss security Security-related issues and pull requests
Projects
None yet
Development

No branches or pull requests

5 participants