-
Notifications
You must be signed in to change notification settings - Fork 1
feat: challenge-response providers with KEK multi-device support #75
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Implement a Protocol-based abstraction for hardware-backed challenge-response authentication, enabling pluggable backends without requiring library imports. Changes: - Add ChallengeResponseProvider Protocol for structural typing - Rename HardwareYubiKey to YubiKeyHmacSha1 (protocol-based naming) - Add Fido2HmacSecret ABC with YubiKeyFido2 implementation - Add FIDO2 hmac-secret support for broader device compatibility - Add testing module with MockYubiKey, MockFido2, MockProvider - Update Database API to use challenge_response_provider parameter - Add ChallengeResponseError and Fido2Error exception hierarchies - Support both 20-byte (YubiKey HMAC-SHA1) and 32-byte (FIDO2) responses - Add multi-key enrollment support in header CustomData This provides the foundation for supporting various hardware security keys through a clean, extensible interface.
Implements Key Encryption Key (KEK) mode to enable multiple hardware devices (YubiKeys, FIDO2 keys) to unlock the same database. Each enrolled device wraps the same KEK with its unique CR output. Two operating modes: - Legacy: CR mixed into composite key (KeePassXC-compatible, YubiKey only) - KEK: CR used to wrap/unwrap KEK (multi-device, any provider type) New enrollment API: - db.enroll_device(provider, label) - enroll a device - db.revoke_device(label) - remove a device - db.list_enrolled_devices() - list enrolled devices - db.kek_mode - whether database uses KEK mode Key design decisions: - KEK mode is default for all enrollments (even single-device) - Legacy mode only for explicit KeePassXC compatibility - FIDO2 always uses KEK mode (32-byte responses rejected in legacy) - Cannot mix legacy and KEK modes on same database
Critical fixes: - Add _kek to zeroize_credentials() in database.py - Fix docstring: nonce is 16 bytes (PyCryptodome default), not 12 - Fix test to check first 16 bytes for nonce uniqueness High priority fixes: - Remove unused import parse_device_key_name - Add wrapped_kek size validation in deserialize_device_entry - Update EnrolledDevice docstring (60 -> 64 bytes) - Add KeePassXC incompatibility warning to enroll_device docstring FIDO2 updates: - Rewrite fido2.py for python-fido2 v2.0 API compatibility - Use DefaultClientDataCollector instead of URL string - Use result.response.attestation_object for credential_id - Use client_extension_results.hmac_get_secret.output1 Lint/type fixes: - Fix import sorting (ruff I001) - Remove unused imports (ruff F401) - Add strict=True to zip() call (ruff B905) - Fix line length issues (ruff E501) - Replace pytest.raises(Exception) with AuthenticationError - Remove unused type: ignore comments
Add methods for KEK mode management: - disable_kek_mode(): Remove KEK protection, return to password-only mode for migration to KeePassXC or other KeePass applications - rotate_kek(): Regenerate KEK and re-wrap for specified devices, use after revoking a device to invalidate old backups Security improvements from audit: - Add runtime warning when enrolling first device (KeePassXC incompatibility) - Remove device count from error messages (information leakage) - Add version forward-compatibility check for unknown CR versions - Fix bug where disable_kek_mode didn't clear challenge_response_provider Include comprehensive security audit report from 6 specialized agents covering cryptographic security, memory safety, API design, attack surface, ecosystem compatibility, and test coverage.
Replace plain SHA-256 hash with HKDF-SHA256 (RFC 5869) when deriving AES keys from challenge-response outputs. This provides domain separation to ensure keys derived for KEK wrapping cannot be confused with keys derived for other purposes, even if the same CR response is used elsewhere. - Add _hkdf_sha256() using standard library hmac module - Add HKDF_INFO_KEK_WRAP constant for domain separation - Update wrap_kek() and unwrap_kek() to use HKDF - Add comprehensive tests for HKDF function
Reorder enroll_device() to test the provider before modifying any database state. Previously, if the provider failed during first enrollment (e.g., device not connected), the database could be left with a salt generated but no KEK set. This only affects first enrollment when converting to KEK mode - subsequent device additions already have the salt set, so provider failure leaves no partial state. The bug was likely missed because most testing uses already-enrolled databases. Now enrollment either fully succeeds or leaves no state changes: 1. Generate temporary salt (not stored yet) 2. Test provider with challenge_response() 3. Only on success, store salt and proceed with KEK generation Add tests verifying atomic behavior for both first enrollment and adding subsequent devices.
Reject challenge-response outputs shorter than 16 bytes (128 bits) to prevent weak key derivation from providers returning very short responses. - Add MIN_CR_RESPONSE_LENGTH constant (16 bytes) - Validate in wrap_kek() and unwrap_kek() - Add tests for length validation YubiKey HMAC-SHA1 (20 bytes) and FIDO2 hmac-secret (32 bytes) both exceed this minimum.
Remove xfail marker from TestMixedProviderTypes and update tests to use KEK mode (enroll_device) instead of legacy mode (challenge_response_provider). Legacy mode only supports YubiKey HMAC-SHA1 (20 bytes), while FIDO2 produces 32-byte responses and must use KEK mode. Tests verify that using the wrong provider type fails to open a database.
Add tests for error handling edge cases identified in security audit: test_kek.py: - Fix docstring typo (60 -> 64 bytes) - Truncated wrapped value at nonce/tag/ciphertext boundaries - Very long CR response handling - Empty CR response rejection test_multi_key.py (new TestKekModeErrorPaths class): - Wrong password + right device - Right password + wrong device - Corrupted file handling - Error messages don't leak sensitive info
New docs/hardware-keys.md covering: - Compatibility matrix (kdbxtool vs KeePassXC/KeePassDX/KeePass) - Legacy vs KEK operating modes - Mode selection guide - Backup strategies (multiple devices, add later, password-only, migration) - Device management (list, revoke, rotate KEK) - Security considerations Update docs/index.md to include new page and mention FIDO2/multi-device.
Hardware Key Testing InstructionsThis PR adds KEK (Key Encryption Key) wrapping for multi-device challenge-response support. To verify the implementation, hardware testing with a physical YubiKey is required. Prerequisites
Running Hardware Tests# Run all hardware tests (22 tests total)
pytest -m hardware tests/test_hardware_keys.py -v
# With specific YubiKey serial
YUBIKEY_SERIAL=12345678 pytest -m hardware -vTest Coverage
KEK Mode Tests (
|
- Rename test_yubikey_hardware.py to test_hardware_keys.py to reflect broader scope (KEK mode, FIDO2, multi-device support) - Add TestKekModeHardware class with 9 tests covering: - Device enrollment and database roundtrip - Wrong password/missing device rejection - Device listing, revocation, KEK rotation - disable_kek_mode migration to password-only - Add TestKeePassXCCompatibility class for format verification - Create tests/HARDWARE_TESTING.md with: - YubiKey setup instructions - Test execution guide - Troubleshooting section - Multi-device and KeePassXC compatibility testing Total hardware tests: 22 (6 provider, 5 legacy, 9 KEK, 2 compatibility)
Replace "legacy" terminology with clearer naming: - VERSION_LEGACY -> VERSION_COMPAT - "Legacy mode" -> "KeePassXC-compatible mode" in docs/comments - Update error messages to reference KeePassXC-compatible mode - Update test names and docstrings The term "legacy" implied deprecated/inferior when this mode is actually the correct choice for users who need KeePassXC/KeePassDX interoperability. The new naming makes the purpose clearer.
H1/H2: Make rotate_kek() atomic with proper zeroization - Build new device entries in temporary list before committing - Wrap in try/finally to zeroize new_kek on failure H3: Fix _hkdf_sha256 to zeroize intermediate values - Use bytearray for PRK/OKM instead of immutable bytes - Zeroize in finally block after deriving output H4/H5: Improve SecureBytes usage in kdbx4.py - _derive_keys now returns tuple[SecureBytes, SecureBytes] - Callers wrap usage in try/finally to zeroize after use - Prevents cipher_key and hmac_key from lingering in memory H6/H7: Add tests for edge cases in device serialization - Tests for missing JSON fields (type, label, id) - Tests for Unicode labels, special characters, null bytes - Tests for very long labels and empty metadata
M1: Replace custom HKDF implementation with PyCryptodome's HKDF
- Uses Cryptodome.Protocol.KDF.HKDF instead of manual expand/extract
- Properly handles multi-block expansion per RFC 5869
M2: _unwrap_kek_from_devices now returns SecureBytes
- Ensures KEK stays wrapped for automatic zeroization
- Callers access .data only when needed
M3: derive_composite_key uses bytearray for intermediates
- All intermediate hashes stored in mutable bytearrays
- Explicit zeroization in finally block after use
M4: Add device label validation in enroll_device()
- Reject empty or whitespace-only labels
- Enforce maximum length of 256 characters
- Add corresponding tests
| pwd_hash = SecureBytes(hashlib.sha256(password.encode("utf-8")).digest()) | ||
| secure_parts.append(pwd_hash) | ||
| parts.append(pwd_hash.data) | ||
| pwd_hash = bytearray(hashlib.sha256(password.encode("utf-8")).digest()) |
Check failure
Code scanning / CodeQL
Use of a broken or weak cryptographic hashing algorithm on sensitive data High
Sensitive data (password)
Sensitive data (password)
Sensitive data (password)
Sensitive data (password)
Sensitive data (password)
Sensitive data (password)
Sensitive data (password)
Sensitive data (password)
Sensitive data (password)
Sensitive data (password)
Sensitive data (password)
Sensitive data (password)
Sensitive data (password)
Sensitive data (password)
Sensitive data (password)
Sensitive data (password)
Sensitive data (password)
Sensitive data (password)
Sensitive data (password)
Sensitive data (password)
Sensitive data (password)
Sensitive data (password)
Sensitive data (password)
Sensitive data (password)
[Sensitive data (password
Copilot Autofix
AI 7 days ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
Summary
Implements hardware-backed challenge-response authentication with KEK (Key Encryption Key) wrapping to enable multiple devices to unlock the same database. Based on PR #63 discussion.
Two operating modes:
Key changes:
ChallengeResponseProviderProtocol for structural typingYubiKeyHmacSha1for KeePassXC-compatible HMAC-SHA1Fido2HmacSecretABC withYubiKeyFido2implementationsecurity/kek.py) for multi-device supportenroll_device(),revoke_device(),list_enrolled_devices()MockYubiKey,MockFido2,MockProviderDesign Decisions
ChallengeResponseProvider- allows third-party implementationssecurity/to avoid implying they're secureKEK Architecture
Each enrolled device wraps the same KEK with its unique CR output (AES-256-GCM).
New Public API
Test plan