Add support to request an attestation report #35

cclaudio · 2023-02-13T21:35:47Z

SNP guests can issue SNP_GUEST_REQUEST[1] commands to request services (e.g. an attestation report) to the PSP firmware. However, that requires crypto functions to protect the communication channel.

This PR makes use of openssl to encrypt and decrypt the payload of SNP_GUEST_REQUEST messages. Both the openssl and the libcrt repositories are added as git submodules. We created the libcrt to provide the symbols required by the openssl build. Its files were imported from musl and OVMF on demand, and adapted to build openssl for the SVSM.

Although I am still having one outstanding issue when I run this code (discussed below), I would like to start collecting feedbacks, specially for the items below:

Attestation report implementation for the SVSM
Best way to maintain the libcrt. Can we add the libcrt repository to https://github.com/AMDESE?

[1] Guest Messages. SEV Secure Nested Paging Firmware ABI Specification, Chapter 7.
https://www.amd.com/system/files/TechDocs/56860.pdf

How to test it

The svsm-report-preview-test branch has one additional patch only for calling the function to request an attestation report.

$ git clone -b svsm-report-preview-test https://github.com/svsm-vtpm/linux-svsm.git
$ cd linux-svsm
$ git submodule update --init
$ make FEATURES=verbose
       # Follow the README to create a SNP guest and configure both the guest and the host
$ cd scripts;
$ sudo ./launch-qemu.sh -hda <guest-qcow2-image> -sev-snp -svsm ../svsm.bin

At the beginning of the boot, you should be able to see log messages like these:

INFO: SNP_GUEST_REQUEST message encrypted (96 bytes)
INFO: SNP_GUEST_REQUEST message decrypted (1216 bytes)
INFO: Report, 1184 bytes, vmpl 0
INFO:     report_id: [15, 2, 6e, 5f, e2, f1, e3, 6e, da, 86, 65, 2, 8b, 1f, ba, 63, 58, 22, 20, 82, 47, 55, 40, 4, bd, 5, 55, ee, b, e, 3a, e]
INFO:     report_data: [31, 32, 33, 34, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]

Known issue [FIXED]

Update: 2023/Mar/6. The recent updates to the svsm-preview-hv HEAD e1287b7c03670c6657b7e3f23d0fd364db27c3a9 fixed this issue.

Sometimes, when we run the SVSM guest with multiple vcpus we hit the issue below. The guest boot hangs forever during (or just after) the "Freeing unused kernel image (initmem) memory: 2696K" step.

This issue also happen with the "svsm-report-preview" branch, which compiles-in the attestation report code, but does not call it.
This issue does not happen when we run with only one vcpu (-smp 1).

[    1.950969] Loading compiled-in module X.509 certificates
[    1.951954] Loaded X.509 cert 'Build time autogenerated kernel key: dfa30ed64a2109e39c458ccbdaea49ccdb527725'
[    1.953092] ima: Allocated hash algorithm: sha1
[    1.953948] ima: No architecture policies found
[    1.954841] evm: Initialising EVM extended attributes:
[    1.955769] evm: security.selinux
[    1.956476] evm: security.SMACK64
[    1.957167] evm: security.SMACK64EXEC
[    1.957894] evm: security.SMACK64TRANSMUTE
[    1.958666] evm: security.SMACK64MMAP
[    1.959384] evm: security.apparmor
[    1.960064] evm: security.ima
[    1.960695] evm: security.capability
[    1.961394] evm: HMAC attrs: 0x1
[    1.964877] PM:   Magic number: 11:135:601
[    1.965665] acpi PNP0C0F:0d: hash matches
[    1.966654] RAS: Correctable Errors collector initialized.
[    1.976227] Freeing unused decrypted memory: 2036K
[    1.977456] Freeing unused kernel image (initmem) memory: 2696K
[  243.770548] INFO: task swapper/0:1 blocked for more than 120 seconds.
[  243.777034]       Not tainted 5.17.0-rc6-snp-guest-3f7b2e900f58 #1
[  243.779410] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[  243.782214] task:swapper/0       state:D stack:    0 pid:    1 ppid:     0 flags:0x00004000
[  243.783567] Call Trace:
[  243.784174]  <TASK>
[  243.784741]  __schedule+0x2e6/0xbb0
[  243.785452]  schedule+0x58/0xd0
[  243.786128]  schedule_timeout+0x1f3/0x290
[  243.786898]  ? __smp_call_single_queue+0x38/0x40
[  243.787719]  ? generic_exec_single+0x3e/0xb0
[  243.788495]  wait_for_completion+0x8b/0xf0
[  243.789248]  rcu_barrier+0x15e/0x1f0
[  243.789953]  ? rest_init+0xe0/0xe0
[  243.790656]  kernel_init+0x4b/0x130
[  243.791338]  ret_from_fork+0x22/0x30
[  243.792027]  </TASK>
[  364.602601] INFO: task swapper/0:1 blocked for more than 241 seconds.
[  364.609143]       Not tainted 5.17.0-rc6-snp-guest-3f7b2e900f58 #1
[  364.611448] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[  364.614102] task:swapper/0       state:D stack:    0 pid:    1 ppid:     0 flags:0x00004000
[  364.615334] Call Trace:
[  364.615906]  <TASK>
[  364.616437]  __schedule+0x2e6/0xbb0
[  364.617118]  schedule+0x58/0xd0
[  364.617763]  schedule_timeout+0x1f3/0x290
[  364.618504]  ? __smp_call_single_queue+0x38/0x40
[  364.619296]  ? generic_exec_single+0x3e/0xb0
[  364.620047]  wait_for_completion+0x8b/0xf0
[  364.620780]  rcu_barrier+0x15e/0x1f0
[  364.621469]  ? rest_init+0xe0/0xe0
[  364.622113]  kernel_init+0x4b/0x130
[  364.622774]  ret_from_fork+0x22/0x30
[  364.623435]  </TASK>
<... same call trace message is printed forever ...>

tlendacky · 2023-02-13T21:52:17Z

src/start/start.S

-	.fill	512, 8, 0
-	.fill	512, 8, 0
-	.fill	512, 8, 0
+	.fill	8192, 8, 0


This results in a stack that is 64K in size 8192 * 8 = 65,536. The original stack was 16,384 in size, not 4,096. Can you try with half that and see how it goes?

Also, can you increase the size of the AP stacks, too. An attestation request can come in on any AP.

@tlendacky I increased the AP stacks to 9 pages (8 + 1 guard page) and changed the bsp to .fill 4096, 8, 0, which gives us 8 stack pages also for the bsp. However, I still see the same problem.

Enabling FEATURES=verbose, I see that it crashes just after the SNP_GUEST_REQUEST request is encrypted. Later I can add some more prints to see where exactly it's crashing.

Launching VM ... /tmp/cmdline.3286670 char device redirected to /dev/pts/25 (label compat_monitor0) > All 17 firmware config files read. > Starting SMP for 0 APs: SecCoreStartupWithStack(0xFFFCC000, 0x820000) INFO: SNP_GUEST_REQUEST message encrypted (96 bytes) KVM: entry failed, hardware error 0xffffffff EAX=0000000a EBX=00000000 ECX=00000000 EDX=00000000 ESI=00000000 EDI=00000000 EBP=00000000 ESP=00000000 EIP=00000000 EFL=00000002 [-------] CPL=0 II=0 A20=1 SMM=0 HLT=0 ES =0000 00000000 00000000 00000000 CS =0000 00000000 00000000 00000000 SS =0000 00000000 00000000 00000000 DS =0000 00000000 00000000 00000000 FS =0000 00000000 00000000 00000000 GS =0000 00000000 00000000 00000000 LDT=0000 00000000 00000000 00000000 TR =0000 00000000 00000000 00000000 GDT= 00000000 00000000 IDT= 00000000 00000000 CR0=80000033 CR2=00000000 CR3=00000000 CR4=00000668 DR0=0000000000000000 DR1=0000000000000000 DR2=0000000000000000 DR3=0000000000000000 DR6=00000000ffff0ff0 DR7=0000000000000400 EFER=0000000000000100 Code=<??> ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ??

Yeah, that seems strange... I would have expected the encryption support to be the big user of stack space, but it looks like the encryption succeeds. Maybe it's the decryption of a larger blob that is causing the issue. It would be nice to understand the openssl stack usage.

Set the stack to the number of pages you need and keep the AP stack size in sync (since an attestation request could come from an AP, too).

src/wrapper.rs

tlendacky

RDRAND and RDSEED are guaranteed available on any platform that supports an SVSM

src/cpu/vc.rs

tlendacky

Can the libcrt files be made part of the SVSM project instead of being a submodule? The project does appear to be MIT license, so it would make it easier to integrate other libc functions should they come up for any other items that are written in C.

tlendacky

Forgot to mention, the Co-developed-by: tag should precede the Signed-off-by: tag and requires Vikram's signed off by. So it should look like:

Co-developed-by: Vikram ...
Signed-off-by: Vikram ...
Signed-off-by: Claudio ...

src/bios.rs

tlendacky · 2023-02-13T22:56:59Z

src/cpu/vc.rs

@@ -116,6 +116,8 @@ const GHCB_NAE_CPUID: u64 = 0x72;
 const GHCB_NAE_IOIO: u64 = 0x7b;
 /// 0x80000010
 const GHCB_NAE_PSC: u64 = 0x80000010;
+/// 0x80000011
+const GHCB_NAE_SNP_GUEST_REQUEST: u64 = 0x80000011;


This will also need to support an extended guest request. That can be done as part of this or as a separate patch.

tlendacky · 2023-02-13T22:58:18Z

src/cpu/vc.rs

+        if !(*ghcb).is_sw_exit_info_2_valid() {
+            vc_terminate_svsm_resp_invalid();
+        }
+        let rc = LOWER_32BITS!((*ghcb).sw_exit_info_2()) as u32;


You will also need to check the upper 32-bits. The guest request can return an error code indicating busy, at which point you should return busy back to the caller (busy is a new SVSM return code in the draft spec). You should make this function return a u64 and just return the full value.

src/cpu/vc.rs

src/psp/guest_request.rs

tlendacky · 2023-02-14T14:11:30Z

src/psp/report.rs

+ * Authors:
+ *   Claudio Carvalho <[email protected]>
+ */
+


This should only be done when requested. So it should be added to the svsm_request.rs file based on the attestation protocol value.

@tlendacky : I started working on gluing the code here to the SVSM_ATTEST_SINGLE_SERVICE command etc. This will be extra commits on top of Claudio's work. Would you like to see all of it in the same PR? This is getting a bit big.

We should probably break the pull request into two pull requests, one that creates the libcrt/openssl integration and one that supports the attestation protocol.

With multiple protocols, svsm_request.rs should probably be split into multiple files: svsm_request.rs and protocols/core.rs. Then the attestation protocol can be added as protocol/attestation.rs, etc, with everything called from svsm_request.rs.

With multiple protocols, svsm_request.rs should probably be split into multiple files: svsm_request.rs and protocols/core.rs.

I tried this extraction in #38 .

Zildj1an

Thanks for your PR :) We need to figure out some organizational bits. We also need an extra commit adding a few words about this on the README.md. Could you please add a subsection 4.1 "Producing an attestation report" where you explain we use openssl and that get_report() can be used for testing?

Zildj1an · 2023-02-14T14:45:52Z

Cargo.toml

@@ -19,6 +19,10 @@ memoffset = "0.6"
 paste = "1.0"
 memchr = { version = "2", default-features = false }
 uuid = { version = "1", default-features = false }
+cty = "0.2.2"


You can run ./scripts/crates.sh to see the licenses of the newly added crates. Until now we would only check if they are MIT as part of CI, but I see this PR adds crates with other licenses (e.g., ISC, BSD 3). We need to update the script to account for these cases so the GitHub check does not fail. Only crates with permissive licenses should be allowed

Makefile

build.rs

external.h

src/bindings.rs

src/psp/report.rs

src/wrapper.rs

.cargo/config.toml

src/wrapper.rs

cclaudio · 2023-02-15T20:18:35Z

Can the libcrt files be made part of the SVSM project instead of being a submodule? The project does appear to be MIT license, so it would make it easier to integrate other libc functions should they come up for any other items that are written in C.

@tlendacky Good idea, I will do that. Additionally, I think I can do some extra clean-up to the libcrt because some of it considers that we are building for the UEFI target, rather than the SVSM target which does not inherit from UEFI.

After updating to rustc 1.71, the SVSM build started to fail: $ rm .prereq $ make prereq $ make ... cryptlib.c:(.text.unlikely+0x8): undefined reference to `abort' assert.c:(.text+0x22): undefined reference to `abort' ... The abort function is implemented in Rust and exported for external libraries. The option "-Wl,-u,abort" prevents it from being removed from the svsm.elf. Signed-off-by: Claudio Carvalho <[email protected]>

Add bindgen to automatically produce Rust FFI (Foreign Functions Interface) bindings out of C headers to be used in other crates. Cargo picks up on the existence of build.rs, then compile and execute it before the rest of the create is built. The resulting bindings are written to src/bindings.rs. Rename the target x86_64-unknown-none as bindgen uses it to properly generate bindings. Co-developed-by: Vikram Narayanan <[email protected]> Signed-off-by: Vikram Narayanan <[email protected]> Signed-off-by: Claudio Carvalho <[email protected]>

With the attestation report support, the SnpSecrets is now accessed from multiple places. Signed-off-by: Claudio Carvalho <[email protected]>

When we request an attestation report to the PSP, the guest crashes with the following error due to stack overflow. Launching VM ... /tmp/cmdline.866937 char device redirected to /dev/pts/183 (label compat_monitor0) KVM: entry failed, hardware error 0xffffffff EAX=00000000 EBX=00000000 ECX=00000000 EDX=00000000 ESI=00000000 EDI=00000000 EBP=00000000 ESP=00000000 EIP=00000000 EFL=00000002 [-------] CPL=0 II=0 A20=1 SMM=0 HLT=0 ES =0000 00000000 00000000 00000000 CS =0000 00000000 00000000 00000000 SS =0000 00000000 00000000 00000000 DS =0000 00000000 00000000 00000000 FS =0000 00000000 00000000 00000000 GS =0000 00000000 00000000 00000000 LDT=0000 00000000 00000000 00000000 TR =0000 00000000 00000000 00000000 GDT= 0000000000000000 00000000 IDT= 0000000000000000 00000000 CR0=80010033 CR2=0000000000000000 CR3=0000000000000000 CR4=00000668 DR0=0000000000000000 DR1=0000000000000000 DR2=0000000000000000 DR3=0000000000000000 DR6=00000000ffff0ff0 DR7=0000000000000400 EFER=0000000000001d00 Code=<??> ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? Currently, the SVSM stack size is 4 pages. The same issue happens if we double the stack size. This patch increases the BSP and AP stack size to 16 pages. Signed-off-by: Claudio Carvalho <[email protected]>

The PSP expects each SNP Guest Request command received to be wrapped by the SnpGuestRequestMsg structure. The command itself goes in the payload of the message and has its own structure definition. When the PSP receives a SnpGuestRequestMsg, it processes the command found in the payload and sends the result back also wrapped by the SnpGuestRequestMsg. The command result has its own structure definition as well. For example, for an attestation report request we wrap the SnpReportRequest structure with the SnpGuestRequestMsg. The PSP will unwrap the message, process the command and send back the attestatio report result in the SnpReportResponse structure, wrapped by the SnpGuestRequestMsg. Other commands types can be wrapped using the SnpGuestRequestMsg. Signed-off-by: Claudio Carvalho <[email protected]>

The function vc_snp_guest_request sends SNP Guest Request messages following the GHCB protocol defined in the Guest Hypervisor Communication Block (GHCB) specification. It supports sending either a extended or non-extended SNP_GUEST_REQUEST message; each one has its own VMGEXIT exit code. Signed-off-by: Claudio Carvalho <[email protected]>

To send a SNP Guest Request command to the PSP, we need to provide the request and response physical addresses to the PSP; depending on the command we also need to provide the physical address of a data buffer. The size of these buffers are 1, 1 and 4 pages, and they need to be shared with the hypervisor. The attestation report is on the critical path to attest the confidential VM. This patch pre-allocates the required buffers at boot time to prevent us from having to handle out-of-memory issues later at boot time or runtime. Access to the buffers are controled through a spinlock. Signed-off-by: Claudio Carvalho <[email protected]>

Calls to openssl usually requires a context. For example, before calling openssl to encrypt data, first we need to create a new context and set it up for the encryption algorithm we want to use. Once we are done with the encryption session, we have to free the context resources to avoid memory leaking. In order to simplify error handling, this patch free the context resources at the beginning of the session rather than at the tail. Signed-off-by: Claudio Carvalho <[email protected]>

As described in the SNP Firmware ABI specification, SNP_GUEST_REQUEST commands are protected using AES_GCM 256 and a VMPCK key to encrypt/decrypt the message. The firmware generates and provides the VMPCK keys to the guest in a especial secrets page that only the guest and firmware have access to, so the hypervisor cannot alter messages without detection nor read the plaintext of the message. We are limiting the send_request() implementation to encrypt/decrypt messages using the VMPCK0 key only. From guest userspace we can send SNP_GUEST_REQUEST commands using other VMPCK keys. The send_request() also supports sending extended SNP_GUEST_REQUEST commands, where the extended part of it is provided by the hypervisor. For example, in an extended attestation report the PSP firmware provides the attestation report and the hypervisor provides the certificate chain needed to verify the attestation report. If a SNP_GUEST_REQUEST fails, we disable the VMPCK0 to prevent it from being used again to send SNP_GUEST_REQUEST commands to the PSP firmware. The only two exceptions are when the hypervisor returns the error codes below: - BUSY: in this case we re-send the message and if it resturns any error code, we disable the VMPCK0. - INVALID_DATA_BUFFER_LEN: the data buffer provided in the extended SNP_GUEST_REQUEST command is too small. So, we re-send the command, but as non-extended SNP_GUEST_REQUEST in order to protect the IV (sequence number) from replay attacks. If it returns any error code, we disable the VMPCK0. The two cases above could be improved later when, for example, timer support is added to the SVSM. Signed-off-by: Claudio Carvalho <[email protected]>

Attestation report is one of the SNP_GUEST_REQUEST commands that can be send to the PSP firmware. The SnpGuestRequest is used to assemble the request and PSP firmware will send back the response in the SnpGuestResponse structure, which contains the signed attestation report. Signed-off-by: Claudio Carvalho <[email protected]>

The get_report() can be used to request an attestation report to the PSP firmware. If the last parameter CertBuf is provided, the certificate chain required to verify the attestation report is saved in the CertBuf.addr provided. If get_report() fails an Err(error_code) is returned. The wrapped error_code is compliant with the Core Protocol error codes defined in the SVSM spec 0.62 (draft). The psp_rc reference provided can be used to further understand the error. Example: let buf: x86_64::addr::VirtAddr = mem::mem_allocate(0x4000).unwrap(); let mut certs: CertsBuf = CertsBuf::new(buf, 0x4000usize); let mut psp_rc: u64 = 0; let mut data: [u8; USER_DATA_SIZE] = [0u8; USER_DATA_SIZE]; data[0] = 0x31; data[1] = 0x32; data[2] = 0x33; data[4] = 0x34; // Test extended attestation report request let result: Result<psp::msg_report::SnpReportResponse, u64> = psp::request::get_report(&data, &mut psp_rc, Some(&mut certs)); if let Ok(resp) = result { prints!("INFO: Report, {} bytes, vmpl {}\n", { resp.get_report_size() }, { resp.get_report().get_vmpl() } ); prints!("INFO: report_id: {:x?}\n", { resp.get_report().get_report_id() }); prints!("INFO: report_data: {:x?}\n", { resp.get_report().get_report_data() }); let sample: *const [u8; 500] = buf.as_ptr() as *const [u8; 500]; prints!("INFO: certs sample {:x?}\n", { unsafe { *sample } }); } Signed-off-by: Claudio Carvalho <[email protected]>

tlendacky reviewed Feb 13, 2023

View reviewed changes

src/wrapper.rs Outdated Show resolved Hide resolved

src/wrapper.rs Outdated Show resolved Hide resolved

src/wrapper.rs Outdated Show resolved Hide resolved

tlendacky reviewed Feb 13, 2023

View reviewed changes

src/cpu/vc.rs Outdated Show resolved Hide resolved

tlendacky reviewed Feb 13, 2023

View reviewed changes

tlendacky reviewed Feb 14, 2023

View reviewed changes

Zildj1an added the enhancement New feature or request label Feb 14, 2023

tlendacky reviewed Feb 14, 2023

View reviewed changes

Zildj1an reviewed Feb 14, 2023

View reviewed changes

cclaudio mentioned this pull request Feb 16, 2023

Use the x86_64-unknown-none built-in target to make distro packaging easier #37

Open

dubek mentioned this pull request Feb 19, 2023

Extract core protocol to protocols/core.rs #38

Merged

cclaudio mentioned this pull request Mar 6, 2023

Add openssl #41

Merged

dubek mentioned this pull request Apr 8, 2023

wrapper: elide libc functions from tests #46

Closed

cclaudio added 4 commits May 9, 2023 15:39

mem/snpsecrets: Move SnpSecrets to its own file

e574daf

With the attestation report support, the SnpSecrets is now accessed from multiple places. Signed-off-by: Claudio Carvalho <[email protected]>

cclaudio mentioned this pull request May 11, 2023

Attestation report support with openssl #55

Open

cclaudio added 7 commits June 20, 2023 10:38

cclaudio force-pushed the svsm-report-preview branch from c895c9a to 438a008 Compare June 22, 2023 16:01

cclaudio closed this Jun 22, 2023

cclaudio deleted the svsm-report-preview branch June 22, 2023 16:03

cclaudio restored the svsm-report-preview branch June 27, 2023 14:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support to request an attestation report #35

Add support to request an attestation report #35

cclaudio commented Feb 13, 2023 •

edited

Loading

tlendacky Feb 13, 2023

cclaudio Feb 21, 2023

tlendacky Feb 21, 2023

tlendacky left a comment

tlendacky left a comment

tlendacky left a comment

tlendacky Feb 13, 2023

tlendacky Feb 13, 2023

tlendacky Feb 14, 2023

dubek Feb 14, 2023

tlendacky Feb 14, 2023

dubek Feb 20, 2023

Zildj1an left a comment

Zildj1an Feb 14, 2023

cclaudio commented Feb 15, 2023

Add support to request an attestation report #35

Add support to request an attestation report #35

Conversation

cclaudio commented Feb 13, 2023 • edited Loading

How to test it

Known issue [FIXED]

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tlendacky left a comment

Choose a reason for hiding this comment

tlendacky left a comment

Choose a reason for hiding this comment

tlendacky left a 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

Zildj1an left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cclaudio commented Feb 15, 2023

cclaudio commented Feb 13, 2023 •

edited

Loading