Merge pull request #306 from LilDojd/fix/replace-passlib

Replace passlib with bcrypt

alchemiscale/security/auth.py

@@ -4,21 +4,68 @@
 
 """
 
-from datetime import datetime, timedelta
-from typing import Union, Optional
 import secrets
+import base64
+import hashlib
+from datetime import datetime, timedelta
+from typing import Optional, Union
 
+import bcrypt
 from fastapi import HTTPException, status
 from fastapi.security import OAuth2PasswordBearer
 from jose import JWTError, jwt
-from passlib.context import CryptContext
 from pydantic import BaseModel
 
-from .models import Token, TokenData, CredentialedEntity
+from .models import CredentialedEntity, Token, TokenData
+
+
+# we set a max size to avoid denial-of-service attacks
+# since an extremely large secret attempted by an attacker can take
+# increasing amounts of time or memory to validate;
+# this is deliberately higher than any reasonable key length
+# this is the same max size that `passlib` defaults to
+MAX_SECRET_SIZE = 4096
+# Bcrypt truncates the secret at first NULL it encounters. For this reason,
+# passlib forbids NULL bytes in the secret. This is not necessary for backwards
+# compatibility, but we follow passlib's lead.
+_BNULL = b"\x00"
+
+
+class BcryptPasswordHandler(object):
+
+    def __init__(self, rounds: int = 12, ident: str = "2b"):
+        self.rounds = rounds
+        self.ident = ident
+
+    def hash(self, key: str) -> str:
+        validate_secret(key)
+
+        # generate a salt unique to this key
+        salt = bcrypt.gensalt(rounds=self.rounds, prefix=self.ident.encode("ascii"))
+        hashed_salted = bcrypt.hashpw(key.encode("utf-8"), salt)
+
+        return hashed_salted.decode("utf-8")
+
+    def verify(self, key: str, hashed_salted: str) -> bool:
+        validate_secret(key)
+
+        return bcrypt.checkpw(key.encode("utf-8"), hashed_salted.encode("utf-8"))
 
 
-pwd_context = CryptContext(schemes=["bcrypt"], deprecated="auto")
 oauth2_scheme = OAuth2PasswordBearer(tokenUrl="token")
+pwd_context = BcryptPasswordHandler()
+
+
+def validate_secret(secret: str):
+    """ensure secret has correct type & size"""
+    if not isinstance(secret, str):
+        raise TypeError("secret must be a string")
+    if len(secret) > MAX_SECRET_SIZE:
+        raise ValueError(
+            f"secret is too long, maximum length is {MAX_SECRET_SIZE} characters"
+        )
+    if _BNULL in secret.encode("utf-8"):
+        raise ValueError("secret contains NULL byte")
 
 
 def generate_secret_key():

alchemiscale/tests/unit/test_security.py

@@ -30,3 +30,36 @@ def test_token_data(secret_key):
     token_data = auth.get_token_data(token=token, secret_key=secret_key)
 
     assert token_data.scopes == ["*-*-*"]
+
+
+def test_bcrypt_password_handler():
+    handler = auth.BcryptPasswordHandler()
+    hash_ = handler.hash("test")
+    assert handler.verify("test", hash_)
+    assert not handler.verify("deadbeef", hash_)
+
+
+def test_bcrypt_against_passlib():
+    """Test the our bcrypt handler has the same behavior as passlib did"""
+
+    # pre-generated hash from
+    # `passlib.context.CryptContext(schemes=["bcrypt"], deprecated="auto").hash()`
+    test_password = "the quick brown fox jumps over the lazy dog"
+    test_hash = "$2b$12$QZTnjdx/sJS7jnEnCqhM3uS8mZ4mhLV5dDfM7ZBUT6LwDiNZ2p7S."
+
+    # test that we get the same thing back from our bcrypt handler
+    handler = auth.BcryptPasswordHandler()
+    assert handler.verify(test_password, test_hash)
+
+
+def test_bcrypt_against_passlib_long():
+    """Test the our bcrypt handler has the same behavior as passlib did for passwords longer than 72 characters, in which bcrypt truncates"""
+
+    # pre-generated hash from
+    # `passlib.context.CryptContext(schemes=["bcrypt"], deprecated="auto").hash()`
+    test_password = "this password is so long, it's longer than 72 characters; this should get truncated by bcrypt, so we can ensure we get the same verification behavior as passlib gives"
+    test_hash = "$2b$12$DQd5IPjlc8z4FZjBIdaquOlVc9whAqnkpZRsnuUUWvfHvwWy.dZ16"
+
+    # test that we get the same thing back from our bcrypt handler
+    handler = auth.BcryptPasswordHandler()
+    assert handler.verify(test_password, test_hash)

devtools/conda-envs/alchemiscale-server.yml

@@ -29,7 +29,6 @@ dependencies:
   - uvicorn
   - gunicorn
   - python-jose
-  - passlib
   - bcrypt
   - python-multipart=0.0.12    # temporarily pinned due to broken 0.14 release on conda-forge
   - starlette

devtools/conda-envs/test.yml

@@ -18,7 +18,7 @@ dependencies:
   - monotonic
   - docker-py    # for grolt
 
-  # user client printing 
+  # user client printing
   - rich
 
   ## object store
@@ -29,7 +29,6 @@ dependencies:
   - uvicorn
   - gunicorn
   - python-jose
-  - passlib
   - bcrypt
   - starlette
   - httpx

docs/conf.py

@@ -43,7 +43,6 @@
     "jose",
     "networkx",
     "numpy",
-    "passlib",
     "py2neo",
     "pydantic",
     "starlette",