All,

This patch prevents ausearch from parsing an event's "header" of node,
type, time and serial twice. That is
[node=<node>] type=<type> msg=audit(<epochsecs>.<msecs>:<serial>)

It does this by passing the llist->e value to output_interpreted_node()
as this structure holds the already parsed event "header" information.

The code does not change the ausearch -i output.

Rgds

On Thu, 2014-10-02 at 19:29 +1000, Burn Alting wrote:
> Thanks Steve,
> 
> Patch to follow this weekend.
> 
> Rgds
> 
> On Wed, 2014-10-01 at 18:28 -0400, Steve Grubb wrote:
> > On Thursday, October 02, 2014 07:52:47 AM Burn Alting wrote:
> > > On Wed, 2014-10-01 at 17:19 -0400, Steve Grubb wrote:
> > > > On Thursday, October 02, 2014 07:08:13 AM Burn Alting wrote:
> > > > > On Wed, 2014-10-01 at 14:54 -0400, Steve Grubb wrote:
> > > > > > > I am uncertain what effect of accepting this additional format 
> > > > > > > would
> > > > > > > have when adding rules to the running audit system - i.e.
> > > > > > > audit_name_to_msg_type() is called by autrace/auditctl when 
> > > > > > > parsing
> > > > > > > rules (ie the msgtype field name).
> > > > > > 
> > > > > > I think ausearch-report.c might be the place that needs updating.
> > > > > 
> > > > > So, could we modify output_interpreted_node() to no longer re-parse 
> > > > > the
> > > > > 
> > > > >    [node=<node>] type=<type> msg=audit(<epochsecs>.<msecs>:<serial>)
> > > > > 
> > > > > header and pass both the lnode and llist->e which has this data 
> > > > > already
> > > > > as the code
> > > > > 
> > > > >           if (num == -1) {
> > > > >           
> > > > >               // see if we are older and wiser now.
> > > > >               bptr = strchr(str, '[');
> > > > >               if (bptr && bptr < ptr) {
> > > > >               
> > > > >                   char *eptr;
> > > > >                   bptr++;
> > > > >                   eptr = strchr(bptr, ']');
> > > > >                   if (eptr) {
> > > > >                   
> > > > >                        *eptr = 0;
> > > > >                        errno = 0;
> > > > >                        num = strtoul(bptr, NULL, 10);
> > > > >                        *eptr = ']';
> > > > >                        if (errno)
> > > > >                        
> > > > >                            num = -1;
> > > > >                   
> > > > >                   }
> > > > >                
> > > > >                }
> > > > >          
> > > > >          }
> > > > > 
> > > > > which parses for
> > > > > 
> > > > >     type=.*[n].*
> > > > > 
> > > > > is no longer needed as we don't have that format any more?
> > > > 
> > > > That is a very loose check for UNKNOWN[####]. If you see a performance
> > > > improvement by refactoring this function, please send a patch. The 
> > > > output
> > > > needs to be identical to the old way.
> > > > 
> > > > Thanks,
> > > > -Steve
> > > 
> > > I can provide a patch to refactor this part of the code, but I want to
> > > confirm there is no longer a need to parse for
> > > 
> > >   type=some_text '[' integer_type ']' some_other_text
> > 
> > While this may have been implied by the code, the fact is that [ ] would 
> > only 
> > be in type fields when its unknown[####].
> > 
> > 
> > > given my refactoring will rely upon the parsing already done by
> > > lib/lookup_table.c:audit_name_to_msg_type(). Remember this routine only
> > > parses for
> > > Given
> > >         type=<type_value>
> > > then
> > >         <type_value>
> > > is parsed for
> > >         - a known string
> > >         - a long integer number, n, found in the specific string
> > >                 "UNKNOWN[n]"
> > >         - a long integer number, n, found in the specific string
> > >                 "n"
> > 
> > These 3 formats are all that it can ever be. So, I think you have a correct 
> > understanding.
> > 
> > -Steve
> 
> 
> --
> Linux-audit mailing list
> [email protected]
> https://www.redhat.com/mailman/listinfo/linux-audit

diff -Npru audit-2.4/src/ausearch-report.c audit-2.4_ausearch_refactor_01/src/ausearch-report.c
--- audit-2.4/src/ausearch-report.c	2014-08-25 02:39:20.000000000 +1000
+++ audit-2.4_ausearch_refactor_01/src/ausearch-report.c	2014-10-06 17:32:43.916773469 +1100
@@ -37,7 +37,7 @@
 static void output_raw(llist *l);
 static void output_default(llist *l);
 static void output_interpreted(llist *l);
-static void output_interpreted_node(const lnode *n);
+static void output_interpreted_node(const lnode *n, const event *e);
 static void interpret(char *name, char *val, int comma, int rtype);
 
 /* The machine based on elf type */
@@ -125,10 +125,10 @@ static void output_interpreted(llist *l)
 		return;
 	}
 	if (n->type >= AUDIT_DAEMON_START && n->type < AUDIT_SYSCALL) 
-		output_interpreted_node(n);
+		output_interpreted_node(n, &(l->e));
 	else {
 		do {
-			output_interpreted_node(n);
+			output_interpreted_node(n, &(l->e));
 		} while ((n=list_prev(l)));
 	}
 }
@@ -137,21 +137,25 @@ static void output_interpreted(llist *l)
  * This function will cycle through a single record and lookup each field's
  * value that it finds. 
  */
-static void output_interpreted_node(const lnode *n)
+static void output_interpreted_node(const lnode *n, const event *e)
 {
 	char *ptr, *str = n->message, *node = NULL;
 	int found, comma = 0;
+	int num = n->type;
+	struct tm *btm;
+	char tmp[32];
 
 	// Reset these because each record could be different
 	machine = -1;
 	cur_syscall = -1;
 
-	/* Check and see if we start with a node */
-	if (str[0] == 'n') {
-		ptr=strchr(str, ' ');
-		if (ptr) {
-			*ptr = 0;
-			node = str;
+	/* Check and see if we start with a node
+ 	 * If we do, and there is a space in the line
+ 	 * move the pointer to the first character past
+ 	 * the space
+  	 */
+	if (e->node) {
+		if ((ptr=strchr(str, ' ')) != NULL) {
 			str = ptr+1;
 		}
 	}
@@ -161,76 +165,34 @@ static void output_interpreted_node(cons
 	if (ptr == NULL) {
 		fprintf(stderr, "can't find time stamp\n");
 		return;
-	} else {
-		time_t t;
-		int milli,num = n->type;
-		unsigned long serial;
-		struct tm *btm;
-		char tmp[32];
-		const char *bptr;
-
-		*ptr++ = 0;
-		if (num == -1) {
-			// see if we are older and wiser now.
-			bptr = strchr(str, '[');
-			if (bptr && bptr < ptr) {
-				char *eptr;
-				bptr++;
-				eptr = strchr(bptr, ']');
-				if (eptr) {
-					*eptr = 0;
-					errno = 0;
-					num = strtoul(bptr, NULL, 10);
-					*eptr = ']';
-					if (errno) 
-						num = -1;
-				}
-			}
-		}
+	}
+
+	*ptr++ = 0;	/* move to the start of the timestamp */
 
-		// print everything up to it.
-		if (num >= 0) {
-			bptr = audit_msg_type_to_name(num);
-			if (bptr) {
-				if (node)
-					printf("%s ", node);
-				printf("type=%s msg=audit(", bptr);
-				goto no_print;
-			}
-		} 
-		if (node)
-			printf("%s ", node);
-		printf("%s(", str);
+	// print everything up to it.
+	if (num >= 0) {
+		const char	* bptr;
+		bptr = audit_msg_type_to_name(num);
+		if (bptr) {
+			if (e->node)
+				printf("node=%s ", e->node);
+			printf("type=%s msg=audit(", bptr);
+			goto no_print;
+		}
+	} 
+	if (e->node)
+		printf("node=%s ", e->node);
+	printf("%s(", str);
 no_print:
 
-		// output formatted time.
-		str = strchr(ptr, '.');
-		if (str == NULL)
-			return;
-		*str++ = 0;
-		errno = 0;
-		t = strtoul(ptr, NULL, 10);
-		if (errno)
-			return;
-		ptr = strchr(str, ':');
-		if (ptr == NULL)
-			return;
-		*ptr++ = 0;
-		milli = strtoul(str, NULL, 10);
-		if (errno)
-			return;
-		str = strchr(ptr, ')');
-		if(str == NULL)
-			return;
-		*str++ = 0;
-		serial = strtoul(ptr, NULL, 10);
-		if (errno)
-			return;
-		btm = localtime(&t);
-		strftime(tmp, sizeof(tmp), "%x %T", btm);
-		printf("%s", tmp);
-		printf(".%03d:%lu) ", milli, serial);
-	}
+	str = strchr(ptr, ')');
+	if(str == NULL)
+		return;
+	*str++ = 0;
+	btm = localtime(&e->sec);
+	strftime(tmp, sizeof(tmp), "%x %T", btm);
+	printf("%s", tmp);
+	printf(".%03d:%lu) ", e->milli, e->serial);
 
 	if (n->type == AUDIT_SYSCALL) { 
 		a0 = n->a0;
--
Linux-audit mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/linux-audit

Reply via email to