fix(security): replace unsalted MD5 password hashing with PBKDF2 (CWE-327/CWE-916)

[sebastiondev]

Summary

Replace unsalted MD5 password hashing with Django's PBKDF2-SHA256 (make_password / check_password) for all user account passwords, while preserving login compatibility for any existing MD5-hashed passwords through transparent rehashing on next successful login.

Vulnerability

• Class: CWE-327 (Use of a Broken or Risky Cryptographic Algorithm) / CWE-916 (Use of Password Hash With Insufficient Computational Effort)
• Affected function: password_encrypt() in apps/common/utils/common.py
• Affected flow: every code path that stores a user password — admin seeding (apps/users/migrations/0001_initial.py), user creation, password reset, profile update — and the login flow in apps/users/serializers/login.py that compared password=password_encrypt(password) directly in the DB query.

The previous implementation was:

def password_encrypt(row_password):
    md5 = hashlib.md5()
    md5.update(row_password.encode())
    return md5.hexdigest()

This is unsalted MD5 with no work factor. Concretely:

1. The default admin user is seeded by migration with password_encrypt('MaxKB@123..'), producing the deterministic hash d880e722c47a34d8e9fce789fc62389d — a value previously hard-coded in UserProfileSerializer to detect "still using default password". Identical passwords across users produce identical hashes.
2. Any compromise that leaks the user table (SQL injection, backup leak, ops insider, dependency RCE, etc.) yields hashes that are trivially reversed via rainbow tables / GPU brute force at billions of guesses per second.
3. Recovered plaintext passwords enable credential stuffing against email, SSO, and other services where users reuse passwords — a real impact that DB read access alone does not provide.

The login serializer also filtered users by password=password_encrypt(password), which both enforces the unsalted-hash design (you can't filter by a salted hash) and mixes the secret comparison into a SQL WHERE clause rather than a constant-time compare.

Fix

apps/common/utils/common.py:

• password_encrypt() now delegates to Django's make_password() (PBKDF2-SHA256, per-password salt, ~600k iterations by default in modern Django).
• Added password_verify() that first calls check_password() (handles PBKDF2 and any future Django hasher) and falls back to a constant-shape MD5 check only when the stored value matches the legacy 32-hex-char shape.
• Added needs_password_upgrade() to detect legacy hashes for transparent rehashing.

apps/users/serializers/login.py:

• Replaced the User.objects.filter(username=..., password=password_encrypt(...)) lookup with a username lookup followed by password_verify(password, user.password).
• On successful login against a legacy MD5 hash, the stored hash is rewritten to PBKDF2 (user.save(update_fields=['password'])).

apps/users/serializers/user.py:

• Replaced the hard-coded MD5 comparison user.password == 'd880e722c47a34d8e9fce789fc62389d' (used to flag "default password still set") with password_verify(CONFIG.get('DEFAULT_PASSWORD', 'MaxKB@123..'), user.password), which works for both legacy and PBKDF2 hashes.

All write-side callers of password_encrypt() (creation, reset, profile update, etc.) automatically store PBKDF2 output without further changes.

What was tested

• Verified make_password() / check_password() round-trip for a sample password produces a PBKDF2 hash and verifies correctly.
• Verified _is_legacy_md5_hash() accepts a 32-hex-char string and rejects PBKDF2-formatted strings (which contain $ separators).
• Walked the login flow end-to-end:
  • Fresh user → hash stored as pbkdf2_sha256$..., login succeeds via check_password.
  • Pre-existing MD5 hash → first login succeeds via the legacy fallback, then needs_password_upgrade() triggers and the row is rewritten to PBKDF2; subsequent logins go through the PBKDF2 path.
  • Wrong password → password_verify returns False, _handle_failed_login runs as before; lockout/captcha logic is unchanged.
• Confirmed no other code paths still call hashlib.md5 for password storage; the only remaining MD5 use is the explicit legacy-fallback helper.
• Diff is intentionally minimal: 3 files, ~66 insertions / 16 deletions.

Security analysis

• Exploitability of the original bug: any read of the user table reveals MD5 hashes that can be cracked offline at GPU speeds; identical passwords across users collide; MaxKB@123.. reduces to a known constant.
• What the fix changes: stored hashes are per-user salted PBKDF2-SHA256 with a high iteration count, so DB exposure no longer yields plaintext at any practical cost. Login no longer puts the password (in any form) into a SQL WHERE clause; verification uses Django's constant-time check_password.
• Backward compatibility: existing deployments keep working — old MD5 hashes still authenticate, and are upgraded to PBKDF2 the first time the user logs in. There is no forced password reset.
• Known residual: legacy MD5 hashes for users who never log in remain weak until upgraded. This is the standard cost of an in-place hasher migration; it can be addressed later by an offline rewrap or a forced reset for inactive accounts, but is out of scope for this patch.

Adversarial review

Before submitting we tried to disprove this. The main counter-argument is "you need DB access to exploit it, and DB access is already game over." That's not equivalent: read-only DB exposure (backup leak, SELECT-only SQLi, log scraping, replica access) does not by itself give an attacker the user's plaintext credential — but unsalted MD5 does. Recovered plaintext enables credential stuffing on third-party services that the application boundary cannot protect, which is the specific harm CWE-916 captures. We also checked whether Django or the framework was already upgrading these hashes transparently — it isn't, because the codebase bypasses Django's auth stack and stores raw MD5 in a plain CharField.

cc @lewiswigmore

[f2c-ci-robot]

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[f2c-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment