mbox series

[RFC,0/3] Allow access to confidential computing secret area

Message ID 20210513062634.2481118-1-dovmurik@linux.ibm.com (mailing list archive)
Headers show
Series Allow access to confidential computing secret area | expand

Message

Dov Murik May 13, 2021, 6:26 a.m. UTC
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.

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:

# modprobe sev_secret
# ls -la /sys/kernel/security/sev_secret
total 0
drwxr-xr-x 2 root root 0 May 12 18:03 .
drwxr-xr-x 3 root root 0 May 12 18:02 ..
-r--r----- 1 root root 0 May 12 18:03 736870e5-84f0-4973-92ec-06879ce3da0b
-r--r----- 1 root root 0 May 12 18:03 83c83f7f-1356-4975-8b7e-d3a0b54312c6
-r--r----- 1 root root 0 May 12 18:03 9553f55d-3da2-43ee-ab5d-ff17f78864d2
-r--r----- 1 root root 0 May 12 18:03 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                                     ..


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


Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ashish Kalra <ashish.kalra@amd.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: James Morris <jmorris@namei.org>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: linux-efi@vger.kernel.org
Cc: linux-security-module@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

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          | 260 ++++++++++++++++++
 include/linux/efi.h                           |  11 +
 14 files changed, 410 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


base-commit: c06a2ba62fc401b7aaefd23f5d0bc06d2457ccc1

Comments

Brijesh Singh May 14, 2021, 1:01 p.m. UTC | #1
Hi Dov,


On 5/13/21 1:26 AM, 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.
>
> 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:
>
> # modprobe sev_secret
> # ls -la /sys/kernel/security/sev_secret
> total 0
> drwxr-xr-x 2 root root 0 May 12 18:03 .
> drwxr-xr-x 3 root root 0 May 12 18:02 ..
> -r--r----- 1 root root 0 May 12 18:03 736870e5-84f0-4973-92ec-06879ce3da0b
> -r--r----- 1 root root 0 May 12 18:03 83c83f7f-1356-4975-8b7e-d3a0b54312c6
> -r--r----- 1 root root 0 May 12 18:03 9553f55d-3da2-43ee-ab5d-ff17f78864d2
> -r--r----- 1 root root 0 May 12 18:03 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                                     ..

I am adding a new virt driver to help get the attestation report for the
SEV-SNP guest. I understand they both are different, in case of the SEV
the attestation is already completed and we are simply exposing the
secret provided after the attestation to the userspace, whereas in SNP,
the userspace is querying the attestation and will probably derive keys
etc based on the attestation report. I am wondering if we should merge
both the SEV secret and SNP attestation query in a single driver ?
Should we cover usecases where SEV guest is not booted under the EFI ?
Also, it appears that this driver need to be manually loaded, should we
create a platform device so that the driver binds to platform device and
use the resource structure to find the location of the secret data?

I was trying to answer some of these questions SNP series. See these patches

https://marc.info/?l=kvm&m=161978514019741&w=2

https://marc.info/?l=kvm&m=161978514119744&w=2

https://marc.info/?l=kvm&m=161978514219751&w=2


>
> [1] https://github.com/tianocore/edk2/commit/01726b6d23d4
>
>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ashish Kalra <ashish.kalra@amd.com>
> Cc: Brijesh Singh <brijesh.singh@amd.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: James Morris <jmorris@namei.org>
> Cc: "Serge E. Hallyn" <serge@hallyn.com>
> Cc: linux-efi@vger.kernel.org
> Cc: linux-security-module@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
>
> 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          | 260 ++++++++++++++++++
>  include/linux/efi.h                           |  11 +
>  14 files changed, 410 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
>
>
> base-commit: c06a2ba62fc401b7aaefd23f5d0bc06d2457ccc1
Dov Murik May 20, 2021, 10:38 a.m. UTC | #2
Hi Brijesh,

On 14/05/2021 16:01, Brijesh Singh wrote:
> Hi Dov,
> 
> 
> On 5/13/21 1:26 AM, 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.
>>
>> 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:
>>
>> # modprobe sev_secret
>> # ls -la /sys/kernel/security/sev_secret
>> total 0
>> drwxr-xr-x 2 root root 0 May 12 18:03 .
>> drwxr-xr-x 3 root root 0 May 12 18:02 ..
>> -r--r----- 1 root root 0 May 12 18:03 736870e5-84f0-4973-92ec-06879ce3da0b
>> -r--r----- 1 root root 0 May 12 18:03 83c83f7f-1356-4975-8b7e-d3a0b54312c6
>> -r--r----- 1 root root 0 May 12 18:03 9553f55d-3da2-43ee-ab5d-ff17f78864d2
>> -r--r----- 1 root root 0 May 12 18:03 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                                     ..
> 
> I am adding a new virt driver to help get the attestation report for the
> SEV-SNP guest. I understand they both are different, in case of the SEV
> the attestation is already completed and we are simply exposing the
> secret provided after the attestation to the userspace, whereas in SNP,
> the userspace is querying the attestation and will probably derive keys
> etc based on the attestation report. 

I don't know a lot about SNP, but are we talking about the same secrets? SEV's LAUNCH_SECRET command allows the guest owner to inject any binary blob, though through the current OVMF implementation it is limited to one page.  These can be used for anything - encryption keys, private keys for signature, passphrases, etc. The SNP secret is used for communication with the PSP only? 

Can the two co-exist?


> I am wondering if we should merge
> both the SEV secret and SNP attestation query in a single driver ?

Generally I'm OK with this, but I want to support plain SEV as well. The current suggested snp-guest driver is only activated on SNP (if I read the code correctly).  Now the question is whether a general SEV (or confidential computing) driver makes sense? It looks like only parts of its functionality will be available depending on the actual hardware available.


> Should we cover usecases where SEV guest is not booted under the EFI ?

This sounds interesting; it might be useful for Kata were we try to minimize the overhead of the VM, so fewer moving parts is better.  So far we don't know how to boot an SEV guest without OVMF (=EFI).  Is this supported with QEMU (we currently inject SEV secrets using QEMU's sev-inject-launch-secret)?  Can you share such instructions?


> Also, it appears that this driver need to be manually loaded, should we
> create a platform device so that the driver binds to platform device and
> use the resource structure to find the location of the secret data?

I designed the sev-secret driver to be hardware-indepedent (maybe a better name would be confidential-computing-secret); it just looks for the declared secrets page and creates a userspace API to read the secrets by GUID.  In that sense, binding it to specific hardware will limit its usage with other platforms that can "prepare" the secrets in the correct place.



> 
> I was trying to answer some of these questions SNP series. See these patches
> 
> https://marc.info/?l=kvm&m=161978514019741&w=2
> 
> https://marc.info/?l=kvm&m=161978514119744&w=2
> 
> https://marc.info/?l=kvm&m=161978514219751&w=2
> 
> 
>>
>> [1] https://github.com/tianocore/edk2/commit/01726b6d23d4
>>
>>
>> Cc: Laszlo Ersek <lersek@redhat.com>
>> Cc: Ashish Kalra <ashish.kalra@amd.com>
>> Cc: Brijesh Singh <brijesh.singh@amd.com>
>> Cc: Tom Lendacky <thomas.lendacky@amd.com>
>> Cc: James Bottomley <jejb@linux.ibm.com>
>> Cc: Ard Biesheuvel <ardb@kernel.org>
>> Cc: James Morris <jmorris@namei.org>
>> Cc: "Serge E. Hallyn" <serge@hallyn.com>
>> Cc: linux-efi@vger.kernel.org
>> Cc: linux-security-module@vger.kernel.org
>> Cc: linux-kernel@vger.kernel.org
>>
>> 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          | 260 ++++++++++++++++++
>>  include/linux/efi.h                           |  11 +
>>  14 files changed, 410 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
>>
>>
>> base-commit: c06a2ba62fc401b7aaefd23f5d0bc06d2457ccc1
Dr. David Alan Gilbert May 20, 2021, 10:56 a.m. UTC | #3
* Brijesh Singh (brijesh.singh@amd.com) wrote:
> Hi Dov,
> 
> 
> On 5/13/21 1:26 AM, 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.
> >
> > 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:
> >
> > # modprobe sev_secret
> > # ls -la /sys/kernel/security/sev_secret
> > total 0
> > drwxr-xr-x 2 root root 0 May 12 18:03 .
> > drwxr-xr-x 3 root root 0 May 12 18:02 ..
> > -r--r----- 1 root root 0 May 12 18:03 736870e5-84f0-4973-92ec-06879ce3da0b
> > -r--r----- 1 root root 0 May 12 18:03 83c83f7f-1356-4975-8b7e-d3a0b54312c6
> > -r--r----- 1 root root 0 May 12 18:03 9553f55d-3da2-43ee-ab5d-ff17f78864d2
> > -r--r----- 1 root root 0 May 12 18:03 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                                     ..
> 
> I am adding a new virt driver to help get the attestation report for the
> SEV-SNP guest. I understand they both are different, in case of the SEV
> the attestation is already completed and we are simply exposing the
> secret provided after the attestation to the userspace, whereas in SNP,
> the userspace is querying the attestation and will probably derive keys
> etc based on the attestation report. I am wondering if we should merge
> both the SEV secret and SNP attestation query in a single driver ?
> Should we cover usecases where SEV guest is not booted under the EFI ?
> Also, it appears that this driver need to be manually loaded, should we
> create a platform device so that the driver binds to platform device and
> use the resource structure to find the location of the secret data?

The nice thing about Dov's device/file is that it's a simple text file
that userspace can then read the secret out of;  I'm not sure if there's
anything similar in SNP (or for that matter TDX, cc'ing in Andi)

Dave

> I was trying to answer some of these questions SNP series. See these patches
> 
> https://marc.info/?l=kvm&m=161978514019741&w=2
> 
> https://marc.info/?l=kvm&m=161978514119744&w=2
> 
> https://marc.info/?l=kvm&m=161978514219751&w=2
> 
> 
> >
> > [1] https://github.com/tianocore/edk2/commit/01726b6d23d4
> >
> >
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Cc: Ashish Kalra <ashish.kalra@amd.com>
> > Cc: Brijesh Singh <brijesh.singh@amd.com>
> > Cc: Tom Lendacky <thomas.lendacky@amd.com>
> > Cc: James Bottomley <jejb@linux.ibm.com>
> > Cc: Ard Biesheuvel <ardb@kernel.org>
> > Cc: James Morris <jmorris@namei.org>
> > Cc: "Serge E. Hallyn" <serge@hallyn.com>
> > Cc: linux-efi@vger.kernel.org
> > Cc: linux-security-module@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> >
> > 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          | 260 ++++++++++++++++++
> >  include/linux/efi.h                           |  11 +
> >  14 files changed, 410 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
> >
> >
> > base-commit: c06a2ba62fc401b7aaefd23f5d0bc06d2457ccc1
Andi Kleen May 20, 2021, 10:02 p.m. UTC | #4
On 5/20/2021 3:56 AM, Dr. David Alan Gilbert wrote:
> * Brijes
> The nice thing about Dov's device/file is that it's a simple text file
> that userspace can then read the secret out of;  I'm not sure if there's
> anything similar in SNP (or for that matter TDX, cc'ing in Andi)

In TDX there are two different mechanisms:

- One is a ACPI table (SVKL) that allows to pass small pieces of data 
like keys from the BIOS. We have a little driver to read and clear the 
SVKL data. This would only be used if the TD BIOS does the negotiation 
for the secrets, which it doesn't do currently.

- In the other model the negotiation is done by a user program, just 
using another driver to issue calls to the TDX module. The calls just 
expose the TDREPORT, which encodes the attestation data, but does not 
actually include any secret. Then the negotiation for the secrets is 
done by the program, which can then pass it out to other programs (like 
mount for encrypted file systems). In such a case the secret is never 
touched by the kernel. At least initially we'll use the second option.

-Andi

57ccc1
Brijesh Singh May 21, 2021, 3:56 p.m. UTC | #5
On 5/20/21 5:02 PM, Andi Kleen wrote:
>
> On 5/20/2021 3:56 AM, Dr. David Alan Gilbert wrote:
>> * Brijes
>> The nice thing about Dov's device/file is that it's a simple text file
>> that userspace can then read the secret out of;  I'm not sure if there's
>> anything similar in SNP (or for that matter TDX, cc'ing in Andi)
>
> In TDX there are two different mechanisms:
>
> - One is a ACPI table (SVKL) that allows to pass small pieces of data
> like keys from the BIOS. We have a little driver to read and clear the
> SVKL data. This would only be used if the TD BIOS does the negotiation
> for the secrets, which it doesn't do currently.
>
> - In the other model the negotiation is done by a user program, just
> using another driver to issue calls to the TDX module. The calls just
> expose the TDREPORT, which encodes the attestation data, but does not
> actually include any secret. Then the negotiation for the secrets is
> done by the program, which can then pass it out to other programs
> (like mount for encrypted file systems). In such a case the secret is
> never touched by the kernel. At least initially we'll use the second
> option.
>
The SEV-SNP attestation approach is very similar to what Andi described
for the TDX. However, in the case of legacy SEV and ES, the attestation
verification is performed before the guest is booted. In this case, the
hyervisor puts the secret provided by the guest owner (after the
attestation) at a fixed location. Dov's driver is simply reading that
fixed location and making it available through the simple text file.

In case of the SEV-SNP and TDX, the guest OS participates during the
attestation flow; the driver working on the behalf of userspace and does
not have access to the secret, so it cannot populate the file with the
secrets in it.

-Brijesh



> -Andi
>
> 57ccc1
James Bottomley May 21, 2021, 4:03 p.m. UTC | #6
On Fri, 2021-05-21 at 10:56 -0500, Brijesh Singh wrote:
[...]
> In case of the SEV-SNP and TDX, the guest OS participates during the
> attestation flow; the driver working on the behalf of userspace and
> does not have access to the secret, so it cannot populate the file
> with the secrets in it.

OK, so for a simple encrypted VM using root on luks, how in SNP does
the boot loader obtain the disk passphrase?

In the non SNP case, it's already upstream: OVMF finds the secret page
and converts it to an EFI config table, which is passed into grub. 
It's starting to sound like we'll need a new grub module for SNP which
will do an active attestation and receive the passphrase over some
channel secure against the cloud provider.  Could you give us an
example of how you think this flow will work?

Thanks,

James
Brijesh Singh May 21, 2021, 4:21 p.m. UTC | #7
On 5/21/21 11:03 AM, James Bottomley wrote:
> On Fri, 2021-05-21 at 10:56 -0500, Brijesh Singh wrote:
> [...]
>> In case of the SEV-SNP and TDX, the guest OS participates during the
>> attestation flow; the driver working on the behalf of userspace and
>> does not have access to the secret, so it cannot populate the file
>> with the secrets in it.
> OK, so for a simple encrypted VM using root on luks, how in SNP does
> the boot loader obtain the disk passphrase?

The GHCB specification v2 provides a new VMGEXIT (GUEST_REQUEST) that be
used either by the guest BIOS or guest kernel to request the attestation
report from the SEV-SNP firmware at any time. In the case of SEV-SNP,
the boot loader need to talk to SEV-SNP firmware to get the attestation
report.

Once the attestation report is available then boot loader need to
connect to guest owner to validate and obtain the secret (i.e disk
decryption key).


> In the non SNP case, it's already upstream: OVMF finds the secret page
> and converts it to an EFI config table, which is passed into grub. 
> It's starting to sound like we'll need a new grub module for SNP which
> will do an active attestation and receive the passphrase over some
> channel secure against the cloud provider.  Could you give us an
> example of how you think this flow will work?

The flow will be like this:

1. During the guest creation, a hypervsior calls LAUNCH_UPDATE (type
secret) to tell PSP to populate a secret page at a fixed guest address. 
Note that secret page is slightly different compare to what we know from
the SEV and ES. In this case the secret page contains a key used for
protecting the communication channel between the PSP and guest.

2. OVMF will create a EFI configuration table with the location of
secrets page.

3. Boot loader (or any other EFI) app can locate the secret page through
the EFI config lookup.

4. Boot loader can use the key from the secret page to communicate with
the PSP and request for the attestation report.

5. Pass the attestation report to the guest owner.

6. The guest owner validates the attestation report and releases the key
directly to the guest.

7. Boot loader can use the secret to unlock or disk or do whatever it
needs with it.


> Thanks,
>
> James
>
>
Andi Kleen May 21, 2021, 4:41 p.m. UTC | #8
> The SEV-SNP attestation approach is very similar to what Andi described
> for the TDX. However, in the case of legacy SEV and ES, the attestation
> verification is performed before the guest is booted. In this case, the
> hyervisor puts the secret provided by the guest owner (after the
> attestation) at a fixed location. Dov's driver is simply reading that
> fixed location and making it available through the simple text file.

That's the same as our SVKL model.

The (not yet posted) driver is here:

https://github.com/intel/tdx/commit/62c2d9fae275d5bf50f869e8cfb71d2ca1f71363

We opted to use ioctls, with the idea that it should be just read and 
cleared once to not let the secret lying around. Typically you would 
just use it to set up dmcrypt or similar once. I think read-and-clear 
with explicit operations is a better model than some virtual file 
because of the security properties.

-Andi
Dr. David Alan Gilbert May 24, 2021, 12:08 p.m. UTC | #9
* Andi Kleen (ak@linux.intel.com) wrote:
> 
> > The SEV-SNP attestation approach is very similar to what Andi described
> > for the TDX. However, in the case of legacy SEV and ES, the attestation
> > verification is performed before the guest is booted. In this case, the
> > hyervisor puts the secret provided by the guest owner (after the
> > attestation) at a fixed location. Dov's driver is simply reading that
> > fixed location and making it available through the simple text file.
> 
> That's the same as our SVKL model.
> 
> The (not yet posted) driver is here:
> 
> https://github.com/intel/tdx/commit/62c2d9fae275d5bf50f869e8cfb71d2ca1f71363
> 

Is there any way we could merge these two so that the TDX/SVKL would
look similar to SEV/ES to userspace?  If we needed some initrd glue here
for luks it would be great if we could have one piece of glue.
[I'm not sure if the numbering/naming of the secrets, and their format
are defined in the same way]

> We opted to use ioctls, with the idea that it should be just read and
> cleared once to not let the secret lying around. Typically you would just
> use it to set up dmcrypt or similar once. I think read-and-clear with
> explicit operations is a better model than some virtual file because of the
> security properties.

Do you think the ioctl is preferable to read+ftruncate/unlink ?
And if it was an ioctl, again could we get some standardisation here -
i.e.
maybe a /dev/confguest with a CONF_COMP_GET_KEY etc ?

Dave

> -Andi
> 
>
James Bottomley May 24, 2021, 3:35 p.m. UTC | #10
On Mon, 2021-05-24 at 13:08 +0100, Dr. David Alan Gilbert wrote:
> * Andi Kleen (ak@linux.intel.com) wrote:
[...]
> > We opted to use ioctls, with the idea that it should be just read
> > and cleared once to not let the secret lying around. Typically you
> > would just use it to set up dmcrypt or similar once. I think read-
> > and-clear with explicit operations is a better model than some
> > virtual file because of the security properties.
> 
> Do you think the ioctl is preferable to read+ftruncate/unlink ?

I really think if we do a unified upper interface it should be file
based.  An ioctl based on will have too much temptation to expose the
architecture of the underlying system.  However, the way to explore
this would be to ask if there's anything the current ioctl based one
can do that a file based one couldn't?

I think ftruncate/unlink is a preferable interface because it puts the
control in the hands of the consumer: you don't know how far the secret
might get shared, so by doing clear on first read the driver is forcing
the user implementation to cache it instead, thus shifting the problem
not solving it.

James
Andi Kleen May 24, 2021, 4:31 p.m. UTC | #11
On 5/24/2021 5:08 AM, Dr. David Alan Gilbert wrote:
> * Andi K
> Is there any way we could merge these two so that the TDX/SVKL would
> look similar to SEV/ES to userspace?  If we needed some initrd glue here
> for luks it would be great if we could have one piece of glue.
> [I'm not sure if the numbering/naming of the secrets, and their format
> are defined in the same way]
Maybe. There might well be differences in the contents as you say. So 
far SVKL doesn't really exist yet,  initially there will be the initrd 
based agents. The agents definitely will need to know about TDX.
>
> Do you think the ioctl is preferable to read+ftruncate/unlink ?
> And if it was an ioctl, again could we get some standardisation here -
> i.e.
> maybe a /dev/confguest with a CONF_COMP_GET_KEY etc ?

The advantage of the two ioctls is that they are very simple. Anything 
with a file system would be a lot more complicated. For security related 
code simplicity is a virtue.

Also since it's a really simple read and clear model I don't expect the 
value to be used widely, since it will be gone after boot anyways.

-andi
James Bottomley May 24, 2021, 5:12 p.m. UTC | #12
On Mon, 2021-05-24 at 09:31 -0700, Andi Kleen wrote:
> On 5/24/2021 5:08 AM, Dr. David Alan Gilbert wrote:
> > * Andi K
> > Is there any way we could merge these two so that the TDX/SVKL
> > would look similar to SEV/ES to userspace?  If we needed some
> > initrd glue here for luks it would be great if we could have one
> > piece of glue. [I'm not sure if the numbering/naming of the
> > secrets, and their format are defined in the same way]
> Maybe. There might well be differences in the contents as you say.
> So far SVKL doesn't really exist yet,  initially there will be the
> initrd based agents. The agents definitely will need to know about
> TDX.
> > Do you think the ioctl is preferable to read+ftruncate/unlink ?
> > And if it was an ioctl, again could we get some standardisation
> > here - i.e. maybe a /dev/confguest with a CONF_COMP_GET_KEY etc ?
> 
> The advantage of the two ioctls is that they are very simple.
> Anything with a file system would be a lot more complicated. For
> security related code simplicity is a virtue.

This RFC contained the FS code.  In size terms its very comparable to
your ioctl.

> Also since it's a really simple read and clear model I don't expect
> the value to be used widely, since it will be gone after boot
> anyways.

Enumeration looks to be problematic with your interface ... what are
you supposed to do, keep calling ACPI_SVKL_GET_KEY_INFO on an advancing
index until it gives you an error and then try to work out what key
you're interested in by one of its numeric properties?

I think a GUIDed structure actually helps here because we don't have to
have someone assign, say, u16 types to keys we're interested in and the
filesystem does all the enumeration for us.  It also means new use
cases can simply expand the properties without waiting for any
internals to catch up.

James
Dov Murik June 8, 2021, 7:48 p.m. UTC | #13
On 24/05/2021 20:12, James Bottomley wrote:
> On Mon, 2021-05-24 at 09:31 -0700, Andi Kleen wrote:
>> On 5/24/2021 5:08 AM, Dr. David Alan Gilbert wrote:
>>> * Andi K
>>> Is there any way we could merge these two so that the TDX/SVKL
>>> would look similar to SEV/ES to userspace?  If we needed some
>>> initrd glue here for luks it would be great if we could have one
>>> piece of glue. [I'm not sure if the numbering/naming of the
>>> secrets, and their format are defined in the same way]
>> Maybe. There might well be differences in the contents as you say.
>> So far SVKL doesn't really exist yet,  initially there will be the
>> initrd based agents. The agents definitely will need to know about
>> TDX.
>>> Do you think the ioctl is preferable to read+ftruncate/unlink ?
>>> And if it was an ioctl, again could we get some standardisation
>>> here - i.e. maybe a /dev/confguest with a CONF_COMP_GET_KEY etc ?
>>
>> The advantage of the two ioctls is that they are very simple.
>> Anything with a file system would be a lot more complicated. For
>> security related code simplicity is a virtue.
> 
> This RFC contained the FS code.  In size terms its very comparable to
> your ioctl.
> 
>> Also since it's a really simple read and clear model I don't expect
>> the value to be used widely, since it will be gone after boot
>> anyways.
> 
> Enumeration looks to be problematic with your interface ... what are
> you supposed to do, keep calling ACPI_SVKL_GET_KEY_INFO on an advancing
> index until it gives you an error and then try to work out what key
> you're interested in by one of its numeric properties?
> 
> I think a GUIDed structure actually helps here because we don't have to
> have someone assign, say, u16 types to keys we're interested in and the
> filesystem does all the enumeration for us.  It also means new use
> cases can simply expand the properties without waiting for any
> internals to catch up.
> 


Following the discussion here (and in various other meetings), the next
version of this patch series will keep the securityfs interface but will
introduce an unlink operation for the securityfs entries that will do
the following:

1. Remove the file entry from the securityfs dir

2. Overwrite the secret data memory (memzero_explicit)

3. Overwrite the GUID in the data memory with some invalid GUID
(ffffffff-ffff-.... ?), so that if the module is removed and loaded
again, the securityfs dir will not contain this entry (we'll ignore
these invalid GUIDs in the iteration).



Dov