On Mon, Oct 25, 2021 at 01:04:10PM +0200, Borislav Petkov wrote: > On Thu, Oct 21, 2021 at 03:41:49PM -0500, Michael Roth wrote: > > On Thu, Oct 21, 2021 at 04:51:06PM +0200, Borislav Petkov wrote: > > > On Wed, Oct 20, 2021 at 11:10:23AM -0500, Michael Roth wrote: > > > > The CPUID calls in snp_cpuid_init() weren't added specifically to induce > > > > the #VC-based SEV MSR read, they were added only because I thought the > > > > gist of your earlier suggestions were to do more validation against the > > > > CPUID table advertised by EFI > > > > > > Well, if EFI is providing us with the CPUID table, who verified it? The > > > attestation process? Is it signed with the AMD platform key? > > > > For CPUID table pages, the only thing that's assured/attested to by firmware > > is that: > > > > 1) it is present at the expected guest physical address (that address > > is generally baked into the EFI firmware, which *is* attested to) > > 2) its contents have been validated by the PSP against the current host > > CPUID capabilities as defined by the AMD PPR (Publication #55898), > > Section 2.1.5.3, "CPUID Policy Enforcement" > > 3) it is encrypted with the guest key > > 4) it is in a validated state at launch > > > > The actual contents of the CPUID table are *not* attested to, > > Why? As counter-intuitive as it sounds, it actually doesn't buy us if the CPUID table is part of the PSP attestation report, since: - the boot stack is attested to, and if the boot stack isn't careful to use the CPUID table at all times, then attesting CPUID table after boot doesn't provide any assurance that the boot wasn't manipulated by CPUID - given the boot stack must take these precautions, guest-specific attestation code is just as capable of attesting the CPUID table contents/values, since it has the same view of the CPUID values that were used during boot. So leaving it to the guest owner to attest it provides some flexibility to guest owners to implement it as they see fit, whereas making it part of the attestation report means that the guest needs the exact contents of the CPUID page for a particular guest configuration so it can be incorporated into the measurement they are expecting, which would likely require some tooling provided by the cloud vendor, since every different guest configuration, or even changes like the ordering in which entries are placed in the table, would affect measurement, so it's not something that could be easily surmised separately with minimal involvement from a cloud vendor. And even if the cloud vendor provided a simple way to export the table contents for measurement, can you really trust it? If you have to audit individual entries to be sure there's nothing fishy, why not just incorporate those checks into the guest owner's attestation flow and leave the vendor out of it completely? So not including it in the measurement meshes well with the overall SEV-SNP approach of reducing the cloud vendor's involvement in the overall attestation process. > > > so in theory it can still be manipulated by a malicious hypervisor as > > part of the initial SNP_LAUNCH_UPDATE firmware commands that provides > > the initial plain-text encoding of the CPUID table that is provided > > to the PSP via SNP_LAUNCH_UPDATE. It's also not signed in any way > > (apparently there were some security reasons for that decision, though > > I don't know the full details). > > So this sounds like an unnecessary complication. I'm sure there are > reasons to do it this way but my simple thinking would simply want the > CPUID page to be read-only and signed so that the guest can trust it > unconditionally. The thing here is that it's not just a specific CPUID page that's valid for all guests for a particular host. Booting a guest with additional vCPUs changes the contents, different CPU models/flags changes the contents, etc. So it needs to be generated for each specific guest configuration, and can't just be a read-only page. Some sort of signature that indicates the PSP's stamp of approval on a particular CPUID page would be nice, but we do sort of have this in the sense that CPUID page 'address' is part of measurement, and can only contain values that were blessed by the PSP. The problem then becomes ensuring that only that address it used for CPUID lookups, and that it's contents weren't manipulated in a way where it's 'valid' as far as the PSP is concerned, but still not the 'expected' values for a particular guest (which is where the attestation mentioned above would come into play). > > > [A guest owner can still validate their CPUID values against known good > > ones as part of their attestation flow, but that is not part of the > > attestation report as reported by SNP firmware. (So long as there is some > > care taken to ensure the source of the CPUID values visible to > > userspace/guest attestion process are the same as what was used by the boot > > stack: i.e. EFI/bootloader/kernel all use the CPUID page at that same > > initial address, or in cases where a copy is used, that copy is placed in > > encrypted/private/validated guest memory so it can't be tampered with during > > boot.] > > This sounds like the good practices advice to guest owners would be, > "Hey, I just booted your SNP guest but for full trust, you should go and > verify the CPUID page's contents." > > "And if I were you, I wouldn't want to run any verification of CPUID > pages' contents on the same guest because it itself hasn't been verified > yet." > > It all sounds weird. Yes, understandably so. But the only way to avoid that sort of weirdness in general is for *all* guest state to be measured, all pages, all registers, etc. Baking that directly into the SEV-SNP attestation report would be a non-starter for most since computing the measurement for all that state independently would require lots of additional inputs from cloud vendor (who we don't necessarily trust in the first place), and constant updates of measurement values since they would change with every guest configuration change, every different starting TSC offset, different, maybe the order in which vCPUs were onlined, stuff that the kernel prints to log buffers, etc. But, if a guest owner wants to attempt clever ways to account for some/all of that in their attestation flow, they are welcome to try. That's sort of the idea behind SNP attestation vs. SEV. Things like page validation/encryption, cpuid enforcement, etc., reduce some of the variables/possibilities guest owners need to account for during attestation to make the process more secure/tenable, but they don't rule out all possibilities, just as Trusted Boot doesn't necessarily mean you can fully trust your OS state immediately afer boot; there are still outside influences at play, and the boot stack should guard against them wherever possible. > > > So, while it's more difficult to do, and the scope of influence is reduced, > > there are still some games that can be played to mess with boot via > > manipulation of the initial CPUID table values, so long as they are within > > the constraints set by the CPUID enforcement policy defined in the PPR. > > > > Unfortunately, the presence of the SEV/SEV-ES/SEV-SNP bits in 0x8000001F, > > EAX, are not enforced by PSP. The only thing enforced there is that the > > hypervisor cannot advertise bits that aren't supported by hardware. So > > no matter how much the boot stack is trusted, the CPUID table does not > > inherit that trust, and even values that we *know* should be true should be > > verified rather than assumed. > > > > But I think there are a couple approaches for verifying this is an SNP > > guest that are robust against this sort of scenario. You've touched on > > some of them in your other replies, so I'll respond there. > > Yah, I guess the kernel can do good enough verification and then the > full thing needs to be done by the guest owner and in *some* userspace > - not necessarily on the currently booted, unverified guest - but > somewhere, where you have maximal flexibility. Exactly, moving attestation into the guest allows for more of the these unexpected states to be accounted for at whatever level of paranoia a guest owner sees fit, while still allowing firmware to provide some basic assurances via attestation report and various features to reduce common attack vectors during/after boot. > > IMHO. > > -- > Regards/Gruss, > Boris. > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpeople.kernel.org%2Ftglx%2Fnotes-about-netiquette&data=04%7C01%7CMichael.Roth%40amd.com%7Cd9f9c20a37ce4a4b0e0608d997a72f6a%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637707566641580997%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=OmfuUZfcf3bkC%2FmSsiInQ1vScK5Onu1lHWAUH3o%2FUmE%3D&reserved=0
