| Message ID | 20210628183431.953934-1-dovmurik@linux.ibm.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Allow access to confidential computing secret area | expand |
Just a couple of notes below: On Mon, Jun 28, 2021 at 06:34:28PM +0000, Dov Murik wrote: > Confidential computing 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. > > Support for secret injection is already available in OVMF (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 copies the secrets from the EFI-provided memory to > kernel reserved memory, and optionally exposes them to userspace via > securityfs using a new sev_secret kernel module. > > The first patch in efi/libstub copies the secret area from the EFI > memory to specially allocated memory; the second patch reserves that > memory block; and the third patch introduces the new sev_secret module > that exposes the content of the secret entries as securityfs files, and > allows clearing out secrets with a file unlink interface. > > This has been tested with AMD SEV guests, but the kernel side of > handling the secret area has no SEV-specific dependencies, and therefore > should be usable for any confidential computing hardware that can > publish the secret area via the standard EFI config table entry. > > Here is a simple example for usage of the sev_secret module in a guest to which > secrets were injected during launch: That's all fine and good but I miss the "why" in this explanation. I.e., a proper use case of a guest owner providing those sekrits to the guest would be good. > > # modprobe sev_secret > # ls -la /sys/kernel/security/sev_secret/ So that sysfs URL becomes an ABI. Shouldn't this be: /sys/kernel/security/coco/ instead which stands for "confidential computing" and contains all kinds of protected guest things. TDX might wanna do something similar there, for example. > 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/sev_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/sev_secret/e6f5a162-d67f-4750-a67c-5d065f2a9910 > > # ls -la /sys/kernel/security/sev_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 > > v2 changes: > - Add unlink support in sev_secret securityfs. > > > Dov Murik (3): > efi/libstub: Copy confidential computing secret area > efi: Reserve confidential computing secret area > virt: Add sev_secret module to expose confidential computing secrets > > drivers/firmware/efi/Makefile | 2 +- > drivers/firmware/efi/confidential-computing.c | 41 +++ > drivers/firmware/efi/efi.c | 5 + > drivers/firmware/efi/libstub/Makefile | 3 +- > .../efi/libstub/confidential-computing.c | 68 ++++ > drivers/firmware/efi/libstub/efi-stub.c | 2 + > drivers/firmware/efi/libstub/efistub.h | 2 + > drivers/firmware/efi/libstub/x86-stub.c | 2 + > 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 ++++++++++++++++++ > include/linux/efi.h | 11 + > 14 files changed, 448 insertions(+), 2 deletions(-) > create mode 100644 drivers/firmware/efi/confidential-computing.c > create mode 100644 drivers/firmware/efi/libstub/confidential-computing.c > 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 Those "confidential-computing.c" filenames are too long. I'd vote for coco.c. Same for your naming: efi_copy_confidential_computing_secret_area() - that is a wow and doesn't look like kernel code to me. :) Another example why it is too long: + {LINUX_EFI_CONFIDENTIAL_COMPUTING_SECRET_AREA_GUID, + &efi.confidential_computing_secret, + "ConfCompSecret"}, I'd do { LINUX_EFI_COCO_SECRET_AREA_GUID, &efi.coco_secret, "ConfCompSecret" },
Hi Boris, On 28/06/2021 22:28, Borislav Petkov wrote: > Just a couple of notes below: > > On Mon, Jun 28, 2021 at 06:34:28PM +0000, Dov Murik wrote: >> Confidential computing 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. >> >> Support for secret injection is already available in OVMF (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 copies the secrets from the EFI-provided memory to >> kernel reserved memory, and optionally exposes them to userspace via >> securityfs using a new sev_secret kernel module. >> >> The first patch in efi/libstub copies the secret area from the EFI >> memory to specially allocated memory; the second patch reserves that >> memory block; and the third patch introduces the new sev_secret module >> that exposes the content of the secret entries as securityfs files, and >> allows clearing out secrets with a file unlink interface. >> >> This has been tested with AMD SEV guests, but the kernel side of >> handling the secret area has no SEV-specific dependencies, and therefore >> should be usable for any confidential computing hardware that can >> publish the secret area via the standard EFI config table entry. >> >> Here is a simple example for usage of the sev_secret module in a guest to which >> secrets were injected during launch: > > That's all fine and good but I miss the "why" in this explanation. I.e., > a proper use case of a guest owner providing those sekrits to the guest > would be good. > OK, I'll add it in the cover letter; something along the lines of: An example would be 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 sev_secret filesystem and proceeds to decrypt the files into memory and then performs the needed computations on the content. 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. >> >> # modprobe sev_secret >> # ls -la /sys/kernel/security/sev_secret/ > > So that sysfs URL becomes an ABI. Shouldn't this be: > > /sys/kernel/security/coco/ > > instead which stands for "confidential computing" and contains all kinds > of protected guest things. TDX might wanna do something similar there, > for example. > On one hand, I agree. This entire series has no SEV-specific code (but I tested it only on SEV). On the other hand, secret injection mechanisms in SEV-SNP and TDX are different beasts than the one used here (SEV). In SEV the secrets must be injected at VM launch time; so when OVMF runs and kernel efistub runs the secrets are already there (patches 1+2). However, in SNP there's no secret injection at launch; (/me hand-waving) the guest can securely talk with the PSP hardware, check the attestation, and if OK then securely contact some Guest Owner secret provider to get the required secrets. Not sure it makes sense for the kernel to be part of this "getting secrets from secret provider and exposing them in securityfs". So maybe for regular SEV we'll use this sev_secret module to get one secret which will allow the guest to contact to the Guest Owner secret provider (and from here continue like SNP or TDX). Brijesh (AMD) also suggested collapsing the proposed sev_secret module into the new sev-guest module ("[PATCH Part1 RFC v3 22/22] virt: Add SEV-SNP guest driver", sent 2021-06-02), and the logic suggested here will be used when SNP is not active. Or taking a step back: Maybe the kernel should not try to unify SEV/SEV-SNP/TDX/PEF/s390x-SE. Each should have its own API. A userspace process will have to understand what is available and get the required info to run the application in a confidential environment. Or maybe we can find an API that fits all these confidential computing mechanisms and expose a unified API that hides the underlying implementation. (I'm not really sure - that's the reason this is an RFC series.) >> 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/sev_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/sev_secret/e6f5a162-d67f-4750-a67c-5d065f2a9910 >> >> # ls -la /sys/kernel/security/sev_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 >> >> v2 changes: >> - Add unlink support in sev_secret securityfs. >> >> >> Dov Murik (3): >> efi/libstub: Copy confidential computing secret area >> efi: Reserve confidential computing secret area >> virt: Add sev_secret module to expose confidential computing secrets >> >> drivers/firmware/efi/Makefile | 2 +- >> drivers/firmware/efi/confidential-computing.c | 41 +++ >> drivers/firmware/efi/efi.c | 5 + >> drivers/firmware/efi/libstub/Makefile | 3 +- >> .../efi/libstub/confidential-computing.c | 68 ++++ >> drivers/firmware/efi/libstub/efi-stub.c | 2 + >> drivers/firmware/efi/libstub/efistub.h | 2 + >> drivers/firmware/efi/libstub/x86-stub.c | 2 + >> 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 ++++++++++++++++++ >> include/linux/efi.h | 11 + >> 14 files changed, 448 insertions(+), 2 deletions(-) >> create mode 100644 drivers/firmware/efi/confidential-computing.c >> create mode 100644 drivers/firmware/efi/libstub/confidential-computing.c >> 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 > > Those "confidential-computing.c" filenames are too long. I'd vote for > coco.c. > When I wrote this I didn't yet encounter "coco" as an abbreviation. Now there's a linux-coco mailing list, but I saw no other mentions of it in the kernel (as an abbreviation for confidential computing). I agree that the full term is too long; I considered conf-comp (but in my mind "conf" is short for "configuration"). I used it in one place: "ConfCompSecret" in patch 2/3. If as a community we settle on coco / CoCo / COCO then I agree these should be renamed. (in QEMU they use CGS = Confidential Guest Support [1].) [1] https://lore.kernel.org/qemu-devel/20210202041315.196530-3-david@gibson.dropbear.id.au/ > Same for your naming: efi_copy_confidential_computing_secret_area() - > that is a wow and doesn't look like kernel code to me. :) > > Another example why it is too long: > > + {LINUX_EFI_CONFIDENTIAL_COMPUTING_SECRET_AREA_GUID, > + &efi.confidential_computing_secret, > + "ConfCompSecret"}, > > I'd do > > { LINUX_EFI_COCO_SECRET_AREA_GUID, &efi.coco_secret, "ConfCompSecret" }, > > I agree. Thanks, -Dov
On Tue, Jun 29, 2021 at 10:16:22AM +0300, Dov Murik wrote: > OK, I'll add it in the cover letter; something along the lines of: > > An example would be 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 > sev_secret filesystem and proceeds to decrypt the files into memory and > then performs the needed computations on the content. > > 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. Yap, much better, thanks! And that whole deal with the providing the secret this way is because you want to be starting the same guest image in the cloud over and over again but each guest owner would have their own decryption key which they would supply this way. Yap, good. > On one hand, I agree. This entire series has no SEV-specific code (but > I tested it only on SEV). > > On the other hand, secret injection mechanisms in SEV-SNP and TDX are > different beasts than the one used here (SEV). In SEV the secrets must > be injected at VM launch time; so when OVMF runs and kernel efistub runs > the secrets are already there (patches 1+2). However, in SNP there's no > secret injection at launch; (/me hand-waving) the guest can securely > talk with the PSP hardware, check the attestation, and if OK then > securely contact some Guest Owner secret provider to get the required > secrets. Not sure it makes sense for the kernel to be part of this > "getting secrets from secret provider and exposing them in securityfs". Which begs the question: why are you even doing this for only SEV instead of supporting SEV-SNP/TDX only? I'm under the impression that people should run only SNP and the equivalent of that in TDX, guests but not those earlier technologies which are lacking in some situations. > So maybe for regular SEV we'll use this sev_secret module to get one > secret which will allow the guest to contact to the Guest Owner secret > provider (and from here continue like SNP or TDX). Brijesh (AMD) also > suggested collapsing the proposed sev_secret module into the new > sev-guest module ("[PATCH Part1 RFC v3 22/22] virt: Add SEV-SNP guest > driver", sent 2021-06-02), Which reminds me - I still need to take a look at that one. > and the logic suggested here will be used when SNP is not active. > > Or taking a step back: Maybe the kernel should not try to unify > SEV/SEV-SNP/TDX/PEF/s390x-SE. Each should have its own API. A > userspace process will have to understand what is available and get the > required info to run the application in a confidential environment. > > Or maybe we can find an API that fits all these confidential computing > mechanisms and expose a unified API that hides the underlying > implementation. > > (I'm not really sure - that's the reason this is an RFC series.) I'm gravitating towards a common API so that userspace doesn't have to care. But that comes at the price of having to define that API properly so that it fits them all. And we all know how that bikeshedding works. :-\ > When I wrote this I didn't yet encounter "coco" as an abbreviation. Now > there's a linux-coco mailing list, but I saw no other mentions of it in > the kernel (as an abbreviation for confidential computing). That's what Joerg and me came up with - "coco" :-) > I agree that the full term is too long; I considered conf-comp (but in > my mind "conf" is short for "configuration"). I used it in one place: > "ConfCompSecret" in patch 2/3. > > If as a community we settle on coco / CoCo / COCO then I agree these > should be renamed. > > (in QEMU they use CGS = Confidential Guest Support [1].) That's not bad too. Thx.