Skip to contentSkip to conversation

fix a bug that use option '-k key-string' cannot search out all matched logs

PATCH In review Peng Haitao 3 participants 5 messages Jul 29, 2008 – Jul 31, 2008

lore
Share & tools
lore
#0 lore raw

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

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>

---
 src/ausearch-parse.c |   55 +++++++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 52 insertions(+), 3 deletions(-)
src/ausearch-parse.c +52 -3
index 0c38be1..fd00013 100755
@@ -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 = ' ';
 	}
Signature · 6 lines
1.5.4.2


-- 
Regards
Peng Haitao
#1 lore raw
Quoted 2 lines · 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

#2 lore raw
In reply to Peng Haitao

Hello Steve,

Quoted 1 line

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:

Quoted 93 lines

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>

---
 src/ausearch-parse.c |   55 +++++++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 52 insertions(+), 3 deletions(-)
src/ausearch-parse.c +52 -3
index 0c38be1..fd00013 100755
@@ -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 = ' ';
 	}
#3 lore raw
Quoted 7 lines · On Tuesday 29 July 2008 21:33:13 zhangxiliang wrote:
Quoted 3 lines

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

#4 lore raw

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:

Quoted 20 lines · On Tuesday 29 July 2008 21:33:13 zhangxiliang wrote:
Quoted 6 lines
Quoted 3 lines

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