Skip to content

fix(attestation): Azure attestation reports never pass verification - #373

Open
rajnisht7 wants to merge 2 commits into
agentrust-io:mainfrom
rajnisht7:no-pass-azure
Open

fix(attestation): Azure attestation reports never pass verification#373
rajnisht7 wants to merge 2 commits into
agentrust-io:mainfrom
rajnisht7:no-pass-azure

Conversation

@rajnisht7

Copy link
Copy Markdown
Contributor

What

Fixes verify_attestation_chain() so it can actually return passed=True for a genuine Azure SEV-SNP attestation.

Why

On Azure, REPORT_DATA is sha256(runtime_data), not the manifest hash, the guest never controls that field there (already documented in this module and in LIMITATIONS.md). The function was still requiring REPORT_DATA to equal the manifest hash for every platform, so a valid Azure report could never pass, no matter how correct the signature and chain were.

Spec impact

None

Test plan

  • pytest -v passes
  • mypy src/agent_manifest passes
  • ruff check src/ tests/ passes
  • New or updated tests cover the change
  • If spec change: CHANGELOG.md updated

DCO

All commits in this PR are signed off (git commit -s). By submitting this PR I certify the Developer Certificate of Origin.

Signed-off-by: rajnisht7 <rajnishtiwari9787@gmail.com>
@rajnisht7
rajnisht7 requested a review from a team as a code owner September 1, 2026 02:32
Signed-off-by: rajnisht7 <rajnishtiwari9787@gmail.com>

@carloshvp carloshvp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes for one blocking trust-boundary problem: the Azure platform label is being treated as proof that the manifest binding was verified.

This branch sets report_data_matched = True whenever platform == "azure-cvm-sev-snp", but verify_attestation_chain() neither calls AzureCVMProvider.verify_manifest_in_report() nor receives an authenticated result from it. The reason string says the check happens separately, but the public function has no ordering or precondition that enforces that statement.

I reproduced this on the submitted head f8f1113ae3684f587d520b08e76016fca1811c3c with one self-consistent, correctly signed SNP report:

  • the report carried a deliberately wrong PCR value;
  • AzureCVMProvider.verify_manifest_in_report(report, manifest) returned False;
  • this changed verify_attestation_chain() returned passed=True, report_data_matched=True, and signature=VERIFIED for the same report;
  • the same signed report also returned passed=True for three unrelated expected_manifest_hash values (11…, 22…, and ff…).

The new positive test encodes the same gap: its report has no pcr_read, no demonstrated vTPM/PCR verification, and still asserts passed is True solely because the platform string is Azure.

report_data being inapplicable is not equivalent to the manifest binding matching. Conflating those states turns the absence of this check into positive verification.

Before this can merge:

  1. Do not synthesize report_data_matched=True from the platform label.
  2. If this public result is meant to pass for Azure, establish the Azure composite binding inside the verification boundary: the SNP/VCEK chain, SNP runtime-data binding to the vTPM AK, the AK-signed TPM quote/PCR evidence, and the PCR value derived from the expected manifest hash.
  3. If that composite verifier is outside this PR's scope, represent Azure report-data binding as not applicable without allowing passed=True; keep the result fail-closed until separately supplied evidence is actually consumed and verified.
  4. Replace the current positive test with load-bearing controls: missing PCR evidence, a mismatching PCR, and arbitrary expected manifest hashes must fail; only authenticated matching Azure binding may pass.

The focused test file is green (14 passed), but it is green around the wrong security property. The direct-SNP non-Azure control is useful and should remain.

Also account for #363 when rebasing: Azure may share the SNP signature primitive, but it remains a distinct binding profile and must be reached through explicit platform dispatch.

@imran-siddique

Copy link
Copy Markdown
Member

Maintainer confirmation on both halves, because the premise and the remedy point different ways here.

@rajnisht7, the problem you found is real and it is ours. On Azure CVM, SEV-SNP runs under a Hyper-V paravisor and the attestation is vTPM-rooted: REPORT_DATA binds the vTPM AK, not the manifest hash, and the guest does not control that field. That is documented in this module and in LIMITATIONS.md because we established it on live SNP hardware, not because it was assumed. A function requiring REPORT_DATA == manifest_hash on every platform genuinely cannot pass a correct Azure report, and that is worth fixing.

@carloshvp is right that this fix cannot land, and the reproduction is the reason. Setting report_data_matched = True on the strength of the platform label makes the label the evidence. The function neither calls AzureCVMProvider.verify_manifest_in_report() nor receives an authenticated result from it, so the reason string saying the check happens separately is a promise the code does not keep:

the report carried a deliberately wrong PCR value; verify_manifest_in_report(report, manifest) returned False; this changed verify_attestation_chain() returned passed=True, report_data_matched=True, and signature=VERIFIED for the same report

A correctly signed report with a wrong PCR returning passed=True is the exact outcome the function exists to prevent. And it is invisible to CI: fourteen checks are green, because nothing in the suite asserts that the two agree.

The shape of a fix that would land. The Azure path needs the binding established, not assumed:

  • verify_attestation_chain() takes the manifest-binding result as an input it can check, or calls the provider itself, rather than inferring it from the platform string.
  • report_data_matched stays False unless something authenticated set it. If the caller did not supply the binding result, the honest outcome is not established, distinct from both verified and mismatched, and it must not report passed=True. That is the same three-state discipline we have been applying across the other verifiers this week: absence of a check is never a pass.
  • A test in @carloshvp's shape, a correctly signed report with a wrong PCR, asserting the function does not return passed=True. That case is the one that would have caught this, and it does not currently exist.

The general rule I want on the record, since this is the second time this month a platform label has stood in for a verification: a platform value says which verification procedure applies. It is never evidence that the procedure ran.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants