[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <fbd87f01b5463e399900df8dcb094a9039bd589f.camel@linux.ibm.com>
Date: Wed, 26 Mar 2025 09:17:05 -0400
From: Mimi Zohar <zohar@...ux.ibm.com>
To: Nicolai Stange <nstange@...e.de>
Cc: Roberto Sassu <roberto.sassu@...wei.com>,
Dmitry Kasatkin
<dmitry.kasatkin@...il.com>,
Eric Snowberg <eric.snowberg@...cle.com>,
Jarkko Sakkinen <jarkko@...nel.org>,
James Bottomley
<James.Bottomley@...senPartnership.com>,
linux-integrity@...r.kernel.org, linux-security-module@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH v2 02/13] ima: always create runtime_measurements
sysfs file for ima_hash
On Wed, 2025-03-26 at 09:21 +0100, Nicolai Stange wrote:
> Mimi Zohar <zohar@...ux.ibm.com> writes:
>
> > On Sun, 2025-03-23 at 15:09 +0100, Nicolai Stange wrote:
> > > runtime_measurements_<hash-algo> sysfs files are getting created for
> > > each PCR bank + for SHA-1.
> > >
> > > Now that runtime_measurements_<hash-algo> sysfs file creation is being
> > > skipped for unsupported hash algorithms, it will become possible that no
> > > such file would be provided at all once SHA-1 is made optional in a
> > > later patch.
> > >
> > > Always create the file for the 'ima_hash' algorithm, even if it's not
> > > associated with any of the PCR banks. As IMA initialization will
> > > continue to fail if the ima_hash algorithm is not available to the
> > > kernel, this guarantees that at least one such file will always be
> > > there.
> > >
> > > Signed-off-by: Nicolai Stange <nstange@...e.de>
> > > ---
> > > security/integrity/ima/ima_fs.c | 6 ++----
> > > 1 file changed, 2 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
> > > index a8df2fe5f4cb..f030ff7f56da 100644
> > > --- a/security/integrity/ima/ima_fs.c
> > > +++ b/security/integrity/ima/ima_fs.c
> > > @@ -436,10 +436,8 @@ static int __init create_securityfs_measurement_lists(void)
> > > u16 algo;
> > > int i;
> > >
> > > - securityfs_measurement_list_count = NR_BANKS(ima_tpm_chip);
> > > -
> > > - if (ima_sha1_idx >= NR_BANKS(ima_tpm_chip))
> > > - securityfs_measurement_list_count++;
> > > + securityfs_measurement_list_count =
> > > + NR_BANKS(ima_tpm_chip) + ima_extra_slots;
> > >
> > > ascii_securityfs_measurement_lists =
> > > kcalloc(securityfs_measurement_list_count, sizeof(struct dentry *),
> >
> > "ima_hash" is the default file hash algorithm. Re-using it as the default
> > complete measurement list assumes that the subsequent kexec'ed kernels configure
> > and define it as the default file hash algorithm as well, which might not be the
> > case.
>
> I don't really see why the ima_hashes would have to match between kexecs
> for this to work -- all events' template hashes are getting recreated
> from scratch anyway after kexec (ima_restore_measurement_list() ->
> ima_calc_field_array_hash()).
>
> That is, if ima_hash=sha256 first, and ima_hash=sha384 after kexec, one
> would have *runtime_measurements_sha256 first and
> *runtime_measurements_sha384 after kexec. And both had exclusively
> template hashes of their respective algo in them each.
>
> What am I missing?
Your solution would work nicely, if the "ima_hash" algorithm could be guaranteed
to be built into the kernel. It's highly unlikely someone would choose a hash
algorithm not built into kernel, but it is possible. hash_setup() only verifies
that the hash algorithm is a valid name.
Either fix hash_setup() to guarantee that the chosen hash algorithm is built
into the kernel or use the CONFIG_IMA_DEFAULT_HASH and add a Kconfig to select
the hash algorithm. This would be in lieu of v2 05/13.
> > Drop this patch.
>
> Fine by me, but just to confirm, in case there's no TPM attached and
> SHA1 was disabled, there would be no /sys/*/*runtime_measurement* at all
> then. Is that Ok?
Of course not. :)
> ima_hash was chosen here only, because after this series, it will be the
> only single algorithm guaranteed to be available.
With the proposed changes to "[RFC PATCH v2 05/13] ima: select CRYPTO_SHA256
from Kconfig', SHA256 would be added to the "extra" measurements if the TPM
SHA256 bank is disabled.
Mimi
Powered by blists - more mailing lists