Re: [PATCH v6 40/42] virt: Add SEV-SNP guest driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 10/27/21 4:05 PM, Peter Gonda wrote:
....


Thanks for updating this sequence number logic. But I still have some
concerns. In verify_and_dec_payload() we check the encryption header
but all these fields are accessible to the hypervisor, meaning it can
change the header and cause this sequence number to not get
incremented. We then will reuse the sequence number for the next
command, which isn't great for AES GCM. It seems very hard to tell if
the FW actually got our request and created a response there by
incrementing the sequence number by 2, or if the hypervisor is acting
in bad faith. It seems like to be safe we need to completely stop
using this vmpck if we cannot confirm the PSP has gotten our request
and created a response. Thoughts?


Very good point, I think we can detect this condition by rearranging the
checks. The verify_and_dec_payload() is called only after the command is
succesful and does the following checks

1) Verifies the header
2) Decrypts the payload
3) Later we increment the sequence

If we arrange to the below order then we can avoid this condition.
1) Decrypt the payload
2) Increment the sequence number
3) Verify the header

The descryption will succeed only if PSP constructed the payload.

Does this make sense ?

Either ordering seems fine to me. I don't think it changes much though
since the header (bytes 30-50 according to the spec) are included in
the authenticated data of the encryption. So any hypervisor modictions
will lead to a decryption failure right?

Either case if we do fail the decryption, what are your thoughts on
not allowing further use of that VMPCK?


We have limited number of VMPCK (total 3). I am not sure switching to
different will change much. HV can quickly exaust it. Once we have SVSM
in-place then its possible that SVSM may use of the VMPCK. If the
decryption failed, then maybe its safe to erase the key from the secrets
page (in other words guest OS cannot use that key for any further
communication). A guest can reload the driver will different VMPCK id
and try again.

SNP cannot really cover DOS at all since the VMM could just never
schedule the VM. In this case we know that the hypervisor is trying to
mess with the guest, so my preference would be to stop sending guest
messages to prevent that duplicated IV usage. If one caller gets an
EBADMSG it knows its in this case but the rest of userspace has no
idea. Maybe log an error?



Yap, we cannot protect against the DOS. This is why I was saying that we zero the key from secrets page so that guest cannot use that key for any future communication (whether its from rest of userspace or kexec kernel). I can update the driver to log the message and ensure that future messages will *not* use that key. The VMPCK ID is a module params, so a guest can reload the driver to use different VMPCK.


thanks




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux