Updated audit failure query functionality
PATCH 1/1 Updated audit failure query functionality
This is an updated patch for the audit failure query functionality. The patch has been completely rewritten to match the programming style and algorithms used in auditd-config.c.
This patch introduces a new query function, get_auditfail_action(enum auditfail_t *failmode), which can be called to determine what action should be taken by a service after audit_open() fails. The function will read the tunable "failure_action" from the new /etc/libaudit.conf file to determine what action should be taken.
This patch provides easy expansion of the /etc/libaudit.conf file to introduce new libaudit tunables if the need arises in the future.
The audit_open() man page will be updated to reference this query function, and a new man page will also be created for get_auditfail_action().
Lisa
-------------------
libaudit.c | 246 +++++++++++++++++++++++++++++++++++++++++++++++++++++ libaudit.h | 11 ++ 2 files changed, 257 insertions(+)
libaudit.c 2006-06-26 15:00:56.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +246 -0
@@ -42,13 +42,56 @@ #include "libaudit.h" #include "private.h" +/* Local prototypes */ +struct nv_pair +{ + const char *name; + const char *value; +}; + +struct kw_pair +{ + const char *name; + int (*parser)(const char *, int); +}; + +struct nv_list +{ + const char *name; + int option; +}; + +struct libaudit_conf +{ + auditfail_t failure_action; +}; int audit_archadded = 0; int audit_syscalladded = 0; unsigned int audit_elf = 0U; +static struct libaudit_conf config; +static int load_libaudit_config(); +static char *get_line(FILE *f, char *buf); +static int nv_split(char *buf, struct nv_pair *nv); +static const struct kw_pair *kw_lookup(const char *val); +static int audit_failure_parser(const char *val, int line); static int name_to_uid(const char *name, uid_t *uid); static int name_to_gid(const char *name, gid_t *gid); +static const struct kw_pair keywords[] = +{ + {"failure_action", audit_failure_parser }, + { NULL, NULL } +}; + +static const struct nv_list failure_actions[] = +{ + {"ignore", FA_IGNORE }, + {"log", FA_LOG }, + {"terminate", FA_TERMINATE }, + { NULL, 0 } +}; + int audit_request_status(int fd) {
@@ -68,6 +111,209 @@ return rc; } +/* + * Set everything to its default value +*/ +static void clear_config() +{ + config.failure_action = FA_IGNORE; +} + +static int load_libaudit_config() +{ + int fd, rc, lineno = 1; + struct stat st; + FILE *f; + char buf[128]; + + clear_config(); + + /* open the file */ + rc = open(CONFIG_FILE, O_NOFOLLOW|O_RDONLY); + if (rc < 0) { + if (errno != ENOENT) { + audit_msg(LOG_ERR, "Error opening config file (%s)", + strerror(errno)); + return 1; + } + audit_msg(LOG_WARNING, + "Config file %s doesn't exist, skipping", CONFIG_FILE); + return 0; + } + fd = rc; + + /* check the file's permissions: owned by root, not world writable, + * not symlink. + */ + audit_msg(LOG_DEBUG, "Config file %s opened for parsing", + CONFIG_FILE); + if (fstat(fd, &st) < 0) { + audit_msg(LOG_ERR, "Error fstat'ing config file (%s)", + strerror(errno)); + return 1; + } + if (st.st_uid != 0) { + audit_msg(LOG_ERR, "Error - %s isn't owned by root", + CONFIG_FILE); + return 1; + } + if ((st.st_mode & S_IWOTH) == S_IWOTH) { + audit_msg(LOG_ERR, "Error - %s is world writable", + CONFIG_FILE); + return 1; + } + if (!S_ISREG(st.st_mode)) { + audit_msg(LOG_ERR, "Error - %s is not a regular file", + CONFIG_FILE); + return 1; + } + + /* it's ok, read line by line */ + f = fdopen(fd, "r"); + if (f == NULL) { + audit_msg(LOG_ERR, "Error - fdopen failed (%s)", + strerror(errno)); + return 1; + } + + while (get_line(f, buf)) { + // convert line into name-value pair + const struct kw_pair *kw; + struct nv_pair nv; + rc = nv_split(buf, &nv); + switch (rc) { + case 0: // fine + break; + case 1: // not the right number of tokens. + audit_msg(LOG_ERR, + "Wrong number of arguments for line %d in %s", + lineno, CONFIG_FILE); + break; + case 2: // no '=' sign + audit_msg(LOG_ERR, + "Missing equal sign for line %d in %s", + lineno, CONFIG_FILE); + break; + default: // something else went wrong... + audit_msg(LOG_ERR, + "Unknown error for line %d in %s", + lineno, CONFIG_FILE); + break; + } + if (nv.name == NULL) { + lineno++; + continue; + } + + /* identify keyword or error */ + kw = kw_lookup(nv.name); + if (kw->name == NULL) { + audit_msg(LOG_ERR, + "Unknown keyword \"%s\" in line %d of %s", + nv.name, lineno, CONFIG_FILE); + fclose(f); + return 1; + } + + /* dispatch to keyword's local parser */ + rc = kw->parser(nv.value, lineno); + if (rc != 0) { + fclose(f); + return 1; // local parser puts message out + } + + lineno++; + } + + fclose(f); + return 0; +} + +int get_auditfail_action(auditfail_t *failmode) +{ + if (load_libaudit_config()) + *failmode = config.failure_action; + return 1; + + *failmode = config.failure_action; + return 0; +} + + +static char *get_line(FILE *f, char *buf) +{ + if (fgets_unlocked(buf, 128, f)) { + /* remove newline */ + char *ptr = strchr(buf, 0x0a); + if (ptr) + *ptr = 0; + return buf; + } + return NULL; +} + +static int nv_split(char *buf, struct nv_pair *nv) +{ + /* Get the name part */ + char *ptr; + + nv->name = NULL; + nv->value = NULL; + ptr = strtok(buf, " "); + if (ptr == NULL) + return 0; /* If there's nothing, go to next line */ + if (ptr[0] == '#') + return 0; /* If there's a comment, go to next line */ + nv->name = ptr; + + /* Check for a '=' */ + ptr = strtok(NULL, " "); + if (ptr == NULL) + return 1; + if (strcmp(ptr, "=") != 0) + return 2; + + /* get the value */ + ptr = strtok(NULL, " "); + if (ptr == NULL) + return 1; + nv->value = ptr; + + /* Make sure there's nothing else */ + ptr = strtok(NULL, " "); + if (ptr) + return 1; + + /* Everything is OK */ + return 0; +} + +static const struct kw_pair *kw_lookup(const char *val) +{ + int i = 0; + while (keywords[i].name != NULL) { + if (strcasecmp(keywords[i].name, val) == 0) + break; + i++; + } + return &keywords[i]; +} + +static int audit_failure_parser(const char *val, int line) +{ + int i; + + audit_msg(LOG_DEBUG, "audit_failure_parser called with: %s", val); + for (i=0; failure_actions[i].name != NULL; i++) { + if (strcasecmp(val, failure_actions[i].name) == 0) { + config.failure_action = failure_actions[i].option; + return 0; + } + } + audit_msg(LOG_ERR, "Option %s not found - line %d", val, line); + return 1; +} + int audit_set_enabled(int fd, uint32_t enabled) { int rc;
libaudit.h 2006-06-26 14:58:53.000000000 -0400 renamed from libaudit.h 2006-05-25 17:38:21.000000000 -0400 +11 -0
@@ -248,6 +248,16 @@ MACH_ALPHA } machine_t; +/* These are the valid audit failure tunable enum values */ +typedef enum { + FA_IGNORE=0, + FA_LOG, + FA_TERMINATE +} auditfail_t; + +/* #defines for the audit failure query */ +#define CONFIG_FILE "/etc/libaudit.conf" + /* * audit_rule_data supports filter rules with both integer and string * fields. It corresponds with AUDIT_ADD_RULE, AUDIT_DEL_RULE and
@@ -362,6 +372,7 @@ /* AUDIT_GET */ extern int audit_request_status(int fd); extern int audit_is_enabled(int fd); +extern int get_auditfail_action(auditfail_t *failmode); /* AUDIT_SET */ typedef enum { WAIT_NO, WAIT_YES } rep_wait_t;
Quoted 4 lines · Lisa Smith
This is an updated patch for the audit failure query functionality. The patch has been completely rewritten to match the programming style and algorithms used in
auditd-config.c.
<snip>
Quoted 2 lines
Lisa
Hi Lisa,
I had some comments below. Thanks!
-tim
Quoted 107 lines
-------------------
libaudit.c | 246 +++++++++++++++++++++++++++++++++++++++++++++++++++++ libaudit.h | 11 ++ 2 files changed, 257 insertions(+)diff -burN
orig/libaudit.csrc/libaudit.clibaudit.c 2006-06-26 15:00:56.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +79
-0@@ -42,13 +42,56 @@ #include "libaudit.h" #include "private.h" +/* Local prototypes */ +struct nv_pair +{ + const char *name; + const char *value; +}; + +struct kw_pair +{ + const char *name; + int (*parser)(const char *, int); +}; + +struct nv_list +{ + const char *name; + int option; +}; + +struct libaudit_conf +{ + auditfail_t failure_action; +}; int audit_archadded = 0; int audit_syscalladded = 0; unsigned int audit_elf = 0U; +static struct libaudit_conf config; +static int load_libaudit_config(); +static char *get_line(FILE *f, char *buf); +static int nv_split(char *buf, struct nv_pair *nv); +static const struct kw_pair *kw_lookup(const char *val); +static int audit_failure_parser(const char *val, int line); static int name_to_uid(const char *name, uid_t *uid); static int name_to_gid(const char *name, gid_t *gid); +static const struct kw_pair keywords[] = +{ + {"failure_action", audit_failure_parser }, + { NULL, NULL } +}; + +static const struct nv_list failure_actions[] = +{ + {"ignore", FA_IGNORE }, + {"log", FA_LOG }, + {"terminate", FA_TERMINATE }, + { NULL, 0 } +}; + int audit_request_status(int fd) {@@ -68,6 +111,209 @@ return rc; } +/* + * Set everything to its default value +*/ +static void clear_config() +{ + config.failure_action = FA_IGNORE; +} + +static int load_libaudit_config() +{ + int fd, rc, lineno = 1; + struct stat st; + FILE *f; + char buf[128]; + + clear_config(); + + /* open the file */ + rc = open(CONFIG_FILE, O_NOFOLLOW|O_RDONLY); + if (rc < 0) { + if (errno != ENOENT) { + audit_msg(LOG_ERR, "Error opening config file (%s)", + strerror(errno)); + return 1; + } + audit_msg(LOG_WARNING, + "Config file %s doesn't exist, skipping", CONFIG_FILE); + return 0; + } + fd = rc; + + /* check the file's permissions: owned by root, not world writable, + * not symlink. + */ + audit_msg(LOG_DEBUG, "Config file %s opened for parsing", + CONFIG_FILE);
libaudit.c 2006-06-26 15:00:56.000000000 -0400 line 159
Quoted 10 lines
libaudit.c 2006-06-26 15:00:56.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +10
-0+ if (fstat(fd, &st) < 0) { + audit_msg(LOG_ERR, "Error fstat'ing config file (%s)", + strerror(errno)); + return 1; + } + if (st.st_uid != 0) { + audit_msg(LOG_ERR, "Error - %s isn't owned by root", + CONFIG_FILE); + return 1; + }
Hm, you really want to hard-code in a root check? What if you're acting as another type of administrator with access to audit? Done know, I could be out of line here.
[..]
Quoted 67 lines
libaudit.c 2006-06-26 15:00:56.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +67
-0+ if ((st.st_mode & S_IWOTH) == S_IWOTH) { + audit_msg(LOG_ERR, "Error - %s is world writable", + CONFIG_FILE); + return 1; + } + if (!S_ISREG(st.st_mode)) { + audit_msg(LOG_ERR, "Error - %s is not a regular file", + CONFIG_FILE); + return 1; + } + + /* it's ok, read line by line */ + f = fdopen(fd, "r"); + if (f == NULL) { + audit_msg(LOG_ERR, "Error - fdopen failed (%s)", + strerror(errno)); + return 1; + } + + while (get_line(f, buf)) { + // convert line into name-value pair + const struct kw_pair *kw; + struct nv_pair nv; + rc = nv_split(buf, &nv); + switch (rc) { + case 0: // fine + break; + case 1: // not the right number of tokens. + audit_msg(LOG_ERR, + "Wrong number of arguments for line %d in %s", + lineno, CONFIG_FILE); + break; + case 2: // no '=' sign + audit_msg(LOG_ERR, + "Missing equal sign for line %d in %s", + lineno, CONFIG_FILE); + break; + default: // something else went wrong... + audit_msg(LOG_ERR, + "Unknown error for line %d in %s", + lineno, CONFIG_FILE); + break; + } + if (nv.name == NULL) { + lineno++; + continue; + } + + /* identify keyword or error */ + kw = kw_lookup(nv.name); + if (kw->name == NULL) { + audit_msg(LOG_ERR, + "Unknown keyword \"%s\" in line %d of %s", + nv.name, lineno, CONFIG_FILE); + fclose(f); + return 1; + } + + /* dispatch to keyword's local parser */ + rc = kw->parser(nv.value, lineno); + if (rc != 0) { + fclose(f); + return 1; // local parser puts message out + } + + lineno++; + }
libaudit.c 2006-06-26 15:00:56.000000000 -0400 line 236
Quoted 10 lines
libaudit.c 2006-06-26 15:00:56.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +10
-0+ + fclose(f); + return 0; +} + +int get_auditfail_action(auditfail_t *failmode) +{ + if (load_libaudit_config()) + *failmode = config.failure_action; + return 1;
Missing brackets?
[..]
Quoted 20 lines
libaudit.c 2006-06-26 15:00:56.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +20
-0+ + *failmode = config.failure_action; + return 0; +} + + +static char *get_line(FILE *f, char *buf) +{ + if (fgets_unlocked(buf, 128, f)) { + /* remove newline */ + char *ptr = strchr(buf, 0x0a); + if (ptr) + *ptr = 0; + return buf; + } + return NULL; +} + +static int nv_split(char *buf, struct nv_pair *nv) +{
libaudit.c 2006-06-26 15:00:56.000000000 -0400 line 266
Quoted 10 lines
libaudit.c 2006-06-26 15:00:56.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +10
-0+ /* Get the name part */ + char *ptr; + + nv->name = NULL; + nv->value = NULL; + ptr = strtok(buf, " "); + if (ptr == NULL) + return 0; /* If there's nothing, go to next line */ + if (ptr[0] == '#') + return 0; /* If there's a comment, go to next line */
I still think this will break if ptr[0] happens to be a space followed by a '#'. I suppose it could break by design if the first character of the line is not '#', but I wouldn't like that much as a user myself.
[..]
libaudit.c 2006-06-26 15:00:56.000000000 -0400 line 273
Quoted 7 lines
libaudit.c 2006-06-26 15:00:56.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +7
-0+ nv->name = ptr; + + /* Check for a '=' */ + ptr = strtok(NULL, " "); + if (ptr == NULL) + return 1; + if (strcmp(ptr, "=") != 0)
No real need to do a strcmp here.
if (ptr && *ptr == '=')
[..]
libaudit.c 2006-06-26 15:00:56.000000000 -0400 line 280
Quoted 7 lines
libaudit.c 2006-06-26 15:00:56.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +7
-0+ return 2; + + /* get the value */ + ptr = strtok(NULL, " "); + if (ptr == NULL) + return 1; + nv->value = ptr;
I'll be the first to admit that I relatively little experience with strtok, but is this pointer assignment even safe? You're pointing into a buffer that could potentially be overwritten by another app, right?
Here's my off-the-top-of-the-head, uncompiled, and untested example using strtok_r...
char *buf;
char *ptr;
buf = (char *)malloc(128);
if (!buf) {
/* handle memory error */
}
ptr = strtok_r(line, "=", buf);
if (!ptr || *buf == '\0') {
/* not "token=value", free memory */
}
/* <do comment check here> */
nv->name = (char*)malloc(strlen(ptr)+1);
if (!nv->name) {
/* handle memory error */
}
strcpy(nv->name, ptr);
nv->value = (char*)malloc(strlen(buf)+1);
if (!nv->value) {
/* handle memory error */
}strcpy(nv->value, buf);
/* clean up memory */
return 0;
You may not care about whether or not you're thread-safe or being able to have multiple word token values... but at least by providing your own buffer, you can keep track of your position in the parsed string in a lovely way.
[..]
Quoted 9 lines
libaudit.c 2006-06-26 15:00:56.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +9
-0+ + /* Make sure there's nothing else */ + ptr = strtok(NULL, " "); + if (ptr) + return 1; + + /* Everything is OK */ + return 0; +}
libaudit.c 2006-06-26 15:00:56.000000000 -0400 line 299
Quoted 10 lines
libaudit.c 2006-06-26 15:00:56.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +10
-0+ +static const struct kw_pair *kw_lookup(const char *val) +{ + int i = 0; + while (keywords[i].name != NULL) { + if (strcasecmp(keywords[i].name, val) == 0) + break; + i++; + } + return &keywords[i];
So I think this would look better a for-loop as you're in fact iterating anyway :) *oh my, looks down at next function*
[..]
libaudit.c 2006-06-26 15:00:56.000000000 -0400 line 312
Quoted 13 lines
libaudit.c 2006-06-26 15:00:56.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +13
-0+} + +static int audit_failure_parser(const char *val, int line) +{ + int i; + + audit_msg(LOG_DEBUG, "audit_failure_parser called with: %s", val); + for (i=0; failure_actions[i].name != NULL; i++) { + if (strcasecmp(val, failure_actions[i].name) == 0) { + config.failure_action = failure_actions[i].option; + return 0; + } + }
:)
[..]
Quoted 35 lines
libaudit.c 2006-06-26 15:00:56.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +4
-0+ audit_msg(LOG_ERR, "Option %s not found - line %d", val, line); + return 1; +} + int audit_set_enabled(int fd, uint32_t enabled) { int rc;diff -burN
orig/libaudit.hsrc/libaudit.hlibaudit.h 2006-06-26 14:58:53.000000000 -0400 renamed from libaudit.h 2006-05-25 17:38:21.000000000 -0400 +11
-0@@ -248,6 +248,16 @@ MACH_ALPHA } machine_t; +/* These are the valid audit failure tunable enum values */ +typedef enum { + FA_IGNORE=0, + FA_LOG, + FA_TERMINATE +} auditfail_t; + +/* #defines for the audit failure query */ +#define CONFIG_FILE "/etc/libaudit.conf" + /* * audit_rule_data supports filter rules with both integer and string * fields. It corresponds with AUDIT_ADD_RULE, AUDIT_DEL_RULE and@@ -362,6 +372,7 @@ /* AUDIT_GET */ extern int audit_request_status(int fd); extern int audit_is_enabled(int fd); +extern int get_auditfail_action(auditfail_t *failmode); /* AUDIT_SET */ typedef enum { WAIT_NO, WAIT_YES } rep_wait_t;
Tim,
Thanks for your comments.
Other than the missing brackets in get_auditfail_action (good catch, thanks), all the other functions were pulled directly from auditd-config.c. It was requested that the new audit failure query code use the functions from auditd for ease of code maintainability. As you pointed out, the parsing only works with a very specific format, which is why I rewrote it in the original patch. This is the parsing mechanism used for auditd.conf.
Steve and others, I assume you'd like to leave the functions as they are, so they match what is in auditd-config.c. If not, please comment on how to handle each of the concerns Tim points out below.
Thanks,
Lisa
Quoted 412 lines · Timothy R. Chavez
Quoted 5 lines · Lisa Smith
This is an updated patch for the audit failure query functionality. The patch has been completely rewritten to match the programming style and algorithms used in
auditd-config.c.<snip>
Quoted 2 lines
Lisa
Hi Lisa,
I had some comments below. Thanks!
-tim
Quoted 107 lines
-------------------
libaudit.c | 246 +++++++++++++++++++++++++++++++++++++++++++++++++++++ libaudit.h | 11 ++ 2 files changed, 257 insertions(+)diff -burN
orig/libaudit.csrc/libaudit.clibaudit.c 2006-06-26 15:00:56.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +80
-0@@ -42,13 +42,56 @@ #include "libaudit.h" #include "private.h" +/* Local prototypes */ +struct nv_pair +{ + const char *name; + const char *value; +}; + +struct kw_pair +{ + const char *name; + int (*parser)(const char *, int); +}; + +struct nv_list +{ + const char *name; + int option; +}; + +struct libaudit_conf +{ + auditfail_t failure_action; +}; int audit_archadded = 0; int audit_syscalladded = 0; unsigned int audit_elf = 0U; +static struct libaudit_conf config; +static int load_libaudit_config(); +static char *get_line(FILE *f, char *buf); +static int nv_split(char *buf, struct nv_pair *nv); +static const struct kw_pair *kw_lookup(const char *val); +static int audit_failure_parser(const char *val, int line); static int name_to_uid(const char *name, uid_t *uid); static int name_to_gid(const char *name, gid_t *gid); +static const struct kw_pair keywords[] = +{ + {"failure_action", audit_failure_parser }, + { NULL, NULL } +}; + +static const struct nv_list failure_actions[] = +{ + {"ignore", FA_IGNORE }, + {"log", FA_LOG }, + {"terminate", FA_TERMINATE }, + { NULL, 0 } +}; + int audit_request_status(int fd) {@@ -68,6 +111,209 @@ return rc; } +/* + * Set everything to its default value +*/ +static void clear_config() +{ + config.failure_action = FA_IGNORE; +} + +static int load_libaudit_config() +{ + int fd, rc, lineno = 1; + struct stat st; + FILE *f; + char buf[128]; + + clear_config(); + + /* open the file */ + rc = open(CONFIG_FILE, O_NOFOLLOW|O_RDONLY); + if (rc < 0) { + if (errno != ENOENT) { + audit_msg(LOG_ERR, "Error opening config file (%s)", + strerror(errno)); + return 1; + } + audit_msg(LOG_WARNING, + "Config file %s doesn't exist, skipping", CONFIG_FILE); + return 0; + } + fd = rc; + + /* check the file's permissions: owned by root, not world writable, + * not symlink. + */ + audit_msg(LOG_DEBUG, "Config file %s opened for parsing", + CONFIG_FILE); + if (fstat(fd, &st) < 0) {libaudit.c 2006-06-26 15:00:56.000000000 -0400 line 160
Quoted 10 lines
libaudit.c 2006-06-26 15:00:56.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +9
-0+ audit_msg(LOG_ERR, "Error fstat'ing config file (%s)", + strerror(errno)); + return 1; + } + if (st.st_uid != 0) { + audit_msg(LOG_ERR, "Error - %s isn't owned by root", + CONFIG_FILE); + return 1; + }Hm, you really want to hard-code in a root check? What if you're acting as another type of administrator with access to audit? Done know, I could be out of line here.
[..]
Quoted 68 lines
libaudit.c 2006-06-26 15:00:56.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +68
-0+ if ((st.st_mode & S_IWOTH) == S_IWOTH) { + audit_msg(LOG_ERR, "Error - %s is world writable", + CONFIG_FILE); + return 1; + } + if (!S_ISREG(st.st_mode)) { + audit_msg(LOG_ERR, "Error - %s is not a regular file", + CONFIG_FILE); + return 1; + } + + /* it's ok, read line by line */ + f = fdopen(fd, "r"); + if (f == NULL) { + audit_msg(LOG_ERR, "Error - fdopen failed (%s)", + strerror(errno)); + return 1; + } + + while (get_line(f, buf)) { + // convert line into name-value pair + const struct kw_pair *kw; + struct nv_pair nv; + rc = nv_split(buf, &nv); + switch (rc) { + case 0: // fine + break; + case 1: // not the right number of tokens. + audit_msg(LOG_ERR, + "Wrong number of arguments for line %d in %s", + lineno, CONFIG_FILE); + break; + case 2: // no '=' sign + audit_msg(LOG_ERR, + "Missing equal sign for line %d in %s", + lineno, CONFIG_FILE); + break; + default: // something else went wrong... + audit_msg(LOG_ERR, + "Unknown error for line %d in %s", + lineno, CONFIG_FILE); + break; + } + if (nv.name == NULL) { + lineno++; + continue; + } + + /* identify keyword or error */ + kw = kw_lookup(nv.name); + if (kw->name == NULL) { + audit_msg(LOG_ERR, + "Unknown keyword \"%s\" in line %d of %s", + nv.name, lineno, CONFIG_FILE); + fclose(f); + return 1; + } + + /* dispatch to keyword's local parser */ + rc = kw->parser(nv.value, lineno); + if (rc != 0) { + fclose(f); + return 1; // local parser puts message out + } + + lineno++; + } +libaudit.c 2006-06-26 15:00:56.000000000 -0400
Quoted 10 lines
libaudit.c 2006-06-26 15:00:56.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +9
-0+ fclose(f); + return 0; +} + +int get_auditfail_action(auditfail_t *failmode) +{ + if (load_libaudit_config()) + *failmode = config.failure_action; + return 1;Missing brackets?
[..]
Quoted 21 lines
libaudit.c 2006-06-26 15:00:56.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +21
-0+ + *failmode = config.failure_action; + return 0; +} + + +static char *get_line(FILE *f, char *buf) +{ + if (fgets_unlocked(buf, 128, f)) { + /* remove newline */ + char *ptr = strchr(buf, 0x0a); + if (ptr) + *ptr = 0; + return buf; + } + return NULL; +} + +static int nv_split(char *buf, struct nv_pair *nv) +{ + /* Get the name part */libaudit.c 2006-06-26 15:00:56.000000000 -0400
Quoted 10 lines
libaudit.c 2006-06-26 15:00:56.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +9
-0+ char *ptr; + + nv->name = NULL; + nv->value = NULL; + ptr = strtok(buf, " "); + if (ptr == NULL) + return 0; /* If there's nothing, go to next line */ + if (ptr[0] == '#') + return 0; /* If there's a comment, go to next line */I still think this will break if ptr[0] happens to be a space followed by a '#'. I suppose it could break by design if the first character of the line is not '#', but I wouldn't like that much as a user myself.
[..]
libaudit.c 2006-06-26 15:00:56.000000000 -0400 line 273
Quoted 8 lines
libaudit.c 2006-06-26 15:00:56.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +7
-0+ nv->name = ptr; + + /* Check for a '=' */ + ptr = strtok(NULL, " "); + if (ptr == NULL) + return 1; + if (strcmp(ptr, "=") != 0)No real need to do a strcmp here.
if (ptr && *ptr == '=')
[..]
libaudit.c 2006-06-26 15:00:56.000000000 -0400 line 280
Quoted 8 lines
libaudit.c 2006-06-26 15:00:56.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +7
-0+ return 2; + + /* get the value */ + ptr = strtok(NULL, " "); + if (ptr == NULL) + return 1; + nv->value = ptr;I'll be the first to admit that I relatively little experience with strtok, but is this pointer assignment even safe? You're pointing into a buffer that could potentially be overwritten by another app, right?
Here's my off-the-top-of-the-head, uncompiled, and untested example using
strtok_r...char *buf;
char *ptr;buf = (char *)malloc(128);
if (!buf) { /* handle memory error */ } ptr = strtok_r(line, "=", buf); if (!ptr || *buf == '\0') { /* not "token=value", free memory */ } /* <do comment check here> */ nv->name = (char*)malloc(strlen(ptr)+1); if (!nv->name) { /* handle memory error */ } strcpy(nv->name, ptr); nv->value = (char*)malloc(strlen(buf)+1); if (!nv->value) { /* handle memory error */ }
strcpy(nv->value, buf);/* clean up memory */
return 0;You may not care about whether or not you're thread-safe or being able to have multiple word token values... but at least by providing your own buffer, you can keep track of your position in the parsed string in a lovely way.
[..]
Quoted 10 lines
libaudit.c 2006-06-26 15:00:56.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +9
-0+ + /* Make sure there's nothing else */ + ptr = strtok(NULL, " "); + if (ptr) + return 1; + + /* Everything is OK */ + return 0; +}libaudit.c 2006-06-26 15:00:56.000000000 -0400 line 299
Quoted 10 lines
libaudit.c 2006-06-26 15:00:56.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +10
-0+ +static const struct kw_pair *kw_lookup(const char *val) +{ + int i = 0; + while (keywords[i].name != NULL) { + if (strcasecmp(keywords[i].name, val) == 0) + break; + i++; + } + return &keywords[i];So I think this would look better a for-loop as you're in fact iterating anyway :) *oh my, looks down at next function*
[..]
Quoted 4 lines
libaudit.c 2006-06-26 15:00:56.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +3
-0+} + +static int audit_failure_parser(const char *val, int line)libaudit.c 2006-06-26 15:00:56.000000000 -0400 line 312
Quoted 10 lines
libaudit.c 2006-06-26 15:00:56.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +10
-0+{ + int i; + + audit_msg(LOG_DEBUG, "audit_failure_parser called with: %s", val); + for (i=0; failure_actions[i].name != NULL; i++) { + if (strcasecmp(val, failure_actions[i].name) == 0) { + config.failure_action = failure_actions[i].option; + return 0; + } + }:)
[..]
Quoted 36 lines
libaudit.c 2006-06-26 15:00:56.000000000 -0400 renamed from libaudit.c 2006-05-25 17:39:52.000000000 -0400 +4
-0+ audit_msg(LOG_ERR, "Option %s not found - line %d", val, line); + return 1; +} +int
audit_set_enabled(int fd, uint32_t enabled){ int rc;diff -burN
orig/libaudit.hsrc/libaudit.hlibaudit.h 2006-06-26 14:58:53.000000000 -0400 renamed from libaudit.h 2006-05-25 17:38:21.000000000 -0400 +11
-0@@ -248,6 +248,16 @@ MACH_ALPHA } machine_t; +/* These are the valid audit failure tunable enum values */ +typedef enum { + FA_IGNORE=0, + FA_LOG, + FA_TERMINATE +} auditfail_t; + +/* #defines for the audit failure query */ +#define CONFIG_FILE "/etc/libaudit.conf" + /* * audit_rule_data supports filter rules with both integer and string * fields. It corresponds with AUDIT_ADD_RULE, AUDIT_DEL_RULE and@@ -362,6 +372,7 @@ /* AUDIT_GET */ extern int audit_request_status(int fd); extern int audit_is_enabled(int fd); +extern int get_auditfail_action(auditfail_t *failmode); /* AUDIT_SET */ typedef enum { WAIT_NO, WAIT_YES } rep_wait_t;
Quoted 3 lines · Lisa Smith
Steve and others, I assume you'd like to leave the functions as they are, so they match what is in
auditd-config.c. If not, please comment on how to handle each of the concerns Tim points out below.
Lisa,
Thanks for working on this. I will integrate it in the next release and take over maintenance of it.
Thanks again...
-Steve