svsm: add SVSM VTPM Service Attestation #541

IT302 · 2024-11-28T14:06:14Z

vTPM service attestation is described in section 8.3 of "Secure VM Service Module for SEV-SNP Guests, Publication #58019 Revision: 1.00 Issue Date: July 2023". It certifies the Endorsement Key (EK) of the vTPM by providing the TPMT_PUBLIC structure of the EK. This is crucial for downstream projects like Keylime, as the SVSM vTPM lacks an EK certificate found in physical TPMs to anchor trust.

The attestation is part of the SVSM Attestation Protocol and uses the SVSM_ATTEST_SINGLE_SERVICE call (see section 7 of the specifications). It is triggered by making an SVSM_ATTEST_SINGLE_SERVICE call with the GUID set to c476f1eb-0123-45a5-9641-b4e7dde5bfe3. The attestation code returns the VMPL0 attestation report and the vTPM Service Manifest Data Structure (TPMT_PUBLIC structure of the EK). The REPORT_DATA in the SNP attestation request is the SHA-512 digest of the input nonce and the vTPM Service Manifest Data Structure.

The vTPM initialization function was modified to generate an RSA 2048-bit EK from the TPM's Endorsement Primary Seed (EPS) and cache the public key as a TPMT_PUBLIC structure. This cached EK public key can be retrieved later for vTPM service attestation. The EK is created with the TCG default EK template (see Table 4 of the "TCG EK Credential Profile For TPM Family 2.0; Level 0 Version 2.5 Revision 2.0"). Since the EK is derived from the EPS, it can be recreated upstream at any time. For example, the same EK can be recreated in an OS using the TSS2 command "tpm2_createek -c ek.ctx -G rsa -u ek.pub" and compared against the one returned by vTPM service attestation.

vTPM service attestation as specified can only return one type of EK, so the implementation supports RSA 2048-bit EK as defined in Table 4 of the "TCG EK Credential Profile For TPM Family 2.0; Level 0 Version 2.5 Revision 2.0," which is the most common Trusted Computing Group(TCG) EK type.

Resolves #437, resolves #361

vTPM service attestation is described in section 8.3 of "Secure VM Service Module for SEV-SNP Guests, Publication #58019 Revision: 1.00 Issue Date: July 2023". It certifies the Endorsement Key (EK) of the vTPM by providing the TPMT_PUBLIC structure of the EK. This is crucial for downstream projects like Keylime, as the SVSM vTPM lacks an EK certificate found in physical TPMs to anchor trust. The attestation is part of the SVSM Attestation Protocol and uses the SVSM_ATTEST_SINGLE_SERVICE call (see section 7 of the specifications). It is triggered by making an SVSM_ATTEST_SINGLE_SERVICE call with the GUID set to c476f1eb-0123-45a5-9641-b4e7dde5bfe3. The attestation code returns the VMPL0 attestation report and the vTPM Service Manifest Data Structure (TPMT_PUBLIC structure of the EK). The REPORT_DATA in the SNP attestation request is the SHA-512 digest of the input nonce and the vTPM Service Manifest Data Structure. The vTPM initialization function was modified to generate an RSA 2048-bit EK from the TPM's Endorsement Primary Seed (EPS) and cache the public key as a TPMT_PUBLIC structure. This cached EK public key can be retrieved later for vTPM service attestation. The EK is created with the TCG default EK template (see Table 4 of the "TCG EK Credential Profile For TPM Family 2.0; Level 0 Version 2.5 Revision 2.0"). Since the EK is derived from the EPS, it can be recreated upstream at any time. For example, the same EK can be recreated in an OS using the TSS2 command "tpm2_createek -c ek.ctx -G rsa -u ek.pub" and compared against the one returned by vTPM service attestation. vTPM service attestation as specified can only return one type of EK, so the implementation supports RSA 2048-bit EK as defined in Table 4 of the "TCG EK Credential Profile For TPM Family 2.0; Level 0 Version 2.5 Revision 2.0," which is the most common Trusted Computing Group(TCG) EK type. Resolves coconut-svsm#437, resolves coconut-svsm#361 Signed-off-by: Geoffrey Ndu <[email protected]>

IT302 · 2024-11-28T14:07:54Z

We need this for our upcoming Keylime TEE enhancements. There is an OS level test script for testing the feature, https://github.com/hpe-security-lab/svsm-vtpm-test

joergroedel · 2024-12-11T15:27:24Z

@cclaudio can you please help to review this?

cclaudio · 2024-12-11T16:47:21Z

Yes, I will review it today later.

deeglaze · 2024-12-11T17:00:00Z

Attest single service doesn’t bind the fact that it’s a single service request, so it could be interpreted as a full attestation that just has one service, right? That could be a problem if you want to ensure a service is missing.

tlendacky · 2024-12-11T18:22:00Z

Attest single service doesn’t bind the fact that it’s a single service request, so it could be interpreted as a full attestation that just has one service, right? That could be a problem if you want to ensure a service is missing.

The manifest output of a single service vs all services is different, with the all services call containing the all services manifest GUID.

stefano-garzarella · 2024-12-13T15:39:33Z

kernel/src/protocols/attestation.rs

+        }
+
+        // Won't panic on amd64 as usize > u32 always
+        let size = self.nonce_size as usize;


I guess this is true in all cases, not just amd64, so Into should be implemented, so I suggest this change:

Suggested change

let size = self.nonce_size as usize;

let size = self.nonce_size.into();

This applies also for manifest and report.

stefano-garzarella · 2024-12-13T15:42:14Z

kernel/src/protocols/attestation.rs

+        // Check that the nonce length is not greater than 4k as we are going to map in only one
+        // page. Nonce size is 64 bytes per Table 21 of
+        // "SEV Secure Nested Paging Firmware ABI Specification, Revision 1.56"
+        if size > PAGE_SIZE {
+            return Err(SvsmReqError::invalid_parameter());
+        }


Since we validate the gpa in get_nonce_gpa_and_size(), what about validate also the size in the same function?

This applies also for manifest and report.

stefano-garzarella · 2024-12-13T15:45:55Z

kernel/src/protocols/attestation.rs

+
+    // Get attestation report from PSP with Sha512(nonce||manifest) as REPORT_DATA.
+    let report = get_attestation_report(hash.as_slice())?;
+    // Validate that the report is not empty


Is this a TODO or an old comment?

Good catch , that's an old comment

stefano-garzarella · 2024-12-13T16:00:34Z

kernel/src/vtpm/tcgtpm/mod.rs

+        // Creates RSA 2048-bit EK using TPM2_CreatePrimary command and TCG default EK template
+        //
+        // TPM2_CreatePrimary command is defined in Table 173 — TPM2_CreatePrimary Command, 365 of
+        // "Trusted Platform Module Library Part 3: Commands-Codes, Family “2.0”, Level 00,
+        // Revision 01.38".
+        //
+        // The TCG default EK template is defined in "Table 2: Default EK Template (TPMT_PUBLIC)
+        // L-1: RSA 2048 (Storage)" of "TCG EK Credential Profile For TPM Family 2.0; Level 0
+        // Version 2.5 Revision 2".
+        //
+        // See also "TCG TSS 2.0 Overview and Common Structures Specification, Version 1.0,
+        // Level 2 Revision 10".


We usually put the documentation of a function before it, and maybe here it's a doc comment, so /// (or //! if you want to keep it inner): https://doc.rust-lang.org/reference/comments.html

This is not meant to be a function documentation. It is an implementation note for whoever is going to work on the code, similar to the one in the init() function below. Perhaps changing "Creates" to "Create" may make the intention clearer.

Okay, I see now!

stefano-garzarella · 2024-12-13T16:05:25Z

kernel/src/vtpm/tcgtpm/mod.rs

+// Definitions from "Trusted Platform Module Library Part 4: Supporting Routines – Code,
+// Family “2.0”, Level 00, Revision 01.38"
+const TPM_ST_SESSIONS: u16 = 0x8002;
+const TPM_CC_CREATEPRIMARY: u32 = 0x00000131;
+const TPM_RH_ENDORSEMENT: u32 = 0x4000000B;
+const TPM_ALG_RSA: u16 = 0x0001;
+const TPM_ALG_SHA256: u16 = 0x000B;
+const TPM_ALG_AES: u16 = 0x0006;
+const TPM_ALG_CFB: u16 = 0x0043;
+const TPM_ALG_NULL: u16 = 0x0010;
+const TPM_KEY_BITS_2048: u16 = 2048;
+const TPM_RS_PW: u32 = 0x40000009;


What about if we generate bindings with bindgen for example from TPMCmd/tpm/include/public/TpmTypes.h ? Anyway, maybe I would keep them in a separate file.

Good idea. Will look into it.

stefano-garzarella · 2024-12-13T16:06:39Z

@joergroedel can you approve the workflow?

cclaudio

Hi @IT302, thank you for the PR. I have made some comments and requested a few changes.

With the ATTEST protocol implemented in the SVSM, we should be able to call it from guest userpsace through the configfs-tsm interface , see [1] for ABI specification and [2] for example. It would be great if we can test this PR through that interface.

[1] - https://github.com/coconut-svsm/linux/blob/svsm/Documentation/ABI/testing/configfs-tsm
[2] - coconut-svsm/linux@70e6f7e

cclaudio · 2024-12-13T18:33:16Z

kernel/src/protocols/attestation.rs

+#[cfg(all(feature = "vtpm", not(test)))]
+const SVSM_ATTEST_VTPM_GUID: u128 = u128::from_le_bytes([
+    0xeb, 0xf1, 0x76, 0xc4, 0x23, 0x01, 0xa5, 0x45, 0x96, 0x41, 0xb4, 0xe7, 0xdd, 0xe5, 0xbf, 0xe3,
+]);


We should use the Uuid struct defined in the kernel crate, it handles byte swaps. Something like:

use crate::utils::fw_meta::Uuid; const SVSM_ATTEST_VTPM_GUID: Uuid = Uuid::from("c476f1eb-0123-45a5-9641-b4e7dde5bfe3");

I tried using the pattern above, but from and from_str are non-const functions, so the compiler did not accept them.
I also attempted the following implementation:

impl Uuid { pub const fn new() -> Self { Uuid { data: [0; 16] } } pub const fn from_bytes(mem: &[u8; 16]) -> Self { Self { data: [ mem[3], mem[2], mem[1], mem[0], mem[5], mem[4], mem[7], mem[6], mem[8], mem[9], mem[10], mem[11], mem[12], mem[13], mem[14], mem[15], ], } } } impl From<&[u8; 16]> for Uuid { fn from(mem: &[u8; 16]) -> Self { Uuid::from_bytes(mem) } }

However, the compiler complained that to use a constant of type Uuid in a pattern, Uuid must be annotated with #[derive(PartialEq)] and that the traits must be derived; manual impls are not sufficient.

Do you understand the reasoning behind the way utils::fw_meta::Uuid is implemented. It seems there is no way to build a UUID from a string or bytes at compile time.

The external uuid crate provides a uuid! macro that allows for compile-time generation of UUIDs. Would it be better to use that instead?

Hmm .. I don't remember why we have our own UUID implementation, maybe for simplicity.

I think you are talking about https://docs.rs/uuid/latest/uuid. It seems they have a support for no-std (see Embedded section), but they also have some dependencies on serde which might not be needed in the SVSM.

@joergroedel any thoughts about start using uuid? We would need to test it, but it seems ok.

I think you are talking about https://docs.rs/uuid/latest/uuid. It seems they have a support for no-std (see Embedded section), but they also have some dependencies on serde which might not be needed in the SVSM.

@cclaudio #528 is going to introduce serde deps in any case, so it may not be a big issue.

@joergroedel any thoughts about start using uuid? We would need to test it, but it seems ok.

+1 on uuid, we also already use it in igvmbuilder but of course with std, so no_std need to be tested, I agree.

cclaudio · 2024-12-13T19:01:45Z

kernel/src/protocols/attestation.rs

+        self.manifest_ver == 0
+    }
+    pub fn get_guid(&self) -> u128 {
+        u128::from_le_bytes(self.guid)


Return Uuid type

cclaudio · 2024-12-13T21:46:34Z

kernel/src/protocols/attestation.rs

+    // "SEV Secure Nested Paging Firmware ABI Specification, Revision 1.56"
+    if report.len() > PAGE_SIZE {
+        log::error!("Malformed VTPM service attestation report");
+        return Err(SvsmReqError::unsupported_protocol());


We should return an error specific for the ATTEST protocol and fix the comment above.

The table 4 in the SVSM spec says that 0x8000_1000 - 0xFFFF_FFFF result codes are defined by the protocol that was invoked. You can use this APIC error as reference https://github.com/coconut-svsm/svsm/blob/main/kernel/src/protocols/errors.rs#L83

cclaudio · 2024-12-13T21:50:45Z

kernel/src/protocols/attestation.rs

+    Ok(())
+}
+
+fn attest_multiple_service(_params: &RequestParams) -> Result<(), SvsmReqError> {


nit. services in plural.

Good catch!

cclaudio · 2024-12-13T21:55:15Z

kernel/src/protocols/attestation.rs

+    /// Checks if gpa is page aligned, valid  and does not cross page boundary.
+    pub fn get_manifest_gpa_and_size(&self) -> Result<(PhysAddr, usize), SvsmReqError> {
+        let gpa = PhysAddr::from(self.manifest_gpa);
+        if !gpa.is_aligned(8) || !valid_phys_address(gpa) || gpa.crosses_page(8) {


is_page_alined() seems a better option here. I think we also should check page boundaries using the value size rather than the type size, e.g.:

if !gpa.is_page_aligned() || !valid_phys_address(gpa) || gpa.crosses_page(self.manifest_size) {

cclaudio · 2024-12-13T23:18:01Z

kernel/src/vtpm/tcgtpm/mod.rs

+        Ok(())
+    }
+
+    fn get_ekpub(&self) -> Result<Vec<u8>, SvsmReqError> {


Should we move this and the create_ek_rsa2048() functions to the libtcgtpm crate? They seem to bring a lot of TPM symbols/details to the kernel crate.

Good idea. Where exactly in libtcgTPM are you thinking? Moving into the core TcgTPM means we will have to patch TcgTPM each time we update it.

A halfway house could be what was suggested by @stefano-garzarella , which is to import the TPM symbols via bindings.

My only concern is that maintaining the create_ek_rsa2048 may not be easy long term because it is hand crafting a TPM command that is relativaly complex. On the other hand, it is just one function, so for now I think can just import the TPM symbols via bindings as @stefano-garzarella suggested.

I was thinking about some options but none of them seem to be optimal. One option could be to move some of its logic to libtcgtpm to reduce the create_ek_rsa2048 complexity in the kernel crate.

Another idea could be to use functions that are already implemented by the upstream TPM implementation, e.g. TPM2_CreatePrimary. However, I just revisited it and it is an internal/private API, in other words, it's not necessarily stable.

cclaudio · 2024-12-13T23:22:57Z

kernel/src/protocols/attestation.rs

+    // none of the TCG EK profiles will produce a manifest i.e. TPMT_PUBLIC larger than 4K.
+    if manifest.len() > PAGE_SIZE {
+        log::error!("Malformed VTPM service attestation manifest");
+        return Err(SvsmReqError::unsupported_protocol());


Same here. We should return an error code specific for the ATTEST protocol

cclaudio · 2024-12-14T00:27:14Z

kernel/src/vtpm/tcgtpm/mod.rs

 }

 impl TcgTpm {
    pub const fn new() -> TcgTpm {
        TcgTpm {
            is_powered_on: false,
+            ekpub: None,


I think it makes more sense to cache the ekpub somewhere in the ATTEST protocol, which is the code that will maintain and consume it. We could cache it in a new sub-module such as protocols/service_manifest.rs that will do manifest operations and maintain the data (ekpub) for services like the vTPM.

The EKpub effectively serves as the vTPM’s “permanent” identity and, in my opinion, should be generated and managed exclusively by the vTPM. Crucially, it is created during the vTPM’s initialization phase, before the vTPM is "handed off" to untrusted lower privileged VMPL users. Keeping the identity generation process fully self-contained within the vTPM’s initialization routine helps ensure that it maintains a single, consistent identity. This approach also reduces the likelihood of being tricked into signing unapproved identities by changing the EPS for example and requesting VTPM attestation.

Each service should generate and provide what it considers as the valid service manifest data, and the attestation protocol subsystem should be only concerned with getting the manifest attested. This way, we can keep coupling of subsystems to the barest minimum and improve maintainability. Perhaps, we should rename vtpm_get_ekpub() to vtpm_get_manifest() to make the intent clearer.

We can create it in the vTPM initialization phase, my suggestion was about where we should cache it. But yeah, I agree with you, caching it here in the TcgTpm structure looks better.

cclaudio · 2024-12-14T00:34:39Z

kernel/src/protocols/attestation.rs

+    let nonce = ops.get_nonce()?;
+
+    // Get the cached EKpub from the VTPM. Returns an error if the EKpub is not cached.
+    let manifest = vtpm_get_ekpub()?;


The ekpub is only part of the manifest, which is described in the table 12 of the SVSM spec. We should rather assemble the manifest structure and add the vTPM service entry to it.

We could create a new sub-module under protocols (e.g. service_manifest.rs) to have all the manifest operations. It could also be a good place to cache the vTPM service entry, which contains the ekpub and the vTPM service GUID.

From my understanding of the SVSM spec, vTPM service attestation does not use the Services Manifest format defined in Table 12: Services Manifest. Page 29 of the spec says “The services manifest is identified by a 16-byte GUID, 4-byte length, and a 4-byte services count
at the beginning of the manifest. …”

Section 7.2 SVSM_ATTEST_SINGLE_SERVICE Call, which covers the attestation of single services, says on page 31:
“The SVSM will assemble a service manifest that will be used as input to the attestation request.
The service will produce a descriptive manifest in a service-defined format. If the size of the
assembled service manifest exceeds the size of the supplied service manifest buffer, RCX will be
set to the size of the service manifest (in bytes), and the call will return
SVSM_ERR_INVALID_PARAMETER.”

I took the above to mean that each service will define its own service manifest, and the services manifest in Table 12 is not a requirement. Note that Section 7.1 and Table 12 talk about services manifest, while Section 7.2 talks about service manifest.

Jumping to 8.3.3 SVSM_ATTEST_SINGLE_SERVICE Manifest Data. It says “The manifest data used in an SVSM_ATTEST_SINGLE_SERVICE call for manifest version 0 is
specified according to the following table.” The table is Table 18: vTPM Service Manifest Data Structure on page 36 which basically says that TPMT_PUBLIC structure of the endorsement key says at byte offset zero.

Did I misinterpret the spec?

No, you did not misinterpret the spec.

SVSM_ATTEST_SERVICES will build a specific formatted table for all services with offsets and lengths to each services manifest with each service supplying the manifest format defined for the SVSM_ATTEST_SERVICES call (for vTPM that is version 0 of the manifest).

SVSM_ATTEST_SINGLE_SERVICE allows you to request a specific manifest version (of which vTPM only currently has version 0) and that manifest is supplied directly in the manifest buffer (document bug: the SVSM_ATTEST_SINGLE_SERVICE RCX description should reference Table 13, not Table 11).

My bad, I just revisited the spec and you guys are right. Sorry about that.

cclaudio · 2024-12-14T00:42:24Z

kernel/src/protocols/attestation.rs

+}
+
+#[cfg(all(feature = "vtpm", not(test)))]
+fn attest_single_vtpm(


Most of this function is generic for any potential SVSM service, not just the vTPM service. I think we should move the vTPM code to a separate function.

Makes sense. Will look into it.

IT302 · 2024-12-16T14:50:22Z

Hi @IT302, thank you for the PR. I have made some comments and requested a few changes.

With the ATTEST protocol implemented in the SVSM, we should be able to call it from guest userpsace through the configfs-tsm interface , see [1] for ABI specification and [2] for example. It would be great if we can test this PR through that interface.

[1] - https://github.com/coconut-svsm/linux/blob/svsm/Documentation/ABI/testing/configfs-tsm [2] - coconut-svsm/linux@70e6f7e

We’ve already tested the PR with the Linux configfs-tsm (check out the test script here: https://github.com/hpe-security-lab/svsm-vtpm-test). We’ve also successfully tested it with our Keylime TEE attester and verifier, and we’ll push those changes up once this PR is merged.

joergroedel added the wait-for-review PR needs for approval by reviewers label Dec 5, 2024

joergroedel requested a review from cclaudio December 11, 2024 15:24

stefano-garzarella reviewed Dec 13, 2024

View reviewed changes

cclaudio suggested changes Dec 14, 2024

View reviewed changes

joergroedel added in-review PR is under active review and not yet approved and removed wait-for-review PR needs for approval by reviewers labels Dec 18, 2024

gemini-15 mentioned this pull request Jan 7, 2025

the echo to svsm doesn't work because it doesn't exist hpe-security-lab/svsm-vtpm-test#2

Closed

	let size = self.nonce_size as usize;
	let size = self.nonce_size.into();

svsm: add SVSM VTPM Service Attestation #541

Are you sure you want to change the base?

svsm: add SVSM VTPM Service Attestation #541

Conversation

IT302 commented Nov 28, 2024

IT302 commented Nov 28, 2024

joergroedel commented Dec 11, 2024

cclaudio commented Dec 11, 2024

deeglaze commented Dec 11, 2024

tlendacky commented Dec 11, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

IT302 Dec 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stefano-garzarella commented Dec 13, 2024

cclaudio left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

IT302 Dec 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cclaudio Dec 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tlendacky Dec 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

IT302 Dec 16, 2024 • edited Loading

Choose a reason for hiding this comment

IT302 commented Dec 16, 2024

IT302 Dec 17, 2024 •

edited

Loading

cclaudio left a comment •

edited

Loading

IT302 Dec 18, 2024 •

edited

Loading

cclaudio Dec 16, 2024 •

edited

Loading

tlendacky Dec 16, 2024 •

edited

Loading

IT302 Dec 16, 2024 •

edited

Loading