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

[Yu Zhiguo <yuzg@cn.fujitsu.com>]

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;
  }