linux-integrity.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH linux rebase] ima: add gid support
@ 2021-09-09 16:51 Alex Henrie
  2021-09-09 16:51 ` [PATCH ltp] IMA: Add tests for uid, gid, fowner, and fgroup options Alex Henrie
  2021-10-04 22:30 ` [PATCH linux rebase] ima: add gid support Mimi Zohar
  0 siblings, 2 replies; 6+ messages in thread
From: Alex Henrie @ 2021-09-09 16:51 UTC (permalink / raw)
  To: linux-integrity, ltp, zohar, pvorel, alexhenrie24
  Cc: Curtis Veit, Alex Henrie

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>
---
This is the patch that Curtis sent in 2015,[1] rebased and with support
for effective GID and the new greater-than and less-than operators.
Moreover, the policy_opt enum is now in the same order as the
policy_tokens array, which I'm guessing is the bug that prevented the
patch from being accepted before.

[1] https://sourceforge.net/p/linux-ima/mailman/message/34210250/
---
 Documentation/ABI/testing/ima_policy |   5 +-
 security/integrity/ima/ima_policy.c  | 197 +++++++++++++++++++++++----
 2 files changed, 174 insertions(+), 28 deletions(-)

diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
index 5c2798534950..f8e5b5598548 100644
--- a/Documentation/ABI/testing/ima_policy
+++ b/Documentation/ABI/testing/ima_policy
@@ -23,7 +23,7 @@ Description:
 			  audit | hash | dont_hash
 		  condition:= base | lsm  [option]
 			base:	[[func=] [mask=] [fsmagic=] [fsuuid=] [uid=]
-				[euid=] [fowner=] [fsname=]]
+				[euid=] [gid=] [egid=] [fowner=] [fgroup=] [fsname=]]
 			lsm:	[[subj_user=] [subj_role=] [subj_type=]
 				 [obj_user=] [obj_role=] [obj_type=]]
 			option:	[[appraise_type=]] [template=] [permit_directio]
@@ -40,7 +40,10 @@ Description:
 			fsuuid:= file system UUID (e.g 8bcbe394-4f13-4144-be8e-5aa9ea2ce2f6)
 			uid:= decimal value
 			euid:= decimal value
+			gid:= decimal value
+			egid:= decimal value
 			fowner:= decimal value
+			fgroup:= decimal value
 		  lsm:  are LSM specific
 		  option:
 			appraise_type:= [imasig] [imasig|modsig]
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 87b9b71cb820..0f2287699f85 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -36,6 +36,9 @@
 #define IMA_KEYRINGS	0x0400
 #define IMA_LABEL	0x0800
 #define IMA_VALIDATE_ALGOS	0x1000
+#define IMA_GID		0x2000
+#define IMA_EGID	0x4000
+#define IMA_FGROUP	0x8000
 
 #define UNKNOWN		0
 #define MEASURE		0x0001	/* same as IMA_MEASURE */
@@ -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() */
 	int pcr;
 	unsigned int allowed_algos; /* bitfield of allowed hash algorithms */
 	struct {
@@ -104,7 +111,8 @@ static_assert(
 
 /*
  * Without LSM specific knowledge, the default policy can only be
- * written in terms of .action, .func, .mask, .fsmagic, .uid, and .fowner
+ * written in terms of .action, .func, .mask, .fsmagic, .uid, .gid,
+ * .fowner, and .fgroup
  */
 
 /*
@@ -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))
+		return false;
+	if (rule->flags & IMA_EGID) {
+		if (has_capability_noaudit(current, CAP_SETGID)) {
+			if (!rule->gid_op(cred->egid, rule->gid)
+			    && !rule->gid_op(cred->sgid, rule->gid)
+			    && !rule->gid_op(cred->gid, rule->gid))
+				return false;
+		} else if (!rule->gid_op(cred->egid, rule->gid))
+			return false;
+	}
 	if ((rule->flags & IMA_FOWNER) &&
 	    !rule->fowner_op(i_uid_into_mnt(mnt_userns, inode), rule->fowner))
 		return false;
+	if ((rule->flags & IMA_FGROUP) &&
+	    !rule->fgroup_op(i_gid_into_mnt(mnt_userns, inode), rule->fgroup))
+		return false;
 	for (i = 0; i < MAX_LSM_RULES; i++) {
 		int rc = 0;
 		u32 osid;
@@ -987,16 +1008,19 @@ void ima_update_policy(void)
 }
 
 /* Keep the enumeration in sync with the policy_tokens! */
-enum {
+enum policy_opt {
 	Opt_measure, Opt_dont_measure,
 	Opt_appraise, Opt_dont_appraise,
 	Opt_audit, Opt_hash, Opt_dont_hash,
 	Opt_obj_user, Opt_obj_role, Opt_obj_type,
 	Opt_subj_user, Opt_subj_role, Opt_subj_type,
-	Opt_func, Opt_mask, Opt_fsmagic, Opt_fsname,
-	Opt_fsuuid, Opt_uid_eq, Opt_euid_eq, Opt_fowner_eq,
-	Opt_uid_gt, Opt_euid_gt, Opt_fowner_gt,
-	Opt_uid_lt, Opt_euid_lt, Opt_fowner_lt,
+	Opt_func, Opt_mask, Opt_fsmagic, Opt_fsname, Opt_fsuuid,
+	Opt_uid_eq, Opt_euid_eq, Opt_gid_eq, Opt_egid_eq,
+	Opt_fowner_eq, Opt_fgroup_eq,
+	Opt_uid_gt, Opt_euid_gt, Opt_gid_gt, Opt_egid_gt,
+	Opt_fowner_gt, Opt_fgroup_gt,
+	Opt_uid_lt, Opt_euid_lt, Opt_gid_lt, Opt_egid_lt,
+	Opt_fowner_lt, Opt_fgroup_lt,
 	Opt_appraise_type, Opt_appraise_flag, Opt_appraise_algos,
 	Opt_permit_directio, Opt_pcr, Opt_template, Opt_keyrings,
 	Opt_label, Opt_err
@@ -1023,13 +1047,22 @@ static const match_table_t policy_tokens = {
 	{Opt_fsuuid, "fsuuid=%s"},
 	{Opt_uid_eq, "uid=%s"},
 	{Opt_euid_eq, "euid=%s"},
+	{Opt_gid_eq, "gid=%s"},
+	{Opt_egid_eq, "egid=%s"},
 	{Opt_fowner_eq, "fowner=%s"},
+	{Opt_fgroup_eq, "fgroup=%s"},
 	{Opt_uid_gt, "uid>%s"},
 	{Opt_euid_gt, "euid>%s"},
+	{Opt_gid_gt, "gid>%s"},
+	{Opt_egid_gt, "egid>%s"},
 	{Opt_fowner_gt, "fowner>%s"},
+	{Opt_fgroup_gt, "fgroup>%s"},
 	{Opt_uid_lt, "uid<%s"},
 	{Opt_euid_lt, "euid<%s"},
+	{Opt_gid_lt, "gid<%s"},
+	{Opt_egid_lt, "egid<%s"},
 	{Opt_fowner_lt, "fowner<%s"},
+	{Opt_fgroup_lt, "fgroup<%s"},
 	{Opt_appraise_type, "appraise_type=%s"},
 	{Opt_appraise_flag, "appraise_flag=%s"},
 	{Opt_appraise_algos, "appraise_algos=%s"},
@@ -1073,22 +1106,36 @@ static int ima_lsm_rule_init(struct ima_rule_entry *entry,
 }
 
 static void ima_log_string_op(struct audit_buffer *ab, char *key, char *value,
-			      bool (*rule_operator)(kuid_t, kuid_t))
+			      enum policy_opt rule_operator)
 {
 	if (!ab)
 		return;
 
-	if (rule_operator == &uid_gt)
+	switch (rule_operator) {
+	case Opt_uid_gt:
+	case Opt_euid_gt:
+	case Opt_gid_gt:
+	case Opt_egid_gt:
+	case Opt_fowner_gt:
+	case Opt_fgroup_gt:
 		audit_log_format(ab, "%s>", key);
-	else if (rule_operator == &uid_lt)
+		break;
+	case Opt_uid_lt:
+	case Opt_euid_lt:
+	case Opt_gid_lt:
+	case Opt_egid_lt:
+	case Opt_fowner_lt:
+	case Opt_fgroup_lt:
 		audit_log_format(ab, "%s<", key);
-	else
+		break;
+	default:
 		audit_log_format(ab, "%s=", key);
+	}
 	audit_log_format(ab, "%s ", value);
 }
 static void ima_log_string(struct audit_buffer *ab, char *key, char *value)
 {
-	ima_log_string_op(ab, key, value, NULL);
+	ima_log_string_op(ab, key, value, Opt_err);
 }
 
 /*
@@ -1163,7 +1210,8 @@ static bool ima_validate_rule(struct ima_rule_entry *entry)
 		if (entry->flags & ~(IMA_FUNC | IMA_MASK | IMA_FSMAGIC |
 				     IMA_UID | IMA_FOWNER | IMA_FSUUID |
 				     IMA_INMASK | IMA_EUID | IMA_PCR |
-				     IMA_FSNAME | IMA_DIGSIG_REQUIRED |
+				     IMA_FSNAME | IMA_GID | IMA_EGID |
+				     IMA_FGROUP | IMA_DIGSIG_REQUIRED |
 				     IMA_PERMIT_DIRECTIO | IMA_VALIDATE_ALGOS))
 			return false;
 
@@ -1174,7 +1222,8 @@ static bool ima_validate_rule(struct ima_rule_entry *entry)
 		if (entry->flags & ~(IMA_FUNC | IMA_MASK | IMA_FSMAGIC |
 				     IMA_UID | IMA_FOWNER | IMA_FSUUID |
 				     IMA_INMASK | IMA_EUID | IMA_PCR |
-				     IMA_FSNAME | IMA_DIGSIG_REQUIRED |
+				     IMA_FSNAME | IMA_GID | IMA_EGID |
+				     IMA_FGROUP | IMA_DIGSIG_REQUIRED |
 				     IMA_PERMIT_DIRECTIO | IMA_MODSIG_ALLOWED |
 				     IMA_CHECK_BLACKLIST | IMA_VALIDATE_ALGOS))
 			return false;
@@ -1186,7 +1235,8 @@ static bool ima_validate_rule(struct ima_rule_entry *entry)
 
 		if (entry->flags & ~(IMA_FUNC | IMA_FSMAGIC | IMA_UID |
 				     IMA_FOWNER | IMA_FSUUID | IMA_EUID |
-				     IMA_PCR | IMA_FSNAME))
+				     IMA_PCR | IMA_FSNAME | IMA_GID | IMA_EGID |
+				     IMA_FGROUP))
 			return false;
 
 		break;
@@ -1195,7 +1245,7 @@ static bool ima_validate_rule(struct ima_rule_entry *entry)
 			return false;
 
 		if (entry->flags & ~(IMA_FUNC | IMA_UID | IMA_PCR |
-				     IMA_KEYRINGS))
+				     IMA_KEYRINGS | IMA_GID))
 			return false;
 
 		if (ima_rule_contains_lsm_cond(entry))
@@ -1207,7 +1257,7 @@ static bool ima_validate_rule(struct ima_rule_entry *entry)
 			return false;
 
 		if (entry->flags & ~(IMA_FUNC | IMA_UID | IMA_PCR |
-				     IMA_LABEL))
+				     IMA_LABEL | IMA_GID))
 			return false;
 
 		if (ima_rule_contains_lsm_cond(entry))
@@ -1276,7 +1326,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 	struct audit_buffer *ab;
 	char *from;
 	char *p;
-	bool uid_token;
+	bool eid_token;
 	struct ima_template_desc *template_desc;
 	int result = 0;
 
@@ -1284,9 +1334,13 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 				       AUDIT_INTEGRITY_POLICY_RULE);
 
 	entry->uid = INVALID_UID;
+	entry->gid = INVALID_GID;
 	entry->fowner = INVALID_UID;
+	entry->fgroup = INVALID_GID;
 	entry->uid_op = &uid_eq;
+	entry->gid_op = &gid_eq;
 	entry->fowner_op = &uid_eq;
+	entry->fgroup_op = &gid_eq;
 	entry->action = UNKNOWN;
 	while ((p = strsep(&rule, " \t")) != NULL) {
 		substring_t args[MAX_OPT_ARGS];
@@ -1504,12 +1558,12 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 			fallthrough;
 		case Opt_uid_eq:
 		case Opt_euid_eq:
-			uid_token = (token == Opt_uid_eq) ||
-				    (token == Opt_uid_gt) ||
-				    (token == Opt_uid_lt);
+			eid_token = (token == Opt_euid_eq) ||
+				    (token == Opt_euid_gt) ||
+				    (token == Opt_euid_lt);
 
-			ima_log_string_op(ab, uid_token ? "uid" : "euid",
-					  args[0].from, entry->uid_op);
+			ima_log_string_op(ab, eid_token ? "euid" : "uid",
+					  args[0].from, token);
 
 			if (uid_valid(entry->uid)) {
 				result = -EINVAL;
@@ -1524,8 +1578,41 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 				    (uid_t)lnum != lnum)
 					result = -EINVAL;
 				else
-					entry->flags |= uid_token
-					    ? IMA_UID : IMA_EUID;
+					entry->flags |= eid_token
+					    ? IMA_EUID : IMA_UID;
+			}
+			break;
+		case Opt_gid_gt:
+		case Opt_egid_gt:
+			entry->gid_op = &gid_gt;
+			fallthrough;
+		case Opt_gid_lt:
+		case Opt_egid_lt:
+			if ((token == Opt_gid_lt) || (token == Opt_egid_lt))
+				entry->gid_op = &gid_lt;
+			fallthrough;
+		case Opt_gid_eq:
+		case Opt_egid_eq:
+			eid_token = (token == Opt_egid_eq) ||
+				    (token == Opt_egid_gt) ||
+				    (token == Opt_egid_lt);
+
+			ima_log_string_op(ab, eid_token ? "egid" : "gid",
+					  args[0].from, token);
+
+			if (gid_valid(entry->gid)) {
+				result = -EINVAL;
+				break;
+			}
+
+			result = kstrtoul(args[0].from, 10, &lnum);
+			if (!result) {
+				entry->gid = make_kgid(current_user_ns(), (gid_t)lnum);
+				if (!gid_valid(entry->gid) || (((gid_t)lnum) != lnum))
+					result = -EINVAL;
+				else
+					entry->flags |= eid_token
+					    ? IMA_EGID : IMA_GID;
 			}
 			break;
 		case Opt_fowner_gt:
@@ -1536,8 +1623,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 				entry->fowner_op = &uid_lt;
 			fallthrough;
 		case Opt_fowner_eq:
-			ima_log_string_op(ab, "fowner", args[0].from,
-					  entry->fowner_op);
+			ima_log_string_op(ab, "fowner", args[0].from, token);
 
 			if (uid_valid(entry->fowner)) {
 				result = -EINVAL;
@@ -1553,6 +1639,30 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 					entry->flags |= IMA_FOWNER;
 			}
 			break;
+		case Opt_fgroup_gt:
+			entry->fgroup_op = &gid_gt;
+			fallthrough;
+		case Opt_fgroup_lt:
+			if (token == Opt_fgroup_lt)
+				entry->fgroup_op = &gid_lt;
+			fallthrough;
+		case Opt_fgroup_eq:
+			ima_log_string_op(ab, "fgroup", args[0].from, token);
+
+			if (gid_valid(entry->fgroup)) {
+				result = -EINVAL;
+				break;
+			}
+
+			result = kstrtoul(args[0].from, 10, &lnum);
+			if (!result) {
+				entry->fgroup = make_kgid(current_user_ns(), (gid_t)lnum);
+				if (!gid_valid(entry->fgroup) || (((gid_t)lnum) != lnum))
+					result = -EINVAL;
+				else
+					entry->flags |= IMA_FGROUP;
+			}
+			break;
 		case Opt_obj_user:
 			ima_log_string(ab, "obj_user", args[0].from);
 			result = ima_lsm_rule_init(entry, args,
@@ -1936,6 +2046,28 @@ int ima_policy_show(struct seq_file *m, void *v)
 		seq_puts(m, " ");
 	}
 
+	if (entry->flags & IMA_GID) {
+		snprintf(tbuf, sizeof(tbuf), "%d", __kgid_val(entry->gid));
+		if (entry->gid_op == &gid_gt)
+			seq_printf(m, pt(Opt_gid_gt), tbuf);
+		else if (entry->gid_op == &gid_lt)
+			seq_printf(m, pt(Opt_gid_lt), tbuf);
+		else
+			seq_printf(m, pt(Opt_gid_eq), tbuf);
+		seq_puts(m, " ");
+	}
+
+	if (entry->flags & IMA_EGID) {
+		snprintf(tbuf, sizeof(tbuf), "%d", __kgid_val(entry->gid));
+		if (entry->gid_op == &gid_gt)
+			seq_printf(m, pt(Opt_egid_gt), tbuf);
+		else if (entry->gid_op == &gid_lt)
+			seq_printf(m, pt(Opt_egid_lt), tbuf);
+		else
+			seq_printf(m, pt(Opt_egid_eq), tbuf);
+		seq_puts(m, " ");
+	}
+
 	if (entry->flags & IMA_FOWNER) {
 		snprintf(tbuf, sizeof(tbuf), "%d", __kuid_val(entry->fowner));
 		if (entry->fowner_op == &uid_gt)
@@ -1947,6 +2079,17 @@ int ima_policy_show(struct seq_file *m, void *v)
 		seq_puts(m, " ");
 	}
 
+	if (entry->flags & IMA_FGROUP) {
+		snprintf(tbuf, sizeof(tbuf), "%d", __kgid_val(entry->fgroup));
+		if (entry->fgroup_op == &gid_gt)
+			seq_printf(m, pt(Opt_fgroup_gt), tbuf);
+		else if (entry->fgroup_op == &gid_lt)
+			seq_printf(m, pt(Opt_fgroup_lt), tbuf);
+		else
+			seq_printf(m, pt(Opt_fgroup_eq), tbuf);
+		seq_puts(m, " ");
+	}
+
 	if (entry->flags & IMA_VALIDATE_ALGOS) {
 		seq_puts(m, "appraise_algos=");
 		ima_policy_show_appraise_algos(m, entry->allowed_algos);
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH ltp] IMA: Add tests for uid, gid, fowner, and fgroup options
  2021-09-09 16:51 [PATCH linux rebase] ima: add gid support Alex Henrie
@ 2021-09-09 16:51 ` Alex Henrie
  2021-09-09 20:21   ` Petr Vorel
  2021-10-04 22:30 ` [PATCH linux rebase] ima: add gid support Mimi Zohar
  1 sibling, 1 reply; 6+ messages in thread
From: Alex Henrie @ 2021-09-09 16:51 UTC (permalink / raw)
  To: linux-integrity, ltp, zohar, pvorel, alexhenrie24; +Cc: Alex Henrie

Requires "ima: add gid support".

Signed-off-by: Alex Henrie <alexh@vpitech.com>
---
 .../integrity/ima/tests/ima_measurements.sh   | 37 ++++++++++++++++++-
 1 file changed, 35 insertions(+), 2 deletions(-)

diff --git a/testcases/kernel/security/integrity/ima/tests/ima_measurements.sh b/testcases/kernel/security/integrity/ima/tests/ima_measurements.sh
index 1927e937c..3c1bcbf88 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_measurements.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_measurements.sh
@@ -8,6 +8,7 @@
 
 TST_NEEDS_CMDS="awk cut sed"
 TST_SETUP="setup"
+TST_CLEANUP="cleanup"
 TST_CNT=3
 TST_NEEDS_DEVICE=1
 
@@ -20,6 +21,13 @@ setup()
 	TEST_FILE="$PWD/test.txt"
 	POLICY="$IMA_DIR/policy"
 	[ -f "$POLICY" ] || tst_res TINFO "not using default policy"
+
+	cat $IMA_POLICY > policy-original
+}
+
+cleanup()
+{
+	cat policy-original > $IMA_POLICY
 }
 
 ima_check()
@@ -103,7 +111,7 @@ test3()
 	local file="$dir/test.txt"
 
 	# Default policy does not measure user files
-	tst_res TINFO "verify not measuring user files"
+	tst_res TINFO "verify not measuring user files by default"
 	tst_check_cmds sudo || return
 
 	if ! id $user >/dev/null 2>/dev/null; then
@@ -116,9 +124,34 @@ test3()
 	cd $dir
 	# need to read file to get updated $ASCII_MEASUREMENTS
 	sudo -n -u $user sh -c "echo $(date) user file > $file; cat $file > /dev/null"
+	EXPECT_FAIL "grep $file $ASCII_MEASUREMENTS"
 	cd ..
 
-	EXPECT_FAIL "grep $file $ASCII_MEASUREMENTS"
+	tst_res TINFO "verify measuring user files when requested via uid"
+	ROD echo "measure uid=$(id -u $user)" \> $IMA_POLICY
+	ROD echo "$(date) uid test" \> $TEST_FILE
+	sudo -n -u $user sh -c "cat $TEST_FILE > /dev/null"
+	ima_check
+
+	tst_res TINFO "verify measuring user files when requested via gid"
+	ROD echo "measure gid=$(id -g $user)" \> $IMA_POLICY
+	ROD echo "$(date) gid test" \> $TEST_FILE
+	sudo -n -u $user sh -c "cat $TEST_FILE > /dev/null"
+	ima_check
+
+	tst_res TINFO "verify measuring user files when requested via fowner"
+	ROD echo "measure fowner=$(id -u $user)" \> $IMA_POLICY
+	ROD echo "$(date) fowner test" \> $TEST_FILE
+	chown $user $TEST_FILE
+	cat $TEST_FILE > /dev/null
+	ima_check
+
+	tst_res TINFO "verify measuring user files when requested via fgroup"
+	ROD echo "measure fgroup=$(id -g $user)" \> $IMA_POLICY
+	ROD echo "$(date) fgroup test" \> $TEST_FILE
+	chgrp $(id -g $user) $TEST_FILE
+	cat $TEST_FILE > /dev/null
+	ima_check
 }
 
 tst_run
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH ltp] IMA: Add tests for uid, gid, fowner, and fgroup options
  2021-09-09 16:51 ` [PATCH ltp] IMA: Add tests for uid, gid, fowner, and fgroup options Alex Henrie
@ 2021-09-09 20:21   ` Petr Vorel
  2021-09-10  0:35     ` Alex Henrie
  0 siblings, 1 reply; 6+ messages in thread
From: Petr Vorel @ 2021-09-09 20:21 UTC (permalink / raw)
  To: Alex Henrie; +Cc: linux-integrity, ltp, zohar, alexhenrie24

Hi Alex,

> Requires "ima: add gid support".
I haven't test the patch yet, but LTP supports (unlike kselftest) various kernel
versions. Thus there should be some check to prevent old kernels failing.
You could certainly wrap new things with if tst_kvcmp. If there is a chance new
functionality can be detected, we prefer it because various features are
sometimes backported to enterprise distros' kernels.

Also, adding new test ima_measurements02.sh with TST_MIN_KVER would also work,
although for IMA tests I usually kept everything in a single file.

...
> +++ b/testcases/kernel/security/integrity/ima/tests/ima_measurements.sh
> @@ -8,6 +8,7 @@

>  TST_NEEDS_CMDS="awk cut sed"
You should add sudo:

TST_NEEDS_CMDS="awk cut sed sudo"
>  TST_SETUP="setup"
> +TST_CLEANUP="cleanup"
>  TST_CNT=3
>  TST_NEEDS_DEVICE=1

> @@ -20,6 +21,13 @@ setup()
>  	TEST_FILE="$PWD/test.txt"
>  	POLICY="$IMA_DIR/policy"
>  	[ -f "$POLICY" ] || tst_res TINFO "not using default policy"
> +
> +	cat $IMA_POLICY > policy-original
This might not work if CONFIG_IMA_READ_POLICY is not set. There is
check_policy_readable() helper in ima_setup.sh. Is it really needed anyway?

> +}
> +
> +cleanup()
> +{
> +	cat policy-original > $IMA_POLICY
Again, this will not work if CONFIG_IMA_WRITE_POLICY not set.
And this is very likely not to be set.

...

Kind regards,
Petr

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH ltp] IMA: Add tests for uid, gid, fowner, and fgroup options
  2021-09-09 20:21   ` Petr Vorel
@ 2021-09-10  0:35     ` Alex Henrie
  2021-09-10  7:33       ` Petr Vorel
  0 siblings, 1 reply; 6+ messages in thread
From: Alex Henrie @ 2021-09-10  0:35 UTC (permalink / raw)
  To: Petr Vorel; +Cc: linux-integrity, ltp, zohar, alexhenrie24

On Thu, 9 Sep 2021 22:21:22 +0200
Petr Vorel <pvorel@suse.cz> wrote:

> > Requires "ima: add gid support".
> I haven't test the patch yet, but LTP supports (unlike kselftest) various kernel
> versions. Thus there should be some check to prevent old kernels failing.
> You could certainly wrap new things with if tst_kvcmp. If there is a chance new
> functionality can be detected, we prefer it because various features are
> sometimes backported to enterprise distros' kernels.
> 
> Also, adding new test ima_measurements02.sh with TST_MIN_KVER would also work,
> although for IMA tests I usually kept everything in a single file.

I'll add a tst_kvcmp check under the assumption that this feature will
be added before Linux 5.15.

> > +++ b/testcases/kernel/security/integrity/ima/tests/ima_measurements.sh
> > @@ -8,6 +8,7 @@
> 
> >  TST_NEEDS_CMDS="awk cut sed"
> You should add sudo:
> 
> TST_NEEDS_CMDS="awk cut sed sudo"

Will do.

> >  TST_SETUP="setup"
> > +TST_CLEANUP="cleanup"
> >  TST_CNT=3
> >  TST_NEEDS_DEVICE=1
> 
> > @@ -20,6 +21,13 @@ setup()
> >  	TEST_FILE="$PWD/test.txt"
> >  	POLICY="$IMA_DIR/policy"
> >  	[ -f "$POLICY" ] || tst_res TINFO "not using default policy"
> > +
> > +	cat $IMA_POLICY > policy-original
> This might not work if CONFIG_IMA_READ_POLICY is not set. There is
> check_policy_readable() helper in ima_setup.sh. Is it really needed anyway?

It looks like CONFIG_IMA_WRITE_POLICY only makes it possible to add new
rules at runtime, not remove them, so the cleanup code didn't actually
work. I'll remove it.

> > +}
> > +
> > +cleanup()
> > +{
> > +	cat policy-original > $IMA_POLICY
> Again, this will not work if CONFIG_IMA_WRITE_POLICY not set.
> And this is very likely not to be set.

The new tests require the policy to be writable. I'll move the
check_policy_writable function from ima_policy.sh to ima_setup.sh and
use it in ima_measurements.sh as well.

Thanks for the feedback,

-Alex

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH ltp] IMA: Add tests for uid, gid, fowner, and fgroup options
  2021-09-10  0:35     ` Alex Henrie
@ 2021-09-10  7:33       ` Petr Vorel
  0 siblings, 0 replies; 6+ messages in thread
From: Petr Vorel @ 2021-09-10  7:33 UTC (permalink / raw)
  To: Alex Henrie; +Cc: linux-integrity, ltp, zohar, alexhenrie24

Hi Alex,

> On Thu, 9 Sep 2021 22:21:22 +0200
> Petr Vorel <pvorel@suse.cz> wrote:

> > > Requires "ima: add gid support".
> > I haven't test the patch yet, but LTP supports (unlike kselftest) various kernel
> > versions. Thus there should be some check to prevent old kernels failing.
> > You could certainly wrap new things with if tst_kvcmp. If there is a chance new
> > functionality can be detected, we prefer it because various features are
> > sometimes backported to enterprise distros' kernels.

> > Also, adding new test ima_measurements02.sh with TST_MIN_KVER would also work,
> > although for IMA tests I usually kept everything in a single file.

> I'll add a tst_kvcmp check under the assumption that this feature will
> be added before Linux 5.15.
+1. Please let me know when you manage to get this mainlined (merged into Mimi's
tree is enough), we should also add the commit hash of this feature.

> > > +++ b/testcases/kernel/security/integrity/ima/tests/ima_measurements.sh
> > > @@ -8,6 +8,7 @@

> > >  TST_NEEDS_CMDS="awk cut sed"
> > You should add sudo:

> > TST_NEEDS_CMDS="awk cut sed sudo"

> Will do.
+1

> > >  TST_SETUP="setup"
> > > +TST_CLEANUP="cleanup"
> > >  TST_CNT=3
> > >  TST_NEEDS_DEVICE=1

> > > @@ -20,6 +21,13 @@ setup()
> > >  	TEST_FILE="$PWD/test.txt"
> > >  	POLICY="$IMA_DIR/policy"
> > >  	[ -f "$POLICY" ] || tst_res TINFO "not using default policy"
> > > +
> > > +	cat $IMA_POLICY > policy-original
> > This might not work if CONFIG_IMA_READ_POLICY is not set. There is
> > check_policy_readable() helper in ima_setup.sh. Is it really needed anyway?

> It looks like CONFIG_IMA_WRITE_POLICY only makes it possible to add new
> rules at runtime, not remove them, so the cleanup code didn't actually
> work. I'll remove it.

FYI I have on my TODO list loading policy before testing [1].

> > > +}
> > > +
> > > +cleanup()
> > > +{
> > > +	cat policy-original > $IMA_POLICY
> > Again, this will not work if CONFIG_IMA_WRITE_POLICY not set.
> > And this is very likely not to be set.

> The new tests require the policy to be writable. I'll move the
> check_policy_writable function from ima_policy.sh to ima_setup.sh and
> use it in ima_measurements.sh as well.

+1.

FYI there is IMA specific README.md [2], in case anything needs to be updated.

> Thanks for the feedback,
yw. Thanks for taking care about testing!

Kind regards,
Petr

> -Alex

[1] https://github.com/linux-test-project/ltp/issues/720
[2] https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/security/integrity/ima/README.md

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH linux rebase] ima: add gid support
  2021-09-09 16:51 [PATCH linux rebase] ima: add gid support Alex Henrie
  2021-09-09 16:51 ` [PATCH ltp] IMA: Add tests for uid, gid, fowner, and fgroup options Alex Henrie
@ 2021-10-04 22:30 ` Mimi Zohar
  1 sibling, 0 replies; 6+ messages in thread
From: Mimi Zohar @ 2021-10-04 22:30 UTC (permalink / raw)
  To: Alex Henrie, linux-integrity, ltp, pvorel, alexhenrie24; +Cc: Curtis Veit

On Thu, 2021-09-09 at 10:51 -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>

Thanks, Alex.  There are a couple of places were the code is
duplicated, except for the indentation.   At least for now, let's keep
the line length at 80 chars.  Similarly, when extending a
comment/documentation, please split the line.

A few minor comments below, otherwise:

Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>

> ---
> This is the patch that Curtis sent in 2015,[1] rebased and with support
> for effective GID and the new greater-than and less-than operators.
> Moreover, the policy_opt enum is now in the same order as the
> policy_tokens array, which I'm guessing is the bug that prevented the
> patch from being accepted before.

That sounds right.

> 
> [1] https://sourceforge.net/p/linux-ima/mailman/message/34210250/
> ---
>  Documentation/ABI/testing/ima_policy |   5 +-
>  security/integrity/ima/ima_policy.c  | 197 +++++++++++++++++++++++----
>  2 files changed, 174 insertions(+), 28 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
> index 5c2798534950..f8e5b5598548 100644
> --- a/Documentation/ABI/testing/ima_policy
> +++ b/Documentation/ABI/testing/ima_policy
> @@ -23,7 +23,7 @@ Description:
>  			  audit | hash | dont_hash
>  		  condition:= base | lsm  [option]
>  			base:	[[func=] [mask=] [fsmagic=] [fsuuid=] [uid=]
> -				[euid=] [fowner=] [fsname=]]
> +				[euid=] [gid=] [egid=] [fowner=] [fgroup=] [fsname=]]

Like in other places where you modified the ordering, let's group uid,
euid, git, egid, etc together on the same line.

>  			lsm:	[[subj_user=] [subj_role=] [subj_type=]
>  				 [obj_user=] [obj_role=] [obj_type=]]
>  			option:	[[appraise_type=]] [template=] [permit_directio]
> @@ -40,7 +40,10 @@ Description:
>  			fsuuid:= file system UUID (e.g 8bcbe394-4f13-4144-be8e-5aa9ea2ce2f6)
>  			uid:= decimal value
>  			euid:= decimal value
> +			gid:= decimal value
> +			egid:= decimal value
>  			fowner:= decimal value
> +			fgroup:= decimal value
>  		  lsm:  are LSM specific
>  		  option:
>  			appraise_type:= [imasig] [imasig|modsig]
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index 87b9b71cb820..0f2287699f85 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -36,6 +36,9 @@
>  #define IMA_KEYRINGS	0x0400
>  #define IMA_LABEL	0x0800
>  #define IMA_VALIDATE_ALGOS	0x1000
> +#define IMA_GID		0x2000
> +#define IMA_EGID	0x4000
> +#define IMA_FGROUP	0x8000

Ok, any additional changes after this will need to increase the "flags"
size.

>  
>  #define UNKNOWN		0
>  #define MEASURE		0x0001	/* same as IMA_MEASURE */
> @@ -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() */
>  	int pcr;
>  	unsigned int allowed_algos; /* bitfield of allowed hash algorithms */
>  	struct {
> @@ -104,7 +111,8 @@ static_assert(
>  
>  /*
>   * Without LSM specific knowledge, the default policy can only be
> - * written in terms of .action, .func, .mask, .fsmagic, .uid, and .fowner
> + * written in terms of .action, .func, .mask, .fsmagic, .uid, .gid,
> + * .fowner, and .fgroup
>   */
>  
>  /*
> @@ -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))
> +		return false;
> +	if (rule->flags & IMA_EGID) {
> +		if (has_capability_noaudit(current, CAP_SETGID)) {
> +			if (!rule->gid_op(cred->egid, rule->gid)
> +			    && !rule->gid_op(cred->sgid, rule->gid)
> +			    && !rule->gid_op(cred->gid, rule->gid))
> +				return false;
> +		} else if (!rule->gid_op(cred->egid, rule->gid))
> +			return false;
> +	}
>  	if ((rule->flags & IMA_FOWNER) &&
>  	    !rule->fowner_op(i_uid_into_mnt(mnt_userns, inode), rule->fowner))
>  		return false;
> +	if ((rule->flags & IMA_FGROUP) &&
> +	    !rule->fgroup_op(i_gid_into_mnt(mnt_userns, inode), rule->fgroup))
> +		return false;
>  	for (i = 0; i < MAX_LSM_RULES; i++) {
>  		int rc = 0;
>  		u32 osid;
> @@ -987,16 +1008,19 @@ void ima_update_policy(void)
>  }
>  
>  /* Keep the enumeration in sync with the policy_tokens! */
> -enum {
> +enum policy_opt {
>  	Opt_measure, Opt_dont_measure,
>  	Opt_appraise, Opt_dont_appraise,
>  	Opt_audit, Opt_hash, Opt_dont_hash,
>  	Opt_obj_user, Opt_obj_role, Opt_obj_type,
>  	Opt_subj_user, Opt_subj_role, Opt_subj_type,
> -	Opt_func, Opt_mask, Opt_fsmagic, Opt_fsname,
> -	Opt_fsuuid, Opt_uid_eq, Opt_euid_eq, Opt_fowner_eq,
> -	Opt_uid_gt, Opt_euid_gt, Opt_fowner_gt,
> -	Opt_uid_lt, Opt_euid_lt, Opt_fowner_lt,
> +	Opt_func, Opt_mask, Opt_fsmagic, Opt_fsname, Opt_fsuuid,
> +	Opt_uid_eq, Opt_euid_eq, Opt_gid_eq, Opt_egid_eq,
> +	Opt_fowner_eq, Opt_fgroup_eq,
> +	Opt_uid_gt, Opt_euid_gt, Opt_gid_gt, Opt_egid_gt,
> +	Opt_fowner_gt, Opt_fgroup_gt,
> +	Opt_uid_lt, Opt_euid_lt, Opt_gid_lt, Opt_egid_lt,
> +	Opt_fowner_lt, Opt_fgroup_lt,
>  	Opt_appraise_type, Opt_appraise_flag, Opt_appraise_algos,
>  	Opt_permit_directio, Opt_pcr, Opt_template, Opt_keyrings,
>  	Opt_label, Opt_err
> @@ -1023,13 +1047,22 @@ static const match_table_t policy_tokens = {
>  	{Opt_fsuuid, "fsuuid=%s"},
>  	{Opt_uid_eq, "uid=%s"},
>  	{Opt_euid_eq, "euid=%s"},
> +	{Opt_gid_eq, "gid=%s"},
> +	{Opt_egid_eq, "egid=%s"},
>  	{Opt_fowner_eq, "fowner=%s"},
> +	{Opt_fgroup_eq, "fgroup=%s"},
>  	{Opt_uid_gt, "uid>%s"},
>  	{Opt_euid_gt, "euid>%s"},
> +	{Opt_gid_gt, "gid>%s"},
> +	{Opt_egid_gt, "egid>%s"},
>  	{Opt_fowner_gt, "fowner>%s"},
> +	{Opt_fgroup_gt, "fgroup>%s"},
>  	{Opt_uid_lt, "uid<%s"},
>  	{Opt_euid_lt, "euid<%s"},
> +	{Opt_gid_lt, "gid<%s"},
> +	{Opt_egid_lt, "egid<%s"},
>  	{Opt_fowner_lt, "fowner<%s"},
> +	{Opt_fgroup_lt, "fgroup<%s"},
>  	{Opt_appraise_type, "appraise_type=%s"},
>  	{Opt_appraise_flag, "appraise_flag=%s"},
>  	{Opt_appraise_algos, "appraise_algos=%s"},
> @@ -1073,22 +1106,36 @@ static int ima_lsm_rule_init(struct ima_rule_entry *entry,
>  }
>  
>  static void ima_log_string_op(struct audit_buffer *ab, char *key, char *value,
> -			      bool (*rule_operator)(kuid_t, kuid_t))
> +			      enum policy_opt rule_operator)
>  {
>  	if (!ab)
>  		return;
>  
> -	if (rule_operator == &uid_gt)
> +	switch (rule_operator) {
> +	case Opt_uid_gt:
> +	case Opt_euid_gt:
> +	case Opt_gid_gt:
> +	case Opt_egid_gt:
> +	case Opt_fowner_gt:
> +	case Opt_fgroup_gt:
>  		audit_log_format(ab, "%s>", key);
> -	else if (rule_operator == &uid_lt)
> +		break;
> +	case Opt_uid_lt:
> +	case Opt_euid_lt:
> +	case Opt_gid_lt:
> +	case Opt_egid_lt:
> +	case Opt_fowner_lt:
> +	case Opt_fgroup_lt:
>  		audit_log_format(ab, "%s<", key);
> -	else
> +		break;
> +	default:
>  		audit_log_format(ab, "%s=", key);
> +	}
>  	audit_log_format(ab, "%s ", value);
>  }
>  static void ima_log_string(struct audit_buffer *ab, char *key, char *value)
>  {
> -	ima_log_string_op(ab, key, value, NULL);
> +	ima_log_string_op(ab, key, value, Opt_err);
>  }
>  
>  /*
> @@ -1163,7 +1210,8 @@ static bool ima_validate_rule(struct ima_rule_entry *entry)
>  		if (entry->flags & ~(IMA_FUNC | IMA_MASK | IMA_FSMAGIC |
>  				     IMA_UID | IMA_FOWNER | IMA_FSUUID |
>  				     IMA_INMASK | IMA_EUID | IMA_PCR |
> -				     IMA_FSNAME | IMA_DIGSIG_REQUIRED |
> +				     IMA_FSNAME | IMA_GID | IMA_EGID |
> +				     IMA_FGROUP | IMA_DIGSIG_REQUIRED |
>  				     IMA_PERMIT_DIRECTIO | IMA_VALIDATE_ALGOS))
>  			return false;
>  
> @@ -1174,7 +1222,8 @@ static bool ima_validate_rule(struct ima_rule_entry *entry)
>  		if (entry->flags & ~(IMA_FUNC | IMA_MASK | IMA_FSMAGIC |
>  				     IMA_UID | IMA_FOWNER | IMA_FSUUID |
>  				     IMA_INMASK | IMA_EUID | IMA_PCR |
> -				     IMA_FSNAME | IMA_DIGSIG_REQUIRED |
> +				     IMA_FSNAME | IMA_GID | IMA_EGID |
> +				     IMA_FGROUP | IMA_DIGSIG_REQUIRED |
>  				     IMA_PERMIT_DIRECTIO | IMA_MODSIG_ALLOWED |
>  				     IMA_CHECK_BLACKLIST | IMA_VALIDATE_ALGOS))
>  			return false;
> @@ -1186,7 +1235,8 @@ static bool ima_validate_rule(struct ima_rule_entry *entry)
>  
>  		if (entry->flags & ~(IMA_FUNC | IMA_FSMAGIC | IMA_UID |
>  				     IMA_FOWNER | IMA_FSUUID | IMA_EUID |
> -				     IMA_PCR | IMA_FSNAME))
> +				     IMA_PCR | IMA_FSNAME | IMA_GID | IMA_EGID |
> +				     IMA_FGROUP))
>  			return false;
>  
>  		break;
> @@ -1195,7 +1245,7 @@ static bool ima_validate_rule(struct ima_rule_entry *entry)
>  			return false;
>  
>  		if (entry->flags & ~(IMA_FUNC | IMA_UID | IMA_PCR |
> -				     IMA_KEYRINGS))
> +				     IMA_KEYRINGS | IMA_GID))
>  			return false;
>  
>  		if (ima_rule_contains_lsm_cond(entry))
> @@ -1207,7 +1257,7 @@ static bool ima_validate_rule(struct ima_rule_entry *entry)
>  			return false;
>  
>  		if (entry->flags & ~(IMA_FUNC | IMA_UID | IMA_PCR |
> -				     IMA_LABEL))
> +				     IMA_LABEL | IMA_GID))

How about moving IMA_GID after IMA_UID?

>  			return false;
>  
>  		if (ima_rule_contains_lsm_cond(entry))
> @@ -1276,7 +1326,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
>  	struct audit_buffer *ab;
>  	char *from;
>  	char *p;
> -	bool uid_token;
> +	bool eid_token;

Took a moment to understand the reason for this change.   Adding a
comment on the line, like "either euid or egid" might have helped.  

>  	struct ima_template_desc *template_desc;
>  	int result = 0;
>  
> @@ -1284,9 +1334,13 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
>  				       AUDIT_INTEGRITY_POLICY_RULE);
>  
>  	entry->uid = INVALID_UID;
> +	entry->gid = INVALID_GID;
>  	entry->fowner = INVALID_UID;
> +	entry->fgroup = INVALID_GID;
>  	entry->uid_op = &uid_eq;
> +	entry->gid_op = &gid_eq;
>  	entry->fowner_op = &uid_eq;
> +	entry->fgroup_op = &gid_eq;
>  	entry->action = UNKNOWN;
>  	while ((p = strsep(&rule, " \t")) != NULL) {
>  		substring_t args[MAX_OPT_ARGS];
> @@ -1504,12 +1558,12 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
>  			fallthrough;
>  		case Opt_uid_eq:
>  		case Opt_euid_eq:
> -			uid_token = (token == Opt_uid_eq) ||
> -				    (token == Opt_uid_gt) ||
> -				    (token == Opt_uid_lt);
> +			eid_token = (token == Opt_euid_eq) ||
> +				    (token == Opt_euid_gt) ||
> +				    (token == Opt_euid_lt);
>  
> -			ima_log_string_op(ab, uid_token ? "uid" : "euid",
> -					  args[0].from, entry->uid_op);
> +			ima_log_string_op(ab, eid_token ? "euid" : "uid",
> +					  args[0].from, token);
>  
>  			if (uid_valid(entry->uid)) {
>  				result = -EINVAL;
> @@ -1524,8 +1578,41 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
>  				    (uid_t)lnum != lnum)
>  					result = -EINVAL;
>  				else
> -					entry->flags |= uid_token
> -					    ? IMA_UID : IMA_EUID;
> +					entry->flags |= eid_token
> +					    ? IMA_EUID : IMA_UID;
> +			}
> +			break;
> +		case Opt_gid_gt:
> +		case Opt_egid_gt:
> +			entry->gid_op = &gid_gt;
> +			fallthrough;
> +		case Opt_gid_lt:
> +		case Opt_egid_lt:
> +			if ((token == Opt_gid_lt) || (token == Opt_egid_lt))
> +				entry->gid_op = &gid_lt;
> +			fallthrough;
> +		case Opt_gid_eq:
> +		case Opt_egid_eq:
> +			eid_token = (token == Opt_egid_eq) ||
> +				    (token == Opt_egid_gt) ||
> +				    (token == Opt_egid_lt);
> +
> +			ima_log_string_op(ab, eid_token ? "egid" : "gid",
> +					  args[0].from, token);
> +
> +			if (gid_valid(entry->gid)) {
> +				result = -EINVAL;
> +				break;
> +			}
> +
> +			result = kstrtoul(args[0].from, 10, &lnum);
> +			if (!result) {
> +				entry->gid = make_kgid(current_user_ns(), (gid_t)lnum);
> +				if (!gid_valid(entry->gid) || (((gid_t)lnum) != lnum))

Example of where the indentation changes from original "uid" code.

> +					result = -EINVAL;
> +				else
> +					entry->flags |= eid_token
> +					    ? IMA_EGID : IMA_GID;
>  			}
>  			break;
>  		case Opt_fowner_gt:
> @@ -1536,8 +1623,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
>  				entry->fowner_op = &uid_lt;
>  			fallthrough;
>  		case Opt_fowner_eq:
> -			ima_log_string_op(ab, "fowner", args[0].from,
> -					  entry->fowner_op);
> +			ima_log_string_op(ab, "fowner", args[0].from, token);
>  
>  			if (uid_valid(entry->fowner)) {
>  				result = -EINVAL;
> @@ -1553,6 +1639,30 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
>  					entry->flags |= IMA_FOWNER;
>  			}
>  			break;
> +		case Opt_fgroup_gt:
> +			entry->fgroup_op = &gid_gt;
> +			fallthrough;
> +		case Opt_fgroup_lt:
> +			if (token == Opt_fgroup_lt)
> +				entry->fgroup_op = &gid_lt;
> +			fallthrough;
> +		case Opt_fgroup_eq:
> +			ima_log_string_op(ab, "fgroup", args[0].from, token);
> +
> +			if (gid_valid(entry->fgroup)) {
> +				result = -EINVAL;
> +				break;
> +			}
> +
> +			result = kstrtoul(args[0].from, 10, &lnum);
> +			if (!result) {
> +				entry->fgroup = make_kgid(current_user_ns(), (gid_t)lnum);
> +				if (!gid_valid(entry->fgroup) || (((gid_t)lnum) != lnum))

and here

> +					result = -EINVAL;
> +				else
> +					entry->flags |= IMA_FGROUP;
> +			}
> +			break;
>  		case Opt_obj_user:
>  			ima_log_string(ab, "obj_user", args[0].from);
>  			result = ima_lsm_rule_init(entry, args,


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-10-04 22:34 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-09 16:51 [PATCH linux rebase] ima: add gid support Alex Henrie
2021-09-09 16:51 ` [PATCH ltp] IMA: Add tests for uid, gid, fowner, and fgroup options Alex Henrie
2021-09-09 20:21   ` Petr Vorel
2021-09-10  0:35     ` Alex Henrie
2021-09-10  7:33       ` Petr Vorel
2021-10-04 22:30 ` [PATCH linux rebase] ima: add gid support Mimi Zohar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).