[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [PATCH] make it match explicitly when use option '-a', '-A' and '-d' to specify "list,action"



Hello Miloslav,
CC Steve,

 Thanks for you comment,

Miloslav Trmač wrote:
I know "list" and "action" can be changed, this is convenient.
No, it is undocumented.  As an author of system-config-audit I'd much
prefer if audit rejected such options, replicating the exact code in
auditctl in order to handle all undocumented behavior the same way as
auditctl is rather impractical.

Indeed it is uncompatible with manpage, but it seems that Mr. Steve
like this convenient method:
=======refer to mail from Steve(2008/05/08)====================
 > I am a little surprised that the "-a always,exit" doesn't cause an
 > > error. I wonder if it works correctly - maybe auditctl code is smart
 > > enough to overcome syntactic dyslexia?  :)

 I was always mixing up the order when writing rules, so I fixed auditctl to
 take it in either order. Its been like this for 3-4 years.


	p = strchr(opt, ',');
	if (p == NULL || strchr(p + 1, ',') != NULL)
		return 2;
would be simpler.
I think so, thanks..


-	if (strstr(opt, "task"))
+	if (!strncmp(opt, "task,", p - opt + 1) || !strcmp(p, ",task"))
  		*flags = AUDIT_FILTER_TASK;
Each string should be recognized only in the documented position IMHO.
The patch also replaces case-sensitive matching by case-insensitive,
which is not described above.

Both strstr and strcmp are case-sensitive.


If such changes in the semantics of the parameter are accepted, at
minimum the auditctl.8 man page should be updated as well.
I think the manpage should introduce that the order of "list"
and "action" can be changed.
What's the opinion of Mr. Steve? If you think the manpage is right,
we should correct the code to compatible with it, and I'll make
another patch.


if order insensitive is right, the simpler patch now:
---
 src/auditctl.c |   26 +++++++++++++++++---------
 1 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/src/auditctl.c b/src/auditctl.c
index 2c136ea..a268bcf 100644
--- a/src/auditctl.c
+++ b/src/auditctl.c
@@ -168,27 +168,35 @@ static void usage(void)
 /* Returns 0 ok, 1 deprecated action, 2 error */
 static int audit_rule_setup(const char *opt, int *flags, int *act)
 {
-	if (strstr(opt, "task"))
+	char *p;
+
+	p = strchr(opt, ',');
+	if (!p || strchr(p + 1, ','))
+		return 2;
+
+	if (!strncmp(opt, "task,", p - opt + 1) || !strcmp(p, ",task"))
 		*flags = AUDIT_FILTER_TASK;
-	else if (strstr(opt, "entry"))
+	else if (!strncmp(opt, "entry,", p - opt + 1) || !strcmp(p, ",entry"))
 		*flags = AUDIT_FILTER_ENTRY;
-	else if (strstr(opt, "exit"))
+	else if (!strncmp(opt, "exit,", p - opt + 1) || !strcmp(p, ",exit"))
 		*flags = AUDIT_FILTER_EXIT;
-	else if (strstr(opt, "user"))
+	else if (!strncmp(opt, "user,", p - opt + 1) || !strcmp(p, ",user"))
 		*flags = AUDIT_FILTER_USER;
-	else if (strstr(opt, "exclude")) {
+	else if (!strncmp(opt, "exclude,", p - opt + 1) || !strcmp(p, ",exclude")) {
 		*flags = AUDIT_FILTER_EXCLUDE;
 		exclude = 1;
 	} else
 		return 2;
-	if (strstr(opt, "never"))
+
+	if (!strncmp(opt, "always,", p - opt + 1) || !strcmp(p, ",always"))
+		*act = AUDIT_ALWAYS;
+	else if (!strncmp(opt, "never,", p - opt + 1) || !strcmp(p, ",never"))
 		*act = AUDIT_NEVER;
-	else if (strstr(opt, "possible"))
+	else if (!strncmp(opt, "possible,", p - opt + 1) || !strcmp(p, ",possible"))
 		return 1;
-	else if (strstr(opt, "always"))
-		*act = AUDIT_ALWAYS;
 	else
 		return 2;
+
 	return 0;
 }





[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]