From 6fdbe758dd257ef7d9d134899a9a88cf4bf6008b Mon Sep 17 00:00:00 2001 From: Susan Poudel Date: Sun, 16 Aug 2026 20:39:13 -0400 Subject: [PATCH] fix(tee): check the TSM provider before reading outblob Reading outblob is what makes the platform generate and sign a report, so checking the provider afterwards meant a mismatched guest signed a report over the caller's binding which was then discarded. Nothing was returned and the entry was removed either way, so this was not a disclosure, but it asked the hardware to sign something no one could use. The provider check now gates the read, and a test asserts outblob is never read on a mismatch so the ordering cannot quietly regress. The fake configfs tree now generates the report on outblob read rather than on inblob write, which is the kernel's actual sequence and is what makes that assertion mean anything. Follow-up requested on review of #86. Signed-off-by: Susan Poudel --- CHANGELOG.md | 2 + src/ca2a_runtime/tee/tsm.py | 21 +++++++++- tests/unit/test_snp_tdx_attest.py | 65 +++++++++++++++++++++++++------ 3 files changed, 75 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b467678..9e11032 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **`collect_report` confirms the configfs-TSM provider before reading `outblob` (#86 follow-up).** Reading `outblob` is what makes the platform generate and sign a report, so checking the provider afterwards meant a mismatched guest signed a report over the caller's binding and the result was then discarded. Nothing was returned and the entry was removed either way, so this was not a disclosure, but it asked the hardware to sign something no one could use. The provider check now gates the read. A test asserts `outblob` is never read on a mismatch, so the ordering cannot quietly regress. + - **The holder proof now commits to `parent_record_hash` (#106).** It committed every other request field that reaches the emitted provenance record, and missed this one, so a party on the path could alter where the hop linked in the DAG while the proof still verified. The result was a record attached to the wrong parent: a misattributed hop rather than forged authority or widened scope, which is why it was rated low, but it was inconsistent on its own terms. The proof already commits to `record_id`, so committing a record's own identifier while leaving its parent link open was half a commitment. Committed either way, so a root hop cannot have a parent bolted onto it. The rule is now stated in P-4a and guarded by a test: every field of the request that reaches the record is committed. - **Removed `ProofReplayCache`, keeping the holder-proof path stateless (#104).** It made a proof single-use by remembering it, but bought that with per-node state in a design that is deliberately stateless, and its expiry pass walked every entry on each call, so it degraded quadratically as it filled. Holder binding is now at-most-once-per-window, bounded by the challenge TTL, which is the same guarantee `ca2a_runtime.challenge` documents for itself. A deployment that needs exactly-once supplies state at the challenge rather than at the proof, so the codebase carries one such decision instead of two. `PeerNode` no longer takes `seen_proofs`, and `verify_holder_proof` no longer takes `seen`. diff --git a/src/ca2a_runtime/tee/tsm.py b/src/ca2a_runtime/tee/tsm.py index cb9a920..a51ba5b 100644 --- a/src/ca2a_runtime/tee/tsm.py +++ b/src/ca2a_runtime/tee/tsm.py @@ -74,6 +74,11 @@ def collect_report(report_data: bytes, *, expect_provider: str) -> tuple[bytes, two processes collecting at once would share one entry, and the second write to ``inblob`` would change the report the first is about to read, so a peer could ship a report committing someone else's key. + + The provider is confirmed before ``outblob`` is read, because the read is + what makes the platform generate and sign the report. Checking afterwards + still fails closed, but only after asking the hardware to sign something over + the caller's binding that is then discarded. """ if len(report_data) > REPORT_DATA_LEN: raise AttestationFailed( @@ -97,14 +102,17 @@ def collect_report(report_data: bytes, *, expect_provider: str) -> tuple[bytes, try: try: (entry / "inblob").write_bytes(report_data) - outblob = (entry / "outblob").read_bytes() provider = (entry / "provider").read_text().strip() except OSError as exc: raise AttestationFailed( - "the configfs-TSM provider did not return a report", + "the configfs-TSM entry did not name its provider", detail=f"{type(exc).__name__}: {exc}", ) from exc + # Checked before outblob is read, because reading outblob is what makes + # the platform generate and sign a report over the caller's binding. On + # the wrong provider that report is discarded, so asking for it at all is + # work the hardware should never have been asked to do. if provider != expect_provider: raise AttestationFailed( "the configfs-TSM provider is not the expected platform", @@ -113,6 +121,15 @@ def collect_report(report_data: bytes, *, expect_provider: str) -> tuple[bytes, f"{expect_provider!r}; the wrong provider was selected for this host" ), ) + + try: + outblob = (entry / "outblob").read_bytes() + except OSError as exc: + raise AttestationFailed( + "the configfs-TSM provider did not return a report", + detail=f"{type(exc).__name__}: {exc}", + ) from exc + if not outblob: raise AttestationFailed( "the configfs-TSM provider returned an empty report", diff --git a/tests/unit/test_snp_tdx_attest.py b/tests/unit/test_snp_tdx_attest.py index 0c921be..b54913b 100644 --- a/tests/unit/test_snp_tdx_attest.py +++ b/tests/unit/test_snp_tdx_attest.py @@ -17,6 +17,7 @@ from __future__ import annotations from pathlib import Path +from types import SimpleNamespace import pytest from cryptography.hazmat.primitives.asymmetric import ec @@ -86,11 +87,15 @@ def install_fake_tsm( provider: str, make_outblob, auxblob: bytes | None = None, -) -> list[Path]: +) -> SimpleNamespace: """Simulate the kernel's configfs-TSM report interface under ``tmp_path``. - Returns the list of entry directories created, so a test can assert on how - the interface was driven rather than only on what came back. + Faithful on the one point these tests turn on: the report is produced when + ``outblob`` is *read*, not when the entry is created. Writing ``inblob`` only + supplies the report data and materialises ``provider``. + + Returns the entries created and the attributes read, so a test can assert on + how the interface was driven rather than only on what came back. """ root = tmp_path / "tsm-report" root.mkdir() @@ -98,8 +103,11 @@ def install_fake_tsm( monkeypatch.setattr("ca2a_runtime.tee.tsm.sys.platform", "linux") entries: list[Path] = [] + reads: list[str] = [] + pending: dict[str, bytes] = {} real_mkdir = Path.mkdir real_write_bytes = Path.write_bytes + real_read_bytes = Path.read_bytes def mkdir(self: Path, *args, **kwargs): # noqa: ANN002, ANN003, ANN202 result = real_mkdir(self, *args, **kwargs) @@ -111,15 +119,25 @@ def write_bytes(self: Path, data: bytes) -> int: written = real_write_bytes(self, data) if self.name == "inblob": entry = self.parent - real_write_bytes(entry / "outblob", make_outblob(data)) + pending[str(entry)] = data (entry / "provider").write_text(provider + "\n") if auxblob is not None: real_write_bytes(entry / "auxblob", auxblob) return written + def read_bytes(self: Path) -> bytes: + if self.name == "outblob": + reads.append("outblob") + data = pending.get(str(self.parent)) + if data is None: + raise FileNotFoundError(self) + return make_outblob(data) + return real_read_bytes(self) + monkeypatch.setattr(Path, "mkdir", mkdir) monkeypatch.setattr(Path, "write_bytes", write_bytes) - return entries + monkeypatch.setattr(Path, "read_bytes", read_bytes) + return SimpleNamespace(entries=entries, reads=reads) def test_collect_report_returns_the_report_and_its_certificates( @@ -153,8 +171,14 @@ def test_collect_report_reports_no_certificates_when_none_are_supplied( def test_collect_report_refuses_the_wrong_platform( monkeypatch: pytest.MonkeyPatch, tmp_path: Path ) -> None: - """A TDX guest answering an SNP collector is a misconfiguration, not evidence.""" - install_fake_tsm( + """A TDX guest answering an SNP collector is a misconfiguration, not evidence. + + The provider is checked before ``outblob`` is read, so the platform is never + asked to sign a report over the caller's binding that would then be thrown + away. Nothing escapes either way, but the signature is work the hardware + should not have been asked for. + """ + fake = install_fake_tsm( monkeypatch, tmp_path, provider=tsm.PROVIDER_TDX_GUEST, @@ -162,6 +186,7 @@ def test_collect_report_refuses_the_wrong_platform( ) with pytest.raises(AttestationFailed, match="not the expected platform"): tsm.collect_report(b"\x00" * 64, expect_provider=tsm.PROVIDER_SEV_GUEST) + assert fake.reads == [] def test_collect_report_refuses_an_empty_report( @@ -199,7 +224,7 @@ def test_each_collection_uses_its_own_entry( write moves the report the first is about to read, so a peer could ship a report committing someone else's key. """ - entries = install_fake_tsm( + fake = install_fake_tsm( monkeypatch, tmp_path, provider=tsm.PROVIDER_SEV_GUEST, @@ -207,7 +232,7 @@ def test_each_collection_uses_its_own_entry( ) tsm.collect_report(b"\x01" * 64, expect_provider=tsm.PROVIDER_SEV_GUEST) tsm.collect_report(b"\x02" * 64, expect_provider=tsm.PROVIDER_SEV_GUEST) - assert len({entry.name for entry in entries}) == 2 + assert len({entry.name for entry in fake.entries}) == 2 def test_collect_report_when_the_kernel_refuses_an_entry( @@ -226,13 +251,31 @@ def refuse(self: Path, *args, **kwargs): # noqa: ANN002, ANN003, ANN202 tsm.collect_report(b"\x00" * 64, expect_provider=tsm.PROVIDER_SEV_GUEST) -def test_collect_report_when_the_provider_returns_nothing_readable( +def test_collect_report_when_the_entry_names_no_provider( monkeypatch: pytest.MonkeyPatch, tmp_path: Path ) -> None: - """An entry that exists but produces no outblob is a failure, not empty evidence.""" + """An entry with no readable attributes cannot be trusted to be the right platform.""" root = tmp_path / "tsm-report" root.mkdir() monkeypatch.setattr(tsm, "TSM_REPORT_DIR", str(root)) + with pytest.raises(AttestationFailed, match="did not name its provider"): + tsm.collect_report(b"\x00" * 64, expect_provider=tsm.PROVIDER_SEV_GUEST) + + +def test_collect_report_when_the_right_provider_returns_no_report( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """The provider matches but outblob cannot be read: a failure, not empty evidence.""" + + def unreadable(_data: bytes) -> bytes: + raise OSError("EIO") + + install_fake_tsm( + monkeypatch, + tmp_path, + provider=tsm.PROVIDER_SEV_GUEST, + make_outblob=unreadable, + ) with pytest.raises(AttestationFailed, match="did not return a report"): tsm.collect_report(b"\x00" * 64, expect_provider=tsm.PROVIDER_SEV_GUEST)