Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [RFC PATCH 0/3] Allow access to confidential computing secret
 area
To:     Brijesh Singh <brijesh.singh@amd.com>, linux-efi@vger.kernel.org
Cc:     Tobin Feldman-Fitzthum <tobin@linux.ibm.com>,
        Tobin Feldman-Fitzthum <tobin@ibm.com>,
        Jim Cadden <jcadden@ibm.com>,
        James Bottomley <jejb@linux.ibm.com>,
        Hubertus Franke <frankeh@us.ibm.com>,
        Mike Rapoport <rppt@linux.ibm.com>,
        Laszlo Ersek <lersek@redhat.com>,
        Ashish Kalra <ashish.kalra@amd.com>,
        Tom Lendacky <thomas.lendacky@amd.com>,
        Ard Biesheuvel <ardb@kernel.org>,
        James Morris <jmorris@namei.org>,
        "Serge E. Hallyn" <serge@hallyn.com>,
        linux-security-module@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20210513062634.2481118-1-dovmurik@linux.ibm.com>
 <2c8ae998-6dd0-bcb9-f735-e90da05ab9d9@amd.com>
From:   Dov Murik <dovmurik@linux.ibm.com>
Message-ID: <c3a781e0-1c76-e0b5-9d65-e1c049dc2deb@linux.ibm.com>
Date:   Thu, 20 May 2021 13:38:28 +0300
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101
 Thunderbird/78.10.2
In-Reply-To: <2c8ae998-6dd0-bcb9-f735-e90da05ab9d9@amd.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
Precedence: bulk

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