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
---
v7 changes:
- Improve description of efi_secret module in Kconfig.
- Fix sparse warnings on pointer address space mismatch
(Reported-by: kernel test robot <[email protected]>)
v6: https://lore.kernel.org/linux-coco/[email protected]/
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/[email protected]/
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/[email protected]/
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 <[email protected]>)
v3: https://lore.kernel.org/linux-coco/[email protected]/
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/[email protected]
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/[email protected]/
RFC: https://lore.kernel.org/linux-coco/[email protected]/
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 | 19 +
drivers/virt/coco/efi_secret/Makefile | 2 +
drivers/virt/coco/efi_secret/efi_secret.c | 337 ++++++++++++++++++
include/linux/efi.h | 10 +
16 files changed, 647 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: 26291c54e111ff6ba87a164d85d4a4e134b7315c
--
2.25.1
On Wed, Feb 02, 2022 at 04:01:57AM +0000, Matthew Garrett wrote:
> On Tue, Feb 01, 2022 at 09:24:50AM -0500, James Bottomley wrote:
> > On Tue, 2022-02-01 at 14:50 +0100, Greg KH wrote:
> > > You all need to work together to come up with a unified place for
> > > this and stop making it platform-specific.
>
> We're talking about things that have massively different semantics.
I see lots of different platforms trying to provide access to their
"secure" firmware data to userspace in different ways. That feels to me
like they are the same thing that userspace would care about in a
unified way.
Unless we expeect userspace tools to have to be platform-specific for
all of this? That does not seem wise.
what am I missing here?
thanks,
greg k-h
On Wed, Feb 02, 2022 at 08:04:01AM +0000, Matthew Garrett wrote:
> On Wed, Feb 02, 2022 at 08:22:03AM +0100, Ard Biesheuvel wrote:
> > On Wed, 2 Feb 2022 at 08:10, Matthew Garrett <[email protected]> wrote:
> > > Which other examples are you thinking of? I think this conversation may
> > > have accidentally become conflated with a different prior one and now
> > > we're talking at cross purposes.
> >
> > This came up a while ago during review of one of the earlier revisions
> > of this patch set.
> >
> > https://lore.kernel.org/linux-efi/[email protected]/
> >
> > which describes another two variations on the theme, for pKVM guests
> > as well as Android bare metal.
>
> Oh, I see! That makes much more sense - sorry, I wasn't Cc:ed on that,
> so thought this was related to the efivars/Power secure boot. My
> apologies, sorry for the noise. In that case, given the apparent
> agreement between the patch owners that a consistent interface would
> work for them, I think I agree with Greg that we should strive for that.
> Given the described behaviour of the Google implementation, it feels
> like the semantics in this implementation would be sufficient for them
> as well, but having confirmation of that would be helpful.
>
> On the other hand, I also agree that a new filesystem for this is
> overkill. I did that for efivarfs and I think the primary lesson from
> that is that people who aren't familiar with the vfs shouldn't be
> writing filesystems. Securityfs seems entirely reasonable, and it's
> consistent with other cases where we expose firmware-provided data
> that's security relevant.
>
> The only thing I personally struggle with here is whether "coco" is the
> best name for it, and whether there are reasonable use cases that
> wouldn't be directly related to confidential computing (eg, if the
> firmware on a bare-metal platform had a mechanism for exposing secrets
> to the OS based on some specific platform security state, it would seem
> reasonable to expose it via this mechanism but it may not be what we'd
> normally think of as Confidential Computing).
>
> But I'd also say that while we only have one implementation currently
> sending patches, it's fine for the code to live in that implementation
> and then be abstracted out once we have another.
Well right now the Android code looks the cleanest and should be about
ready to be merged into my tree.
But I can almost guarantee that that interface is not what anyone else
wants to use, so if you think somehow that everyone else is going to
want to deal with a char device node and a simple mmap, with a DT
description of the thing, hey, I'm all for it :)
Seriously, people need to come up with something sane or this is going
to be a total mess.
thanks,
greg k-h
On Wed, Feb 02, 2022 at 08:05:23AM +0100, Greg KH wrote:
> I see different platform patches trying to stick these blobs in
> different locations and ways to access (securityfs, sysfs, char device
> node), which seems crazy to me. Why can't we at least pick one way to
> access these to start with, and then have the filesystem layout be
> platform-specific as needed, which will give the correct hints to
> userspace as to what it needs to do here?
Which other examples are you thinking of? I think this conversation may
have accidentally become conflated with a different prior one and now
we're talking at cross purposes.
On Tue, Feb 01, 2022 at 12:44:08PM +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.
>
> 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
Please see my comments on the powerpc version of this type of thing:
https://lore.kernel.org/r/[email protected]
You all need to work together to come up with a unified place for this
and stop making it platform-specific.
Until then, we can't take this.
sorry,
greg k-h
On Wed, Feb 02, 2022 at 07:10:02AM +0100, Greg KH wrote:
> On Wed, Feb 02, 2022 at 04:01:57AM +0000, Matthew Garrett wrote:
> > We're talking about things that have massively different semantics.
>
> I see lots of different platforms trying to provide access to their
> "secure" firmware data to userspace in different ways. That feels to me
> like they are the same thing that userspace would care about in a
> unified way.
EFI variables are largely for the OS to provide information to the
firmware, while this patchset is to provide information from the
firmware to the OS. I don't see why we'd expect to use the same userland
tooling for both.
In the broader case - I don't think we *can* use the same userland
tooling for everything. For example, the patches to add support for
manipulating the Power secure boot keys originally attempted to make it
look like efivars, but the underlying firmware semantics are
sufficiently different that even exposing the same kernel interface
wouldn't be a sufficient abstraction and userland would still need to
behave differently. Exposing an interface that looks consistent but
isn't is arguably worse for userland than exposing explicitly distinct
interfaces.
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 <[email protected]>
---
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
On Wed, Feb 02, 2022 at 06:54:43AM +0000, Matthew Garrett wrote:
> On Wed, Feb 02, 2022 at 07:10:02AM +0100, Greg KH wrote:
> > On Wed, Feb 02, 2022 at 04:01:57AM +0000, Matthew Garrett wrote:
> > > We're talking about things that have massively different semantics.
> >
> > I see lots of different platforms trying to provide access to their
> > "secure" firmware data to userspace in different ways. That feels to me
> > like they are the same thing that userspace would care about in a
> > unified way.
>
> EFI variables are largely for the OS to provide information to the
> firmware, while this patchset is to provide information from the
> firmware to the OS. I don't see why we'd expect to use the same userland
> tooling for both.
I totally agree, I'm not worried about EFI variables here, I don't know
why that came up.
> In the broader case - I don't think we *can* use the same userland
> tooling for everything. For example, the patches to add support for
> manipulating the Power secure boot keys originally attempted to make it
> look like efivars, but the underlying firmware semantics are
> sufficiently different that even exposing the same kernel interface
> wouldn't be a sufficient abstraction and userland would still need to
> behave differently. Exposing an interface that looks consistent but
> isn't is arguably worse for userland than exposing explicitly distinct
> interfaces.
So what does userspace really need here? Just the ability to find if
the platform has blobs that it cares about, and how to read/write them.
I see different platform patches trying to stick these blobs in
different locations and ways to access (securityfs, sysfs, char device
node), which seems crazy to me. Why can't we at least pick one way to
access these to start with, and then have the filesystem layout be
platform-specific as needed, which will give the correct hints to
userspace as to what it needs to do here?
thanks,
greg k-h
On 2/2/22 03:25, Greg KH wrote:
> On Wed, Feb 02, 2022 at 08:04:01AM +0000, Matthew Garrett wrote:
>> On Wed, Feb 02, 2022 at 08:22:03AM +0100, Ard Biesheuvel wrote:
>>> On Wed, 2 Feb 2022 at 08:10, Matthew Garrett <[email protected]> wrote:
>>>> Which other examples are you thinking of? I think this conversation may
>>>> have accidentally become conflated with a different prior one and now
>>>> we're talking at cross purposes.
>>> This came up a while ago during review of one of the earlier revisions
>>> of this patch set.
>>>
>>> https://lore.kernel.org/linux-efi/[email protected]/
>>>
>>> which describes another two variations on the theme, for pKVM guests
>>> as well as Android bare metal.
>> Oh, I see! That makes much more sense - sorry, I wasn't Cc:ed on that,
>> so thought this was related to the efivars/Power secure boot. My
>> apologies, sorry for the noise. In that case, given the apparent
>> agreement between the patch owners that a consistent interface would
>> work for them, I think I agree with Greg that we should strive for that.
>> Given the described behaviour of the Google implementation, it feels
>> like the semantics in this implementation would be sufficient for them
>> as well, but having confirmation of that would be helpful.
>>
>> On the other hand, I also agree that a new filesystem for this is
>> overkill. I did that for efivarfs and I think the primary lesson from
>> that is that people who aren't familiar with the vfs shouldn't be
>> writing filesystems. Securityfs seems entirely reasonable, and it's
>> consistent with other cases where we expose firmware-provided data
>> that's security relevant.
>>
>> The only thing I personally struggle with here is whether "coco" is the
>> best name for it, and whether there are reasonable use cases that
>> wouldn't be directly related to confidential computing (eg, if the
>> firmware on a bare-metal platform had a mechanism for exposing secrets
>> to the OS based on some specific platform security state, it would seem
>> reasonable to expose it via this mechanism but it may not be what we'd
>> normally think of as Confidential Computing).
>>
>> But I'd also say that while we only have one implementation currently
>> sending patches, it's fine for the code to live in that implementation
>> and then be abstracted out once we have another.
> Well right now the Android code looks the cleanest and should be about
> ready to be merged into my tree.
>
> But I can almost guarantee that that interface is not what anyone else
> wants to use, so if you think somehow that everyone else is going to
> want to deal with a char device node and a simple mmap, with a DT
> description of the thing, hey, I'm all for it :)
>
> Seriously, people need to come up with something sane or this is going
> to be a total mess.
>
Thanks for adding us to this discussion. I think somehow my last post
got html content and didn't make to mailing list, so am posting it
again. Sorry to those who are receiving it twice.
If I have understood the discussion right, the key idea discussed here
is to unify multiple different interfaces(this one, and [1]) exposing
secrets for confidential computing usecase via securityfs.
And the suggestion is to see if the proposed pseries interface [2] can
unify with the coco interface.
At high level, pseries interface is reading/writing/adding/deleting
variables using the sysfs interface, but the underlying semantics and
actual usecases are quite different.
The variables exposed via pseries proposed interface are:
* Variables owned by firmware as read-only.
* Variables owned by bootloader as read-only.
* Variables owned by OS and get updated as signed updates. These support
both read/write.
* Variables owned by OS and get directly updated(unsigned) eg config
information or some boot variables. These support both read/write.
It can be extended to support variables which contain secrets like
symmetric keys, are owned by OS and stored in platform keystore.
Naming convention wise also, there are differences like pseries
variables do not use GUIDs.
The initial patchset discusses secure boot usecase, but it would be
extended for other usecases as well.
First, I feel the purpose itself is different here. If we still
continue, I fear if we will get into similar situation as Matthew
mentioned in context of efivars -
"the patches to add support for
manipulating the Power secure boot keys originally attempted to make it
look like efivars, but the underlying firmware semantics are
sufficiently different that even exposing the same kernel interface
wouldn't be a sufficient abstraction and userland would still need to
behave differently. Exposing an interface that looks consistent but
isn't is arguably worse for userland than exposing explicitly distinct
interfaces."
With that, I believe the scope of pseries interface is different and
much broader than being discussed here. So, I wonder if it would be
better to still keep pseries interface separate from this and have its
own platform specific interface.
I would be happy to hear the ideas.
[1] https://lore.kernel.org/linux-efi/[email protected]/
[2] https://lore.kernel.org/all/[email protected]/
Thanks & Regards,
- Nayna