[PATCH v8 0/4] Allow guest access to EFI confidential computing secret area

[PATCH v8 0/4] 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 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 third 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/secrets/coco
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

# hd /sys/kernel/security/secrets/coco/e6f5a162-d67f-4750-a67c-5d065f2a9910
00000000  74 68 65 73 65 2d 61 72  65 2d 74 68 65 2d 6b 61  |these-are-the-ka|
00000010  74 61 2d 73 65 63 72 65  74 73 00 01 02 03 04 05  |ta-secrets......|
00000020  06 07                                             |..|
00000022

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

# ls -la /sys/kernel/security/secrets/coco
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


---

v8 changes:
 - Change path of filesystem to <securityfs>/secrets/coco and fix the
   documentation accordingly (Thanks Gerd, Matthew)
 - Remove patch 2/5 (of v7) because the latest OVMF release (edk2-stable202202)
   already contains the fix to mark the launch secret page as EFI_RESERVED_TYPE.

v7: https://lore.kernel.org/linux-coco/20220201124413.1093099-1-dovmurik@linux.ibm.com/
v7 changes:
 - Improve description of efi_secret module in Kconfig.
 - Fix sparse warnings on pointer address space mismatch
   (Reported-by: kernel test robot <lkp@intel.com>)

v6: https://lore.kernel.org/linux-coco/20211129114251.3741721-1-dovmurik@linux.ibm.com/
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 (4):
  efi: Save location of EFI confidential computing 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 secrets/coco documentation

 Documentation/ABI/testing/securityfs-secrets-coco |  51 +++
 Documentation/security/index.rst                  |   1 +
 Documentation/security/secrets/coco.rst           | 103 ++++++
 Documentation/security/secrets/index.rst          |   9 +
 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/virt/Kconfig                              |   3 +
 drivers/virt/Makefile                             |   1 +
 drivers/virt/coco/efi_secret/Kconfig              |  19 ++
 drivers/virt/coco/efi_secret/Makefile             |   2 +
 drivers/virt/coco/efi_secret/efi_secret.c         | 337 ++++++++++++++++++++
 include/linux/efi.h                               |  10 +
 15 files changed, 620 insertions(+)
 create mode 100644 Documentation/ABI/testing/securityfs-secrets-coco
 create mode 100644 Documentation/security/secrets/coco.rst
 create mode 100644 Documentation/security/secrets/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: 7e57714cd0ad2d5bb90e50b5096a0e671dec1ef3
-- 
2.25.1

[PATCH v8 1/4] 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>
Reviewed-by: Gerd Hoffmann <kraxel@redhat.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 7de3f5b6e8d0..378d044b2463 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 ccd4d3f91c98..771d4cd06b56 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -405,6 +405,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)
@@ -596,6 +597,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;
@@ -1335,4 +1337,12 @@ extern void efifb_setup_from_dmi(struct screen_info *si, const char *opt);
 static inline void efifb_setup_from_dmi(struct screen_info *si, const char *opt) { }
 #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 v8 2/4] 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 "secrets/coco" 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 "secrets/coco" 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>
---
 Documentation/ABI/testing/securityfs-secrets-coco |  51 +++
 drivers/virt/Kconfig                              |   3 +
 drivers/virt/Makefile                             |   1 +
 drivers/virt/coco/efi_secret/Kconfig              |  16 +
 drivers/virt/coco/efi_secret/Makefile             |   2 +
 drivers/virt/coco/efi_secret/efi_secret.c         | 337 ++++++++++++++++++++
 6 files changed, 410 insertions(+)

diff --git a/Documentation/ABI/testing/securityfs-secrets-coco b/Documentation/ABI/testing/securityfs-secrets-coco
new file mode 100644
index 000000000000..f2b6909155f9
--- /dev/null
+++ b/Documentation/ABI/testing/securityfs-secrets-coco
@@ -0,0 +1,51 @@
+What:		security/secrets/coco
+Date:		February 2022
+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>/secrets/coco,
+		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/secrets/coco
+		  -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/secrets/coco/e6f5a162-d67f-4750-a67c-5d065f2a9910
+		  the-content-of-the-secret-data
+
+		Wiping a secret by unlinking a file::
+
+		  # rm /sys/kernel/security/secrets/coco/e6f5a162-d67f-4750-a67c-5d065f2a9910
+		  # ls -l /sys/kernel/security/secrets/coco
+		  -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..4404d198f3b2
--- /dev/null
+++ b/drivers/virt/coco/efi_secret/Kconfig
@@ -0,0 +1,16 @@
+# 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.
+	  The EFI secret area is a memory area designated by the firmware for
+	  confidential computing secret injection (for example for AMD SEV
+	  guests).  The driver exposes the secrets as files in
+	  <securityfs>/secrets/coco.  Files can be read and deleted (deleting
+	  a file wipes the secret from memory).
+
+	  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..06aa0eed253e
--- /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) 2022 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 "secrets/coco" 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 <linux/cacheflush.h>
+
+#define EFI_SECRET_NUM_FILES 64
+
+struct efi_secret {
+	struct dentry *secrets_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->secrets_dir);
+	s->secrets_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->secrets_dir = NULL;
+	s->fs_dir = NULL;
+	memset(s->fs_files, 0, sizeof(s->fs_files));
+
+	dent = securityfs_create_dir("secrets", NULL);
+	if (IS_ERR(dent)) {
+		pr_err("Error creating secrets securityfs directory entry err=%ld\n",
+		       PTR_ERR(dent));
+		return PTR_ERR(dent);
+	}
+	s->secrets_dir = dent;
+
+	dent = securityfs_create_dir("coco", s->secrets_dir);
+	if (IS_ERR(dent)) {
+		pr_err("Error creating coco 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 = (void __force *)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 securitfs secrets/coco\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 v8 3/4] 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>/secrets/coco 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>
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/firmware/efi/Makefile        |  1 +
 drivers/firmware/efi/coco.c          | 58 ++++++++++++++++++++
 drivers/virt/coco/efi_secret/Kconfig |  3 +
 3 files changed, 62 insertions(+)

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..f8efd240ab05
--- /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 = (void __force *)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((void __iomem *)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 4404d198f3b2..dc8da2921e36 100644
--- a/drivers/virt/coco/efi_secret/Kconfig
+++ b/drivers/virt/coco/efi_secret/Kconfig
@@ -14,3 +14,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 v8 4/4] docs: security: Add secrets/coco 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>
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
---
 Documentation/security/index.rst         |   1 +
 Documentation/security/secrets/coco.rst  | 103 ++++++++++++++++++++
 Documentation/security/secrets/index.rst |   9 ++
 3 files changed, 113 insertions(+)

diff --git a/Documentation/security/index.rst b/Documentation/security/index.rst
index 16335de04e8c..6ed8d2fa6f9e 100644
--- a/Documentation/security/index.rst
+++ b/Documentation/security/index.rst
@@ -17,3 +17,4 @@ Security Documentation
    tpm/index
    digsig
    landlock
+   secrets/index
diff --git a/Documentation/security/secrets/coco.rst b/Documentation/security/secrets/coco.rst
new file mode 100644
index 000000000000..262e7abb1b24
--- /dev/null
+++ b/Documentation/security/secrets/coco.rst
@@ -0,0 +1,103 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+==============================
+Confidential Computing secrets
+==============================
+
+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
+secrets to userspace applications via securityfs.  The details of the
+efi_secret filesystem interface are in [secrets-coco-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/secrets/coco
+	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
+
+	# hd /sys/kernel/security/secrets/coco/e6f5a162-d67f-4750-a67c-5d065f2a9910
+	00000000  74 68 65 73 65 2d 61 72  65 2d 74 68 65 2d 6b 61  |these-are-the-ka|
+	00000010  74 61 2d 73 65 63 72 65  74 73 00 01 02 03 04 05  |ta-secrets......|
+	00000020  06 07                                             |..|
+	00000022
+
+	# rm /sys/kernel/security/secrets/coco/e6f5a162-d67f-4750-a67c-5d065f2a9910
+
+	# ls -la /sys/kernel/security/secrets/coco
+	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
+.. [secrets-coco-abi] Documentation/ABI/testing/securityfs-secrets-coco
+.. [sev-api-spec] https://www.amd.com/system/files/TechDocs/55766_SEV-KM_API_Specification.pdf
diff --git a/Documentation/security/secrets/index.rst b/Documentation/security/secrets/index.rst
new file mode 100644
index 000000000000..ced34e9c43bd
--- /dev/null
+++ b/Documentation/security/secrets/index.rst
@@ -0,0 +1,9 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=====================
+Secrets documentation
+=====================
+
+.. toctree::
+
+   coco
-- 
2.25.1

Re: [PATCH v8 3/4] efi: Load efi_secret module if EFI secret area is populated

[Ard Biesheuvel]

On Mon, 28 Feb 2022 at 12:43, Dov Murik <dovmurik@linux.ibm.com> wrote:
>
> 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>/secrets/coco 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>
> Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>

It would be better to simply expose a platform device and associated
driver, instead of hooking into the module machinery directly.

We already do something similar for the EFI rtc and the efivars
subsystem, using platform_device_register_simple()


> ---
>  drivers/firmware/efi/Makefile        |  1 +
>  drivers/firmware/efi/coco.c          | 58 ++++++++++++++++++++
>  drivers/virt/coco/efi_secret/Kconfig |  3 +
>  3 files changed, 62 insertions(+)
>
> 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..f8efd240ab05
> --- /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 = (void __force *)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((void __iomem *)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 4404d198f3b2..dc8da2921e36 100644
> --- a/drivers/virt/coco/efi_secret/Kconfig
> +++ b/drivers/virt/coco/efi_secret/Kconfig
> @@ -14,3 +14,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
>

Re: [PATCH v8 3/4] efi: Load efi_secret module if EFI secret area is populated

[Dov Murik]

On 28/02/2022 14:49, Ard Biesheuvel wrote:
> On Mon, 28 Feb 2022 at 12:43, Dov Murik <dovmurik@linux.ibm.com> wrote:
>>
>> 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>/secrets/coco 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>
>> Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
> 
> It would be better to simply expose a platform device and associated
> driver, instead of hooking into the module machinery directly.
> 
> We already do something similar for the EFI rtc and the efivars
> subsystem, using platform_device_register_simple()
> 

Thanks Ard, I'll look into this.

I hope the mechanism you suggest allows me to perform complex check to
see if the device is really there (in this case: check for an efi
variable, map memory as encrypted, verify it starts with a specific GUID
-- everything before request_module() in the code below).

-Dov


> 
>> ---
>>  drivers/firmware/efi/Makefile        |  1 +
>>  drivers/firmware/efi/coco.c          | 58 ++++++++++++++++++++
>>  drivers/virt/coco/efi_secret/Kconfig |  3 +
>>  3 files changed, 62 insertions(+)
>>
>> 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..f8efd240ab05
>> --- /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 = (void __force *)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((void __iomem *)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 4404d198f3b2..dc8da2921e36 100644
>> --- a/drivers/virt/coco/efi_secret/Kconfig
>> +++ b/drivers/virt/coco/efi_secret/Kconfig
>> @@ -14,3 +14,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
>>

Re: [PATCH v8 3/4] efi: Load efi_secret module if EFI secret area is populated

[Ard Biesheuvel]

On Mon, 28 Feb 2022 at 14:07, Dov Murik <dovmurik@linux.ibm.com> wrote:
>
>
>
> On 28/02/2022 14:49, Ard Biesheuvel wrote:
> > On Mon, 28 Feb 2022 at 12:43, Dov Murik <dovmurik@linux.ibm.com> wrote:
> >>
> >> 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>/secrets/coco 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>
> >> Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
> >
> > It would be better to simply expose a platform device and associated
> > driver, instead of hooking into the module machinery directly.
> >
> > We already do something similar for the EFI rtc and the efivars
> > subsystem, using platform_device_register_simple()
> >
>
> Thanks Ard, I'll look into this.
>
> I hope the mechanism you suggest allows me to perform complex check to
> see if the device is really there (in this case: check for an efi
> variable, map memory as encrypted, verify it starts with a specific GUID
> -- everything before request_module() in the code below).
>

There is the device part and the driver part. Some of this belongs in
the core code that registers the platform device, and some of it
belongs in the driver. At this point, it probably does not matter that
much which side does what, as the platform driver simply probes and
can perform whatever check it needs, as long as it can back out
gracefully (although I understand that, in this particular case, there
are reasons why the driver may decide to wipe the secret)

Re: [PATCH v8 2/4] virt: Add efi_secret module to expose confidential computing secrets

[Gerd Hoffmann]

On Mon, Feb 28, 2022 at 11:42:52AM +0000, Dov Murik wrote:
> 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 "secrets/coco" 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 "secrets/coco" 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>

Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>

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

[Borislav Petkov]

On Mon, Feb 28, 2022 at 11:42:50AM +0000, 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.

Right, so this thing.

Tom and I were talking about SEV* guest debugging today and I believe
there might be another use case for this: SEV-ES guests cannot find out
from an attestation report - like SNP guests can - whether they're being
debugged or not so it would be very helpful if the fact that a -ES guest
is being debugged, could be supplied through such a secrets blob.

Because then, when I'm singlestepping the guest with gdb over the
gdbstub, the guest could determine based on those guest-owner previously
injected secrets whether it should allow debugging or not.

And this is where your set comes in.

However, I'm wondering if - instead of defining your own secrets structs
etc - you could use the SNP confidential computing blob machinery the
SNP set is adding. In particular:

https://lore.kernel.org/all/20220307213356.2797205-30-brijesh.singh@amd.com/

And you're adding another GUID but maybe you could simply use the SNP
thing called EFI_CC_BLOB_GUID and mimick that layout.

That should unify things more. And then guest kernel code could query
the blob also for debugging policy and so on.

Thoughts, opinions?

-- 
Regards/Gruss,
    Boris.

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

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

[Dov Murik]

Hello Boris,

On 24/03/2022 18:33, Borislav Petkov wrote:
> On Mon, Feb 28, 2022 at 11:42:50AM +0000, 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.
> 
> Right, so this thing.
> 
> Tom and I were talking about SEV* guest debugging today and I believe
> there might be another use case for this: SEV-ES guests cannot find out
> from an attestation report - like SNP guests can - whether they're being
> debugged or not so it would be very helpful if the fact that a -ES guest
> is being debugged, could be supplied through such a secrets blob.
> 
> Because then, when I'm singlestepping the guest with gdb over the
> gdbstub, the guest could determine based on those guest-owner previously
> injected secrets whether it should allow debugging or not.
> 

Let's see if I understand this correctly:

You want the guest to know if the its own SEV VM policy allows
debugging.  And that flag has to be trusted -- so passed from the Guest
Owner in a secure channel (otherwise the host could set it to
ALLOW_DEBUGGING even though the Guest Owner didn't approve that).



> And this is where your set comes in.
> 
> However, I'm wondering if - instead of defining your own secrets structs
> etc - you could use the SNP confidential computing blob machinery the
> SNP set is adding. In particular:
> 
> https://lore.kernel.org/all/20220307213356.2797205-30-brijesh.singh@amd.com/
> 
> And you're adding another GUID but maybe you could simply use the SNP
> thing called EFI_CC_BLOB_GUID and mimick that layout.
> 
> That should unify things more. And then guest kernel code could query
> the blob also for debugging policy and so on.
>

Maybe you could do that, but that will unify things that are not the
same, so I think it is the wrong approach here.

The SNP secrets are secrets generated by the AMD-SP and allow the guest
to communicate with the PSP.  There are exactly 4 of them (if I remember
correctly) and they are only AES-256-GCM keys (if I remember correctly).

On the other hand, the SEV launch secrets (which this series is about)
are secrets populated by the Guest Owner and are intended for the proper
operation of the applications in the guest (example use cases: luks
passphrase, secret API keys, file decryption keys, encrypted container
images keys, ...).

The SEV launch secrets area can also be read by grub [1], for example,
to fetch a luks passphrase from there (instead of from keyboard).
That's why its structure is generic.

[1] https://lists.gnu.org/archive/html/grub-devel/2022-02/msg00066.html


> Thoughts, opinions?
> 

I think Guest Owner can add a 1-byte predefined secret to the SEV secret
table, let's say an entry with GUID 2b91a212-b0e1-4816-b021-1e9991ddb6af
and value "\x01" to indicate debugging is allowed.

With the efi_secrets module, a 1-byte file called
/sys/kernel/security/secrets/coco/2b91a212-b0e1-4816-b021-1e9991ddb6af
will appear with "\x01" in its content.

This can indicate to the guest that debugging was permitted by the Guest
Owner.

If you want this unified in the kernel, maybe we can look for this entry
and set the relevant kernel variable.

-Dov

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

[Borislav Petkov]

Hi Dov,

On Tue, Mar 29, 2022 at 03:55:38PM +0300, Dov Murik wrote:
> Let's see if I understand this correctly:
> 
> You want the guest to know if the its own SEV VM policy allows
> debugging.  And that flag has to be trusted -- so passed from the Guest
> Owner in a secure channel (otherwise the host could set it to
> ALLOW_DEBUGGING even though the Guest Owner didn't approve that).

Yeah, and then dump all the guest memory and thus bypass the whole
memory encryption fun. So yeah, it should be encrypted and accessible
only to the guest and supplied by the guest owner.

> The SEV launch secrets area can also be read by grub [1], for example,
> to fetch a luks passphrase from there (instead of from keyboard).
> That's why its structure is generic.

Ok, fair enough.

> I think Guest Owner can add a 1-byte predefined secret to the SEV secret
> table, let's say an entry with GUID 2b91a212-b0e1-4816-b021-1e9991ddb6af
> and value "\x01" to indicate debugging is allowed.
> 
> With the efi_secrets module, a 1-byte file called
> /sys/kernel/security/secrets/coco/2b91a212-b0e1-4816-b021-1e9991ddb6af

I'd love it if that were more user-friendly:

/sys/kernel/security/secrets/coco/attributes

and there's:

debugging:1
...

and others.

> will appear with "\x01" in its content.
> 
> This can indicate to the guest that debugging was permitted by the Guest
> Owner.

But yeah, that should be the gist of the functionality.

> If you want this unified in the kernel, maybe we can look for this entry
> and set the relevant kernel variable.

So now that I think of it, it would be even nicer if the fact whether
guest debugging is allowed, were available to the guest *very early*
during boot. Because I think the most important cases where you'd want
to singlestep a SEV* guest with the qemu gdbstub is early guest kernel
boot code. So it would be cool if we'd have access to the debugging
setting that early.

Lemme have a look at your patches in detail to get an idea what's
happening there.

Thx.

-- 
Regards/Gruss,
    Boris.

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

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

[Dov Murik]

On 29/03/2022 21:30, Borislav Petkov wrote:

> 
> So now that I think of it, it would be even nicer if the fact whether
> guest debugging is allowed, were available to the guest *very early*
> during boot. Because I think the most important cases where you'd want
> to singlestep a SEV* guest with the qemu gdbstub is early guest kernel
> boot code. So it would be cool if we'd have access to the debugging
> setting that early.
> 
> Lemme have a look at your patches in detail to get an idea what's
> happening there.

Is efi_config_parse_tables() early enough?  That's where we learn for
the first time that the firmware has a launch-secrets area that we can
look at.

We can add there (say, next to the call to efi_tpm_eventlog_init()) a
code to:

1. map the secret area (ioremap_encrypted())
2. parse the table, look for the "sev debug enabled" GUID.
3. set the value of the kernel variable that we can later use anywhere.


Of course Ard might know about a better mechanism or place to do that.


-Dov

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

[Dov Murik]

On 29/03/2022 23:28, Dov Murik wrote:
> 
> 
> On 29/03/2022 21:30, Borislav Petkov wrote:
> 
>>
>> So now that I think of it, it would be even nicer if the fact whether
>> guest debugging is allowed, were available to the guest *very early*
>> during boot. Because I think the most important cases where you'd want
>> to singlestep a SEV* guest with the qemu gdbstub is early guest kernel
>> boot code. So it would be cool if we'd have access to the debugging
>> setting that early.
>>
>> Lemme have a look at your patches in detail to get an idea what's
>> happening there.
> 

After a night's sleep I figured out that an SEV guest cannot tell if a
value it's reading was (a) encrypted by the host using
KVM_SEV_LAUNCH_UPDATE_DATA, or (b) added using secret injection using
KVM_SEV_LAUNCH_SECRET.

The only difference is that if the host is using
KVM_SEV_LAUNCH_UPDATE_DATA, then it changes the measurement.  But maybe
for debugging scenarios we (= Guest Owner) don't care about the
measurement being correct.

If that's the case, we don't need a secure channel and secret injection.
You can use a simple "sev=debug" (or whatever) in the kernel
command-line to indicate your needs.


Did I miss something?


-Dov

Re: [PATCH v8 3/4] efi: Load efi_secret module if EFI secret area is populated

[Dov Murik]

Hello Ard,

On 28/02/2022 15:15, Ard Biesheuvel wrote:
> On Mon, 28 Feb 2022 at 14:07, Dov Murik <dovmurik@linux.ibm.com> wrote:
>>
>>
>>
>> On 28/02/2022 14:49, Ard Biesheuvel wrote:
>>> On Mon, 28 Feb 2022 at 12:43, Dov Murik <dovmurik@linux.ibm.com> wrote:
>>>>
>>>> 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>/secrets/coco 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>
>>>> Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
>>>
>>> It would be better to simply expose a platform device and associated
>>> driver, instead of hooking into the module machinery directly.
>>>
>>> We already do something similar for the EFI rtc and the efivars
>>> subsystem, using platform_device_register_simple()
>>>
>>
>> Thanks Ard, I'll look into this.
>>
>> I hope the mechanism you suggest allows me to perform complex check to
>> see if the device is really there (in this case: check for an efi
>> variable, map memory as encrypted, verify it starts with a specific GUID
>> -- everything before request_module() in the code below).
>>
> 
> There is the device part and the driver part. Some of this belongs in
> the core code that registers the platform device, and some of it
> belongs in the driver. At this point, it probably does not matter that
> much which side does what, as the platform driver simply probes and
> can perform whatever check it needs, as long as it can back out
> gracefully (although I understand that, in this particular case, there
> are reasons why the driver may decide to wipe the secret)

I finally got to implement this, it seems like it makes the code simple.
Thanks for the advice.

Just making sure I understand correctly: in this approach this we rely
on udev to load the efi_secret module (aliased as "platform:efi_secret")
and call its .probe() function?  If there's no udev, the module will not
be loaded automatically.  Did I understand that correctly?


In such a case, the only thing needed to add in efi.c is:


diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index 378d044b2463..b92eabc554e6 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -425,6 +425,9 @@ static int __init efisubsys_init(void)
        if (efi_enabled(EFI_DBG) && efi_enabled(EFI_PRESERVE_BS_REGIONS))
                efi_debugfs_init();

+       if (efi.coco_secret != EFI_INVALID_TABLE_ADDR)
+               platform_device_register_simple("efi_secret", 0, NULL, 0);
+
        return 0;

 err_remove_group:



Does this seem OK? (before I re-spin the whole series.)

Thanks,
-Dov

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

[Borislav Petkov]

On Wed, Mar 30, 2022 at 09:11:54AM +0300, Dov Murik wrote:
> If that's the case, we don't need a secure channel and secret injection.
> You can use a simple "sev=debug" (or whatever) in the kernel
> command-line to indicate your needs.

Yeah, that would work for a normal SEV guest.

However, if it is an -ES guest, you need to somehow tell it as the guest
owner: "hey you're being debugged and that's fine."

Because if you want to singlestep the thing, you're going to land in
the #VC handler and destroy registers so you want to save them first if
you're being debugged and then shovel them out to the host somehow. And
that's another question but first things first.

And "if you're being debugged" needs to be somehow told the guest
through a secure channel so that the HV doesn't go and simply enable
debugging by booting with "sev=debug" and bypass it all.

And SNP has access to the policy in the attestation report, says Tom, so
that's possible there.

So we need a way to add the debugging aspect to the measurement and be
able to recreate that measurement quickly so that a simple debugging
session of a kernel in a guest can work pretty much the same with a SEV*
guest.

I'm still digging the details tho...

-- 
Regards/Gruss,
    Boris.

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

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

[Dov Murik]

On 31/03/2022 12:19, Borislav Petkov wrote:
> On Wed, Mar 30, 2022 at 09:11:54AM +0300, Dov Murik wrote:
>> If that's the case, we don't need a secure channel and secret injection.
>> You can use a simple "sev=debug" (or whatever) in the kernel
>> command-line to indicate your needs.
> 
> Yeah, that would work for a normal SEV guest.
> 
> However, if it is an -ES guest, you need to somehow tell it as the guest
> owner: "hey you're being debugged and that's fine."
> 
> Because if you want to singlestep the thing, you're going to land in
> the #VC handler and destroy registers so you want to save them first if
> you're being debugged and then shovel them out to the host somehow. And
> that's another question but first things first.
> 
> And "if you're being debugged" needs to be somehow told the guest
> through a secure channel so that the HV doesn't go and simply enable
> debugging by booting with "sev=debug" and bypass it all.
> 

Note that the HV can also start the VM with SEV completely turned off.
Similarly, it can enable debugging and "fool" the guest.  Of course all
this tricks will affect the measurement, and then the Guest Owner will
know that something is wrong and won't inject the secrets.  If you don't
rely on secret injection anyway, then I think a kernel command-line
param is good enough.  (I might be missing a scenario though)


Maybe you can use KVM_SEV_GET_ATTESTATION_REPORT (ask the host to do it
for you).  But I think it returns only the launch digest, and you can't
figure out the SEV Policy field from it.



> And SNP has access to the policy in the attestation report, says Tom, so
> that's possible there.

True. But not in really early boot? This is all in the sev-guest
platform driver.


> 
> So we need a way to add the debugging aspect to the measurement and be
> able to recreate that measurement quickly so that a simple debugging
> session of a kernel in a guest can work pretty much the same with a SEV*
> guest.
> 
> I'm still digging the details tho...
> 

Re: [PATCH v8 3/4] efi: Load efi_secret module if EFI secret area is populated

[Ard Biesheuvel]

On Thu, 31 Mar 2022 at 11:05, Dov Murik <dovmurik@linux.ibm.com> wrote:
>
> Hello Ard,
>
> On 28/02/2022 15:15, Ard Biesheuvel wrote:
> > On Mon, 28 Feb 2022 at 14:07, Dov Murik <dovmurik@linux.ibm.com> wrote:
> >>
> >>
> >>
> >> On 28/02/2022 14:49, Ard Biesheuvel wrote:
> >>> On Mon, 28 Feb 2022 at 12:43, Dov Murik <dovmurik@linux.ibm.com> wrote:
> >>>>
> >>>> 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>/secrets/coco 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>
> >>>> Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
> >>>
> >>> It would be better to simply expose a platform device and associated
> >>> driver, instead of hooking into the module machinery directly.
> >>>
> >>> We already do something similar for the EFI rtc and the efivars
> >>> subsystem, using platform_device_register_simple()
> >>>
> >>
> >> Thanks Ard, I'll look into this.
> >>
> >> I hope the mechanism you suggest allows me to perform complex check to
> >> see if the device is really there (in this case: check for an efi
> >> variable, map memory as encrypted, verify it starts with a specific GUID
> >> -- everything before request_module() in the code below).
> >>
> >
> > There is the device part and the driver part. Some of this belongs in
> > the core code that registers the platform device, and some of it
> > belongs in the driver. At this point, it probably does not matter that
> > much which side does what, as the platform driver simply probes and
> > can perform whatever check it needs, as long as it can back out
> > gracefully (although I understand that, in this particular case, there
> > are reasons why the driver may decide to wipe the secret)
>
> I finally got to implement this, it seems like it makes the code simple.
> Thanks for the advice.
>
> Just making sure I understand correctly: in this approach this we rely
> on udev to load the efi_secret module (aliased as "platform:efi_secret")
> and call its .probe() function?  If there's no udev, the module will not
> be loaded automatically.  Did I understand that correctly?
>

Apologies, I am swamped in email and only spotted this now.

This looks good to me: is this what you implemented in the end?

Re: [PATCH v8 3/4] efi: Load efi_secret module if EFI secret area is populated

[Dov Murik]

On 12/04/2022 16:08, Ard Biesheuvel wrote:
> On Thu, 31 Mar 2022 at 11:05, Dov Murik <dovmurik@linux.ibm.com> wrote:
>>
>> Hello Ard,
>>
>> On 28/02/2022 15:15, Ard Biesheuvel wrote:
>>> On Mon, 28 Feb 2022 at 14:07, Dov Murik <dovmurik@linux.ibm.com> wrote:
>>>>
>>>>
>>>>
>>>> On 28/02/2022 14:49, Ard Biesheuvel wrote:
>>>>> On Mon, 28 Feb 2022 at 12:43, Dov Murik <dovmurik@linux.ibm.com> wrote:
>>>>>>
>>>>>> 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>/secrets/coco 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>
>>>>>> Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
>>>>>
>>>>> It would be better to simply expose a platform device and associated
>>>>> driver, instead of hooking into the module machinery directly.
>>>>>
>>>>> We already do something similar for the EFI rtc and the efivars
>>>>> subsystem, using platform_device_register_simple()
>>>>>
>>>>
>>>> Thanks Ard, I'll look into this.
>>>>
>>>> I hope the mechanism you suggest allows me to perform complex check to
>>>> see if the device is really there (in this case: check for an efi
>>>> variable, map memory as encrypted, verify it starts with a specific GUID
>>>> -- everything before request_module() in the code below).
>>>>
>>>
>>> There is the device part and the driver part. Some of this belongs in
>>> the core code that registers the platform device, and some of it
>>> belongs in the driver. At this point, it probably does not matter that
>>> much which side does what, as the platform driver simply probes and
>>> can perform whatever check it needs, as long as it can back out
>>> gracefully (although I understand that, in this particular case, there
>>> are reasons why the driver may decide to wipe the secret)
>>
>> I finally got to implement this, it seems like it makes the code simple.
>> Thanks for the advice.
>>
>> Just making sure I understand correctly: in this approach this we rely
>> on udev to load the efi_secret module (aliased as "platform:efi_secret")
>> and call its .probe() function?  If there's no udev, the module will not
>> be loaded automatically.  Did I understand that correctly?
>>
> 
> Apologies, I am swamped in email and only spotted this now.
> 
> This looks good to me: is this what you implemented in the end?

Yes indeed. So this old patch 3 was replaced by this much simpler code
in the next iteration (v9):



diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index 378d044b2463..b92eabc554e6 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -425,6 +425,9 @@ static int __init efisubsys_init(void)
 	if (efi_enabled(EFI_DBG) && efi_enabled(EFI_PRESERVE_BS_REGIONS))
 		efi_debugfs_init();
 
+	if (efi.coco_secret != EFI_INVALID_TABLE_ADDR)
+		platform_device_register_simple("efi_secret", 0, NULL, 0);
+
 	return 0;
 
 err_remove_group:




(this is were I missed the #ifdef CONFIG_EFI_COCO_SECRET ).

Thanks again for the suggestion.

-Dov