feat(tdx): expose the platform floor, TEE_TCB_SVN and TDATTRIBUTES (#117) - #126
Open
zohebk8s wants to merge 2 commits into
Open
feat(tdx): expose the platform floor, TEE_TCB_SVN and TDATTRIBUTES (#117)#126zohebk8s wants to merge 2 commits into
zohebk8s wants to merge 2 commits into
Conversation
…gentrust-io#117) snp.py parses the fields a firmware-floor appraisal needs: policy, vmpl, reported_tcb. tdx.py stopped at report_data, mrtd and the raw body, so a caller wanting the same floor on Intel had to index into raw or skip the appraisal. TdxReport now carries TEE_TCB_SVN and TDATTRIBUTES, parsed once alongside mrtd. The offsets were validated against a real GCP c3 TDX quote and cross-checked against this parser's own output on the same bytes: report_data equals raw[520, 584) and mrtd equals raw[136, 184), so these two fields sit inside a layout the parser already agrees with rather than one this change asserts. Byte 0 of TEE_TCB_SVN is appraised and nothing else, and bit 0 of TDATTRIBUTES is appraised and nothing else. The rest of both is carried and not judged, in those words, because the next reader will assume a parsed field is an appraised one. The two captures already here make the reason concrete: 0d 01 08 and 0d 01 04, the same SEAM SVN 13 with a different byte 2, so whatever byte 2 tracks it is not the SEAM module version and anything comparing the array whole would order those captures on a byte nobody can name. A test edits every carried byte and every carried attribute bit and asserts the verdict does not move, because that rule is the easiest thing here to lose in a later refactor. The floor is caller-supplied, as SEV-SNP's already is, and it is the same object rather than a second mechanism. forbid_debug is shared across vendors because the SEV-SNP guest-policy debug bit and the TDX TDATTRIBUTES debug bit are one operator intent expressed twice. Results have three states. not_evaluated always names its reason, so an unappraisable platform cannot read as green to anyone aggregating; an appraisal that only abstained is not ok, because nothing was established. A quote that will not parse and a body too short for the offset both fail, with the parse error and the length found, rather than abstaining: a broken artifact and a missing policy must not read the same. There is no VMPL analogue on TDX and none is invented. Staleness sits beside the verdict and never inside it, as a stateless per-appraisal observation. No watermark is kept: a stored high-water mark needs somewhere to live and gives an attacker something to move, and one run against a platform reporting a high SVN would shift it for everything appraised afterwards. Aggregating the observations into a fleet view is the caller's job, because only the caller knows what its fleet is. Refs agentrust-io#117 Signed-off-by: Mohammed Zoheb Shaik <zoheb.shaik7@gmail.com>
…oor-117 Signed-off-by: Mohammed Zoheb Shaik <zoheb.shaik7@gmail.com> # Conflicts: # CHANGELOG.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What and why
Expose the TDX platform floor, as approved on #117. SPEC section 3.2.
snp.pyparses the fields a firmware-floor appraisal needs:policy,vmpl,reported_tcb.tdx.pystopped atreport_data,mrtdand the raw 584-byte body, so a caller wanting the same floor on Intel had to index intorawthemselves or skip the appraisal. That asymmetry is the issue.Both fields on
TdxReport.TEE_TCB_SVNat[0, 16)andTDATTRIBUTESat[120, 128), parsed once alongsidemrtd. The offsets were validated against a real GCP c3 TDX quote and cross-checked against this parser's own output on the same bytes:report.report_dataequalsraw[520, 584)andreport.mrtdequalsraw[136, 184), so these two fields sit inside a layout the parser already agrees with rather than one this change asserts. There is a test for that cross-check.Carried and not judged. Byte 0 of
TEE_TCB_SVNis appraised and nothing else; bit 0 ofTDATTRIBUTESis appraised and nothing else. The docstring says "carried and not judged" in those words, because the next reader will assume a parsed field is an appraised one. The two captures already here make the reason concrete:0d 01 08and0d 01 04, the same SEAM SVN 13 with a different byte 2, so anything comparing the array whole would order them on a byte nobody can name.One floor object, not a second mechanism.
PlatformFlooris caller-supplied, as SEV-SNP's already is.forbid_debugis shared across vendors because the SEV-SNP guest-policy debug bit and the TDXTDATTRIBUTESdebug bit are one operator intent expressed twice. There is no VMPL analogue on TDX and none is invented: nothing is emitted for it.Three states.
passed,failed,not_evaluated, and an abstention always names its reason. An appraisal that only abstained is notok, because nothing was established, and anot_evaluatedcheck is never truthy. A quote that will not parse and a body too short for the offset bothfail, with the parse error and the length found, rather than abstaining: a broken artifact and a missing policy must not read the same.Staleness beside the verdict, never inside it. Stateless and per appraisal, in the shape agreed:
{"floor": 13, "reported": 15, "floor_behind": true}. No watermark is kept anywhere. A stored high-water mark needs somewhere to live and gives an attacker something to move, and one run against a platform reporting a high SVN would shift it for everything appraised afterwards, leaving either a fleet that looks stale when it is not or a floor that looks current when it is not. Aggregating these into a fleet view is the caller's job, because only the caller knows what its fleet is. A floor behind the fleet does not fail the release; there is a test for that too.The capture lands separately, per the issue.
Type
Checklist
git commit -s)pytest -q,mypy --strict src/wcm, andbanditpass locally; coverage 80% or better. 735 passed, 4 skipped; mypy clean over 34 files; bandit clean.CHANGELOG.mdupdatedNotes for reviewers
The test you asked for
test_editing_byte_two_makes_no_difference_to_the_verdictedits byte 2 of a copy and asserts the whole appraisal is unchanged, checks and staleness included.test_editing_any_carried_byte_makes_no_differencedoes the same for every index from 1 to 15, andtest_editing_a_carried_attribute_bit_makes_no_differenceflips bit 28, which is set on every capture here and which nobody has given a meaning.test_byte_zero_is_the_one_that_does_move_the_verdictis the counterpart, so those tests cannot pass by appraising nothing at all. That was the failure mode worth guarding against: a carried-not-judged test suite that goes green because the appraisal quietly stopped appraising.Where the three states show up
FloorCheck.okis true only forpassed.not_evaluatedis deliberately not truthy, because rounding it up to verified is the thing this type exists to prevent, andFloorAppraisal.okrequires at least one check and all of them passed.On the staleness shape
stalenessisNonewhen there is no floor to be behind, so an abstention reports nothing rather than a zero.as_dict()puts it at the top level besidechecksandok, and a test asserts it appears in no individual check.One thing worth your eye
PlatformFloorcarriesseam_svnandforbid_debugand nothing else. The SEV-SNP fields from the proposal's table are not here, because this PR does not appraise SEV-SNP and a floor field that nothing reads is the same problem as a check that always passes. They belong on this same object when the SNP appraisal lands, which keeps it one mechanism rather than two. Say if you would rather they went in now as reserved.