Re: [RFC PATCH v2 3/3] virt: Add sev_secret module to expose confidential computing secrets

On 28/06/2021 22:30, Borislav Petkov wrote:

On Mon, Jun 28, 2021 at 06:34:31PM +0000, Dov Murik wrote:

quoted

The new sev_secret module exposes the confidential computing secret area
via securityfs interface.

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

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

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

Signed-off-by: Dov Murik <redacted>
---
 drivers/virt/Kconfig                 |   2 +
 drivers/virt/Makefile                |   1 +
 drivers/virt/sev_secret/Kconfig      |  11 +
 drivers/virt/sev_secret/Makefile     |   2 +
 drivers/virt/sev_secret/sev_secret.c | 298 +++++++++++++++++++++++++++
 5 files changed, 314 insertions(+)
 create mode 100644 drivers/virt/sev_secret/Kconfig
 create mode 100644 drivers/virt/sev_secret/Makefile
 create mode 100644 drivers/virt/sev_secret/sev_secret.c

Same question here: maybe have 

drivers/virt/coco/

and put all coco guest stuff in there.

I agree, according to what comes up of the conversation we have in
replies to the cover letter.

quoted

diff --git a/drivers/virt/Kconfig b/drivers/virt/Kconfig
index 8061e8ef449f..c222cc625891 100644
--- a/drivers/virt/Kconfig
+++ b/drivers/virt/Kconfig

@@ -36,4 +36,6 @@ source "drivers/virt/vboxguest/Kconfig"
 source "drivers/virt/nitro_enclaves/Kconfig"
 
 source "drivers/virt/acrn/Kconfig"
+
+source "drivers/virt/sev_secret/Kconfig"
 endif

diff --git a/drivers/virt/Makefile b/drivers/virt/Makefile
index 3e272ea60cd9..0765e5418d1d 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-y				+= sev_secret/

diff --git a/drivers/virt/sev_secret/Kconfig b/drivers/virt/sev_secret/Kconfig
new file mode 100644
index 000000000000..4505526b8ef1
--- /dev/null
+++ b/drivers/virt/sev_secret/Kconfig

@@ -0,0 +1,11 @@
+# SPDX-License-Identifier: GPL-2.0-only
+config AMD_SEV_SECRET_SECURITYFS
+	tristate "AMD SEV secret area securityfs support"
+	depends on EFI

That probably needs to depend on CONFIG_AMD_MEM_ENCRYPT - otherwise
what's the point for it.

But not if it's a generic driver that is useful for other confidential
computing implementations.  Consider some non-encrypting guest memory
isolation mechanism where the host can't read most guest pages; this
module might be useful there too.

Also, isn't it a bit weird to depend on CONFIG_AMD_MEM_ENCRYPT but not
use any of its APIs?


-Dov