verifier: Bump Max Supported Version of Attestation Report to 5

[AdithyaKrishnan]

In line with the upcoming changes in VirTEE, I'm enabling support for Attestation Reports with Versions up to 5.

[AdithyaKrishnan]

cc: @cclaudio @DGonzalezVillal

[DGonzalezVillal]

The PR to put this on the library has been merged: virtee/sev#312

Waiting on some other PRs to cut new library release with full v5 support. Current library version should not break with v5 reports I believe. Only some fields might be missing.

[AdithyaKrishnan]

cc: @fitzthum - Requesting your review

[fitzthum]

Do we need to pick up the new version of virtee to support this report version?

[DGonzalezVillal]

Do we need to pick up the new version of virtee to support this report version?

We are verifying on our side to confirm. I think you might because the MIT vectors added, the new library might be needed

[mkulke]

fyi: a bump for the az-snp-vtpm crate using sev 6.2.1 has been merged since: #895

[cclaudio]

I just tested it.
Looks good to me

[DGonzalezVillal]

Hello,

I'd still recommend holding of on merging this. I don't know about your test suites, but I have been seeing internally that machines with v5 of the report will fail to parse attestation reports without an updated version of the library:

virtee/snpguest#113

[mythi]

with v5 of the report will fail to parse attestation reports without an updated version of the library:

are you saying this PR should add a sev crate version bump as well (once it is available)?

[hyperfinitism]

The latest release (sev v6.2.1) does not yet support SEV-SNP ABI Spec Rev 1.58. In particular, the AttestationReport struct is missing the newly introduced fields LAUNCH_MIT_VECTOR and CURRENT_MIT_VECTOR. As a result, parsing a V5 attestation report with v6.2.1 will likely fail. To properly support V5, the sev crate needs to be bumped as you mentioned.

[hyperfinitism]

A new version of sev (v6.3.0) supporting ABI Spec Rev 1.58 is now available and ready for upgrade.

cargo upgrade -p sev@6.3.0

[mythi]

A new version of sev (v6.3.0) supporting ABI Spec Rev 1.58 is now available and ready for upgrade.

cargo upgrade -p sev@6.3.0

we cannot bump until virtee/sev#326 is fixed

[Xynnn007]

note: virtee/sev#326 is fixed.

[hyperfinitism]

For full support for V5, it might not be enough to just update the parsing of the Attestation Report (via bumping the sev crate). The newly added fields such as TIO_EN and PAGE_SWAP_DISABLE also need to be handled — for example in

trustee/deps/verifier/src/snp/mod.rs

Line 381 in 6490c70

pub(crate) fn parse_tee_evidence(report: &AttestationReport) -> TeeEvidenceParsedClaim {

trustee/deps/verifier/src/az_snp_vtpm/mod.rs

Line 288 in 6490c70

pub(crate) fn parse_tee_evidence_az(report: &AttestationReport) -> TeeEvidenceParsedClaim {

[fitzthum]

Ok we putting this in the release or no? @AdithyaKrishnan @ryansavino

[DGonzalezVillal]

@fitzthum

Just updating the version report as mentioned won’t be enough to fully support the new attestation report. I’ve released version 7.1.0 of the Virtee library, which includes the CPUID fix and reintroduces serde in the crate. This release should address the trustee issues that have been observed.

Since @AdithyaKrishnan opened the PR, it’s assigned to him to incorporate these changes and add full support into trustee. Corresponding updates will also need to be made on the guest-components side.

We also wanted to ask about the release timeline. We want to ensure these changes are included before release, but we don’t want them to hold things up unnecessarily.

[fitzthum]

We also wanted to ask about the release timeline. We want to ensure these changes are included before release, but we don’t want them to hold things up unnecessarily.

We only have a day or two to land things in this repo before the release. cc @AdithyaKrishnan

[fitzthum]

Just updating the version report as mentioned won’t be enough to fully support the new attestation report.

What would happen if we merged this without any of the other changes? Would the new reports work at all? What do you mean by "fully support?"

[DGonzalezVillal]

If you bump to release 6.3.0 then V5 of the report will be supported, you shouldn't get parsing issues anymore. The CPUID fix won't be included although.

v 7.1.0 includes the CPUID fix, but there are some code changes required.

If you merged this without any other changes then the library won't parse V5 reports and you will still see the CPUID compiling issue.

[DGonzalezVillal]

Quickest way to add support would be bumping to 6.3.0 in addition to bumping the max report version to 5. You would only still see the CPUID compilation issues.

Most complete solution is 7.1.0, but it will require some code changes.

[mythi]

If you bump to release 6.3.0 then V5 of the report will be supported, you shouldn't get parsing issues anymore. The CPUID fix won't be included although.

Trustee uses v6.3.1 currently. See c96dbe6.

[DGonzalezVillal]

Oh I didn't know, then you can just bump the max report version.

[DGonzalezVillal]

You'll still see the cpuid issues although, but if you're ok with that, then this can be merged and we can have a separate PR for the 7.1.0 update.

[AdithyaKrishnan]

I can create a follow-up PR then for 7.1.0 update?

[fitzthum]

I can create a follow-up PR then for 7.1.0 update?

Ok. I guess the TODOs will be to make whatever code changes we need and also to add the new report fields to the parsed claims.

[DGonzalezVillal]

Guest components should also be bumped to 6.3.1 if you guys haven't

[fitzthum]

Ok. Let's merge this. We will also need to update guest components to get support for the v5 reports.

[AdithyaKrishnan]

PR to update GC confidential-containers/guest-components#1107