Re: [PATCH v2 7/7] powerpc/pseries: Implement secvars for dynamic secure boot
On Fri, 2023-01-06 at 21:49 +1100, Michael Ellerman wrote: > > > diff --git a/arch/powerpc/platforms/pseries/Kconfig > > b/arch/powerpc/platforms/pseries/Kconfig > > index a3b4d99567cb..94e08c405d50 100644 > > --- a/arch/powerpc/platforms/pseries/Kconfig > > +++ b/arch/powerpc/platforms/pseries/Kconfig > > @@ -162,6 +162,19 @@ config PSERIES_PLPKS > > > > If unsure, select N. > > > > +config PSERIES_PLPKS_SECVAR > > + depends on PSERIES_PLPKS > > + depends on PPC_SECURE_BOOT > > + bool "Support for the PLPKS secvar interface" > > + help > > + PowerVM can support dynamic secure boot with user-defined > > keys > > + through the PLPKS. Keystore objects used in dynamic > > secure boot > > + can be exposed to the kernel and userspace through the > > powerpc > > + secvar infrastructure. Select this to enable the PLPKS > > backend > > + for secvars for use in pseries dynamic secure boot. > > + > > + If unsure, select N. > > I don't think we need that config option at all, or if we do it > should > not be user selectable and just enabled automatically by > PSERIES_PLPKS. I actually think we should get rid of both PSERIES_PLPKS_SECVAR and PSERIES_PLPKS, and just use PPC_SECURE_BOOT / PPC_SECVAR_SYSFS. -- Andrew DonnellanOzLabs, ADL Canberra a...@linux.ibm.com IBM Australia Limited
Re: [PATCH v2 7/7] powerpc/pseries: Implement secvars for dynamic secure boot
On Fri, 2023-01-06 at 21:49 +1100, Michael Ellerman wrote:
>
> > +static int plpks_get_variable(const char *key, uint64_t key_len,
> > + u8 *data, uint64_t *data_size)
> > +{
> > + struct plpks_var var = {0};
> > + u16 ucs2_namelen;
> > + u8 *ucs2_name;
> > + int rc = 0;
> > +
> > + ucs2_namelen = get_ucs2name(key, &ucs2_name);
> > + if (!ucs2_namelen)
> > + return -ENOMEM;
> > +
> > + var.name = ucs2_name;
> > + var.namelen = ucs2_namelen;
> > + var.os = PLPKS_VAR_LINUX;
> > + rc = plpks_read_os_var(&var);
> > +
> > + if (rc)
> > + goto err;
> > +
> > + *data_size = var.datalen + sizeof(var.policy);
> > +
> > + // We can be called with data = NULL to just get the object
> > size.
> > + if (data) {
> > + memcpy(data, &var.policy, sizeof(var.policy));
> > + memcpy(data + sizeof(var.policy), var.data,
> > var.datalen);
> > + }
>
> There's a lot of allocation and copying going on. The secvar-sysfs.c
> data_read() has kzalloc'ed data, but only after already doing the
> hcall
> to get the size. Then plpks_read_os_var() does an allocation for the
> hcall and then another allocation of the exact data size. Then
> data_read()
> does another copy into the sysfs supplied buffer.
>
> So to read a single variable we do the hcall twice, and allocate/copy
> the content of the variable 4 times?
>
> - Hypervisor into "output" in plpks_read_var().
> - "output" into "var->data" in plpks_read_var().
> - "var.data" into "data" in plpks_get_variable().
> - "data" into "buf" in data_read().
>
> As long as maxobjsize is < PAGE_SIZE I think we could do the hcall
> directly into "buf". Maybe we want to avoid writing into "buf"
> directly
> in case the hcall fails or something, but the other 3 copies seem
> unnecessary.
In the general case, I don't like passing buffer pointers straight from
parameters into hcalls, since the address has to be in the linear map,
and that's a detail I'd rather hide from callers. But otherwise, yes I
think we can probably shift to having the caller allocate the buffers.
--
Andrew DonnellanOzLabs, ADL Canberra
a...@linux.ibm.com IBM Australia Limited
Re: [PATCH v2 7/7] powerpc/pseries: Implement secvars for dynamic secure boot
On Mon, 2023-01-09 at 16:20 +1100, Andrew Donnellan wrote:
> On Mon, 2023-01-09 at 14:34 +1100, Russell Currey wrote:
> >
> > > > +static int plpks_secvar_init(void)
> > > > +{
> > > > + if (!plpks_is_available())
> > > > + return -ENODEV;
> > > > +
> > > > + set_secvar_ops(&plpks_secvar_ops);
> > > > + set_secvar_config_attrs(config_attrs);
> > > > + return 0;
> > > > +}
> > > > +device_initcall(plpks_secvar_init);
> > >
> > > That must be a machine_device_initcall(pseries, ...), otherwise
> > > we
> > > will
> > > blow up doing a hcall on powernv in plpks_is_available().
> >
> > OK, can do. I don't understand your case of how powernv could hit
> > this, but I think I to have to move plpks_is_available() into
> > include/,
> > so it's going to be even more possible anyway.
>
> Kernels can be compiled with both pseries and powernv support, in
> which
> case plpks_secvar_init() will be called unconditionally even when
> booting on a powernv machine.
>
> I can confirm that as it is, booting this on powernv qemu causes a
> panic.
Of course, I'm not sure why I thought an initcall in a platform that
wasn't active would magically not run on other platforms.
>
Re: [PATCH v2 7/7] powerpc/pseries: Implement secvars for dynamic secure boot
On Mon, 2023-01-09 at 14:34 +1100, Russell Currey wrote:
>
> > > +static int plpks_secvar_init(void)
> > > +{
> > > + if (!plpks_is_available())
> > > + return -ENODEV;
> > > +
> > > + set_secvar_ops(&plpks_secvar_ops);
> > > + set_secvar_config_attrs(config_attrs);
> > > + return 0;
> > > +}
> > > +device_initcall(plpks_secvar_init);
> >
> > That must be a machine_device_initcall(pseries, ...), otherwise we
> > will
> > blow up doing a hcall on powernv in plpks_is_available().
>
> OK, can do. I don't understand your case of how powernv could hit
> this, but I think I to have to move plpks_is_available() into
> include/,
> so it's going to be even more possible anyway.
Kernels can be compiled with both pseries and powernv support, in which
case plpks_secvar_init() will be called unconditionally even when
booting on a powernv machine.
I can confirm that as it is, booting this on powernv qemu causes a
panic.
--
Andrew DonnellanOzLabs, ADL Canberra
a...@linux.ibm.com IBM Australia Limited
Re: [PATCH v2 7/7] powerpc/pseries: Implement secvars for dynamic secure boot
On Fri, 2023-01-06 at 17:49 +1100, Russell Currey wrote: > > > > > + */ > > > > Inconsistent comment style > > True, I'm using // for multi-line comments in other places. I think > my > brain decided that this one was too long for that, but I'll make the > other multi-line comments similarly old-fashioned for consistency. Sigh, I was trying to encourage you to move into the future... -- Andrew DonnellanOzLabs, ADL Canberra a...@linux.ibm.com IBM Australia Limited
Re: [PATCH v2 7/7] powerpc/pseries: Implement secvars for dynamic secure boot
On Fri, 2023-01-06 at 21:49 +1100, Michael Ellerman wrote: > Russell Currey writes: > > The pseries platform can support dynamic secure boot (i.e. secure > > boot > > using user-defined keys) using variables contained with the PowerVM > > LPAR > > Platform KeyStore (PLPKS). Using the powerpc secvar API, expose > > the > > relevant variables for pseries dynamic secure boot through the > > existing > > secvar filesystem layout. > > > > The relevant variables for dynamic secure boot are signed in the > > keystore, and can only be modified using the H_PKS_SIGNED_UPDATE > > hcall. > > Object labels in the keystore are encoded using ucs2 format. With > > our > > fixed variable names we don't have to care about encoding outside > > of the > > necessary byte padding. > > > > When a user writes to a variable, the first 8 bytes of data must > > contain > > the signed update flags as defined by the hypervisor. > > > > When a user reads a variable, the first 4 bytes of data contain the > > policies defined for the object. > > > > Limitations exist due to the underlying implementation of sysfs > > binary > > attributes, as is the case for the OPAL secvar implementation - > > partial writes are unsupported and writes cannot be larger than > > PAGE_SIZE. > > > > Co-developed-by: Nayna Jain > > Signed-off-by: Nayna Jain > > Co-developed-by: Andrew Donnellan > > Signed-off-by: Andrew Donnellan > > Signed-off-by: Russell Currey > > --- > > v2: Remove unnecessary config vars from sysfs and document the > > others, > > thanks to review from Greg. If we end up needing to expose > > more, we > > can add them later and update the docs. > > > > Use sysfs_emit() instead of sprintf(), thanks to Greg. > > > > Change the size of the sysfs binary attributes to include the > > 8-byte > > flags header, preventing truncation of large writes. > > > > Documentation/ABI/testing/sysfs-secvar | 67 - > > arch/powerpc/platforms/pseries/Kconfig | 13 + > > arch/powerpc/platforms/pseries/Makefile | 4 +- > > arch/powerpc/platforms/pseries/plpks-secvar.c | 245 > > ++ > > 4 files changed, 326 insertions(+), 3 deletions(-) > > create mode 100644 arch/powerpc/platforms/pseries/plpks-secvar.c > > > > diff --git a/Documentation/ABI/testing/sysfs-secvar > > b/Documentation/ABI/testing/sysfs-secvar > > index feebb8c57294..466a8cb92b92 100644 > > --- a/Documentation/ABI/testing/sysfs-secvar > > +++ b/Documentation/ABI/testing/sysfs-secvar > > @@ -34,7 +34,7 @@ Description: An integer representation of the > > size of the content of the > > > > What: /sys/firmware/secvar/vars//data > > Date: August 2019 > > -Contact: Nayna Jain h > > +Contact: Nayna Jain > > Description: A read-only file containing the value of the > > variable. The size > > of the file represents the maximum size of the > > variable data. > > > > @@ -44,3 +44,68 @@ Contact: Nayna Jain > > Description: A write-only file that is used to submit the new > > value for the > > variable. The size of the file represents the > > maximum size of > > the variable data that can be written. > > + > > +What: /sys/firmware/secvar/config > > +Date: December 2022 > > +Contact: Nayna Jain > > +Description: This optional directory contains read-only config > > attributes as > > + defined by the secure variable implementation. All > > data is in > > + ASCII format. The directory is only created if the > > backing > > + implementation provides variables to populate it, > > which at > > + present is only PLPKS on the pseries platform. > > I think it's OK to mention that currently this only exists for PLPKS > ... > > > +What: /sys/firmware/secvar/config/version > > +Date: December 2022 > > +Contact: Nayna Jain > > +Description: RO file, only present if the secvar implementation > > is PLPKS. > > ... but I don't think we want to specify that files are only present > for PLPKS. > > Because if another backend wanted to create them in future, that > would > technically be an ABI change. Some are going to be PLPKS-specific, but for generic stuff like this I can change the description. > > > + Contains the config version as reported by the > > hypervisor in > > + ASCII decimal format. > > + > > +What: /sys/firmware/secvar/config/max_object_size > > +Date: December 2022 > > +Contact: Nayna Jain > > +Description: RO file, only present if the secvar implementation > > is PLPKS. > > + > > + Contains the maximum allowed size of objects in the > > keystore > > + in bytes, represented in ASCII decimal format. > > + > > + This is not necessarily the same as the max size > > that can be > > + written to an update file as
Re: [PATCH v2 7/7] powerpc/pseries: Implement secvars for dynamic secure boot
On Fri, 2023-01-06 at 21:49 +1100, Michael Ellerman wrote:
> > +What: /sys/firmware/secvar/config/supported_policies
> > +Date: December 2022
> > +Contact: Nayna Jain
> > +Description: RO file, only present if the secvar implementation
> > is PLPKS.
> > +
> > + Contains a bitmask of supported policy flags by the
> > hypervisor,
> > + represented as an 8 byte hexadecimal ASCII string.
> > Consult the
> > + hypervisor documentation for what these flags are.
> > +
> > +What: /sys/firmware/secvar/config/signed_update_algorithm
> > s
> > +Date: December 2022
> > +Contact: Nayna Jain
> > +Description: RO file, only present if the secvar implementation
> > is PLPKS.
> > +
> > + Contains a bitmask of flags indicating which
> > algorithms the
> > + hypervisor supports objects to be signed with when
> > modifying
> > + secvars, represented as a 16 byte hexadecimal ASCII
> > string.
> > + Consult the hypervisor documentation for what these
> > flags mean.
>
> Can this at least say "as defined in PAPR version X section Y"?
We don't currently have a PAPR reference for this - that will come
eventually.
>
> > diff --git a/arch/powerpc/platforms/pseries/Kconfig
> > b/arch/powerpc/platforms/pseries/Kconfig
> > index a3b4d99567cb..94e08c405d50 100644
> > --- a/arch/powerpc/platforms/pseries/Kconfig
> > +++ b/arch/powerpc/platforms/pseries/Kconfig
> > @@ -162,6 +162,19 @@ config PSERIES_PLPKS
> >
> > If unsure, select N.
> >
> > +config PSERIES_PLPKS_SECVAR
> > + depends on PSERIES_PLPKS
> > + depends on PPC_SECURE_BOOT
> > + bool "Support for the PLPKS secvar interface"
> > + help
> > + PowerVM can support dynamic secure boot with user-defined
> > keys
> > + through the PLPKS. Keystore objects used in dynamic
> > secure boot
> > + can be exposed to the kernel and userspace through the
> > powerpc
> > + secvar infrastructure. Select this to enable the PLPKS
> > backend
> > + for secvars for use in pseries dynamic secure boot.
> > +
> > + If unsure, select N.
>
> I don't think we need that config option at all, or if we do it
> should
> not be user selectable and just enabled automatically by
> PSERIES_PLPKS.
>
> > diff --git a/arch/powerpc/platforms/pseries/Makefile
> > b/arch/powerpc/platforms/pseries/Makefile
> > index 92310202bdd7..807756991f9d 100644
> > --- a/arch/powerpc/platforms/pseries/Makefile
> > +++ b/arch/powerpc/platforms/pseries/Makefile
> > @@ -27,8 +27,8 @@ obj-$(CONFIG_PAPR_SCM)+=
> > papr_scm.o
> > obj-$(CONFIG_PPC_SPLPAR) += vphn.o
> > obj-$(CONFIG_PPC_SVM) += svm.o
> > obj-$(CONFIG_FA_DUMP) += rtas-fadump.o
> > -obj-$(CONFIG_PSERIES_PLPKS) += plpks.o
> > -
> > +obj-$(CONFIG_PSERIES_PLPKS)+= plpks.o
> > +obj-$(CONFIG_PSERIES_PLPKS_SECVAR) += plpks-secvar.o
>
> I'm not convinced the secvar parts need to be in a separate C file.
>
> If it was all in plpks.c we could avoid all/most of plpks.h and a
> bunch
> of accessors and so on.
>
> But I don't feel that strongly about it if you think it's better
> separate.
I feel pretty strongly that we should keep it separate. We're going to
need a header file anyway because in future patches to come shortly we
need to wire plpks up with the integrity subsystem to load keys into
kernel keyrings.
>
> > diff --git a/arch/powerpc/platforms/pseries/plpks-secvar.c
> > b/arch/powerpc/platforms/pseries/plpks-secvar.c
> > new file mode 100644
> > index ..8298f039bef4
> > --- /dev/null
> > +++ b/arch/powerpc/platforms/pseries/plpks-secvar.c
> > @@ -0,0 +1,245 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Secure variable implementation using the PowerVM LPAR Platform
> > KeyStore (PLPKS)
> > + *
> > + * Copyright 2022, IBM Corporation
> > + * Authors: Russell Currey
> > + * Andrew Donnellan
> > + * Nayna Jain
> > + */
> > +
> > +#define pr_fmt(fmt) "secvar: "fmt
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include "plpks.h"
> > +
> > +// Config attributes for sysfs
> > +#define PLPKS_CONFIG_ATTR(name, fmt, func) \
> > + static ssize_t name##_show(struct kobject *kobj,\
> > + struct kobj_attribute *attr, \
> > + char *buf) \
> > + { \
> > + return sysfs_emit(buf, fmt, func());\
> > + } \
> > + static struct kobj_attribute attr_##name = __ATTR_RO(name)
> > +
> > +PLPKS_CONFIG_ATTR(version, "%u\n", plpks_get_version);
> > +PLPKS_CONFIG_ATTR(max_object_size, "%u\n",
> > plpks_get_maxobjectsize);
> > +P
Re: [PATCH v2 7/7] powerpc/pseries: Implement secvars for dynamic secure boot
Russell Currey writes: > The pseries platform can support dynamic secure boot (i.e. secure boot > using user-defined keys) using variables contained with the PowerVM LPAR > Platform KeyStore (PLPKS). Using the powerpc secvar API, expose the > relevant variables for pseries dynamic secure boot through the existing > secvar filesystem layout. > > The relevant variables for dynamic secure boot are signed in the > keystore, and can only be modified using the H_PKS_SIGNED_UPDATE hcall. > Object labels in the keystore are encoded using ucs2 format. With our > fixed variable names we don't have to care about encoding outside of the > necessary byte padding. > > When a user writes to a variable, the first 8 bytes of data must contain > the signed update flags as defined by the hypervisor. > > When a user reads a variable, the first 4 bytes of data contain the > policies defined for the object. > > Limitations exist due to the underlying implementation of sysfs binary > attributes, as is the case for the OPAL secvar implementation - > partial writes are unsupported and writes cannot be larger than PAGE_SIZE. > > Co-developed-by: Nayna Jain > Signed-off-by: Nayna Jain > Co-developed-by: Andrew Donnellan > Signed-off-by: Andrew Donnellan > Signed-off-by: Russell Currey > --- > v2: Remove unnecessary config vars from sysfs and document the others, > thanks to review from Greg. If we end up needing to expose more, we > can add them later and update the docs. > > Use sysfs_emit() instead of sprintf(), thanks to Greg. > > Change the size of the sysfs binary attributes to include the 8-byte > flags header, preventing truncation of large writes. > > Documentation/ABI/testing/sysfs-secvar| 67 - > arch/powerpc/platforms/pseries/Kconfig| 13 + > arch/powerpc/platforms/pseries/Makefile | 4 +- > arch/powerpc/platforms/pseries/plpks-secvar.c | 245 ++ > 4 files changed, 326 insertions(+), 3 deletions(-) > create mode 100644 arch/powerpc/platforms/pseries/plpks-secvar.c > > diff --git a/Documentation/ABI/testing/sysfs-secvar > b/Documentation/ABI/testing/sysfs-secvar > index feebb8c57294..466a8cb92b92 100644 > --- a/Documentation/ABI/testing/sysfs-secvar > +++ b/Documentation/ABI/testing/sysfs-secvar > @@ -34,7 +34,7 @@ Description:An integer representation of the size > of the content of the > > What:/sys/firmware/secvar/vars//data > Date:August 2019 > -Contact: Nayna Jain h > +Contact: Nayna Jain > Description: A read-only file containing the value of the variable. The size > of the file represents the maximum size of the variable data. > > @@ -44,3 +44,68 @@ Contact: Nayna Jain > Description: A write-only file that is used to submit the new value for the > variable. The size of the file represents the maximum size of > the variable data that can be written. > + > +What:/sys/firmware/secvar/config > +Date:December 2022 > +Contact: Nayna Jain > +Description: This optional directory contains read-only config attributes as > + defined by the secure variable implementation. All data is in > + ASCII format. The directory is only created if the backing > + implementation provides variables to populate it, which at > + present is only PLPKS on the pseries platform. I think it's OK to mention that currently this only exists for PLPKS ... > +What:/sys/firmware/secvar/config/version > +Date:December 2022 > +Contact: Nayna Jain > +Description: RO file, only present if the secvar implementation is PLPKS. ... but I don't think we want to specify that files are only present for PLPKS. Because if another backend wanted to create them in future, that would technically be an ABI change. > + Contains the config version as reported by the hypervisor in > + ASCII decimal format. > + > +What:/sys/firmware/secvar/config/max_object_size > +Date:December 2022 > +Contact: Nayna Jain > +Description: RO file, only present if the secvar implementation is PLPKS. > + > + Contains the maximum allowed size of objects in the keystore > + in bytes, represented in ASCII decimal format. > + > + This is not necessarily the same as the max size that can be > + written to an update file as writes can contain more than > + object data, you should use the size of the update file for > + that purpose. > + > +What:/sys/firmware/secvar/config/total_size > +Date:December 2022 > +Contact: Nayna Jain > +Description: RO file, only present if the secvar implementation is PLPKS. > + > + Contains the total size of the PLPKS in bytes, represented in > + ASCII decimal format. Simila
Re: [PATCH v2 7/7] powerpc/pseries: Implement secvars for dynamic secure boot
On Thu, 2023-01-05 at 19:15 +1100, Andrew Donnellan wrote: > On Fri, 2022-12-30 at 15:20 +1100, Russell Currey wrote: > > The pseries platform can support dynamic secure boot (i.e. secure > > boot > > using user-defined keys) using variables contained with the PowerVM > > LPAR > > Platform KeyStore (PLPKS). Using the powerpc secvar API, expose > > the > > relevant variables for pseries dynamic secure boot through the > > existing > > secvar filesystem layout. > > > > The relevant variables for dynamic secure boot are signed in the > > keystore, and can only be modified using the H_PKS_SIGNED_UPDATE > > hcall. > > Object labels in the keystore are encoded using ucs2 format. With > > our > > fixed variable names we don't have to care about encoding outside > > of > > the > > necessary byte padding. > > > > When a user writes to a variable, the first 8 bytes of data must > > contain > > the signed update flags as defined by the hypervisor. > > > > When a user reads a variable, the first 4 bytes of data contain the > > policies defined for the object. > > > > Limitations exist due to the underlying implementation of sysfs > > binary > > attributes, as is the case for the OPAL secvar implementation - > > partial writes are unsupported and writes cannot be larger than > > PAGE_SIZE. > > The PAGE_SIZE limit, in practice, will be a major limitation with 4K > pages (we expect several of the variables to regularly be larger than > 4K) but won't be much of an issue for 64K (that's all the storage > space > the hypervisor will give you anyway). > > In a previous internal version, we printed a message when PAGE_SIZE < > plpks_get_maxobjectsize(), might be worth still doing that? Yeah, we should do that in the secvar code. The same limitation applies on the powernv side as well. > > > > > Co-developed-by: Nayna Jain > > Signed-off-by: Nayna Jain > > Co-developed-by: Andrew Donnellan > > Signed-off-by: Andrew Donnellan > > Signed-off-by: Russell Currey > > Some minor comments for v3 on a patch which already carries my > signoff... > > > --- > > v2: Remove unnecessary config vars from sysfs and document the > > others, > > thanks to review from Greg. If we end up needing to expose > > more, > > we > > can add them later and update the docs. > > > > Use sysfs_emit() instead of sprintf(), thanks to Greg. > > > > Change the size of the sysfs binary attributes to include the > > 8- > > byte > > flags header, preventing truncation of large writes. > > > > Documentation/ABI/testing/sysfs-secvar | 67 - > > arch/powerpc/platforms/pseries/Kconfig | 13 + > > arch/powerpc/platforms/pseries/Makefile | 4 +- > > arch/powerpc/platforms/pseries/plpks-secvar.c | 245 > > ++ > > 4 files changed, 326 insertions(+), 3 deletions(-) > > create mode 100644 arch/powerpc/platforms/pseries/plpks-secvar.c > > > > diff --git a/Documentation/ABI/testing/sysfs-secvar > > b/Documentation/ABI/testing/sysfs-secvar > > index feebb8c57294..466a8cb92b92 100644 > > --- a/Documentation/ABI/testing/sysfs-secvar > > +++ b/Documentation/ABI/testing/sysfs-secvar > > @@ -34,7 +34,7 @@ Description: An integer representation of the > > size > > of the content of the > > > > What: /sys/firmware/secvar/vars//data > > Date: August 2019 > > -Contact: Nayna Jain h > > +Contact: Nayna Jain > > Description: A read-only file containing the value of the > > variable. The size > > of the file represents the maximum size of the > > variable data. > > > > @@ -44,3 +44,68 @@ Contact: Nayna Jain > > Description: A write-only file that is used to submit the new > > value for the > > variable. The size of the file represents the > > maximum > > size of > > the variable data that can be written. > > + > > +What: /sys/firmware/secvar/config > > +Date: December 2022 > > +Contact: Nayna Jain > > +Description: This optional directory contains read-only config > > attributes as > > + defined by the secure variable implementation. All > > data is in > > + ASCII format. The directory is only created if the > > backing > > + implementation provides variables to populate it, > > which at > > + present is only PLPKS on the pseries platform. > > + > > +What: /sys/firmware/secvar/config/version > > +Date: December 2022 > > +Contact: Nayna Jain > > +Description: RO file, only present if the secvar implementation > > is > > PLPKS. > > + > > + Contains the config version as reported by the > > hypervisor in > > + ASCII decimal format. > > + > > +What: /sys/firmware/secvar/config/max_object_size > > +Date: December 2022 > > +Contact: Nayna Jain > > +Description: RO file, only present if the secvar implementation > > is > > PLPKS. > > + > > +
Re: [PATCH v2 7/7] powerpc/pseries: Implement secvars for dynamic secure boot
On Fri, 2022-12-30 at 15:20 +1100, Russell Currey wrote: > The pseries platform can support dynamic secure boot (i.e. secure > boot > using user-defined keys) using variables contained with the PowerVM > LPAR > Platform KeyStore (PLPKS). Using the powerpc secvar API, expose the > relevant variables for pseries dynamic secure boot through the > existing > secvar filesystem layout. > > The relevant variables for dynamic secure boot are signed in the > keystore, and can only be modified using the H_PKS_SIGNED_UPDATE > hcall. > Object labels in the keystore are encoded using ucs2 format. With > our > fixed variable names we don't have to care about encoding outside of > the > necessary byte padding. > > When a user writes to a variable, the first 8 bytes of data must > contain > the signed update flags as defined by the hypervisor. > > When a user reads a variable, the first 4 bytes of data contain the > policies defined for the object. > > Limitations exist due to the underlying implementation of sysfs > binary > attributes, as is the case for the OPAL secvar implementation - > partial writes are unsupported and writes cannot be larger than > PAGE_SIZE. The PAGE_SIZE limit, in practice, will be a major limitation with 4K pages (we expect several of the variables to regularly be larger than 4K) but won't be much of an issue for 64K (that's all the storage space the hypervisor will give you anyway). In a previous internal version, we printed a message when PAGE_SIZE < plpks_get_maxobjectsize(), might be worth still doing that? > > Co-developed-by: Nayna Jain > Signed-off-by: Nayna Jain > Co-developed-by: Andrew Donnellan > Signed-off-by: Andrew Donnellan > Signed-off-by: Russell Currey Some minor comments for v3 on a patch which already carries my signoff... > --- > v2: Remove unnecessary config vars from sysfs and document the > others, > thanks to review from Greg. If we end up needing to expose more, > we > can add them later and update the docs. > > Use sysfs_emit() instead of sprintf(), thanks to Greg. > > Change the size of the sysfs binary attributes to include the 8- > byte > flags header, preventing truncation of large writes. > > Documentation/ABI/testing/sysfs-secvar | 67 - > arch/powerpc/platforms/pseries/Kconfig | 13 + > arch/powerpc/platforms/pseries/Makefile | 4 +- > arch/powerpc/platforms/pseries/plpks-secvar.c | 245 > ++ > 4 files changed, 326 insertions(+), 3 deletions(-) > create mode 100644 arch/powerpc/platforms/pseries/plpks-secvar.c > > diff --git a/Documentation/ABI/testing/sysfs-secvar > b/Documentation/ABI/testing/sysfs-secvar > index feebb8c57294..466a8cb92b92 100644 > --- a/Documentation/ABI/testing/sysfs-secvar > +++ b/Documentation/ABI/testing/sysfs-secvar > @@ -34,7 +34,7 @@ Description: An integer representation of the size > of the content of the > > What: /sys/firmware/secvar/vars//data > Date: August 2019 > -Contact: Nayna Jain h > +Contact: Nayna Jain > Description: A read-only file containing the value of the > variable. The size > of the file represents the maximum size of the > variable data. > > @@ -44,3 +44,68 @@ Contact: Nayna Jain > Description: A write-only file that is used to submit the new > value for the > variable. The size of the file represents the maximum > size of > the variable data that can be written. > + > +What: /sys/firmware/secvar/config > +Date: December 2022 > +Contact: Nayna Jain > +Description: This optional directory contains read-only config > attributes as > + defined by the secure variable implementation. All > data is in > + ASCII format. The directory is only created if the > backing > + implementation provides variables to populate it, > which at > + present is only PLPKS on the pseries platform. > + > +What: /sys/firmware/secvar/config/version > +Date: December 2022 > +Contact: Nayna Jain > +Description: RO file, only present if the secvar implementation is > PLPKS. > + > + Contains the config version as reported by the > hypervisor in > + ASCII decimal format. > + > +What: /sys/firmware/secvar/config/max_object_size > +Date: December 2022 > +Contact: Nayna Jain > +Description: RO file, only present if the secvar implementation is > PLPKS. > + > + Contains the maximum allowed size of objects in the > keystore > + in bytes, represented in ASCII decimal format. > + > + This is not necessarily the same as the max size that > can be > + written to an update file as writes can contain more > than > + object data, you should use the size of the update > file for > + that purpose. > + > +What: /sys/firmware/
