linux-kernel - Re: [PATCH v2 2/3] powerpc/secvar: Expose secvars relevant to the key management mode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aEB3MPAYeAaFVpTc@kitsune.suse.cz>
Date: Wed, 4 Jun 2025 18:41:20 +0200
From: Michal Suchánek <msuchanek@...e.de>
To: Srish Srinivasan <ssrish@...ux.ibm.com>
Cc: linux-integrity@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org,
	maddy@...ux.ibm.com, mpe@...erman.id.au, npiggin@...il.com,
	christophe.leroy@...roup.eu, naveen@...nel.org, ajd@...ux.ibm.com,
	zohar@...ux.ibm.com, nayna@...ux.ibm.com,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/3] powerpc/secvar: Expose secvars relevant to the
 key management mode

On Thu, May 29, 2025 at 10:39:58PM +0530, Srish Srinivasan wrote:
> 
> On 5/23/25 11:49 AM, Michal Suchánek wrote:
> > Hello,
> > 
> > On Wed, May 21, 2025 at 04:27:58PM +0530, Srish Srinivasan wrote:
> > > The PLPKS enabled PowerVM LPAR sysfs exposes all of the secure boot
> > > secvars irrespective of the key management mode.
> > > 
> > > The PowerVM LPAR supports static and dynamic key management for secure
> > > boot. The key management option can be updated in the management
> > > console. Only in the dynamic key mode can the user modify the secure
> > > boot secvars db, dbx, grubdb, grubdbx, and sbat, which are exposed via
> > > the sysfs interface. But the sysfs interface exposes these secvars even
> > > in the static key mode. This could lead to errors when reading them or
> > > writing to them in the static key mode.
> > would it cause an error when reading these variables or only when
> > writing them?
> > 
> > Thanks
> > 
> > Michal
> 
> Hi Michal,
> Thanks for taking a look.
> 
> 
> Yes, when PKS is enabled without enabling dynamic key secure boot, the
> secvars
> are NOT yet initialized with the default keys built into the binaries, and
> therefore
> reading them will result in an error.

That suggests that 'cannot be written' as said in the documentation and
commit message, which would imply readonly, is misleading. The value is
not accessible at all.

> Now, while in static key management mode with PKS enabled, if one tries to
> populate secvars that are relevant to dynamic key management, the write does
> not fail as long as the "Platform KeyStore Signed Update Infrastructure"
> flag on
> the HMC is enabled and the signed updates are authorized by valid PK/KEK
> keys.

Which suggests that some variables can if fact be written

> However, secvars like db and grubdb populated while in static key management
> mode are not used by the Partition Firmware or grub as SB_VERSION is not
> present,

but are not used until the key management is switched to dynamic

> i.e dynamic key secure boot has not been enabled yet. In this case, when
> there is a
> transition from static key management to dynamic key management, secvars
> with
> the signed update policy bit set will not be overwritten by the hypervisor
> with the
> default keys. Now, if the keys written into these secvars were not the ones
> that were
> used to sign the grub and kernel, it would fail to verify them.

Which is the case even for the case the system is already in dynamic key
mode, unless the variables are append-only.

Thanks

Michal

> These are the reasons behind the decision to expose only those secvars that
> are
> relevant to the key management mode.
> 
> > 
> > 
> > > Expose only PK, trustedcadb, and moduledb in the static key mode to
> > > enable loading of signed third-party kernel modules.
> > > 
> > > Co-developed-by: Souradeep <soura@...p.linux.ibm.com>
> > > Signed-off-by: Souradeep <soura@...p.linux.ibm.com>
> > > Signed-off-by: Srish Srinivasan <ssrish@...ux.ibm.com>
> > > Reviewed-by: Mimi Zohar <zohar@...ux.ibm.com>
> > > Reviewed-by: Stefan Berger <stefanb@...ux.ibm.com>
> > > Reviewed-by: Nayna Jain <nayna@...ux.ibm.com>
> > > Reviewed-by: Andrew Donnellan <ajd@...ux.ibm.com>
> > > ---
> > >   Documentation/ABI/testing/sysfs-secvar        |  6 ++++
> > >   arch/powerpc/platforms/pseries/plpks-secvar.c | 28 ++++++++++++++++---
> > >   2 files changed, 30 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/Documentation/ABI/testing/sysfs-secvar b/Documentation/ABI/testing/sysfs-secvar
> > > index 45281888e520..948df3446a03 100644
> > > --- a/Documentation/ABI/testing/sysfs-secvar
> > > +++ b/Documentation/ABI/testing/sysfs-secvar
> > > @@ -37,6 +37,12 @@ Description:	Each secure variable is represented as a directory named as
> > >   		representation. The data and size can be determined by reading
> > >   		their respective attribute files.
> > > +		Only secvars relevant to the key management mode are exposed.
> > > +		Only in the dynamic key mode can the user modify the secure boot
> > > +		secvars db, dbx, grubdb, grubdbx, and sbat. PK, trustedcadb and
> > > +		moduledb are the secvars common to both static and dynamic key
> > > +		management modes.
> > > +
> > >   What:		/sys/firmware/secvar/vars/<variable_name>/size
> > >   Date:		August 2019
> > >   Contact:	Nayna Jain <nayna@...ux.ibm.com>
> > > diff --git a/arch/powerpc/platforms/pseries/plpks-secvar.c b/arch/powerpc/platforms/pseries/plpks-secvar.c
> > > index 767e5e8c6990..f9e9cc40c9d0 100644
> > > --- a/arch/powerpc/platforms/pseries/plpks-secvar.c
> > > +++ b/arch/powerpc/platforms/pseries/plpks-secvar.c
> > > @@ -59,7 +59,14 @@ static u32 get_policy(const char *name)
> > >   		return PLPKS_SIGNEDUPDATE;
> > >   }
> > > -static const char * const plpks_var_names[] = {
> > > +static const char * const plpks_var_names_static[] = {
> > > +	"PK",
> > > +	"moduledb",
> > > +	"trustedcadb",
> > > +	NULL,
> > > +};
> > > +
> > > +static const char * const plpks_var_names_dynamic[] = {
> > >   	"PK",
> > >   	"KEK",
> > >   	"db",
> > > @@ -213,21 +220,34 @@ static int plpks_max_size(u64 *max_size)
> > >   	return 0;
> > >   }
> > > +static const struct secvar_operations plpks_secvar_ops_static = {
> > > +	.get = plpks_get_variable,
> > > +	.set = plpks_set_variable,
> > > +	.format = plpks_secvar_format,
> > > +	.max_size = plpks_max_size,
> > > +	.config_attrs = config_attrs,
> > > +	.var_names = plpks_var_names_static,
> > > +};
> > > -static const struct secvar_operations plpks_secvar_ops = {
> > > +static const struct secvar_operations plpks_secvar_ops_dynamic = {
> > >   	.get = plpks_get_variable,
> > >   	.set = plpks_set_variable,
> > >   	.format = plpks_secvar_format,
> > >   	.max_size = plpks_max_size,
> > >   	.config_attrs = config_attrs,
> > > -	.var_names = plpks_var_names,
> > > +	.var_names = plpks_var_names_dynamic,
> > >   };
> > >   static int plpks_secvar_init(void)
> > >   {
> > > +	u8 mode;
> > > +
> > >   	if (!plpks_is_available())
> > >   		return -ENODEV;
> > > -	return set_secvar_ops(&plpks_secvar_ops);
> > > +	mode = plpks_get_sb_keymgmt_mode();
> > > +	if (mode)
> > > +		return set_secvar_ops(&plpks_secvar_ops_dynamic);
> > > +	return set_secvar_ops(&plpks_secvar_ops_static);
> > >   }
> > >   machine_device_initcall(pseries, plpks_secvar_init);
> > > -- 
> > > 2.47.1
> > > 
> > > 
>