[PATCH v6 0/5] Allow guest access to EFI confidential computing secret area

[PATCH v6 0/5] Allow guest access to EFI confidential computing secret area

[Dov Murik]

Confidential computing (coco) hardware such as AMD SEV (Secure Encrypted
Virtualization) allows guest owners to inject secrets into the VMs
memory without the host/hypervisor being able to read them.  In SEV,
secret injection is performed early in the VM launch process, before the
guest starts running.

OVMF already reserves designated area for secret injection (in its
AmdSev package; see edk2 commit 01726b6d23d4 "OvmfPkg/AmdSev: Expose the
Sev Secret area using a configuration table" [1]), but the secrets were
not available in the guest kernel.

The patch series keeps the address of the EFI-provided memory for
injected secrets, and exposes the secrets to userspace via securityfs
using a new efi_secret kernel module.  The module is autoloaded (by the
EFI driver) if the secret area is populated.

The first patch in EFI keeps the address of the secret area as passed in
the EFI configuration table.  The second patch is a quirk fix for older
firmwares didn't mark the secrets page as EFI_RESERVED_TYPE.  The third
patch introduces the new efi_secret module that exposes the content of
the secret entries as securityfs files, and allows clearing out secrets
with a file unlink interface.  The fourth patch auto-loads the
efi_secret module during startup if the injected secrets area is
populated.  The last patch documents the data flow of confidential
computing secret injection.

As a usage example, consider a guest performing computations on
encrypted files.  The Guest Owner provides the decryption key (= secret)
using the secret injection mechanism.  The guest application reads the
secret from the efi_secret filesystem and proceeds to decrypt the files
into memory and then performs the needed computations on the content.

In this example, the host can't read the files from the disk image
because they are encrypted.  Host can't read the decryption key because
it is passed using the secret injection mechanism (= secure channel).
Host can't read the decrypted content from memory because it's a
confidential (memory-encrypted) guest.

This has been tested with AMD SEV and SEV-ES guests, but the kernel side
of handling the secret area has no SEV-specific dependencies, and
therefore might be usable (perhaps with minor changes) for any
confidential computing hardware that can publish the secret area via the
standard EFI config table entry.

To enable this functionality, set CONFIG_EFI_SECRET=m when building the
guest kernel.

Here is a simple example for usage of the efi_secret module in a guest
to which an EFI secret area with 4 secrets was injected during launch:

# ls -la /sys/kernel/security/coco/efi_secret
total 0
drwxr-xr-x 2 root root 0 Jun 28 11:54 .
drwxr-xr-x 3 root root 0 Jun 28 11:54 ..
-r--r----- 1 root root 0 Jun 28 11:54 736870e5-84f0-4973-92ec-06879ce3da0b
-r--r----- 1 root root 0 Jun 28 11:54 83c83f7f-1356-4975-8b7e-d3a0b54312c6
-r--r----- 1 root root 0 Jun 28 11:54 9553f55d-3da2-43ee-ab5d-ff17f78864d2
-r--r----- 1 root root 0 Jun 28 11:54 e6f5a162-d67f-4750-a67c-5d065f2a9910

# xxd /sys/kernel/security/coco/efi_secret/e6f5a162-d67f-4750-a67c-5d065f2a9910
00000000: 7468 6573 652d 6172 652d 7468 652d 6b61  these-are-the-ka
00000010: 7461 2d73 6563 7265 7473 0001 0203 0405  ta-secrets......
00000020: 0607                                     ..

# rm /sys/kernel/security/coco/efi_secret/e6f5a162-d67f-4750-a67c-5d065f2a9910

# ls -la /sys/kernel/security/coco/efi_secret
total 0
drwxr-xr-x 2 root root 0 Jun 28 11:55 .
drwxr-xr-x 3 root root 0 Jun 28 11:54 ..
-r--r----- 1 root root 0 Jun 28 11:54 736870e5-84f0-4973-92ec-06879ce3da0b
-r--r----- 1 root root 0 Jun 28 11:54 83c83f7f-1356-4975-8b7e-d3a0b54312c6
-r--r----- 1 root root 0 Jun 28 11:54 9553f55d-3da2-43ee-ab5d-ff17f78864d2


[1] https://github.com/tianocore/edk2/commit/01726b6d23d4


---

v6 changes:
 - Autoload the efi_secret module if the secret area is populated
   (thanks Greg KH).
 - efi_secret: Depend on X86_64 because we use ioremap_encrypted() which
   is only defined for this arch.
 - efi_secret.c: Remove unneeded tableheader_guid local variable.
 - Documentation fixes.

v5: https://lore.kernel.org/linux-coco/20211118113359.642571-1-dovmurik@linux.ibm.com/
v5 changes:
 - Simplify EFI code: instead of copying the secret area, the firmware
   marks the secret area as EFI_RESERVED_TYPE, and then the uefi_init()
   code just keeps the pointer as it appears in the EFI configuration
   table.  The use of reserved pages is similar to the AMD SEV-SNP
   patches for handling SNP-Secrets and SNP-CPUID pages.
 - In order to handle OVMF releases out there which mark the
   confidential computing secrets page as EFI_BOOT_SERVICES_DATA, add
   efi/libstub code that detects this and fixes the E820 map to reserve
   this page.
 - In the efi_secret module code, map the secrets page using
   ioremap_encrypted (again, similar to the AMD SEV-SNP guest patches
   for accessing SNP-Secrets and SNP-CPUID pages).
 - Add documentation in Documentation/security/coco/efi_secret.

v4: https://lore.kernel.org/linux-coco/20211020061408.3447533-1-dovmurik@linux.ibm.com/
v4 changes:
 - Guard all the new EFI and efi-stub code (patches 1+2) with #ifdef
   CONFIG_EFI_COCO_SECRET (thanks Greg KH).  Selecting
   CONFIG_EFI_SECRET=m (patch 3) will enable the EFI parts as well.
 - Guard call to clflush_cache_range() with #ifdef CONFIG_X86
   (Reported-by: kernel test robot <lkp@intel.com>)

v3: https://lore.kernel.org/linux-coco/20211014130848.592611-1-dovmurik@linux.ibm.com/
v3 changes:
 - Rename the module to efi_secret
 - Remove the exporting of clean_cache_range
 - Use clflush_cache_range in wipe_memory
 - Document function wipe_memory
 - Initialize efi.coco_secret to EFI_INVALID_TABLE_ADDR to correctly detect
   when there's no secret area published in the EFI configuration tables

v2: https://lore.kernel.org/linux-coco/20211007061838.1381129-1-dovmurik@linux.ibm.com
v2 changes:
 - Export clean_cache_range()
 - When deleteing a secret, call clean_cache_range() after explicit_memzero
 - Add Documentation/ABI/testing/securityfs-coco-sev_secret

v1: https://lore.kernel.org/linux-coco/20210809190157.279332-1-dovmurik@linux.ibm.com/

RFC: https://lore.kernel.org/linux-coco/20210628183431.953934-1-dovmurik@linux.ibm.com/


Dov Murik (5):
  efi: Save location of EFI confidential computing area
  efi/libstub: Reserve confidential computing secret area
  virt: Add efi_secret module to expose confidential computing secrets
  efi: Load efi_secret module if EFI secret area is populated
  docs: security: Add coco/efi_secret documentation

 .../ABI/testing/securityfs-coco-efi_secret    |  51 +++
 Documentation/security/coco/efi_secret.rst    | 102 ++++++
 Documentation/security/coco/index.rst         |   9 +
 Documentation/security/index.rst              |   1 +
 arch/x86/platform/efi/efi.c                   |   3 +
 drivers/firmware/efi/Kconfig                  |  16 +
 drivers/firmware/efi/Makefile                 |   1 +
 drivers/firmware/efi/coco.c                   |  58 +++
 drivers/firmware/efi/efi.c                    |   6 +
 drivers/firmware/efi/libstub/x86-stub.c       |  28 ++
 drivers/virt/Kconfig                          |   3 +
 drivers/virt/Makefile                         |   1 +
 drivers/virt/coco/efi_secret/Kconfig          |  14 +
 drivers/virt/coco/efi_secret/Makefile         |   2 +
 drivers/virt/coco/efi_secret/efi_secret.c     | 337 ++++++++++++++++++
 include/linux/efi.h                           |  10 +
 16 files changed, 642 insertions(+)
 create mode 100644 Documentation/ABI/testing/securityfs-coco-efi_secret
 create mode 100644 Documentation/security/coco/efi_secret.rst
 create mode 100644 Documentation/security/coco/index.rst
 create mode 100644 drivers/firmware/efi/coco.c
 create mode 100644 drivers/virt/coco/efi_secret/Kconfig
 create mode 100644 drivers/virt/coco/efi_secret/Makefile
 create mode 100644 drivers/virt/coco/efi_secret/efi_secret.c


base-commit: 42eb8fdac2fc5d62392dcfcf0253753e821a97b0
-- 
2.25.1

[PATCH v6 1/5] efi: Save location of EFI confidential computing area

[Dov Murik]

Confidential computing (coco) hardware such as AMD SEV (Secure Encrypted
Virtualization) allows a guest owner to inject secrets into the VMs
memory without the host/hypervisor being able to read them.

Firmware support for secret injection is available in OVMF, which
reserves a memory area for secret injection and includes a pointer to it
the in EFI config table entry LINUX_EFI_COCO_SECRET_TABLE_GUID.

If EFI exposes such a table entry, uefi_init() will keep a pointer to
the EFI config table entry in efi.coco_secret, so it can be used later
by the kernel (specifically drivers/virt/coco/efi_secret).  It will also
appear in the kernel log as "CocoSecret=ADDRESS"; for example:

    [    0.000000] efi: EFI v2.70 by EDK II
    [    0.000000] efi: CocoSecret=0x7f22e680 SMBIOS=0x7f541000 ACPI=0x7f77e000 ACPI 2.0=0x7f77e014 MEMATTR=0x7ea0c018

The new functionality can be enabled with CONFIG_EFI_COCO_SECRET=y.

Signed-off-by: Dov Murik <dovmurik@linux.ibm.com>
---
 arch/x86/platform/efi/efi.c  |  3 +++
 drivers/firmware/efi/Kconfig | 16 ++++++++++++++++
 drivers/firmware/efi/efi.c   |  6 ++++++
 include/linux/efi.h          | 10 ++++++++++
 4 files changed, 35 insertions(+)

diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index 147c30a81f15..1591d67e0bcd 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -93,6 +93,9 @@ static const unsigned long * const efi_tables[] = {
 #ifdef CONFIG_LOAD_UEFI_KEYS
 	&efi.mokvar_table,
 #endif
+#ifdef CONFIG_EFI_COCO_SECRET
+	&efi.coco_secret,
+#endif
 };
 
 u64 efi_setup;		/* efi setup_data physical address */
diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig
index 2c3dac5ecb36..6fa251b3709f 100644
--- a/drivers/firmware/efi/Kconfig
+++ b/drivers/firmware/efi/Kconfig
@@ -284,3 +284,19 @@ config EFI_CUSTOM_SSDT_OVERLAYS
 
 	  See Documentation/admin-guide/acpi/ssdt-overlays.rst for more
 	  information.
+
+config EFI_COCO_SECRET
+	bool "EFI Confidential Computing Secret Area Support"
+	depends on EFI
+	help
+	  Confidential Computing platforms (such as AMD SEV) allow the
+	  Guest Owner to securely inject secrets during guest VM launch.
+	  The secrets are placed in a designated EFI reserved memory area.
+
+	  In order to use the secrets in the kernel, the location of the secret
+	  area (as published in the EFI config table) must be kept.
+
+	  If you say Y here, the address of the EFI secret area will be kept
+	  for usage inside the kernel.  This will allow the
+	  virt/coco/efi_secret module to access the secrets, which in turn
+	  allows userspace programs to access the injected secrets.
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index ae79c3300129..3cc3a7449c64 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -46,6 +46,9 @@ struct efi __read_mostly efi = {
 #ifdef CONFIG_LOAD_UEFI_KEYS
 	.mokvar_table		= EFI_INVALID_TABLE_ADDR,
 #endif
+#ifdef CONFIG_EFI_COCO_SECRET
+	.coco_secret		= EFI_INVALID_TABLE_ADDR,
+#endif
 };
 EXPORT_SYMBOL(efi);
 
@@ -528,6 +531,9 @@ static const efi_config_table_type_t common_tables[] __initconst = {
 #endif
 #ifdef CONFIG_LOAD_UEFI_KEYS
 	{LINUX_EFI_MOK_VARIABLE_TABLE_GUID,	&efi.mokvar_table,	"MOKvar"	},
+#endif
+#ifdef CONFIG_EFI_COCO_SECRET
+	{LINUX_EFI_COCO_SECRET_AREA_GUID,	&efi.coco_secret,	"CocoSecret"	},
 #endif
 	{},
 };
diff --git a/include/linux/efi.h b/include/linux/efi.h
index dbd39b20e034..f68aa768a4e2 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -359,6 +359,7 @@ void efi_native_runtime_setup(void);
 #define LINUX_EFI_MEMRESERVE_TABLE_GUID		EFI_GUID(0x888eb0c6, 0x8ede, 0x4ff5,  0xa8, 0xf0, 0x9a, 0xee, 0x5c, 0xb9, 0x77, 0xc2)
 #define LINUX_EFI_INITRD_MEDIA_GUID		EFI_GUID(0x5568e427, 0x68fc, 0x4f3d,  0xac, 0x74, 0xca, 0x55, 0x52, 0x31, 0xcc, 0x68)
 #define LINUX_EFI_MOK_VARIABLE_TABLE_GUID	EFI_GUID(0xc451ed2b, 0x9694, 0x45d3,  0xba, 0xba, 0xed, 0x9f, 0x89, 0x88, 0xa3, 0x89)
+#define LINUX_EFI_COCO_SECRET_AREA_GUID		EFI_GUID(0xadf956ad, 0xe98c, 0x484c,  0xae, 0x11, 0xb5, 0x1c, 0x7d, 0x33, 0x64, 0x47)
 
 /* OEM GUIDs */
 #define DELLEMC_EFI_RCI2_TABLE_GUID		EFI_GUID(0x2d9f28a2, 0xa886, 0x456a,  0x97, 0xa8, 0xf1, 0x1e, 0xf2, 0x4f, 0xf4, 0x55)
@@ -550,6 +551,7 @@ extern struct efi {
 	unsigned long			tpm_log;		/* TPM2 Event Log table */
 	unsigned long			tpm_final_log;		/* TPM2 Final Events Log table */
 	unsigned long			mokvar_table;		/* MOK variable config table */
+	unsigned long			coco_secret;		/* Confidential computing secret table */
 
 	efi_get_time_t			*get_time;
 	efi_set_time_t			*set_time;
@@ -1283,4 +1285,12 @@ static inline struct efi_mokvar_table_entry *efi_mokvar_entry_find(
 }
 #endif
 
+struct linux_efi_coco_secret_area {
+	u64	base_pa;
+	u64	size;
+};
+
+/* Header of a populated EFI secret area */
+#define EFI_SECRET_TABLE_HEADER_GUID	EFI_GUID(0x1e74f542, 0x71dd, 0x4d66,  0x96, 0x3e, 0xef, 0x42, 0x87, 0xff, 0x17, 0x3b)
+
 #endif /* _LINUX_EFI_H */
-- 
2.25.1

[PATCH v6 2/5] efi/libstub: Reserve confidential computing secret area

[Dov Murik]

Some older firmware declare the confidential computing secret area as
EFI_BOOT_SERVICES_DATA region.  Fix this up by treating this memory
region as EFI_RESERVED_TYPE, as it should be.

If that memory region is already EFI_RESERVED_TYPE then this has no
effect on the E820 map.

Signed-off-by: Dov Murik <dovmurik@linux.ibm.com>
---
 drivers/firmware/efi/libstub/x86-stub.c | 28 +++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
index f14c4ff5839f..dabfa33ae2b3 100644
--- a/drivers/firmware/efi/libstub/x86-stub.c
+++ b/drivers/firmware/efi/libstub/x86-stub.c
@@ -24,6 +24,7 @@
 const efi_system_table_t *efi_system_table;
 extern u32 image_offset;
 static efi_loaded_image_t *image = NULL;
+static u64 efi_coco_secret_phys_addr = U64_MAX;
 
 static efi_status_t
 preserve_pci_rom_image(efi_pci_io_protocol_t *pci, struct pci_setup_rom **__rom)
@@ -443,6 +444,21 @@ static void add_e820ext(struct boot_params *params,
 		params->hdr.setup_data = (unsigned long)e820ext;
 }
 
+#ifdef CONFIG_EFI_COCO_SECRET
+static void efi_set_coco_secret_phys_addr(void)
+{
+	struct linux_efi_coco_secret_area *secret_area =
+		get_efi_config_table(LINUX_EFI_COCO_SECRET_AREA_GUID);
+
+	if (!secret_area || secret_area->size == 0 || secret_area->size >= SZ_4G)
+		return;
+
+	efi_coco_secret_phys_addr = secret_area->base_pa;
+}
+#else
+static void efi_set_coco_secret_phys_addr(void) {}
+#endif
+
 static efi_status_t
 setup_e820(struct boot_params *params, struct setup_data *e820ext, u32 e820ext_size)
 {
@@ -494,6 +510,16 @@ setup_e820(struct boot_params *params, struct setup_data *e820ext, u32 e820ext_s
 				e820_type = E820_TYPE_SOFT_RESERVED;
 			else
 				e820_type = E820_TYPE_RAM;
+#ifdef CONFIG_EFI_COCO_SECRET
+			if (d->phys_addr == efi_coco_secret_phys_addr)
+				/*
+				 * Fix a quirk in firmwares which don't mark
+				 * the EFI confidential computing area as
+				 * EFI_RESERVED_TYPE, but instead as
+				 * EFI_BOOT_SERVICES_DATA.
+				 */
+				e820_type = E820_TYPE_RESERVED;
+#endif
 			break;
 
 		case EFI_ACPI_MEMORY_NVS:
@@ -793,6 +819,8 @@ unsigned long efi_main(efi_handle_t handle,
 
 	efi_retrieve_tpm2_eventlog();
 
+	efi_set_coco_secret_phys_addr();
+
 	setup_graphics(boot_params);
 
 	setup_efi_pci(boot_params);
-- 
2.25.1

[PATCH v6 3/5] virt: Add efi_secret module to expose confidential computing secrets

[Dov Murik]

The new efi_secret module exposes the confidential computing (coco)
EFI secret area via securityfs interface.

When the module is loaded (and securityfs is mounted, typically under
/sys/kernel/security), a "coco/efi_secret" directory is created in
securityfs.  In it, a file is created for each secret entry.  The name
of each such file is the GUID of the secret entry, and its content is
the secret data.

This allows applications running in a confidential computing setting to
read secrets provided by the guest owner via a secure secret injection
mechanism (such as AMD SEV's LAUNCH_SECRET command).

Removing (unlinking) files in the "coco/efi_secret" directory will zero
out the secret in memory, and remove the filesystem entry.  If the
module is removed and loaded again, that secret will not appear in the
filesystem.

Signed-off-by: Dov Murik <dovmurik@linux.ibm.com>
---
 .../ABI/testing/securityfs-coco-efi_secret    |  51 +++
 drivers/virt/Kconfig                          |   3 +
 drivers/virt/Makefile                         |   1 +
 drivers/virt/coco/efi_secret/Kconfig          |  11 +
 drivers/virt/coco/efi_secret/Makefile         |   2 +
 drivers/virt/coco/efi_secret/efi_secret.c     | 337 ++++++++++++++++++
 6 files changed, 405 insertions(+)
 create mode 100644 Documentation/ABI/testing/securityfs-coco-efi_secret
 create mode 100644 drivers/virt/coco/efi_secret/Kconfig
 create mode 100644 drivers/virt/coco/efi_secret/Makefile
 create mode 100644 drivers/virt/coco/efi_secret/efi_secret.c

diff --git a/Documentation/ABI/testing/securityfs-coco-efi_secret b/Documentation/ABI/testing/securityfs-coco-efi_secret
new file mode 100644
index 000000000000..770abac2c4bb
--- /dev/null
+++ b/Documentation/ABI/testing/securityfs-coco-efi_secret
@@ -0,0 +1,51 @@
+What:		security/coco/efi_secret
+Date:		October 2021
+Contact:	Dov Murik <dovmurik@linux.ibm.com>
+Description:
+		Exposes confidential computing (coco) EFI secrets to
+		userspace via securityfs.
+
+		EFI can declare memory area used by confidential computing
+		platforms (such as AMD SEV and SEV-ES) for secret injection by
+		the Guest Owner during VM's launch.  The secrets are encrypted
+		by the Guest Owner and decrypted inside the trusted enclave,
+		and therefore are not readable by the untrusted host.
+
+		The efi_secret module exposes the secrets to userspace.  Each
+		secret appears as a file under <securityfs>/coco/efi_secret,
+		where the filename is the GUID of the entry in the secrets
+		table.  This module is loaded automatically by the EFI driver
+		if the EFI secret area is populated.
+
+		Two operations are supported for the files: read and unlink.
+		Reading the file returns the content of secret entry.
+		Unlinking the file overwrites the secret data with zeroes and
+		removes the entry from the filesystem.  A secret cannot be read
+		after it has been unlinked.
+
+		For example, listing the available secrets::
+
+		  # modprobe efi_secret
+		  # ls -l /sys/kernel/security/coco/efi_secret
+		  -r--r----- 1 root root 0 Jun 28 11:54 736870e5-84f0-4973-92ec-06879ce3da0b
+		  -r--r----- 1 root root 0 Jun 28 11:54 83c83f7f-1356-4975-8b7e-d3a0b54312c6
+		  -r--r----- 1 root root 0 Jun 28 11:54 9553f55d-3da2-43ee-ab5d-ff17f78864d2
+		  -r--r----- 1 root root 0 Jun 28 11:54 e6f5a162-d67f-4750-a67c-5d065f2a9910
+
+		Reading the secret data by reading a file::
+
+		  # cat /sys/kernel/security/coco/efi_secret/e6f5a162-d67f-4750-a67c-5d065f2a9910
+		  the-content-of-the-secret-data
+
+		Wiping a secret by unlinking a file::
+
+		  # rm /sys/kernel/security/coco/efi_secret/e6f5a162-d67f-4750-a67c-5d065f2a9910
+		  # ls -l /sys/kernel/security/coco/efi_secret
+		  -r--r----- 1 root root 0 Jun 28 11:54 736870e5-84f0-4973-92ec-06879ce3da0b
+		  -r--r----- 1 root root 0 Jun 28 11:54 83c83f7f-1356-4975-8b7e-d3a0b54312c6
+		  -r--r----- 1 root root 0 Jun 28 11:54 9553f55d-3da2-43ee-ab5d-ff17f78864d2
+
+		Note: The binary format of the secrets table injected by the
+		Guest Owner is described in
+		drivers/virt/coco/efi_secret/efi_secret.c under "Structure of
+		the EFI secret area".
diff --git a/drivers/virt/Kconfig b/drivers/virt/Kconfig
index 8061e8ef449f..fe7a6579b974 100644
--- a/drivers/virt/Kconfig
+++ b/drivers/virt/Kconfig
@@ -36,4 +36,7 @@ source "drivers/virt/vboxguest/Kconfig"
 source "drivers/virt/nitro_enclaves/Kconfig"
 
 source "drivers/virt/acrn/Kconfig"
+
+source "drivers/virt/coco/efi_secret/Kconfig"
+
 endif
diff --git a/drivers/virt/Makefile b/drivers/virt/Makefile
index 3e272ea60cd9..efdb015783f9 100644
--- a/drivers/virt/Makefile
+++ b/drivers/virt/Makefile
@@ -8,3 +8,4 @@ obj-y				+= vboxguest/
 
 obj-$(CONFIG_NITRO_ENCLAVES)	+= nitro_enclaves/
 obj-$(CONFIG_ACRN_HSM)		+= acrn/
+obj-$(CONFIG_EFI_SECRET)	+= coco/efi_secret/
diff --git a/drivers/virt/coco/efi_secret/Kconfig b/drivers/virt/coco/efi_secret/Kconfig
new file mode 100644
index 000000000000..be363ed48f24
--- /dev/null
+++ b/drivers/virt/coco/efi_secret/Kconfig
@@ -0,0 +1,11 @@
+# SPDX-License-Identifier: GPL-2.0-only
+config EFI_SECRET
+	tristate "EFI secret area securityfs support"
+	depends on EFI && X86_64
+	select EFI_COCO_SECRET
+	select SECURITYFS
+	help
+	  This is a driver for accessing the EFI secret area via securityfs.
+
+	  To compile this driver as a module, choose M here.
+	  The module will be called efi_secret.
diff --git a/drivers/virt/coco/efi_secret/Makefile b/drivers/virt/coco/efi_secret/Makefile
new file mode 100644
index 000000000000..c7047ce804f7
--- /dev/null
+++ b/drivers/virt/coco/efi_secret/Makefile
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0-only
+obj-$(CONFIG_EFI_SECRET) += efi_secret.o
diff --git a/drivers/virt/coco/efi_secret/efi_secret.c b/drivers/virt/coco/efi_secret/efi_secret.c
new file mode 100644
index 000000000000..f46ee466a69a
--- /dev/null
+++ b/drivers/virt/coco/efi_secret/efi_secret.c
@@ -0,0 +1,337 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * efi_secret module
+ *
+ * Copyright (C) 2021 IBM Corporation
+ * Author: Dov Murik <dovmurik@linux.ibm.com>
+ */
+
+/**
+ * DOC: efi_secret: Allow reading EFI confidential computing (coco) secret area
+ * via securityfs interface.
+ *
+ * When the module is loaded (and securityfs is mounted, typically under
+ * /sys/kernel/security), a "coco/efi_secret" directory is created in
+ * securityfs.  In it, a file is created for each secret entry.  The name of
+ * each such file is the GUID of the secret entry, and its content is the
+ * secret data.
+ */
+
+#include <linux/seq_file.h>
+#include <linux/fs.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/io.h>
+#include <linux/security.h>
+#include <linux/efi.h>
+#include <asm/cacheflush.h>
+
+#define EFI_SECRET_NUM_FILES 64
+
+struct efi_secret {
+	struct dentry *coco_dir;
+	struct dentry *fs_dir;
+	struct dentry *fs_files[EFI_SECRET_NUM_FILES];
+	void __iomem *secret_data;
+	u64 secret_data_len;
+};
+
+/*
+ * Structure of the EFI secret area
+ *
+ * Offset   Length
+ * (bytes)  (bytes)  Usage
+ * -------  -------  -----
+ *       0       16  Secret table header GUID (must be 1e74f542-71dd-4d66-963e-ef4287ff173b)
+ *      16        4  Length of bytes of the entire secret area
+ *
+ *      20       16  First secret entry's GUID
+ *      36        4  First secret entry's length in bytes (= 16 + 4 + x)
+ *      40        x  First secret entry's data
+ *
+ *    40+x       16  Second secret entry's GUID
+ *    56+x        4  Second secret entry's length in bytes (= 16 + 4 + y)
+ *    60+x        y  Second secret entry's data
+ *
+ * (... and so on for additional entries)
+ *
+ * The GUID of each secret entry designates the usage of the secret data.
+ */
+
+/**
+ * struct secret_header - Header of entire secret area; this should be followed
+ * by instances of struct secret_entry.
+ * @guid:	Must be EFI_SECRET_TABLE_HEADER_GUID
+ * @len:	Length in bytes of entire secret area, including header
+ */
+struct secret_header {
+	efi_guid_t guid;
+	u32 len;
+} __attribute((packed));
+
+/**
+ * struct secret_entry - Holds one secret entry
+ * @guid:	Secret-specific GUID (or NULL_GUID if this secret entry was deleted)
+ * @len:	Length of secret entry, including its guid and len fields
+ * @data:	The secret data (full of zeros if this secret entry was deleted)
+ */
+struct secret_entry {
+	efi_guid_t guid;
+	u32 len;
+	u8 data[];
+} __attribute((packed));
+
+static size_t secret_entry_data_len(struct secret_entry *e)
+{
+	return e->len - sizeof(*e);
+}
+
+static struct efi_secret the_efi_secret;
+
+static inline struct efi_secret *efi_secret_get(void)
+{
+	return &the_efi_secret;
+}
+
+static int efi_secret_bin_file_show(struct seq_file *file, void *data)
+{
+	struct secret_entry *e = file->private;
+
+	if (e)
+		seq_write(file, e->data, secret_entry_data_len(e));
+
+	return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(efi_secret_bin_file);
+
+/*
+ * Overwrite memory content with zeroes, and ensure that dirty cache lines are
+ * actually written back to memory, to clear out the secret.
+ */
+static void wipe_memory(void *addr, size_t size)
+{
+	memzero_explicit(addr, size);
+#ifdef CONFIG_X86
+	clflush_cache_range(addr, size);
+#endif
+}
+
+static int efi_secret_unlink(struct inode *dir, struct dentry *dentry)
+{
+	struct efi_secret *s = efi_secret_get();
+	struct inode *inode = d_inode(dentry);
+	struct secret_entry *e = (struct secret_entry *)inode->i_private;
+	int i;
+
+	if (e) {
+		/* Zero out the secret data */
+		wipe_memory(e->data, secret_entry_data_len(e));
+		e->guid = NULL_GUID;
+	}
+
+	inode->i_private = NULL;
+
+	for (i = 0; i < EFI_SECRET_NUM_FILES; i++)
+		if (s->fs_files[i] == dentry)
+			s->fs_files[i] = NULL;
+
+	/*
+	 * securityfs_remove tries to lock the directory's inode, but we reach
+	 * the unlink callback when it's already locked
+	 */
+	inode_unlock(dir);
+	securityfs_remove(dentry);
+	inode_lock(dir);
+
+	return 0;
+}
+
+static const struct inode_operations efi_secret_dir_inode_operations = {
+	.lookup         = simple_lookup,
+	.unlink         = efi_secret_unlink,
+};
+
+static int efi_secret_map_area(void)
+{
+	int ret;
+	struct efi_secret *s = efi_secret_get();
+	struct linux_efi_coco_secret_area *secret_area;
+
+	if (efi.coco_secret == EFI_INVALID_TABLE_ADDR) {
+		pr_err("Secret area address is not available\n");
+		return -EINVAL;
+	}
+
+	secret_area = memremap(efi.coco_secret, sizeof(*secret_area), MEMREMAP_WB);
+	if (secret_area == NULL) {
+		pr_err("Could not map secret area EFI config entry\n");
+		return -ENOMEM;
+	}
+	if (!secret_area->base_pa || secret_area->size < sizeof(struct secret_header)) {
+		pr_err("Invalid secret area memory location (base_pa=0x%llx size=0x%llx)\n",
+		       secret_area->base_pa, secret_area->size);
+		ret = -EINVAL;
+		goto unmap;
+	}
+
+	s->secret_data = ioremap_encrypted(secret_area->base_pa, secret_area->size);
+	if (s->secret_data == NULL) {
+		pr_err("Could not map secret area\n");
+		ret = -ENOMEM;
+		goto unmap;
+	}
+
+	s->secret_data_len = secret_area->size;
+	ret = 0;
+
+unmap:
+	memunmap(secret_area);
+	return ret;
+}
+
+static void efi_secret_securityfs_teardown(void)
+{
+	struct efi_secret *s = efi_secret_get();
+	int i;
+
+	for (i = (EFI_SECRET_NUM_FILES - 1); i >= 0; i--) {
+		securityfs_remove(s->fs_files[i]);
+		s->fs_files[i] = NULL;
+	}
+
+	securityfs_remove(s->fs_dir);
+	s->fs_dir = NULL;
+
+	securityfs_remove(s->coco_dir);
+	s->coco_dir = NULL;
+
+	pr_debug("Removed efi_secret securityfs entries\n");
+}
+
+static int efi_secret_securityfs_setup(void)
+{
+	struct efi_secret *s = efi_secret_get();
+	int ret = 0, i = 0, bytes_left;
+	unsigned char *ptr;
+	struct secret_header *h;
+	struct secret_entry *e;
+	struct dentry *dent;
+	char guid_str[EFI_VARIABLE_GUID_LEN + 1];
+
+	s->coco_dir = NULL;
+	s->fs_dir = NULL;
+	memset(s->fs_files, 0, sizeof(s->fs_files));
+
+	dent = securityfs_create_dir("coco", NULL);
+	if (IS_ERR(dent)) {
+		pr_err("Error creating coco securityfs directory entry err=%ld\n", PTR_ERR(dent));
+		return PTR_ERR(dent);
+	}
+	s->coco_dir = dent;
+
+	dent = securityfs_create_dir("efi_secret", s->coco_dir);
+	if (IS_ERR(dent)) {
+		pr_err("Error creating efi_secret securityfs directory entry err=%ld\n",
+		       PTR_ERR(dent));
+		return PTR_ERR(dent);
+	}
+	d_inode(dent)->i_op = &efi_secret_dir_inode_operations;
+	s->fs_dir = dent;
+
+	ptr = s->secret_data;
+	h = (struct secret_header *)ptr;
+	if (efi_guidcmp(h->guid, EFI_SECRET_TABLE_HEADER_GUID)) {
+		pr_err("EFI secret area does not start with correct GUID\n");
+		ret = -EINVAL;
+		goto err_cleanup;
+	}
+	if (h->len < sizeof(*h)) {
+		pr_err("EFI secret area reported length is too small\n");
+		ret = -EINVAL;
+		goto err_cleanup;
+	}
+	if (h->len > s->secret_data_len) {
+		pr_err("EFI secret area reported length is too big\n");
+		ret = -EINVAL;
+		goto err_cleanup;
+	}
+
+	bytes_left = h->len - sizeof(*h);
+	ptr += sizeof(*h);
+	while (bytes_left >= (int)sizeof(*e) && i < EFI_SECRET_NUM_FILES) {
+		e = (struct secret_entry *)ptr;
+		if (e->len < sizeof(*e) || e->len > (unsigned int)bytes_left) {
+			pr_err("EFI secret area is corrupted\n");
+			ret = -EINVAL;
+			goto err_cleanup;
+		}
+
+		/* Skip deleted entries (which will have NULL_GUID) */
+		if (efi_guidcmp(e->guid, NULL_GUID)) {
+			efi_guid_to_str(&e->guid, guid_str);
+
+			dent = securityfs_create_file(guid_str, 0440, s->fs_dir, (void *)e,
+						      &efi_secret_bin_file_fops);
+			if (IS_ERR(dent)) {
+				pr_err("Error creating efi_secret securityfs entry\n");
+				ret = PTR_ERR(dent);
+				goto err_cleanup;
+			}
+
+			s->fs_files[i++] = dent;
+		}
+		ptr += e->len;
+		bytes_left -= e->len;
+	}
+
+	pr_debug("Created %d entries in efi_secret securityfs\n", i);
+	return 0;
+
+err_cleanup:
+	efi_secret_securityfs_teardown();
+	return ret;
+}
+
+static void efi_secret_unmap_area(void)
+{
+	struct efi_secret *s = efi_secret_get();
+
+	if (s->secret_data) {
+		iounmap(s->secret_data);
+		s->secret_data = NULL;
+		s->secret_data_len = 0;
+	}
+}
+
+static int __init efi_secret_init(void)
+{
+	int ret;
+
+	ret = efi_secret_map_area();
+	if (ret)
+		return ret;
+
+	ret = efi_secret_securityfs_setup();
+	if (ret)
+		goto err_unmap;
+
+	return ret;
+
+err_unmap:
+	efi_secret_unmap_area();
+	return ret;
+}
+
+static void __exit efi_secret_exit(void)
+{
+	efi_secret_securityfs_teardown();
+	efi_secret_unmap_area();
+}
+
+module_init(efi_secret_init);
+module_exit(efi_secret_exit);
+
+MODULE_DESCRIPTION("Confidential computing EFI secret area access");
+MODULE_AUTHOR("IBM");
+MODULE_LICENSE("GPL");
-- 
2.25.1

[PATCH v6 4/5] efi: Load efi_secret module if EFI secret area is populated

[Dov Murik]

If the efi_secret module is built, register a late_initcall in the EFI
driver which checks whether the EFI secret area is available and
populated, and then requests to load the efi_secret module.

This will cause the <securityfs>/coco/efi_secret directory to appear in
guests into which secrets were injected; in other cases, the module is
not loaded.

Signed-off-by: Dov Murik <dovmurik@linux.ibm.com>
---
 drivers/firmware/efi/Makefile        |  1 +
 drivers/firmware/efi/coco.c          | 58 ++++++++++++++++++++++++++++
 drivers/virt/coco/efi_secret/Kconfig |  3 ++
 3 files changed, 62 insertions(+)
 create mode 100644 drivers/firmware/efi/coco.c

diff --git a/drivers/firmware/efi/Makefile b/drivers/firmware/efi/Makefile
index c02ff25dd477..49c4a8c0bfc4 100644
--- a/drivers/firmware/efi/Makefile
+++ b/drivers/firmware/efi/Makefile
@@ -32,6 +32,7 @@ obj-$(CONFIG_APPLE_PROPERTIES)		+= apple-properties.o
 obj-$(CONFIG_EFI_RCI2_TABLE)		+= rci2-table.o
 obj-$(CONFIG_EFI_EMBEDDED_FIRMWARE)	+= embedded-firmware.o
 obj-$(CONFIG_LOAD_UEFI_KEYS)		+= mokvar-table.o
+obj-$(CONFIG_EFI_COCO_SECRET)		+= coco.o
 
 fake_map-y				+= fake_mem.o
 fake_map-$(CONFIG_X86)			+= x86_fake_mem.o
diff --git a/drivers/firmware/efi/coco.c b/drivers/firmware/efi/coco.c
new file mode 100644
index 000000000000..9885ab75e12f
--- /dev/null
+++ b/drivers/firmware/efi/coco.c
@@ -0,0 +1,58 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Confidential computing (coco) secret area handling
+ *
+ * Copyright (C) 2021 IBM Corporation
+ * Author: Dov Murik <dovmurik@linux.ibm.com>
+ */
+
+#define pr_fmt(fmt) "efi: " fmt
+
+#include <linux/efi.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/kmod.h>
+
+#ifdef CONFIG_EFI_SECRET_MODULE
+
+/*
+ * Load the efi_secret module if the EFI secret area is populated
+ */
+static int __init load_efi_secret_module(void)
+{
+	struct linux_efi_coco_secret_area *area;
+	efi_guid_t *header_guid;
+	int ret = 0;
+
+	if (efi.coco_secret == EFI_INVALID_TABLE_ADDR)
+		return 0;
+
+	area = memremap(efi.coco_secret, sizeof(*area), MEMREMAP_WB);
+	if (!area) {
+		pr_err("Failed to map confidential computing secret area descriptor\n");
+		return -ENOMEM;
+	}
+	if (!area->base_pa || area->size < sizeof(*header_guid))
+		goto unmap_desc;
+
+	header_guid = ioremap_encrypted(area->base_pa, sizeof(*header_guid));
+	if (!header_guid) {
+		pr_err("Failed to map secret area\n");
+		ret = -ENOMEM;
+		goto unmap_desc;
+	}
+	if (efi_guidcmp(*header_guid, EFI_SECRET_TABLE_HEADER_GUID))
+		goto unmap_encrypted;
+
+	ret = request_module("efi_secret");
+
+unmap_encrypted:
+	iounmap(header_guid);
+
+unmap_desc:
+	memunmap(area);
+	return ret;
+}
+late_initcall(load_efi_secret_module);
+
+#endif
diff --git a/drivers/virt/coco/efi_secret/Kconfig b/drivers/virt/coco/efi_secret/Kconfig
index be363ed48f24..06a32613815e 100644
--- a/drivers/virt/coco/efi_secret/Kconfig
+++ b/drivers/virt/coco/efi_secret/Kconfig
@@ -9,3 +9,6 @@ config EFI_SECRET
 
 	  To compile this driver as a module, choose M here.
 	  The module will be called efi_secret.
+
+	  The module is loaded automatically by the EFI driver if the EFI
+	  secret area is populated.
-- 
2.25.1

[PATCH v6 5/5] docs: security: Add coco/efi_secret documentation

[Dov Murik]

Add documentation for the efi_secret module which allows access
to Confidential Computing injected secrets.

Signed-off-by: Dov Murik <dovmurik@linux.ibm.com>
---
 Documentation/security/coco/efi_secret.rst | 102 +++++++++++++++++++++
 Documentation/security/coco/index.rst      |   9 ++
 Documentation/security/index.rst           |   1 +
 3 files changed, 112 insertions(+)
 create mode 100644 Documentation/security/coco/efi_secret.rst
 create mode 100644 Documentation/security/coco/index.rst

diff --git a/Documentation/security/coco/efi_secret.rst b/Documentation/security/coco/efi_secret.rst
new file mode 100644
index 000000000000..fa74899b68a3
--- /dev/null
+++ b/Documentation/security/coco/efi_secret.rst
@@ -0,0 +1,102 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+==========
+EFI secret
+==========
+
+This document describes how Confidential Computing secret injection is handled
+from the firmware to the operating system, in the EFI driver and the efi_secret
+kernel module.
+
+
+Introduction
+============
+
+Confidential Computing (coco) hardware such as AMD SEV (Secure Encrypted
+Virtualization) allows guest owners to inject secrets into the VMs
+memory without the host/hypervisor being able to read them.  In SEV,
+secret injection is performed early in the VM launch process, before the
+guest starts running.
+
+The efi_secret kernel module allows userspace applications to access these
+secrets via securityfs.
+
+
+Secret data flow
+================
+
+The guest firmware may reserve a designated memory area for secret injection,
+and publish its location (base GPA and length) in the EFI configuration table
+under a ``LINUX_EFI_COCO_SECRET_AREA_GUID`` entry
+(``adf956ad-e98c-484c-ae11-b51c7d336447``).  This memory area should be marked
+by the firmware as ``EFI_RESERVED_TYPE``, and therefore the kernel should not
+be use it for its own purposes.
+
+During the VM's launch, the virtual machine manager may inject a secret to that
+area.  In AMD SEV and SEV-ES this is performed using the
+``KVM_SEV_LAUNCH_SECRET`` command (see [sev]_).  The strucutre of the injected
+Guest Owner secret data should be a GUIDed table of secret values; the binary
+format is described in ``drivers/virt/coco/efi_secret/efi_secret.c`` under
+"Structure of the EFI secret area".
+
+On kernel start, the kernel's EFI driver saves the location of the secret area
+(taken from the EFI configuration table) in the ``efi.coco_secret`` field.
+Later it checks if the secret area is populated: it maps the area and checks
+whether its content begins with ``EFI_SECRET_TABLE_HEADER_GUID``
+(``1e74f542-71dd-4d66-963e-ef4287ff173b``).  If the secret area is populated,
+the EFI driver will autoload the efi_secret kernel module, which exposes the
+secretes to userspace applications via securityfs.  The details of the
+efi_secret filesystem interface are in [efi-secret-abi]_.
+
+
+Application usage example
+=========================
+
+Consider a guest performing computations on encrypted files.  The Guest Owner
+provides the decryption key (= secret) using the secret injection mechanism.
+The guest application reads the secret from the efi_secret filesystem and
+proceeds to decrypt the files into memory and then performs the needed
+computations on the content.
+
+In this example, the host can't read the files from the disk image
+because they are encrypted.  Host can't read the decryption key because
+it is passed using the secret injection mechanism (= secure channel).
+Host can't read the decrypted content from memory because it's a
+confidential (memory-encrypted) guest.
+
+Here is a simple example for usage of the efi_secret module in a guest
+to which an EFI secret area with 4 secrets was injected during launch::
+
+	# ls -la /sys/kernel/security/coco/efi_secret
+	total 0
+	drwxr-xr-x 2 root root 0 Jun 28 11:54 .
+	drwxr-xr-x 3 root root 0 Jun 28 11:54 ..
+	-r--r----- 1 root root 0 Jun 28 11:54 736870e5-84f0-4973-92ec-06879ce3da0b
+	-r--r----- 1 root root 0 Jun 28 11:54 83c83f7f-1356-4975-8b7e-d3a0b54312c6
+	-r--r----- 1 root root 0 Jun 28 11:54 9553f55d-3da2-43ee-ab5d-ff17f78864d2
+	-r--r----- 1 root root 0 Jun 28 11:54 e6f5a162-d67f-4750-a67c-5d065f2a9910
+
+	# xxd /sys/kernel/security/coco/efi_secret/e6f5a162-d67f-4750-a67c-5d065f2a9910
+	00000000: 7468 6573 652d 6172 652d 7468 652d 6b61  these-are-the-ka
+	00000010: 7461 2d73 6563 7265 7473 0001 0203 0405  ta-secrets......
+	00000020: 0607                                     ..
+
+	# rm /sys/kernel/security/coco/efi_secret/e6f5a162-d67f-4750-a67c-5d065f2a9910
+
+	# ls -la /sys/kernel/security/coco/efi_secret
+	total 0
+	drwxr-xr-x 2 root root 0 Jun 28 11:55 .
+	drwxr-xr-x 3 root root 0 Jun 28 11:54 ..
+	-r--r----- 1 root root 0 Jun 28 11:54 736870e5-84f0-4973-92ec-06879ce3da0b
+	-r--r----- 1 root root 0 Jun 28 11:54 83c83f7f-1356-4975-8b7e-d3a0b54312c6
+	-r--r----- 1 root root 0 Jun 28 11:54 9553f55d-3da2-43ee-ab5d-ff17f78864d2
+
+
+References
+==========
+
+See [sev-api-spec]_ for more info regarding SEV ``LAUNCH_SECRET`` operation.
+
+.. [sev] Documentation/virt/kvm/amd-memory-encryption.rst
+.. [efi-secret-abi] Documentation/ABI/testing/securityfs-coco-efi_secret
+.. [sev-api-spec] https://www.amd.com/system/files/TechDocs/55766_SEV-KM_API_Specification.pdf
diff --git a/Documentation/security/coco/index.rst b/Documentation/security/coco/index.rst
new file mode 100644
index 000000000000..56b803d4b33e
--- /dev/null
+++ b/Documentation/security/coco/index.rst
@@ -0,0 +1,9 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+====================================
+Confidential Computing documentation
+====================================
+
+.. toctree::
+
+   efi_secret
diff --git a/Documentation/security/index.rst b/Documentation/security/index.rst
index 16335de04e8c..3662e93a3eba 100644
--- a/Documentation/security/index.rst
+++ b/Documentation/security/index.rst
@@ -17,3 +17,4 @@ Security Documentation
    tpm/index
    digsig
    landlock
+   coco/index
-- 
2.25.1

Re: [PATCH v6 3/5] virt: Add efi_secret module to expose confidential computing secrets

[kernel test robot]

Hi Dov,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on 42eb8fdac2fc5d62392dcfcf0253753e821a97b0]

url:    https://github.com/0day-ci/linux/commits/Dov-Murik/Allow-guest-access-to-EFI-confidential-computing-secret-area/20211129-194749
base:   42eb8fdac2fc5d62392dcfcf0253753e821a97b0
config: x86_64-randconfig-s032-20211206 (https://download.01.org/0day-ci/archive/20211206/202112061528.k1C2Xe9d-lkp@intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.4-dirty
        # https://github.com/0day-ci/linux/commit/ebf498903d5371698bd13ed4005b4d61702f8223
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Dov-Murik/Allow-guest-access-to-EFI-confidential-computing-secret-area/20211129-194749
        git checkout ebf498903d5371698bd13ed4005b4d61702f8223
        # save the config file to linux build tree
        mkdir build_dir
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/virt/coco/efi_secret/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)
>> drivers/virt/coco/efi_secret/efi_secret.c:242:13: sparse: sparse: incorrect type in assignment (different address spaces) @@     expected unsigned char *ptr @@     got void [noderef] __iomem *secret_data @@
   drivers/virt/coco/efi_secret/efi_secret.c:242:13: sparse:     expected unsigned char *ptr
   drivers/virt/coco/efi_secret/efi_secret.c:242:13: sparse:     got void [noderef] __iomem *secret_data

vim +242 drivers/virt/coco/efi_secret/efi_secret.c

   211	
   212	static int efi_secret_securityfs_setup(void)
   213	{
   214		struct efi_secret *s = efi_secret_get();
   215		int ret = 0, i = 0, bytes_left;
   216		unsigned char *ptr;
   217		struct secret_header *h;
   218		struct secret_entry *e;
   219		struct dentry *dent;
   220		char guid_str[EFI_VARIABLE_GUID_LEN + 1];
   221	
   222		s->coco_dir = NULL;
   223		s->fs_dir = NULL;
   224		memset(s->fs_files, 0, sizeof(s->fs_files));
   225	
   226		dent = securityfs_create_dir("coco", NULL);
   227		if (IS_ERR(dent)) {
   228			pr_err("Error creating coco securityfs directory entry err=%ld\n", PTR_ERR(dent));
   229			return PTR_ERR(dent);
   230		}
   231		s->coco_dir = dent;
   232	
   233		dent = securityfs_create_dir("efi_secret", s->coco_dir);
   234		if (IS_ERR(dent)) {
   235			pr_err("Error creating efi_secret securityfs directory entry err=%ld\n",
   236			       PTR_ERR(dent));
   237			return PTR_ERR(dent);
   238		}
   239		d_inode(dent)->i_op = &efi_secret_dir_inode_operations;
   240		s->fs_dir = dent;
   241	
 > 242		ptr = s->secret_data;
   243		h = (struct secret_header *)ptr;
   244		if (efi_guidcmp(h->guid, EFI_SECRET_TABLE_HEADER_GUID)) {
   245			pr_err("EFI secret area does not start with correct GUID\n");
   246			ret = -EINVAL;
   247			goto err_cleanup;
   248		}
   249		if (h->len < sizeof(*h)) {
   250			pr_err("EFI secret area reported length is too small\n");
   251			ret = -EINVAL;
   252			goto err_cleanup;
   253		}
   254		if (h->len > s->secret_data_len) {
   255			pr_err("EFI secret area reported length is too big\n");
   256			ret = -EINVAL;
   257			goto err_cleanup;
   258		}
   259	
   260		bytes_left = h->len - sizeof(*h);
   261		ptr += sizeof(*h);
   262		while (bytes_left >= (int)sizeof(*e) && i < EFI_SECRET_NUM_FILES) {
   263			e = (struct secret_entry *)ptr;
   264			if (e->len < sizeof(*e) || e->len > (unsigned int)bytes_left) {
   265				pr_err("EFI secret area is corrupted\n");
   266				ret = -EINVAL;
   267				goto err_cleanup;
   268			}
   269	
   270			/* Skip deleted entries (which will have NULL_GUID) */
   271			if (efi_guidcmp(e->guid, NULL_GUID)) {
   272				efi_guid_to_str(&e->guid, guid_str);
   273	
   274				dent = securityfs_create_file(guid_str, 0440, s->fs_dir, (void *)e,
   275							      &efi_secret_bin_file_fops);
   276				if (IS_ERR(dent)) {
   277					pr_err("Error creating efi_secret securityfs entry\n");
   278					ret = PTR_ERR(dent);
   279					goto err_cleanup;
   280				}
   281	
   282				s->fs_files[i++] = dent;
   283			}
   284			ptr += e->len;
   285			bytes_left -= e->len;
   286		}
   287	
   288		pr_debug("Created %d entries in efi_secret securityfs\n", i);
   289		return 0;
   290	
   291	err_cleanup:
   292		efi_secret_securityfs_teardown();
   293		return ret;
   294	}
   295	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

Re: [PATCH v6 3/5] virt: Add efi_secret module to expose confidential computing secrets

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 4815 bytes --]

Hi Dov,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on 42eb8fdac2fc5d62392dcfcf0253753e821a97b0]

url:    https://github.com/0day-ci/linux/commits/Dov-Murik/Allow-guest-access-to-EFI-confidential-computing-secret-area/20211129-194749
base:   42eb8fdac2fc5d62392dcfcf0253753e821a97b0
config: x86_64-randconfig-s032-20211206 (https://download.01.org/0day-ci/archive/20211206/202112061528.k1C2Xe9d-lkp(a)intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.4-dirty
        # https://github.com/0day-ci/linux/commit/ebf498903d5371698bd13ed4005b4d61702f8223
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Dov-Murik/Allow-guest-access-to-EFI-confidential-computing-secret-area/20211129-194749
        git checkout ebf498903d5371698bd13ed4005b4d61702f8223
        # save the config file to linux build tree
        mkdir build_dir
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/virt/coco/efi_secret/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)
>> drivers/virt/coco/efi_secret/efi_secret.c:242:13: sparse: sparse: incorrect type in assignment (different address spaces) @@     expected unsigned char *ptr @@     got void [noderef] __iomem *secret_data @@
   drivers/virt/coco/efi_secret/efi_secret.c:242:13: sparse:     expected unsigned char *ptr
   drivers/virt/coco/efi_secret/efi_secret.c:242:13: sparse:     got void [noderef] __iomem *secret_data

vim +242 drivers/virt/coco/efi_secret/efi_secret.c

   211	
   212	static int efi_secret_securityfs_setup(void)
   213	{
   214		struct efi_secret *s = efi_secret_get();
   215		int ret = 0, i = 0, bytes_left;
   216		unsigned char *ptr;
   217		struct secret_header *h;
   218		struct secret_entry *e;
   219		struct dentry *dent;
   220		char guid_str[EFI_VARIABLE_GUID_LEN + 1];
   221	
   222		s->coco_dir = NULL;
   223		s->fs_dir = NULL;
   224		memset(s->fs_files, 0, sizeof(s->fs_files));
   225	
   226		dent = securityfs_create_dir("coco", NULL);
   227		if (IS_ERR(dent)) {
   228			pr_err("Error creating coco securityfs directory entry err=%ld\n", PTR_ERR(dent));
   229			return PTR_ERR(dent);
   230		}
   231		s->coco_dir = dent;
   232	
   233		dent = securityfs_create_dir("efi_secret", s->coco_dir);
   234		if (IS_ERR(dent)) {
   235			pr_err("Error creating efi_secret securityfs directory entry err=%ld\n",
   236			       PTR_ERR(dent));
   237			return PTR_ERR(dent);
   238		}
   239		d_inode(dent)->i_op = &efi_secret_dir_inode_operations;
   240		s->fs_dir = dent;
   241	
 > 242		ptr = s->secret_data;
   243		h = (struct secret_header *)ptr;
   244		if (efi_guidcmp(h->guid, EFI_SECRET_TABLE_HEADER_GUID)) {
   245			pr_err("EFI secret area does not start with correct GUID\n");
   246			ret = -EINVAL;
   247			goto err_cleanup;
   248		}
   249		if (h->len < sizeof(*h)) {
   250			pr_err("EFI secret area reported length is too small\n");
   251			ret = -EINVAL;
   252			goto err_cleanup;
   253		}
   254		if (h->len > s->secret_data_len) {
   255			pr_err("EFI secret area reported length is too big\n");
   256			ret = -EINVAL;
   257			goto err_cleanup;
   258		}
   259	
   260		bytes_left = h->len - sizeof(*h);
   261		ptr += sizeof(*h);
   262		while (bytes_left >= (int)sizeof(*e) && i < EFI_SECRET_NUM_FILES) {
   263			e = (struct secret_entry *)ptr;
   264			if (e->len < sizeof(*e) || e->len > (unsigned int)bytes_left) {
   265				pr_err("EFI secret area is corrupted\n");
   266				ret = -EINVAL;
   267				goto err_cleanup;
   268			}
   269	
   270			/* Skip deleted entries (which will have NULL_GUID) */
   271			if (efi_guidcmp(e->guid, NULL_GUID)) {
   272				efi_guid_to_str(&e->guid, guid_str);
   273	
   274				dent = securityfs_create_file(guid_str, 0440, s->fs_dir, (void *)e,
   275							      &efi_secret_bin_file_fops);
   276				if (IS_ERR(dent)) {
   277					pr_err("Error creating efi_secret securityfs entry\n");
   278					ret = PTR_ERR(dent);
   279					goto err_cleanup;
   280				}
   281	
   282				s->fs_files[i++] = dent;
   283			}
   284			ptr += e->len;
   285			bytes_left -= e->len;
   286		}
   287	
   288		pr_debug("Created %d entries in efi_secret securityfs\n", i);
   289		return 0;
   290	
   291	err_cleanup:
   292		efi_secret_securityfs_teardown();
   293		return ret;
   294	}
   295	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

Re: [PATCH v6 0/5] Allow guest access to EFI confidential computing secret area

[Dov Murik]

Gentle ping for this series.

(also at https://lore.kernel.org/linux-coco/20211129114251.3741721-1-dovmurik@linux.ibm.com/ )

[+cc Gerd, Lenny]

Thanks,
-Dov

On 29/11/2021 13:42, Dov Murik wrote:
> Confidential computing (coco) hardware such as AMD SEV (Secure Encrypted
> Virtualization) allows guest owners to inject secrets into the VMs
> memory without the host/hypervisor being able to read them.  In SEV,
> secret injection is performed early in the VM launch process, before the
> guest starts running.
> 
> OVMF already reserves designated area for secret injection (in its
> AmdSev package; see edk2 commit 01726b6d23d4 "OvmfPkg/AmdSev: Expose the
> Sev Secret area using a configuration table" [1]), but the secrets were
> not available in the guest kernel.
> 
> The patch series keeps the address of the EFI-provided memory for
> injected secrets, and exposes the secrets to userspace via securityfs
> using a new efi_secret kernel module.  The module is autoloaded (by the
> EFI driver) if the secret area is populated.
> 
> The first patch in EFI keeps the address of the secret area as passed in
> the EFI configuration table.  The second patch is a quirk fix for older
> firmwares didn't mark the secrets page as EFI_RESERVED_TYPE.  The third
> patch introduces the new efi_secret module that exposes the content of
> the secret entries as securityfs files, and allows clearing out secrets
> with a file unlink interface.  The fourth patch auto-loads the
> efi_secret module during startup if the injected secrets area is
> populated.  The last patch documents the data flow of confidential
> computing secret injection.
> 
> As a usage example, consider a guest performing computations on
> encrypted files.  The Guest Owner provides the decryption key (= secret)
> using the secret injection mechanism.  The guest application reads the
> secret from the efi_secret filesystem and proceeds to decrypt the files
> into memory and then performs the needed computations on the content.
> 
> In this example, the host can't read the files from the disk image
> because they are encrypted.  Host can't read the decryption key because
> it is passed using the secret injection mechanism (= secure channel).
> Host can't read the decrypted content from memory because it's a
> confidential (memory-encrypted) guest.
> 
> This has been tested with AMD SEV and SEV-ES guests, but the kernel side
> of handling the secret area has no SEV-specific dependencies, and
> therefore might be usable (perhaps with minor changes) for any
> confidential computing hardware that can publish the secret area via the
> standard EFI config table entry.
> 
> To enable this functionality, set CONFIG_EFI_SECRET=m when building the
> guest kernel.
> 
> Here is a simple example for usage of the efi_secret module in a guest
> to which an EFI secret area with 4 secrets was injected during launch:
> 
> # ls -la /sys/kernel/security/coco/efi_secret
> total 0
> drwxr-xr-x 2 root root 0 Jun 28 11:54 .
> drwxr-xr-x 3 root root 0 Jun 28 11:54 ..
> -r--r----- 1 root root 0 Jun 28 11:54 736870e5-84f0-4973-92ec-06879ce3da0b
> -r--r----- 1 root root 0 Jun 28 11:54 83c83f7f-1356-4975-8b7e-d3a0b54312c6
> -r--r----- 1 root root 0 Jun 28 11:54 9553f55d-3da2-43ee-ab5d-ff17f78864d2
> -r--r----- 1 root root 0 Jun 28 11:54 e6f5a162-d67f-4750-a67c-5d065f2a9910
> 
> # xxd /sys/kernel/security/coco/efi_secret/e6f5a162-d67f-4750-a67c-5d065f2a9910
> 00000000: 7468 6573 652d 6172 652d 7468 652d 6b61  these-are-the-ka
> 00000010: 7461 2d73 6563 7265 7473 0001 0203 0405  ta-secrets......
> 00000020: 0607                                     ..
> 
> # rm /sys/kernel/security/coco/efi_secret/e6f5a162-d67f-4750-a67c-5d065f2a9910
> 
> # ls -la /sys/kernel/security/coco/efi_secret
> total 0
> drwxr-xr-x 2 root root 0 Jun 28 11:55 .
> drwxr-xr-x 3 root root 0 Jun 28 11:54 ..
> -r--r----- 1 root root 0 Jun 28 11:54 736870e5-84f0-4973-92ec-06879ce3da0b
> -r--r----- 1 root root 0 Jun 28 11:54 83c83f7f-1356-4975-8b7e-d3a0b54312c6
> -r--r----- 1 root root 0 Jun 28 11:54 9553f55d-3da2-43ee-ab5d-ff17f78864d2
> 
> 
> [1] https://github.com/tianocore/edk2/commit/01726b6d23d4
> 
> 
> ---
> 
> v6 changes:
>  - Autoload the efi_secret module if the secret area is populated
>    (thanks Greg KH).
>  - efi_secret: Depend on X86_64 because we use ioremap_encrypted() which
>    is only defined for this arch.
>  - efi_secret.c: Remove unneeded tableheader_guid local variable.
>  - Documentation fixes.
> 
> v5: https://lore.kernel.org/linux-coco/20211118113359.642571-1-dovmurik@linux.ibm.com/
> v5 changes:
>  - Simplify EFI code: instead of copying the secret area, the firmware
>    marks the secret area as EFI_RESERVED_TYPE, and then the uefi_init()
>    code just keeps the pointer as it appears in the EFI configuration
>    table.  The use of reserved pages is similar to the AMD SEV-SNP
>    patches for handling SNP-Secrets and SNP-CPUID pages.
>  - In order to handle OVMF releases out there which mark the
>    confidential computing secrets page as EFI_BOOT_SERVICES_DATA, add
>    efi/libstub code that detects this and fixes the E820 map to reserve
>    this page.
>  - In the efi_secret module code, map the secrets page using
>    ioremap_encrypted (again, similar to the AMD SEV-SNP guest patches
>    for accessing SNP-Secrets and SNP-CPUID pages).
>  - Add documentation in Documentation/security/coco/efi_secret.
> 
> v4: https://lore.kernel.org/linux-coco/20211020061408.3447533-1-dovmurik@linux.ibm.com/
> v4 changes:
>  - Guard all the new EFI and efi-stub code (patches 1+2) with #ifdef
>    CONFIG_EFI_COCO_SECRET (thanks Greg KH).  Selecting
>    CONFIG_EFI_SECRET=m (patch 3) will enable the EFI parts as well.
>  - Guard call to clflush_cache_range() with #ifdef CONFIG_X86
>    (Reported-by: kernel test robot <lkp@intel.com>)
> 
> v3: https://lore.kernel.org/linux-coco/20211014130848.592611-1-dovmurik@linux.ibm.com/
> v3 changes:
>  - Rename the module to efi_secret
>  - Remove the exporting of clean_cache_range
>  - Use clflush_cache_range in wipe_memory
>  - Document function wipe_memory
>  - Initialize efi.coco_secret to EFI_INVALID_TABLE_ADDR to correctly detect
>    when there's no secret area published in the EFI configuration tables
> 
> v2: https://lore.kernel.org/linux-coco/20211007061838.1381129-1-dovmurik@linux.ibm.com
> v2 changes:
>  - Export clean_cache_range()
>  - When deleteing a secret, call clean_cache_range() after explicit_memzero
>  - Add Documentation/ABI/testing/securityfs-coco-sev_secret
> 
> v1: https://lore.kernel.org/linux-coco/20210809190157.279332-1-dovmurik@linux.ibm.com/
> 
> RFC: https://lore.kernel.org/linux-coco/20210628183431.953934-1-dovmurik@linux.ibm.com/
> 
> 
> Dov Murik (5):
>   efi: Save location of EFI confidential computing area
>   efi/libstub: Reserve confidential computing secret area
>   virt: Add efi_secret module to expose confidential computing secrets
>   efi: Load efi_secret module if EFI secret area is populated
>   docs: security: Add coco/efi_secret documentation
> 
>  .../ABI/testing/securityfs-coco-efi_secret    |  51 +++
>  Documentation/security/coco/efi_secret.rst    | 102 ++++++
>  Documentation/security/coco/index.rst         |   9 +
>  Documentation/security/index.rst              |   1 +
>  arch/x86/platform/efi/efi.c                   |   3 +
>  drivers/firmware/efi/Kconfig                  |  16 +
>  drivers/firmware/efi/Makefile                 |   1 +
>  drivers/firmware/efi/coco.c                   |  58 +++
>  drivers/firmware/efi/efi.c                    |   6 +
>  drivers/firmware/efi/libstub/x86-stub.c       |  28 ++
>  drivers/virt/Kconfig                          |   3 +
>  drivers/virt/Makefile                         |   1 +
>  drivers/virt/coco/efi_secret/Kconfig          |  14 +
>  drivers/virt/coco/efi_secret/Makefile         |   2 +
>  drivers/virt/coco/efi_secret/efi_secret.c     | 337 ++++++++++++++++++
>  include/linux/efi.h                           |  10 +
>  16 files changed, 642 insertions(+)
>  create mode 100644 Documentation/ABI/testing/securityfs-coco-efi_secret
>  create mode 100644 Documentation/security/coco/efi_secret.rst
>  create mode 100644 Documentation/security/coco/index.rst
>  create mode 100644 drivers/firmware/efi/coco.c
>  create mode 100644 drivers/virt/coco/efi_secret/Kconfig
>  create mode 100644 drivers/virt/coco/efi_secret/Makefile
>  create mode 100644 drivers/virt/coco/efi_secret/efi_secret.c
> 
> 
> base-commit: 42eb8fdac2fc5d62392dcfcf0253753e821a97b0

Re: [PATCH v6 0/5] Allow guest access to EFI confidential computing secret area

[Borislav Petkov]

On Mon, Nov 29, 2021 at 11:42:46AM +0000, Dov Murik wrote:
> As a usage example, consider a guest performing computations on
> encrypted files.  The Guest Owner provides the decryption key (= secret)
> using the secret injection mechanism.  The guest application reads the
> secret from the efi_secret filesystem and proceeds to decrypt the files
> into memory and then performs the needed computations on the content.
> 
> In this example, the host can't read the files from the disk image
> because they are encrypted.  Host can't read the decryption key because
> it is passed using the secret injection mechanism (= secure channel).
> Host can't read the decrypted content from memory because it's a
> confidential (memory-encrypted) guest.

So maybe I don't understand the example properly or something's missing
but why can't the guest owner simply scp the secrets into the guest? Why
is this special thing needed?

The secret below says "...kata-secrets" so this sounds like
something-automated-containers-thing where they'd profit from getting
secrets automatically supplied to the guest. But I guess there you can
scp too...

So what am I missing?

Thx.

-- 
Regards/Gruss,
    Boris.

SUSE Software Solutions Germany GmbH, GF: Ivo Totev, HRB 36809, AG Nürnberg

Re: [PATCH v6 0/5] Allow guest access to EFI confidential computing secret area

[Dov Murik]

Hello Boris,

On 03/01/2022 20:59, Borislav Petkov wrote:
> On Mon, Nov 29, 2021 at 11:42:46AM +0000, Dov Murik wrote:
>> As a usage example, consider a guest performing computations on
>> encrypted files.  The Guest Owner provides the decryption key (= secret)
>> using the secret injection mechanism.  The guest application reads the
>> secret from the efi_secret filesystem and proceeds to decrypt the files
>> into memory and then performs the needed computations on the content.
>>
>> In this example, the host can't read the files from the disk image
>> because they are encrypted.  Host can't read the decryption key because
>> it is passed using the secret injection mechanism (= secure channel).
>> Host can't read the decrypted content from memory because it's a
>> confidential (memory-encrypted) guest.
> 
> So maybe I don't understand the example properly or something's missing
> but why can't the guest owner simply scp the secrets into the guest? Why
> is this special thing needed?

If the Guest Owner chooses to inject secrets via scp, it needs
to be sure it is scp-ing to the correct VM - the one that has SEV
enabled and was measured at launch.

One way to achieve that would be to inject the guest's SSH private key
using the proposed efi_secret mechanism.  This way the Guest Owner is
sure it is talking to the correct guest and not to some other VM that
was started by the untrusted cloud provider (say, with SEV disabled so
the cloud provider can steal its memory content).


> 
> The secret below says "...kata-secrets" so this sounds like
> something-automated-containers-thing where they'd profit from getting
> secrets automatically supplied to the guest. But I guess there you can
> scp too...

Indeed this proposed efi_secret module is in use for enabling SEV
confidential containers using Kata containers [1], but there's nothing
specific in the current patch series about containers.  The patch series
just exposes the launch-injected SEV secrets to userspace as virtual files
(under securityfs).

[1] https://github.com/confidential-containers/attestation-agent/tree/main/src/kbc_modules/offline_sev_kbc


> 
> So what am I missing?
> 

It boils down to: the confidential guest needs to have access to a
secret which the untrusted host can't read, and which is essential for
the normal operation of the guest.  This secret can be a decryption key,
an SSH private key, an API key to a Key Management system, etc.  If a
malicious cloud provider tries to start that VM without a secret (or
with the wrong one), the actual workload that the guest is supposed to
run will not execute meaningfully.

The proposed patch series exposes the SEV injected secrets as virtual
files, which can later be used as decryption keys (as done in the kata
confidential containers use-case), or SSH private keys, or any other
possible implementation.

-Dov

Re: [PATCH v6 0/5] Allow guest access to EFI confidential computing secret area

[Borislav Petkov]

On Tue, Jan 04, 2022 at 09:02:03AM +0200, Dov Murik wrote:
> If the Guest Owner chooses to inject secrets via scp, it needs
> to be sure it is scp-ing to the correct VM - the one that has SEV
> enabled and was measured at launch.

Hmm, I'd expect that to be part of the attestation dance. I admit,
though, I have only listened about the whole attestation bla from the
sidelines so I'm unclear whether that's part of that protocol. I guess
Tom and Brijesh should have a better idea here.

> One way to achieve that would be to inject the guest's SSH private key

Well, is that "one way" or *the way*?

> using the proposed efi_secret mechanism.  This way the Guest Owner is
> sure it is talking to the correct guest and not to some other VM that
> was started by the untrusted cloud provider (say, with SEV disabled so
> the cloud provider can steal its memory content).

Because we would need *some* way of verifying the owner is talking
to the correct guest. And if so, this should be made part of the big
picture of SEV guest attestation. Or is this part of that attestation
dance?

I guess I'm wondering where in the big picture this fits into?

> Indeed this proposed efi_secret module is in use for enabling SEV
> confidential containers using Kata containers [1], but there's nothing
> specific in the current patch series about containers.  The patch series
> just exposes the launch-injected SEV secrets to userspace as virtual files
> (under securityfs).
> 
> [1] https://github.com/confidential-containers/attestation-agent/tree/main/src/kbc_modules/offline_sev_kbc

So one of the aspects for this is to use it in automated deployments.

> It boils down to: the confidential guest needs to have access to a
> secret which the untrusted host can't read, and which is essential for
> the normal operation of the guest.  This secret can be a decryption key,
> an SSH private key, an API key to a Key Management system, etc.  If a
> malicious cloud provider tries to start that VM without a secret (or
> with the wrong one), the actual workload that the guest is supposed to
> run will not execute meaningfully.
> 
> The proposed patch series exposes the SEV injected secrets as virtual
> files, which can later be used as decryption keys (as done in the kata
> confidential containers use-case), or SSH private keys, or any other
> possible implementation.

Right, and is this going to be the proper way to authenticate SEV guests
to their owners or is this just another technique for safely supplying
secrets into the guest?

I hope I'm making some sense here...

-- 
Regards/Gruss,
    Boris.

SUSE Software Solutions Germany GmbH, GF: Ivo Totev, HRB 36809, AG Nürnberg

Re: [PATCH v6 0/5] Allow guest access to EFI confidential computing secret area

[Dr. David Alan Gilbert]

* Borislav Petkov (bp@suse.de) wrote:
> On Tue, Jan 04, 2022 at 09:02:03AM +0200, Dov Murik wrote:
> > If the Guest Owner chooses to inject secrets via scp, it needs
> > to be sure it is scp-ing to the correct VM - the one that has SEV
> > enabled and was measured at launch.
> 
> Hmm, I'd expect that to be part of the attestation dance. I admit,
> though, I have only listened about the whole attestation bla from the
> sidelines so I'm unclear whether that's part of that protocol. I guess
> Tom and Brijesh should have a better idea here.

There's more than one type of dance; this partially varies
depending on the system (SEV/TDX etc) and also depends on how you depend
to boot your VM (separate kernel or VM disk).   Also it's important to
note that when the dance happens varies - in SEV and SEV-ES this happens
before the guest executes any code.
So at the end of the dance, the guest owner hands over that secret - but
only then does the geust start booting; that secret has to go somewhere
to be used by something later.
For example, something might pull out that key and use it to decrypt a
disk that then has other secrets on it (e.g. your ssh key).

Dave

> > One way to achieve that would be to inject the guest's SSH private key
> 
> Well, is that "one way" or *the way*?
> 
> > using the proposed efi_secret mechanism.  This way the Guest Owner is
> > sure it is talking to the correct guest and not to some other VM that
> > was started by the untrusted cloud provider (say, with SEV disabled so
> > the cloud provider can steal its memory content).
> 
> Because we would need *some* way of verifying the owner is talking
> to the correct guest. And if so, this should be made part of the big
> picture of SEV guest attestation. Or is this part of that attestation
> dance?
> 
> I guess I'm wondering where in the big picture this fits into?
> 
> > Indeed this proposed efi_secret module is in use for enabling SEV
> > confidential containers using Kata containers [1], but there's nothing
> > specific in the current patch series about containers.  The patch series
> > just exposes the launch-injected SEV secrets to userspace as virtual files
> > (under securityfs).
> > 
> > [1] https://github.com/confidential-containers/attestation-agent/tree/main/src/kbc_modules/offline_sev_kbc
> 
> So one of the aspects for this is to use it in automated deployments.
> 
> > It boils down to: the confidential guest needs to have access to a
> > secret which the untrusted host can't read, and which is essential for
> > the normal operation of the guest.  This secret can be a decryption key,
> > an SSH private key, an API key to a Key Management system, etc.  If a
> > malicious cloud provider tries to start that VM without a secret (or
> > with the wrong one), the actual workload that the guest is supposed to
> > run will not execute meaningfully.
> > 
> > The proposed patch series exposes the SEV injected secrets as virtual
> > files, which can later be used as decryption keys (as done in the kata
> > confidential containers use-case), or SSH private keys, or any other
> > possible implementation.
> 
> Right, and is this going to be the proper way to authenticate SEV guests
> to their owners or is this just another technique for safely supplying
> secrets into the guest?
> 
> I hope I'm making some sense here...
> 
> -- 
> Regards/Gruss,
>     Boris.
> 
> SUSE Software Solutions Germany GmbH, GF: Ivo Totev, HRB 36809, AG Nürnberg
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH v6 0/5] Allow guest access to EFI confidential computing secret area

[Borislav Petkov]

On Wed, Jan 05, 2022 at 11:43:25AM +0000, Dr. David Alan Gilbert wrote:
> There's more than one type of dance;

So Brijesh and I talked about this a bit yesterday. There's all kinds of
dances...

> this partially varies depending on the system (SEV/TDX etc)

By "SEV" I guess you mean pre-SNP because SNP attestation is reportedly
much better.

TDX I'm being told is not interested in something like that atm. I guess
they wanna do something different wrt attestation.

So what we're talking about here is pre-SNP attestation, AFAICT.

> and also depends on how you depend to boot your VM (separate kernel
> or VM disk). Also it's important to note that when the dance happens
> varies - in SEV and SEV-ES this happens before the guest executes any
> code. So at the end of the dance, the guest owner hands over that
> secret - but only then does the geust start booting;

Right.

> that secret has to go somewhere to be used by something later. For
> example, something might pull out that key and use it to decrypt a
> disk that then has other secrets on it (e.g. your ssh key).

That is the other example I heard about.

So, to sum up: this looks like part of a pre-SNP attestation flow, i.e.,
for SEV and SEV-ES guests.

Follow-up question: is this going to be used by other cloud vendors too?
Or am I gonna get another implementation of sharing secrets with a guest
which is just a little bit different but sender #2 can't use this one
because raisins?

Because that would not be good.

So, is this what cloud vendors using SEV/-ES guests would like to use
and what they all agree upon?

Thx.

-- 
Regards/Gruss,
    Boris.

SUSE Software Solutions Germany GmbH, GF: Ivo Totev, HRB 36809, AG Nürnberg

Re: [PATCH v6 0/5] Allow guest access to EFI confidential computing secret area

[Dr. David Alan Gilbert]

* Borislav Petkov (bp@suse.de) wrote:
> On Wed, Jan 05, 2022 at 11:43:25AM +0000, Dr. David Alan Gilbert wrote:
> > There's more than one type of dance;
> 
> So Brijesh and I talked about this a bit yesterday. There's all kinds of
> dances...
> 
> > this partially varies depending on the system (SEV/TDX etc)
> 
> By "SEV" I guess you mean pre-SNP because SNP attestation is reportedly
> much better.
> 
> TDX I'm being told is not interested in something like that atm. I guess
> they wanna do something different wrt attestation.

I thought I saw something in their patch series where they also had a
secret that got passed down from EFI?  As I remember they had it with an
ioctl and something; but it felt to me if it would be great if it was
shared.

> So what we're talking about here is pre-SNP attestation, AFAICT.
> 
> > and also depends on how you depend to boot your VM (separate kernel
> > or VM disk). Also it's important to note that when the dance happens
> > varies - in SEV and SEV-ES this happens before the guest executes any
> > code. So at the end of the dance, the guest owner hands over that
> > secret - but only then does the geust start booting;
> 
> Right.
> 
> > that secret has to go somewhere to be used by something later. For
> > example, something might pull out that key and use it to decrypt a
> > disk that then has other secrets on it (e.g. your ssh key).
> 
> That is the other example I heard about.
> 
> So, to sum up: this looks like part of a pre-SNP attestation flow, i.e.,
> for SEV and SEV-ES guests.

Yeh. SNP is a whole different question.

> Follow-up question: is this going to be used by other cloud vendors too?
> Or am I gonna get another implementation of sharing secrets with a guest
> which is just a little bit different but sender #2 can't use this one
> because raisins?
> 
> Because that would not be good.
> 
> So, is this what cloud vendors using SEV/-ES guests would like to use
> and what they all agree upon?

I'd love to hear from those other cloud vendors; I've not been able to
find any detail on how their SEV(-ES) systems actually work.

However, this aims to be just a comms mechanism to pass that secret;
so it's pretty low down in the stack and is there for them to use -
hopefully it's general enough.
(An interesting question is what exactly gets passed in this key and
what it means).

All the contentious stuff I've seen seems to be further up the stack - like
who does the attestation and where they get the secrets and how they
know what a valid measurement looks like.

Dave

> Thx.
> 
> -- 
> Regards/Gruss,
>     Boris.
> 
> SUSE Software Solutions Germany GmbH, GF: Ivo Totev, HRB 36809, AG Nürnberg
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH v6 0/5] Allow guest access to EFI confidential computing secret area

[Borislav Petkov]

On Wed, Jan 05, 2022 at 08:07:04PM +0000, Dr. David Alan Gilbert wrote:
> I thought I saw something in their patch series where they also had a
> secret that got passed down from EFI?

Probably. I've seen so many TDX patchsets so that I'm completely
confused what is what.

> As I remember they had it with an ioctl and something; but it felt to
> me if it would be great if it was shared.

I guess we could try to share

https://lore.kernel.org/r/20211210154332.11526-28-brijesh.singh@amd.com

for SNP and TDX.

> I'd love to hear from those other cloud vendors; I've not been able to
> find any detail on how their SEV(-ES) systems actually work.

Same here.

> However, this aims to be just a comms mechanism to pass that secret;
> so it's pretty low down in the stack and is there for them to use -
> hopefully it's general enough.

Exactly!

> (An interesting question is what exactly gets passed in this key and
> what it means).
> 
> All the contentious stuff I've seen seems to be further up the stack - like
> who does the attestation and where they get the secrets and how they
> know what a valid measurement looks like.

It would be much much better if all the parties involved would sit down
and decide on a common scheme so that implementation can be shared but
getting everybody to agree is likely hard...

-- 
Regards/Gruss,
    Boris.

SUSE Software Solutions Germany GmbH, GF: Ivo Totev, HRB 36809, AG Nürnberg

Re: [PATCH v6 0/5] Allow guest access to EFI confidential computing secret area

[Peter Gonda]

On Fri, Jan 7, 2022 at 4:59 AM Borislav Petkov <bp@suse.de> wrote:
>
> On Wed, Jan 05, 2022 at 08:07:04PM +0000, Dr. David Alan Gilbert wrote:
> > I thought I saw something in their patch series where they also had a
> > secret that got passed down from EFI?
>
> Probably. I've seen so many TDX patchsets so that I'm completely
> confused what is what.
>
> > As I remember they had it with an ioctl and something; but it felt to
> > me if it would be great if it was shared.
>
> I guess we could try to share
>
> https://lore.kernel.org/r/20211210154332.11526-28-brijesh.singh@amd.com
>
> for SNP and TDX.
>
> > I'd love to hear from those other cloud vendors; I've not been able to
> > find any detail on how their SEV(-ES) systems actually work.
>
> Same here.
>
> > However, this aims to be just a comms mechanism to pass that secret;
> > so it's pretty low down in the stack and is there for them to use -
> > hopefully it's general enough.
>
> Exactly!
>
> > (An interesting question is what exactly gets passed in this key and
> > what it means).
> >
> > All the contentious stuff I've seen seems to be further up the stack - like
> > who does the attestation and where they get the secrets and how they
> > know what a valid measurement looks like.
>
> It would be much much better if all the parties involved would sit down
> and decide on a common scheme so that implementation can be shared but
> getting everybody to agree is likely hard...

I saw a request for other cloud provider input here. A little
background for our SEV VMs in GCE we rely on our vTPM for attestation,
we do this because of SEV security properties quoting from AMD being
to protect guests from a benign but vulnerable hypervisor. So a
benign/compliant hypervisor's vTPM wouldn't lie to the guest. So we
added a few bits in the PCRs to allow users to see their SEV status in
vTPM quotes.

It would be very interesting to offer an attestation solution that
doesn't rely on our virtual TPM. But after reading through this cover
letter and the linked OVMF patches I am confused what's the high level
flow you are working towards? Are you loading in some OVMF using
LAUNCH_UPDATE_DATA, getting the measurement with LAUNCH_MEASURE, then
sending that to the customer who can then craft a "secret" (maybe say
SSH key) for injection with LAUNCH_SECRET? Thats sounds good but there
are a lot details left unattested there, how do you know you will boot
from the image loaded with the PSP into a known state? Do you have
some documentation I could read through to try and understand a little
more and apologies if I missed it.

>
> --
> Regards/Gruss,
>     Boris.
>
> SUSE Software Solutions Germany GmbH, GF: Ivo Totev, HRB 36809, AG Nürnberg
>

Re: [PATCH v6 0/5] Allow guest access to EFI confidential computing secret area

[Dov Murik]

On 07/01/2022 21:16, Peter Gonda wrote:
> On Fri, Jan 7, 2022 at 4:59 AM Borislav Petkov <bp@suse.de> wrote:
>>
>> On Wed, Jan 05, 2022 at 08:07:04PM +0000, Dr. David Alan Gilbert wrote:
>>> I thought I saw something in their patch series where they also had a
>>> secret that got passed down from EFI?
>>
>> Probably. I've seen so many TDX patchsets so that I'm completely
>> confused what is what.
>>
>>> As I remember they had it with an ioctl and something; but it felt to
>>> me if it would be great if it was shared.
>>
>> I guess we could try to share
>>
>> https://lore.kernel.org/r/20211210154332.11526-28-brijesh.singh@amd.com
>>
>> for SNP and TDX.
>>
>>> I'd love to hear from those other cloud vendors; I've not been able to
>>> find any detail on how their SEV(-ES) systems actually work.
>>
>> Same here.
>>
>>> However, this aims to be just a comms mechanism to pass that secret;
>>> so it's pretty low down in the stack and is there for them to use -
>>> hopefully it's general enough.
>>
>> Exactly!
>>
>>> (An interesting question is what exactly gets passed in this key and
>>> what it means).
>>>
>>> All the contentious stuff I've seen seems to be further up the stack - like
>>> who does the attestation and where they get the secrets and how they
>>> know what a valid measurement looks like.
>>
>> It would be much much better if all the parties involved would sit down
>> and decide on a common scheme so that implementation can be shared but
>> getting everybody to agree is likely hard...
> 
> I saw a request for other cloud provider input here. A little
> background for our SEV VMs in GCE we rely on our vTPM for attestation,
> we do this because of SEV security properties quoting from AMD being
> to protect guests from a benign but vulnerable hypervisor. So a
> benign/compliant hypervisor's vTPM wouldn't lie to the guest. So we
> added a few bits in the PCRs to allow users to see their SEV status in
> vTPM quotes.

Thanks Peter for explaining the GCE approach.  If I understand correctly,
if the hypervisor is malicious, it could lie to the vTPM and set those
"few bits" so it'll look like the VM is running with SEV enabled (whereas
in fact it is running without memory encryption).  But that's outside the
scope, right?


> 
> It would be very interesting to offer an attestation solution that
> doesn't rely on our virtual TPM. But after reading through this cover
> letter and the linked OVMF patches I am confused what's the high level
> flow you are working towards? Are you loading in some OVMF using
> LAUNCH_UPDATE_DATA, getting the measurement with LAUNCH_MEASURE, then
> sending that to the customer who can then craft a "secret" (maybe say
> SSH key) for injection with LAUNCH_SECRET? Thats sounds good but there
> are a lot details left unattested there, how do you know you will boot
> from the image loaded with the PSP into a known state? Do you have
> some documentation I could read through to try and understand a little
> more and apologies if I missed it.
> 

We rely on the OvmfPkg/AmdSev build of OVMF, which is a stripped down
firmware that can boot only in one of two ways:

a. direct boot to kernel/initrd/cmdline whose hashes are included in
   the LAUNCH_MEASURE data (see [1], [2], [3]).
b. boot to embedded grub (in OVMF) which decrypts an encrypted boot drive
   using a secret from LAUNCH_SECRET (see [4] and [5]).

For the current series (efi_secret kernel module), method (a) is the relevant
one.  The Cloud Provider boots a guest VM with OVMF, and -kernel/-initrd/-append. The
content of OVMF and the hashes of kernel/initrd/cmdline are measured in LAUNCH_MEASURE
and sent to the Guest Owner (=Customer).  If they match, the Guest Owner should perform 
a LAUNCH_SECRET and inject the secret to a designated SEV launch secrets page.  

Then the guest starts running; when the kernel starts the EFI driver saves the address
of this page (patch 1), and later exposes the secrets in this page as files in
securityfs (patch 3). The secrets can be used by the guest workload in whatever way
it needs (examples discussed above: ssh private key; API key to obtain secrets;
decryption key for some encrypted files; ...). 

Any attempt of a malicious cloud provider to modify OVMF, kernel, initrd, or cmdline
will cause the SEV measurement to be wrong. 

Note again that the proposed kernel patch series has nothing to do with the measurement
sequence -- it assumes the secrets page is already populated, and exposes it
to userspace as files.

Hope this explains our approach.


[1] https://www.youtube.com/watch?v=jzP8RlTRErk
[2] https://lore.kernel.org/qemu-devel/20210930054915.13252-1-dovmurik@linux.ibm.com/
[3] https://lore.kernel.org/qemu-devel/20211111100048.3299424-1-dovmurik@linux.ibm.com/

[4] https://www.youtube.com/watch?v=rCsIxzM6C_I
[5] https://blog.hansenpartnership.com/deploying-encrypted-images-for-confidential-computing/



-Dov

Re: [PATCH v6 0/5] Allow guest access to EFI confidential computing secret area

[Dr. David Alan Gilbert]

* Peter Gonda (pgonda@google.com) wrote:
> On Fri, Jan 7, 2022 at 4:59 AM Borislav Petkov <bp@suse.de> wrote:
> >
> > On Wed, Jan 05, 2022 at 08:07:04PM +0000, Dr. David Alan Gilbert wrote:
> > > I thought I saw something in their patch series where they also had a
> > > secret that got passed down from EFI?
> >
> > Probably. I've seen so many TDX patchsets so that I'm completely
> > confused what is what.
> >
> > > As I remember they had it with an ioctl and something; but it felt to
> > > me if it would be great if it was shared.
> >
> > I guess we could try to share
> >
> > https://lore.kernel.org/r/20211210154332.11526-28-brijesh.singh@amd.com
> >
> > for SNP and TDX.
> >
> > > I'd love to hear from those other cloud vendors; I've not been able to
> > > find any detail on how their SEV(-ES) systems actually work.
> >
> > Same here.
> >
> > > However, this aims to be just a comms mechanism to pass that secret;
> > > so it's pretty low down in the stack and is there for them to use -
> > > hopefully it's general enough.
> >
> > Exactly!
> >
> > > (An interesting question is what exactly gets passed in this key and
> > > what it means).
> > >
> > > All the contentious stuff I've seen seems to be further up the stack - like
> > > who does the attestation and where they get the secrets and how they
> > > know what a valid measurement looks like.
> >
> > It would be much much better if all the parties involved would sit down
> > and decide on a common scheme so that implementation can be shared but
> > getting everybody to agree is likely hard...
> 
> I saw a request for other cloud provider input here.

Thanks for the reply!

> A little
> background for our SEV VMs in GCE we rely on our vTPM for attestation,
> we do this because of SEV security properties quoting from AMD being
> to protect guests from a benign but vulnerable hypervisor. So a
> benign/compliant hypervisor's vTPM wouldn't lie to the guest. So we
> added a few bits in the PCRs to allow users to see their SEV status in
> vTPM quotes.

OK, so we're trying to protect from a malicious hypervisor - we don't
trust anything on the host (other than the CPU, and it's got to be
signing the attestation);  we don't think there's a way to do that with
a vTPM on plain SEV/SEV-ES

> It would be very interesting to offer an attestation solution that
> doesn't rely on our virtual TPM. But after reading through this cover
> letter and the linked OVMF patches I am confused what's the high level
> flow you are working towards? Are you loading in some OVMF using
> LAUNCH_UPDATE_DATA, getting the measurement with LAUNCH_MEASURE, then
> sending that to the customer who can then craft a "secret" (maybe say
> SSH key) for injection with LAUNCH_SECRET? Thats sounds good but there
> are a lot details left unattested there, how do you know you will boot
> from the image loaded with the PSP into a known state? Do you have
> some documentation I could read through to try and understand a little
> more and apologies if I missed it.

I'll defer to Dov's reply on that.

Dave

> >
> > --
> > Regards/Gruss,
> >     Boris.
> >
> > SUSE Software Solutions Germany GmbH, GF: Ivo Totev, HRB 36809, AG Nürnberg
> >
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK