From 10154cb19194847d001357f3af8a4aabbe963d53 Mon Sep 17 00:00:00 2001 From: Frank Niessink Date: Sun, 9 Feb 2025 17:36:19 +0100 Subject: [PATCH] Use ARGON2 hashes to verify user LDAP passwords instead of SSHA1. Fixes #6233. --- components/api_server/pyproject.toml | 1 + .../requirements/requirements-dev.txt | 31 ++++++++++++++++++- .../api_server/requirements/requirements.txt | 31 ++++++++++++++++++- components/api_server/src/routes/auth.py | 28 ++++------------- .../api_server/tests/routes/test_auth.py | 17 ++++++---- docker/ldap_ldifs/jane_doe.ldif | 2 +- docker/ldap_ldifs/john_doe.ldif | 2 +- docs/src/changelog.md | 1 + docs/src/software.md | 6 ++++ 9 files changed, 87 insertions(+), 32 deletions(-) diff --git a/components/api_server/pyproject.toml b/components/api_server/pyproject.toml index a2974ec81f..6c3f7c18d3 100644 --- a/components/api_server/pyproject.toml +++ b/components/api_server/pyproject.toml @@ -7,6 +7,7 @@ classifiers = [ "Programming Language :: Python :: 3.13", ] dependencies = [ + "argon2-cffi==23.1.0", "bottle==0.13.2", "cryptography==44.0.0", "gevent==24.11.1", diff --git a/components/api_server/requirements/requirements-dev.txt b/components/api_server/requirements/requirements-dev.txt index 13344487d3..a169521222 100644 --- a/components/api_server/requirements/requirements-dev.txt +++ b/components/api_server/requirements/requirements-dev.txt @@ -4,6 +4,33 @@ annotated-types==0.7.0 \ --hash=sha256:1f02e8b43a8fbbc3f3e0d4f0f4bfc8131bcb4eebe8849b8e5c773f3a1c582a53 \ --hash=sha256:aff07c09a53a08bc8cfccb9c85b05f1aa9a2a6f23728d790723543408344ce89 # via pydantic +argon2-cffi==23.1.0 \ + --hash=sha256:879c3e79a2729ce768ebb7d36d4609e3a78a4ca2ec3a9f12286ca057e3d0db08 \ + --hash=sha256:c670642b78ba29641818ab2e68bd4e6a78ba53b7eff7b4c3815ae16abf91c7ea + # via api-server (pyproject.toml) +argon2-cffi-bindings==21.2.0 \ + --hash=sha256:20ef543a89dee4db46a1a6e206cd015360e5a75822f76df533845c3cbaf72670 \ + --hash=sha256:2c3e3cc67fdb7d82c4718f19b4e7a87123caf8a93fde7e23cf66ac0337d3cb3f \ + --hash=sha256:3b9ef65804859d335dc6b31582cad2c5166f0c3e7975f324d9ffaa34ee7e6583 \ + --hash=sha256:3e385d1c39c520c08b53d63300c3ecc28622f076f4c2b0e6d7e796e9f6502194 \ + --hash=sha256:58ed19212051f49a523abb1dbe954337dc82d947fb6e5a0da60f7c8471a8476c \ + --hash=sha256:5e00316dabdaea0b2dd82d141cc66889ced0cdcbfa599e8b471cf22c620c329a \ + --hash=sha256:603ca0aba86b1349b147cab91ae970c63118a0f30444d4bc80355937c950c082 \ + --hash=sha256:6a22ad9800121b71099d0fb0a65323810a15f2e292f2ba450810a7316e128ee5 \ + --hash=sha256:8cd69c07dd875537a824deec19f978e0f2078fdda07fd5c42ac29668dda5f40f \ + --hash=sha256:93f9bf70084f97245ba10ee36575f0c3f1e7d7724d67d8e5b08e61787c320ed7 \ + --hash=sha256:9524464572e12979364b7d600abf96181d3541da11e23ddf565a32e70bd4dc0d \ + --hash=sha256:b2ef1c30440dbbcba7a5dc3e319408b59676e2e039e2ae11a8775ecf482b192f \ + --hash=sha256:b746dba803a79238e925d9046a63aa26bf86ab2a2fe74ce6b009a1c3f5c8f2ae \ + --hash=sha256:bb89ceffa6c791807d1305ceb77dbfacc5aa499891d2c55661c6459651fc39e3 \ + --hash=sha256:bd46088725ef7f58b5a1ef7ca06647ebaf0eb4baff7d1d0d177c6cc8744abd86 \ + --hash=sha256:ccb949252cb2ab3a08c02024acb77cfb179492d5701c7cbdbfd776124d4d2367 \ + --hash=sha256:d4966ef5848d820776f5f562a7d45fdd70c2f330c961d0d745b784034bd9f48d \ + --hash=sha256:e415e3f62c8d124ee16018e491a009937f8cf7ebf5eb430ffc5de21b900dad93 \ + --hash=sha256:ed2937d286e2ad0cc79a7087d3c272832865f779430e0cc2b4f3718d3159b0cb \ + --hash=sha256:f1152ac548bd5b8bcecfb0b0371f082037e47128653df2e8ba6e914d384f3c3e \ + --hash=sha256:f9f8b450ed0547e3d473fdc8612083fd08dd2120d6ac8f73828df9b7d45bb351 + # via argon2-cffi bottle==0.13.2 \ --hash=sha256:27569ab8d1332fbba3e400b3baab2227ab4efb4882ff147af05a7c00ed73409c \ --hash=sha256:e53803b9d298c7d343d00ba7d27b0059415f04b9f6f40b8d58b5bf914ba9d348 @@ -80,7 +107,9 @@ cffi==1.17.1 \ --hash=sha256:f79fc4fc25f1c8698ff97788206bb3c2598949bfe0fef03d299eb1b5356ada99 \ --hash=sha256:f7f5baafcc48261359e14bcd6d9bff6d4b28d9103847c9e136694cb0501aef87 \ --hash=sha256:fc48c783f9c87e60831201f2cce7f3b2e4846bf4d8728eabe54d60700b318a0b - # via cryptography + # via + # argon2-cffi-bindings + # cryptography charset-normalizer==3.4.1 \ --hash=sha256:0167ddc8ab6508fe81860a57dd472b2ef4060e8d378f0cc555707126830f2537 \ --hash=sha256:01732659ba9b5b873fc117534143e4feefecf3b2078b0a6a2e925271bb6f4cfa \ diff --git a/components/api_server/requirements/requirements.txt b/components/api_server/requirements/requirements.txt index cb462c04f3..a752a42739 100644 --- a/components/api_server/requirements/requirements.txt +++ b/components/api_server/requirements/requirements.txt @@ -1,5 +1,32 @@ # This file was autogenerated by uv via the following command: # ci/pip-compile.sh +argon2-cffi==23.1.0 \ + --hash=sha256:879c3e79a2729ce768ebb7d36d4609e3a78a4ca2ec3a9f12286ca057e3d0db08 \ + --hash=sha256:c670642b78ba29641818ab2e68bd4e6a78ba53b7eff7b4c3815ae16abf91c7ea + # via api-server (pyproject.toml) +argon2-cffi-bindings==21.2.0 \ + --hash=sha256:20ef543a89dee4db46a1a6e206cd015360e5a75822f76df533845c3cbaf72670 \ + --hash=sha256:2c3e3cc67fdb7d82c4718f19b4e7a87123caf8a93fde7e23cf66ac0337d3cb3f \ + --hash=sha256:3b9ef65804859d335dc6b31582cad2c5166f0c3e7975f324d9ffaa34ee7e6583 \ + --hash=sha256:3e385d1c39c520c08b53d63300c3ecc28622f076f4c2b0e6d7e796e9f6502194 \ + --hash=sha256:58ed19212051f49a523abb1dbe954337dc82d947fb6e5a0da60f7c8471a8476c \ + --hash=sha256:5e00316dabdaea0b2dd82d141cc66889ced0cdcbfa599e8b471cf22c620c329a \ + --hash=sha256:603ca0aba86b1349b147cab91ae970c63118a0f30444d4bc80355937c950c082 \ + --hash=sha256:6a22ad9800121b71099d0fb0a65323810a15f2e292f2ba450810a7316e128ee5 \ + --hash=sha256:8cd69c07dd875537a824deec19f978e0f2078fdda07fd5c42ac29668dda5f40f \ + --hash=sha256:93f9bf70084f97245ba10ee36575f0c3f1e7d7724d67d8e5b08e61787c320ed7 \ + --hash=sha256:9524464572e12979364b7d600abf96181d3541da11e23ddf565a32e70bd4dc0d \ + --hash=sha256:b2ef1c30440dbbcba7a5dc3e319408b59676e2e039e2ae11a8775ecf482b192f \ + --hash=sha256:b746dba803a79238e925d9046a63aa26bf86ab2a2fe74ce6b009a1c3f5c8f2ae \ + --hash=sha256:bb89ceffa6c791807d1305ceb77dbfacc5aa499891d2c55661c6459651fc39e3 \ + --hash=sha256:bd46088725ef7f58b5a1ef7ca06647ebaf0eb4baff7d1d0d177c6cc8744abd86 \ + --hash=sha256:ccb949252cb2ab3a08c02024acb77cfb179492d5701c7cbdbfd776124d4d2367 \ + --hash=sha256:d4966ef5848d820776f5f562a7d45fdd70c2f330c961d0d745b784034bd9f48d \ + --hash=sha256:e415e3f62c8d124ee16018e491a009937f8cf7ebf5eb430ffc5de21b900dad93 \ + --hash=sha256:ed2937d286e2ad0cc79a7087d3c272832865f779430e0cc2b4f3718d3159b0cb \ + --hash=sha256:f1152ac548bd5b8bcecfb0b0371f082037e47128653df2e8ba6e914d384f3c3e \ + --hash=sha256:f9f8b450ed0547e3d473fdc8612083fd08dd2120d6ac8f73828df9b7d45bb351 + # via argon2-cffi bottle==0.13.2 \ --hash=sha256:27569ab8d1332fbba3e400b3baab2227ab4efb4882ff147af05a7c00ed73409c \ --hash=sha256:e53803b9d298c7d343d00ba7d27b0059415f04b9f6f40b8d58b5bf914ba9d348 @@ -76,7 +103,9 @@ cffi==1.17.1 \ --hash=sha256:f79fc4fc25f1c8698ff97788206bb3c2598949bfe0fef03d299eb1b5356ada99 \ --hash=sha256:f7f5baafcc48261359e14bcd6d9bff6d4b28d9103847c9e136694cb0501aef87 \ --hash=sha256:fc48c783f9c87e60831201f2cce7f3b2e4846bf4d8728eabe54d60700b318a0b - # via cryptography + # via + # argon2-cffi-bindings + # cryptography charset-normalizer==3.4.1 \ --hash=sha256:0167ddc8ab6508fe81860a57dd472b2ef4060e8d378f0cc555707126830f2537 \ --hash=sha256:01732659ba9b5b873fc117534143e4feefecf3b2078b0a6a2e925271bb6f4cfa \ diff --git a/components/api_server/src/routes/auth.py b/components/api_server/src/routes/auth.py index 16b77b631b..18b2e9bf08 100644 --- a/components/api_server/src/routes/auth.py +++ b/components/api_server/src/routes/auth.py @@ -1,7 +1,5 @@ """Login/logout.""" -import base64 -import hashlib import logging import os import re @@ -10,6 +8,7 @@ from http.cookies import Morsel from typing import cast +import argon2 import bottle from ldap3 import ALL, Connection, Server, ServerPool, AUTO_BIND_NO_TLS from ldap3.core import exceptions @@ -51,21 +50,9 @@ def set_session_cookie(session_id: SessionId, expires_datetime: datetime) -> Non bottle.response.set_cookie("session_id", session_id, **options) -def check_password(ssha_ldap_salted_password, password) -> bool: - """Check the OpenLDAP tagged digest against the given password.""" - # See https://www.openldap.org/doc/admin24/security.html#SSHA%20password%20storage%20scheme - # We should (also) support SHA512 as SHA1 is no longer considered to be secure. - ssha_prefix = b"{SSHA}" - if not ssha_ldap_salted_password.startswith(ssha_prefix): # pragma: no feature-test-cover - logging.warning("Only SSHA LDAP password digest supported!") - raise exceptions.LDAPInvalidAttributeSyntaxResult - digest_salt_b64 = ssha_ldap_salted_password.removeprefix(ssha_prefix) - digest_salt = base64.b64decode(digest_salt_b64) - digest = digest_salt[:20] - salt = digest_salt[20:] - sha = hashlib.sha1(bytes(password, "utf-8")) # nosec, # noqa: S324 - sha.update(salt) # nosec - return digest == sha.digest() +def check_password(password_hash, password) -> bool: + """Check the OpenLDAP password hash against the given password.""" + return argon2.PasswordHasher().verify(password_hash.decode().removeprefix("{ARGON2}"), password) def get_credentials() -> tuple[str, str]: @@ -108,11 +95,8 @@ def verify_user(database: Database, username: str, password: str) -> User: attributes=["userPassword", "cn", "mail"], ) result = lookup_connection.entries[0] - if salted_password := result.userPassword.value: - if check_password(salted_password, password): - logging.info("LDAP salted password check for %s succeeded", username) - else: - raise exceptions.LDAPInvalidCredentialsResult # noqa: TRY301 + if (password_hash := result.userPassword.value) and check_password(password_hash, password): + logging.info("LDAP password check for %s succeeded", username) else: # pragma: no feature-test-cover with Connection(ldap_server_pool, user=result.entry_dn, password=password, auto_bind=AUTO_BIND_NO_TLS): logging.info("LDAP bind for %s succeeded", username) diff --git a/components/api_server/tests/routes/test_auth.py b/components/api_server/tests/routes/test_auth.py index dab59c6f47..50bff17856 100644 --- a/components/api_server/tests/routes/test_auth.py +++ b/components/api_server/tests/routes/test_auth.py @@ -4,6 +4,7 @@ from datetime import datetime, timedelta, UTC from unittest.mock import Mock, patch +import argon2 import bottle import ldap3 from ldap3.core import exceptions @@ -138,7 +139,9 @@ def test_forwardauth_login_no_header(self, connection_mock, connection_enter): def test_successful_login(self, connection_mock, connection_enter): """Test successful login.""" connection_mock.return_value = None - self.ldap_entry.userPassword.value = b"{SSHA}W841/YybjO4TmqcNTqnBxFKd3SJggaPr" + self.ldap_entry.userPassword.value = ( + b"{ARGON2}$argon2id$v=19$m=65536,t=3,p=4$rTmNv6FmvVQaSIXzLvcStQ$F2qtxHsklGS+sgWMrbekjeUn4bWbMvt3Liqsw7jOV1I" + ) connection_enter.return_value = self.ldap_connection self.assertEqual(self.login_ok, login(self.database)) self.assert_cookie_has_session_id() @@ -191,24 +194,26 @@ def test_login_search_error(self, logging_mock, connection_mock, connection_ente @patch.object(logging, "warning") def test_login_password_hash_error(self, logging_mock, connection_mock, connection_enter): - """Test login fails when LDAP password hash is not salted SHA1.""" + """Test login fails when LDAP password hash is not ARGON2.""" connection_mock.return_value = None self.ldap_entry.userPassword.value = b"{XSHA}whatever-here" connection_enter.return_value = self.ldap_connection self.assertEqual(self.login_nok, login(self.database)) self.assert_ldap_connection_search_called() - self.assertEqual("Only SSHA LDAP password digest supported!", logging_mock.call_args_list[0][0][0]) - self.assert_log(logging_mock, exceptions.LDAPInvalidAttributeSyntaxResult) + self.assertEqual("LDAP error: %s", logging_mock.call_args_list[0][0][0]) + self.assert_log(logging_mock, argon2.exceptions.InvalidHashError) @patch.object(logging, "warning") def test_login_wrong_password(self, logging_mock, connection_mock, connection_enter): """Test login when search error of the login user occurs.""" connection_mock.return_value = None - self.ldap_entry.userPassword.value = b"{SSHA}W841/abcdefghijklmnopqrstuvwxyz0" + self.ldap_entry.userPassword.value = ( + b"{ARGON2}$argon2id$v=19$m=65536,t=3,p=4$rTmNv6FmvvQaSIXzLvcStQ$F2qtxHsklGS+sgWMrbekjeUn4bWbMvt3Liqsw7jOV1I" + ) connection_enter.return_value = self.ldap_connection self.assertEqual(self.login_nok, login(self.database)) self.assert_ldap_connection_search_called() - self.assert_log(logging_mock, exceptions.LDAPInvalidCredentialsResult) + self.assert_log(logging_mock, argon2.exceptions.VerifyMismatchError) @patch("routes.auth.datetime", MOCK_DATETIME) def test_login_changed_details(self, connection_mock, connection_enter): diff --git a/docker/ldap_ldifs/jane_doe.ldif b/docker/ldap_ldifs/jane_doe.ldif index d8f5ca2ed2..e01bb51eeb 100644 --- a/docker/ldap_ldifs/jane_doe.ldif +++ b/docker/ldap_ldifs/jane_doe.ldif @@ -10,6 +10,6 @@ loginShell: /bin/bash homeDirectory: /home/jadoe uidNumber: 14583103 gidNumber: 14564100 -userPassword: {SSHA}/mWnh0bZyUvrHwKccTa/D8HpzfUc8+js +userPassword: {ARGON2}$argon2id$v=19$m=65536,t=3,p=4$rTmNv6FmvVQaSIXzLvcStQ$F2qtxHsklGS+sgWMrbekjeUn4bWbMvt3Liqsw7jOV1I mail: janedoe@example.org gecos: Jane User diff --git a/docker/ldap_ldifs/john_doe.ldif b/docker/ldap_ldifs/john_doe.ldif index bdf7acfa72..100c262ef8 100644 --- a/docker/ldap_ldifs/john_doe.ldif +++ b/docker/ldap_ldifs/john_doe.ldif @@ -10,6 +10,6 @@ loginShell: /bin/bash homeDirectory: /home/jodoe uidNumber: 14583102 gidNumber: 14564100 -userPassword: {SSHA}CIa/0T/K5jws0gCxfdtndFit8aMhttPB +userPassword: {ARGON2}$argon2id$v=19$m=65536,t=3,p=4$aH5Wxb8qIkQL6qT4Uv6iMg$/Lp5apcrXLvFX1GRdWCZKFSxtS7Dvg2vQivs51qtFEk mail: johndoe@example.org gecos: John User diff --git a/docs/src/changelog.md b/docs/src/changelog.md index 7efa4ab750..fc3f0c0d50 100644 --- a/docs/src/changelog.md +++ b/docs/src/changelog.md @@ -16,6 +16,7 @@ If your currently installed *Quality-time* version is not the latest version, pl ### Fixed +- Use ARGON2 hashes to verify user LDAP passwords instead of SSHA1. Fixes [#6233](https://github.com/ICTU/quality-time/issues/6233). - Make the API-server return HTTP status 404 on non-existing endpoints instead of 200. Fixes [#9860](https://github.com/ICTU/quality-time/issues/9860). - Input to a multiple choice input fields, such as the metric tags field and the issue identifiers field, would not saved on tab. Fixes [#10814](https://github.com/ICTU/quality-time/issues/10814). diff --git a/docs/src/software.md b/docs/src/software.md index e9a5033012..4fe7738b41 100644 --- a/docs/src/software.md +++ b/docs/src/software.md @@ -453,3 +453,9 @@ The LDAP database has two users: |----------|-----------------------|----------|----------| | Jane Doe | `janedoe@example.org` | `jadoe` | `secret` | | John Doe | `johndoe@example.org` | `jodoe` | `secret` | + +The `{SSHA512}` hashes for the `userPassword`s in the LDIF-files were generated using [pySSHA-slapd](https://github.com/peppelinux/pySSHA-slapd): + +```console +python3 ssha.py -p secret -salt_size 8 -enc ssha512 +```