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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 23 Oct 2020 12:20:09 +0200
From:   Mickaël Salaün <mic@...ikod.net>
To:     Mike Snitzer <snitzer@...hat.com>, Alasdair Kergon <agk@...hat.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Deven Bowers <deven.desai@...ux.microsoft.com>,
        Jaskaran Khurana <jaskarankhurana@...ux.microsoft.com>,
        dm-devel@...hat.com, linux-integrity@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>,
        Mickaël Salaün <mic@...ux.microsoft.com>,
        Milan Broz <gmazyland@...il.com>
Subject: Re: [PATCH v2] dm verity: Add support for signature verification with
 2nd keyring

It seems that there is no more question. Mike, Alasdair, could you
please consider to merge this into the tree?

On 16/10/2020 14:19, Mickaël Salaün wrote:
> 
> On 16/10/2020 13:08, Milan Broz wrote:
>> 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.
> It doesn't make sense to set DM_VERITY_VERIFY_ROOTHASH_SIG in generic
> distro because such policy is configured at build time in the kernel
> with hardcoded CAs. If the new option is not set then nothing change. I
> don't see why it could be an issue for use cases we previously defined
> (with DM_VERITY_VERIFY_ROOTHASH_SIG).
> 
>> 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 documentation remains true.
> DM_VERITY_VERIFY_ROOTHASH_SIG_SECONDARY_KEYRING depends on
> DM_VERITY_VERIFY_ROOTHASH_SIG.
> 
>>
>> The current userspace configuration through veritysetup does not need
>> any patches for your patch, correct?
> 
> Right, it's only different from the kernel point of view.
> 
>>
>> Thanks,
>> Milan
>>