Skip to content

Commit

Permalink
Use SSHA512 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 9, 2025
1 parent 23b475b commit 28a22f5
Show file tree
Hide file tree
Showing 6 changed files with 21 additions and 13 deletions.
13 changes: 6 additions & 7 deletions components/api_server/src/routes/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,17 +54,16 @@ def set_session_cookie(session_id: SessionId, expires_datetime: datetime) -> Non
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}"
ssha_prefix = b"{SSHA512}"
if not ssha_ldap_salted_password.startswith(ssha_prefix): # pragma: no feature-test-cover
logging.warning("Only SSHA LDAP password digest supported!")
logging.warning("Only SSHA512 LDAP password digest supported!")
raise exceptions.LDAPInvalidAttributeSyntaxResult
digest_size = hashlib.sha512(b"dummy hash to get digest size").digest_size
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
digest = digest_salt[:digest_size]
salt = digest_salt[digest_size:]
sha = hashlib.sha512(password.encode("utf-8") + salt)

Check failure

Code scanning / CodeQL

Use of a broken or weak cryptographic hashing algorithm on sensitive data High

Sensitive data (password)
is used in a hashing algorithm (SHA512) that is insecure for password hashing, since it is not a computationally expensive hash function.
Sensitive data (password)
is used in a hashing algorithm (SHA512) that is insecure for password hashing, since it is not a computationally expensive hash function.
Sensitive data (password)
is used in a hashing algorithm (SHA512) that is insecure for password hashing, since it is not a computationally expensive hash function.
return digest == sha.digest()


Expand Down
10 changes: 6 additions & 4 deletions components/api_server/tests/routes/test_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,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"{SSHA512}j9W7XoTPEcAVdG1zAodfj2ZETkKR3SDeI/Cm7WGbHyjWAGUik/69KKd5dWSdmxEtEHAmhplAZpwElJnp+C4SB6AeR5xa68+S"
)
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,20 +193,20 @@ 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 salted SHA512."""
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.assertEqual("Only SSHA512 LDAP password digest supported!", logging_mock.call_args_list[0][0][0])
self.assert_log(logging_mock, exceptions.LDAPInvalidAttributeSyntaxResult)

@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"{SSHA512}W841/abcdefghijklmnopqrstuvwxyz0"
connection_enter.return_value = self.ldap_connection
self.assertEqual(self.login_nok, login(self.database))
self.assert_ldap_connection_search_called()
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: {SSHA512}EZIwikIRmd1JP1eutDHsPYIVy8aGGU07y+i9kBJ0gOmaxVEQd9caGneB50HmHURAUmjiGcd/eanoM1cMqKEyKiLGw3ucsUhS
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: {SSHA512}j9W7XoTPEcAVdG1zAodfj2ZETkKR3SDeI/Cm7WGbHyjWAGUik/69KKd5dWSdmxEtEHAmhplAZpwElJnp+C4SB6AeR5xa68+S
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 SSHA512 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).

## v5.24.0 - 2025-02-06
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 28a22f5

Please sign in to comment.