Re: [PATCH v3 2/3] LoadPin: Enable loading from trusted dm-verity devices

[Kees Cook]

On May 13, 2022 9:32:12 AM PDT, Mike Snitzer <snitzer at redhat.com> wrote:
>On Wed, May 04 2022 at  3:54P -0400,
>Matthias Kaehlcke <mka at chromium.org> wrote:
>
>> Extend LoadPin to allow loading of kernel files from trusted dm-verity [1]
>> devices.
>> 
>> This change adds the concept of trusted verity devices to LoadPin. LoadPin
>> maintains a list of root digests of verity devices it considers trusted.
>> Userspace can populate this list through an ioctl on the new LoadPin
>> securityfs entry 'dm-verity'. The ioctl receives a file descriptor of
>> a file with verity digests as parameter. Verity reads the digests from
>> this file after confirming that the file is located on the pinned root.
>> The list of trusted digests can only be set up once, which is typically
>> done at boot time.
>> 
>> When a kernel file is read LoadPin first checks (as usual) whether the file
>> is located on the pinned root, if so the file can be loaded. Otherwise, if
>> the verity extension is enabled, LoadPin determines whether the file is
>> located on a verity backed device and whether the root digest of that
>> device is in the list of trusted digests. The file can be loaded if the
>> verity device has a trusted root digest.
>> 
>> Background:
>> 
>> As of now LoadPin restricts loading of kernel files to a single pinned
>> filesystem, typically the rootfs. This works for many systems, however it
>> can result in a bloated rootfs (and OTA updates) on platforms where
>> multiple boards with different hardware configurations use the same rootfs
>> image. Especially when 'optional' files are large it may be preferable to
>> download/install them only when they are actually needed by a given board.
>> Chrome OS uses Downloadable Content (DLC) [2] to deploy certain 'packages'
>> at runtime. As an example a DLC package could contain firmware for a
>> peripheral that is not present on all boards. DLCs use dm-verity to verify
>> the integrity of the DLC content.
>> 
>> [1] https://www.kernel.org/doc/html/latest/admin-guide/device-mapper/verity.html
>> [2] https://chromium.googlesource.com/chromiumos/platform2/+/HEAD/dlcservice/docs/developer.md
>> 
>> Signed-off-by: Matthias Kaehlcke <mka at chromium.org>
>> ---
>> 
>> Changes in v3:
>> - added securityfs for LoadPin (currently only populated when
>>   CONFIG_SECURITY_LOADPIN_VERITY=y)
>> - added uapi include for LoadPin
>> - changed the interface for setting up the list of trusted
>>   digests from sysctl to ioctl on securityfs entry
>> - added stub for loadpin_is_fs_trusted() to be used
>>   CONFIG_SECURITY_LOADPIN_VERITY is not select
>> - depend on CONFIG_SECURITYFS instead of CONFIG_SYSTCL
>> - updated Kconfig help
>> - minor changes in read_trusted_verity_root_digests()
>> - updated commit message
>> 
>> Changes in v2:
>> - userspace now passes the path of the file with the verity digests
>>   via systcl, instead of the digests themselves
>> - renamed sysctl file to 'trusted_verity_root_digests_path'
>> - have CONFIG_SECURITY_LOADPIN_VERITY depend on CONFIG_SYSCTL
>> - updated Kconfig doc
>> - updated commit message
>> 
>>  include/uapi/linux/loadpin.h |  19 ++++
>>  security/loadpin/Kconfig     |  16 +++
>>  security/loadpin/loadpin.c   | 184 ++++++++++++++++++++++++++++++++++-
>>  3 files changed, 218 insertions(+), 1 deletion(-)
>>  create mode 100644 include/uapi/linux/loadpin.h
>
>I would certainly need some Reviewed-by:s from security and/or loadpin
>experts if I were to pick this patch up.

Alternatively, since it's mostly touching loadpin, I can carry it in my tree, as long as you've Acked the dm bits. :)

>Did you see the issues the kernel test robot emailed about?
>
>You'd do well to fix those issues up when submitting another revision
>of this patchset.

Agreed.


-- 
Kees Cook