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
