Re: [RFC: PATCH] Audit Failure Query Functionality
James,
> + /* 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.
> + 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.
> +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.
> + /* 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.
Actually, this code will handle "x=y", "x =y", "x= y" and
"x = y".
> +/* 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.
> +
> +/* #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;
> +};
> +
This should be namespaced.
Will do.
> /*
> * 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.
I'll move these declarations.
Thanks for the comments.
Lisa