On Fri, Oct 07, 2022 at 12:43:05PM +0100, Daniel P. Berrangé wrote:
The doc reads well; and is a strict improvement. I only have a couple
of small nits inline.
[...]
> Memory
> ------
>
> @@ -373,6 +387,94 @@ running:
> # dmesg | grep -i sev
> AMD Secure Encrypted Virtualization (SEV) active
>
> +Guest attestation for SEV/SEV-ES from a trusted host
> +====================================================
> +
> +Before a confidential guest is used, it may be desirable to attest to boot
nit: "attest to" --> "attest the"
> +measurement. To be trustworthy the attestation process needs to be performed
> +from a machine that is already trusted. This would typically be a physical
> +machine that the guest owner controls, or could be a previously launched
> +confidential guest that has already itself been attested. Most notably, it is
> +**not** possible to securely attest a guest from the hypervisor host itself,
> +as the goal of the attestation process is to detect whether the hypervisor is
> +malicious.
> +
> +Performing an attestation requires that the ``<launchSecurity>`` element is
> +configured with a guest owner DH certificate, and a session data blob. These
nit: "DH certificate" --> "Diffie-Hellman (DH) certificate"?
> +must be unique for every guest launch attempt. Any reuse will open avenues of
> +attack for the hypervisor to fake the measurement. Unique data can be generated
> +using the ``sevctl`` tool.
Might want to mention that the ``sevtool`` needs to be installed
separately (or link to its GitHub repo?). I see it packaged at least in
Fedora.
> +
> +First of all the PDH for the hypervisor host needs to be obtained.
Might want to expand the "PDH" on first use: PDH (Platform
Diffie-Hellman key). It would also be nice to define what this key
actually is (there are at least 11 keys, looking at AMD's SEV API doc).
Consider rephrasing the above to something like:
"First of all the Platform Diffie-Hellman key (PDH) for the
hypervisor host needs to be obtained. The PDH is used to negotiate
a master secret between the SEV firmware and external entities."
> The admin
> +of the hypervisor can extract this using::
> +
> + $ sevctl export --full ${hostname}.pdh
> +
> +Upon receiving the PDH associated with the hypervisor, the guest owner should
> +validate its integrity::
[...]
> +With it launched, it is possible to query the launch measurement::
> +
> + $ virsh domlaunchsecinfo ${myvmname}
> + sev-measurement: LMnv8i8N2QejezMPkscShF0cyPYCslgUoCxGWRqQuyt0Q0aUjVkH/T6NcmkwZkWp
> + sev-api-major : 0
> + sev-api-minor : 24
> + sev-build-id : 15
> + sev-policy : 3
> +
> +The techiques required to validate the measurement reported are beyond the
> +scope of this document. Fortunately, libvirt provides a tool that can be used
> +to perform this validation::
> +
> + $ virt-qemu-sev-validate \
> + --measurement LMnv8i8N2QejezMPkscShF0cyPYCslgUoCxGWRqQuyt0Q0aUjVkH/T6NcmkwZkWp
> + --api-major 0
> + --api-minor 24
> + --build-id 15
> + --policy 3
> + --tik ${myvmname}_tik.bin
> + --tek ${myvmname}_tek.bin
> + OK: Looks good to me
Nice!
I'm going to give this a try on an SEV-ES box. Regardless, the rest of
the doc reads well. Thank you for documenting!
Feel free to choose what nits to address from the above. With or without
the nits addressed:
Reviewed-by: Kashyap Chamarthy <kchamart@xxxxxxxxxx>
> +The `man page <../manpages/virt-qemu-sev-validate.html>`__ for
> +``virt-qemu-sev-validate`` outlines a great many other ways to invoke this
> +tool.
> +
> Limitations
> ===========
>
> --
> 2.37.3
>
--
/kashyap
