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@xxxxxxxxxx https://listman.redhat.com/mailman/listinfo/dm-devel