Message-ID: <20221209221719.GA3055@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
Date:   Fri, 9 Dec 2022 14:17:19 -0800
From:   Fan Wu <wufan@...ux.microsoft.com>
To:     Eric Biggers <ebiggers@...nel.org>
Cc:     Luca Boccassi <bluca@...ian.org>, linux-fscrypt@...r.kernel.org,
        linux-ext4@...r.kernel.org, linux-f2fs-devel@...ts.sourceforge.net,
        linux-btrfs@...r.kernel.org, linux-integrity@...r.kernel.org,
        Jes Sorensen <jsorensen@...a.com>,
        Victor Hsieh <victorhsieh@...gle.com>
Subject: Re: [PATCH] fsverity: mark builtin signatures as deprecated

On Thu, Dec 08, 2022 at 12:55:33PM -0800, Eric Biggers wrote:
> On Thu, Dec 08, 2022 at 10:43:01AM +0000, Luca Boccassi wrote:
> > On Wed, 2022-12-07 at 19:35 -0800, Eric Biggers wrote:
> > > From: Eric Biggers <ebiggers@...gle.com>
> > > 
> > > fsverity builtin signatures, at least as currently implemented, are a
> > > mistake and should not be used.  They mix the authentication policy
> > > between the kernel and userspace, which is not a clean design and causes
> > > confusion.  For builtin signatures to actually provide any security
> > > benefit, userspace still has to enforce that specific files have
> > > fsverity enabled.  Since userspace needs to do this, a better design is
> > > to have that same userspace code do the signature check too.
> > > 
> > > That allows better signature formats and algorithms to be used, avoiding
> > > in-kernel parsing of the notoriously bad PKCS#7 format.  It is also
> > > needed anyway when different keys need to be trusted for different
> > > files, or when it's desired to use fsverity for integrity-only or
> > > auditing on some files and for authenticity on other files.  Basically,
> > > the builtin signatures don't work for any nontrivial use case.
> > > 
> > > (IMA appraisal is another alternative.  It goes in the opposite
> > > direction -- the full policy is moved into the kernel.)
> > > 
> > > For these reasons, the master branch of AOSP no longer uses builtin
> > > signatures.  It still uses fsverity for some files, but signatures are
> > > verified in userspace when needed.
> > > 
> > > None of the public uses of builtin signatures outside Android seem to
> > > have gotten going, either.  Support for builtin signatures was added to
> > > RPM.  However, https://fedoraproject.org/wiki/Changes/FsVerityRPM was
> > > subsequently rejected from Fedora and seems to have been abandoned.
> > > There is also https://github.com/ostreedev/ostree/pull/2269, which was
> > > never merged.  Neither proposal mentioned a plan to set
> > > fs.verity.require_signatures=1 and enforce that files have fs-verity
> > > enabled -- so, they would have had no security benefit on their own.
> > > 
> > > I'd be glad to hear about any other users of builtin signatures that may
> > > exist, and help with the details of what should be used instead.
> > > 
> > > Anyway, the feature can't simply be removed, due to the need to maintain
> > > backwards compatibility.  But let's at least make it clear that it's
> > > deprecated.  Update the documentation accordingly, and rename the
> > > kconfig option to CONFIG_FS_VERITY_DEPRECATED_BUILTINSIG.  Also remove
> > > the kconfig option from the s390 defconfigs, as it's unneeded there.
> > 
> > Hi,
> > 
> > Thanks for starting this discussion, it's an interesting topic.
> > 
> > At MSFT we use fsverity in production, with signatures enforced by the
> > kernel (and policy enforced by the IPE LSM). It's just too easy to fool
> > userspace with well-timed swaps and who knows what else. This is not
> > any different from dm-verity from our POV, it complements it. I very
> > much want the kernel to be in charge of verification and validation, at
> > the time of use.
> 
> Well, IPE is not upstream, and it duplicates functionality that already exists
> upstream (IMA appraisal).  So from an upstream perspective it doesn't really
> matter currently.  That's interesting that you've already deployed IPE in
> production anyway.  To re-iterate my question at
> https://lore.kernel.org/r/YqKGcdM3t5gjqBpq@sol.localdomain which was ignored,
> can you elaborate on why IPE should exist when IMA appraisal already exists (and
> already supports fsverity), and why IPE uses the fsverity builtin signatures?
> And are you sure that X.509 and PKCS#7 should be used in a new system?  These
> days, if you go through any sort of crypto or security review, you will be told
> not to use those formats since they are overly complex and prone to bugs.
> 
> - Eric

IPE is just a different approach, like btrfs and ext4,
for a small subset of IMA's features: appraisal.

IMA is a fantastic solution for a whole lot of scenarios that IPE will not
cover, now or ever. In fact, in production, we use IMA (both measurement
and appraisal) and IPE side-by-side. They're not mutually exclusive.
 
IPE's approach is to really focus on the policy enforcement side, delegating
the authenticity (or even just integrity) claim to other subsystems. It does
this by enforcing an opaqueness between these integrations with other subsystems,
drawing a clear line between the mechanism of the authenticity claim,
and the enforcement of that claim against authorized values in policy. This
allows for a variable security bar based on what mechanisms are selected to
be used in the system, and what are present in the policy. For instance, the security
bar of a system using dm-verity with signed root hashes as its primary authenticity
claim, as opposed to fs-verity will have a different security bar.

This is why IPE uses the built-in signatures: they are managed by fs-verity,
and IPE does not implement its own signature format or other method of
validating either the integrity or authenticity of a data source,
explicitly, as that would be contrary to its design philosophy.

We have also noticed many use cases for the fs-verity build-in signatures. Proposals
exist to use them[1]. Package managers were updated to use them[2]. We are
successfully using them in production. Therefore we prefer to keep the existing
build-in signatures.

[1]https://fedoraproject.org/wiki/Changes/FsVerityRPM#Enable_fs-verity_in_RPM
[2]https://github.com/rpm-software-management/rpm/issues/1121

- Fan

Powered by blists - more mailing lists