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

From: Matthias Kaehlcke <mka@chromium.org>
To: Kees Cook <keescook@chromium.org>
Cc: Alasdair Kergon <agk@redhat.com>,
	Mike Snitzer <snitzer@kernel.org>,
	James Morris <jmorris@namei.org>,
	"Serge E . Hallyn" <serge@hallyn.com>,
	linux-kernel@vger.kernel.org,
	linux-security-module@vger.kernel.org, dm-devel@redhat.com,
	linux-raid@vger.kernel.org,
	Douglas Anderson <dianders@chromium.org>,
	Song Liu <song@kernel.org>
Subject: Re: [PATCH v2 2/3] LoadPin: Enable loading from trusted dm-verity devices
Date: Mon, 2 May 2022 15:44:56 -0700	[thread overview]
Message-ID: <YnBe6K72iKSDSqk9@google.com> (raw)
In-Reply-To: <202204302316.AF04961@keescook>

On Sat, Apr 30, 2022 at 11:21:54PM -0700, Kees Cook wrote:
> On Tue, Apr 26, 2022 at 02:31:09PM -0700, Matthias Kaehlcke wrote:
> > I'm still doubting what would be the best way to configure
> > the list of trusted digests. The approach in v2 of writing
> > a path through sysctl is flexible, but it also feels a bit
> > odd. I did some experiments with passing a file descriptor
> > through sysctl, but it's also odd and has its own issues.
> > Passing the list through a kernel parameter seems hacky.
> > A Kconfig string would work, but can be have issues when
> > the same config is used for different platforms, where
> > some may have trusted digests and others not.
> 
> I prefer the idea of passing an fd, since that can just use LoadPin
> itself to verify the origin of the fd.
> 
> I also agree, though, that it's weird as a sysctl. Possible thoughts:
> 
> - make it a new ioctl on /dev/mapper/control (seems reasonable given
>   that it's specifically about dm devices).
> - have LoadPin grow a securityfs node, maybe something like
>   /sys/kernel/security/loadpin/dm-verify and do the ioctl there (seems
>   reasonable given that it's specifically about LoadPin, but is perhaps
>   more overhead to built the securityfs).

Thanks for your feedback!

Agreed that an ioctl is preferable over a sysctl interface. I wasn't aware
of securityfs and prefer it over a /dev/mapper/control ioctl. Ultimately
the list of digests is meaningful to LoadPin, not (directly) to the device
mapper / verity. I'm not sure how well this feature of integrating LoadPin
with verity will be by the verity maintainers in the first place, it's
probably best to limit the LoadPin specific stuff in verity to a minimum.
I experimented a bit with the securityfs option, building it doesn't seem
too much of an overhead. If loadpin.c ends up too cluttered with the
verity and securityfs stuff I could try to outsource some of it to (a)
dedicated file(s).