Skip to contentSkip to conversation

Audit Failure Query Functionality

RFC PATCH In review Lisa Smith 3 participants 5 messages Jun 13, 2006 – Jun 14, 2006

lore
Share & tools
lore

Lisa Smith

author
#0 lore raw

RFC PATCH 1/1 Audit Failure Query Functionality

This is this initial patch for the audit failure query functionality.

Currently, each service and application is responsible for determining the action to take when the audit subsystem is unavailable (stop the action or service, continue on, log a message). Different customers may want different actions taken, and this flexibility is not possible if the action is hard-coded into the application.

This patch introduces a new query function, audit_failure_action(), which can be called to determine what action should be taken by the service after audit_open() fails. The function will read the tunable "auditfailure" from the new /etc/libaudit.conf file to determine what action should be taken. audit_failure_action() can also be passed a filename so that an application-specific action can be defined within its own file.

The audit_open() man page will be updated to introduce application developers to the new query function that should be used when audit_open() returns failure. A new man page will also be created for the audit_failure_action() function.

This patch is backed against audit version 1.2.3.

Lisa

----

 libaudit.c |  163 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 libaudit.h |   25 ++++++++
 2 files changed, 188 insertions(+)
libaudit.c 2006-06-13 14:46:33.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +163 -0
diff -burN orig/libaudit.c src/libaudit.c
@@ -68,6 +68,169 @@
        return rc;
 }
 
+/*
+ * This function will retrieve the audit failure tunable value
+ * from the filename passed in.  If no file is specified, the default
+ * /etc/libaudit.conf file will be used.
+ */
+auditfail_t audit_failure_action(char *file)
+{
+       int ret;
+       struct nv_pair nv;
+
+       if (file == NULL)
+               file = AUDIT_FAIL_CONFIG;
+
+       /* Find the audit failure action tunable in the config file */
+       ret = search_audituser_conf(file, AUDIT_FAIL_KEYWORD, &nv);
+
+       if (ret == -1) {
+               audit_msg(LOG_WARNING, "Error in %s", file);
+               return ERR;
+       } else if (ret == 1) {
+               /* Keyword not found, so do the default action */
+               return IGNORE;
+       }
+
+       /* Translate tunable string to valid enum */
+       if (strncmp(nv.value, AUDIT_FAIL_IGNORE,
+               strlen(AUDIT_FAIL_IGNORE)) == 0) {
+               free (nv.name);
+               free (nv.value);
+               return IGNORE;
+       }
+       else if (strncmp(nv.value, AUDIT_FAIL_LOG,
+               strlen(AUDIT_FAIL_LOG)) == 0) {
+               free (nv.name);
+               free (nv.value);
+               return LOG;
+        }
+       else if (strncmp(nv.value, AUDIT_FAIL_TERM,
+               strlen(AUDIT_FAIL_TERM)) == 0) {
+               free (nv.name);
+               free (nv.value);
+               return TERM;
+        }
+       else {
+               /* If we get this far, the failure action was not valid */
+               audit_msg(LOG_ERR, "Invalid %s keyword value in %s",
+                       AUDIT_FAIL_KEYWORD, file);
+               free (nv.name);
+               free (nv.value);
+               return ERR;
+       }
+}
+
+/*
+ * This function searches for a keyword pair in the passed in filename.
+ * If the keyword pair is found, it is saved in the nv structure and zero
+ * is returned. If the file can not be opened, -1 is returned.
+ * If the keyword is not found in the file, 1 is returned.
+ *
+ * nv->name and nv->value must be freed if an error will be returned from this
+ * function after nv_split() is called. If this function returns success,
+ * the caller must free nv->name and nv->value when finished using the
+ * values.
+ */
+int search_audituser_conf(char *file, char *keyword, struct nv_pair *nv)
+{
+       int rc, lineno = 1;
+       size_t len = 0;
+       ssize_t bytesread;
+       FILE *fp;
+       char *buf = NULL;
+
+       /* Open the file for line by line reading*/
+       fp = fopen(file, "r");
+       if (fp == NULL) {
+               audit_msg(LOG_ERR, "Error - fdopen failed for %s (%s)",
+                       file, strerror(errno));
+               return -1;
+        }
+
+       while ((bytesread = getline(&buf, &len, fp)) != -1) {
+
+               if (buf[0] == '#') {
+                       lineno++;
+                       continue;       // Ignore comments
+               }
+
+               /* Convert line into name-value pair */
+               rc = nv_split(buf, nv);
+               if (rc == 1) {
+                       audit_msg(LOG_ERR, "Error on line %d in %s", lineno,
+                               file);
+                       lineno++;
+                       continue;
+                }
+
+               /* Find the name-value pair */
+               if (strcmp(nv->name, keyword) == 0)
+               {
+                       fclose(fp);
+                       if (buf)
+                               free (buf);
+                       return 0;
+               }
+
+               lineno++;
+       }
+
+       /* If we get here, the keyword was not found in the file */
+       audit_msg(LOG_ERR, "Keyword %s not found in %s", keyword, file);
+       fclose(fp);
+       if (buf)
+               free (buf);
+       if (nv->name)
+               free (nv->name);
+       if (nv->value)
+               free (nv->value);
+       return 1;
+}
+
+/*
+ *  This function parses a line looking for a keyword = value pair
+ *  and if found, returns it in the nv structure. If the function
+ *  returns success, the calling function is expected to free
+ *  nv->name and nv->value.
+ */
+int nv_split(char *buffer, struct nv_pair *nv)
+{
+       /* Get the name part */
+       char *saveptr, *ptr = NULL;
+       char *buf = strdup(buffer);
+
+       /* Look for = in buf */
+       nv->name = NULL;
+       nv->value = NULL;
+       ptr = strtok_r(buf, " =", &saveptr);
+       if ((ptr == NULL) || !(strcmp(ptr,"\n"))) {
+               return 0;   // If there's nothing, go to next line
+       }
+       nv->name = strdup(ptr);
+
+        /* Get the keyword value */
+       ptr = strtok_r(NULL, " =", &saveptr);
+       if (ptr == NULL) {
+               free (nv->name);
+               return 1;
+       }
+       nv->value = strdup(ptr);
+
+       /* Make sure there's nothing else on the line */
+       ptr = strtok_r(NULL, " ", &saveptr);
+       if (ptr) {
+               free (nv->name);
+               free (nv->value);
+               return 1;
+       }
+
+       /* Everything is OK */
+       return 0;
+}
+
+
+
 int audit_set_enabled(int fd, uint32_t enabled)
 {
        int rc;
libaudit.h 2006-06-13 13:01:30.000000000 -0400 renamed from libaudit.h 2006-05-25 17:38:21.000000000 -0400 +25 -0
diff -burN orig/libaudit.h src/libaudit.h
@@ -248,6 +248,28 @@
         MACH_ALPHA
 } machine_t;
 
+/* These are the valid audit failure tunable enum values */
+typedef enum {
+       ERR=-1,
+       IGNORE=0,
+       LOG,
+       TERM
+} auditfail_t;
+
+/* #defines for the audit failure query  */
+#define AUDIT_FAIL_CONFIG      "/etc/libaudit.conf"
+#define AUDIT_FAIL_KEYWORD     "auditfailure"
+#define AUDIT_FAIL_IGNORE      "ignore"
+#define AUDIT_FAIL_LOG         "log"
+#define AUDIT_FAIL_TERM        "terminate"
+
+/* Name-value pair */
+struct nv_pair
+{
+       char *name;
+       char *value;
+};
+
 /*
  * audit_rule_data supports filter rules with both integer and string
  * fields. It corresponds with AUDIT_ADD_RULE, AUDIT_DEL_RULE and
@@ -362,6 +384,9 @@
 /* AUDIT_GET */
 extern int audit_request_status(int fd);
 extern int audit_is_enabled(int fd);
+extern auditfail_t audit_failure_action(char *file);
+static int search_audituser_conf(char *file, char *keyword, struct nv_pair *nv);
+static int nv_split(char *buf, struct nv_pair *nv);
 
 /* AUDIT_SET */
 typedef enum { WAIT_NO, WAIT_YES } rep_wait_t;
#1 lore raw
Quoted 2 lines · Lisa Smith

This is this initial patch for the audit failure query functionality.

<snip>

Quoted 5 lines

This patch is backed against audit version 1.2.3.

Lisa

Hello Lisa,

I'm a little rusty... I just had a few things..

-tim

Quoted 21 lines

----

 libaudit.c |  163 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 libaudit.h |   25 ++++++++
 2 files changed, 188 insertions(+)

diff -burN orig/libaudit.c src/libaudit.c

libaudit.c 2006-06-13 14:46:33.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +8 -0
@@ -68,6 +68,169 @@
        return rc;
 }
 
+/*
+ * This function will retrieve the audit failure tunable value
+ * from the filename passed in.  If no file is specified, the default
+ * /etc/libaudit.conf file will be used.
+ */
+auditfail_t audit_failure_action(char *file)
+{
+       int ret;

libaudit.c 2006-06-13 14:46:33.000000000 -0400 line 88

Quoted 10 lines
libaudit.c 2006-06-13 14:46:33.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +10 -0
+       struct nv_pair nv;
+
+       if (file == NULL)
+               file = AUDIT_FAIL_CONFIG;
+
+       /* Find the audit failure action tunable in the config file */
+       ret = search_audituser_conf(file, AUDIT_FAIL_KEYWORD, &nv);
+
+       if (ret == -1) {
+               audit_msg(LOG_WARNING, "Error in %s", file);

Would you not want to use LOG_ERR here? Or if you really did intend a LOG_WARNING using the word "Error" in your message might be confusing.

[..]

libaudit.c 2006-06-13 14:46:33.000000000 -0400 line 89

Quoted 1 line
libaudit.c 2006-06-13 14:46:33.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +1 -0
+               return ERR;

This is really awkward to me. I think it makes more sense to make this function simply return success or failure and update a parameter passed to it by the user with the correct failure mode. As it stands now, ERR isn't really a failure mode. May I also suggest making your enumeration a little more descriptive?

[..]

Quoted 22 lines
libaudit.c 2006-06-13 14:46:33.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +22 -0
+       } else if (ret == 1) {
+               /* Keyword not found, so do the default action */
+               return IGNORE;
+       }
+
+       /* Translate tunable string to valid enum */
+       if (strncmp(nv.value, AUDIT_FAIL_IGNORE,
+               strlen(AUDIT_FAIL_IGNORE)) == 0) {
+               free (nv.name);
+               free (nv.value);
+               return IGNORE;
+       }
+       else if (strncmp(nv.value, AUDIT_FAIL_LOG,
+               strlen(AUDIT_FAIL_LOG)) == 0) {
+               free (nv.name);
+               free (nv.value);
+               return LOG;
+        }
+       else if (strncmp(nv.value, AUDIT_FAIL_TERM,
+               strlen(AUDIT_FAIL_TERM)) == 0) {
+               free (nv.name);
+               free (nv.value);

libaudit.c 2006-06-13 14:46:33.000000000 -0400 line 121

Quoted 10 lines
libaudit.c 2006-06-13 14:46:33.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +10 -0
+               return TERM;
+        }
+       else {
+               /* If we get this far, the failure action was not valid */
+               audit_msg(LOG_ERR, "Invalid %s keyword value in %s",
+                       AUDIT_FAIL_KEYWORD, file);
+               free (nv.name);
+               free (nv.value);
+               return ERR;
+       }

Looks like there's a lot of duplicate code here. This can be better organized.

[..]

Quoted 18 lines
libaudit.c 2006-06-13 14:46:33.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +18 -0
+}
+
+/*
+ * This function searches for a keyword pair in the passed in filename.
+ * If the keyword pair is found, it is saved in the nv structure and zero
+ * is returned. If the file can not be opened, -1 is returned.
+ * If the keyword is not found in the file, 1 is returned.
+ *
+ * nv->name and nv->value must be freed if an error will be returned from this
+ * function after nv_split() is called. If this function returns success,
+ * the caller must free nv->name and nv->value when finished using the
+ * values.
+ */
+int search_audituser_conf(char *file, char *keyword, struct nv_pair *nv)
+{
+       int rc, lineno = 1;
+       size_t len = 0;
+       ssize_t bytesread;

libaudit.c 2006-06-13 14:46:33.000000000 -0400 line 149

Quoted 10 lines
libaudit.c 2006-06-13 14:46:33.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +10 -0
+       FILE *fp;
+       char *buf = NULL;
+
+       /* Open the file for line by line reading*/
+       fp = fopen(file, "r");
+       if (fp == NULL) {
+               audit_msg(LOG_ERR, "Error - fdopen failed for %s (%s)",
+                       file, strerror(errno));
+               return -1;
+        }

Why return -1, why not return errno?

[..]

libaudit.c 2006-06-13 14:46:33.000000000 -0400 line 156

Quoted 7 lines
libaudit.c 2006-06-13 14:46:33.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +7 -0
+
+       while ((bytesread = getline(&buf, &len, fp)) != -1) {
+
+               if (buf[0] == '#') {
+                       lineno++;
+                       continue;       // Ignore comments
+               }

So will this blow up if buf[0] == ' ' ?

[..]

Quoted 5 lines
libaudit.c 2006-06-13 14:46:33.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +5 -0
+
+               /* Convert line into name-value pair */
+               rc = nv_split(buf, nv);
+               if (rc == 1) {
+                       audit_msg(LOG_ERR, "Error on line %d in %s", lineno,

libaudit.c 2006-06-13 14:46:33.000000000 -0400 line 171

Quoted 10 lines
libaudit.c 2006-06-13 14:46:33.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +10 -0
+                               file);
+                       lineno++;
+                       continue;
+                }
+
+               /* Find the name-value pair */
+               if (strcmp(nv->name, keyword) == 0)
+               {
+                       fclose(fp);
+                       if (buf)

Superfluous conditional.

[..]

libaudit.c 2006-06-13 14:46:33.000000000 -0400 line 182

Quoted 11 lines
libaudit.c 2006-06-13 14:46:33.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +11 -0
+                               free (buf);
+                       return 0;
+               }
+
+               lineno++;
+       }
+
+       /* If we get here, the keyword was not found in the file */
+       audit_msg(LOG_ERR, "Keyword %s not found in %s", keyword, file);
+       fclose(fp);
+       if (buf)

Again.

[..]

libaudit.c 2006-06-13 14:46:33.000000000 -0400 line 187

Quoted 5 lines
libaudit.c 2006-06-13 14:46:33.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +5 -0
+               free (buf);
+       if (nv->name)
+               free (nv->name);
+       if (nv->value)
+               free (nv->value);

:)

[..]

Quoted 10 lines
libaudit.c 2006-06-13 14:46:33.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +10 -0
+       return 1;
+}
+
+/*
+ *  This function parses a line looking for a keyword = value pair
+ *  and if found, returns it in the nv structure. If the function
+ *  returns success, the calling function is expected to free
+ *  nv->name and nv->value.
+ */
+int nv_split(char *buffer, struct nv_pair *nv)

libaudit.c 2006-06-13 14:46:33.000000000 -0400 line 207

Quoted 10 lines
libaudit.c 2006-06-13 14:46:33.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +10 -0
+{
+       /* Get the name part */
+       char *saveptr, *ptr = NULL;
+       char *buf = strdup(buffer);
+
+       /* Look for = in buf */
+       nv->name = NULL;
+       nv->value = NULL;
+       ptr = strtok_r(buf, " =", &saveptr);
+       if ((ptr == NULL) || !(strcmp(ptr,"\n"))) {

You can kill that strcmp, right?

if (!ptr || *ptr == '\n') {

[..]

Quoted 48 lines
libaudit.c 2006-06-13 14:46:33.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +26 -0
+               return 0;   // If there's nothing, go to next line
+       }
+       nv->name = strdup(ptr);
+
+        /* Get the keyword value */
+       ptr = strtok_r(NULL, " =", &saveptr);
+       if (ptr == NULL) {
+               free (nv->name);
+               return 1;
+       }
+       nv->value = strdup(ptr);
+
+       /* Make sure there's nothing else on the line */
+       ptr = strtok_r(NULL, " ", &saveptr);
+       if (ptr) {
+               free (nv->name);
+               free (nv->value);
+               return 1;
+       }
+
+       /* Everything is OK */
+       return 0;
+}
+
+
+
 int audit_set_enabled(int fd, uint32_t enabled)
 {
        int rc;

diff -burN orig/libaudit.h src/libaudit.h

libaudit.h 2006-06-13 13:01:30.000000000 -0400 renamed from libaudit.h 2006-05-25 17:38:21.000000000 -0400 +12 -0
@@ -248,6 +248,28 @@
         MACH_ALPHA
 } machine_t;
 
+/* These are the valid audit failure tunable enum values */
+typedef enum {
+       ERR=-1,
+       IGNORE=0,
+       LOG,
+       TERM
+} auditfail_t;
+
+/* #defines for the audit failure query  */
+#define AUDIT_FAIL_CONFIG      "/etc/libaudit.conf"
+#define AUDIT_FAIL_KEYWORD     "auditfailure"
+#define AUDIT_FAIL_IGNORE      "ignore"

libaudit.h 2006-06-13 13:01:30.000000000 -0400 line 272

Quoted 10 lines
libaudit.h 2006-06-13 13:01:30.000000000 -0400 renamed from libaudit.h 2006-05-25 17:38:21.000000000 -0400 +10 -0
+#define AUDIT_FAIL_LOG         "log"
+#define AUDIT_FAIL_TERM        "terminate"
+
+/* Name-value pair */
+struct nv_pair
+{
+       char *name;
+       char *value;
+};
+

You should just write a little helper routine called something like:

	free_nv(struct nv_pair *nv);

It's obvious what it does and it consolidates some code and makes things look cleaner.

Quoted 13 lines
libaudit.h 2006-06-13 13:01:30.000000000 -0400 renamed from libaudit.h 2006-05-25 17:38:21.000000000 -0400 +3 -0
 /*
  * audit_rule_data supports filter rules with both integer and string
  * fields. It corresponds with AUDIT_ADD_RULE, AUDIT_DEL_RULE and
@@ -362,6 +384,9 @@
 /* AUDIT_GET */
 extern int audit_request_status(int fd);
 extern int audit_is_enabled(int fd);
+extern auditfail_t audit_failure_action(char *file);
+static int search_audituser_conf(char *file, char *keyword, struct nv_pair *nv);
+static int nv_split(char *buf, struct nv_pair *nv);
 
 /* AUDIT_SET */
 typedef enum { WAIT_NO, WAIT_YES } rep_wait_t;
author
#2 lore raw

Tim,

Quoted 10 lines

libaudit.c 2006-06-13 14:46:33.000000000 -0400 line 88

Quoted 6 lines
libaudit.c 2006-06-13 14:46:33.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +5 -0
+       /* Find the audit failure action tunable in the config file */
+       ret = search_audituser_conf(file, AUDIT_FAIL_KEYWORD, &nv);
+
+       if (ret == -1) {
+               audit_msg(LOG_WARNING, "Error in %s", file);

Would you not want to use LOG_ERR here? Or if you really did intend a LOG_WARNING using the word "Error" in your message might be confusing.

Good catch. I've changed this to LOG_ERR.

Quoted 8 lines
Quoted 2 lines

+ return ERR;

This is really awkward to me. I think it makes more sense to make this function simply return success or failure and update a parameter passed to it by the user with the correct failure mode. As it stands now, ERR isn't really a failure mode. May I also suggest making your enumeration a little more descriptive?

How about on any error IGNORE is simply returned, and the ERR mode is eliminated? There will still be an audit log message, so the error does not vanish. This means the caller will always have one of the three failure modes to handle.

What did you have in mind to make the enumeration more descriptive?

Quoted 10 lines

libaudit.c 2006-06-13 14:46:33.000000000 -0400 line 149

Quoted 9 lines
libaudit.c 2006-06-13 14:46:33.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +8 -0
+
+       /* Open the file for line by line reading*/
+       fp = fopen(file, "r");
+       if (fp == NULL) {
+               audit_msg(LOG_ERR, "Error - fdopen failed for %s (%s)",
+                       file, strerror(errno));
+               return -1;
+        }
Quoted 2 lines

Why return -1, why not return errno?

The caller only cares that the file could not be opened - it doesn't matter why. If we passes errno back, nothing would be done with that value anyway. The errno will be posted in the log, however, if an admin has a need to debug a problem.

Quoted 13 lines

[..]

libaudit.c 2006-06-13 14:46:33.000000000 -0400 line 156

Quoted 8 lines
libaudit.c 2006-06-13 14:46:33.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +7 -0
+
+       while ((bytesread = getline(&buf, &len, fp)) != -1) {
+
+               if (buf[0] == '#') {
+                       lineno++;
+                       continue;       // Ignore comments
+               }

So will this blow up if buf[0] == ' ' ?

No. getline() will retrieve a new line from a file until there are no more lines in the file. After all lines have been read, getline() will return -1 and drop out of the while loop.

Quoted 12 lines

[..]

Quoted 10 lines
libaudit.c 2006-06-13 14:46:33.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +10 -0
+
+               /* Convert line into name-value pair */
+               rc = nv_split(buf, nv);
+               if (rc == 1) {
+                       audit_msg(LOG_ERR, "Error on line %d in %s", lineno,
+                               file);
+                       lineno++;
+                       continue;
+                }
+
Quoted 9 lines

libaudit.c 2006-06-13 14:46:33.000000000 -0400 line 171

Quoted 6 lines
libaudit.c 2006-06-13 14:46:33.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +5 -0
+               /* Find the name-value pair */
+               if (strcmp(nv->name, keyword) == 0)
+               {
+                       fclose(fp);
+                       if (buf)

Superfluous conditional.

Fixed, thanks.

Quoted 24 lines
Quoted 11 lines
libaudit.c 2006-06-13 14:46:33.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +10 -0
+       return 1;
+}
+
+/*
+ *  This function parses a line looking for a keyword = value pair
+ *  and if found, returns it in the nv structure. If the function
+ *  returns success, the calling function is expected to free
+ *  nv->name and nv->value.
+ */
+int nv_split(char *buffer, struct nv_pair *nv)

libaudit.c 2006-06-13 14:46:33.000000000 -0400 line 207

Quoted 10 lines
libaudit.c 2006-06-13 14:46:33.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +10 -0
+{
+       /* Get the name part */
+       char *saveptr, *ptr = NULL;
+       char *buf = strdup(buffer);
+
+       /* Look for = in buf */
+       nv->name = NULL;
+       nv->value = NULL;
+       ptr = strtok_r(buf, " =", &saveptr);
+       if ((ptr == NULL) || !(strcmp(ptr,"\n"))) {

You can kill that strcmp, right?

Quoted 2 lines

if (!ptr || *ptr == '\n') {

Yup, corrected.

Quoted 8 lines

You should just write a little helper routine called something like:

	free_nv(struct nv_pair *nv);

It's obvious what it does and it consolidates some code and makes things look cleaner.

Great idea; will do.

Thanks for your comments.

Lisa

#3 lore raw
In reply to Lisa Smith
Quoted 1 line · Lisa Smith

This is this initial patch for the audit failure query functionality.

[...]

Quoted 28 lines
 libaudit.c |  163 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 libaudit.h |   25 ++++++++
 2 files changed, 188 insertions(+)

diff -burN orig/libaudit.c src/libaudit.c

libaudit.c 2006-06-13 14:46:33.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +17 -0
@@ -68,6 +68,169 @@
        return rc;
 }
 
+/*
+ * This function will retrieve the audit failure tunable value
+ * from the filename passed in.  If no file is specified, the default
+ * /etc/libaudit.conf file will be used.
+ */
+auditfail_t audit_failure_action(char *file)
+{
+       int ret;
+       struct nv_pair nv;
+
+       if (file == NULL)
+               file = AUDIT_FAIL_CONFIG;
+
+       /* Find the audit failure action tunable in the config file */
+       ret = search_audituser_conf(file, AUDIT_FAIL_KEYWORD, &nv);
+
+       if (ret == -1) {

libaudit.c 2006-06-13 14:46:33.000000000 -0400 line 97

Quoted 10 lines
libaudit.c 2006-06-13 14:46:33.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +10 -0
+               audit_msg(LOG_WARNING, "Error in %s", file);
+               return ERR;
+       } else if (ret == 1) {
+               /* Keyword not found, so do the default action */
+               return IGNORE;
+       }
+
+       /* Translate tunable string to valid enum */
+       if (strncmp(nv.value, AUDIT_FAIL_IGNORE,
+               strlen(AUDIT_FAIL_IGNORE)) == 0) {

This means that "ignores" will be valid, as will "logout".

Quoted 4 lines
libaudit.c 2006-06-13 14:46:33.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +4 -0
+               free (nv.name);
+               free (nv.value);
+               return IGNORE;
+       }

[...]

Quoted 26 lines
libaudit.c 2006-06-13 14:46:33.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +26 -0
+/*
+ * This function searches for a keyword pair in the passed in filename.
+ * If the keyword pair is found, it is saved in the nv structure and zero
+ * is returned. If the file can not be opened, -1 is returned.
+ * If the keyword is not found in the file, 1 is returned.
+ *
+ * nv->name and nv->value must be freed if an error will be returned from this
+ * function after nv_split() is called. If this function returns success,
+ * the caller must free nv->name and nv->value when finished using the
+ * values.
+ */
+int search_audituser_conf(char *file, char *keyword, struct nv_pair *nv)
+{
+       int rc, lineno = 1;
+       size_t len = 0;
+       ssize_t bytesread;
+       FILE *fp;
+       char *buf = NULL;
+
+       /* Open the file for line by line reading*/
+       fp = fopen(file, "r");
+       if (fp == NULL) {
+               audit_msg(LOG_ERR, "Error - fdopen failed for %s (%s)",
+                       file, strerror(errno));
+               return -1;
+        }

libaudit.c 2006-06-13 14:46:33.000000000 -0400 line 159

Quoted 10 lines
libaudit.c 2006-06-13 14:46:33.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +10 -0
+
+       while ((bytesread = getline(&buf, &len, fp)) != -1) {
+
+               if (buf[0] == '#') {
+                       lineno++;
+                       continue;       // Ignore comments
+               }
+
+               /* Convert line into name-value pair */
+               rc = nv_split(buf, nv);

The values in nv are leaked when there isn't a match or an error.

Quoted 32 lines
libaudit.c 2006-06-13 14:46:33.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +32 -0
+               if (rc == 1) {
+                       audit_msg(LOG_ERR, "Error on line %d in %s", lineno,
+                               file);
+                       lineno++;
+                       continue;
+                }
+
+               /* Find the name-value pair */
+               if (strcmp(nv->name, keyword) == 0)
+               {
+                       fclose(fp);
+                       if (buf)
+                               free (buf);
+                       return 0;
+               }
+
+               lineno++;
+       }
+
+       /* If we get here, the keyword was not found in the file */
+       audit_msg(LOG_ERR, "Keyword %s not found in %s", keyword, file);
+       fclose(fp);
+       if (buf)
+               free (buf);
+       if (nv->name)
+               free (nv->name);
+       if (nv->value)
+               free (nv->value);
+       return 1;
+}
+
+/*

libaudit.c 2006-06-13 14:46:33.000000000 -0400 line 201

Quoted 10 lines
libaudit.c 2006-06-13 14:46:33.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +10 -0
+ *  This function parses a line looking for a keyword = value pair
+ *  and if found, returns it in the nv structure. If the function
+ *  returns success, the calling function is expected to free
+ *  nv->name and nv->value.
+ */
+int nv_split(char *buffer, struct nv_pair *nv)
+{
+       /* Get the name part */
+       char *saveptr, *ptr = NULL;
+       char *buf = strdup(buffer);

This is always leaked.

libaudit.c 2006-06-13 14:46:33.000000000 -0400 line 213

Quoted 12 lines
libaudit.c 2006-06-13 14:46:33.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +12 -0
+
+       /* Look for = in buf */
+       nv->name = NULL;
+       nv->value = NULL;
+       ptr = strtok_r(buf, " =", &saveptr);
+       if ((ptr == NULL) || !(strcmp(ptr,"\n"))) {
+               return 0;   // If there's nothing, go to next line
+       }
+       nv->name = strdup(ptr);
+
+        /* Get the keyword value */
+       ptr = strtok_r(NULL, " =", &saveptr);

I appreciate this is somewhat easier given C's default string API, but
it would be really nice to do the right thing if the user uses "x=y" instead of needing "x =y".
This also isn't how auditd parses the it's file.

Quoted 27 lines
libaudit.c 2006-06-13 14:46:33.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +20 -0
+       if (ptr == NULL) {
+               free (nv->name);
+               return 1;
+       }
+       nv->value = strdup(ptr);
+
+       /* Make sure there's nothing else on the line */
+       ptr = strtok_r(NULL, " ", &saveptr);
+       if (ptr) {
+               free (nv->name);
+               free (nv->value);
+               return 1;
+       }
+
+       /* Everything is OK */
+       return 0;
+}
+
+
+
 int audit_set_enabled(int fd, uint32_t enabled)
 {
        int rc;

diff -burN orig/libaudit.h src/libaudit.h

libaudit.h 2006-06-13 13:01:30.000000000 -0400 line 257

Quoted 10 lines
libaudit.h 2006-06-13 13:01:30.000000000 -0400 renamed from libaudit.h 2006-05-25 17:38:21.000000000 -0400 +7 -0
@@ -248,6 +248,28 @@
         MACH_ALPHA
 } machine_t;
 
+/* These are the valid audit failure tunable enum values */
+typedef enum {
+       ERR=-1,
+       IGNORE=0,
+       LOG,
+       TERM
+} auditfail_t;

These enum values should be namespaced esp. as they are very generic
names.

Quoted 5 lines
libaudit.h 2006-06-13 13:01:30.000000000 -0400 renamed from libaudit.h 2006-05-25 17:38:21.000000000 -0400 +5 -0
+
+/* #defines for the audit failure query  */
+#define AUDIT_FAIL_CONFIG      "/etc/libaudit.conf"
+#define AUDIT_FAIL_KEYWORD     "auditfailure"
+#define AUDIT_FAIL_IGNORE      "ignore"

libaudit.h 2006-06-13 13:01:30.000000000 -0400 line 272

Quoted 10 lines
libaudit.h 2006-06-13 13:01:30.000000000 -0400 renamed from libaudit.h 2006-05-25 17:38:21.000000000 -0400 +10 -0
+#define AUDIT_FAIL_LOG         "log"
+#define AUDIT_FAIL_TERM        "terminate"
+
+/* Name-value pair */
+struct nv_pair
+{
+       char *name;
+       char *value;
+};
+

This should be namespaced.

libaudit.h 2006-06-13 13:01:30.000000000 -0400 line 389

Quoted 10 lines
libaudit.h 2006-06-13 13:01:30.000000000 -0400 renamed from libaudit.h 2006-05-25 17:38:21.000000000 -0400 +3 -0
 /*
  * audit_rule_data supports filter rules with both integer and string
  * fields. It corresponds with AUDIT_ADD_RULE, AUDIT_DEL_RULE and
@@ -362,6 +384,9 @@
 /* AUDIT_GET */
 extern int audit_request_status(int fd);
 extern int audit_is_enabled(int fd);
+extern auditfail_t audit_failure_action(char *file);
+static int search_audituser_conf(char *file, char *keyword, struct nv_pair *nv);
+static int nv_split(char *buf, struct nv_pair *nv);

These shouldn't be in the public .h file.

libaudit.h 2006-06-13 13:01:30.000000000 -0400 line 392

Quoted 2 lines
libaudit.h 2006-06-13 13:01:30.000000000 -0400 renamed from libaudit.h 2006-05-25 17:38:21.000000000 -0400 +0 -0
 /* AUDIT_SET */
 typedef enum { WAIT_NO, WAIT_YES } rep_wait_t;

Could these be namespaced too?

Signature · 1 line
James Antill <[email protected]>
author
#4 lore raw

James,

Quoted 5 lines

libaudit.c 2006-06-13 14:46:33.000000000 -0400 line 97

Quoted 3 lines
libaudit.c 2006-06-13 14:46:33.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +3 -0
+       /* Translate tunable string to valid enum */
+       if (strncmp(nv.value, AUDIT_FAIL_IGNORE,
+               strlen(AUDIT_FAIL_IGNORE)) == 0) {

This means that "ignores" will be valid, as will "logout".

Hmm... Good point. I'll see what I can do about that.

Quoted 12 lines

libaudit.c 2006-06-13 14:46:33.000000000 -0400 line 159

Quoted 9 lines
libaudit.c 2006-06-13 14:46:33.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +9 -0
+       while ((bytesread = getline(&buf, &len, fp)) != -1) {
+
+               if (buf[0] == '#') {
+                       lineno++;
+                       continue;       // Ignore comments
+               }
+
+               /* Convert line into name-value pair */
+               rc = nv_split(buf, nv);

The values in nv are leaked when there isn't a match or an error.

Quoted 7 lines

libaudit.c 2006-06-13 14:46:33.000000000 -0400 line 201

Quoted 5 lines
libaudit.c 2006-06-13 14:46:33.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +5 -0
+int nv_split(char *buffer, struct nv_pair *nv)
+{
+       /* Get the name part */
+       char *saveptr, *ptr = NULL;
+       char *buf = strdup(buffer);

This is always leaked.

Good catches. I've fixed both these leaks.

Quoted 12 lines

libaudit.c 2006-06-13 14:46:33.000000000 -0400 line 213

Quoted 11 lines
libaudit.c 2006-06-13 14:46:33.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +11 -0
+       /* Look for = in buf */
+       nv->name = NULL;
+       nv->value = NULL;
+       ptr = strtok_r(buf, " =", &saveptr);
+       if ((ptr == NULL) || !(strcmp(ptr,"\n"))) {
+               return 0;   // If there's nothing, go to next line
+       }
+       nv->name = strdup(ptr);
+
+        /* Get the keyword value */
+       ptr = strtok_r(NULL, " =", &saveptr);
Quoted 4 lines

I appreciate this is somewhat easier given C's default string API, but
it would be really nice to do the right thing if the user uses "x=y" instead of needing "x =y".
This also isn't how auditd parses the it's file.

Actually, this code will handle "x=y", "x =y", "x= y" and "x = y".

Quoted 20 lines

libaudit.h 2006-06-13 13:01:30.000000000 -0400 line 257

Quoted 7 lines
libaudit.h 2006-06-13 13:01:30.000000000 -0400 renamed from libaudit.h 2006-05-25 17:38:21.000000000 -0400 +7 -0
+/* These are the valid audit failure tunable enum values */
+typedef enum {
+       ERR=-1,
+       IGNORE=0,
+       LOG,
+       TERM
+} auditfail_t;

These enum values should be namespaced esp. as they are very generic
names.

Quoted 9 lines
libaudit.h 2006-06-13 13:01:30.000000000 -0400 renamed from libaudit.h 2006-05-25 17:38:21.000000000 -0400 +9 -0
+
+/* #defines for the audit failure query  */
+#define AUDIT_FAIL_CONFIG      "/etc/libaudit.conf"
+#define AUDIT_FAIL_KEYWORD     "auditfailure"
+#define AUDIT_FAIL_IGNORE      "ignore"
+#define AUDIT_FAIL_LOG         "log"
+#define AUDIT_FAIL_TERM        "terminate"
+
+/* Name-value pair */
Quoted 8 lines

libaudit.h 2006-06-13 13:01:30.000000000 -0400 line 272

Quoted 6 lines
libaudit.h 2006-06-13 13:01:30.000000000 -0400 renamed from libaudit.h 2006-05-25 17:38:21.000000000 -0400 +6 -0
+struct nv_pair
+{
+       char *name;
+       char *value;
+};
+

This should be namespaced.

Will do.

Quoted 4 lines
Quoted 4 lines

/*
* audit_rule_data supports filter rules with both integer and string
* fields. It corresponds with AUDIT_ADD_RULE, AUDIT_DEL_RULE and

(quoted diff) +0 -0
@@ -362,6 +384,9 @@
Quoted 8 lines

libaudit.h 2006-06-13 13:01:30.000000000 -0400 line 389

Quoted 6 lines
libaudit.h 2006-06-13 13:01:30.000000000 -0400 renamed from libaudit.h 2006-05-25 17:38:21.000000000 -0400 +3 -0
 /* AUDIT_GET */
 extern int audit_request_status(int fd);
 extern int audit_is_enabled(int fd);
+extern auditfail_t audit_failure_action(char *file);
+static int search_audituser_conf(char *file, char *keyword, struct nv_pair *nv);
+static int nv_split(char *buf, struct nv_pair *nv);

These shouldn't be in the public .h file.

I'll move these declarations.

Thanks for the comments.

Lisa