IMA: do not allow the same rule to specify the same thing twice

IMA will accept rules which specify things twice and will only pay attention to the last one. We should reject such rules. Signed-off-by: Eric Paris <eparis@redhat.com> Acked-by: Mimi Zohar <zohar@us.ibm.com> Signed-off-by: James Morris <jmorris@namei.org>

IMA: do not allow the same rule to specify the same thing twice
IMA will accept rules which specify things twice and will only pay attention to the last one. We should reject such rules. Signed-off-by: Eric Paris <eparis@redhat.com> Acked-by: Mimi Zohar <zohar@us.ibm.com> Signed-off-by: James Morris <jmorris@namei.org>
Eric Paris · James Morris
1 parent 6ccd045630
Showing 1 changed file with 33 additions and 1 deletions Side-by-side Diff
security/integrity/ima/ima_policy.c
@@ -245,6 +245,9 @@
 {
 	int result;
  
+	if (entry->lsm[lsm_rule].rule)
+		return -EINVAL;
+
 	entry->lsm[lsm_rule].type = audit_type;
 	result = security_filter_rule_init(entry->lsm[lsm_rule].type,
 					   Audit_equal, args,
@@ -260,6 +263,7 @@
  
 	ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_INTEGRITY_RULE);
  
+	entry->uid = -1;
 	entry->action = -1;
 	while ((p = strsep(&rule, " ")) != NULL) {
 		substring_t args[MAX_OPT_ARGS];
  
  
@@ -274,14 +278,26 @@
 		switch (token) {
 		case Opt_measure:
 			audit_log_format(ab, "%s ", "measure");
+
+			if (entry->action != UNKNOWN)
+				result = -EINVAL;
+
 			entry->action = MEASURE;
 			break;
 		case Opt_dont_measure:
 			audit_log_format(ab, "%s ", "dont_measure");
+
+			if (entry->action != UNKNOWN)
+				result = -EINVAL;
+
 			entry->action = DONT_MEASURE;
 			break;
 		case Opt_func:
 			audit_log_format(ab, "func=%s ", args[0].from);
+
+			if (entry->func)
+				result  = -EINVAL;
+
 			if (strcmp(args[0].from, "FILE_CHECK") == 0)
 				entry->func = FILE_CHECK;
 			/* PATH_CHECK is for backwards compat */
@@ -298,6 +314,10 @@
 			break;
 		case Opt_mask:
 			audit_log_format(ab, "mask=%s ", args[0].from);
+
+			if (entry->mask)
+				result = -EINVAL;
+
 			if ((strcmp(args[0].from, "MAY_EXEC")) == 0)
 				entry->mask = MAY_EXEC;
 			else if (strcmp(args[0].from, "MAY_WRITE") == 0)
@@ -313,6 +333,12 @@
 			break;
 		case Opt_fsmagic:
 			audit_log_format(ab, "fsmagic=%s ", args[0].from);
+
+			if (entry->fsmagic) {
+				result = -EINVAL;
+				break;
+			}
+
 			result = strict_strtoul(args[0].from, 16,
 						&entry->fsmagic);
 			if (!result)
@@ -320,6 +346,12 @@
 			break;
 		case Opt_uid:
 			audit_log_format(ab, "uid=%s ", args[0].from);
+
+			if (entry->uid != -1) {
+				result = -EINVAL;
+				break;
+			}
+
 			result = strict_strtoul(args[0].from, 10, &lnum);
 			if (!result) {
 				entry->uid = (uid_t) lnum;
@@ -370,7 +402,7 @@
 			break;
 		}
 	}
-	if (entry->action == UNKNOWN)
+	if (!result && (entry->action == UNKNOWN))
 		result = -EINVAL;
  
 	audit_log_format(ab, "res=%d", !!result);
...	...	@@ -245,6 +245,9 @@
245	245	{
246	246	int result;
247	247
	248	+ if (entry->lsm[lsm_rule].rule)
	249	+ return -EINVAL;
	250	+
248	251	entry->lsm[lsm_rule].type = audit_type;
249	252	result = security_filter_rule_init(entry->lsm[lsm_rule].type,
250	253	Audit_equal, args,
...	...	@@ -260,6 +263,7 @@
260	263
261	264	ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_INTEGRITY_RULE);
262	265
	266	+ entry->uid = -1;
263	267	entry->action = -1;
264	268	while ((p = strsep(&rule, " ")) != NULL) {
265	269	substring_t args[MAX_OPT_ARGS];
266	270
267	271
...	...	@@ -274,14 +278,26 @@
274	278	switch (token) {
275	279	case Opt_measure:
276	280	audit_log_format(ab, "%s ", "measure");
	281	+
	282	+ if (entry->action != UNKNOWN)
	283	+ result = -EINVAL;
	284	+
277	285	entry->action = MEASURE;
278	286	break;
279	287	case Opt_dont_measure:
280	288	audit_log_format(ab, "%s ", "dont_measure");
	289	+
	290	+ if (entry->action != UNKNOWN)
	291	+ result = -EINVAL;
	292	+
281	293	entry->action = DONT_MEASURE;
282	294	break;
283	295	case Opt_func:
284	296	audit_log_format(ab, "func=%s ", args[0].from);
	297	+
	298	+ if (entry->func)
	299	+ result = -EINVAL;
	300	+
285	301	if (strcmp(args[0].from, "FILE_CHECK") == 0)
286	302	entry->func = FILE_CHECK;
287	303	/* PATH_CHECK is for backwards compat */
...	...	@@ -298,6 +314,10 @@
298	314	break;
299	315	case Opt_mask:
300	316	audit_log_format(ab, "mask=%s ", args[0].from);
	317	+
	318	+ if (entry->mask)
	319	+ result = -EINVAL;
	320	+
301	321	if ((strcmp(args[0].from, "MAY_EXEC")) == 0)
302	322	entry->mask = MAY_EXEC;
303	323	else if (strcmp(args[0].from, "MAY_WRITE") == 0)
...	...	@@ -313,6 +333,12 @@
313	333	break;
314	334	case Opt_fsmagic:
315	335	audit_log_format(ab, "fsmagic=%s ", args[0].from);
	336	+
	337	+ if (entry->fsmagic) {
	338	+ result = -EINVAL;
	339	+ break;
	340	+ }
	341	+
316	342	result = strict_strtoul(args[0].from, 16,
317	343	&entry->fsmagic);
318	344	if (!result)
...	...	@@ -320,6 +346,12 @@
320	346	break;
321	347	case Opt_uid:
322	348	audit_log_format(ab, "uid=%s ", args[0].from);
	349	+
	350	+ if (entry->uid != -1) {
	351	+ result = -EINVAL;
	352	+ break;
	353	+ }
	354	+
323	355	result = strict_strtoul(args[0].from, 10, &lnum);
324	356	if (!result) {
325	357	entry->uid = (uid_t) lnum;
...	...	@@ -370,7 +402,7 @@
370	402	break;
371	403	}
372	404	}
373		- if (entry->action == UNKNOWN)
	405	+ if (!result && (entry->action == UNKNOWN))
374	406	result = -EINVAL;
375	407
376	408	audit_log_format(ab, "res=%d", !!result);