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]

Message-ID: <55389f91-60a5-05db-b3e1-8f24aa356893@digikod.net>
Date:   Fri, 16 Oct 2020 14:19:12 +0200
From:   Mickaël Salaün <mic@...ikod.net>
To:     Milan Broz <gmazyland@...il.com>, Mike Snitzer <snitzer@...hat.com>
Cc:     Alasdair Kergon <agk@...hat.com>,
        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>
Subject: Re: [PATCH v2] dm verity: Add support for signature verification with
 2nd keyring


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
>