[redhat-lspp] Re: [RFC: PATCH] Audit Failure Query Functionality
Lisa Smith
lisa.m.smith at hp.com
Wed Jun 14 18:05:29 UTC 2006
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
More information about the redhat-lspp
mailing list