[PATCH 1/2] SELinux Context Label based audit filtering
by Dustin Kirkland
Kernel audit component
This patch is against David Woodhouse's last update of his audit-2.6 git
tree, plus a patch submitted by Amy Griffis on 2006-01-17 that adds
support for strings in audit rules. This patch can be found here:
https://www.redhat.com/archives/linux-audit/2006-January/msg00064.html
Let me highlight and explain a few key points:
I've added another function similar to audit_comparator():
audit_str_comparator(). This function takes a left string, an operator,
and a right string and returns true or false based on the comparison.
NULL is considered absolute zero, such that any valid string is greater
than NULL. Additionally, two NULL strings are considered equal. These
assumptions are made such that ugly NULL pointer dereferences are easily
avoided.
Please double check my memory management. I need to kmalloc() space to
memcpy() the string from the netlink message into a kernel rule
structure. Obviously, this needs to be cleaned up properly upon rule
destruction, which I think I'm doing properly.
I also split the function audit_log_task_context() into two parts. The
first part allocates the memory necessary and returns the a context
(aka, label) as a string pointer--this new function is called
audit_get_task_label(). In turn, audit_log_task_context() simply calls
audit_get_task_label() and inserts that string into the audit record.
And audit_get_task_label() is called just before the filter code such
that it has a label to potentially filter upon. Again, proper kfree()'s
are required as audit_get_task_label() does the memory allocation and
the consumer must free.
Note that this code actually only provides enough functionality to
filter on _task_ labels. I'm looking for input or acknowledgment from
the SELinux guys (cc'd) on the validity of the approach herein.
Additionally, I'm open to suggestions on how I might similarly collect
object and user labels for the same filtering mechanism (if required).
I hope to easily extend this patch to handle those as well, though I
wanted to put this much forth immediately to incorporate suggestions.
Comments appreciated...
:-Dustin
---
diff --git a/include/linux/audit.h b/include/linux/audit.h
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -140,6 +140,11 @@
#define AUDIT_PERS 10
#define AUDIT_ARCH 11
#define AUDIT_MSGTYPE 12
+#define AUDIT_SE_USER 13 /* security label user */
+#define AUDIT_SE_ROLE 14 /* security label role */
+#define AUDIT_SE_TYPE 15 /* security label type */
+#define AUDIT_SE_CAT 16 /* security label category */
+#define AUDIT_SE_SENS 17 /* security label sensitivity */
/* These are ONLY useful when checking
* at syscall exit time (AUDIT_AT_EXIT). */
diff --git a/kernel/audit.c b/kernel/audit.c
diff --git a/kernel/audit.h b/kernel/audit.h
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -56,6 +56,7 @@ struct audit_field {
u32 type;
u32 val;
u32 op;
+ char *buf;
};
struct audit_krule {
@@ -78,6 +79,7 @@ struct audit_entry {
extern int audit_pid;
extern int audit_comparator(const u32 left, const u32 op, const u32 right);
+extern int audit_str_comparator(const char *left, const u32 op, const char *right);
extern void audit_send_reply(int pid, int seq, int type,
int done, int multi,
diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -160,15 +160,14 @@ static struct audit_entry *audit_data_to
{
int err = 0;
struct audit_entry *entry;
- void *bufp;
/* size_t remain = datasz - sizeof(struct audit_rule_data); */
int i;
+ int offset = 0;
entry = audit_to_entry_common((struct audit_rule *)data);
if (IS_ERR(entry))
goto exit_nofree;
- bufp = data->buf;
entry->rule.vers_ops = 2;
for (i = 0; i < data->field_count; i++) {
struct audit_field *f = &entry->rule.fields[i];
@@ -180,10 +179,26 @@ static struct audit_entry *audit_data_to
f->op = data->fieldflags[i] & AUDIT_OPERATORS;
f->type = data->fields[i];
+ f->val = data->values[i];
switch(f->type) {
/* call type-specific conversion routines here */
+ case AUDIT_SE_USER:
+ case AUDIT_SE_ROLE:
+ case AUDIT_SE_TYPE:
+ case AUDIT_SE_CAT:
+ case AUDIT_SE_SENS:
+ f->buf = kmalloc(f->val + 1, GFP_KERNEL);
+ if (!f->buf) {
+ err = -ENOMEM;
+ goto exit_free;
+ }
+ memcpy(f->buf, data->buf + offset, f->val);
+ f->buf[f->val] = 0;
+ offset += f->val;
+ break;
default:
- f->val = data->values[i];
+ f->buf = NULL;
+ break;
}
}
@@ -326,7 +341,13 @@ static inline int audit_add_rule(struct
static inline void audit_free_rule(struct rcu_head *head)
{
+ int i;
struct audit_entry *e = container_of(head, struct audit_entry, rcu);
+ for (i=0; i<e->rule.field_count; i++) {
+ struct audit_field *f = &e->rule.fields[i];
+ if (f->buf)
+ kfree(f->buf);
+ }
kfree(e);
}
@@ -521,6 +542,39 @@ int audit_comparator(const u32 left, con
}
}
+int audit_str_comparator(const char *left, const u32 op, const char *right)
+{
+ u32 i = 0;
+
+ if ( (left == NULL) || (right == NULL) ) {
+ /* first handle cases where left or right are NULL */
+ if ( (left == NULL) && (right == NULL) )
+ i = 0;
+ else if ( (left == NULL) && (right != NULL) )
+ i = -1;
+ else if ( (left != NULL) && (right == NULL))
+ i = 1;
+ } else
+ /* ok, not NULL, so compare strings */
+ i = strcmp(left, right);
+
+ switch(op) {
+ case AUDIT_EQUAL:
+ return (i == 0);
+ case AUDIT_NOT_EQUAL:
+ return (i != 0);
+ case AUDIT_LESS_THAN:
+ return (i < 0);
+ case AUDIT_LESS_THAN_OR_EQUAL:
+ return (i <= 0);
+ case AUDIT_GREATER_THAN:
+ return (i > 0);
+ case AUDIT_GREATER_THAN_OR_EQUAL:
+ return (i >= 0);
+ default:
+ return -EINVAL;
+ }
+}
static int audit_filter_user_rules(struct netlink_skb_parms *cb,
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -157,15 +157,23 @@ struct audit_context {
#endif
};
+static char *audit_get_task_label(void);
/* Compare a task_struct with an audit_rule. Return 1 on match, 0
* otherwise. */
static int audit_filter_rules(struct task_struct *tsk,
struct audit_krule *rule,
struct audit_context *ctx,
- enum audit_state *state)
+ enum audit_state *state,
+ char *label)
{
int i, j;
+ char *user, *role, *type, *cat, *sens;
+ user = strsep(&label, ":");
+ role = strsep(&label, ":");
+ type = strsep(&label, ":");
+ cat = strsep(&label, ":");
+ sens = strsep(&label, ":");
for (i = 0; i < rule->field_count; i++) {
struct audit_field *f = &rule->fields[i];
@@ -206,6 +214,21 @@ static int audit_filter_rules(struct tas
if (ctx)
result = audit_comparator(ctx->arch, f->op, f->val);
break;
+ case AUDIT_SE_USER:
+ result = audit_str_comparator(user, f->op, f->buf);
+ break;
+ case AUDIT_SE_ROLE:
+ result = audit_str_comparator(role, f->op, f->buf);
+ break;
+ case AUDIT_SE_TYPE:
+ result = audit_str_comparator(type, f->op, f->buf);
+ break;
+ case AUDIT_SE_CAT:
+ result = audit_str_comparator(cat, f->op, f->buf);
+ break;
+ case AUDIT_SE_SENS:
+ result = audit_str_comparator(sens, f->op, f->buf);
+ break;
case AUDIT_EXIT:
if (ctx && ctx->return_valid)
@@ -283,15 +306,18 @@ static enum audit_state audit_filter_tas
{
struct audit_entry *e;
enum audit_state state;
+ char *label = audit_get_task_label();
rcu_read_lock();
list_for_each_entry_rcu(e, &audit_filter_list[AUDIT_FILTER_TASK], list) {
- if (audit_filter_rules(tsk, &e->rule, NULL, &state)) {
+ if (audit_filter_rules(tsk, &e->rule, NULL, &state, label)) {
rcu_read_unlock();
return state;
}
}
rcu_read_unlock();
+ if (label)
+ kfree(label);
return AUDIT_BUILD_CONTEXT;
}
@@ -306,6 +332,7 @@ static enum audit_state audit_filter_sys
{
struct audit_entry *e;
enum audit_state state;
+ char *label = audit_get_task_label();
if (audit_pid && tsk->tgid == audit_pid)
return AUDIT_DISABLED;
@@ -317,13 +344,15 @@ static enum audit_state audit_filter_sys
list_for_each_entry_rcu(e, list, list) {
if ((e->rule.mask[word] & bit) == bit
- && audit_filter_rules(tsk, &e->rule, ctx, &state)) {
+ && audit_filter_rules(tsk, &e->rule, ctx, &state, label)) {
rcu_read_unlock();
return state;
}
}
}
rcu_read_unlock();
+ if (label)
+ kfree(label);
return AUDIT_BUILD_CONTEXT;
}
@@ -503,32 +532,44 @@ static inline void audit_free_context(st
static void audit_log_task_context(struct audit_buffer *ab)
{
- char *ctx = NULL;
+ char *label = audit_get_task_label();
+
+ if (label == NULL)
+ audit_panic("error in audit_log_task_context");
+ else {
+ audit_log_format(ab, " subj=%s", label);
+ kfree(label);
+ }
+ return;
+}
+
+static char *audit_get_task_label(void)
+{
+ char *label = NULL;
ssize_t len = 0;
len = security_getprocattr(current, "current", NULL, 0);
if (len < 0) {
if (len != -EINVAL)
goto error_path;
- return;
+ return NULL;
}
- ctx = kmalloc(len, GFP_KERNEL);
- if (!ctx)
+ label = kmalloc(len, GFP_KERNEL);
+ if (!label)
goto error_path;
- len = security_getprocattr(current, "current", ctx, len);
+ len = security_getprocattr(current, "current", label, len);
if (len < 0 )
goto error_path;
- audit_log_format(ab, " subj=%s", ctx);
- return;
+ return label;
error_path:
- if (ctx)
- kfree(ctx);
- audit_panic("error in audit_log_task_context");
- return;
+ if (label)
+ kfree(label);
+ audit_panic("error in audit_get_task_label");
+ return NULL;
}
static void audit_log_task_info(struct audit_buffer *ab)
19 years, 3 months
[PATCH 2/2] SELinux Context Label based audit filtering
by Dustin Kirkland
Userspace audit component
This patch is against Steve Grubb's audit-1.1.13 audit release, plus a
patch from Amy Griffis on 2006-02-02 that adds a couple of key functions
needed to pass arbitrary length strings in audit rules to and from the
kernel. This patch can be found here:
https://www.redhat.com/archives/linux-audit/2006-February/msg00001.html
There has been discussion on this list and elsewhere with Steve
explaining his commitment to maintain compatibility between various
kernel and userspace audit versions. I expect that this subtle yet
powerful change in the data structures exchanged between kernel and
userspace audit will test our mettle on that point. Some amount of code
duplication will be required for a time to support both kernels using
the old audit_rule structure and the new audit_rule_data paradigm. I do
not disagree with this, and I understand that the patch that follows
violates these compatibility requirements.
However, this patch is one of the first consumers of the new
audit_rule_data structure, and as such I think it will be most easily
understood and reviewed without the excess code required for dual
compatibility. I invite suggestions on how we should avoid the "drop
in" replacement of audit_rule that this simpler patch provides. I think
Steve has some ideas on the matter, perhaps requiring a VERSION field in
the netlink messages between kernel and userspace audit. This should
probably be further hashed out in a thread of it's own.
Back to this patch... Some key points are called out:
New fields acceptable to the -F option have been added (se_user,
se_role, se_type, se_cat, se_sens). The se_ prefix (or something like)
is necessary to distinguish between some of the loaded terms, such as
"user" and "type".
Note that I left in this patch a debug function that I found useful,
print_audit_rule_data(). This function will present a pretty clean view
of the current state of an audit_rule_data structure. This was key to
me developing in the uncharted territory around populating and passing
this new structure to the kernel. Feel free to drop it at any point (or
augment it on the other hand if you see it as being useful).
I also extracted the chunk of code out of audit_rule_fieldpair() that
would determine which operator was used in the optarg argument. This
now lives in audit_get_operator() and returns the integer value of the
contained operator. It cleans up audit_rule_fieldpair() and makes it a
bit more readable.
Speaking of which, several functions (including audit_rule_fieldpair(),
audit_rule_syscall(), audit_rule_syscallbyname()) have been updated to
use audit_rule_data structure instead of audit_rule. The new structure
is a clean replacement for the former, able to provide all of it's
predecessor's functionality and more. However, the code that Steve
eventually merges into the audit tree will likely require either a void*
that can be cast appropriately (which I think he's spoken against), or a
duplicate function that continues to work with the older structure.
The audit_rule_fieldpair() switch statement is able to extend the
existing structure via realloc() to make room for the new string, which
is inserted at the appropriate offset and the buffer lengths are
recorded.
A couple of bits of code that will eventually be in the kernel headers
are temporarily stubbed into libaudit.h such that the code will compile.
This includes #defines for AUDIT_SE_* and the audit_rule_data structure.
I'm working on adding support for listing audit rules containing
strings. I should be able to put this forth shortly. In the interest
of getting this patches out for comment immediately, though, I'm posting
what I have working now.
I've been manually testing using test cases such as these, coupled with
a very small binary that performs a simple create/delete of an ipc
message queue):
# ./auditctl -a exit,always -S ipc -F "se_role=system_r"
# ./auditctl -a exit,always -S ipc -F "se_user=user_u"
# ./auditctl -a exit,always -S ipc -F "se_user!=user_u"
...and so on...
:-Dustin
---
diff -urpN audit-1.1.3/lib/fieldtab.h audit-1.1.3.dustin/lib/fieldtab.h
--- audit-1.1.3/lib/fieldtab.h 2006-01-05 14:30:40.000000000 -0600
+++ audit-1.1.3.dustin/lib/fieldtab.h 2006-01-10 13:40:05.000000000 -0600
@@ -34,6 +34,11 @@ _S(AUDIT_LOGINUID, "loginuid" )
_S(AUDIT_PERS, "pers" )
_S(AUDIT_ARCH, "arch" )
_S(AUDIT_MSGTYPE, "msgtype" )
+_S(AUDIT_SE_USER, "se_user" )
+_S(AUDIT_SE_ROLE, "se_role" )
+_S(AUDIT_SE_TYPE, "se_type" )
+_S(AUDIT_SE_CAT, "se_cat" )
+_S(AUDIT_SE_SENS, "se_sens" )
_S(AUDIT_DEVMAJOR, "devmajor" )
_S(AUDIT_DEVMINOR, "devminor" )
diff -urpN audit-1.1.3/lib/libaudit.c audit-1.1.3.dustin/lib/libaudit.c
--- audit-1.1.3/lib/libaudit.c 2006-02-02 11:02:12.000000000 -0600
+++ audit-1.1.3.dustin/lib/libaudit.c 2006-02-02 13:14:25.000000000 -0600
@@ -48,6 +48,7 @@ int audit_syscalladded = 0;
unsigned int audit_elf = 0U;
static int name_to_uid(const char *name, uid_t *uid);
static int name_to_gid(const char *name, gid_t *gid);
+static void print_audit_rule_data(struct audit_rule_data *rd);
int audit_request_status(int fd)
@@ -580,7 +581,7 @@ int audit_log_if_enabled(int fd, int typ
return rc;
}
-int audit_rule_syscall(struct audit_rule *rule, int scall)
+int audit_rule_syscall(struct audit_rule_data *rule, int scall)
{
int word = AUDIT_WORD(scall);
int bit = AUDIT_BIT(scall);
@@ -591,7 +592,7 @@ int audit_rule_syscall(struct audit_rule
return 0;
}
-int audit_rule_syscallbyname(struct audit_rule *rule,
+int audit_rule_syscallbyname(struct audit_rule_data *rule,
const char *scall)
{
int nr, i;
@@ -602,7 +603,10 @@ int audit_rule_syscallbyname(struct audi
rule->mask[i] = ~0;
return 0;
}
- machine = audit_elf_to_machine(audit_elf);
+ if (!audit_elf)
+ machine = audit_detect_machine();
+ else
+ machine = audit_elf_to_machine(audit_elf);
if (machine < 0)
return -2;
nr = audit_name_to_syscall(scall, machine);
@@ -626,44 +630,58 @@ int audit_rule_field(struct audit_rule *
return 0;
}
-int audit_rule_fieldpair(struct audit_rule *rule, const char *pair, int flags)
+int audit_get_operator(const char *pair, char **v)
+{
+ /* look for 2-char operators first
+ then look for 1-char operators afterwards
+ when found, null out the bytes under the operators to split
+ and set value pointer just past operator bytes
+ */
+ if ( (*v = strstr(pair, "!=")) ) {
+ *(*v)++ = '\0';
+ *(*v)++ = '\0';
+ return AUDIT_NOT_EQUAL;
+ }
+ if ( (*v = strstr(pair, ">=")) ) {
+ *(*v)++ = '\0';
+ *(*v)++ = '\0';
+ return AUDIT_GREATER_THAN_OR_EQUAL;
+ }
+ if ( (*v = strstr(pair, "<=")) ) {
+ *(*v)++ = '\0';
+ *(*v)++ = '\0';
+ return AUDIT_LESS_THAN_OR_EQUAL;
+ }
+ if ( (*v = strstr(pair, "=")) ) {
+ *(*v)++ = '\0';
+ return AUDIT_EQUAL;
+ }
+ if ( (*v = strstr(pair, ">")) ) {
+ *(*v)++ = '\0';
+ return AUDIT_GREATER_THAN;
+ }
+ if ( (*v = strstr(pair, "<")) ) {
+ *(*v)++ = '\0';
+ return AUDIT_LESS_THAN;
+ }
+ return 0;
+}
+
+int audit_rule_fieldpair(struct audit_rule_data *rule,
+ const char *pair, int flags)
{
const char *f = pair;
char *v;
int op;
int field;
int vlen;
+ int total_size;
+ int offset;
if (f == NULL)
return -1;
- /* look for 2-char operators first
- then look for 1-char operators afterwards
- when found, null out the bytes under the operators to split
- and set value pointer just past operator bytes
- */
- if ( (v = strstr(pair, "!=")) ) {
- *v++ = '\0';
- *v++ = '\0';
- op = AUDIT_NOT_EQUAL;
- } else if ( (v = strstr(pair, ">=")) ) {
- *v++ = '\0';
- *v++ = '\0';
- op = AUDIT_GREATER_THAN_OR_EQUAL;
- } else if ( (v = strstr(pair, "<=")) ) {
- *v++ = '\0';
- *v++ = '\0';
- op = AUDIT_LESS_THAN_OR_EQUAL;
- } else if ( (v = strstr(pair, "=")) ) {
- *v++ = '\0';
- op = AUDIT_EQUAL;
- } else if ( (v = strstr(pair, ">")) ) {
- *v++ = '\0';
- op = AUDIT_GREATER_THAN;
- } else if ( (v = strstr(pair, "<")) ) {
- *v++ = '\0';
- op = AUDIT_LESS_THAN;
- }
+ op = audit_get_operator(pair, &v);
if (v == NULL || f == v)
return -1;
@@ -673,7 +691,9 @@ int audit_rule_fieldpair(struct audit_ru
return -2;
audit_msg(LOG_DEBUG,"f%d%s%s\n", field, audit_operator_to_symbol(op),v);
- rule->fields[rule->field_count] = field | op;
+ rule->fields[rule->field_count] = field;
+ rule->fieldflags[rule->field_count] = op;
+
switch (field)
{
case AUDIT_UID:
@@ -819,6 +839,22 @@ int audit_rule_fieldpair(struct audit_ru
case AUDIT_DEVMAJOR...AUDIT_SUCCESS:
if (flags == AUDIT_FILTER_ENTRY)
return -7;
+ case AUDIT_SE_USER:
+ case AUDIT_SE_ROLE:
+ case AUDIT_SE_TYPE:
+ case AUDIT_SE_CAT:
+ case AUDIT_SE_SENS:
+ rule->values[rule->field_count] = strlen(v);
+ rule->buflen += strlen(v);
+ total_size = sizeof(*rule) + rule->buflen;
+ if (realloc(rule, total_size) == NULL) {
+ printf("Cannot realloc memory!\n");
+ return -3;
+ }
+ offset = total_size - sizeof(*rule) - strlen(v);
+ strncpy(&rule->buf[offset], v, strlen(v));
+ break;
+
/* fallthrough */
default:
rule->values[rule->field_count] = strtol(v, NULL, 0);
@@ -828,6 +864,30 @@ int audit_rule_fieldpair(struct audit_ru
return 0;
}
+/*
+ * Debug function useful to sanity check the contents of this new
+ * audit_rule_data structure, and in particular the variable length
+ * strings
+ */
+void print_audit_rule_data(struct audit_rule_data *rd) {
+ int i;
+ printf("====================================\n");
+ printf("flags = [0x%x]\n", rd->flags);
+ printf("action = [%d]\n", rd->action);
+ printf("field_count = [%d]\n", rd->field_count);
+ for (i=0; i<AUDIT_BITMASK_SIZE; i++)
+ if (rd->mask[i])
+ printf("mask[%d] = [0x%x]\n", i, rd->mask[i]);
+ for (i=0; i<AUDIT_MAX_FIELDS; i++) {
+ if (rd->fields[i] || rd->values[i] || rd->fieldflags[i])
+ printf("fields[%d] = [%d], values[%d] = [%d], fieldflags[%d] = [0x%x]\n", i, rd->fields[i], i, rd->values[i], i, rd->fieldflags[i]);
+ }
+ printf("buflen = [%d]\n", rd->buflen);
+ printf("buf = [%s]\n", rd->buf);
+ printf("TOTAL_SIZE = [%d]\n", sizeof(*rd) + rd->buflen);
+ printf("====================================\n");
+}
+
void audit_rule_free(struct audit_rule *rule)
{
if (rule)
diff -urpN audit-1.1.3/lib/libaudit.h audit-1.1.3.dustin/lib/libaudit.h
--- audit-1.1.3/lib/libaudit.h 2006-02-02 11:02:12.000000000 -0600
+++ audit-1.1.3.dustin/lib/libaudit.h 2006-02-02 13:14:47.000000000 -0600
@@ -182,6 +182,13 @@ extern "C" {
#ifndef AUDIT_MSGTYPE
#define AUDIT_MSGTYPE 12
#endif
+#ifndef AUDIT_SE_USER
+#define AUDIT_SE_USER 13
+#define AUDIT_SE_ROLE 14
+#define AUDIT_SE_TYPE 15
+#define AUDIT_SE_CAT 16
+#define AUDIT_SE_SENS 17
+#endif
/* This is new list defines from audit.h */
#ifndef AUDIT_FILTER_USER
@@ -236,6 +243,30 @@ struct watch_transport {
};
#endif
+
+#ifndef AUDIT_ADD_RULE
+#define AUDIT_ADD_RULE 1011 /* Add syscall filtering rule */
+#define AUDIT_DEL_RULE 1012 /* Delete syscall filtering rule */
+#define AUDIT_LIST_RULES 1013 /* List syscall filtering rules */
+/* audit_rule_data supports filter rules with both integer and string
+ * fields. It corresponds with AUDIT_ADD_RULE, AUDIT_DEL_RULE and
+ * AUDIT_LIST_RULES requests.
+ */
+struct audit_rule_data {
+ __u32 flags; /* AUDIT_PER_{TASK,CALL}, AUDIT_PREPEND */
+ __u32 action; /* AUDIT_NEVER, AUDIT_POSSIBLE, AUDIT_ALWAYS */
+ __u32 field_count;
+ __u32 mask[AUDIT_BITMASK_SIZE];
+ __u32 fields[AUDIT_MAX_FIELDS];
+ __u32 values[AUDIT_MAX_FIELDS];
+ __u32 fieldflags[AUDIT_MAX_FIELDS];
+ __u32 buflen; /* total length of string fields */
+ char buf[0]; /* string fields buffer */
+};
+#endif
+
+
+
struct audit_watch {
uint32_t dev_major;
uint32_t dev_minor;
@@ -386,12 +417,12 @@ extern int audit_log_user_avc_message(in
const char *tty, uid_t uid);
/* Rule-building helper functions */
-extern int audit_rule_syscall(struct audit_rule *rule, int scall);
-extern int audit_rule_syscallbyname(struct audit_rule *rule,
+extern int audit_rule_syscall(struct audit_rule_data *rule, int scall);
+extern int audit_rule_syscallbyname(struct audit_rule_data *rule,
const char *scall);
extern int audit_rule_field(struct audit_rule *rule, int field,
unsigned int value);
-extern int audit_rule_fieldpair(struct audit_rule *rule, const char *pair,
+extern int audit_rule_fieldpair(struct audit_rule_data *rule, const char *pair,
int flags);
extern void audit_rule_free(struct audit_rule *rule);
diff -urpN audit-1.1.3/src/auditctl.c audit-1.1.3.dustin/src/auditctl.c
--- audit-1.1.3/src/auditctl.c 2006-01-04 16:19:55.000000000 -0600
+++ audit-1.1.3.dustin/src/auditctl.c 2006-02-02 13:15:09.000000000 -0600
@@ -70,6 +70,7 @@ static int list_requested = 0;
static int add = AUDIT_FILTER_UNSET, del = AUDIT_FILTER_UNSET, action = 0;
static int ins = 0, rem = 0, ignore = 0;
static struct audit_rule rule;
+static struct audit_rule_data *rule_data;
static struct audit_watch watch;
extern int audit_archadded;
@@ -92,6 +93,7 @@ static int reset_vars(void)
ins = 0;
rem = 0;
+ rule_data = calloc(1, sizeof(*rule_data));
memset(&rule, 0, sizeof(rule));
memset(&watch, 0, sizeof(watch));
if ((fd = audit_open()) < 0) {
@@ -158,6 +160,8 @@ static int audit_rule_setup(const char *
*act = AUDIT_ALWAYS;
else
return 1;
+ rule_data->flags = *flags;
+ rule_data->action = *act;
return 0;
}
@@ -498,7 +502,7 @@ static int setopt(int count, char *vars[
audit_elf = elf;
}
}
- switch (audit_rule_syscallbyname(&rule, optarg))
+ switch (audit_rule_syscallbyname(rule_data, optarg))
{
case 0:
audit_syscalladded = 1;
@@ -527,8 +531,8 @@ static int setopt(int count, char *vars[
fprintf(stderr, "List must be given before field\n");
retval = -1;
break;
- }
- switch (audit_rule_fieldpair(&rule, optarg, flags))
+ }
+ switch (audit_rule_fieldpair(rule_data, optarg, flags))
{
case 0:
break;
@@ -863,14 +867,14 @@ static int handle_request(int status)
// if !task add syscall any if not specified
if ((add & AUDIT_FILTER_MASK) != AUDIT_FILTER_TASK &&
audit_syscalladded != 1)
- audit_rule_syscallbyname(&rule, "all");
- rc = audit_add_rule(fd, &rule, add, action);
+ audit_rule_syscallbyname(rule_data, "all");
+ rc = audit_add_rule_data(fd, rule_data);
}
else if (del != AUDIT_FILTER_UNSET) {
if ((del & AUDIT_FILTER_MASK) != AUDIT_FILTER_TASK &&
audit_syscalladded != 1)
- audit_rule_syscallbyname(&rule, "all");
- rc = audit_delete_rule(fd, &rule, del, action);
+ audit_rule_syscallbyname(rule_data, "all");
+ rc = audit_del_rule_data(fd, rule_data);
}
else if (ins && !rem)
rc = audit_insert_watch(fd, &watch);
19 years, 3 months
[PATCH] Fix audit operators
by Dustin Kirkland
Darrel Goeddel initiated a discussion on IRC regarding the possibility
of audit_comparator() returning -EINVAL signaling an invalid operator.
It is possible when creating the rule to assure that the operator is one
of the 6 sane values. Here's a snip from include/linux/audit.h Note
that 0 (nonsense) and 7 (all operators) are not valid values for an
operator.
...
#define AUDIT_NEGATE 0x80000000
/* These are the supported operators.
* 4 2 1
* = > <
* -------
* 0 0 0 0 nonsense
* 0 0 1 1 <
* 0 1 0 2 >
* 0 1 1 3 !=
* 1 0 0 4 =
* 1 0 1 5 <=
* 1 1 0 6 >=
* 1 1 1 7 all operators
*/
#define AUDIT_LESS_THAN 0x10000000
#define AUDIT_GREATER_THAN 0x20000000
#define AUDIT_NOT_EQUAL 0x30000000
#define AUDIT_EQUAL 0x40000000
#define AUDIT_LESS_THAN_OR_EQUAL (AUDIT_LESS_THAN|AUDIT_EQUAL)
#define AUDIT_GREATER_THAN_OR_EQUAL (AUDIT_GREATER_THAN|AUDIT_EQUAL)
#define AUDIT_OPERATORS (AUDIT_EQUAL|AUDIT_NOT_EQUAL)
...
Furthermore, prior to adding these extended operators, flagging the
AUDIT_NEGATE bit implied !=, and otherwise == was assumed.
The following code forces the operator to be != if the AUDIT_NEGATE bit
was flipped on. And if no operator was specified, == is assumed. The
only invalid condition is if the AUDIT_NEGATE bit is off and all of the
AUDIT_EQUAL, AUDIT_LESS_THAN, and AUDIT_GREATER_THAN bits are
on--clearly a nonsensical operator.
Now that this is handled at rule insertion time, the default -EINVAL
return of audit_comparator() is eliminated such that the function can
only return 1 or 0.
If this is acceptable, let's get this applied to the current tree.
:-Dustin
--
diff -urpN a/kernel/auditfilter.c b/kernel/auditfilter.c
--- a/kernel/auditfilter.c 2006-02-16 11:57:17.000000000 -0600
+++ b/kernel/auditfilter.c 2006-02-16 13:30:33.000000000 -0600
@@ -139,11 +139,17 @@ static struct audit_entry *audit_rule_to
f->val = rule->values[i];
entry->rule.vers_ops = (f->op & AUDIT_OPERATORS) ? 2 : 1;
+
+ /* Support for legacy operators where
+ * AUDIT_NEGATE bit signifies != and otherwise assumes == */
if (f->op & AUDIT_NEGATE)
- f->op |= AUDIT_NOT_EQUAL;
- else if (!(f->op & AUDIT_OPERATORS))
- f->op |= AUDIT_EQUAL;
- f->op &= ~AUDIT_NEGATE;
+ f->op = AUDIT_NOT_EQUAL;
+ else if (!f->op)
+ f->op = AUDIT_EQUAL;
+ else if (f->op == AUDIT_OPERATORS) {
+ err = -EINVAL;
+ goto exit_free;
+ }
}
exit_nofree:
@@ -537,8 +543,6 @@ int audit_comparator(const u32 left, con
return (left > right);
case AUDIT_GREATER_THAN_OR_EQUAL:
return (left >= right);
- default:
- return -EINVAL;
}
}
19 years, 3 months
Re: [RFC][PATCH] collect security labels on user processes generating audit messages
by Timothy R. Chavez
On Thu, 2006-02-16 at 12:03 -0700, Lamont R. Peterson wrote:
> On Wednesday 15 February 2006 10:17am, Linda Knippers wrote:
> > Steve Grubb wrote:
> > > This should be a separate thread since the topic is different.
> > >
> > > On Wednesday 15 February 2006 11:14, Linda Knippers wrote:
> > >>Amy submitted a patch a while back to eliminate the "name=" field
> > >>to avoid "name=(null)" from the audit records if there was no name
> > >>but I don't think the patch went anywhere.
> > >
> > > Right. I want all audit fields to have name=value. If we have %s in the
> > > message and pass NULL to it, snprintf is already going to put "(null)" so
> > > what's wrong with just using this precedent?
> >
> > The problem is that "(null)" is a valid file name.
> >
> > [ljk@cert-e2 ~]$ touch "(null)"
> > [ljk@cert-e2 ~]$ ls -l "(null)"
> > -rw-rw-r-- 1 ljk ljk 0 Feb 17 11:14 (null)
> >
> > When I look at audit records generated by those commands I see records
> > like this:
> >
> > type=SYSCALL msg=audit(1140192875.311:3789): arch=c000003e syscall=132
> > success=yes exit=0 a0=7fbffffc51 a1=0 a2=1b6 a3=0 items=1 pid=2116
> > auid=501 uid=501 gid=501 euid=501 suid=501 fsuid=501 egid=501 sgid=501
> > fsgid=501 comm="touch" exe="/bin/touch"
> > type=CWD msg=audit(1140192875.311:3789): cwd="/home/ljk"
> > type=PATH msg=audit(1140192875.311:3789): name="(null)" flags=1
> > inode=6537222 dev=fd:01 mode=0100664 ouid=501 ogid=501 rdev=00:00
> >
> > How can I tell from the audit records that the file name was "(null)"
> > vs. having "(null)" manufactured by the audit system?
>
> How about:
>
> type=PATH msg=audit(1140192875.311:3789): name=NULL flags=1
>
> in cases where it truly is NULL? The double-quotes "" are used to quote
> file-names and without them, we have some kind of meta-value, instead.
>
> [snip]
The difference is too subtle. I imagine that will get confusing. What
we use to represent a NULL value isn't as important is how we
distinguish it. For instance, simply encoding "NULL" the filename will
make the distinction between 'name=NULL' and
name="NULL" (name="78857676") , clearer.
If we "strongly suggest" the admin use ausearch to read the log, then we
could let ausearch make the distinction between quoted and unquoted
NULL's clearer, rather than the kernel.
-tim
19 years, 3 months
[PATCH] pass gfp_mask to kmalloc() done from audit_log_exit()
by Alexander Viro
When caller tells us that there are constraints on allocations we are allowed
to do, we'd better follow those for more than the first allocation.
Signed-off-by: Al Viro <viro(a)zeniv.linux.org.uk>
---
kernel/auditsc.c | 20 ++++++++++++--------
1 files changed, 12 insertions(+), 8 deletions(-)
483cfcd5b5076acedd1729b23f84a901ab165baf
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 8b92132..a9e991b 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -503,7 +503,7 @@ static inline void audit_free_context(st
printk(KERN_ERR "audit: freed %d contexts\n", count);
}
-static void audit_log_task_context(struct audit_buffer *ab)
+static void audit_log_task_context(struct audit_buffer *ab, gfp_t gfp_mask)
{
char *ctx = NULL;
ssize_t len = 0;
@@ -515,7 +515,7 @@ static void audit_log_task_context(struc
return;
}
- ctx = kmalloc(len, GFP_KERNEL);
+ ctx = kmalloc(len, gfp_mask);
if (!ctx)
goto error_path;
@@ -533,7 +533,7 @@ error_path:
return;
}
-static void audit_log_task_info(struct audit_buffer *ab)
+static void audit_log_task_info(struct audit_buffer *ab, gfp_t gfp_mask)
{
char name[sizeof(current->comm)];
struct mm_struct *mm = current->mm;
@@ -546,6 +546,10 @@ static void audit_log_task_info(struct a
if (!mm)
return;
+ /*
+ * this is brittle; all callers that pass GFP_ATOMIC will have
+ * NULL current->mm and we won't get here.
+ */
down_read(&mm->mmap_sem);
vma = mm->mmap;
while (vma) {
@@ -559,7 +563,7 @@ static void audit_log_task_info(struct a
vma = vma->vm_next;
}
up_read(&mm->mmap_sem);
- audit_log_task_context(ab);
+ audit_log_task_context(ab, gfp_mask);
}
static void audit_log_exit(struct audit_context *context, gfp_t gfp_mask)
@@ -595,12 +599,12 @@ static void audit_log_exit(struct audit_
context->gid,
context->euid, context->suid, context->fsuid,
context->egid, context->sgid, context->fsgid);
- audit_log_task_info(ab);
+ audit_log_task_info(ab, gfp_mask);
audit_log_end(ab);
for (aux = context->aux; aux; aux = aux->next) {
- ab = audit_log_start(context, GFP_KERNEL, aux->type);
+ ab = audit_log_start(context, gfp_mask, aux->type);
if (!ab)
continue; /* audit_panic has been called */
@@ -637,7 +641,7 @@ static void audit_log_exit(struct audit_
}
if (context->pwd && context->pwdmnt) {
- ab = audit_log_start(context, GFP_KERNEL, AUDIT_CWD);
+ ab = audit_log_start(context, gfp_mask, AUDIT_CWD);
if (ab) {
audit_log_d_path(ab, "cwd=", context->pwd, context->pwdmnt);
audit_log_end(ab);
@@ -647,7 +651,7 @@ static void audit_log_exit(struct audit_
unsigned long ino = context->names[i].ino;
unsigned long pino = context->names[i].pino;
- ab = audit_log_start(context, GFP_KERNEL, AUDIT_PATH);
+ ab = audit_log_start(context, gfp_mask, AUDIT_PATH);
if (!ab)
continue; /* audit_panic has been called */
--
0.99.9.GIT
19 years, 3 months
Incorrect value of ptrace's 4th argument on zSeries
by Michael C Thompson
Hey all,
I've found an issue with the logging of the value of the 4th argument of
the ptrace syscall.
The call is: ptrace(PTRACE_TRACEME,0,0,0) and ptrace(PTRACE_KILL,1,0,0)
the value of the 4th argument, that is the 0, is logged as the following:
type=SYSCALL msg=audit(1140022035.377:246959): arch=16 syscall=26
success=yes exit=0 a0=0 a1=0 a2=0 a3=20000000000 items=0 pid=5236 auid=500
uid=501 gid=501 euid=501 suid=0 fsuid=501 egid=501 sgid=0 fsgid=501
comm="ptrace_test"
exe="/rhcc/lspp/tests/LTP/ltp-merged/testcases/audit/syscalls/ptrace_test"
As you can see, a3 is logged as "a3=20000000000".
I am not sure if this extends to other syscalls, but this issue makes
logging with specific argument values challenging at best.
Mike
19 years, 3 months
Unable to filter on negative values
by Michael C Thompson
Hey all,
Apparently, this is a repeated report of a known problem, but here it is
anways:
I believe there is a short coming with auditctl and specifying a filter
for a negative value for the field, such as exit, a0, etc.
Here are the steps you can use to verify this:
#include <unistd.h>
int main() {
pread(-1,NULL,0,0);
}
Compile the above and add the following rules:
# auditctl -a exit,always -S pread -- captures record
# auditctl -D
# auditctl -a exit,always -S pread -F exit=-9 -- (return code on the
system I am using) no record
This can also be done with any syscall (like chmod if you don't want to
code C), as long as you filter on the right value. It seems that any
negative value which you try to filter on will fail.
If you have any questions or want more information as to what I've seen,
just ask.
Mike
19 years, 3 months
[PATCH 0/2] audit string fields interface + consumer
by Amy Griffis
Following are two patches, the first of which provides an interface
for specifying audit rules with string fields. The second patch adds
a new string field AUDIT_WATCH. These patches are an update of the
previous audit interface patches I have posted to this list.
These patches are functionally similar to the previous posts; however,
I believe you will find the organization of the code to be quite
different and much improved. I have updated the interface based on
the feedback I received. I also resolved the issue of differentiating
between inode-based and path-based (or watch-based) filtering by
creating a new field AUDIT_WATCH and adding it to the switch in
audit_filter_rules().
Following is a summary of the interface.
A new struct audit_rule_data and corresponding netlink message types
have been added. Additionally, the SELinux nlmsg_audit_perms[] table
has been updated with the new netlink message types.
The new struct allows userspace to supply one or more string fields
packed in a variable length buffer. The kernel expects the buffer to
be neither null-delimited nor null-terminated.
The length of string data for a given field is provided as its value
element in the array. The kernel provides the buflen element for
convenience on rule listing, to allow userspace to allocate memory for
the buffer without walking the array to tabulate lengths. Buflen is
ignored coming from userspace. Instead, the kernel ensures that the
total of the lengths specified in the value elements do not exceed the
length of the message payload minus sizeof(struct audit_rule_data).
Several routines have been added to auditfilter.c to translate between
the kernel's rule representation and the two userspace rule
representations using structs audit_rule and audit_rule_data.
19 years, 3 months
Problem meeting FAU_SEL with trusted programs
by Matt Anderson
After posting my patch last week I got some feedback about a problem
with FAU_SEL.1 Selectable Audit and FAU_SAR.3 Selectable Audit Review.
Since cups is generating audit messages on behalf of the user the uid
and auid fields in the record are those of cupsd and not the user who
submitted the print job.
type=USER_LABELED_EXPORT msg=audit(1139496671.166:828):
user pid=20396 uid=0 auid=4294967295
msg='job=21 user="mra" printer="localp" with labels "secret" "secret":
exe="/usr/sbin/cupsd" (hostname=orb.zko.hp.com, addr=16.116.96.203,
terminal=pts/1 res=success)'
Even if the audit message string was changed to include 'uid=500' there
would still be a discrepancy between the various fields of the record.
I mentioned this on #audit and Steve said he could add the ability to
ausearch to look for the uid= substring in the message, but I'm not sure
that meets the first requirement.
If we have two uid and auid fields in all of these trusted program
generated messages is it always the one in the message string that is
correct? Does ausearch always have to look into the message string to
see if there is a (a)uid there to match on? Can a trusted program be
trusted to pass in the auid and uid to audit? If it can then there
would be only one uid and auid in the record, do we need the
identification information about the trusted program which passed this
data to the kernel?
It seems like other trusted programs (at least cron) will also have this
problem of a server generating messages on behalf of a user and needing
to pass audit records into the kernel with that user's information.
Don't we need an audit interface to support that?
-matt
19 years, 3 months
git tree
by Steve Grubb
>From Al:
OK... Untangling the mess around ppoll/unshare for backport to
2.6.15-based tree turns out to be just too messy. Therefore,
git://git.kernel.org/pub/scm/linux/kernel/git/viro/audit-current.git/
contains the tree based on Linus' current. Branches:
* origin = mainline
* audit = ported stuff, that's where everything got to settle
Amy, I've moved 3 of 4 patches to current tree; please see if they are
OK (the 3rd might be worth a look) and see if you could port the last one.
Currently ported stuff is in branch called amg (3 changesets, starts at
current tip of audit).
I'm going to keep the entire construction within a day from
mainline; experiments on much larger patchset show that it's quite
feasible. Questions:
* what kind of tags do we want for that animal? E.g. for
kernels we are testing, etc.
* do we want LSPP srpms to be put there? It's not hard to do;
about half an hour for each...
* what of pending patches needs to be ported to current tree
ASAP? Note that we've got unshare from mainline; everything that had
been in old tree is there and so's jbaron's vm86 patch.
19 years, 3 months
