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. -Fan > I would also split this patch in two, one for fsverity_digest= and one > for fsverity_signature=. > > Roberto