get_field_str() and interpret_field() bug with multi-word fields
by Jonathan Kelly
Hi again,
For what it's worth, I dug through the code a bit, and am pretty sure that this particular issue exists in lines 71-78 of ellist.c:
ptr = strtok_r(buf, " ", &saved);
if (ptr == NULL)
return -1;
do { // If there's an '=' sign, its a keeper
nvnode n;
char *val = strchr(ptr, '=');
if (val) {
Basically, it's splitting the string at " " and discarding anything that doesn't contain '=', which is what is resulting in anything after the initial space in a field being discarded. Splitting at '\s\w+=' (pardon my regexp) instead would allow for the desired results, unless I'm mistaken, but would require some significant recoding of that function (beyond my capacity as a C programmer without much fail and gnashing of teeth). I hope this is helpful!
Best regards,
Jonathan Kelly
16 years, 1 month
[PATCH] Uids should not be allowed to set to negative
by Cai Xianchao
Hello Steve,
When I tried to set uid to negative, no error message was outputed and
the return value was 0. In the rule list, the value of uid was also
negative. Negative uid does not exist and the negative user can't be
added. So, I think uids can't be set to negative.
It is also strange that gid can't be set to negative, while uid can.
I did as follows:
#auditctl -a exit,always -F uid=-1
#auditctl -l
LIST_RULES: exit,always uid=-1 (0xffffffff) syscall=all
Signed-off-by: Cai Xianchao <caixianchao(a)cn.fujistu.com>
---
diff --git a/deprecated.c b/deprecated.c
index 6bf42dd..94954a5 100644
--- a/deprecated.c
+++ b/deprecated.c
@@ -257,10 +257,6 @@ int audit_rule_fieldpair(struct audit_rule *rule, const char *pair, int flags)
if (isdigit((char)*(v)))
rule->values[rule->field_count] =
strtol(v, NULL, 0);
- else if (vlen >= 2 && *(v)=='-' &&
- (isdigit((char)*(v+1))))
- rule->values[rule->field_count] =
- strtol(v, NULL, 0);
else {
if (name_to_uid(v,
&rule->values[rule->field_count])) {
diff --git a/libaudit.c b/libaudit.c
index 7d48d78..85adbc5 100644
--- a/libaudit.c
+++ b/libaudit.c
@@ -850,10 +850,6 @@ int audit_rule_fieldpair_data(struct audit_rule_data **rulep, const char *pair,
if (isdigit((char)*(v)))
rule->values[rule->field_count] =
strtol(v, NULL, 0);
- else if (vlen >= 2 && *(v)=='-' &&
- (isdigit((char)*(v+1))))
- rule->values[rule->field_count] =
- strtol(v, NULL, 0);
else {
if (audit_name_to_uid(v,
&rule->values[rule->field_count])) {
16 years, 1 month
[PATCH] Error message is not appropriate when try to set gid to negative
by Cai Xianchao
Hello Steve,
The error message is not appropriate when I try to set gid to
negative. the value of "Unknown group:" should be the gid rather
than "gid". There is the same problem in uids.
I did as fallows:
#auditctl -a exit,always -F gid=-1
Unknown group: gid
-F unknown field: gid=-1
Signed-off-by: Cai Xianchao <caixianchao(a)cn.fujistu.com>
---
diff --git a/deprecated.c b/deprecated.c
index e05e826..2d32ad3 100644
--- a/deprecated.c
+++ b/deprecated.c
@@ -259,7 +259,7 @@ int audit_rule_fieldpair(struct audit_rule *rule,
const char *pair, int flags)
if (name_to_uid(v,
&rule->values[rule->field_count])) {
audit_msg(LOG_ERR, "Unknown
user: %s",
- pair);
+ v);
return -2;
}
}
@@ -275,7 +275,7 @@ int audit_rule_fieldpair(struct audit_rule *rule,
const char *pair, int flags)
if (name_to_gid(v,
&rule->values[rule->field_count])) {
audit_msg(LOG_ERR, "Unknown
group: %s",
- pair);
+ v);
return -2;
}
}
diff --git a/libaudit.c b/libaudit.c
index 4bedfaf..37e96e5 100644
--- a/libaudit.c
+++ b/libaudit.c
@@ -851,7 +851,7 @@ int audit_rule_fieldpair_data(struct audit_rule_data
**rulep, const char *pair,
if (audit_name_to_uid(v,
&rule->values[rule->field_count])) {
audit_msg(LOG_ERR, "Unknown
user: %s",
- pair);
+ v);
return -2;
}
}
@@ -867,7 +867,7 @@ int audit_rule_fieldpair_data(struct audit_rule_data
**rulep, const char *pair,
if (audit_name_to_gid(v,
&rule->values[rule->field_count])) {
audit_msg(LOG_ERR, "Unknown
group: %s",
- pair);
+ v);
return -2;
}
}
16 years, 2 months
[PATCH] Re-implement auditd main loop using libev
by DJ Delorie
Hi! This is my first post here, so be gentle... :-)
This is the first in a (hopefully short) series of patches to add the
server side of the remote logging that was started in the 1.7 series.
The idea here is to use the libev event library to manage events
coming in to auditd, be they from netlink, signals, or some network
connection (a future patch); rather than the custom event loop that
only supported netlink. This also simplifies signal handling, as
signals are recognized synchronously in the main context instead of in
a signal context.
Comments?
DJ
Proposed ChangeLog entry:
- Reimplement auditd main loop using libev (DJ Delorie)
diff -x .svn -urpN pristine/src/auditd.c trunk/src/auditd.c
--- pristine/src/auditd.c 2008-08-07 15:54:51.000000000 -0400
+++ trunk/src/auditd.c 2008-08-07 19:27:35.000000000 -0400
@@ -45,6 +45,10 @@
#include "auditd-dispatch.h"
#include "private.h"
+#include "ev.h"
+
+#define EV_STOP() ev_unloop (ev_default_loop (EVFLAG_AUTO), EVUNLOOP_ALL), stop = 1;
+
#define DEFAULT_BUF_SZ 448
#define DMSG_SIZE (DEFAULT_BUF_SZ + 48)
#define SUCCESS 0
@@ -52,9 +56,6 @@
/* Global Data */
volatile int stop = 0;
-volatile int hup = 0;
-volatile int rot = 0;
-volatile int resume = 0;
/* Local data */
static int fd = -1;
@@ -62,6 +63,8 @@ static struct daemon_conf config;
static const char *pidfile = "/var/run/auditd.pid";
static int init_pipe[2];
static int do_fork = 1;
+static struct auditd_reply_list *rep = NULL;
+static int hup_info_requested = 0, usr1_info_requested = 0;
/* Local function prototypes */
static void close_down(void);
@@ -88,9 +91,9 @@ static void usage(void)
/*
* SIGTERM handler
*/
-static void term_handler( int sig )
+static void term_handler( struct ev_loop *loop, struct ev_signal *sig, int revents )
{
- stop = 1;
+ EV_STOP ();
}
/*
@@ -104,31 +107,47 @@ static void thread_killer( int sig )
/*
* Used with sigalrm to force exit
*/
-static void hup_handler( int sig )
+static void hup_handler( struct ev_loop *loop, struct ev_signal *sig, int revents )
{
- hup = 1;
+ int rc;
+
+ rc = audit_request_signal_info(fd);
+ if (rc < 0)
+ send_audit_event(AUDIT_DAEMON_CONFIG,
+ "auditd error getting hup info - no change, sending auid=? pid=? subj=? res=failed");
+ else
+ hup_info_requested = 1;
}
/*
* Used to force log rotation
*/
-static void user1_handler( int sig )
+static void user1_handler( struct ev_loop *loop, struct ev_signal *sig, int revents )
{
- rot = 1;
+ int rc;
+
+ rc = audit_request_signal_info(fd);
+ if (rc < 0)
+ send_audit_event(AUDIT_DAEMON_ROTATE,
+ "auditd error getting usr1 info - no change, sending auid=? pid=? subj=? res=failed");
+ else
+ usr1_info_requested = 1;
}
/*
* Used to resume logging
*/
-static void user2_handler( int sig )
+static void user2_handler( struct ev_loop *loop, struct ev_signal *sig, int revents )
{
- resume = 1;
+ resume_logging();
+ send_audit_event(AUDIT_DAEMON_RESUME,
+ "auditd resuming logging, sending auid=? pid=? subj=? res=success");
}
/*
* Used with email alerts to cleanup
*/
-static void child_handler( int sig )
+static void child_handler( struct ev_loop *loop, struct ev_signal *sig, int revents )
{
while (waitpid(-1, NULL, WNOHANG) > 0)
; /* empty */
@@ -323,19 +342,100 @@ static void tell_parent(int status)
} while (rc < 0 && errno == EINTR);
}
+static void netlink_handler( struct ev_loop *loop, struct ev_io *io, int revents )
+{
+ if (rep == NULL) {
+ if ((rep = malloc(sizeof(*rep))) == NULL) {
+ char emsg[DEFAULT_BUF_SZ];
+ snprintf(emsg, sizeof(emsg),
+ "auditd error halt, auid=%u pid=%d res=failed",
+ audit_getloginuid(), getpid());
+ EV_STOP ();
+ //FIXME add subj
+ send_audit_event(AUDIT_DAEMON_ABORT, emsg);
+ audit_msg(LOG_ERR,
+ "Cannot allocate audit reply, exiting");
+ close_down();
+ if (pidfile)
+ unlink(pidfile);
+ shutdown_dispatcher();
+ return;
+ }
+ }
+ if (audit_get_reply(fd, &rep->reply,
+ GET_REPLY_NONBLOCKING, 0) > 0) {
+ switch (rep->reply.type)
+ { /* For now dont process these */
+ case NLMSG_NOOP:
+ case NLMSG_DONE:
+ case NLMSG_ERROR:
+ case AUDIT_GET: /* Or these */
+ case AUDIT_LIST:
+ case AUDIT_LIST_RULES:
+ case AUDIT_FIRST_DAEMON...AUDIT_LAST_DAEMON:
+ break;
+ case AUDIT_SIGNAL_INFO:
+ if (hup_info_requested) {
+ audit_msg(LOG_DEBUG,
+ "HUP detected, starting config manager");
+ if (start_config_manager(rep)) {
+ send_audit_event(
+ AUDIT_DAEMON_CONFIG,
+ "auditd error getting hup info - no change,"
+ " sending auid=? pid=? subj=? res=failed");
+ }
+ rep = NULL;
+ hup_info_requested = 0;
+ } else if(usr1_info_requested){
+ char usr1[MAX_AUDIT_MESSAGE_LENGTH];
+ if (rep->reply.len == 24) {
+ snprintf(usr1,
+ sizeof(usr1),
+ "auditd sending auid=? pid=? subj=?");
+ } else {
+ snprintf(usr1,
+ sizeof(usr1),
+ "auditd sending auid=%u pid=%d subj=%s",
+ rep->reply.signal_info->uid,
+ rep->reply.signal_info->pid,
+ rep->reply.signal_info->ctx);
+ }
+ send_audit_event(
+ AUDIT_DAEMON_ROTATE,
+ usr1);
+ }
+ break;
+ default:
+ distribute_event(rep);
+ rep = NULL;
+ break;
+ }
+ } else {
+ if (errno == EFBIG) {
+ // FIXME do err action
+ }
+ }
+}
+
int main(int argc, char *argv[])
{
struct sigaction sa;
fd_set read_mask;
- struct auditd_reply_list *rep = NULL;
struct rlimit limit;
- int hup_info_requested = 0, usr1_info_requested = 0;
int i;
int opt_foreground = 0, opt_allow_links = 0;
enum startup_state opt_startup = startup_enable;
int c;
extern char *optarg;
extern int optind;
+ struct ev_loop *loop;
+ struct ev_io netlink_watcher;
+ struct ev_signal sigterm_watcher;
+ struct ev_signal sighup_watcher;
+ struct ev_signal sigusr1_watcher;
+ struct ev_signal sigusr2_watcher;
+ struct ev_signal sigchld_watcher;
+ int rc;
/* Get params && set mode */
while ((c = getopt(argc, argv, "flns:")) != -1) {
@@ -400,17 +500,6 @@ int main(int argc, char *argv[])
sa.sa_handler = SIG_IGN;
for (i=1; i<NSIG; i++)
sigaction( i, &sa, NULL );
- /* Set handler for the ones we care about */
- sa.sa_handler = term_handler;
- sigaction( SIGTERM, &sa, NULL );
- sa.sa_handler = hup_handler;
- sigaction( SIGHUP, &sa, NULL );
- sa.sa_handler = user1_handler;
- sigaction( SIGUSR1, &sa, NULL );
- sa.sa_handler = user2_handler;
- sigaction( SIGUSR2, &sa, NULL );
- sa.sa_handler = child_handler;
- sigaction( SIGCHLD, &sa, NULL );
atexit(clean_exit);
@@ -561,147 +650,50 @@ int main(int argc, char *argv[])
if (do_fork)
close(init_pipe[1]);
- for (;;) {
- struct timeval tv;
- int retval;
-
- if (rep == NULL) {
- if ((rep = malloc(sizeof(*rep))) == NULL) {
- char emsg[DEFAULT_BUF_SZ];
- snprintf(emsg, sizeof(emsg),
- "auditd error halt, auid=%u pid=%d res=failed",
- audit_getloginuid(), getpid());
- stop = 1;
-//FIXME add subj
- send_audit_event(AUDIT_DAEMON_ABORT, emsg);
- audit_msg(LOG_ERR,
- "Cannot allocate audit reply, exiting");
- close_down();
- if (pidfile)
- unlink(pidfile);
- shutdown_dispatcher();
- return 1;
- }
- }
-
- /* This will pause for 30 seconds */
- do {
- tv.tv_sec = 30;
- tv.tv_usec = 0;
- FD_ZERO(&read_mask);
- FD_SET(fd, &read_mask);
- retval = select(fd+1, &read_mask, NULL, NULL, &tv);
- } while (retval == -1 && errno == EINTR &&
- !stop && !hup && !rot && !resume);
-
- if (!stop && !hup && !rot && !resume && retval > 0) {
- if (audit_get_reply(fd, &rep->reply,
- GET_REPLY_NONBLOCKING, 0) > 0) {
- switch (rep->reply.type)
- { /* For now dont process these */
- case NLMSG_NOOP:
- case NLMSG_DONE:
- case NLMSG_ERROR:
- case AUDIT_GET: /* Or these */
- case AUDIT_LIST:
- case AUDIT_LIST_RULES:
- case AUDIT_FIRST_DAEMON...AUDIT_LAST_DAEMON:
- break;
- case AUDIT_SIGNAL_INFO:
- if (hup_info_requested) {
- audit_msg(LOG_DEBUG,
- "HUP detected, starting config manager");
- if (start_config_manager(rep)) {
- send_audit_event(
- AUDIT_DAEMON_CONFIG,
- "auditd error getting hup info - no change,"
- " sending auid=? pid=? subj=? res=failed");
- }
- rep = NULL;
- hup_info_requested = 0;
- } else if(usr1_info_requested){
- char usr1[MAX_AUDIT_MESSAGE_LENGTH];
- if (rep->reply.len == 24) {
- snprintf(usr1,
- sizeof(usr1),
- "auditd sending auid=? pid=? subj=?");
- } else {
- snprintf(usr1,
- sizeof(usr1),
- "auditd sending auid=%u pid=%d subj=%s",
- rep->reply.signal_info->uid,
- rep->reply.signal_info->pid,
- rep->reply.signal_info->ctx);
- }
- send_audit_event(
- AUDIT_DAEMON_ROTATE,
- usr1);
- }
- break;
- default:
- distribute_event(rep);
- rep = NULL;
- break;
- }
- } else {
- if (errno == EFBIG) {
- // FIXME do err action
- }
- }
- }
- if (hup) {
- int rc;
- hup = 0;
- rc = audit_request_signal_info(fd);
- if (rc < 0)
- send_audit_event(AUDIT_DAEMON_CONFIG,
- "auditd error getting hup info - no change, sending auid=? pid=? subj=? res=failed");
- else
- hup_info_requested = 1;
- }
- if (rot) {
- int rc;
- rot = 0;
- rc = audit_request_signal_info(fd);
- if (rc < 0)
- send_audit_event(AUDIT_DAEMON_ROTATE,
- "auditd error getting usr1 info - no change, sending auid=? pid=? subj=? res=failed");
- else
- usr1_info_requested = 1;
- }
- if (resume) {
- resume = 0;
- resume_logging();
- send_audit_event(AUDIT_DAEMON_RESUME,
- "auditd resuming logging, sending auid=? pid=? subj=? res=success");
- }
- if (stop) {
- /* Write message to log that we are going down */
- int rc;
-
- rc = audit_request_signal_info(fd);
- if (rc > 0) {
- struct audit_reply trep;
-
- rc = get_reply(fd, &trep, rc);
- if (rc > 0) {
- char txt[MAX_AUDIT_MESSAGE_LENGTH];
- snprintf(txt, sizeof(txt),
- "auditd normal halt, sending auid=%u pid=%d subj=%s res=success",
- trep.signal_info->uid,
- trep.signal_info->pid,
- trep.signal_info->ctx);
- send_audit_event(AUDIT_DAEMON_END, txt);
- }
- }
- if (rc <= 0)
- send_audit_event(AUDIT_DAEMON_END,
- "auditd normal halt, sending auid=? pid=? subj=? res=success");
- free(rep);
- shutdown_dispatcher();
- break;
- }
- }
+ loop = ev_default_loop (EVFLAG_NOENV);
+
+ ev_io_init (&netlink_watcher, netlink_handler, fd, EV_READ);
+ ev_io_start (loop, &netlink_watcher);
+
+ ev_signal_init (&sigterm_watcher, term_handler, SIGTERM);
+ ev_signal_start (loop, &sigterm_watcher);
+
+ ev_signal_init (&sighup_watcher, hup_handler, SIGHUP);
+ ev_signal_start (loop, &sighup_watcher);
+
+ ev_signal_init (&sigusr1_watcher, user1_handler, SIGUSR1);
+ ev_signal_start (loop, &sigusr1_watcher);
+
+ ev_signal_init (&sigusr2_watcher, user2_handler, SIGUSR2);
+ ev_signal_start (loop, &sigusr2_watcher);
+
+ ev_signal_init (&sigchld_watcher, child_handler, SIGCHLD);
+ ev_signal_start (loop, &sigchld_watcher);
+
+ ev_loop (loop, 0);
+
+ /* Write message to log that we are going down */
+
+ rc = audit_request_signal_info(fd);
+ if (rc > 0) {
+ struct audit_reply trep;
+
+ rc = get_reply(fd, &trep, rc);
+ if (rc > 0) {
+ char txt[MAX_AUDIT_MESSAGE_LENGTH];
+ snprintf(txt, sizeof(txt),
+ "auditd normal halt, sending auid=%u pid=%d subj=%s res=success",
+ trep.signal_info->uid,
+ trep.signal_info->pid,
+ trep.signal_info->ctx);
+ send_audit_event(AUDIT_DAEMON_END, txt);
+ }
+ }
+ if (rc <= 0)
+ send_audit_event(AUDIT_DAEMON_END,
+ "auditd normal halt, sending auid=? pid=? subj=? res=success");
+ free(rep);
+ shutdown_dispatcher();
close_down();
free_config(&config);
16 years, 2 months
[Patch]Fix the error in the output of "auditctl -s" when auditd is stoped
by Chu Li
Hi Steve,
When auditd is stoped, "auditctl -s" will show "pid=0". I think it's not
correct information. It's better to tell users "auditd not started".
Signed-off-by: Chu Li <chul(a)cn.fujitsu.com>
---
diff --git a/src/auditctl.c b/src/auditctl.c
index 10894f9..b26dd82 100755
--- a/src/auditctl.c
+++ b/src/auditctl.c
@@ -1411,12 +1411,15 @@ static int audit_print_reply(struct audit_reply *rep)
printed = 1;
return 0;
case AUDIT_GET:
- printf("AUDIT_STATUS: enabled=%d flag=%d pid=%d"
- " rate_limit=%d backlog_limit=%d lost=%d backlog=%u\n",
+ printf("AUDIT_STATUS: enabled=%d flag=%d"
+ " rate_limit=%d backlog_limit=%d lost=%d backlog=%u ",
rep->status->enabled, rep->status->failure,
- rep->status->pid, rep->status->rate_limit,
- rep->status->backlog_limit, rep->status->lost,
- rep->status->backlog);
+ rep->status->rate_limit, rep->status->backlog_limit,
+ rep->status->lost, rep->status->backlog);
+ if(rep->status->pid != 0)
+ printf("pid=%d\n", rep->status->pid);
+ else
+ printf("auditd_not_started\n");
printed = 1;
return 0;
case AUDIT_LIST:
Regards
Chu Li
16 years, 2 months
[PATCH 1/2] Fix the bug for missing field name before operator
by Zhang Xiliang
Hello Steve,
Steve Grubb said the following on 2008-08-07 3:19:
> >
> > Yes, this was in attempt to make sure that they didn't type -F =10. In that
> > case v will equal f because they start at the same address.
> >
> > -Steve
> >
I think the way "f == v" can't make sure that they didn't type -F =10.
After "v = strstr(pair, "=")" and v++. The v will not equal to f.
For example,
auditctl -a exit,always -F =10
Error message "-F unknown field: =10" is output.
It is checked by "audit_name_to_field()", but not "f == v".
Because before v++, the "*v" is set to 0. we can use "*f == 0" to check out the case. The patch is for it.
Signed-off-by: Zhang Xiliang <zhangxiliang(a)cn.fujitsu.com>
---
lib/deprecated.c | 5 ++++-
lib/libaudit.c | 5 ++++-
src/auditctl.c | 4 ++++
3 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/lib/deprecated.c b/lib/deprecated.c
index af1780b..6bf42dd 100644
--- a/lib/deprecated.c
+++ b/lib/deprecated.c
@@ -227,8 +227,11 @@ int audit_rule_fieldpair(struct audit_rule *rule, const char *pair, int flags)
// op = AUDIT_EQUAL;
}
- if (v == NULL || f == v)
+ if (v == NULL)
return -1;
+
+ if (*f == 0)
+ return -22;
if (*v == 0)
return -20;
diff --git a/lib/libaudit.c b/lib/libaudit.c
index 42c2176..e0f108a 100644
--- a/lib/libaudit.c
+++ b/lib/libaudit.c
@@ -820,8 +820,11 @@ int audit_rule_fieldpair_data(struct audit_rule_data **rulep, const char *pair,
op = AUDIT_BIT_MASK;
}
- if (v == NULL || f == v)
+ if (v == NULL)
return -1;
+
+ if (*f == 0)
+ return -22;
if (*v == 0)
return -20;
diff --git a/src/auditctl.c b/src/auditctl.c
index 10894f9..6144795 100644
--- a/src/auditctl.c
+++ b/src/auditctl.c
@@ -852,6 +852,10 @@ static int setopt(int count, char *vars[])
"-F value should be a number for %s\n", optarg);
retval = -1;
break;
+ case -22:
+ fprintf(stderr,
+ "-F missing field name before operator for %s\n", optarg);
+ retval = -1;
default:
retval = -1;
break;
16 years, 2 months
[PATCH]Delete unnecessary codes in auditd-config.c and audispd-pconfig.c
by Chu Li
Hi Steve,
I think such codes about "basename" in auditd-config.c and audispd-pconfig.c
is unnecessary. In these part "nv->value" can't be null and "basename()" will
never return an empty string. And the variable "base" is not used in the following
codes. So such codes are better to be deleted.
Signed-off-by: Chu Li <chul(a)cn.fujitsu.com>
---
diff --git a/src/auditd-config.c b/src/auditd-config.c
index 8977502..ca3d3a3 100644
--- a/src/auditd-config.c
+++ b/src/auditd-config.c
@@ -434,14 +434,14 @@ static const struct kw_pair *kw_lookup(const char *val)
static int log_file_parser(struct nv_pair *nv, int line,
struct daemon_conf *config)
{
- char *dir = NULL, *tdir, *base;
+ char *dir = NULL, *tdir;
DIR *d;
int fd, mode;
struct stat buf;
audit_msg(LOG_DEBUG, "log_file_parser called with: %s", nv->value);
- /* split name into dir and basename. */
+ /* get dir from name. */
tdir = strdup(nv->value);
if (tdir)
dir = dirname(tdir);
@@ -453,14 +453,6 @@ static int log_file_parser(struct nv_pair *nv, int line,
return 1;
}
- base = basename((char *)nv->value);
- if (base == 0 || strlen(base) == 0) {
- audit_msg(LOG_ERR, "The file name: %s is too short - line %d",
- base, line);
- free((void *)tdir);
- return 1;
- }
-
/* verify the directory path exists */
d = opendir(dir);
if (d == NULL) {
@@ -577,7 +569,7 @@ static int qos_parser(struct nv_pair *nv, int line,
static int dispatch_parser(struct nv_pair *nv, int line,
struct daemon_conf *config)
{
- char *dir = NULL, *tdir, *base;
+ char *dir = NULL, *tdir;
int fd;
struct stat buf;
@@ -587,7 +579,7 @@ static int dispatch_parser(struct nv_pair *nv, int line,
return 0;
}
- /* split name into dir and basename. */
+ /* get dir from name. */
tdir = strdup(nv->value);
if (tdir)
dir = dirname(tdir);
@@ -600,12 +592,6 @@ static int dispatch_parser(struct nv_pair *nv, int line,
}
free((void *)tdir);
- base = basename((char *)nv->value);
- if (base == 0 || strlen(base) == 0) {
- audit_msg(LOG_ERR, "The file name: %s is too short - line %d",
- base, line);
- return 1;
- }
/* if the file exists, see that its regular, owned by root,
* and not world anything */
fd = open(nv->value, O_RDONLY);
diff --git a/audisp/audispd-pconfig.c b/audisp/audispd-pconfig.c
index 63b3307..c630a00 100644
--- a/audisp/audispd-pconfig.c
+++ b/audisp/audispd-pconfig.c
@@ -361,7 +361,7 @@ static int direction_parser(struct nv_pair *nv, int line,
static int path_parser(struct nv_pair *nv, int line,
plugin_conf_t *config)
{
- char *dir = NULL, *tdir, *base;
+ char *dir = NULL, *tdir;
struct stat buf;
if (nv->value == NULL) {
@@ -374,7 +374,7 @@ static int path_parser(struct nv_pair *nv, int line,
return 0;
}
- /* split name into dir and basename. */
+ /* get dir form name. */
tdir = strdup(nv->value);
if (tdir)
dir = dirname(tdir);
@@ -387,12 +387,6 @@ static int path_parser(struct nv_pair *nv, int line,
}
free((void *)tdir);
- base = basename((char *)nv->value);
- if (base == 0 || strlen(base) == 0) {
- audit_msg(LOG_ERR, "The file name: %s is too short - line %d",
- base, line);
- return 1;
- }
/* If the file exists, see that its regular, owned by root,
* and not world anything */
if (stat(nv->value, &buf) < 0) {
Regards
Chu Li
Regards
Chu Li
16 years, 2 months
[PATCH 2/2] Use a new funtion to instead of outing error message for field checking
by Zhang Xiliang
Hello Steve,
The method of outing error message for field checking is too big. It is disadvantage to modify.
Create a helper function to output error messages.
It should be more pretty and smart.
Signed-off-by: Zhang Xiliang <zhangxiliang(a)cn.fujitsu.com>
---
lib/Makefile.am | 2 +-
lib/errormsg.h | 58 ++++++++++++++++++++++
lib/libaudit.c | 26 ++++++++++
src/auditctl.c | 135 ++++------------------------------------------------
src/mt/Makefile.am | 4 +-
5 files changed, 97 insertions(+), 128 deletions(-)
create mode 100644 lib/errormsg.h
diff --git a/lib/Makefile.am b/lib/Makefile.am
index 13ccbb9..c5b2c6c 100644
--- a/lib/Makefile.am
+++ b/lib/Makefile.am
@@ -30,7 +30,7 @@ lib_LTLIBRARIES = libaudit.la
include_HEADERS = libaudit.h
libaudit_la_SOURCES = libaudit.c message.c netlink.c \
lookup_table.c audit_logging.c deprecated.c \
- private.h $(BUILT_SOURCES)
+ private.h errormsg.h $(BUILT_SOURCES)
libaudit_la_LIBADD =
libaudit_la_DEPENDENCIES = $(libaudit_la_SOURCES) ../config.h
libaudit_la_LDFLAGS = -Wl,-z,relro
diff --git a/lib/errormsg.h b/lib/errormsg.h
new file mode 100644
index 0000000..6ee68d1
--- /dev/null
+++ b/lib/errormsg.h
@@ -0,0 +1,58 @@
+/* errormsg.h --
+ * Copyright 2008 FUJITSU Inc.
+ * All Rights Reserved.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ * Authors:
+ * Zhang Xiliang <zhangxiliang(a)cn.fujitsu.com>
+ */
+
+struct msg_tab {
+ int key; /* error number */
+ /*
+ * the field string position in the error message
+ * 0: don't output field string
+ * 1: output field string before error message
+ * 2: output field string after error message
+ */
+ int position;
+ const char *cvalue;
+};
+
+static const struct msg_tab err_msgtab[] = {
+ { -1, 2, "-F missing opration for" },
+ { -2, 2, "-F unknown field:" },
+ { -3, 1, "must be before -S" },
+ { -4, 1, "machine type not found" },
+ { -5, 1, "elf mapping not found" },
+ { -6, 1, "requested bit level not supported by machine" },
+ { -7, 1, "can only be used with exit filter list" },
+ { -8, 2, "-F unknown message type -" },
+ { -9, 0, "msgtype field can only be used with exclude filter list" },
+ { -10, 0, "Failed upgrading rule" },
+ { -11, 0, "String value too long" },
+ { -12, 0, "Only msgtype field can be used with exclude filter" },
+ { -13, 1, "only takes = or != operators" },
+ { -14, 0, "Permission can only contain \'rwxa\'" },
+ { -15, 2, "-F unknown errno -"},
+ { -16, 2, "-F unknown file type - " },
+ { -17, 1, "can only be used with exit and entry filter list" },
+ { -18, 1, "can not be used with exclude filter list" },
+ { -19, 0, "Key field needs a watch or syscall given prior to it" },
+ { -20, 2, "-F missing value after opration for" },
+ { -21, 2, "-F value should be number for" },
+ { -22, 2, "-F missing field name before operator for" }
+};
diff --git a/lib/libaudit.c b/lib/libaudit.c
index e0f108a..7d48d78 100644
--- a/lib/libaudit.c
+++ b/lib/libaudit.c
@@ -39,6 +39,7 @@
#include "libaudit.h"
#include "private.h"
+#include "errormsg.h"
/* #defines for the audit failure query */
#define CONFIG_FILE "/etc/libaudit.conf"
@@ -1153,3 +1154,28 @@ int audit_detect_machine(void)
return -1;
}
hidden_def(audit_detect_machine)
+
+void audit_number_to_errmsg(int errnumber, const char *opt)
+{
+ unsigned int i;
+
+ for (i = 0; i < sizeof(err_msgtab)/sizeof(struct msg_tab); i++) {
+ if (err_msgtab[i].key == errnumber) {
+ switch (err_msgtab[i].position)
+ {
+ case 0:
+ fprintf(stderr, "%s\n", err_msgtab[i].cvalue);
+ break;
+ case 1:
+ fprintf(stderr, "%s %s\n", opt, err_msgtab[i].cvalue);
+ break;
+ case 2:
+ fprintf(stderr, "%s %s\n", err_msgtab[i].cvalue, opt);
+ break;
+ default:
+ break;
+ }
+ return;
+ }
+ }
+}
diff --git a/src/auditctl.c b/src/auditctl.c
index 6144795..96aebe7 100644
--- a/src/auditctl.c
+++ b/src/auditctl.c
@@ -733,133 +733,16 @@ static int setopt(int count, char *vars[])
}
if (which == NEW)
rc = audit_rule_fieldpair_data(&rule_new,optarg,flags);
-//FIXME: make this a function
- switch (rc)
- {
- case 0:
- if (which == NEW && rule_new->fields[rule_new->field_count-1] ==
- AUDIT_PERM)
- audit_permadded = 1;
- break;
- case -1:
- fprintf(stderr, "-F missing operator for %s\n",
- optarg);
- retval = -1;
- break;
- case -2:
- fprintf(stderr, "-F unknown field: %s\n",
- optarg);
- retval = -1;
- break;
- case -3:
- fprintf(stderr,
- "-F %s must be before -S\n",
- optarg);
- retval = -1;
- break;
- case -4:
- fprintf(stderr,
- "-F %s machine type not found\n",
- optarg);
- retval = -1;
- break;
- case -5:
- fprintf(stderr,
- "-F %s elf mapping not found\n",
- optarg);
- retval = -1;
- break;
- case -6:
- fprintf(stderr,
- "-F %s requested bit level not supported by machine\n",
- optarg);
- retval = -1;
- break;
- case -7:
- fprintf(stderr,
- "Field %s can only be used with exit filter list\n",
- optarg);
- retval = -1;
- break;
- case -8:
- fprintf(stderr,
- "-F unknown message type - %s\n",
- optarg);
- retval = -1;
- break;
- case -9:
- fprintf(stderr,
- "msgtype field can only be used with exclude filter list\n");
- retval = -1;
- break;
- case -10:
- fprintf(stderr,
- "Failed upgrading rule\n");
- retval = -1;
- case -11:
- fprintf(stderr,
- "String value too long\n");
- retval = -1;
- break;
- case -12:
- fprintf(stderr,
- "Only msgtype field can be used with exclude filter\n");
- retval = -1;
- break;
- case -13:
- fprintf(stderr,
- "Field (%s) only takes = or != operators\n", optarg);
- retval = -1;
- break;
- case -14:
- fprintf(stderr,
- "Permission (%s) can only contain \'rwxa\n",
- optarg);
- retval = -1;
- break;
- case -15:
- fprintf(stderr,
- "-F unknown errno - %s\n", optarg);
- retval = -1;
- break;
- case -16:
- fprintf(stderr,
- "-F unknown file type - %s\n", optarg);
- retval = -1;
- break;
- case -17:
- fprintf(stderr,
- "Field %s can only be used with exit and entry filter list\n", optarg);
- retval = -1;
- break;
- case -18:
- fprintf(stderr,
- "Field %s can not be used with exclude filter list\n", optarg);
- retval = -1;
- break;
- case -19:
- fprintf(stderr,
- "Key field needs a watch or syscall given prior to it\n");
- retval = -1;
- break;
- case -20:
- fprintf(stderr,
- "-F missing value after operator for %s\n", optarg);
- retval = -1;
- break;
- case -21:
- fprintf(stderr,
- "-F value should be a number for %s\n", optarg);
- retval = -1;
- break;
- case -22:
- fprintf(stderr,
- "-F missing field name before operator for %s\n", optarg);
- retval = -1;
- default:
- retval = -1;
- break;
+
+ if (rc != 0) {
+ audit_number_to_errmsg(rc, optarg);
+ retval = -1;
+ } else {
+ if (which == NEW && rule_new->fields[rule_new->field_count-1] ==
+ AUDIT_PERM)
+ audit_permadded = 1;
}
+
break;
case 'm':
if (audit_log_user_message( fd, AUDIT_USER, optarg, NULL,
diff --git a/src/mt/Makefile.am b/src/mt/Makefile.am
index e840287..7581225 100644
--- a/src/mt/Makefile.am
+++ b/src/mt/Makefile.am
@@ -43,7 +43,7 @@ lib_OBJECTS = $(libauditmt_a_OBJECTS)
libaudit.h:
cp ${top_srcdir}/lib/libaudit.h .
-libaudit.c: libaudit.h private.h
+libaudit.c: libaudit.h private.h errormsg.h
cp ${top_srcdir}/lib/libaudit.c .
message.c: libaudit.h
cp ${top_srcdir}/lib/message.c .
@@ -89,6 +89,8 @@ optabs.h:
cp ${top_builddir}/lib/optabs.h .
errtabs.h:
cp ${top_builddir}/lib/errtabs.h .
+errormsg.h:
+ cp ${top_builddir}/lib/errormsg.h .
lookup_table.o: ${top_builddir}/config.h gen_tables.h i386_tables.h \
ia64_tables.h ppc_tables.h s390_tables.h s390x_tables.h \
16 years, 2 months
[PATCH]Delete unnecessary codes in auditd-config.c and audispd-pconfig.c
by Chu Li
Hi Steve,
I think such code about "basename" in auditd-config.c and audispd-pconfig.c
is unnecessary. In these part codes "nv->value" can't be null and "basename()" will
never return an empty string. And the variable "base" is not used in the following
codes. So such codes are better to be deleted.
Do you agree with me?
Signed-off-by: Chu Li <chul(a)cn.fujitsu.com>
---
diff --git a/src/auditd-config.c b/src/auditd-config.c
index 8977502..ca3d3a3 100644
--- a/src/auditd-config.c
+++ b/src/auditd-config.c
@@ -434,14 +434,14 @@ static const struct kw_pair *kw_lookup(const char *val)
static int log_file_parser(struct nv_pair *nv, int line,
struct daemon_conf *config)
{
- char *dir = NULL, *tdir, *base;
+ char *dir = NULL, *tdir;
DIR *d;
int fd, mode;
struct stat buf;
audit_msg(LOG_DEBUG, "log_file_parser called with: %s", nv->value);
- /* split name into dir and basename. */
+ /* get dir from name. */
tdir = strdup(nv->value);
if (tdir)
dir = dirname(tdir);
@@ -453,14 +453,6 @@ static int log_file_parser(struct nv_pair *nv, int line,
return 1;
}
- base = basename((char *)nv->value);
- if (base == 0 || strlen(base) == 0) {
- audit_msg(LOG_ERR, "The file name: %s is too short - line %d",
- base, line);
- free((void *)tdir);
- return 1;
- }
-
/* verify the directory path exists */
d = opendir(dir);
if (d == NULL) {
@@ -577,7 +569,7 @@ static int qos_parser(struct nv_pair *nv, int line,
static int dispatch_parser(struct nv_pair *nv, int line,
struct daemon_conf *config)
{
- char *dir = NULL, *tdir, *base;
+ char *dir = NULL, *tdir;
int fd;
struct stat buf;
@@ -587,7 +579,7 @@ static int dispatch_parser(struct nv_pair *nv, int line,
return 0;
}
- /* split name into dir and basename. */
+ /* get dir from name. */
tdir = strdup(nv->value);
if (tdir)
dir = dirname(tdir);
@@ -600,12 +592,6 @@ static int dispatch_parser(struct nv_pair *nv, int line,
}
free((void *)tdir);
- base = basename((char *)nv->value);
- if (base == 0 || strlen(base) == 0) {
- audit_msg(LOG_ERR, "The file name: %s is too short - line %d",
- base, line);
- return 1;
- }
/* if the file exists, see that its regular, owned by root,
* and not world anything */
fd = open(nv->value, O_RDONLY);
diff --git a/audisp/audispd-pconfig.c b/audisp/audispd-pconfig.c
index 63b3307..c630a00 100644
--- a/audisp/audispd-pconfig.c
+++ b/audisp/audispd-pconfig.c
@@ -361,7 +361,7 @@ static int direction_parser(struct nv_pair *nv, int line,
static int path_parser(struct nv_pair *nv, int line,
plugin_conf_t *config)
{
- char *dir = NULL, *tdir, *base;
+ char *dir = NULL, *tdir;
struct stat buf;
if (nv->value == NULL) {
@@ -374,7 +374,7 @@ static int path_parser(struct nv_pair *nv, int line,
return 0;
}
- /* split name into dir and basename. */
+ /* get dir form name. */
tdir = strdup(nv->value);
if (tdir)
dir = dirname(tdir);
@@ -387,12 +387,6 @@ static int path_parser(struct nv_pair *nv, int line,
}
free((void *)tdir);
- base = basename((char *)nv->value);
- if (base == 0 || strlen(base) == 0) {
- audit_msg(LOG_ERR, "The file name: %s is too short - line %d",
- base, line);
- return 1;
- }
/* If the file exists, see that its regular, owned by root,
* and not world anything */
if (stat(nv->value, &buf) < 0) {
Regards
Chu Li
16 years, 2 months
