2022-01-22 20:00:26

by Nayna Jain

[permalink] [raw]
Subject: [RFC PATCH 0/2] powerpc/pseries: add support for local secure storage called Platform Keystore(PKS)

PowerVM provides an isolated Platform Keystore(PKS) storage allocation
for each partition with individually managed access controls to store
sensitive information securely. Linux Kernel can access this storage by
interfacing with hypervisor using a new set of hypervisor calls.

PowerVM guest secure boot intend to use Platform Keystore for the
purpose of storing public keys. Secure boot requires public keys to
be able to verify the grub and boot kernel. To allow authenticated
manipulation of keys, it supports variables to store key authorities
- PK/KEK and code signing keys - db. It also supports denied list to
disallow booting even if signed with valid key. This is done via
denied list database - dbx or sbat. These variables would be stored in
PKS, and are managed and controlled by firmware.

The purpose of this patchset is to add support for users to
read/write/add/delete variables required for secure boot on PowerVM.

Nayna Jain (2):
pseries: define driver for Platform Keystore
pseries: define sysfs interface to expose PKS variables

Documentation/ABI/testing/sysfs-pksvar | 77 +++
arch/powerpc/include/asm/hvcall.h | 13 +-
arch/powerpc/include/asm/pks.h | 84 +++
arch/powerpc/platforms/pseries/Kconfig | 17 +
arch/powerpc/platforms/pseries/Makefile | 2 +
arch/powerpc/platforms/pseries/pks.c | 494 ++++++++++++++++++
arch/powerpc/platforms/pseries/pksvar-sysfs.c | 356 +++++++++++++
7 files changed, 1042 insertions(+), 1 deletion(-)
create mode 100644 Documentation/ABI/testing/sysfs-pksvar
create mode 100644 arch/powerpc/include/asm/pks.h
create mode 100644 arch/powerpc/platforms/pseries/pks.c
create mode 100644 arch/powerpc/platforms/pseries/pksvar-sysfs.c

--
2.27.0


2022-01-23 14:24:00

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [RFC PATCH 0/2] powerpc/pseries: add support for local secure storage called Platform Keystore(PKS)

On Fri, Jan 21, 2022 at 07:56:35PM -0500, Nayna Jain wrote:
> PowerVM provides an isolated Platform Keystore(PKS) storage allocation
> for each partition with individually managed access controls to store
> sensitive information securely. Linux Kernel can access this storage by
> interfacing with hypervisor using a new set of hypervisor calls.
>
> PowerVM guest secure boot intend to use Platform Keystore for the
> purpose of storing public keys. Secure boot requires public keys to
> be able to verify the grub and boot kernel. To allow authenticated
> manipulation of keys, it supports variables to store key authorities
> - PK/KEK and code signing keys - db. It also supports denied list to
> disallow booting even if signed with valid key. This is done via
> denied list database - dbx or sbat. These variables would be stored in
> PKS, and are managed and controlled by firmware.
>
> The purpose of this patchset is to add support for users to
> read/write/add/delete variables required for secure boot on PowerVM.

Ok, this is like the 3rd or 4th different platform-specific proposal for
this type of functionality. I think we need to give up on
platform-specific user/kernel apis on this (random sysfs/securityfs
files scattered around the tree), and come up with a standard place for
all of this.

Please work with the other developers of the other drivers for this to
make this unified so that userspace has a chance to use this in a sane
manner.

thanks,

greg k-h

2022-01-24 15:03:37

by Daniel Axtens

[permalink] [raw]
Subject: Re: [RFC PATCH 0/2] powerpc/pseries: add support for local secure storage called Platform Keystore(PKS)

Hi Greg,

> Ok, this is like the 3rd or 4th different platform-specific proposal for
> this type of functionality. I think we need to give up on
> platform-specific user/kernel apis on this (random sysfs/securityfs
> files scattered around the tree), and come up with a standard place for
> all of this.

I agree that we do have a number of platforms exposing superficially
similar functionality. Indeed, back in 2019 I had a crack at a unified
approach: [1] [2].

Looking back at it now, I am not sure it ever would have worked because
the semantics of the underlying firmware stores are quite
different. Here are the ones I know about:

- OpenPower/PowerNV Secure Variables:

* Firmware semantics:
- flat variable space
- variables are fixed in firmware, can neither be created nor
destroyed
- variable names are ASCII
- no concept of policy/attributes

* Current kernel interface semantics:
- names are case sensitive
- directory per variable


- (U)EFI variables:

* Firmware semantics:
- flat variable space
- variables can be created/destroyed but the semantics are fiddly
[3]
- variable names are UTF-16 + UUID
- variables have 32-bit attributes

* efivarfs interface semantics:
- file per variable
- attributes are the first 4 bytes of the file
- names are partially case-insensitive (UUID part) and partially
case-sensitive ('name' part)

* sysfs interface semantics (as used by CONFIG_GOOGLE_SMI)
- directory per variable
- attributes are a separate sysfs file
- to create a variable you write a serialised structure to
`/sys/firmware/efi/vars/new_var`, to delete a var you write
to `.../del_var`
- names are case-sensitive including the UUID


- PowerVM Partition Key Store Variables:

* Firmware semantics:
- _not_ a flat space, there are 3 domains ("consumers"): firmware,
bootloader and OS (not yet supported by the patch set)
- variables can be created and destroyed but the semantics are
fiddly and fiddly in different ways to UEFI [4]
- variable names are arbitrary byte strings: the hypervisor permits
names to contain nul and /.
- variables have 32-bit attributes ("policy") that don't align with
UEFI attributes

* No stable kernel interface yet

Even if we could come up with some stable kernel interface features
(e.g. decide if we want file per variable vs directory per variable), I
don't know how easy it would be to deal with the underlying semantic
differences - I think userspace would still need substantial
per-platform knowledge.

Or have I misunderstood what you're asking for? (If you want them all to
live under /sys/firmware, these ones all already do...)

Kind regards,
Daniel


[1]: https://lists.ozlabs.org/pipermail/linuxppc-dev/2019-May/190735.html
[2]: discussion continues at https://lists.ozlabs.org/pipermail/linuxppc-dev/2019-June/191365.html
[3]: https://lists.ozlabs.org/pipermail/linuxppc-dev/2019-June/191496.html

[4]: An unsigned variable cannot be updated, it can only be deleted
(unless it was created with the immutable policy) and then
re-created. A signed variable, on the other hand, can be updated
and the only way to delete it is to submit a validly signed empty
update.

2022-02-02 08:45:39

by Dave Hansen

[permalink] [raw]
Subject: Re: [RFC PATCH 0/2] powerpc/pseries: add support for local secure storage called Platform Keystore(PKS)

On 1/21/22 16:56, Nayna Jain wrote:
> Nayna Jain (2):
> pseries: define driver for Platform Keystore
> pseries: define sysfs interface to expose PKS variables

Hi Folks,

There another feature that we might want to consider in the naming here:

> https://lore.kernel.org/all/[email protected]/

Protection Keys for Supervisor pages is also called PKS. It's also not
entirely impossible that powerpc might want to start using this code at
some point, just like what happened with the userspace protection keys[1].

I don't think it's the end of the world either way, but it might save a
hapless user or kernel developer some confusion if we can avoid
including two "PKS" features in the kernel. I just wanted to make sure
we were aware of the other's existence. :)

1.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/powerpc/include/asm/pkeys.h

2022-02-02 12:49:42

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [RFC PATCH 0/2] powerpc/pseries: add support for local secure storage called Platform Keystore(PKS)

On Mon, Jan 24, 2022 at 11:25:17AM +1100, Daniel Axtens wrote:
> Hi Greg,
>
> > Ok, this is like the 3rd or 4th different platform-specific proposal for
> > this type of functionality. I think we need to give up on
> > platform-specific user/kernel apis on this (random sysfs/securityfs
> > files scattered around the tree), and come up with a standard place for
> > all of this.
>
> I agree that we do have a number of platforms exposing superficially
> similar functionality. Indeed, back in 2019 I had a crack at a unified
> approach: [1] [2].
>
> Looking back at it now, I am not sure it ever would have worked because
> the semantics of the underlying firmware stores are quite
> different. Here are the ones I know about:
>
> - OpenPower/PowerNV Secure Variables:
>
> * Firmware semantics:
> - flat variable space
> - variables are fixed in firmware, can neither be created nor
> destroyed
> - variable names are ASCII
> - no concept of policy/attributes
>
> * Current kernel interface semantics:
> - names are case sensitive
> - directory per variable
>
>
> - (U)EFI variables:
>
> * Firmware semantics:
> - flat variable space
> - variables can be created/destroyed but the semantics are fiddly
> [3]
> - variable names are UTF-16 + UUID
> - variables have 32-bit attributes
>
> * efivarfs interface semantics:
> - file per variable
> - attributes are the first 4 bytes of the file
> - names are partially case-insensitive (UUID part) and partially
> case-sensitive ('name' part)
>
> * sysfs interface semantics (as used by CONFIG_GOOGLE_SMI)
> - directory per variable
> - attributes are a separate sysfs file
> - to create a variable you write a serialised structure to
> `/sys/firmware/efi/vars/new_var`, to delete a var you write
> to `.../del_var`
> - names are case-sensitive including the UUID
>
>
> - PowerVM Partition Key Store Variables:
>
> * Firmware semantics:
> - _not_ a flat space, there are 3 domains ("consumers"): firmware,
> bootloader and OS (not yet supported by the patch set)
> - variables can be created and destroyed but the semantics are
> fiddly and fiddly in different ways to UEFI [4]
> - variable names are arbitrary byte strings: the hypervisor permits
> names to contain nul and /.
> - variables have 32-bit attributes ("policy") that don't align with
> UEFI attributes
>
> * No stable kernel interface yet
>
> Even if we could come up with some stable kernel interface features
> (e.g. decide if we want file per variable vs directory per variable), I
> don't know how easy it would be to deal with the underlying semantic
> differences - I think userspace would still need substantial
> per-platform knowledge.
>
> Or have I misunderstood what you're asking for? (If you want them all to
> live under /sys/firmware, these ones all already do...)

I want them to be unified in some way, right now there are lots of
proposals for the same type of thing, but in different places (sysfs,
securityfs, somewhere else), and with different names.

Please work together.

thanks,

greg k-h