Merge pull request #151 from oasisprotocol/uniyalabhishek/security/rofl-auth-token-key-sync

accounting: encrypt AuthToken key sync

Author: uniyalabhishek

src/auth/auth_token_service.py

@@ -74,10 +74,7 @@ def initialize(self) -> None:
         # key_manager should already be initialized by main.py lifespan
         enc_key = key_manager.enc_key
         self._aead = AEAD(enc_key)
-        logger.info(
-            "AuthTokenService initialized with key prefix: %s...",
-            enc_key[:4].hex(),
-        )
+        logger.info("AuthTokenService initialized")
 
     @property
     def aead(self) -> AEAD:

src/clients/rofl.py

@@ -3,16 +3,23 @@
 import base64
 import logging
 from dataclasses import dataclass
+from typing import Final
 
 import cbor2
 from eth_account import Account
+from hexbytes import HexBytes
 from oasis_rofl_client import AsyncRoflClient
 from web3.types import TxParams
 
 from src.abi.accounting import get_error_name
 
 logger = logging.getLogger(__name__)
 
+# Plaintext ROFL submissions expose calldata before Sapphire execution. Keep this
+# empty unless a reviewed transaction selector has a concrete need to be public.
+# Entries are lowercase initial 4-byte function selectors without the 0x prefix.
+_PLAINTEXT_TX_SELECTOR_ALLOWLIST: Final[frozenset[str]] = frozenset()
+
 
 @dataclass
 class RoflSubmissionResult:
@@ -67,6 +74,32 @@ def _decode_revert_reason(raw_message: str | None) -> str:
         return raw_message
 
 
+def _tx_selector_hex(tx: TxParams) -> str:
+    data = tx.get("data")
+    if data is None:
+        raise ValueError("Transaction must include 'data' field")
+
+    try:
+        data_bytes = HexBytes(data)
+    except Exception as exc:
+        raise ValueError("Transaction data must be valid hex") from exc
+
+    if len(data_bytes) < 4:
+        raise ValueError("Plaintext transaction data must include a 4-byte selector")
+    return bytes(data_bytes[:4]).hex()
+
+
+def _require_plaintext_allowlisted(tx: TxParams) -> str:
+    selector = _tx_selector_hex(tx)
+    if selector not in _PLAINTEXT_TX_SELECTOR_ALLOWLIST:
+        raise ValueError(
+            "Plaintext ROFL transaction submission is not allowed for selector "
+            f"{selector}. Add a reviewed selector allow-list entry before calling "
+            "submit_tx(..., encrypt=False)."
+        )
+    return selector
+
+
 # Dedicated key for authenticating signed view queries on Sapphire (onlyROFLQuery modifier).
 # Published on-chain at startup via setRoflSignerAddress so msg.sender checks succeed.
 ROFL_QUERY_SIGNER_KEY = "rofl_query_signer.key"
@@ -129,12 +162,12 @@ async def get_keypair(self, key_id: str = ROFL_QUERY_SIGNER_KEY):
             logger.error(f"Error generating keypair: {e}")
             raise
 
-    async def submit_tx(self, tx: TxParams, encrypt: bool = False) -> RoflSubmissionResult:
+    async def submit_tx(self, tx: TxParams, encrypt: bool = True) -> RoflSubmissionResult:
         """Submit a transaction to the ROFL daemon for signing and relay.
 
         Args:
             tx: Transaction parameters (must include 'to', 'data', 'gas', 'value')
-            encrypt: Whether to encrypt the transaction (default: False)
+            encrypt: Whether to encrypt the transaction (default: True)
 
         Returns:
             RoflSubmissionResult: Submission ID and optional ok payload (ABI-encoded return value)
@@ -147,11 +180,20 @@ async def submit_tx(self, tx: TxParams, encrypt: bool = False) -> RoflSubmission
         if "to" not in tx or "data" not in tx:
             raise ValueError("Transaction must include 'to' and 'data' fields")
 
+        plaintext_selector = None
+        if not encrypt:
+            plaintext_selector = _require_plaintext_allowlisted(tx)
+
         logger.info(
             "Submitting transaction via ROFL to %s with gas %s",
             tx["to"],
             tx.get("gas"),
         )
+        if plaintext_selector is not None:
+            logger.warning(
+                "Submitting allow-listed plaintext ROFL transaction selector %s",
+                plaintext_selector,
+            )
 
         try:
             # AsyncRoflClient.sign_submit returns decoded CBOR directly

src/main.py

@@ -32,6 +32,7 @@
 settings = load_settings()
 
 logging.getLogger("httpx").setLevel(logging.WARNING)
+# WARNING: oasis-rofl-client DEBUG logs include full tx payloads before appd encryption!
 logging.getLogger().setLevel(getattr(logging, settings.log_level.upper(), logging.INFO))
 
 
@@ -88,13 +89,10 @@ async def lifespan(_app: FastAPI):
     if not os.getenv("DISABLE_ROFL_KEYS"):
         try:
             await auth_token_key_manager.sync_key_to_contract()
-            logger.info("AuthToken encryption key synced to contract")
-        except Exception as e:
-            logger.warning(
-                f"Failed to sync AuthToken encryption key to contract: {e}. "
-                "Continuing startup - on fresh deployments the key will be set later, "
-                "on restarts the key may already be set."
-            )
+        except Exception:
+            logger.exception("Failed to sync AuthToken encryption key to contract")
+            raise
+        logger.info("AuthToken encryption key synced to contract")
 
         await bootstrap_rofl_signer_address(get_accounting_contract_service())
 

src/services/accounting_contract.py

@@ -1167,7 +1167,7 @@ async def set_auth_token_enc_key(self, enc_key: bytes) -> None:
             "gas": self.gas_limit,
             "data": Web3.to_hex(data),
         }
-        await self.rofl_client.submit_tx(tx)
+        await self.rofl_client.submit_tx(tx, encrypt=True)
 
     async def get_siwe_domain(self) -> Dict[str, Any]:
         if self._siwe_domain is None:

test/py/test_accounting_contract_service.py

@@ -494,6 +494,41 @@ async def test_set_rofl_signer_address_rejects_invalid_address() -> None:
         await service.set_rofl_signer_address("not-an-address")
 
 
+@pytest.mark.asyncio
+async def test_set_auth_token_enc_key_submits_encrypted_tx() -> None:
+    auth_address = "0x2222222222222222222222222222222222222222"
+    enc_key = bytes.fromhex("11" * 32)
+
+    rofl_client = MagicMock()
+    rofl_client.submit_tx = AsyncMock(
+        return_value=RoflSubmissionResult(submission_id="sub-1", ok_payload=None)
+    )
+
+    service = AccountingContractService.__new__(AccountingContractService)
+    service.gas_limit = 500_000
+    service.rofl_client = rofl_client
+    service._get_siwe_auth_address = AsyncMock(return_value=auth_address)
+
+    await service.set_auth_token_enc_key(enc_key)
+
+    selector = Web3.keccak(text="setAuthTokenEncKey(bytes32)")[:4]
+    expected_tx = {
+        "to": auth_address,
+        "value": 0,
+        "gas": 500_000,
+        "data": Web3.to_hex(selector + enc_key),
+    }
+    rofl_client.submit_tx.assert_awaited_once_with(expected_tx, encrypt=True)
+
+
+@pytest.mark.asyncio
+async def test_set_auth_token_enc_key_rejects_wrong_key_length() -> None:
+    service = AccountingContractService.__new__(AccountingContractService)
+
+    with pytest.raises(ValueError, match="Encryption key must be 32 bytes"):
+        await service.set_auth_token_enc_key(b"\x11" * 31)
+
+
 def _make_service_with_confidential_reader(contract: MagicMock) -> AccountingContractService:
     service = AccountingContractService.__new__(AccountingContractService)
     service._get_confidential_reader_contract = AsyncMock(return_value=contract)

test/py/test_main_lifespan.py

@@ -0,0 +1,30 @@
+"""Tests for application startup security invariants."""
+
+from types import SimpleNamespace
+from unittest.mock import AsyncMock
+
+import pytest
+
+import src.main as main
+
+
+@pytest.mark.asyncio
+async def test_lifespan_aborts_when_auth_token_key_sync_fails(monkeypatch) -> None:
+    monkeypatch.delenv("DISABLE_ROFL_KEYS", raising=False)
+
+    jwt_key_manager = SimpleNamespace(initialize=AsyncMock())
+    auth_token_key_manager = SimpleNamespace(
+        initialize=AsyncMock(),
+        sync_key_to_contract=AsyncMock(side_effect=RuntimeError("sync failed")),
+    )
+    bootstrap_rofl_signer_address = AsyncMock()
+
+    monkeypatch.setattr(main, "get_jwt_key_manager", lambda: jwt_key_manager)
+    monkeypatch.setattr(main, "get_auth_token_key_manager", lambda: auth_token_key_manager)
+    monkeypatch.setattr(main, "bootstrap_rofl_signer_address", bootstrap_rofl_signer_address)
+
+    with pytest.raises(RuntimeError, match="sync failed"):
+        async with main.lifespan(None):
+            pass
+
+    bootstrap_rofl_signer_address.assert_not_awaited()

test/test_rofl_client.py

@@ -101,12 +101,34 @@ async def test_submit_tx_success(self, mock_async_client_class):
             "gas": 100000,
             "value": 0,
         }
-        result = await client.submit_tx(tx, encrypt=False)
+        result = await client.submit_tx(tx)
 
         # Verify - returns RoflSubmissionResult with hex-encoded CBOR as submission_id
         expected = cbor2.dumps({"ok": b""}).hex()
         self.assertEqual(result.submission_id, expected)
         self.assertEqual(result.ok_payload, b"")
+        mock_client.sign_submit.assert_awaited_once_with(tx, True)
+
+    @patch("src.clients.rofl.AsyncRoflClient")
+    async def test_submit_tx_encrypts_by_default(self, mock_async_client_class):
+        """Test that submit_tx encrypts transactions unless explicitly disabled."""
+        from src.clients.rofl import RoflAppdClient
+
+        mock_client = MagicMock()
+        mock_client.sign_submit = AsyncMock(return_value={"ok": b""})
+        mock_async_client_class.return_value = mock_client
+
+        client = RoflAppdClient()
+        tx: TxParams = {
+            "to": "0x0987654321098765432109876543210987654321",
+            "data": "0xabcdef",
+            "gas": 100000,
+            "value": 0,
+        }
+
+        await client.submit_tx(tx)
+
+        mock_client.sign_submit.assert_awaited_once_with(tx, True)
 
     @patch("src.clients.rofl.AsyncRoflClient")
     async def test_submit_tx_reverted_raises_error(self, mock_async_client_class):
@@ -135,13 +157,58 @@ async def test_submit_tx_reverted_raises_error(self, mock_async_client_class):
 
         # Should raise TransactionRevertedError
         with self.assertRaises(TransactionRevertedError) as ctx:
-            await client.submit_tx(tx, encrypt=False)
+            await client.submit_tx(tx)
 
         error = ctx.exception
         self.assertEqual(error.code, 8)
         self.assertEqual(error.module, "evm")
         self.assertIn("InvalidSignature", str(error))
 
+    @patch("src.clients.rofl.AsyncRoflClient")
+    async def test_submit_tx_rejects_unallowlisted_plaintext(self, mock_async_client_class):
+        """Test that plaintext submission is blocked unless allow-listed."""
+        from src.clients.rofl import RoflAppdClient
+
+        mock_client = MagicMock()
+        mock_client.sign_submit = AsyncMock(return_value={"ok": b""})
+        mock_async_client_class.return_value = mock_client
+
+        client = RoflAppdClient()
+        tx: TxParams = {
+            "to": "0x0987654321098765432109876543210987654321",
+            "data": "0xabcdef01",
+            "gas": 100000,
+            "value": 0,
+        }
+
+        with self.assertRaises(ValueError) as ctx:
+            await client.submit_tx(tx, encrypt=False)
+
+        self.assertIn("Plaintext ROFL transaction submission is not allowed", str(ctx.exception))
+        mock_client.sign_submit.assert_not_awaited()
+
+    @patch("src.clients.rofl._PLAINTEXT_TX_SELECTOR_ALLOWLIST", frozenset({"abcdef01"}))
+    @patch("src.clients.rofl.AsyncRoflClient")
+    async def test_submit_tx_allows_allowlisted_plaintext_selector(self, mock_async_client_class):
+        """Test that plaintext submission requires a reviewed selector allow-list entry."""
+        from src.clients.rofl import RoflAppdClient
+
+        mock_client = MagicMock()
+        mock_client.sign_submit = AsyncMock(return_value={"ok": b""})
+        mock_async_client_class.return_value = mock_client
+
+        client = RoflAppdClient()
+        tx: TxParams = {
+            "to": "0x0987654321098765432109876543210987654321",
+            "data": "0xabcdef0100000000",
+            "gas": 100000,
+            "value": 0,
+        }
+
+        await client.submit_tx(tx, encrypt=False)
+
+        mock_client.sign_submit.assert_awaited_once_with(tx, False)
+
     @patch("src.clients.rofl.AsyncRoflClient")
     async def test_submit_tx_requires_to_and_data(self, mock_async_client_class):
         """Test that submit_tx raises ValueError without 'to' or 'data'."""