Re: [PATCH] dm verity: fallback to platform keyring also if key in trusted keyring is rejected

["Jarkko Sakkinen" <jarkko@kernel.org>]

On Tue Sep 24, 2024 at 9:27 PM EEST, Mikulas Patocka wrote:
>
>
> On Tue, 24 Sep 2024, Jarkko Sakkinen wrote:
>
> > On Mon Sep 23, 2024 at 5:04 PM EEST, Mikulas Patocka wrote:
> > >
> > >
> > > On Sun, 22 Sep 2024, luca.boccassi@gmail.com wrote:
> > >
> > > > From: Luca Boccassi <bluca@debian.org>
> > > > 
> > > > If enabled, we fallback to the platform keyring if the trusted keyring doesn't have
> > > > the key used to sign the roothash. But if pkcs7_verify() rejects the key for other
> > > > reasons, such as usage restrictions, we do not fallback. Do so.
> > > > 
> > > > Follow-up for 6fce1f40e95182ebbfe1ee3096b8fc0b37903269
> > > > 
> > > > Suggested-by: Serge Hallyn <serge@hallyn.com>
> > > > Signed-off-by: Luca Boccassi <bluca@debian.org>
> > >
> > > Hi
> > >
> > > I'm not an expert in keyrings.
> > >
> > > I added keyring maintainers to the CC. Please review this patch and 
> > > Ack/Nack it.
> > >
> > > Mikulas
> > >
> > > > ---
> > > >  drivers/md/dm-verity-verify-sig.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/md/dm-verity-verify-sig.c b/drivers/md/dm-verity-verify-sig.c
> > > > index d351d7d39c60..a9e2c6c0a33c 100644
> > > > --- a/drivers/md/dm-verity-verify-sig.c
> > > > +++ b/drivers/md/dm-verity-verify-sig.c
> > > > @@ -127,7 +127,7 @@ int verity_verify_root_hash(const void *root_hash, size_t root_hash_len,
> > > >  #endif
> > > >  				VERIFYING_UNSPECIFIED_SIGNATURE, NULL, NULL);
> > > >  #ifdef CONFIG_DM_VERITY_VERIFY_ROOTHASH_SIG_PLATFORM_KEYRING
> > > > -	if (ret == -ENOKEY)
> > > > +	if (ret == -ENOKEY || ret == -EKEYREJECTED)
> > > >  		ret = verify_pkcs7_signature(root_hash, root_hash_len, sig_data,
> > > >  					sig_len,
> > > >  					VERIFY_USE_PLATFORM_KEYRING,
> > > > -- 
> > > > 2.39.5
> > > > 
> > 
> > I know nothing about dm-verity. What does it even do?
> > 
> > BR, Jarkko
>
> dm-verity provides a read-only device with integrity checking. dm-verity 
> stores hash for every block on the block device and checks the hash when 
> reading the block. If the hash doesn't match, it can do one of these 
> actions (depending on configuration):
> - return I/O error
> - try to correct the data using forward error correction
> - log the mismatch and do nothing
> - restart the machine
> - call panic()
>
> dm-verity is mostly used for the immutable system partition on Android 
> phones. For more info, see 
> Documentation/admin-guide/device-mapper/verity.rst
>
> The above patch changes the way that the signature of the root hash is 
> verified. I have no clue whether the patch can or can't subvert system 
> security, that's why I'd like to have some more reviews of the patch 
> before accepting it.

I guess someone who knows all this already should review it.

Doesn't dm-verity have a maintainer?

BR, Jarkko