Re: [dm-devel] [RFC PATCH v9 13/16] ipe: enable support for fs-verity as a trust provider

[Fan Wu <wufan@linux.microsoft.com>]

On Thu, Feb 02, 2023 at 10:51:56AM +0100, Roberto Sassu wrote:
> 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
> 
It does not accept any signature. For fsverity, the signature must be signed
by a key in the fsverity_keyring and similarly for dmverity the signature
must be signed by a key in the kernel builtin trusted keys or secondary keyring.
Therefore, the root of trust here is the system configured keyrings. 

The Boolean properties dmverity_signature and fsverity_signature are used
to differentiate the existence of signature because the signature is optional.
In a =TRUE case of these two properties, we know the digests are signed
by a key we can trust. And in a =FALSE case we know the file is from a unsigned
dmverity or fsverity, we could use a stricter policy to deny them. 

I agree that having the ability to restrict signers is better, but the feedback
from the last version was asking us to keep the initial version as simple
as possible. We definitely want to add more properties, but we will invest
more time in them once the initial version is accepted. 

-Fan

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel