Re: [dm-devel] [PATCH v2] dm verity: Add support for signature verification with 2nd keyring

From: Milan Broz <gmazyland@gmail.com>
To: "Mickaël Salaün" <mic@digikod.net>, "Mike Snitzer" <snitzer@redhat.com>
Cc: "Deven Bowers" <deven.desai@linux.microsoft.com>,
	linux-kernel@vger.kernel.org,
	"Jarkko Sakkinen" <jarkko.sakkinen@linux.intel.com>,
	"Mickaël Salaün" <mic@linux.microsoft.com>,
	dm-devel@redhat.com, linux-integrity@vger.kernel.org,
	"Andrew Morton" <akpm@linux-foundation.org>,
	"Alasdair Kergon" <agk@redhat.com>,
	"Jaskaran Khurana" <jaskarankhurana@linux.microsoft.com>
Subject: Re: [dm-devel] [PATCH v2] dm verity: Add support for signature verification with 2nd keyring
Date: Fri, 16 Oct 2020 13:08:43 +0200	[thread overview]
Message-ID: <b7ccaa01-0398-f108-a70d-c67753d9fa6d@gmail.com> (raw)
In-Reply-To: <b7ba2ff9-5f5f-8c1e-dfaa-33da56d3d8de@digikod.net>

On 16/10/2020 10:49, MickaÃ«l SalaÃŒn wrote:
> On 16/10/2020 10:29, MickaÃ«l SalaÃŒn wrote:
>>
>> On 15/10/2020 18:52, Mike Snitzer wrote:
>>> Can you please explain why you've decided to make this a Kconfig CONFIG
>>> knob?  Why not either add: a dm-verity table argument? A dm-verity
>>> kernel module parameter? or both (to allow a particular default but then
>>> per-device override)?
>>
>> The purpose of signed dm-verity images is to authenticate files, or said
>> in another way, to enable the kernel to trust disk images in a flexible
>> way (i.e. thanks to certificate's chain of trust). Being able to update
>> such chain at run time requires to use the second trusted keyring. This
>> keyring automatically includes the certificate authorities from the
>> builtin trusted keyring, which are required to dynamically populate the
>> secondary trusted keyring with certificates signed by an already trusted
>> authority. The roots of trust must then be included at build time in the
>> builtin trusted keyring.
>>
>> To be meaningful, using dm-verity signatures implies to have a
>> restricted user space, i.e. even the root user has limited power over
>> the kernel and the rest of the system. Blindly trusting data provided by
>> user space (e.g. dm-verity table argument, kernel module parameter)
>> defeat the purpose of (mandatory) authenticated images.
>>
>>>
>>> Otherwise, _all_ DM verity devices will be configured to use secondary
>>> keyring fallback.  Is that really desirable?
>>
>> That is already the current state (on purpose).
> 
> I meant that when DM_VERITY_VERIFY_ROOTHASH_SIG is set, dm-verity
> signature becomes mandatory. This new configuration
> DM_VERITY_VERIFY_ROOTHASH_SIG_SECONDARY_KEYRING extend this trust to the
> secondary trusted keyring, which contains certificates signed (directly
> or indirectly) by CA from the builtin trusted keyring.
> 
> So yes, this new (optional) configuration *extends* the source of trust
> for all dm-verity devices, and yes, it is desirable. I think it should
> have been this way from the beginning (as for other authentication
> mechanisms) but it wasn't necessary at that time.

Well, I understand why you need a config option here.
And using the secondary keyring actually makes much more sense to me than
the original approach.

But please do not forget that dm-verity is sometimes used in different
contexts where such strict in-kernel certificate trust is unnecessary.
With your configure options set, you deliberately remove the possibility
to configure such devices.
I understand that it is needed for "trusted" systems, but we should be clear
in the documentation.
Maybe also add note to /Documentation/admin-guide/device-mapper/verity.rst ?
We already mention DM_VERITY_VERIFY_ROOTHASH_SIG there.

The current userspace configuration through veritysetup does not need
any patches for your patch, correct?

Thanks,
Milan

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel