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 >
