Skip to content

Commit

Permalink
Replace passlib with bcrypt
Browse files Browse the repository at this point in the history
- Replaced passlib with bcrypt
- Removed all references to passlib

Change-Id: If27738f7b04218e807bf436251ab9e7b38724801
  • Loading branch information
Zatcmk committed Nov 14, 2023
1 parent dac4275 commit 6865ed0
Show file tree
Hide file tree
Showing 10 changed files with 33 additions and 130 deletions.
2 changes: 0 additions & 2 deletions .layering.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@ package-definitions:
ordereddict: [ordereddict]
paho: [paho]
paramiko: [paramiko]
passlib: [passlib]
Pillow: [PIL]
pipfile: [pipfile]
playwright: [playwright]
Expand Down Expand Up @@ -347,7 +346,6 @@ allowed-package-relationships:
deepdiff,
exchangelib,
jinja2,
passlib,
pycryptodomex,
pyopenssl,
python-dateutil,
Expand Down
2 changes: 0 additions & 2 deletions Pipfile
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ types-mypy-extensions = "*" # used for type checking
types-oauthlib = "*" # used for type checking
types-paho-mqtt = "*" # used for type checking
types-paramiko = "*" # used for type checking
types-passlib = "*" # used for type checking
types-pillow = "*" # used for type checking
types-protobuf = "*" # used for type checking
types-psutil = "*" # used for type checking
Expand Down Expand Up @@ -133,7 +132,6 @@ flask = "~=2.2" # direct dependency
pytz = "==2023.3" # direct dependency
openapi-spec-validator = "==0.5.6" # direct dependency
psutil = "==5.9.4" # needed for omdlib
passlib = "==1.7.4" # needed for omdlib
defusedxml = "==0.7.1" # needed by jira
oauthlib = "==3.2.2" # needed by requests-oauthlib and jira
requests-oauthlib = "==1.3.1" # needed by jira
Expand Down
18 changes: 1 addition & 17 deletions Pipfile.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion cmk/gui/userdb/htpasswd.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
#
# See:
# - https://httpd.apache.org/docs/2.4/misc/password_encryptions.html
# - https://passlib.readthedocs.io/en/stable/lib/passlib.apache.html
#
def hash_password(password: Password) -> PasswordHash:
"""Hash a password
Expand Down
71 changes: 24 additions & 47 deletions cmk/utils/crypto/password_hashing.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,18 @@
# conditions defined in the file COPYING, which is part of this source code package.
"""This module implements password hashing and validation of password hashes.
At the moment it wraps the respective functionality from passlib, with the goal of replacing
passlib in the future.
The password hashing functions return hashes in the Modular Crypto Format
(https://passlib.readthedocs.io/en/stable/modular_crypt_format.html#modular-crypt-format).
The format contains an identifier for the hash algorithm that was used, the number of rounds,
a salt, and the actual checksum -- which is all the information needed to verify the hash with a
given password (see `verify`).
"""
import logging
import re
import sys

# pylint errors are suppressed since this is the only module that should import passlib.
import passlib.context # pylint: disable=passlib-module-import
import passlib.exc # pylint: disable=passlib-module-import
from passlib import hash as passlib_hash # pylint: disable=passlib-module-import
import bcrypt

from cmk.utils.crypto.password import Password, PasswordHash
from cmk.utils.exceptions import MKException
Expand All @@ -29,8 +25,6 @@
# Using code should not be able to change the number of rounds (to unsafe values), but test code
# has to run with reduced rounds. They can be monkeypatched here.
BCRYPT_ROUNDS = 12
# The default modern bcrypt identifier is "2b", but htpasswd will only accept "2y".
BCRYPT_IDENT = "2y"


class PasswordTooLongError(MKException):
Expand Down Expand Up @@ -61,29 +55,14 @@ def hash_password(password: Password, *, allow_truncation: bool = False) -> Pass
:raise: PasswordTooLongError if the provided password is longer than 72 bytes.
:raise: ValueError if the input password contains null bytes.
"""
try:
return PasswordHash(
# The typing stubs for passlib are buggy (incorrect use of multiple inheritance).
passlib_hash.bcrypt.using( # type: ignore[call-arg]
rounds=BCRYPT_ROUNDS, truncate_error=not allow_truncation, ident=BCRYPT_IDENT
).hash(password.raw)
)
except passlib.exc.PasswordTruncateError as e:
raise PasswordTooLongError(e)


_context = passlib.context.CryptContext(
# The only scheme we support is bcrypt. This includes the regular '$2b$' form of the hash,
# as well as Apache's legacy form '$2y$' (which we currently also create).
#
# Other hashing schemes that were supported in the past should have been migrated to bcrypt
# with Werk #14391. For the record, hashes that could be encountered on old installations were
# sha256_crypt, md5_crypt, apr_md5_crypt and des_crypt.
schemes=["bcrypt"],
# There are currently no "deprecated" algorithms that we auto-update on login.
deprecated=[],
bcrypt__ident=BCRYPT_IDENT,
)
if len(password.raw_bytes) <= 72 or allow_truncation:
hash_pass = bcrypt.hashpw(password.raw_bytes, bcrypt.gensalt(BCRYPT_ROUNDS)).decode("utf-8")
assert hash_pass.startswith("$2b$")
return PasswordHash(
"$2y$" + hash_pass.removeprefix("$2b$")
) # Forcing 2y for htpasswd, bcrypt will still verify 2y hashes but not generate them.
raise PasswordTooLongError("Password too long")


def verify(password: Password, password_hash: PasswordHash) -> None:
Expand All @@ -95,28 +74,26 @@ def verify(password: Password, password_hash: PasswordHash) -> None:
:return: None if the password is valid; raises an exception otherwise (see below).
:raise: PasswordInvalidError if the password does not match the hash.
:raise: ValueError - if the hash algorithm in `password_hash` could not be identified.
- if the identified hash algorithm specifies too few rounds.
- if `password` or `password_hash` contain invalid characters (e.g. NUL).
"""
try:
valid = _context.verify(password.raw, password_hash)
except passlib.exc.UnknownHashError:
if not bcrypt.checkpw(password.raw_bytes, password_hash.encode("utf-8")):
logger.warning(
"Invalid hash. Only bcrypt is supported.",
exc_info=sys.exc_info(),
)
raise ValueError("Invalid hash")
if not valid:
raise PasswordInvalidError
if "\0" in password_hash:
raise ValueError("Null character identified in password hash.")
raise PasswordInvalidError("Checkmk failed to validate the provided password")


def is_unsupported_legacy_hash(password_hash: PasswordHash) -> bool:
"""Was the hash algorithm used for this hash once supported but isn't anymore?"""
legacy = ["sha256_crypt", "md5_crypt", "apr_md5_crypt", "des_crypt"]
return (
passlib.context.CryptContext(schemes=legacy + ["bcrypt"]).identify(
password_hash, required=False
)
in legacy
)
regex_list = [
r"^\$5\$(rounds=[0-9]+\$)?[a-zA-Z0-9\/.]{0,16}\$[a-zA-Z0-9\/.]{43}$", # SHA256 crypt
r"^\$1\$[a-zA-Z0-9\/.]{0,8}\$[a-zA-Z0-9\/.]{22}", # MD5 crypt
r"^\$apr1\$[a-zA-Z0-9\/.]{0,8}\$[a-zA-Z0-9\/.]{22}$", # Apache MD5
r"^[a-zA-Z0-9\/.]{13}$", # DES crypt
]
for regex in regex_list:
if re.match(regex, password_hash):
return True
return False
1 change: 0 additions & 1 deletion omd/Licenses.csv
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,6 @@ Python module: opsgenie-sdk,2.1.5,Apache-2.0,https://opensource.org/licenses/Apa
Python module: ordered-set,4.1.0,MIT,https://opensource.org/licenses/MIT,,
Python module: paho-mqtt,1.6.1,EPL-2.0,https://opensource.org/licenses/EPL-2.0,,also BSD-3-Clause
Python module: paramiko,2.10.3,LGPL-2.1,https://opensource.org/licenses/LGPL-2.1,,
Python module: passlib,1.7.4,BSD-3-Clause,https://opensource.org/licenses/BSD-3-Clause,,
Python module: pbr,5.11.1,Apache-2.0,https://opensource.org/licenses/Apache-2.0,,
Python module: pillow,9.5.0,PIL,http://www.pythonware.com/products/pil/license.htm,,
Python module: ply,3.11,BSD-3-Clause,https://opensource.org/licenses/BSD-3-Clause,,
Expand Down
1 change: 0 additions & 1 deletion tests/code_quality/test_pipfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,6 @@ def get_undeclared_dependencies() -> Iterable[Import]:


CEE_UNUSED_PACKAGES = [
"bcrypt", # optional for passlib, we need it
"cython",
"defusedxml",
"docutils",
Expand Down
13 changes: 0 additions & 13 deletions tests/testlib/pylint_checker_forbidden_objects.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ def register(linter: PyLinter) -> None:
linter.register_checker(TypingNamedTupleChecker(linter))
linter.register_checker(SixEnsureStrBinChecker(linter))
linter.register_checker(ABCMetaChecker(linter))
linter.register_checker(PasslibImportChecker(linter))


class ForbiddenObjectChecker(BaseChecker):
Expand Down Expand Up @@ -154,15 +153,3 @@ class ABCMetaChecker(ForbiddenMetaclassChecker):
"Inheritance from ABC should be used instead to define metaclass",
)
}


class PasslibImportChecker(ForbiddenImportChecker):
name = "passlib-module-import"
target_lib = "passlib"
msgs = {
"E9310": (
"Imports passlib",
"passlib-module-import",
"Passlib should not be used directly. Use cmk.utils.crypto.password_hashing instead.",
),
}
10 changes: 8 additions & 2 deletions tests/unit/cmk/utils/crypto/test_password_hashing.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ def test_verify_invalid_password_failure(password: str, password_hash: str) -> N
],
)
def test_verify_invalid_hash_failure(password: str, password_hash: str) -> None:
with pytest.raises(ValueError, match="Invalid hash"):
with pytest.raises(ValueError, match="Invalid salt"):
ph.verify(Password(password), PasswordHash(password_hash))


Expand Down Expand Up @@ -108,7 +108,7 @@ def test_verify_null_bytes(password: str, password_hash: str) -> None:
],
)
def test_verify_invalid_rounds(password: str, pw_hash: str) -> None:
with pytest.raises(ValueError, match="rounds"):
with pytest.raises(ValueError, match="Invalid salt"):
ph.verify(Password(password), PasswordHash(pw_hash))


Expand All @@ -118,6 +118,12 @@ def test_verify_invalid_rounds(password: str, pw_hash: str) -> None:
(True, "$1$49rn5.0y$XoUJMucpN.aQUEOquaj5C/"),
(True, "$apr1$EpPwa/X9$TB2UcQxmrSTJWQQcwHzJM/"),
(True, "WsbFVbJdvDcpY"),
(True, "48c/R8JAv757A"), # Des crypt
(True, "$1$28772684$iEwNOgGugqO9.bIz5sk8k/"), # MD5 Crypt
(
True,
"$5$rounds=5000$GX7BopJZJxPc/KEK$le16UF8I2Anb.rOrn22AUPWvzUETDGefUmAV8AZkGcD",
), # Sha256 crypt
(True, "$5$rounds=1000$.J4mcfJGFGgWJA7R$bDhUCLMe2v1.L3oWclfsVYMyOhsS/6RmyzqFRyCgDi/"),
(False, "foobar"), # ignore unrecognized algorithms
(False, ""),
Expand Down
44 changes: 0 additions & 44 deletions tests/unit/test_pylint_checker_forbidden_objects.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,10 @@
import astroid # type: ignore[import]
import pytest
from pylint.lint import PyLinter
from pylint.testutils import CheckerTestCase, MessageTest

from tests.testlib.pylint_checker_forbidden_objects import (
ABCMetaChecker,
ForbiddenFunctionChecker,
PasslibImportChecker,
SixEnsureStrBinChecker,
TypingNamedTupleChecker,
)
Expand Down Expand Up @@ -305,45 +303,3 @@ class a(metaclass=smth): #@
def test_abcmeta_usage(call_code: str, ref_value: bool, abcmeta_checker: ABCMetaChecker) -> None:
node = astroid.extract_node(call_code)
assert abcmeta_checker._visit_classdef(node) is ref_value


class TestPasslibImportChecker(CheckerTestCase):
CHECKER_CLASS = PasslibImportChecker

@pytest.mark.parametrize(
"code,expect_message",
[
("import passlib #@", True),
("import passlib as findme #@", True),
("import passlib.context #@", True),
("import passlib2 #@", False),
("import passlib2 as passlib #@", False),
("import otherpasslib #@", False),
("import other.passlib #@", False),
],
)
def test_passlib_import(self, code: str, expect_message: bool) -> None:
node = astroid.extract_node(code)
with self.assertAddsMessages(
MessageTest(msg_id=self.checker.name, node=node), ignore_position=True
) if expect_message else self.assertNoMessages():
self.checker.visit_import(node)

@pytest.mark.parametrize(
"code,expect_message",
[
("from passlib import * #@", True),
("from passlib import context #@", True),
("from passlib import context as alias #@", True),
("from passlib.context import CryptContext #@", True),
("from elsewhere import passlib #@", False),
("from elsewhere import passlib as passlib #@", False),
("from passlib2 import context #@", False),
],
)
def test_passlib_importfrom(self, code: str, expect_message: bool) -> None:
node = astroid.extract_node(code)
with self.assertAddsMessages(
MessageTest(msg_id=self.checker.name, node=node), ignore_position=True
) if expect_message else self.assertNoMessages():
self.checker.visit_importfrom(node)

0 comments on commit 6865ed0

Please sign in to comment.