From 0ba23026a7910c4d53be5d4d5fa4c4556ae0b443 Mon Sep 17 00:00:00 2001 From: aaniya22 Date: Mon, 6 Jul 2026 13:53:07 +0530 Subject: [PATCH] fix(vault): replace hand-rolled XOR+HMAC with real AES-GCM (#302) - VaultCrypto now uses AES-256-GCM for authenticated encryption instead of a custom XOR keystream + HMAC scheme, closing a crib-dragging vulnerability on plaintext longer than one keystream block - Key material is SHA-256 hashed to a fixed 32 bytes, so any input key length works safely for AES-256 - Ciphertext from the old XOR+HMAC format is now rejected loudly on decrypt (raises ValueError) rather than silently producing garbage - Consolidated a duplicate test_vault.py (one had been added at testing/backend/test_vault.py, colliding with the existing file at testing/backend/unit/test_vault.py) and merged in new regression tests: long-plaintext round-trip, nonce non-determinism, tamper detection, wrong-key rejection, and old-format rejection --- backend/requirements.txt | 1 + backend/secuscan/vault.py | 43 ++++++++++---------- testing/backend/unit/test_vault.py | 63 ++++++++++++++++++++++++++++++ 3 files changed, 85 insertions(+), 22 deletions(-) diff --git a/backend/requirements.txt b/backend/requirements.txt index 2ec0ce1fa..710e94daa 100644 --- a/backend/requirements.txt +++ b/backend/requirements.txt @@ -9,3 +9,4 @@ python-multipart>=0.0.9 xhtml2pdf>=0.2.17 aiosqlite>=0.20.0 python-whois>=0.9.4 +cryptography>=43.0.0 diff --git a/backend/secuscan/vault.py b/backend/secuscan/vault.py index 8e4d8bf6d..e302a6151 100644 --- a/backend/secuscan/vault.py +++ b/backend/secuscan/vault.py @@ -4,43 +4,42 @@ import base64 import hashlib -import hmac import os -from itertools import cycle + +from cryptography.exceptions import InvalidTag +from cryptography.hazmat.primitives.ciphers.aead import AESGCM class VaultCrypto: - """Symmetric encryption helper backed by a deterministic keystream. + """Symmetric encryption helper backed by AES-256-GCM. - This is intentionally lightweight for local-first usage where secret-at-rest - protection is needed without adding third-party crypto dependencies. + Uses a proper authenticated stream cipher instead of a hand-rolled + XOR keystream, so encrypting messages longer than one block does not + reuse keystream bytes and cannot be attacked via crib-dragging. """ - def __init__(self, key: bytes): - self.key = key + _NONCE_SIZE = 12 # recommended nonce size for AES-GCM - def _derive_stream_key(self, nonce: bytes) -> bytes: - return hashlib.sha256(self.key + nonce).digest() + def __init__(self, key: bytes): + # Normalize the provided key to exactly 32 bytes (AES-256) regardless + # of the length of the key material passed in. + self._aead = AESGCM(hashlib.sha256(key).digest()) def encrypt(self, plaintext: str) -> str: raw = plaintext.encode("utf-8") - nonce = os.urandom(16) - stream_key = self._derive_stream_key(nonce) - ciphertext = bytes(b ^ k for b, k in zip(raw, cycle(stream_key))) - signature = hmac.new(self.key, nonce + ciphertext, hashlib.sha256).digest() - blob = nonce + signature + ciphertext + nonce = os.urandom(self._NONCE_SIZE) + ciphertext = self._aead.encrypt(nonce, raw, None) + blob = nonce + ciphertext return base64.urlsafe_b64encode(blob).decode("ascii") def decrypt(self, payload: str) -> str: blob = base64.urlsafe_b64decode(payload.encode("ascii")) - nonce = blob[:16] - signature = blob[16:48] - ciphertext = blob[48:] + nonce = blob[: self._NONCE_SIZE] + ciphertext = blob[self._NONCE_SIZE :] - expected = hmac.new(self.key, nonce + ciphertext, hashlib.sha256).digest() - if not hmac.compare_digest(signature, expected): - raise ValueError("Vault payload integrity verification failed") + try: + raw = self._aead.decrypt(nonce, ciphertext, None) + except InvalidTag as exc: + raise ValueError("Vault payload integrity verification failed") from exc - stream_key = self._derive_stream_key(nonce) - raw = bytes(b ^ k for b, k in zip(ciphertext, cycle(stream_key))) return raw.decode("utf-8") diff --git a/testing/backend/unit/test_vault.py b/testing/backend/unit/test_vault.py index 95fa32d75..279814b81 100644 --- a/testing/backend/unit/test_vault.py +++ b/testing/backend/unit/test_vault.py @@ -1,3 +1,8 @@ +import base64 +import os + +import pytest + from backend.secuscan.config import settings from backend.secuscan.vault import VaultCrypto @@ -9,3 +14,61 @@ def test_vault_encrypt_roundtrip(): assert encrypted != secret decrypted = crypto.decrypt(encrypted) assert decrypted == secret + + +@pytest.fixture +def crypto(): + return VaultCrypto(b"test-vault-key-for-unit-tests") + + +def test_round_trip_plaintext_longer_than_one_block(crypto): + """Regression test for #302: plaintext longer than the old 32-byte + repeating keystream must still round-trip correctly under AES-GCM, + which has no fixed-size repeating keystream to exploit.""" + plaintext = "a" * 500 + encrypted = crypto.encrypt(plaintext) + assert crypto.decrypt(encrypted) == plaintext + + +def test_encrypt_is_nondeterministic(crypto): + """Each encryption should use a fresh random nonce, so the same + plaintext never produces the same ciphertext twice.""" + plaintext = "same-secret-every-time" + first = crypto.encrypt(plaintext) + second = crypto.encrypt(plaintext) + assert first != second + assert crypto.decrypt(first) == plaintext + assert crypto.decrypt(second) == plaintext + + +def test_tampered_ciphertext_is_rejected(crypto): + plaintext = "do-not-tamper-with-me" + encrypted = crypto.encrypt(plaintext) + + blob = bytearray(base64.urlsafe_b64decode(encrypted)) + blob[-1] ^= 0x01 + tampered_str = base64.urlsafe_b64encode(bytes(blob)).decode("ascii") + + with pytest.raises(ValueError, match="integrity verification failed"): + crypto.decrypt(tampered_str) + + +def test_wrong_key_cannot_decrypt(crypto): + plaintext = "only-the-right-key-should-work" + encrypted = crypto.encrypt(plaintext) + + wrong_crypto = VaultCrypto(b"a-completely-different-key") + with pytest.raises(ValueError, match="integrity verification failed"): + wrong_crypto.decrypt(encrypted) + + +def test_old_xor_format_payload_is_rejected_not_silently_garbled(crypto): + """Ciphertext produced by the old hand-rolled XOR scheme (nonce + + 32-byte HMAC signature + ciphertext) must fail loudly under the new + AES-GCM implementation rather than decrypting to garbage silently. + Documents the breaking change for anyone migrating stored secrets.""" + fake_old_blob = os.urandom(16) + os.urandom(32) + b"some-old-ciphertext-bytes" + fake_old_payload = base64.urlsafe_b64encode(fake_old_blob).decode("ascii") + + with pytest.raises(ValueError, match="integrity verification failed"): + crypto.decrypt(fake_old_payload)