Message-ID: <6ddfa7344d01b21a93d3909af9dac0ae5e2a79ee.camel@huaweicloud.com>
Subject: Re: [RFC PATCH v9 13/16] ipe: enable support for fs-verity as a
 trust provider
From:   Roberto Sassu <roberto.sassu@huaweicloud.com>
To:     Fan Wu <wufan@linux.microsoft.com>
Cc:     corbet@lwn.net, zohar@linux.ibm.com, jmorris@namei.org,
        serge@hallyn.com, tytso@mit.edu, ebiggers@kernel.org,
        axboe@kernel.dk, agk@redhat.com, snitzer@kernel.org,
        eparis@redhat.com, paul@paul-moore.com, linux-doc@vger.kernel.org,
        linux-integrity@vger.kernel.org,
        linux-security-module@vger.kernel.org,
        linux-fscrypt@vger.kernel.org, linux-block@vger.kernel.org,
        dm-devel@redhat.com, linux-audit@redhat.com,
        roberto.sassu@huawei.com, linux-kernel@vger.kernel.org,
        Deven Bowers <deven.desai@linux.microsoft.com>
Date:   Thu, 02 Feb 2023 10:51:56 +0100
In-Reply-To: <20230201235031.GC9075@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
References: <1675119451-23180-1-git-send-email-wufan@linux.microsoft.com>
         <1675119451-23180-14-git-send-email-wufan@linux.microsoft.com>
         <d62907da62b5e0b25c9d7bd4b3119a3d1827bd29.camel@huaweicloud.com>
         <20230201235031.GC9075@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.36.5-0ubuntu1 
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Wed, 2023-02-01 at 15:50 -0800, Fan Wu wrote:
> On Tue, Jan 31, 2023 at 03:00:08PM +0100, Roberto Sassu wrote:
> > On Mon, 2023-01-30 at 14:57 -0800, Fan Wu wrote:
> > > +/**
> > > + * evaluate_fsv_sig_false - Analyze @ctx against a fsv sig false property.
> > > + * @ctx: Supplies a pointer to the context being evaluated.
> > > + * @p: Supplies a pointer to the property being evaluated.
> > > + *
> > > + * Return:
> > > + * * true	- The current @ctx match the @p
> > > + * * false	- The current @ctx doesn't match the @p
> > > + */
> > > +static bool evaluate_fsv_sig_false(const struct ipe_eval_ctx *const ctx,
> > > +				   struct ipe_prop *p)
> > > +{
> > > +	return !ctx->ino ||
> > > +	       !IS_VERITY(ctx->ino) ||
> > > +	       !ctx->ipe_inode ||
> > > +	       !ctx->ipe_inode->fs_verity_signed;
> > > +}
> > > +
> > > +/**
> > > + * evaluate_fsv_sig_true - Analyze @ctx against a fsv sig true property.
> > > + * @ctx: Supplies a pointer to the context being evaluated.
> > > + * @p: Supplies a pointer to the property being evaluated.
> > > + *
> > > + * Return:
> > > + * * true - The current @ctx match the @p
> > > + * * false - The current @ctx doesn't match the @p
> > > + */
> > > +static bool evaluate_fsv_sig_true(const struct ipe_eval_ctx *const ctx,
> > > +				  struct ipe_prop *p)
> > > +{
> > > +	return ctx->ino &&
> > > +	       IS_VERITY(ctx->ino) &&
> > > +	       ctx->ipe_inode &&
> > > +	       ctx->ipe_inode->fs_verity_signed;
> > > +}
> > 
> > Isn't better to just define one function and prepend a ! in
> > evaluate_property()?
> Yes that's a better way to do it, I will take this idea.
> 
> > Not sure about the usefulness of the fsverity_signature= property as it
> > is. I would at minimum allow to specify which keyring signatures are
> > verified against, and ensure that the keyring has a restriction.
> > 
> > And maybe I would call fsverity_verify_signature() directly, after
> > extending it to pass the desired keyring.
> > 
> Thanks for the suggestion.
> For the initial version we only have the fsverity_signature property
> to enable the policy can make decision based on the existence of the
> signature. In the future we plan to add more properties to leverage
> the remaining signature information so we can have the restrictions
> you mentioned.

Uhm, these boolean properties feel like something is missing. In my
opinion, one cannot accept just any signature, but should be able to
specify the approved signers.

Roberto

> -Fan
> 
> > I would also split this patch in two, one for fsverity_digest= and one
> > for fsverity_signature=.
> > 
> > Roberto