diff --git a/bot_bottle/orchestrator/service.py b/bot_bottle/orchestrator/service.py index a43d9b14..68e6e7fd 100644 --- a/bot_bottle/orchestrator/service.py +++ b/bot_bottle/orchestrator/service.py @@ -366,15 +366,23 @@ class OrchestratorCore: value with *env_var_secret*, and restores ``_tokens[bottle_id]``. Returns True on success, False when no stored secrets exist for this bottle or decryption fails (wrong key / corrupt data).""" - from .store.secret_store import decrypt_value + from .store.secret_store import decrypt_value, encrypt_value, is_legacy_blob encrypted = self.registry.get_agent_secrets(bottle_id) if not encrypted: return False try: - self._tokens[bottle_id] = {k: decrypt_value(env_var_secret, v) - for k, v in encrypted.items()} + decrypted = { + k: decrypt_value(env_var_secret, v) for k, v in encrypted.items() + } except ValueError: return False + self._tokens[bottle_id] = decrypted + if any(is_legacy_blob(value) for value in encrypted.values()): + migrated = { + key: encrypt_value(env_var_secret, value) + for key, value in decrypted.items() + } + self.registry.store_agent_secrets(bottle_id, migrated) return True # --- consolidated gateway ---------------------------------------------- diff --git a/bot_bottle/orchestrator/store/secret_store.py b/bot_bottle/orchestrator/store/secret_store.py index 7daf0767..10d7d209 100644 --- a/bot_bottle/orchestrator/store/secret_store.py +++ b/bot_bottle/orchestrator/store/secret_store.py @@ -12,9 +12,15 @@ reattachment path reads ENV_VAR_SECRET from the running agent container via ``POST /bottles//reprovision_gateway``; the orchestrator decrypts the stored rows and re-populates ``_tokens``. -Encryption scheme: HMAC-SHA256 used as a PRF in CTR mode (stdlib-only, -no external deps). Each value is encrypted independently. The output blob is -``nonce (16 bytes) || ciphertext`` encoded as URL-safe base64 (no padding). +Encryption scheme: encrypt-then-MAC using independent HMAC-SHA256-derived +encryption and authentication subkeys (stdlib-only, no external deps). Each +value is encrypted independently. New output blobs are: + +``version || nonce (16 bytes) || ciphertext || tag (32 bytes)`` + +encoded as URL-safe base64 (no padding). The version marker lets the reader +accept legacy ``nonce || ciphertext`` rows long enough to rewrite them in the +authenticated format after a successful reprovision. keystream_block_i = HMAC-SHA256(key, nonce || i.to_bytes(4, "big")) ciphertext_i = plaintext_i XOR keystream_block_i[:len(plaintext_i)] @@ -30,6 +36,8 @@ import secrets _KEY_BYTES = 32 # 256-bit key from ENV_VAR_SECRET _NONCE_BYTES = 16 # 128-bit random nonce per encrypt call _BLOCK = 32 # HMAC-SHA256 output width == one keystream block +_TAG_BYTES = 32 +_VERSION = b"BBSE1" # Env-var name the agent container receives at startup. ENV_VAR_SECRET_NAME = "ENV_VAR_SECRET" @@ -41,7 +49,13 @@ def new_env_var_secret() -> str: def _b64dec(s: str) -> bytes: - return base64.urlsafe_b64decode(s + "=" * (-len(s) % 4)) + return base64.b64decode( + s + "=" * (-len(s) % 4), altchars=b"-_", validate=True, + ) + + +def _subkey(key: bytes, purpose: bytes) -> bytes: + return hmac.new(key, b"bot-bottle-secret-store:" + purpose, hashlib.sha256).digest() def _keystream(key: bytes, nonce: bytes, block_index: int) -> bytes: @@ -53,42 +67,90 @@ def _keystream(key: bytes, nonce: bytes, block_index: int) -> bytes: def encrypt_value(secret_b64: str, plaintext: str) -> str: """Encrypt a single string value with *secret_b64* (the ENV_VAR_SECRET). - Returns a URL-safe base64 blob ``nonce || ciphertext`` suitable for + Returns a URL-safe base64 authenticated blob suitable for the ``bottled_agent_secrets.value`` column.""" key = _b64dec(secret_b64) + encryption_key = _subkey(key, b"encryption") + authentication_key = _subkey(key, b"authentication") pt = plaintext.encode() nonce = secrets.token_bytes(_NONCE_BYTES) ct = bytearray() for i in range(0, len(pt), _BLOCK): chunk = pt[i : i + _BLOCK] - ks = _keystream(key, nonce, i)[: len(chunk)] + ks = _keystream(encryption_key, nonce, i // _BLOCK)[: len(chunk)] ct.extend(p ^ k for p, k in zip(chunk, ks)) - return base64.urlsafe_b64encode(nonce + bytes(ct)).rstrip(b"=").decode() + authenticated = _VERSION + nonce + bytes(ct) + tag = hmac.new(authentication_key, authenticated, hashlib.sha256).digest() + return base64.urlsafe_b64encode(authenticated + tag).rstrip(b"=").decode() -def decrypt_value(secret_b64: str, blob_b64: str) -> str: - """Decrypt a blob produced by :func:`encrypt_value`. - - Returns the original plaintext string. Raises ``ValueError`` for malformed - input or a key mismatch (wrong key produces garbage, not an error, unless - the plaintext is non-UTF-8 — treat all such failures as wrong key).""" - key = _b64dec(secret_b64) +def is_legacy_blob(blob_b64: str) -> bool: + """Whether *blob_b64* uses the pre-authentication storage format.""" try: - blob = _b64dec(blob_b64) - except Exception as exc: - raise ValueError(f"invalid ciphertext blob: {exc}") from exc + return not _b64dec(blob_b64).startswith(_VERSION) + except (ValueError, TypeError): + return False + + +def _decrypt_legacy(key: bytes, blob: bytes) -> str: + """Read the original ``nonce || ciphertext`` format for migration only.""" if len(blob) < _NONCE_BYTES: raise ValueError("ciphertext blob too short") nonce, ciphertext = blob[:_NONCE_BYTES], blob[_NONCE_BYTES:] pt = bytearray() + # The legacy format used the byte offset as the PRF counter. for i in range(0, len(ciphertext), _BLOCK): chunk = ciphertext[i : i + _BLOCK] ks = _keystream(key, nonce, i)[: len(chunk)] pt.extend(c ^ k for c, k in zip(chunk, ks)) try: return bytes(pt).decode() + except UnicodeDecodeError as exc: + raise ValueError(f"decryption produced non-UTF-8 output: {exc}") from exc + + +def decrypt_value(secret_b64: str, blob_b64: str) -> str: + """Decrypt a blob produced by :func:`encrypt_value`. + + Returns the original plaintext string. Raises ``ValueError`` for malformed + input, authentication failure, or a key mismatch. Legacy unauthenticated + rows remain readable so callers can migrate them immediately.""" + key = _b64dec(secret_b64) + try: + blob = _b64dec(blob_b64) + except (ValueError, TypeError) as exc: + raise ValueError(f"invalid ciphertext blob: {exc}") from exc + if not blob.startswith(_VERSION): + return _decrypt_legacy(key, blob) + minimum = len(_VERSION) + _NONCE_BYTES + _TAG_BYTES + if len(blob) < minimum: + raise ValueError("ciphertext blob too short") + authenticated, supplied_tag = blob[:-_TAG_BYTES], blob[-_TAG_BYTES:] + authentication_key = _subkey(key, b"authentication") + expected_tag = hmac.new( + authentication_key, authenticated, hashlib.sha256, + ).digest() + if not hmac.compare_digest(supplied_tag, expected_tag): + raise ValueError("ciphertext authentication failed") + nonce_start = len(_VERSION) + nonce = blob[nonce_start : nonce_start + _NONCE_BYTES] + ciphertext = blob[nonce_start + _NONCE_BYTES : -_TAG_BYTES] + encryption_key = _subkey(key, b"encryption") + pt = bytearray() + for i in range(0, len(ciphertext), _BLOCK): + chunk = ciphertext[i : i + _BLOCK] + ks = _keystream(encryption_key, nonce, i // _BLOCK)[: len(chunk)] + pt.extend(c ^ k for c, k in zip(chunk, ks)) + try: + return bytes(pt).decode() except UnicodeDecodeError as exc: raise ValueError(f"decryption produced non-UTF-8 output (wrong key?): {exc}") from exc -__all__ = ["ENV_VAR_SECRET_NAME", "new_env_var_secret", "encrypt_value", "decrypt_value"] +__all__ = [ + "ENV_VAR_SECRET_NAME", + "new_env_var_secret", + "encrypt_value", + "decrypt_value", + "is_legacy_blob", +] diff --git a/tests/unit/test_orchestrator_secret_store.py b/tests/unit/test_orchestrator_secret_store.py index 7301b126..070d3c00 100644 --- a/tests/unit/test_orchestrator_secret_store.py +++ b/tests/unit/test_orchestrator_secret_store.py @@ -2,6 +2,9 @@ from __future__ import annotations +import base64 +import hashlib +import hmac import unittest from bot_bottle.orchestrator.store.secret_store import ( @@ -68,15 +71,28 @@ class TestDecryptErrors(unittest.TestCase): def test_wrong_key_raises_value_error(self) -> None: ct = encrypt_value(self.secret, "secret-token") other_key = new_env_var_secret() - # Wrong key produces garbage bytes; decrypt_value raises ValueError - # when the result is non-UTF-8 (which is very likely for 12-char data). - # We allow it to succeed only if garbage happens to be valid UTF-8, but - # the plaintext must not match. - try: - result = decrypt_value(other_key, ct) - self.assertNotEqual("secret-token", result) - except ValueError: - pass + with self.assertRaisesRegex(ValueError, "authentication failed"): + decrypt_value(other_key, ct) + + def test_tampered_ciphertext_raises_value_error(self) -> None: + raw = bytearray(base64.urlsafe_b64decode( + encrypt_value(self.secret, "secret-token") + "==" + )) + raw[22] ^= 1 + tampered = base64.urlsafe_b64encode(raw).rstrip(b"=").decode() + with self.assertRaisesRegex(ValueError, "authentication failed"): + decrypt_value(self.secret, tampered) + + def test_reads_legacy_ciphertext_for_migration(self) -> None: + key = base64.urlsafe_b64decode(self.secret + "==") + nonce = b"0123456789abcdef" + plaintext = b"legacy-token" + stream = hmac.new( + key, nonce + (0).to_bytes(4, "big"), hashlib.sha256, + ).digest() + ciphertext = bytes(p ^ k for p, k in zip(plaintext, stream)) + legacy = base64.urlsafe_b64encode(nonce + ciphertext).rstrip(b"=").decode() + self.assertEqual("legacy-token", decrypt_value(self.secret, legacy)) def test_truncated_blob_raises_value_error(self) -> None: with self.assertRaises(ValueError):