Skip to content

fix(security): replace FNV1a and XOR placeholders with real cryptography#563

Merged
kcenon merged 3 commits into
mainfrom
fix/issue-562-replace-security-placeholders
Apr 13, 2026
Merged

fix(security): replace FNV1a and XOR placeholders with real cryptography#563
kcenon merged 3 commits into
mainfrom
fix/issue-562-replace-security-placeholders

Conversation

@kcenon

@kcenon kcenon commented Apr 13, 2026

Copy link
Copy Markdown
Owner

Summary

  • Replace FNV1a password hashing with PBKDF2-HMAC-SHA256 (OpenSSL)
  • Replace XOR encryption with AES-256-GCM (OpenSSL)
  • Implement actual key rotation (decrypt all, re-encrypt with new key)
  • Add legacy format migration path (fnv1a: / xor: prefix detection)
  • Add stderr warnings when OpenSSL is unavailable

Related Issues

Closes #562

Files Changed

File Change
database/security/secure_connection.cpp Conditional OpenSSL implementation for all 3 security functions

Test Plan

  • With OpenSSL: PBKDF2 hash/verify, AES-256-GCM encrypt/decrypt round-trip
  • Without OpenSSL: stderr warning emitted, fallback behavior preserved
  • Key rotation: all credentials re-encrypted successfully
  • Legacy fnv1a: passwords verified correctly
  • Legacy XOR (with/without xor: prefix) decrypted correctly
  • Existing credential_test, data_masking_test pass

All three security functions were explicitly marked placeholders:
- hash_password(): FNV1a (no salt, deterministic, trivially reversible)
- encrypt_data()/encrypt_field_data(): XOR with key (trivially breakable)
- rotate_encryption_keys(): no-op returning true

When OpenSSL is available (__has_include<openssl/evp.h>):
- PBKDF2-HMAC-SHA256 for password hashing (16-byte salt, 100k iterations)
- AES-256-GCM for data/field encryption (12-byte IV, 16-byte auth tag)
- Actual key rotation: decrypt all credentials, re-encrypt with new key

When OpenSSL is absent:
- Keep placeholders with stderr warning on first use
- Tag output format (fnv1a:, xor:, aes:) for migration detection
- Legacy format transparently decoded in decrypt functions

Closes #562
@github-actions

Copy link
Copy Markdown
Contributor

Benchmark Results

No comparison reports available. Baseline may not be established yet.

kcenon added 2 commits April 13, 2026 11:18
…lude

__has_include(<openssl/evp.h>) detects header presence but not library
link availability, causing linker errors in CI where OpenSSL headers
are installed but -lssl -lcrypto are not linked.

- Add find_package(OpenSSL QUIET) to root CMakeLists.txt
- Add target_compile_definitions(DATABASE_HAS_OPENSSL) and
  target_link_libraries(OpenSSL::Crypto) in database/CMakeLists.txt
- Change source from __has_include to #ifdef DATABASE_HAS_OPENSSL
OpenSSL::Crypto as a PRIVATE link dependency gets recorded in the
CMake export target file. Consumers that don't find_package(OpenSSL)
then fail at CMake generate step. Wrap with $<BUILD_INTERFACE:> to
exclude from the exported target properties.
@kcenon kcenon merged commit cf20e44 into main Apr 13, 2026
31 checks passed
@kcenon kcenon deleted the fix/issue-562-replace-security-placeholders branch April 13, 2026 03:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: replace FNV1a and XOR security placeholders with real cryptography

1 participant