[PATCH 2/2] fix a bug that use option '-k key-string' cannot search out all matched logs

[PATCH 2/2] fix a bug that use option '-k key-string' cannot search out all matched logs

[Peng Haitao]

Hello Steve, 

Use option '-k key-string' cannot search out the log which contains the given key-string and message type is CONFIG_CHANGE.

For example:
echo 'node=RHEL5.2GA type=CONFIG_CHANGE msg=audit(1217404709.683:23182): auid=0 subj=root:system_r:auditctl_t:s0-s0:c0.c1023 op=remove rule key="haha" list=4 res=1' | ausearch -k haha
The output is: <no matches>

Signed-off-by: Peng Haitao <penght@cn.fujitsu.com>
---
 src/ausearch-parse.c |   55 +++++++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 52 insertions(+), 3 deletions(-)

diff --git a/src/ausearch-parse.c b/src/ausearch-parse.c
index 0c38be1..fd00013 100755
--- a/src/ausearch-parse.c
+++ b/src/ausearch-parse.c
@@ -1411,7 +1411,7 @@ static int parse_simple_message(const lnode *n, search_items *s)
 	errno = 0;
 	s->loginuid = strtoul(ptr, NULL, 10);
 	if (errno)
-		return 2;
+		return 1;
 	if (term)
 		*term = ' ';
 	else
@@ -1437,7 +1437,56 @@ static int parse_simple_message(const lnode *n, search_items *s)
 				else	// Set it back to something sane
 					term = str;
 			} else
-				return 3;
+				return 2;
+		}
+	}
+
+	if (event_key) {
+		str = strstr(term, "key=");
+		if (str != NULL) {
+			if (!s->key) {
+				//create
+				s->key = malloc(sizeof(slist));
+				if (s->key == NULL)
+					return 3;
+				slist_create(s->key);
+			}
+			ptr = str + 4;
+			if (*ptr == '"') {
+				ptr++;
+				term = strchr(ptr, '"');
+				if (term != NULL) {
+					*term = 0;
+					if (s->key) {
+						// append
+						snode sn;
+						sn.str = strdup(ptr);
+						sn.key = NULL;
+						sn.hits = 1;
+						slist_append(s->key, &sn);
+					}
+					*term = '"';
+				} else
+					return 4;
+			} else {
+				if (s->key) {
+					char *saved=NULL;
+					char *keyptr = unescape(ptr);
+					char *kptr = strtok_r(keyptr,
+						key_sep, &saved);
+					while (kptr) {
+						snode sn;
+						// append
+						sn.str = strdup(kptr);
+						sn.key = NULL;
+						sn.hits = 1;
+						slist_append(s->key, &sn);
+						kptr = strtok_r(NULL,
+							key_sep, &saved);
+					}
+					free(keyptr);
+				}
+			}
 		}
 	}
 
@@ -1457,7 +1506,7 @@ static int parse_simple_message(const lnode *n, search_items *s)
 		errno = 0;
 		s->success = strtoul(ptr, NULL, 10);
 		if (errno)
-			return 4;
+			return 5;
 		if (term)
 			*term = ' ';
 	}
-- 
1.5.4.2


-- 
Regards
Peng Haitao

Re: [PATCH 2/2] fix a bug that use option '-k key-string' cannot search out all matched logs

[Steve Grubb]

On Tuesday 29 July 2008 01:41:59 Peng Haitao wrote:
> Use option '-k key-string' cannot search out the log which contains the
> given key-string and message type is CONFIG_CHANGE.

Patch applied - Thanks!  The way that the return codes work in the parsers is 
that I wanted each return to have a different return code so that I can tell 
exactly which line it fails on when I debug. I made some adjustmenst to the 
patch for this.

Thanks,
-Steve

Re: [graphics 06448] [PATCH 2/2] fix a bug that use option '-k key-string' cannot search out all matched logs

[zhangxiliang]

Hello Steve, 

> echo 'node=RHEL5.2GA type=CONFIG_CHANGE msg=audit(1217404709.683:23182): auid=0 subj=root:system_r:auditctl_t:s0-s0:c0.c1023 op=remove rule key="haha" list=4 res=1'

Why the message which type is "CONFIG_CHANGE" contains "key" field? 
The "CONFIG_CHANGE" audit message should only describe the audit object status.

You can get the audit message by following steps:
1. # touch test1
2. # auditctl -w `pwd`/test1 -k haha
3. # mv test1 test2

I think we'd better not output "key" field in "CONFIG_CHANGE" message.
What's your opinion? If you agree with me, I'll make a patch for kernel. 

Peng Haitao said the following on 2008-07-29 13:41:
> Hello Steve, 
> 
> Use option '-k key-string' cannot search out the log which contains the given key-string and message type is CONFIG_CHANGE.
> 
> For example:
> echo 'node=RHEL5.2GA type=CONFIG_CHANGE msg=audit(1217404709.683:23182): auid=0 subj=root:system_r:auditctl_t:s0-s0:c0.c1023 op=remove rule key="haha" list=4 res=1' | ausearch -k haha
> The output is: <no matches>
> 
> Signed-off-by: Peng Haitao <penght@cn.fujitsu.com>
> ---
>  src/ausearch-parse.c |   55 +++++++++++++++++++++++++++++++++++++++++++++++--
>  1 files changed, 52 insertions(+), 3 deletions(-)
> 
> diff --git a/src/ausearch-parse.c b/src/ausearch-parse.c
> index 0c38be1..fd00013 100755
> --- a/src/ausearch-parse.c
> +++ b/src/ausearch-parse.c
> @@ -1411,7 +1411,7 @@ static int parse_simple_message(const lnode *n, search_items *s)
>  	errno = 0;
>  	s->loginuid = strtoul(ptr, NULL, 10);
>  	if (errno)
> -		return 2;
> +		return 1;
>  	if (term)
>  		*term = ' ';
>  	else
> @@ -1437,7 +1437,56 @@ static int parse_simple_message(const lnode *n, search_items *s)
>  				else	// Set it back to something sane
>  					term = str;
>  			} else
> -				return 3;
> +				return 2;
> +		}
> +	}
> +
> +	if (event_key) {
> +		str = strstr(term, "key=");
> +		if (str != NULL) {
> +			if (!s->key) {
> +				//create
> +				s->key = malloc(sizeof(slist));
> +				if (s->key == NULL)
> +					return 3;
> +				slist_create(s->key);
> +			}
> +			ptr = str + 4;
> +			if (*ptr == '"') {
> +				ptr++;
> +				term = strchr(ptr, '"');
> +				if (term != NULL) {
> +					*term = 0;
> +					if (s->key) {
> +						// append
> +						snode sn;
> +						sn.str = strdup(ptr);
> +						sn.key = NULL;
> +						sn.hits = 1;
> +						slist_append(s->key, &sn);
> +					}
> +					*term = '"';
> +				} else
> +					return 4;
> +			} else {
> +				if (s->key) {
> +					char *saved=NULL;
> +					char *keyptr = unescape(ptr);
> +					char *kptr = strtok_r(keyptr,
> +						key_sep, &saved);
> +					while (kptr) {
> +						snode sn;
> +						// append
> +						sn.str = strdup(kptr);
> +						sn.key = NULL;
> +						sn.hits = 1;
> +						slist_append(s->key, &sn);
> +						kptr = strtok_r(NULL,
> +							key_sep, &saved);
> +					}
> +					free(keyptr);
> +				}
> +			}
>  		}
>  	}
>  
> @@ -1457,7 +1506,7 @@ static int parse_simple_message(const lnode *n, search_items *s)
>  		errno = 0;
>  		s->success = strtoul(ptr, NULL, 10);
>  		if (errno)
> -			return 4;
> +			return 5;
>  		if (term)
>  			*term = ' ';
>  	}

Re: [graphics 06448] [PATCH 2/2] fix a bug that use option '-k key-string' cannot search out all matched logs

[Steve Grubb]

On Tuesday 29 July 2008 21:33:13 zhangxiliang wrote:
> > echo 'node=RHEL5.2GA type=CONFIG_CHANGE msg=audit(1217404709.683:23182):
> > auid=0 subj=root:system_r:auditctl_t:s0-s0:c0.c1023 op=remove rule
> > key="haha" list=4 res=1'
>
> Why the message which type is "CONFIG_CHANGE" contains "key" field?
> The "CONFIG_CHANGE" audit message should only describe the audit object
> status.

The reason that the key field is output is an attempt at telling the security 
officer more about which rule was deleted. Yes at the commandline you know 
what rules you just deleted, but if all you have is the logs and it happened 
some time in the past, how do you know *exactly* which rule was deleted? This 
gets us closer without having to write something in the kernel that iterates 
through the fields and changes them to text. We really can't do that in the 
kernel.

-Steve

Re: [graphics 06448] [PATCH 2/2] fix a bug that use option '-k key-string' cannot search out all matched logs

[zhangxiliang]

Hello Steve, 

Thanks for your detailed explanation.
But I found two cases for "CONFIG_CHANGE" message miss "key" field.

for example1:
1. # touch temp_file
2. # auditctl -w `pwd`/temp_file -k temp_file
3. # rm -f temp_file
The "CONFIG_CHANGE" message is "type=CONFIG_CHANGE msg=audit(1217667917.265:1027): op=updated rules specifying path="/root/temp_file" with dev=4294967295 ino=4294967295  list=0 res=1".

for example2:
1. # auditctl -e 2
2. # auditctl -a exit,always -S open -k temp_file
The "CONFIG_CHANGE" message is "type=CONFIG_CHANGE msg=audit(1217668048.831:1031): user pid=13202 uid=0 auid=0 ses=1 subj=root:system_r:auditctl_t:s0-s0:c0.c1023 audit_enabled=2 res=0".

Are they OK or error?

Steve Grubb said the following on 2008-07-30 19:06:
> On Tuesday 29 July 2008 21:33:13 zhangxiliang wrote:
>>> echo 'node=RHEL5.2GA type=CONFIG_CHANGE msg=audit(1217404709.683:23182):
>>> auid=0 subj=root:system_r:auditctl_t:s0-s0:c0.c1023 op=remove rule
>>> key="haha" list=4 res=1'
>> Why the message which type is "CONFIG_CHANGE" contains "key" field?
>> The "CONFIG_CHANGE" audit message should only describe the audit object
>> status.
> 
> The reason that the key field is output is an attempt at telling the security 
> officer more about which rule was deleted. Yes at the commandline you know 
> what rules you just deleted, but if all you have is the logs and it happened 
> some time in the past, how do you know *exactly* which rule was deleted? This 
> gets us closer without having to write something in the kernel that iterates 
> through the fields and changes them to text. We really can't do that in the 
> kernel.
> 
> -Steve
> 
> 
>