Re: [PATCH v2] ima: add gid support

[Mimi Zohar <zohar@linux.ibm.com>]

Hi Alex,

On Mon, 2021-10-04 at 18:32 -0600, Alex Henrie wrote:
> From: Curtis Veit <veit@vpieng.com>
> 
> IMA currently supports the concept of rules based on uid where the rule
> is based on the uid of the file owner or the uid of the user accessing
> the file. It is useful to have similar rules based on gid. This patch
> provides that ability.
> 
> Signed-off-by: Curtis Veit <veit@vpieng.com>
> Co-developed-by: Alex Henrie <alexh@vpitech.com>
> Signed-off-by: Alex Henrie <alexh@vpitech.com>
> ---
> v2: Trivial changes that Mimi requested

Sorry, scripts/check-patch.pl reported some warnings.  Two more trivial changes.

> ---
>  Documentation/ABI/testing/ima_policy |   8 +-
>  security/integrity/ima/ima_policy.c  | 201 +++++++++++++++++++++++----
>  2 files changed, 180 insertions(+), 29 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
> index 5c2798534950..e1a04bd3b9e5 100644
> --- a/Documentation/ABI/testing/ima_policy

> @@ -78,9 +81,13 @@ struct ima_rule_entry {
>  	unsigned long fsmagic;
>  	uuid_t fsuuid;
>  	kuid_t uid;
> +	kgid_t gid;
>  	kuid_t fowner;
> +	kgid_t fgroup;
>  	bool (*uid_op)(kuid_t, kuid_t);    /* Handlers for operators       */
> +	bool (*gid_op)(kgid_t, kgid_t);
>  	bool (*fowner_op)(kuid_t, kuid_t); /* uid_eq(), uid_gt(), uid_lt() */
> +	bool (*fgroup_op)(kgid_t, kgid_t); /* gid_eq(), gid_gt(), gid_lt() */

scripts/checkpatch.pl complains about missing variables.

>  	int pcr;
>  	unsigned int allowed_algos; /* bitfield of allowed hash algorithms */
>  	struct {
> 
> @@ -582,10 +590,23 @@ static bool ima_match_rules(struct ima_rule_entry *rule,
>  		} else if (!rule->uid_op(cred->euid, rule->uid))
>  			return false;
>  	}
> -
> +	if ((rule->flags & IMA_GID) && !rule->gid_op(rule->gid, cred->gid))

All of uid_op/gid_op calls in ima_match_rules() pass the "cred->xxxx,
rule->xxx" except here, where it is rule->gid, cred->rule.   Reversing
the parameters here will help with addressing the checkpatch.pl
warning.

thanks,

Mimi