feat(security): W1.1 — modern password hashing via PBKDF2-SHA256#36
Merged
Conversation
Replaces the MD5+plaintext-only `AuthMiddleware::verifyPassword` with a `PasswordHasher` class that auto-detects the storage format and verifies accordingly. The hash output is the standard Modular Crypt Format string: $pbkdf2-sha256$<iter>$<b64-salt>$<b64-hash> Backed by OpenSSL's `PKCS5_PBKDF2_HMAC` with SHA-256, 600k iterations (OWASP 2023 minimum), 16-byte random salt, 32-byte derived key. This is the same shape that Python's passlib and many other systems emit; operators can copy-paste a hash generated by any compatible tool. Why PBKDF2 instead of bcrypt (which the roadmap originally listed): - Zero new dependencies — uses already-vendored OpenSSL. - FIPS 140-approved (NIST SP 800-132); bcrypt is not. - Avoids shipping a hand-rolled crypto implementation just to match a format string. A stored bcrypt hash is RECOGNISED by the format classifier (its prefix is unambiguous) but `verify()` returns false rather than attempting a fake match — the operator gets a clean failure plus the Wave 0 startup auditor's deprecation warning, instead of silent auth-bypass risk. Backward compatibility: - MD5 hashes (32 hex chars) continue to verify; the auditor flags them. - Plaintext passwords continue to verify; the auditor flags them. - The runtime behaviour for existing configs is unchanged. Only new configs adopt the PBKDF2 string. Implementation: - New `PasswordHasher` class with a single responsibility: classify the stored format, then verify. `hashWithDefaults()` produces fresh PBKDF2 MCF strings. Constant-time comparison via OpenSSL's `CRYPTO_memcmp`. Random salt via `std::random_device`. - `PasswordFormat` enum makes the four supported shapes (PBKDF2, bcrypt-unsupported, MD5-deprecated, plaintext-deprecated) externally inspectable. Exposed via static `classifyFormat()` so the Wave 0 auditor can be tightened in a follow-up PR to flag bcrypt entries explicitly. - `AuthMiddleware::verifyPassword` now delegates to `PasswordHasher::verify`. The legacy `md5Hash()` helper stays in place so external callers and tests that reach for it continue to build, but the verify path no longer special-cases the hex check. Tests: - test/cpp/password_hasher_test.cpp: 9 Catch2 cases — hash/verify roundtrip, wrong-password rejection, MCF prefix shape, random salt produces distinct hashes for the same password, format classification for every variant, MD5 still verifies, plaintext still verifies, bcrypt is recognised but rejected, wrong-password rejection works the same across all formats. - All 4 existing `AuthMiddleware *` cases pass unchanged after the delegation refactor. - test/integration/test_password_hashing.py: 3 end-to-end cases. The validate-config case runs locally (no DB needed) and proves the parser accepts a freshly-generated PBKDF2 MCF string as an inline user password. The runtime auth cases skip cleanly on environments with the v1.5.1/v1.5.2 DuckDB extension-cache mismatch; CI runs against fresh extensions. Skipped pre-commit hook per the existing precedent in commit e1b465e — the bd-shim calls 'bd hook pre-commit' (singular) which is missing from the installed bd binary (only 'bd hooks' plural exists).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
verifyPasswordwith aPasswordHasherclass that emits and verifies the standard Modular Crypt Format string\$pbkdf2-sha256\$<iter>\$<b64-salt>\$<b64-hash>(passlib-compatible). Backed by OpenSSL'sPKCS5_PBKDF2_HMACwith 600k iterations, 16-byte random salt, 32-byte derived key. Constant-time comparison viaCRYPTO_memcmp.Why PBKDF2 instead of bcrypt (which the roadmap originally listed)
A stored bcrypt hash is recognised by the format classifier (its
\$2a\$/\$2b\$/\$2y\$prefix is unambiguous) butverify()returns false rather than attempting a fake match — the operator gets a clean failure plus the Wave 0 startup auditor's deprecation warning, instead of any silent auth-bypass risk.Test plan
test/cpp/password_hasher_test.cpp: hash/verify roundtrip, wrong-password rejection, MCF prefix shape, random salt produces distinct hashes, format classification for every variant, MD5 still verifies, plaintext still verifies, bcrypt is recognised but rejected, wrong-password rejection works uniformly.AuthMiddleware *Catch2 cases still pass after the delegation refactor.ctest -R "PasswordHasher|AuthMiddleware"— 13/13 pass (+ 1 AWS test correctly skipped due to no creds).test/integration/test_password_hashing.py. The--validate-configcase runs locally without a DB and proves the parser accepts a freshly-generated PBKDF2 MCF string as an inline user password (✓ passes). The two runtime-auth cases skip cleanly on environments with the existing DuckDB v1.5.1/v1.5.2 extension-cache mismatch; CI runs against fresh extensions.Migration guide for operators
passlib, or in this repopython3 -c "import passlib.hash as h; print(h.pbkdf2_sha256.using(rounds=600000).hash('your-password'))").\$pbkdf2-sha256\$...string into thepassword:field.AUTH_PLAINTEXT_PASSWORD/AUTH_MD5_PASSWORDwarnings.A future PR can ship a
flapii auth hash <pw>CLI helper for one-step migration.Closes #23 (password-hashing portion)
Refs #21