Jarkko Sakkinen wrote:
> On Fri Jul 28, 2023 at 7:30 PM UTC, Dan Williams wrote:
> > The bulk of the justification for this patch kit is in "[PATCH 1/4]
>
> /patch kit/patch set/
>
> > keys: Introduce tsm keys". The short summary is that the current
> > approach of adding new char devs and new ioctls, for what amounts to the
> > same functionality with minor formatting differences across vendors, is
> > untenable. Common concepts and the community benefit from common
> > infrastructure.
> >
> > Use Keys to build common infrastructure for confidential computing
>
> /Keys/Linux keyring/
>
> > attestation report blobs, convert sevguest to use it (leaving the
> > deprecation question alone for now), and pave the way for tdx-guest and
> > the eventual risc-v equivalent to use it in lieu of new ioctls.
> >
> > The sevguest conversion is only compile-tested.
> >
> > This submission is To:David since he needs to sign-off on the idea of a
> > new Keys type, the rest is up to the confidential-computing driver
> > maintainers to adopt.
> >
> > Changes from / credit for internal review:
> > - highlight copy_{to,from}_sockptr() as a common way to mix
> > copy_user() and memcpy() paths (Andy)
> > - add MODULE_DESCRIPTION() (Andy)
> > - clarify how the user-defined portion blob might be used (Elena)
> > - clarify the key instantiation options (Sathya)
> > - drop usage of a list for registering providers (Sathya)
> > - drop list.h include from tsm.h (Andy)
> > - add a comment for how TSM_DATA_MAX was derived (Andy)
> > - stop open coding kmemdup_nul() (Andy)
> > - add types.h to tsm.h (Andy)
> > - fix punctuation in comment (Andy)
> > - reorder security/keys/Makefile (Andy)
> > - add some missing includes to tsm.c (Andy)
> > - undo an 81 column clang-format line break (Andy)
> > - manually reflow tsm_token indentation (Andy)
> > - move allocations after input validation in tsm_instantiate() (Andy)
> > - switch to bin2hex() in tsm_read() (Andy)
> > - move init/exit declarations next to their functions (Andy)
> >
> >
> > ---
> >
> > Dan Williams (4):
> > keys: Introduce tsm keys
> > virt: sevguest: Prep for kernel internal {get,get_ext}_report()
> > mm/slab: Add __free() support for kvfree
> > virt: sevguest: Add TSM key support for SNP_{GET,GET_EXT}_REPORT
> >
> >
> > drivers/virt/coco/sev-guest/Kconfig | 2
> > drivers/virt/coco/sev-guest/sev-guest.c | 135 ++++++++++++++-
> > include/keys/tsm.h | 71 ++++++++
> > include/linux/slab.h | 2
> > security/keys/Kconfig | 12 +
> > security/keys/Makefile | 1
> > security/keys/tsm.c | 282 +++++++++++++++++++++++++++++++
> > 7 files changed, 494 insertions(+), 11 deletions(-)
> > create mode 100644 include/keys/tsm.h
> > create mode 100644 security/keys/tsm.c
> >
> > base-commit: 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5
>
> So how does this scale? Does it scale to TDX, SGX, TPM's or even TEE's
> (ARM SM, RISC-V Keystone etc.). I'm not sure about the scope but we want
> of course something that adapts to multiple use cases, right?
TPMs and TEEs are covered by trusted-keys. I do think a "TSM" flavor of
trusted-keys is in scope for where some of these implementations are
headed, but that comes later. I talk about that in the changelog that
functionality like SNP_GET_DERIVED_KEY likely wants to have a
trusted-keys frontend and not isolated behind a vendor-specific ioctl
interface.
This facility is different, it is just aiming to unify this attestation
report flow. It scales to any driver that can provide the ->auth_new()
operation. I have the sev-guest conversion in this set, and Sathya has
tested this with tdx-guest. I am hoping Samuel can evaluate it for
cove-guest or whatever that driver ends up being called.
On Fri Jul 28, 2023 at 7:44 PM UTC, Dan Williams wrote:
> Jarkko Sakkinen wrote:
> > On Fri Jul 28, 2023 at 7:30 PM UTC, Dan Williams wrote:
> > > The bulk of the justification for this patch kit is in "[PATCH 1/4]
> >
> > /patch kit/patch set/
> >
> > > keys: Introduce tsm keys". The short summary is that the current
> > > approach of adding new char devs and new ioctls, for what amounts to the
> > > same functionality with minor formatting differences across vendors, is
> > > untenable. Common concepts and the community benefit from common
> > > infrastructure.
> > >
> > > Use Keys to build common infrastructure for confidential computing
> >
> > /Keys/Linux keyring/
> >
> > > attestation report blobs, convert sevguest to use it (leaving the
> > > deprecation question alone for now), and pave the way for tdx-guest and
> > > the eventual risc-v equivalent to use it in lieu of new ioctls.
> > >
> > > The sevguest conversion is only compile-tested.
> > >
> > > This submission is To:David since he needs to sign-off on the idea of a
> > > new Keys type, the rest is up to the confidential-computing driver
> > > maintainers to adopt.
> > >
> > > Changes from / credit for internal review:
> > > - highlight copy_{to,from}_sockptr() as a common way to mix
> > > copy_user() and memcpy() paths (Andy)
> > > - add MODULE_DESCRIPTION() (Andy)
> > > - clarify how the user-defined portion blob might be used (Elena)
> > > - clarify the key instantiation options (Sathya)
> > > - drop usage of a list for registering providers (Sathya)
> > > - drop list.h include from tsm.h (Andy)
> > > - add a comment for how TSM_DATA_MAX was derived (Andy)
> > > - stop open coding kmemdup_nul() (Andy)
> > > - add types.h to tsm.h (Andy)
> > > - fix punctuation in comment (Andy)
> > > - reorder security/keys/Makefile (Andy)
> > > - add some missing includes to tsm.c (Andy)
> > > - undo an 81 column clang-format line break (Andy)
> > > - manually reflow tsm_token indentation (Andy)
> > > - move allocations after input validation in tsm_instantiate() (Andy)
> > > - switch to bin2hex() in tsm_read() (Andy)
> > > - move init/exit declarations next to their functions (Andy)
> > >
> > >
> > > ---
> > >
> > > Dan Williams (4):
> > > keys: Introduce tsm keys
> > > virt: sevguest: Prep for kernel internal {get,get_ext}_report()
> > > mm/slab: Add __free() support for kvfree
> > > virt: sevguest: Add TSM key support for SNP_{GET,GET_EXT}_REPORT
> > >
> > >
> > > drivers/virt/coco/sev-guest/Kconfig | 2
> > > drivers/virt/coco/sev-guest/sev-guest.c | 135 ++++++++++++++-
> > > include/keys/tsm.h | 71 ++++++++
> > > include/linux/slab.h | 2
> > > security/keys/Kconfig | 12 +
> > > security/keys/Makefile | 1
> > > security/keys/tsm.c | 282 +++++++++++++++++++++++++++++++
> > > 7 files changed, 494 insertions(+), 11 deletions(-)
> > > create mode 100644 include/keys/tsm.h
> > > create mode 100644 security/keys/tsm.c
> > >
> > > base-commit: 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5
> >
> > So how does this scale? Does it scale to TDX, SGX, TPM's or even TEE's
> > (ARM SM, RISC-V Keystone etc.). I'm not sure about the scope but we want
> > of course something that adapts to multiple use cases, right?
>
> TPMs and TEEs are covered by trusted-keys. I do think a "TSM" flavor of
> trusted-keys is in scope for where some of these implementations are
> headed, but that comes later. I talk about that in the changelog that
> functionality like SNP_GET_DERIVED_KEY likely wants to have a
> trusted-keys frontend and not isolated behind a vendor-specific ioctl
> interface.
TEE's and TPM's are not the exact same thing. Are we sure that any
future ARM SMC like TEE interface what you say will hold?
Why do we need a new key type, when we have already trusted keys?
This whole territory should be better defined so that everything
will fit together.
> This facility is different, it is just aiming to unify this attestation
> report flow. It scales to any driver that can provide the ->auth_new()
> operation. I have the sev-guest conversion in this set, and Sathya has
> tested this with tdx-guest. I am hoping Samuel can evaluate it for
> cove-guest or whatever that driver ends up being called.
What about SGX without TDX?
BR, Jarkko
Jarkko Sakkinen wrote:
> On Fri Jul 28, 2023 at 7:44 PM UTC, Dan Williams wrote:
> > Jarkko Sakkinen wrote:
> > > On Fri Jul 28, 2023 at 7:30 PM UTC, Dan Williams wrote:
> > > > The bulk of the justification for this patch kit is in "[PATCH 1/4]
> > >
> > > /patch kit/patch set/
> > >
> > > > keys: Introduce tsm keys". The short summary is that the current
> > > > approach of adding new char devs and new ioctls, for what amounts to the
> > > > same functionality with minor formatting differences across vendors, is
> > > > untenable. Common concepts and the community benefit from common
> > > > infrastructure.
> > > >
> > > > Use Keys to build common infrastructure for confidential computing
> > >
> > > /Keys/Linux keyring/
> > >
> > > > attestation report blobs, convert sevguest to use it (leaving the
> > > > deprecation question alone for now), and pave the way for tdx-guest and
> > > > the eventual risc-v equivalent to use it in lieu of new ioctls.
> > > >
> > > > The sevguest conversion is only compile-tested.
> > > >
> > > > This submission is To:David since he needs to sign-off on the idea of a
> > > > new Keys type, the rest is up to the confidential-computing driver
> > > > maintainers to adopt.
> > > >
> > > > Changes from / credit for internal review:
> > > > - highlight copy_{to,from}_sockptr() as a common way to mix
> > > > copy_user() and memcpy() paths (Andy)
> > > > - add MODULE_DESCRIPTION() (Andy)
> > > > - clarify how the user-defined portion blob might be used (Elena)
> > > > - clarify the key instantiation options (Sathya)
> > > > - drop usage of a list for registering providers (Sathya)
> > > > - drop list.h include from tsm.h (Andy)
> > > > - add a comment for how TSM_DATA_MAX was derived (Andy)
> > > > - stop open coding kmemdup_nul() (Andy)
> > > > - add types.h to tsm.h (Andy)
> > > > - fix punctuation in comment (Andy)
> > > > - reorder security/keys/Makefile (Andy)
> > > > - add some missing includes to tsm.c (Andy)
> > > > - undo an 81 column clang-format line break (Andy)
> > > > - manually reflow tsm_token indentation (Andy)
> > > > - move allocations after input validation in tsm_instantiate() (Andy)
> > > > - switch to bin2hex() in tsm_read() (Andy)
> > > > - move init/exit declarations next to their functions (Andy)
> > > >
> > > >
> > > > ---
> > > >
> > > > Dan Williams (4):
> > > > keys: Introduce tsm keys
> > > > virt: sevguest: Prep for kernel internal {get,get_ext}_report()
> > > > mm/slab: Add __free() support for kvfree
> > > > virt: sevguest: Add TSM key support for SNP_{GET,GET_EXT}_REPORT
> > > >
> > > >
> > > > drivers/virt/coco/sev-guest/Kconfig | 2
> > > > drivers/virt/coco/sev-guest/sev-guest.c | 135 ++++++++++++++-
> > > > include/keys/tsm.h | 71 ++++++++
> > > > include/linux/slab.h | 2
> > > > security/keys/Kconfig | 12 +
> > > > security/keys/Makefile | 1
> > > > security/keys/tsm.c | 282 +++++++++++++++++++++++++++++++
> > > > 7 files changed, 494 insertions(+), 11 deletions(-)
> > > > create mode 100644 include/keys/tsm.h
> > > > create mode 100644 security/keys/tsm.c
> > > >
> > > > base-commit: 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5
> > >
> > > So how does this scale? Does it scale to TDX, SGX, TPM's or even TEE's
> > > (ARM SM, RISC-V Keystone etc.). I'm not sure about the scope but we want
> > > of course something that adapts to multiple use cases, right?
> >
> > TPMs and TEEs are covered by trusted-keys. I do think a "TSM" flavor of
> > trusted-keys is in scope for where some of these implementations are
> > headed, but that comes later. I talk about that in the changelog that
> > functionality like SNP_GET_DERIVED_KEY likely wants to have a
> > trusted-keys frontend and not isolated behind a vendor-specific ioctl
> > interface.
>
> TEE's and TPM's are not the exact same thing. Are we sure that any
> future ARM SMC like TEE interface what you say will hold?
Agree, they are not the same thing, I assume that's why trusted-keys has
a TEE and a TPM backend. Also that's the point of common interface
proposals for the per vendor experts to take a look and make sure it
fits their needs. If you have contacts there, please highlight this
thread to them.
> Why do we need a new key type, when we have already trusted keys?
As I mentioned to James to the comment from him about vTPM, if that ends
up just looking like a standard TPM to Linux then nothing new is needed.
> This whole territory should be better defined so that everything
> will fit together.
Yes, the per-vendor differentiation in this space is an impediment to
kernel interface design.
> > This facility is different, it is just aiming to unify this attestation
> > report flow. It scales to any driver that can provide the ->auth_new()
> > operation. I have the sev-guest conversion in this set, and Sathya has
> > tested this with tdx-guest. I am hoping Samuel can evaluate it for
> > cove-guest or whatever that driver ends up being called.
>
> What about SGX without TDX?
My hope would be that anything that can not be fronted by TPM2_Quote
directly can by frontend by this "TSM" class device (as I will be
switching from Keyring to sysfs).
On Mon, 2023-07-31 at 10:09 +0000, Jarkko Sakkinen wrote:
> > This facility is different, it is just aiming to unify this attestation
> > report flow. It scales to any driver that can provide the ->auth_new()
> > operation. I have the sev-guest conversion in this set, and Sathya has
> > tested this with tdx-guest. I am hoping Samuel can evaluate it for
> > cove-guest or whatever that driver ends up being called.
>
> What about SGX without TDX?
SGX attestation is completely among userspace enclaves, and the existing SGX
userspace stack has fully adopted what is needed to do attestation. Why do we
need to cover SGX?
On Tue Aug 1, 2023 at 1:41 AM EEST, Huang, Kai wrote:
> On Mon, 2023-07-31 at 10:09 +0000, Jarkko Sakkinen wrote:
> > > This facility is different, it is just aiming to unify this attestation
> > > report flow. It scales to any driver that can provide the ->auth_new()
> > > operation. I have the sev-guest conversion in this set, and Sathya has
> > > tested this with tdx-guest. I am hoping Samuel can evaluate it for
> > > cove-guest or whatever that driver ends up being called.
> >
> > What about SGX without TDX?
>
> SGX attestation is completely among userspace enclaves, and the existing SGX
> userspace stack has fully adopted what is needed to do attestation. Why do we
> need to cover SGX?
I have no answer to that. I'm merely trying to understand what this is.
BR, Jarkko