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

[Mickaël Salaün]

On 16/10/2020 10:29, Mickaël Salaün wrote:
> 
> On 15/10/2020 18:52, Mike Snitzer wrote:
>> On Thu, Oct 15 2020 at 11:05am -0400,
>> Mickaël Salaün <mic@xxxxxxxxxxx> wrote:
>>
>>> From: Mickaël Salaün <mic@xxxxxxxxxxxxxxxxxxx>
>>>
>>> Add a new configuration DM_VERITY_VERIFY_ROOTHASH_SIG_SECONDARY_KEYRING
>>> to enable dm-verity signatures to be verified against the secondary
>>> trusted keyring.  Instead of relying on the builtin trusted keyring
>>> (with hard-coded certificates), the second trusted keyring can include
>>> certificate authorities from the builtin trusted keyring and child
>>> certificates loaded at run time.  Using the secondary trusted keyring
>>> enables to use dm-verity disks (e.g. loop devices) signed by keys which
>>> did not exist at kernel build time, leveraging the certificate chain of
>>> trust model.  In practice, this makes it possible to update certificates
>>> without kernel update and reboot, aligning with module and kernel
>>> (kexec) signature verification which already use the secondary trusted
>>> keyring.
>>>
>>> Cc: Alasdair Kergon <agk@xxxxxxxxxx>
>>> Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
>>> Cc: Jarkko Sakkinen <jarkko.sakkinen@xxxxxxxxxxxxxxx>
>>> Cc: Jaskaran Khurana <jaskarankhurana@xxxxxxxxxxxxxxxxxxx>
>>> Cc: Mike Snitzer <snitzer@xxxxxxxxxx>
>>> Cc: Milan Broz <gmazyland@xxxxxxxxx>
>>> Signed-off-by: Mickaël Salaün <mic@xxxxxxxxxxxxxxxxxxx>
>>> ---
>>>
>>> Previous version:
>>> https://lore.kernel.org/lkml/20201002071802.535023-1-mic@xxxxxxxxxxx/
>>>
>>> Changes since v1:
>>> * Extend the commit message (asked by Jarkko Sakkinen).
>>> * Rename the Kconfig "help" keyword according to commit 84af7a6194e4
>>>   ("checkpatch: kconfig: prefer 'help' over '---help---'").
>>
>> 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.

> 
>>
>> Regardless, I really don't see why a Kconfig knob is appropriate.
> 
> Moreover, a Kconfig knob makes sense as much as
> DM_VERITY_VERIFY_ROOTHASH_SIG,
> IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY, MODULE_SIG_FORCE and
> other similar authentication mechanisms. Indeed, when using these
> configurations, we want the kernel to enforce a specific policy.
> 
> Obviously, we can't make the DM_VERITY_VERIFY_ROOTHASH_SIG relies on the
> secondary trusted keyring without important security implications for
> systems already using this configuration (and then the builtin trusted
> keyring as the unique source of trust).
>