Skip to content

Commit

Permalink
Use ARGON2 hashes to verify user LDAP passwords instead of SSHA1.
Browse files Browse the repository at this point in the history
Fixes #6233.
  • Loading branch information
fniessink committed Feb 10, 2025
1 parent 1192af7 commit 1f4f196
Show file tree
Hide file tree
Showing 9 changed files with 88 additions and 33 deletions.
1 change: 1 addition & 0 deletions components/api_server/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
31 changes: 30 additions & 1 deletion components/api_server/requirements/requirements-dev.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 \
Expand Down
31 changes: 30 additions & 1 deletion components/api_server/requirements/requirements.txt
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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 \
Expand Down
30 changes: 7 additions & 23 deletions components/api_server/src/routes/auth.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
"""Login/logout."""

import base64
import hashlib
import logging
import os
import re
Expand All @@ -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
Expand Down Expand Up @@ -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]:
Expand Down Expand Up @@ -108,14 +95,11 @@ 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 succeeded")
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)
logging.info("LDAP bind succeeded")
except Exception as reason: # noqa: BLE001
user = User(username)
logging.warning("LDAP error: %s", reason)
Expand Down
17 changes: 11 additions & 6 deletions components/api_server/tests/routes/test_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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):
Expand Down
2 changes: 1 addition & 1 deletion docker/ldap_ldifs/jane_doe.ldif
Original file line number Diff line number Diff line change
Expand Up @@ -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: [email protected]
gecos: Jane User
2 changes: 1 addition & 1 deletion docker/ldap_ldifs/john_doe.ldif
Original file line number Diff line number Diff line change
Expand Up @@ -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: [email protected]
gecos: John User
1 change: 1 addition & 0 deletions docs/src/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).

Expand Down
6 changes: 6 additions & 0 deletions docs/src/software.md
Original file line number Diff line number Diff line change
Expand Up @@ -453,3 +453,9 @@ The LDAP database has two users:
|----------|-----------------------|----------|----------|
| Jane Doe | `[email protected]` | `jadoe` | `secret` |
| John Doe | `[email protected]` | `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
```

0 comments on commit 1f4f196

Please sign in to comment.