firmware/guest: add ALIAS_CHECK_COMPLETE to attestation report

[amit3s]

After mitigating CVE-2024-21944, SEV firmware exposes a new bit to the guest attestation report confirming the mitigation of the CVE. Include this bit in the report.

The bit is currently documented in the security bulletin page (below), and will be included in the spec in its next update.

https://www.amd.com/en/resources/product-security/bulletin/amd-sb-3015.html

[tylerfanelli]

LGTM, although there also seems to be another addition to PLATFORM_STATUS as well. Would you mind also adding that?

[amit3s]

LGTM, although there also seems to be another addition to PLATFORM_STATUS as well. Would you mind also adding that?

Sure - but looking at that code, it's quite out of date (and buggy), it'll be more work than just adding the feature. Can't promise to get it working in the next couple weeks, though..

[tylerfanelli]

What do you mean? It's not as simple as adding a field to PLATFORM_STATUS? What code is buggy and out-of-date?

[amit3s]

What do you mean? It's not as simple as adding a field to PLATFORM_STATUS? What code is buggy and out-of-date?

There are more fields described in the current spec for byte 03h.

For the bug - is_rmp_init currently is a u8 -- which is the entire byte 03h; where it should just be the first bit of that byte. Other bits in that byte are used for other things -- including this new DRAM_ALIAS_CHECKED bit.

[tylerfanelli]

So it can be replaced by a bitflags! representation of a u8 and all bitfields of that struct can be represented. I wouldn't call that "buggy", just needing an update to accommodate some fields added.

I'll add in the changes.

[tylerfanelli]

@DGonzalezVillal @larrydewey PTAL.

[amit3s]

So it can be replaced by a bitflags! representation of a u8 and all bitfields of that struct can be represented. I wouldn't call that "buggy", just needing an update to accommodate some fields added.

Well, if RMP init failed, but some other bit there is set, the current code will insist RMP init is done -- qualifies to be called "buggy" :)

[tylerfanelli]

So it can be replaced by a bitflags! representation of a u8 and all bitfields of that struct can be represented. I wouldn't call that "buggy", just needing an update to accommodate some fields added.

Well, if RMP init failed, but some other bit there is set, the current code will insist RMP init is done -- qualifies to be called "buggy" :)

Yes, sorry, I didn't realize that RMP init failed with this.

[DGonzalezVillal]

Hello @amit3s ,

We can't merge this changes like this as of right now. This FW update bumps the Attestation Report from version 2 to 3, and we are looking at how we want to keep backwards compatibility between versions inside of the library for the Attestation Report. There are other fields that need to be added to the attestation report that will be present in this version bump, so while as a community we decide how we want to move forward to the Attestation Report changes, we won't be modifying the field yet.

[amit3s]

Hello @amit3s ,

We can't merge this changes like this as of right now. This FW update bumps the Attestation Report from version 2 to 3, and we are looking at how we want to keep backwards compatibility between versions inside of the library for the Attestation Report.

The addition of this field doesn't necessitate a version update. If an update is happening, it's a separate thing, right?

There are other fields that need to be added to the attestation report that will be present in this version bump, so while as a community we decide how we want to move forward to the Attestation Report changes, we won't be modifying the field yet.

I don't mind either way, but this seems wrong to me - you're gating addition of a bitfield to the report - that firmware already publishes - on something unrelated.

[DGonzalezVillal]

The addition of this field doesn't necessitate a version update. If an update is happening, it's a separate thing, right?

It does because this is only present on version 3 of the Attestation report. If you have hw that is not updated and is still using version 2, then this field won't be present.

I don't mind either way, but this seems wrong to me - you're gating addition of a bitfield to the report - that firmware already publishes - on something unrelated.

Not really gating it, it's more of a matter on how to integrate it. Again this fix bumped the attestation report from version 2 to version 3.

Not saying this is the main solution but this is one possible way of integrating it: #268