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)