On Wed Sep 25, 2024 at 12:59 AM EEST, Eric Biggers wrote: > On Wed, Sep 25, 2024 at 12:36:01AM +0300, Jarkko Sakkinen wrote: > > 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@xxxxxxxxx wrote: > > > > > > > > > > > From: Luca Boccassi <bluca@xxxxxxxxxx> > > > > > > > > > > > > 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@xxxxxxxxxx> > > > > > > Signed-off-by: Luca Boccassi <bluca@xxxxxxxxxx> > > > > > > > > > > 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? > > > > This patch only affects dm-verity's in-kernel signature verification support, > which has only been present since Linux v5.4 and is not used by Android or > Chrome OS. The whole feature seems weird to me, and it is prone to be misused; > signatures are best verified by trusted userspace code instead (e.g. initramfs). > But apparently there are people who use the dm-verity in-kernel signatures. I > think systemd has some support for it, as does the recently-upstreamed IPE LSM. > I don't know what else. The exact semantics of the "trusted" and "platform" > keyrings are not entirely clear to me, but given that dm-verity already trusts > keys from both keyrings this patch seems reasonable. The people who actually > use this feature are in the best position to make that call, though. https://lwn.net/Articles/459420/ Isn't Netflix using FreeBSD these days? :-) Right, and Chromebooks also mentioned in the same article (from 2011). For me this looks almost like abaddonware... > > - Eric BR, Jarkko
