Discussion:
[RFC] [PATCH]
(too old to reply)
Darrel Goeddel
2006-02-16 15:19:33 UTC
Permalink
Hello,

This patch provides the selinux "backend" for the audit system to perform
filtering based on the process context. Dustin Kirkland has previously
a portion of this functionality here:

http://www.redhat.com/archives/linux-audit/2006-February/msg00004.html

This interfaces included in this patch will allow selinux to perform more
efficient matches based on lower level constructs within the selinux module,
rather than relying on string comparisons. It also allows for dominance checks
on the mls portion of the contexts that are impossible with only string
comparisons. Dustin's previous patch will be modified to take advantage of
the new interface.

This is still a work in progress (I'm guessing that the conversion of Dustin's
earlier work will point out some improvements to these interfaces). I also
need to check the context of memory allocations.

I'm only allow == and != for type, role, and user because they seemed to be
the only ones that make sense, is that OK, or should I take them all even
though they may not do anything useful? Does the general approach seem acceptable?

The patch is against Al Viro's audit tree:
http://www.kernel.org/git/?p=linux/kernel/git/viro/audit-current.git;a=summary


diff --git a/include/linux/audit.h b/include/linux/audit.h
index 4bb4b9f..dd4f759 100644
--- 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_SEN 16 /* security label sensitivity label */
+#define AUDIT_SE_CLR 17 /* security label clearance label */

/* These are ONLY useful when checking
* at syscall exit time (AUDIT_AT_EXIT). */
diff --git a/include/linux/selinux.h b/include/linux/selinux.h
new file mode 100644
index 0000000..5a402a5
--- /dev/null
+++ b/include/linux/selinux.h
@@ -0,0 +1,83 @@
+/*
+ * SELinux services exported to the rest of the kernel.
+ *
+ * Author: James Morris <***@redhat.com>
+ *
+ * Copyright (C) 2005 Red Hat, Inc., James Morris <***@redhat.com>
+ * Copyright (C) 2006 Trusted Computer Solutions <***@trustedcs.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2,
+ * as published by the Free Software Foundation.
+ */
+#ifndef _LINUX_SELINUX_H
+#define _LINUX_SELINUX_H
+
+#ifdef CONFIG_SECURITY_SELINUX
+
+/**
+ * selinux_audit_rule_init - alloc/init an selinux audit rule structure.
+ * @field: the field this rule refers to
+ * @op: the operater the rule uses
+ * @rulestr: the text "target" of the rule
+ * @rule: address of the rule structure pointer to be returned
+ *
+ * Returns 0 if successful, -errno if not. On success, the rule structure
+ * will be allocated internally. The caller must free this structure with
+ * selinux_audit_rule_free() after use.
+ */
+int selinux_audit_rule_init(u32 field, u32 op, const char *rulestr,
+ void **rule);
+
+/**
+ * selinux_audit_rule_free - free an selinux audit rule structure.
+ * @rule: address of the seliux_audit_rule structure to be freed
+ *
+ * This will free all memory associated with the given rule.
+ */
+void selinux_audit_rule_free(void *rule);
+
+/**
+ * selinux_audit_rule_match - determine if a context ID matches a rule.
+ * @ctxid: the context ID to check
+ * @rule: the audit rule created by selinux_audit_rule_init()
+ *
+ * Returns 1 if the context id matches the rule, 0 if it does not, and
+ * -errno on failure.
+ */
+int selinux_audit_rule_match(u32 ctxid, void *rule);
+
+/**
+ * selinux_task_getsecid - retrieve the context ID of a process.
+ * @tsk: the task_struct of the process we are interested in
+ *
+ * Returns the context ID of the process.
+ */
+
+int selinux_task_getsecid(struct task_struct *tsk);
+
+#else
+
+int selinux_audit_rule_init(u32 field, u32 op, const char *rulestr, void **rule)
+{
+ return -ENOTSUPP;
+}
+
+void selinux_audit_rule_free(void *rule)
+{
+ return;
+}
+
+int selinux_audit_rule_match(u32 ctxid, void *rule)
+{
+ return 0;
+}
+
+int selinux_task_getsecid(struct task_struct *tsk)
+{
+ return 0;
+}
+
+#endif /* CONFIG_SECURITY_SELINUX */
+
+#endif /* _LINUX_SELINUX_H */
diff --git a/security/selinux/Makefile b/security/selinux/Makefile
index 688c0a2..faf2e02 100644
--- a/security/selinux/Makefile
+++ b/security/selinux/Makefile
@@ -4,7 +4,7 @@

obj-$(CONFIG_SECURITY_SELINUX) := selinux.o ss/

-selinux-y := avc.o hooks.o selinuxfs.o netlink.o nlmsgtab.o netif.o
+selinux-y := avc.o hooks.o selinuxfs.o netlink.o nlmsgtab.o netif.o exports.o

selinux-$(CONFIG_SECURITY_NETWORK_XFRM) += xfrm.o

diff --git a/security/selinux/exports.c b/security/selinux/exports.c
new file mode 100644
index 0000000..a44e301
--- /dev/null
+++ b/security/selinux/exports.c
@@ -0,0 +1,44 @@
+/*
+ * SELinux services exported to the rest of the kernel.
+ *
+ * Author: James Morris <***@redhat.com>
+ *
+ * Copyright (C) 2005 Red Hat, Inc., James Morris <***@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2,
+ * as published by the Free Software Foundation.
+ */
+#include <linux/types.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/selinux.h>
+
+#include "security.h"
+#include "objsec.h"
+
+int selinux_audit_rule_init(u32 field, u32 op, const char *rulestr, void **rule)
+{
+ return security_aurule_init(field, op, rulestr, rule);
+}
+
+void selinux_audit_rule_free(void *rule)
+{
+ return security_aurule_free(rule);
+}
+
+int selinux_audit_rule_match(u32 ctxid, void *rule)
+{
+ return security_aurule_match(ctxid, rule);
+}
+
+int selinux_task_getsecid(struct task_struct *tsk)
+{
+ struct task_security_struct *tsec = tsk->security;
+ return tsec->sid;
+}
+
+EXPORT_SYMBOL_GPL(selinux_audit_rule_init);
+EXPORT_SYMBOL_GPL(selinux_audit_rule_free);
+EXPORT_SYMBOL_GPL(selinux_audit_rule_match);
+EXPORT_SYMBOL_GPL(selinux_task_getsecid);
diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
index 5f016c9..bfea536 100644
--- a/security/selinux/include/security.h
+++ b/security/selinux/include/security.h
@@ -96,5 +96,9 @@ int security_fs_use(const char *fstype,
int security_genfs_sid(const char *fstype, char *name, u16 sclass,
u32 *sid);

+int security_aurule_init(u32 field, u32 op, const char *rulestr, void **rule);
+void security_aurule_free(void *rule);
+int security_aurule_match(u32 ctxid, void *rule);
+
#endif /* _SELINUX_SECURITY_H_ */

diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index d877cd1..480df81 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -1810,3 +1810,250 @@ out:
POLICY_RDUNLOCK;
return rc;
}
+
+struct selinux_audit_rule {
+ u32 au_skip;
+ u32 au_op;
+ u32 au_field;
+ u32 au_seqno;
+ struct context au_ctxt;
+ char *au_str;
+};
+
+/* needs policy read lock held */
+static void aurule_init_context(struct selinux_audit_rule *aurule)
+{
+ struct role_datum *roledatum;
+ struct type_datum *typedatum;
+ struct user_datum *userdatum;
+ char *tmpstr;
+ int rc = 0;
+
+ switch (aurule->au_field) {
+ case AUDIT_SE_USER:
+ userdatum = hashtab_search(policydb.p_users.table,
+ aurule->au_str);
+ if (!userdatum)
+ rc = -EINVAL;
+ else
+ aurule->au_ctxt.user = userdatum->value;
+ break;
+ case AUDIT_SE_ROLE:
+ roledatum = hashtab_search(policydb.p_roles.table,
+ aurule->au_str);
+ if (!roledatum)
+ rc = -EINVAL;
+ else
+ aurule->au_ctxt.role = roledatum->value;
+ break;
+ case AUDIT_SE_TYPE:
+ typedatum = hashtab_search(policydb.p_types.table,
+ aurule->au_str);
+ if (!typedatum)
+ rc = -EINVAL;
+ else
+ aurule->au_ctxt.type = typedatum->value;
+ break;
+ case AUDIT_SE_SEN:
+ case AUDIT_SE_CLR:
+ /* TODO:figure out proper allocation below */
+ tmpstr = kstrdup(aurule->au_str, GFP_KERNEL);
+ rc = mls_context_to_sid(':', &tmpstr, &aurule->au_ctxt, NULL,
+ SECSID_NULL);
+ kfree(tmpstr);
+ break;
+ default:
+ rc = -EINVAL;
+ break;
+ }
+
+ if (rc) {
+ aurule->au_skip = 1;
+ context_destroy(&aurule->au_ctxt);
+ } else {
+ /* we merely flags this rule to not be processed - the role,
+ user, type, or level of the rule may not be valid now, but
+ may be after a future policy reload. */
+ aurule->au_skip = 0;
+ }
+
+ return;
+}
+
+int security_aurule_init(u32 field, u32 op, const char *rulestr, void **rule)
+{
+ struct selinux_audit_rule *tmprule;
+
+ *rule = NULL;
+
+ switch (field) {
+ case AUDIT_SE_USER:
+ case AUDIT_SE_ROLE:
+ case AUDIT_SE_TYPE:
+ /* only 'equals' and 'not equals' make sense */
+ if (op != AUDIT_EQUAL && op != AUDIT_NOT_EQUAL)
+ return -EINVAL;
+ case AUDIT_SE_SEN:
+ case AUDIT_SE_CLR:
+ /* we do not allow a range, indicated by '-' */
+ if (strchr(rulestr, '-'))
+ return -EINVAL;
+ }
+
+ /* TODO:figure out proper allocations below */
+ tmprule = kzalloc(sizeof(struct selinux_audit_rule), GFP_KERNEL);
+ if (!tmprule)
+ return -ENOMEM;
+ tmprule->au_str = kstrdup(rulestr, GFP_KERNEL);
+ if (!tmprule->au_str) {
+ kfree(tmprule);
+ return -ENOMEM;
+ }
+ tmprule->au_op = op;
+ tmprule->au_field = field;
+ context_init(&tmprule->au_ctxt);
+
+ if (!ss_initialized) {
+ tmprule->au_seqno = latest_granting;
+ tmprule->au_skip = 1;
+ *rule = tmprule;
+ return 0;
+ }
+
+ POLICY_RDLOCK;
+
+ tmprule->au_seqno = latest_granting;
+ aurule_init_context(tmprule);
+
+ POLICY_RDUNLOCK;
+
+ *rule = tmprule;
+
+ return 0;
+}
+
+void security_aurule_free(void *rule)
+{
+ struct selinux_audit_rule *aurule = rule;
+
+ kfree(aurule->au_str);
+ context_destroy(&aurule->au_ctxt);
+ kfree(aurule);
+}
+
+int security_aurule_match(u32 ctxid, void *rule)
+{
+ struct selinux_audit_rule *aurule = rule;
+ struct context *ctxt;
+ struct mls_level *level;
+ int match = 0;
+
+ if (!rule || !ss_initialized)
+ return 0;
+
+ POLICY_RDLOCK;
+
+ if (aurule->au_seqno < latest_granting) {
+ context_destroy(&aurule->au_ctxt);
+ aurule->au_seqno = latest_granting;
+ aurule_init_context(aurule);
+ }
+
+ if (aurule->au_skip)
+ goto out;
+
+ ctxt = sidtab_search(&sidtab, ctxid);
+ if (!ctxt) {
+ /* TODO: what to do? */
+ printk(KERN_ERR "security_aurule_match: unrecognized SID %d\n",
+ ctxid);
+ match = -EINVAL;
+ goto out;
+ }
+
+ switch (aurule->au_field) {
+ case AUDIT_SE_USER:
+ switch (aurule->au_op) {
+ case AUDIT_EQUAL:
+ match = (ctxt->user == aurule->au_ctxt.user);
+ break;
+ case AUDIT_NOT_EQUAL:
+ match = (ctxt->user != aurule->au_ctxt.user);
+ break;
+ default:
+ match = -EINVAL;
+ break;
+ }
+ break;
+ case AUDIT_SE_ROLE:
+ switch (aurule->au_op) {
+ case AUDIT_EQUAL:
+ match = (ctxt->role == aurule->au_ctxt.role);
+ break;
+ case AUDIT_NOT_EQUAL:
+ match = (ctxt->role != aurule->au_ctxt.role);
+ break;
+ default:
+ match = -EINVAL;
+ break;
+ }
+ break;
+ case AUDIT_SE_TYPE:
+ switch (aurule->au_op) {
+ case AUDIT_EQUAL:
+ match = (ctxt->type == aurule->au_ctxt.type);
+ break;
+ case AUDIT_NOT_EQUAL:
+ match = (ctxt->type != aurule->au_ctxt.type);
+ break;
+ default:
+ match = -EINVAL;
+ break;
+ }
+ break;
+ case AUDIT_SE_SEN:
+ case AUDIT_SE_CLR:
+ level = (aurule->au_op == AUDIT_SE_SEN ?
+ &ctxt->range.level[0] : &ctxt->range.level[1]);
+ switch (aurule->au_op) {
+ case AUDIT_EQUAL:
+ match = mls_level_eq(&aurule->au_ctxt.range.level[0],
+ level);
+ break;
+ case AUDIT_NOT_EQUAL:
+ match = !mls_level_eq(&aurule->au_ctxt.range.level[0],
+ level);
+ break;
+ case AUDIT_LESS_THAN:
+ match = mls_level_dom(&aurule->au_ctxt.range.level[0],
+ level);
+ break;
+ case AUDIT_LESS_THAN_OR_EQUAL:
+ match = (mls_level_eq(&aurule->au_ctxt.range.level[0],
+ level) ||
+ mls_level_dom(&aurule->au_ctxt.range.level[0],
+ level));
+ break;
+ case AUDIT_GREATER_THAN:
+ match = mls_level_dom(level,
+ &aurule->au_ctxt.range.level[0]);
+ break;
+ case AUDIT_GREATER_THAN_OR_EQUAL:
+ match = (mls_level_eq(&aurule->au_ctxt.range.level[0],
+ level) ||
+ mls_level_dom(level,
+ &aurule->au_ctxt.range.level[0]));
+ break;
+ default:
+ match = -EINVAL;
+ break;
+ }
+ default:
+ match = -EINVAL;
+ break;
+ }
+
+out:
+ POLICY_RDUNLOCK;
+ return match;
+}
--
Darrel
Stephen Smalley
2006-02-16 18:12:43 UTC
Permalink
Post by Darrel Goeddel
This is still a work in progress (I'm guessing that the conversion of Dustin's
earlier work will point out some improvements to these interfaces). I also
need to check the context of memory allocations.
Needs to be GFP_ATOMIC when the policy rdlock is held.
Post by Darrel Goeddel
I'm only allow == and != for type, role, and user because they seemed to be
the only ones that make sense, is that OK, or should I take them all even
though they may not do anything useful? Does the general approach seem acceptable?
Role dominance is another possibility, although I don't know if it would
be used in practice. Did you consider trying to leverage or factor
common code with constraint_expr_eval?
Post by Darrel Goeddel
+int selinux_audit_rule_init(u32 field, u32 op, const char *rulestr,
+ void **rule);
We could alternatively define an opaque struct type for the rule, and
just keep the definition private to SELinux. That allows proper type
checking in the audit code when they are passed around and stored.
Post by Darrel Goeddel
+int selinux_task_getsecid(struct task_struct *tsk);
SID is an unsigned 32-bit quantity, but you return a signed int. And
typically it seems that it is preferred to return-by-argument and leave
the return value as either an integer error status (0 or -errno) or void
if no errors are possible.
Post by Darrel Goeddel
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index d877cd1..480df81 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
POLICY_RDUNLOCK;
return rc;
}
+
+struct selinux_audit_rule {
+ u32 au_skip;
Not sure I understand when skipped rules get re-processed.
Post by Darrel Goeddel
+static void aurule_init_context(struct selinux_audit_rule *aurule)
<snip>
Post by Darrel Goeddel
+ if (rc) {
+ aurule->au_skip = 1;
+ context_destroy(&aurule->au_ctxt);
+ } else {
+ /* we merely flags this rule to not be processed - the role,
+ user, type, or level of the rule may not be valid now, but
+ may be after a future policy reload. */
+ aurule->au_skip = 0;
+ }
Hmm..seems like you'd want an error status returned all the way to
userspace (auditctl) so that the user knows it isn't active.
Post by Darrel Goeddel
+ if (!ss_initialized) {
+ tmprule->au_seqno = latest_granting;
+ tmprule->au_skip = 1;
+ *rule = tmprule;
+ return 0;
+ }
I think we should just reject audit filters on contexts until policy is
loaded, and not try to defer them.
Post by Darrel Goeddel
+int security_aurule_match(u32 ctxid, void *rule)
+{
<snip>
Post by Darrel Goeddel
+ POLICY_RDLOCK;
+
+ if (aurule->au_seqno < latest_granting) {
+ context_destroy(&aurule->au_ctxt);
+ aurule->au_seqno = latest_granting;
+ aurule_init_context(aurule);
+ }
Interesting approach; I was expecting to have the audit system register
an AVC callback for reloads (similar to netif table) and initiate the
re-processing of its audit rules at that time. And simply fail on
filters with stale seqnos if there happened to be an interleaving with
the policy reload. I suppose that this is more robust.
Post by Darrel Goeddel
+ if (aurule->au_skip)
+ goto out;
Ok, but when does the skip flag ever get cleared?
Post by Darrel Goeddel
+ ctxt = sidtab_search(&sidtab, ctxid);
+ if (!ctxt) {
+ /* TODO: what to do? */
+ printk(KERN_ERR "security_aurule_match: unrecognized SID %d\n",
+ ctxid);
+ match = -EINVAL;
+ goto out;
+ }
Unlikely since sidtab_search remaps invalid SIDs to unlabeled to deal
with invalidated SIDs due to reloads.
Post by Darrel Goeddel
+ match = (mls_level_eq(&aurule->au_ctxt.range.level[0],
+ level) ||
+ mls_level_dom(&aurule->au_ctxt.range.level[0],
+ level));
+ break;
Isn't checking both redundant? Did you mean to force !mls_level_eq in
the LESS_THAN case?
Post by Darrel Goeddel
+ match = mls_level_dom(level,
+ &aurule->au_ctxt.range.level[0]);
+ break;
+ match = (mls_level_eq(&aurule->au_ctxt.range.level[0],
+ level) ||
+ mls_level_dom(level,
+ &aurule->au_ctxt.range.level[0]));
+ break;
Ditto.
--
Stephen Smalley
National Security Agency
Darrel Goeddel
2006-02-16 20:09:04 UTC
Permalink
Post by Stephen Smalley
Post by Darrel Goeddel
This is still a work in progress (I'm guessing that the conversion of Dustin's
earlier work will point out some improvements to these interfaces). I also
need to check the context of memory allocations.
Needs to be GFP_ATOMIC when the policy rdlock is held.
Thanks. Now I have to figure out how to propogate ENOMEM...
Post by Stephen Smalley
Post by Darrel Goeddel
I'm only allow == and != for type, role, and user because they seemed to be
the only ones that make sense, is that OK, or should I take them all even
though they may not do anything useful? Does the general approach seem acceptable?
Role dominance is another possibility, although I don't know if it would
be used in practice. Did you consider trying to leverage or factor
common code with constraint_expr_eval?
True on role dominance. No on constraint_expr_eval - I'll give it a look.
Post by Stephen Smalley
Post by Darrel Goeddel
+int selinux_audit_rule_init(u32 field, u32 op, const char *rulestr,
+ void **rule);
We could alternatively define an opaque struct type for the rule, and
just keep the definition private to SELinux. That allows proper type
checking in the audit code when they are passed around and stored.
Post by Darrel Goeddel
+int selinux_task_getsecid(struct task_struct *tsk);
SID is an unsigned 32-bit quantity, but you return a signed int. And
typically it seems that it is preferred to return-by-argument and leave
the return value as either an integer error status (0 or -errno) or void
if no errors are possible.
I'll change to:

void selinux_task_ctxid(struct task_struct *tsk, u32 *ctxid)

Thats in-line with James' selinux_sk_ctxid
Post by Stephen Smalley
Post by Darrel Goeddel
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index d877cd1..480df81 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
POLICY_RDUNLOCK;
return rc;
}
+
+struct selinux_audit_rule {
+ u32 au_skip;
Not sure I understand when skipped rules get re-processed.
I'll add some comments for that. A rule will get reprocessed when a new
policy is loaded (triggered by the seqno check in security_aurule_match).
That will then reset the au_skip field appropriately. The idea behind this
is to allow a rule to be present for a type, role, etc. that does not exist
in the current policy, but they are never processed. If the item is included
in a policy that is loaded later, the rule will be setup completely and
activated (au_skip = 0). Conversely, if an item disappears, the rule is
just inactivated.

This give the same behavior as the other audit rule fields. For example, a
rule can be there for a uid that is not used on the system and it will simply
never match. If that uid is later used on the system, the rule will then be
used. Same goes for inums that aren't used and probably most other fields...
Post by Stephen Smalley
Post by Darrel Goeddel
+static void aurule_init_context(struct selinux_audit_rule *aurule)
<snip>
Post by Darrel Goeddel
+ if (rc) {
+ aurule->au_skip = 1;
+ context_destroy(&aurule->au_ctxt);
+ } else {
+ /* we merely flags this rule to not be processed - the role,
+ user, type, or level of the rule may not be valid now, but
+ may be after a future policy reload. */
+ aurule->au_skip = 0;
+ }
Hmm..seems like you'd want an error status returned all the way to
userspace (auditctl) so that the user knows it isn't active.
The above comment hopefully explains my thinking on this one. Steve Grubb had
the same question and seemed to buy into my explanation.
Post by Stephen Smalley
Post by Darrel Goeddel
+ if (!ss_initialized) {
+ tmprule->au_seqno = latest_granting;
+ tmprule->au_skip = 1;
+ *rule = tmprule;
+ return 0;
+ }
I think we should just reject audit filters on contexts until policy is
loaded, and not try to defer them.
True. My previous "testing" did add rules before policy load with an __initcall in
the audit module - but I somehow don't think that will be standard practice...
Post by Stephen Smalley
Post by Darrel Goeddel
+int security_aurule_match(u32 ctxid, void *rule)
+{
<snip>
Post by Darrel Goeddel
+ POLICY_RDLOCK;
+
+ if (aurule->au_seqno < latest_granting) {
+ context_destroy(&aurule->au_ctxt);
+ aurule->au_seqno = latest_granting;
+ aurule_init_context(aurule);
+ }
Interesting approach; I was expecting to have the audit system register
an AVC callback for reloads (similar to netif table) and initiate the
re-processing of its audit rules at that time. And simply fail on
filters with stale seqnos if there happened to be an interleaving with
the policy reload. I suppose that this is more robust.
I was hoping you'd agree on this one. This idea seemed much simpler to me and
I think it avoids quite a bit of extra code for the rule rebuilding.
Post by Stephen Smalley
Post by Darrel Goeddel
+ if (aurule->au_skip)
+ goto out;
Ok, but when does the skip flag ever get cleared?
See above comment on rule re-processing.
Post by Stephen Smalley
Post by Darrel Goeddel
+ ctxt = sidtab_search(&sidtab, ctxid);
+ if (!ctxt) {
+ /* TODO: what to do? */
+ printk(KERN_ERR "security_aurule_match: unrecognized SID %d\n",
+ ctxid);
+ match = -EINVAL;
+ goto out;
+ }
Unlikely since sidtab_search remaps invalid SIDs to unlabeled to deal
with invalidated SIDs due to reloads.
True. I'm thinking this is a pretty bad one since it really shouldn't happen.
I'll have to see how the user intends to handle the return of this function
before a final patch will be ready.
Post by Stephen Smalley
Post by Darrel Goeddel
+ match = (mls_level_eq(&aurule->au_ctxt.range.level[0],
+ level) ||
+ mls_level_dom(&aurule->au_ctxt.range.level[0],
+ level));
+ break;
Isn't checking both redundant? Did you mean to force !mls_level_eq in
the LESS_THAN case?
Post by Darrel Goeddel
+ match = mls_level_dom(level,
+ &aurule->au_ctxt.range.level[0]);
+ break;
+ match = (mls_level_eq(&aurule->au_ctxt.range.level[0],
+ level) ||
+ mls_level_dom(level,
+ &aurule->au_ctxt.range.level[0]));
+ break;
Ditto.
Yep to both. I was thinking of the old mls_level_realtion function that would return
EQ before DOM... You think I'd know better on that one ;)

Thanks for the feedback. Next version will incorporate the changes.
--
Darrel
Stephen Smalley
2006-02-16 20:36:45 UTC
Permalink
Post by Darrel Goeddel
True on role dominance. No on constraint_expr_eval - I'll give it a look.
Note that we'd still want separate audit representation (so that we
don't expose the constraint representation directly to audit), but could
convert it to constraint form during rule setup and then just feed it to
constraint_expr_eval or some common helper.
Post by Darrel Goeddel
I'll add some comments for that. A rule will get reprocessed when a new
policy is loaded (triggered by the seqno check in security_aurule_match).
That will then reset the au_skip field appropriately. The idea behind this
is to allow a rule to be present for a type, role, etc. that does not exist
in the current policy, but they are never processed. If the item is included
in a policy that is loaded later, the rule will be setup completely and
activated (au_skip = 0). Conversely, if an item disappears, the rule is
just inactivated.
This give the same behavior as the other audit rule fields. For example, a
rule can be there for a uid that is not used on the system and it will simply
never match. If that uid is later used on the system, the rule will then be
used. Same goes for inums that aren't used and probably most other fields...
True, but seems prone to error, e.g. user typo on a name or user using
an obsolete name never knows he made a mistake. Unless auditctl itself
does validity checking, which requires it to interact with
libsemanage/libsepol.
--
Stephen Smalley
National Security Agency
Darrel Goeddel
2006-02-17 14:43:58 UTC
Permalink
Post by Stephen Smalley
Post by Darrel Goeddel
True on role dominance. No on constraint_expr_eval - I'll give it a look.
Note that we'd still want separate audit representation (so that we
don't expose the constraint representation directly to audit), but could
convert it to constraint form during rule setup and then just feed it to
constraint_expr_eval or some common helper.
Post by Darrel Goeddel
I'll add some comments for that. A rule will get reprocessed when a new
policy is loaded (triggered by the seqno check in security_aurule_match).
That will then reset the au_skip field appropriately. The idea behind this
is to allow a rule to be present for a type, role, etc. that does not exist
in the current policy, but they are never processed. If the item is included
in a policy that is loaded later, the rule will be setup completely and
activated (au_skip = 0). Conversely, if an item disappears, the rule is
just inactivated.
This give the same behavior as the other audit rule fields. For example, a
rule can be there for a uid that is not used on the system and it will simply
never match. If that uid is later used on the system, the rule will then be
used. Same goes for inums that aren't used and probably most other fields...
True, but seems prone to error, e.g. user typo on a name or user using
an obsolete name never knows he made a mistake. Unless auditctl itself
does validity checking, which requires it to interact with
libsemanage/libsepol.
Yes it does, but that seems to be the nature of the beast with the audit rules.
If we perform the validity check at this level, we may bonk rules that we really
do want to enforce. There is also the question of how to handle a situation
like the following:

- we have an audit filter set up for matching the role darrel_r
- we load a policy that does not have darrel_r
- we reload the original policy

In the current scheme, the rule would simply become inactive when the policy
does not contain the target, and it would be reactivated when a policy that
did contain the target was loaded. We would never miss an action involving
darrel_r.

If we needed to toss out the darrel_r rule because it became invalid, later
operations involving darrel_r would go unaudited unless the audit rules
are reloaded along with a policy reload. Do we want to marry policy loads
with audit filter loads (and how would we know which filter to load anyway)?

It would seem to me that we need the current functionality of keeping all rules
that are set up and revalidating them upon policy loads. If we don't do it here,
it would need to be done at the audit layer - it might not be as pretty there.

As the policy becomes more dynamic (adding/removing policy modules, etc.) this
would become more of a problem.

Anybody else have an idea on this (Steve G)? Am I being to paranoid about usage?
--
Darrel
Dustin Kirkland
2006-02-17 16:04:27 UTC
Permalink
Post by Darrel Goeddel
It would seem to me that we need the current functionality of keeping all rules
that are set up and revalidating them upon policy loads. If we don't do it here,
it would need to be done at the audit layer - it might not be as pretty there.
I don't know... My first thoughts are that it seems like the audit
layer should be ignorant of policy loads/reloads--that's not really it's
business.


:-Dustin
Darrel Goeddel
2006-02-17 16:23:38 UTC
Permalink
Post by Dustin Kirkland
Post by Darrel Goeddel
It would seem to me that we need the current functionality of keeping all rules
that are set up and revalidating them upon policy loads. If we don't do it here,
it would need to be done at the audit layer - it might not be as pretty there.
I don't know... My first thoughts are that it seems like the audit
layer should be ignorant of policy loads/reloads--that's not really it's
business.
I was trying to avoid knowledge of policy reloads (and really policy in general)
with the rule skipping piece and the sequo checking when the rule is used. As
far as the audit system is concerned, SELinux could be rolling dice to determine
matches.

As for the big picture. Steve Grubb suggested a compromise of keeping the current
methodology and enhancing it with KERN_INFO messages. I'd suggest message for the
following:

- a rule that is inactive is inserted
- a rule that was active becomes inactive
- a rule that was inactive becomes active

Does that fit the bill?
--
Darrel
Stephen Smalley
2006-02-17 18:26:29 UTC
Permalink
Post by Dustin Kirkland
Post by Darrel Goeddel
It would seem to me that we need the current functionality of keeping all rules
that are set up and revalidating them upon policy loads. If we don't do it here,
it would need to be done at the audit layer - it might not be as pretty there.
I don't know... My first thoughts are that it seems like the audit
layer should be ignorant of policy loads/reloads--that's not really it's
business.
Disagree - it is caching policy information, and thus should register a
callback for notification of reloads so that it can re-process its audit
rules at that time, similar to the netif table. That would presumably
address the locking concern as well.
--
Stephen Smalley
National Security Agency
Darrel Goeddel
2006-02-16 22:52:48 UTC
Permalink
OK, I lied. The next version (below) only has interface changes.
None of the other changes are in yet. I'll have another version
tomorrow for that...

This has the new interface for getting task sids - selinux_task_ctxid().
It also gets rid of using void pointers for audit rules in the interface.
Stephen, was I on the same page as you with that change?

This should solidify the interface so Dustin can start playing with it.


diff --git a/include/linux/audit.h b/include/linux/audit.h
index 4bb4b9f..dd4f759 100644
--- 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_SEN 16 /* security label sensitivity label */
+#define AUDIT_SE_CLR 17 /* security label clearance label */

/* These are ONLY useful when checking
* at syscall exit time (AUDIT_AT_EXIT). */
diff --git a/include/linux/selinux.h b/include/linux/selinux.h
new file mode 100644
index 0000000..822a177
--- /dev/null
+++ b/include/linux/selinux.h
@@ -0,0 +1,89 @@
+/*
+ * SELinux services exported to the rest of the kernel.
+ *
+ * Author: James Morris <***@redhat.com>
+ *
+ * Copyright (C) 2005 Red Hat, Inc., James Morris <***@redhat.com>
+ * Copyright (C) 2006 Trusted Computer Solutions <***@trustedcs.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2,
+ * as published by the Free Software Foundation.
+ */
+#ifndef _LINUX_SELINUX_H
+#define _LINUX_SELINUX_H
+
+#ifdef CONFIG_SECURITY_SELINUX
+
+struct selinux_audit_rule;
+
+/**
+ * selinux_audit_rule_init - alloc/init an selinux audit rule structure.
+ * @field: the field this rule refers to
+ * @op: the operater the rule uses
+ * @rulestr: the text "target" of the rule
+ * @rule: address of the rule structure pointer to be returned
+ *
+ * Returns 0 if successful, -errno if not. On success, the rule structure
+ * will be allocated internally. The caller must free this structure with
+ * selinux_audit_rule_free() after use.
+ */
+int selinux_audit_rule_init(u32 field, u32 op, const char *rulestr,
+ struct selinux_audit_rule **rule);
+
+/**
+ * selinux_audit_rule_free - free an selinux audit rule structure.
+ * @rule: address of the rule structure to be freed
+ *
+ * This will free all memory associated with the given rule.
+ */
+void selinux_audit_rule_free(struct selinux_audit_rule *rule);
+
+/**
+ * selinux_audit_rule_match - determine if a context ID matches a rule.
+ * @ctxid: the context ID to check
+ * @rule: the audit rule created by selinux_audit_rule_init()
+ *
+ * Returns 1 if the context id matches the rule, 0 if it does not, and
+ * -errno on failure.
+ */
+int selinux_audit_rule_match(u32 ctxid, struct selinux_audit_rule *rule);
+
+/**
+ * selinux_task_ctxid - determine a context ID for a process.
+ * @tsk: the task object
+ * @ctxid: ID value returned via this
+ *
+ * On return, ctxid will contain an ID for the context. This value
+ * should only be used opaquely.
+ */
+void selinux_task_ctxid(struct task_struct *tsk, u32 *ctxid);
+
+#else
+
+static inline int selinux_audit_rule_init(u32 field, u32 op,
+ const char *rulestr,
+ struct selinux_audit_rule **rule)
+{
+ return -ENOTSUPP;
+}
+
+static inline void selinux_audit_rule_free(struct selinux_audit_rule *rule)
+{
+ return;
+}
+
+static inline int selinux_audit_rule_match(u32 ctxid,
+ struct selinux_audit_rule *rule)
+{
+ return 0;
+}
+
+static inline void selinux_task_ctxid(struct task_struct *tsk, u32 *ctxid)
+{
+ *ctxid = 0;
+}
+
+#endif /* CONFIG_SECURITY_SELINUX */
+
+#endif /* _LINUX_SELINUX_H */
diff --git a/security/selinux/Makefile b/security/selinux/Makefile
index 688c0a2..faf2e02 100644
--- a/security/selinux/Makefile
+++ b/security/selinux/Makefile
@@ -4,7 +4,7 @@

obj-$(CONFIG_SECURITY_SELINUX) := selinux.o ss/

-selinux-y := avc.o hooks.o selinuxfs.o netlink.o nlmsgtab.o netif.o
+selinux-y := avc.o hooks.o selinuxfs.o netlink.o nlmsgtab.o netif.o exports.o

selinux-$(CONFIG_SECURITY_NETWORK_XFRM) += xfrm.o

diff --git a/security/selinux/exports.c b/security/selinux/exports.c
new file mode 100644
index 0000000..56e13ac
--- /dev/null
+++ b/security/selinux/exports.c
@@ -0,0 +1,45 @@
+/*
+ * SELinux services exported to the rest of the kernel.
+ *
+ * Author: James Morris <***@redhat.com>
+ *
+ * Copyright (C) 2005 Red Hat, Inc., James Morris <***@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2,
+ * as published by the Free Software Foundation.
+ */
+#include <linux/types.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/selinux.h>
+
+#include "security.h"
+#include "objsec.h"
+
+int selinux_audit_rule_init(u32 field, u32 op, const char *rulestr,
+ struct selinux_audit_rule **rule)
+{
+ return security_aurule_init(field, op, rulestr, rule);
+}
+
+void selinux_audit_rule_free(struct selinux_audit_rule *rule)
+{
+ return security_aurule_free(rule);
+}
+
+int selinux_audit_rule_match(u32 ctxid, struct selinux_audit_rule *rule)
+{
+ return security_aurule_match(ctxid, rule);
+}
+
+void selinux_task_ctxid(struct task_struct *tsk, u32 *ctxid)
+{
+ struct task_security_struct *tsec = tsk->security;
+ *ctxid = tsec->sid;
+}
+
+EXPORT_SYMBOL_GPL(selinux_audit_rule_init);
+EXPORT_SYMBOL_GPL(selinux_audit_rule_free);
+EXPORT_SYMBOL_GPL(selinux_audit_rule_match);
+EXPORT_SYMBOL_GPL(selinux_task_ctxid);
diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
index 5f016c9..51a719e 100644
--- a/security/selinux/include/security.h
+++ b/security/selinux/include/security.h
@@ -96,5 +96,12 @@ int security_fs_use(const char *fstype,
int security_genfs_sid(const char *fstype, char *name, u16 sclass,
u32 *sid);

+struct selinux_audit_rule;
+
+int security_aurule_init(u32 field, u32 op, const char *rulestr,
+ struct selinux_audit_rule **rule);
+void security_aurule_free(struct selinux_audit_rule *rule);
+int security_aurule_match(u32 ctxid, struct selinux_audit_rule *rule);
+
#endif /* _SELINUX_SECURITY_H_ */

diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index d877cd1..48f0b85 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -1810,3 +1810,248 @@ out:
POLICY_RDUNLOCK;
return rc;
}
+
+struct selinux_audit_rule {
+ u32 au_skip;
+ u32 au_op;
+ u32 au_field;
+ u32 au_seqno;
+ struct context au_ctxt;
+ char *au_str;
+};
+
+/* needs policy read lock held */
+static void aurule_init_context(struct selinux_audit_rule *rule)
+{
+ struct role_datum *roledatum;
+ struct type_datum *typedatum;
+ struct user_datum *userdatum;
+ char *tmpstr;
+ int rc = 0;
+
+ switch (rule->au_field) {
+ case AUDIT_SE_USER:
+ userdatum = hashtab_search(policydb.p_users.table,
+ rule->au_str);
+ if (!userdatum)
+ rc = -EINVAL;
+ else
+ rule->au_ctxt.user = userdatum->value;
+ break;
+ case AUDIT_SE_ROLE:
+ roledatum = hashtab_search(policydb.p_roles.table,
+ rule->au_str);
+ if (!roledatum)
+ rc = -EINVAL;
+ else
+ rule->au_ctxt.role = roledatum->value;
+ break;
+ case AUDIT_SE_TYPE:
+ typedatum = hashtab_search(policydb.p_types.table,
+ rule->au_str);
+ if (!typedatum)
+ rc = -EINVAL;
+ else
+ rule->au_ctxt.type = typedatum->value;
+ break;
+ case AUDIT_SE_SEN:
+ case AUDIT_SE_CLR:
+ /* TODO:figure out proper allocation below */
+ tmpstr = kstrdup(rule->au_str, GFP_KERNEL);
+ rc = mls_context_to_sid(':', &tmpstr, &rule->au_ctxt, NULL,
+ SECSID_NULL);
+ kfree(tmpstr);
+ break;
+ default:
+ rc = -EINVAL;
+ break;
+ }
+
+ if (rc) {
+ rule->au_skip = 1;
+ context_destroy(&rule->au_ctxt);
+ } else {
+ /* we merely flags this rule to not be processed - the role,
+ user, type, or level of the rule may not be valid now, but
+ may be after a future policy reload. */
+ rule->au_skip = 0;
+ }
+
+ return;
+}
+
+int security_aurule_init(u32 field, u32 op, const char *rulestr,
+ struct selinux_audit_rule **rule)
+{
+ struct selinux_audit_rule *tmprule;
+
+ *rule = NULL;
+
+ switch (field) {
+ case AUDIT_SE_USER:
+ case AUDIT_SE_ROLE:
+ case AUDIT_SE_TYPE:
+ /* only 'equals' and 'not equals' make sense */
+ if (op != AUDIT_EQUAL && op != AUDIT_NOT_EQUAL)
+ return -EINVAL;
+ case AUDIT_SE_SEN:
+ case AUDIT_SE_CLR:
+ /* we do not allow a range, indicated by '-' */
+ if (strchr(rulestr, '-'))
+ return -EINVAL;
+ }
+
+ /* TODO:figure out proper allocations below */
+ tmprule = kzalloc(sizeof(struct selinux_audit_rule), GFP_KERNEL);
+ if (!tmprule)
+ return -ENOMEM;
+ tmprule->au_str = kstrdup(rulestr, GFP_KERNEL);
+ if (!tmprule->au_str) {
+ kfree(tmprule);
+ return -ENOMEM;
+ }
+ tmprule->au_op = op;
+ tmprule->au_field = field;
+ context_init(&tmprule->au_ctxt);
+
+ if (!ss_initialized) {
+ tmprule->au_seqno = latest_granting;
+ tmprule->au_skip = 1;
+ *rule = tmprule;
+ return 0;
+ }
+
+ POLICY_RDLOCK;
+
+ tmprule->au_seqno = latest_granting;
+ aurule_init_context(tmprule);
+
+ POLICY_RDUNLOCK;
+
+ *rule = tmprule;
+
+ return 0;
+}
+
+void security_aurule_free(struct selinux_audit_rule *rule)
+{
+ kfree(rule->au_str);
+ context_destroy(&rule->au_ctxt);
+ kfree(rule);
+}
+
+int security_aurule_match(u32 ctxid, struct selinux_audit_rule *rule)
+{
+ struct context *ctxt;
+ struct mls_level *level;
+ int match = 0;
+
+ if (!rule || !ss_initialized)
+ return 0;
+
+ POLICY_RDLOCK;
+
+ if (rule->au_seqno < latest_granting) {
+ context_destroy(&rule->au_ctxt);
+ rule->au_seqno = latest_granting;
+ aurule_init_context(rule);
+ }
+
+ if (rule->au_skip)
+ goto out;
+
+ ctxt = sidtab_search(&sidtab, ctxid);
+ if (!ctxt) {
+ /* TODO: what to do? */
+ printk(KERN_ERR "security_aurule_match: unrecognized SID %d\n",
+ ctxid);
+ match = -EINVAL;
+ goto out;
+ }
+
+ switch (rule->au_field) {
+ case AUDIT_SE_USER:
+ switch (rule->au_op) {
+ case AUDIT_EQUAL:
+ match = (ctxt->user == rule->au_ctxt.user);
+ break;
+ case AUDIT_NOT_EQUAL:
+ match = (ctxt->user != rule->au_ctxt.user);
+ break;
+ default:
+ match = -EINVAL;
+ break;
+ }
+ break;
+ case AUDIT_SE_ROLE:
+ switch (rule->au_op) {
+ case AUDIT_EQUAL:
+ match = (ctxt->role == rule->au_ctxt.role);
+ break;
+ case AUDIT_NOT_EQUAL:
+ match = (ctxt->role != rule->au_ctxt.role);
+ break;
+ default:
+ match = -EINVAL;
+ break;
+ }
+ break;
+ case AUDIT_SE_TYPE:
+ switch (rule->au_op) {
+ case AUDIT_EQUAL:
+ match = (ctxt->type == rule->au_ctxt.type);
+ break;
+ case AUDIT_NOT_EQUAL:
+ match = (ctxt->type != rule->au_ctxt.type);
+ break;
+ default:
+ match = -EINVAL;
+ break;
+ }
+ break;
+ case AUDIT_SE_SEN:
+ case AUDIT_SE_CLR:
+ level = (rule->au_op == AUDIT_SE_SEN ?
+ &ctxt->range.level[0] : &ctxt->range.level[1]);
+ switch (rule->au_op) {
+ case AUDIT_EQUAL:
+ match = mls_level_eq(&rule->au_ctxt.range.level[0],
+ level);
+ break;
+ case AUDIT_NOT_EQUAL:
+ match = !mls_level_eq(&rule->au_ctxt.range.level[0],
+ level);
+ break;
+ case AUDIT_LESS_THAN:
+ match = mls_level_dom(&rule->au_ctxt.range.level[0],
+ level);
+ break;
+ case AUDIT_LESS_THAN_OR_EQUAL:
+ match = (mls_level_eq(&rule->au_ctxt.range.level[0],
+ level) ||
+ mls_level_dom(&rule->au_ctxt.range.level[0],
+ level));
+ break;
+ case AUDIT_GREATER_THAN:
+ match = mls_level_dom(level,
+ &rule->au_ctxt.range.level[0]);
+ break;
+ case AUDIT_GREATER_THAN_OR_EQUAL:
+ match = (mls_level_eq(&rule->au_ctxt.range.level[0],
+ level) ||
+ mls_level_dom(level,
+ &rule->au_ctxt.range.level[0]));
+ break;
+ default:
+ match = -EINVAL;
+ break;
+ }
+ default:
+ match = -EINVAL;
+ break;
+ }
+
+out:
+ POLICY_RDUNLOCK;
+ return match;
+}
--
Darrel
Amy Griffis
2006-02-17 16:49:33 UTC
Permalink
Hi Darrel,
Post by Darrel Goeddel
Post by Stephen Smalley
Post by Darrel Goeddel
+int security_aurule_match(u32 ctxid, void *rule)
+{
<snip>
Post by Darrel Goeddel
+ POLICY_RDLOCK;
+
+ if (aurule->au_seqno < latest_granting) {
+ context_destroy(&aurule->au_ctxt);
+ aurule->au_seqno = latest_granting;
+ aurule_init_context(aurule);
+ }
Interesting approach; I was expecting to have the audit system register
an AVC callback for reloads (similar to netif table) and initiate the
re-processing of its audit rules at that time. And simply fail on
filters with stale seqnos if there happened to be an interleaving with
the policy reload. I suppose that this is more robust.
I was hoping you'd agree on this one. This idea seemed much simpler
to me and I think it avoids quite a bit of extra code for the rule
rebuilding.
I don't think it can be that simple. Historically, audit filter
operations have been read-only. Rcu is used because waiting on any
other kind of lock would be a bottleneck to the syscall path.

You are introducing a write operation to part of the filterlist while
we are only holding read locks (rcu_read_lock() in auditsc.c and
POLICY_RDLOCK here). This could conflict with other readers and
writers of this data.

One option is to introduce a field-specific lock. When audit rules
are configured such that the field applies to only a few syscalls,
then syscall processing isn't affected very much. However, we can't
dictate that audit rules are written in this way, so I doubt we can
make a case for this.

The other option requires audit to be aware of the update so it can
make a new copy of the rule and do a list_replace_rcu(). This
shouldn't happen during filtering though.

I'm of the opinion that syscall filtering should remain read-only.
Anything else isn't going to scale well.

Regards,
Amy
Stephen Smalley
2006-02-17 18:27:53 UTC
Permalink
Post by Amy Griffis
I don't think it can be that simple. Historically, audit filter
operations have been read-only. Rcu is used because waiting on any
other kind of lock would be a bottleneck to the syscall path.
You are introducing a write operation to part of the filterlist while
we are only holding read locks (rcu_read_lock() in auditsc.c and
POLICY_RDLOCK here). This could conflict with other readers and
writers of this data.
One option is to introduce a field-specific lock. When audit rules
are configured such that the field applies to only a few syscalls,
then syscall processing isn't affected very much. However, we can't
dictate that audit rules are written in this way, so I doubt we can
make a case for this.
The other option requires audit to be aware of the update so it can
make a new copy of the rule and do a list_replace_rcu(). This
shouldn't happen during filtering though.
I'm of the opinion that syscall filtering should remain read-only.
Anything else isn't going to scale well.
Yes, thanks for pointing this out. Which means that audit needs to
register a callback for policy reloads and re-process the audit filters
upon the reload operation itself, not at filter check time.
--
Stephen Smalley
National Security Agency
Darrel Goeddel
2006-02-21 21:32:13 UTC
Permalink
Thanks for everyone's feedback on the original patch. This is basically
a rewrite that does the following:

1) Add a callback mechanism for the audit facility to rebuild the selinux
audit filters upon policy reload. This callback should re-init the
selinux rules to sync them with the currently loaded policy.
2) Make the matching function read-only. This is accomplished with the help
of #1. If a stale rule (on that does not match the current policy) is
used, the matching function will return an error - I think this should
be treated as a match by audit to avoid potential data loss.
3) The whole "skip" mechanism has been removed (I think this idea should be
retained and implemented in the audit code, for reason previously stated).
This means that -EINVAL is returned for when adding a rule for an item
that are not valid in the currently loaded policy.
5) The private copies of the operator, field, and target string have been
removed. The op and field are now passed in via the matching function and
the string is no longer necessary after #3.
6) Fixed mls comparison logic.
7) Also fixed a bug when processing mls levels.

The outstanding things I can think of are:

1) Add role <, >, <=, and >= operations.
2) Implement the callback function in the audit code. I have a functional
version that does not follow proper locking - I just need to get that
cleaned up.
3) Update Dustin's work to the latest API. I already did this along with
my start at #2. I'll post that all in a follow-up.

Anything else that I forgot on that list?

I believe that this patch should be sufficient and that only additional work
in the selinux code would be to add the role dominance support. I'd say that
we should wait on that until we do the object filter support. I think this
patch is ready to go, barring negative feedback here.
--
Darrel


Signed-off-by: Darrel Goeddel <***@trustedcs.com>


diff --git a/include/linux/audit.h b/include/linux/audit.h
index 4bb4b9f..dd4f759 100644
--- 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_SEN 16 /* security label sensitivity label */
+#define AUDIT_SE_CLR 17 /* security label clearance label */

/* These are ONLY useful when checking
* at syscall exit time (AUDIT_AT_EXIT). */
diff --git a/include/linux/selinux.h b/include/linux/selinux.h
new file mode 100644
index 0000000..7b8cb41
--- /dev/null
+++ b/include/linux/selinux.h
@@ -0,0 +1,108 @@
+/*
+ * SELinux services exported to the rest of the kernel.
+ *
+ * Author: James Morris <***@redhat.com>
+ *
+ * Copyright (C) 2005 Red Hat, Inc., James Morris <***@redhat.com>
+ * Copyright (C) 2006 Trusted Computer Solutions, Inc. <***@trustedcs.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2,
+ * as published by the Free Software Foundation.
+ */
+#ifndef _LINUX_SELINUX_H
+#define _LINUX_SELINUX_H
+
+#ifdef CONFIG_SECURITY_SELINUX
+
+struct selinux_audit_rule;
+
+/**
+ * selinux_audit_rule_init - alloc/init an selinux audit rule structure.
+ * @field: the field this rule refers to
+ * @op: the operater the rule uses
+ * @rulestr: the text "target" of the rule
+ * @rule: pointer to the new rule structure returned via this
+ *
+ * Returns 0 if successful, -errno if not. On success, the rule structure
+ * will be allocated internally. The caller must free this structure with
+ * selinux_audit_rule_free() after use.
+ */
+int selinux_audit_rule_init(u32 field, u32 op, char *rulestr,
+ struct selinux_audit_rule **rule);
+
+/**
+ * selinux_audit_rule_free - free an selinux audit rule structure.
+ * @rule: pointer to the audit rule to be freed
+ *
+ * This will free all memory associated with the given rule.
+ * If @rule is NULL, no operation is performed.
+ */
+void selinux_audit_rule_free(struct selinux_audit_rule *rule);
+
+/**
+ * selinux_audit_rule_match - determine if a context ID matches a rule.
+ * @ctxid: the context ID to check
+ * @field: the field this rule refers to
+ * @op: the operater the rule uses
+ * @rule: pointer to the audit rule to check against
+ *
+ * Returns 1 if the context id matches the rule, 0 if it does not, and
+ * -errno on failure.
+ */
+int selinux_audit_rule_match(u32 ctxid, u32 field, u32 op,
+ struct selinux_audit_rule *rule);
+
+/**
+ * selinux_audit_set_callback - set the callback for policy reloads.
+ * @callback: the function to call when the policy is reloaded
+ *
+ * This sets the function callback function that will update the rules
+ * upon policy reloads. This callback should rebuild all existing rules
+ * using selinux_audit_rule_init().
+ */
+void selinux_audit_set_callback(int (*callback)(void));
+
+/**
+ * selinux_task_ctxid - determine a context ID for a process.
+ * @tsk: the task object
+ * @ctxid: ID value returned via this
+ *
+ * On return, ctxid will contain an ID for the context. This value
+ * should only be used opaquely.
+ */
+void selinux_task_ctxid(struct task_struct *tsk, u32 *ctxid);
+
+#else
+
+static inline int selinux_audit_rule_init(u32 field, u32 op,
+ char *rulestr,
+ struct selinux_audit_rule **rule)
+{
+ return -ENOTSUPP;
+}
+
+static inline void selinux_audit_rule_free(struct selinux_audit_rule *rule)
+{
+ return;
+}
+
+static inline int selinux_audit_rule_match(u32 ctxid, u32 field, u32 op,
+ struct selinux_audit_rule *rule)
+{
+ return 0;
+}
+
+static inline void selinux_audit_set_callback(int (*callback)(void))
+{
+ return;
+}
+
+static inline void selinux_task_ctxid(struct task_struct *tsk, u32 *ctxid)
+{
+ *ctxid = 0;
+}
+
+#endif /* CONFIG_SECURITY_SELINUX */
+
+#endif /* _LINUX_SELINUX_H */
diff --git a/security/selinux/Makefile b/security/selinux/Makefile
index 688c0a2..faf2e02 100644
--- a/security/selinux/Makefile
+++ b/security/selinux/Makefile
@@ -4,7 +4,7 @@

obj-$(CONFIG_SECURITY_SELINUX) := selinux.o ss/

-selinux-y := avc.o hooks.o selinuxfs.o netlink.o nlmsgtab.o netif.o
+selinux-y := avc.o hooks.o selinuxfs.o netlink.o nlmsgtab.o netif.o exports.o

selinux-$(CONFIG_SECURITY_NETWORK_XFRM) += xfrm.o

diff --git a/security/selinux/exports.c b/security/selinux/exports.c
new file mode 100644
index 0000000..5129add
--- /dev/null
+++ b/security/selinux/exports.c
@@ -0,0 +1,48 @@
+/*
+ * SELinux services exported to the rest of the kernel.
+ *
+ * Author: James Morris <***@redhat.com>
+ *
+ * Copyright (C) 2005 Red Hat, Inc., James Morris <***@redhat.com>
+ * Copyright (C) 2006 Trusted Computer Solutions, Inc. <***@trustedcs.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2,
+ * as published by the Free Software Foundation.
+ */
+#include <linux/types.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/selinux.h>
+
+#include "security.h"
+#include "objsec.h"
+
+int selinux_audit_rule_init(u32 field, u32 op, char *rulestr,
+ struct selinux_audit_rule **rule)
+{
+ return security_aurule_init(field, op, rulestr, rule);
+}
+
+void selinux_audit_rule_free(struct selinux_audit_rule *rule)
+{
+ return security_aurule_free(rule);
+}
+
+int selinux_audit_rule_match(u32 ctxid, u32 field, u32 op,
+ struct selinux_audit_rule *rule)
+{
+ return security_aurule_match(ctxid, field, op, rule);
+}
+
+void selinux_audit_set_callback(int (*callback)(void))
+{
+ security_aurule_set_callback(callback);
+}
+
+void selinux_task_ctxid(struct task_struct *tsk, u32 *ctxid)
+{
+ struct task_security_struct *tsec = tsk->security;
+ *ctxid = tsec->sid;
+}
+EXPORT_SYMBOL_GPL(selinux_task_ctxid);
diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
index 5f016c9..bc75a26 100644
--- a/security/selinux/include/security.h
+++ b/security/selinux/include/security.h
@@ -96,5 +96,13 @@ int security_fs_use(const char *fstype,
int security_genfs_sid(const char *fstype, char *name, u16 sclass,
u32 *sid);

+struct selinux_audit_rule;
+int security_aurule_init(u32 field, u32 op, char *rulestr,
+ struct selinux_audit_rule **rule);
+void security_aurule_free(struct selinux_audit_rule *rule);
+int security_aurule_match(u32 ctxid, u32 field, u32 op,
+ struct selinux_audit_rule *rule);
+void security_aurule_set_callback(int (*callback)(void));
+
#endif /* _SELINUX_SECURITY_H_ */

diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index d877cd1..5e05c5a 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -7,12 +7,13 @@
* Updated: Trusted Computer Solutions, Inc. <***@trustedcs.com>
*
* Support for enhanced MLS infrastructure.
+ * Support for context based audit filters.
*
* Updated: Frank Mayer <***@tresys.com> and Karl MacMillan <***@tresys.com>
*
* Added conditional policy language extensions
*
- * Copyright (C) 2004-2005 Trusted Computer Solutions, Inc.
+ * Copyright (C) 2004-2006 Trusted Computer Solutions, Inc.
* Copyright (C) 2003 - 2004 Tresys Technology, LLC
* Copyright (C) 2003 Red Hat, Inc., James Morris <***@redhat.com>
* This program is free software; you can redistribute it and/or modify
@@ -1810,3 +1811,238 @@ out:
POLICY_RDUNLOCK;
return rc;
}
+
+struct selinux_audit_rule {
+ u32 au_seqno;
+ struct context au_ctxt;
+};
+
+int security_aurule_init(u32 field, u32 op, char *rulestr,
+ struct selinux_audit_rule **rule)
+{
+ struct selinux_audit_rule *tmprule;
+ struct role_datum *roledatum;
+ struct type_datum *typedatum;
+ struct user_datum *userdatum;
+ char *tmpstr, *freestr;
+ int rc = 0;
+
+ *rule = NULL;
+
+ if (!ss_initialized)
+ return -ENOTSUPP;
+
+ switch (field) {
+ case AUDIT_SE_USER:
+ case AUDIT_SE_ROLE:
+ case AUDIT_SE_TYPE:
+ /* only 'equals' and 'not equals' fit user, role, and type */
+ if (op != AUDIT_EQUAL && op != AUDIT_NOT_EQUAL)
+ return -EINVAL;
+ break;
+ case AUDIT_SE_SEN:
+ case AUDIT_SE_CLR:
+ /* we do not allow a range, indicated by the presense of '-' */
+ if (strchr(rulestr, '-'))
+ return -EINVAL;
+ break;
+ default:
+ /* only the above fileds are valid */
+ return -EINVAL;
+ }
+
+ tmprule = kzalloc(sizeof(struct selinux_audit_rule), GFP_KERNEL);
+ if (!tmprule)
+ return -ENOMEM;
+
+ context_init(&tmprule->au_ctxt);
+
+ POLICY_RDLOCK;
+
+ tmprule->au_seqno = latest_granting;
+
+ switch (field) {
+ case AUDIT_SE_USER:
+ userdatum = hashtab_search(policydb.p_users.table, rulestr);
+ if (!userdatum)
+ rc = -EINVAL;
+ else
+ tmprule->au_ctxt.user = userdatum->value;
+ break;
+ case AUDIT_SE_ROLE:
+ roledatum = hashtab_search(policydb.p_roles.table, rulestr);
+ if (!roledatum)
+ rc = -EINVAL;
+ else
+ tmprule->au_ctxt.role = roledatum->value;
+ break;
+ case AUDIT_SE_TYPE:
+ typedatum = hashtab_search(policydb.p_types.table, rulestr);
+ if (!typedatum)
+ rc = -EINVAL;
+ else
+ tmprule->au_ctxt.type = typedatum->value;
+ break;
+ case AUDIT_SE_SEN:
+ case AUDIT_SE_CLR:
+ /* we need freestr because mls_context_to_sid will change
+ the value of tmpstr */
+ tmpstr = freestr = kstrdup(rulestr, GFP_ATOMIC);
+ if (!tmpstr) {
+ rc = -ENOMEM;
+ } else {
+ rc = mls_context_to_sid(':', &tmpstr, &tmprule->au_ctxt,
+ NULL, SECSID_NULL);
+ kfree(freestr);
+ }
+ break;
+ }
+
+ POLICY_RDUNLOCK;
+
+ if (rc) {
+ security_aurule_free(tmprule);
+ tmprule = NULL;
+ }
+
+ *rule = tmprule;
+
+ return rc;
+}
+
+void security_aurule_free(struct selinux_audit_rule *rule)
+{
+ if (rule) {
+ context_destroy(&rule->au_ctxt);
+ kfree(rule);
+ }
+}
+
+int security_aurule_match(u32 ctxid, u32 field, u32 op,
+ struct selinux_audit_rule *rule)
+{
+ struct context *ctxt;
+ struct mls_level *level;
+ int match = 0;
+
+ if (!rule)
+ return 0;
+
+ POLICY_RDLOCK;
+
+ if (rule->au_seqno < latest_granting) {
+ match = -ESTALE;
+ goto out;
+ }
+
+ ctxt = sidtab_search(&sidtab, ctxid);
+ if (!ctxt) {
+ printk(KERN_ERR "security_aurule_match: unrecognized SID %d\n",
+ ctxid);
+ match = -EINVAL;
+ goto out;
+ }
+
+ /* a field/op pair that is not caught here will simply fall through
+ without a match */
+ switch (field) {
+ case AUDIT_SE_USER:
+ switch (op) {
+ case AUDIT_EQUAL:
+ match = (ctxt->user == rule->au_ctxt.user);
+ break;
+ case AUDIT_NOT_EQUAL:
+ match = (ctxt->user != rule->au_ctxt.user);
+ break;
+ }
+ break;
+ case AUDIT_SE_ROLE:
+ switch (op) {
+ case AUDIT_EQUAL:
+ match = (ctxt->role == rule->au_ctxt.role);
+ break;
+ case AUDIT_NOT_EQUAL:
+ match = (ctxt->role != rule->au_ctxt.role);
+ break;
+ }
+ break;
+ case AUDIT_SE_TYPE:
+ switch (op) {
+ case AUDIT_EQUAL:
+ match = (ctxt->type == rule->au_ctxt.type);
+ break;
+ case AUDIT_NOT_EQUAL:
+ match = (ctxt->type != rule->au_ctxt.type);
+ break;
+ }
+ break;
+ case AUDIT_SE_SEN:
+ case AUDIT_SE_CLR:
+ level = (op == AUDIT_SE_SEN ?
+ &ctxt->range.level[0] : &ctxt->range.level[1]);
+ switch (op) {
+ case AUDIT_EQUAL:
+ match = mls_level_eq(&rule->au_ctxt.range.level[0],
+ level);
+ break;
+ case AUDIT_NOT_EQUAL:
+ match = !mls_level_eq(&rule->au_ctxt.range.level[0],
+ level);
+ break;
+ case AUDIT_LESS_THAN:
+ match = (mls_level_dom(&rule->au_ctxt.range.level[0],
+ level) &&
+ !mls_level_eq(&rule->au_ctxt.range.level[0],
+ level));
+ break;
+ case AUDIT_LESS_THAN_OR_EQUAL:
+ match = mls_level_dom(&rule->au_ctxt.range.level[0],
+ level);
+ break;
+ case AUDIT_GREATER_THAN:
+ match = (mls_level_dom(level,
+ &rule->au_ctxt.range.level[0]) &&
+ !mls_level_eq(level,
+ &rule->au_ctxt.range.level[0]));
+ break;
+ case AUDIT_GREATER_THAN_OR_EQUAL:
+ match = mls_level_dom(level,
+ &rule->au_ctxt.range.level[0]);
+ break;
+ }
+ }
+
+out:
+ POLICY_RDUNLOCK;
+ return match;
+}
+
+static int (*aurule_callback)(void) = NULL;
+
+static int aurule_avc_callback(u32 event, u32 ssid, u32 tsid,
+ u16 class, u32 perms, u32 *retained)
+{
+ int err = 0;
+
+ if (event == AVC_CALLBACK_RESET && aurule_callback)
+ err = aurule_callback();
+ return err;
+}
+
+static int __init aurule_init(void)
+{
+ int err;
+
+ err = avc_add_callback(aurule_avc_callback, AVC_CALLBACK_RESET,
+ SECSID_NULL, SECSID_NULL, SECCLASS_NULL, 0);
+ if (err)
+ panic("avc_add_callback() failed, error %d\n", err);
+
+ return err;
+}
+__initcall(aurule_init);
+
+void security_aurule_set_callback(int (*callback)(void))
+{
+ aurule_callback = callback;
+}
Darrel Goeddel
2006-02-21 23:59:30 UTC
Permalink
Darrel Goeddel wrote:
...snip...
Post by Darrel Goeddel
1) Add role <, >, <=, and >= operations.
2) Implement the callback function in the audit code. I have a functional
version that does not follow proper locking - I just need to get that
cleaned up.
3) Update Dustin's work to the latest API. I already did this along with
my start at #2. I'll post that all in a follow-up.
The updated version of Dustin's patch I referred to is below. The changes are
are follows:

- Add the se_str char * to the audit_field. This stores a copy of the string
target that the rule is based on. This is needed for the audit_compare_rule
addition (below). It will also be used in the callback for policy reloads
(also below). This is managed along with the se_rule field. This also gets
rid of a memory leak where the unpacked string was not being freed.
- printk a warning and ignore invalid selinux rules (but still hang on to them
so they may be activated with a later policy reload).
- Change audit_compare_rule to compare the string values of the selinux rules
to see if the rule exists. Previously, it thought that any selinux filters
based on equal length string were equal.
- Change audit_krule_to_data to put in the string target of the rule for the
selinux filters. This will hopefully allow the display of the type, level,
etc. when dumping rules via AUDIT_LIST_RULES (I'll test tommorrow).
- Add a selinux callback for re-initializing the se_rule field when there is
a policy reload. THIS NEEDS WORK - It doesn't obey proper locking yet, but
it is functional. I need to get my head around the locking of the audit
structures a little better - I'll also take suggestions ;)
- Obtain the sid of the task in audit_filter_rules instead of the callers
obtaining it and passing it in.


diff --git a/kernel/audit.h b/kernel/audit.h
index eb33354..3ffc70a 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -59,9 +59,11 @@ struct audit_watch {
};

struct audit_field {
- u32 type;
- u32 val;
- u32 op;
+ u32 type;
+ u32 val;
+ u32 op;
+ char *se_str;
+ struct selinux_audit_rule *se_rule;
};

struct audit_krule {
diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index 3712295..306e941 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -25,6 +25,7 @@
#include <linux/fs.h>
#include <linux/namei.h>
#include <linux/netlink.h>
+#include <linux/selinux.h>
#include "audit.h"

/* There are three lists of rules -- one to search at task creation
@@ -50,6 +51,12 @@ static inline void audit_free_watch(stru

static inline void audit_free_rule(struct audit_entry *e)
{
+ int i;
+ for (i = 0; i < e->rule.field_count; i++) {
+ struct audit_field *f = &e->rule.fields[i];
+ kfree(f->se_str);
+ selinux_audit_rule_free(f->se_rule);
+ }
kfree(e->rule.fields);
kfree(e);
}
@@ -222,7 +229,7 @@ static struct audit_entry *audit_data_to
void *bufp;
size_t remain = datasz - sizeof(struct audit_rule_data);
int i;
- char *path;
+ char *str;

entry = audit_to_entry_common((struct audit_rule *)data);
if (IS_ERR(entry))
@@ -241,16 +248,40 @@ static struct audit_entry *audit_data_to
f->op = data->fieldflags[i] & AUDIT_OPERATORS;
f->type = data->fields[i];
f->val = data->values[i];
+ f->se_str = NULL;
+ f->se_rule = NULL;
switch(f->type) {
+ case AUDIT_SE_USER:
+ case AUDIT_SE_ROLE:
+ case AUDIT_SE_TYPE:
+ case AUDIT_SE_SEN:
+ case AUDIT_SE_CLR:
+ str = audit_unpack_string(&bufp, &remain, f->val);
+ if (IS_ERR(str))
+ goto exit_free;
+ entry->rule.buflen += f->val;
+
+ err = selinux_audit_rule_init(f->type, f->op, str,
+ &f->se_rule);
+ if (err == -EINVAL) {
+ printk(KERN_WARNING "selinux audit rule for item %s is invalid\n", str);
+ err = 0;
+ }
+ if (err) {
+ kfree(str);
+ goto exit_free;
+ } else
+ f->se_str = str;
+ break;
case AUDIT_WATCH:
- path = audit_unpack_string(&bufp, &remain, f->val);
- if (IS_ERR(path))
+ str = audit_unpack_string(&bufp, &remain, f->val);
+ if (IS_ERR(str))
goto exit_free;
entry->rule.buflen += f->val;

- err = audit_to_watch(path, &entry->rule, i);
+ err = audit_to_watch(str, &entry->rule, i);
if (err) {
- kfree(path);
+ kfree(str);
goto exit_free;
}
break;
@@ -333,6 +364,15 @@ static struct audit_rule_data *audit_kru
data->buflen += data->values[i] =
audit_pack_string(&bufp, krule->watch->path);
break;
+ case AUDIT_SE_USER:
+ case AUDIT_SE_ROLE:
+ case AUDIT_SE_TYPE:
+ case AUDIT_SE_SEN:
+ case AUDIT_SE_CLR:
+ data->buflen += data->values[i] =
+ audit_pack_string(&bufp,
+ krule->fields[i].se_str);
+ break;
default:
data->values[i] = f->val;
}
@@ -370,6 +410,14 @@ static int audit_compare_rule(struct aud
if (audit_compare_watch(a->watch, b->watch))
return 1;
break;
+ case AUDIT_SE_USER:
+ case AUDIT_SE_ROLE:
+ case AUDIT_SE_TYPE:
+ case AUDIT_SE_SEN:
+ case AUDIT_SE_CLR:
+ if (strcmp(a->fields[i].se_str, b->fields[i].se_str))
+ return 1;
+ break;
default:
if (a->fields[i].val != b->fields[i].val)
return 1;
@@ -640,6 +688,9 @@ int audit_comparator(const u32 left, con
default:
return -EINVAL;
}
+ /* should NEVER get here */
+ BUG();
+ return 0;
}


@@ -726,3 +777,45 @@ unlock_and_return:
rcu_read_unlock();
return result;
}
+
+static int selinux_callback(void)
+{
+ struct audit_entry *entry;
+ int i, j, err = 0;
+
+ /* TODO: add proper locking. */
+ for (i = 0; i < AUDIT_NR_FILTERS; i++) {
+ list_for_each_entry(entry, &audit_filter_list[i], list) {
+ for (j = 0; j < entry->rule.field_count; j++) {
+ struct audit_field *f = &entry->rule.fields[j];
+ switch (f->type) {
+ case AUDIT_SE_USER:
+ case AUDIT_SE_ROLE:
+ case AUDIT_SE_TYPE:
+ case AUDIT_SE_SEN:
+ case AUDIT_SE_CLR:
+ selinux_audit_rule_free(f->se_rule);
+ err = selinux_audit_rule_init(f->type,
+ f->op, f->se_str, &f->se_rule);
+ if (err == -EINVAL) {
+ printk(KERN_WARNING "selinux audit rule for item %s is invalid\n", f->se_str);
+ err = 0;
+ }
+ if (err)
+ goto out;
+ }
+ }
+ }
+ }
+
+out:
+ return err;
+}
+
+static int __init register_selinux_callback(void)
+{
+ selinux_audit_set_callback(&selinux_callback);
+ return 0;
+}
+__initcall(register_selinux_callback);
+
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index cd83289..c52c80a 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -58,6 +58,7 @@
#include <linux/security.h>
#include <linux/list.h>
#include <linux/tty.h>
+#include <linux/selinux.h>

#include "audit.h"

@@ -168,6 +169,9 @@ static int audit_filter_rules(struct tas
enum audit_state *state)
{
int i, j;
+ u32 sid;
+
+ selinux_task_ctxid(tsk, &sid);

for (i = 0; i < rule->field_count; i++) {
struct audit_field *f = &rule->fields[i];
@@ -258,6 +262,18 @@ static int audit_filter_rules(struct tas
if (ctx)
result = audit_comparator(ctx->loginuid, f->op, f->val);
break;
+ case AUDIT_SE_USER:
+ case AUDIT_SE_ROLE:
+ case AUDIT_SE_TYPE:
+ case AUDIT_SE_SEN:
+ case AUDIT_SE_CLR:
+ /* NOTE: this may return negative values indicating
+ a temporary error. We simply treat this as a
+ match for now to avoid losing information that
+ may be wanted. */
+ result = selinux_audit_rule_match(sid, f->type, f->op,
+ f->se_rule);
+ break;
case AUDIT_ARG0:
case AUDIT_ARG1:
case AUDIT_ARG2:
--
Darrel
Dustin Kirkland
2006-02-22 06:17:10 UTC
Permalink
Post by Darrel Goeddel
The updated version of Dustin's patch I referred to is below.
Wow, Darrel, thanks for updating my patch. That's definitely above
and beyond my expectations--I figured I would be reworking it to match
your API changes.

More comments inline, but one big, general one... We need to make
sure that we test this with SELinux compiled out of the kernel, as
well as with SELinux disabled on a running system. Obviously the
functionality won't be there, but we need to make sure that we handle
those situations gracefully before pushing upstream past our git tree.
Post by Darrel Goeddel
- Add the se_str char * to the audit_field. This stores a copy of the string
target that the rule is based on. This is needed for the audit_compare_rule
addition (below). It will also be used in the callback for policy reloads
(also below). This is managed along with the se_rule field. This also gets
rid of a memory leak where the unpacked string was not being freed.
Good catch on the mem leak.
Post by Darrel Goeddel
- printk a warning and ignore invalid selinux rules (but still hang on to them
so they may be activated with a later policy reload).
Interesting... Is this the recommended approach by the SELinux folks?
Post by Darrel Goeddel
- Change audit_compare_rule to compare the string values of the selinux rules
to see if the rule exists. Previously, it thought that any selinux filters
based on equal length string were equal.
- Change audit_krule_to_data to put in the string target of the rule for the
selinux filters. This will hopefully allow the display of the type, level,
etc. when dumping rules via AUDIT_LIST_RULES (I'll test tommorrow).
Ah yes, these were unfinished pieces of my original patch. Thanks for
polishing it off.
Amy noted both of these and I was saving them to-do as soon as your
API was updated.
Post by Darrel Goeddel
- Add a selinux callback for re-initializing the se_rule field when there is
a policy reload. THIS NEEDS WORK - It doesn't obey proper locking yet, but
it is functional. I need to get my head around the locking of the audit
structures a little better - I'll also take suggestions ;)
Outside of my expertise. Deferring.
Post by Darrel Goeddel
- Obtain the sid of the task in audit_filter_rules instead of the callers
obtaining it and passing it in.
diff --git a/kernel/audit.h b/kernel/audit.h
index eb33354..3ffc70a 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -59,9 +59,11 @@ struct audit_watch {
};
struct audit_field {
- u32 type;
- u32 val;
- u32 op;
+ u32 type;
+ u32 val;
+ u32 op;
+ char *se_str;
+ struct selinux_audit_rule *se_rule;
};
struct audit_krule {
diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index 3712295..306e941 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -25,6 +25,7 @@
#include <linux/fs.h>
#include <linux/namei.h>
#include <linux/netlink.h>
+#include <linux/selinux.h>
#include "audit.h"
/* There are three lists of rules -- one to search at task creation
@@ -50,6 +51,12 @@ static inline void audit_free_watch(stru
static inline void audit_free_rule(struct audit_entry *e)
{
+ int i;
+ for (i = 0; i < e->rule.field_count; i++) {
+ struct audit_field *f = &e->rule.fields[i];
+ kfree(f->se_str);
The memory leak fix. Thanks.
Post by Darrel Goeddel
+ selinux_audit_rule_free(f->se_rule);
+ }
kfree(e->rule.fields);
kfree(e);
}
<snip>
Post by Darrel Goeddel
}
+
+static int selinux_callback(void)
Is this function name descriptive enough? At the least, a little
documentation might help explain what this is all about ;-)
Post by Darrel Goeddel
+{
+ struct audit_entry *entry;
+ int i, j, err = 0;
+
+ /* TODO: add proper locking. */
+ for (i = 0; i < AUDIT_NR_FILTERS; i++) {
+ list_for_each_entry(entry, &audit_filter_list[i], list) {
+ for (j = 0; j < entry->rule.field_count; j++) {
+ struct audit_field *f = &entry->rule.fields[j];
+ switch (f->type) {
+ selinux_audit_rule_free(f->se_rule);
+ err = selinux_audit_rule_init(f->type,
+ f->op, f->se_str, &f->se_rule);
+ if (err == -EINVAL) {
+ printk(KERN_WARNING "selinux audit rule for item %s is invalid\n", f->se_str);
+ err = 0;
Is this desired behavior?

<snip>
Post by Darrel Goeddel
@@ -258,6 +262,18 @@ static int audit_filter_rules(struct tas
if (ctx)
result = audit_comparator(ctx->loginuid, f->op, f->val);
break;
+ /* NOTE: this may return negative values indicating
+ a temporary error. We simply treat this as a
+ match for now to avoid losing information that
+ may be wanted. */
Can we get clarification from Klaus that this meets LSPP/RBACPP
requirements? I would think that this conservative approach would be
acceptable.
Post by Darrel Goeddel
+ result = selinux_audit_rule_match(sid, f->type, f->op,
+ f->se_rule);
+ break;
Stephen Smalley
2006-02-22 15:09:27 UTC
Permalink
Post by Dustin Kirkland
Post by Darrel Goeddel
- printk a warning and ignore invalid selinux rules (but still hang on to them
so they may be activated with a later policy reload).
Interesting... Is this the recommended approach by the SELinux folks?
Not by me, but Darrel thought it would be important to allowing audit
filters to survive across policy reloads and later revived as
appropriate without needing to reload the audit filters as well. I'm
not clear that it matters in production environments (versus just policy
development boxes).
--
Stephen Smalley
National Security Agency
Stephen Smalley
2006-02-22 14:58:52 UTC
Permalink
Post by Darrel Goeddel
The updated version of Dustin's patch I referred to is below. The changes are
- printk a warning and ignore invalid selinux rules (but still hang on to them
so they may be activated with a later policy reload).
Should this be a printk or an audit_log call?
Post by Darrel Goeddel
@@ -370,6 +410,14 @@ static int audit_compare_rule(struct aud
if (audit_compare_watch(a->watch, b->watch))
return 1;
break;
+ if (strcmp(a->fields[i].se_str, b->fields[i].se_str))
+ return 1;
+ break;
Do you want to catch aliases here? If so, you need to have SELinux look
up the strings and compare the actual values. But possibly that isn't
critical for the purposes of just preventing duplicate filters.
--
Stephen Smalley
National Security Agency
Darrel Goeddel
2006-02-23 23:31:42 UTC
Permalink
Post by Stephen Smalley
Post by Darrel Goeddel
The updated version of Dustin's patch I referred to is below. The changes are
- printk a warning and ignore invalid selinux rules (but still hang on to them
so they may be activated with a later policy reload).
Should this be a printk or an audit_log call?
Steve G had suggested syslogging it, so I went with the printk. What would
be more noticeable?
Post by Stephen Smalley
Post by Darrel Goeddel
@@ -370,6 +410,14 @@ static int audit_compare_rule(struct aud
if (audit_compare_watch(a->watch, b->watch))
return 1;
break;
+ if (strcmp(a->fields[i].se_str, b->fields[i].se_str))
+ return 1;
+ break;
Do you want to catch aliases here? If so, you need to have SELinux look
up the strings and compare the actual values. But possibly that isn't
critical for the purposes of just preventing duplicate filters.
I like treating them separately because the are conceptually different to the
creator of the audit rules. If X and Y are both types in the policy, then we
should be able to define rules based on them. And yes... this kinda goes back
to keeping rules around even if they are currently invalid (or aliases).
Lets say that the current policy has X aliased to Y, and a ploicy reload
results in X and Y being distinct types - I think the audit rules should be in
there for X and Y.

If we do away with the idea of invalid (or aliased) rules around, we could add
in a comparator function for selinux, but I think we are fine the way it is.
--
Darrel
Stephen Smalley
2006-02-24 13:32:25 UTC
Permalink
Post by Darrel Goeddel
Post by Stephen Smalley
Post by Darrel Goeddel
The updated version of Dustin's patch I referred to is below. The changes are
- printk a warning and ignore invalid selinux rules (but still hang on to them
so they may be activated with a later policy reload).
Should this be a printk or an audit_log call?
Steve G had suggested syslogging it, so I went with the printk. What would
be more noticeable?
Anything user-triggerable should likely be using audit_log. Internal
kernel errors reflecting a bug within the kernel might still use
printk(KERN_ERR...). But I think we want to migrate SELinux and audit
over to using audit_log whenever possible, only using printk as the
fallback for things like audit_panic, no audit daemon, etc.
--
Stephen Smalley
National Security Agency
Steve Grubb
2006-02-24 15:08:33 UTC
Permalink
Post by Stephen Smalley
Post by Stephen Smalley
Should this be a printk or an audit_log call?
Steve G had suggested syslogging it, so I went with the printk.  What
would be more noticeable?
Yes I did. The reasoning is that the rule is still there and waiting to become
valid. I believe there was also some conversation about making a retry
whenever policy was reloaded. So, in effect, I think this is something worthy
of a syslog and not an audit event. I think it falls into the same category
as doing an audit on an inode that doesn't exist.
Post by Stephen Smalley
Anything user-triggerable should likely be using audit_log.
Its not really user triggerable. You have to be capable of loading audit
rules....which is an auditable event.
Post by Stephen Smalley
Internal kernel errors reflecting a bug within the kernel might still use
printk(KERN_ERR...).
But I think we want to migrate SELinux and audit over to using audit_log
whenever possible, only using printk as the fallback for things like
audit_panic, no audit daemon, etc.
This should be the current state right now. If we have a place where something
auditable does not create an event, please point it out.

-Steve
Stephen Smalley
2006-02-22 15:07:22 UTC
Permalink
Post by Darrel Goeddel
- Add a selinux callback for re-initializing the se_rule field when there is
a policy reload. THIS NEEDS WORK - It doesn't obey proper locking yet, but
it is functional. I need to get my head around the locking of the audit
structures a little better - I'll also take suggestions ;)
<snip>
Post by Darrel Goeddel
rcu_read_unlock();
return result;
}
+
+static int selinux_callback(void)
+{
+ struct audit_entry *entry;
+ int i, j, err = 0;
+
+ /* TODO: add proper locking. */
+ for (i = 0; i < AUDIT_NR_FILTERS; i++) {
+ list_for_each_entry(entry, &audit_filter_list[i], list) {
+ for (j = 0; j < entry->rule.field_count; j++) {
+ struct audit_field *f = &entry->rule.fields[j];
+ switch (f->type) {
+ selinux_audit_rule_free(f->se_rule);
+ err = selinux_audit_rule_init(f->type,
+ f->op, f->se_str, &f->se_rule);
+ if (err == -EINVAL) {
+ printk(KERN_WARNING "selinux audit rule for item %s is invalid\n", f->se_str);
+ err = 0;
+ }
+ if (err)
+ goto out;
+ }
+ }
+ }
+ }
For RCU, you need to make a new copy of the entry, mutate the copy (not
the original), list_replace_rcu the old original with the modified copy,
and call_rcu to drop the original when it is safe to do so. Look at the
SELinux AVC code for an example (avc_update_node).
--
Stephen Smalley
National Security Agency
Stephen Smalley
2006-02-22 15:24:00 UTC
Permalink
Post by Stephen Smalley
Post by Darrel Goeddel
- Add a selinux callback for re-initializing the se_rule field when there is
a policy reload. THIS NEEDS WORK - It doesn't obey proper locking yet, but
it is functional. I need to get my head around the locking of the audit
structures a little better - I'll also take suggestions ;)
<snip>
Post by Darrel Goeddel
rcu_read_unlock();
return result;
}
+
+static int selinux_callback(void)
+{
+ struct audit_entry *entry;
+ int i, j, err = 0;
+
+ /* TODO: add proper locking. */
+ for (i = 0; i < AUDIT_NR_FILTERS; i++) {
+ list_for_each_entry(entry, &audit_filter_list[i], list) {
+ for (j = 0; j < entry->rule.field_count; j++) {
+ struct audit_field *f = &entry->rule.fields[j];
+ switch (f->type) {
+ selinux_audit_rule_free(f->se_rule);
+ err = selinux_audit_rule_init(f->type,
+ f->op, f->se_str, &f->se_rule);
+ if (err == -EINVAL) {
+ printk(KERN_WARNING "selinux audit rule for item %s is invalid\n", f->se_str);
+ err = 0;
+ }
+ if (err)
+ goto out;
+ }
+ }
+ }
+ }
For RCU, you need to make a new copy of the entry, mutate the copy (not
the original), list_replace_rcu the old original with the modified copy,
and call_rcu to drop the original when it is safe to do so. Look at the
SELinux AVC code for an example (avc_update_node).
Oh, and it looks like you should be holding the audit_netlink_mutex as
well to synchronize with adding, removing and listing of rules. That is
easy though - the harder part is grasping the RCU model.
Documentation/RCU is very helpful.
--
Stephen Smalley
National Security Agency
Stephen Smalley
2006-02-22 14:46:32 UTC
Permalink
Post by Darrel Goeddel
diff --git a/security/selinux/exports.c b/security/selinux/exports.c
new file mode 100644
index 0000000..5129add
--- /dev/null
+++ b/security/selinux/exports.c
+int selinux_audit_rule_init(u32 field, u32 op, char *rulestr,
+ struct selinux_audit_rule **rule)
+{
+ return security_aurule_init(field, op, rulestr, rule);
+}
I'd drop these wrapper functions, and just name the underlying functions
in services.c with the exported names (i.e. selinux_ prefix). The use
of security_ prefix in the SELinux code should likely be converted in
general to selinux_ (via separate patch) along with other namespace
cleanups; it is a legacy of when SELinux was a kernel patch and there
was no LSM.
Post by Darrel Goeddel
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index d877cd1..5e05c5a 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
<snip>
Post by Darrel Goeddel
+int security_aurule_init(u32 field, u32 op, char *rulestr,
+ struct selinux_audit_rule **rule)
+{
<snip>
Post by Darrel Goeddel
+ /* we need freestr because mls_context_to_sid will change
+ the value of tmpstr */
+ tmpstr = freestr = kstrdup(rulestr, GFP_ATOMIC);
+ if (!tmpstr) {
+ rc = -ENOMEM;
+ } else {
+ rc = mls_context_to_sid(':', &tmpstr, &tmprule->au_ctxt,
+ NULL, SECSID_NULL);
+ kfree(freestr);
+ }
Let's move this into a helper in mls.c with a nicer interface, similar
to what Ivan has done in libsepol (mls_from_string).
Post by Darrel Goeddel
+int security_aurule_match(u32 ctxid, u32 field, u32 op,
+ struct selinux_audit_rule *rule)
+{
+ struct context *ctxt;
+ struct mls_level *level;
+ int match = 0;
+
+ if (!rule)
+ return 0;
Should this be an error?
Post by Darrel Goeddel
+ ctxt = sidtab_search(&sidtab, ctxid);
+ if (!ctxt) {
+ printk(KERN_ERR "security_aurule_match: unrecognized SID %d\n",
+ ctxid);
Should we be using printk(KERN_ERR...) or
audit_log(...AUDIT_SELINUX_ERR) for SELinux errors for all new code?
Post by Darrel Goeddel
+ match = (mls_level_dom(&rule->au_ctxt.range.level[0],
+ level) &&
+ !mls_level_eq(&rule->au_ctxt.range.level[0],
+ level));
+ break;
+ match = mls_level_dom(&rule->au_ctxt.range.level[0],
+ level);
+ break;
I'm not clear as to why we truly need both sets of operators in the
audit filters (<= n versus just < (n+1)), but that is unrelated to
SELinux per se.

I take it that you either decided against leveraging the constraint code
or haven't looked at that option yet? IOW, convert an audit filter to a
constraint expression when the rule is initialized, and later just call
a common helper shared with constraint_expr_eval?
Post by Darrel Goeddel
+static int (*aurule_callback)(void) = NULL;
+
+static int aurule_avc_callback(u32 event, u32 ssid, u32 tsid,
+ u16 class, u32 perms, u32 *retained)
+{
+ int err = 0;
+
+ if (event == AVC_CALLBACK_RESET && aurule_callback)
+ err = aurule_callback();
+ return err;
+}
Hmm...on an error return, we will stop walking the callback list
presently on avc_ss_reset, which could prevent other callbacks (like the
netif one) from occurring. Likely should change that.
--
Stephen Smalley
National Security Agency
Darrel Goeddel
2006-02-23 17:42:04 UTC
Permalink
Post by Stephen Smalley
Post by Darrel Goeddel
diff --git a/security/selinux/exports.c b/security/selinux/exports.c
new file mode 100644
index 0000000..5129add
--- /dev/null
+++ b/security/selinux/exports.c
+int selinux_audit_rule_init(u32 field, u32 op, char *rulestr,
+ struct selinux_audit_rule **rule)
+{
+ return security_aurule_init(field, op, rulestr, rule);
+}
I'd drop these wrapper functions, and just name the underlying functions
in services.c with the exported names (i.e. selinux_ prefix). The use
of security_ prefix in the SELinux code should likely be converted in
general to selinux_ (via separate patch) along with other namespace
cleanups; it is a legacy of when SELinux was a kernel patch and there
was no LSM.
Done.
Post by Stephen Smalley
Post by Darrel Goeddel
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index d877cd1..5e05c5a 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
<snip>
Post by Darrel Goeddel
+int security_aurule_init(u32 field, u32 op, char *rulestr,
+ struct selinux_audit_rule **rule)
+{
<snip>
Post by Darrel Goeddel
+ /* we need freestr because mls_context_to_sid will change
+ the value of tmpstr */
+ tmpstr = freestr = kstrdup(rulestr, GFP_ATOMIC);
+ if (!tmpstr) {
+ rc = -ENOMEM;
+ } else {
+ rc = mls_context_to_sid(':', &tmpstr, &tmprule->au_ctxt,
+ NULL, SECSID_NULL);
+ kfree(freestr);
+ }
Let's move this into a helper in mls.c with a nicer interface, similar
to what Ivan has done in libsepol (mls_from_string).
Done.
Post by Stephen Smalley
Post by Darrel Goeddel
+int security_aurule_match(u32 ctxid, u32 field, u32 op,
+ struct selinux_audit_rule *rule)
+{
+ struct context *ctxt;
+ struct mls_level *level;
+ int match = 0;
+
+ if (!rule)
+ return 0;
Should this be an error?
I'll treat this as an error here. If we decide to go with allowing invalid
to hang around waiting to become valid, we can conditionally call
selinux_audit_rule_match.
Post by Stephen Smalley
Post by Darrel Goeddel
+ ctxt = sidtab_search(&sidtab, ctxid);
+ if (!ctxt) {
+ printk(KERN_ERR "security_aurule_match: unrecognized SID %d\n",
+ ctxid);
Should we be using printk(KERN_ERR...) or
audit_log(...AUDIT_SELINUX_ERR) for SELinux errors for all new code?
Switched to audit log. I also now pass in the audit_context to
selinux_audit_rule_match so it gets used for the logging.
Post by Stephen Smalley
Post by Darrel Goeddel
+ match = (mls_level_dom(&rule->au_ctxt.range.level[0],
+ level) &&
+ !mls_level_eq(&rule->au_ctxt.range.level[0],
+ level));
+ break;
+ match = mls_level_dom(&rule->au_ctxt.range.level[0],
+ level);
+ break;
I'm not clear as to why we truly need both sets of operators in the
audit filters (<= n versus just < (n+1)), but that is unrelated to
SELinux per se.
I take it that you either decided against leveraging the constraint code
or haven't looked at that option yet? IOW, convert an audit filter to a
constraint expression when the rule is initialized, and later just call
a common helper shared with constraint_expr_eval?
I have briefly look into it. No huge benefit jumped out at me right away,
but it is something I'll look into more later.
Post by Stephen Smalley
Post by Darrel Goeddel
+static int (*aurule_callback)(void) = NULL;
+
+static int aurule_avc_callback(u32 event, u32 ssid, u32 tsid,
+ u16 class, u32 perms, u32 *retained)
+{
+ int err = 0;
+
+ if (event == AVC_CALLBACK_RESET && aurule_callback)
+ err = aurule_callback();
+ return err;
+}
Hmm...on an error return, we will stop walking the callback list
presently on avc_ss_reset, which could prevent other callbacks (like the
netif one) from occurring. Likely should change that.
Done.

Here is a new one:


Signed-off-by: Darrel Goeddel <***@trustedcs.com>


diff --git a/include/linux/audit.h b/include/linux/audit.h
index 4bb4b9f..dd4f759 100644
--- 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_SEN 16 /* security label sensitivity label */
+#define AUDIT_SE_CLR 17 /* security label clearance label */

/* These are ONLY useful when checking
* at syscall exit time (AUDIT_AT_EXIT). */
diff --git a/include/linux/selinux.h b/include/linux/selinux.h
new file mode 100644
index 0000000..5abe524
--- /dev/null
+++ b/include/linux/selinux.h
@@ -0,0 +1,112 @@
+/*
+ * SELinux services exported to the rest of the kernel.
+ *
+ * Author: James Morris <***@redhat.com>
+ *
+ * Copyright (C) 2005 Red Hat, Inc., James Morris <***@redhat.com>
+ * Copyright (C) 2006 Trusted Computer Solutions, Inc. <***@trustedcs.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2,
+ * as published by the Free Software Foundation.
+ */
+#ifndef _LINUX_SELINUX_H
+#define _LINUX_SELINUX_H
+
+#ifdef CONFIG_SECURITY_SELINUX
+
+struct selinux_audit_rule;
+struct audit_context;
+
+/**
+ * selinux_audit_rule_init - alloc/init an selinux audit rule structure.
+ * @field: the field this rule refers to
+ * @op: the operater the rule uses
+ * @rulestr: the text "target" of the rule
+ * @rule: pointer to the new rule structure returned via this
+ *
+ * Returns 0 if successful, -errno if not. On success, the rule structure
+ * will be allocated internally. The caller must free this structure with
+ * selinux_audit_rule_free() after use.
+ */
+int selinux_audit_rule_init(u32 field, u32 op, char *rulestr,
+ struct selinux_audit_rule **rule);
+
+/**
+ * selinux_audit_rule_free - free an selinux audit rule structure.
+ * @rule: pointer to the audit rule to be freed
+ *
+ * This will free all memory associated with the given rule.
+ * If @rule is NULL, no operation is performed.
+ */
+void selinux_audit_rule_free(struct selinux_audit_rule *rule);
+
+/**
+ * selinux_audit_rule_match - determine if a context ID matches a rule.
+ * @ctxid: the context ID to check
+ * @field: the field this rule refers to
+ * @op: the operater the rule uses
+ * @rule: pointer to the audit rule to check against
+ * @actx: the audit context (can be NULL) associated with the check
+ *
+ * Returns 1 if the context id matches the rule, 0 if it does not, and
+ * -errno on failure.
+ */
+int selinux_audit_rule_match(u32 ctxid, u32 field, u32 op,
+ struct selinux_audit_rule *rule,
+ struct audit_context *actx);
+
+/**
+ * selinux_audit_set_callback - set the callback for policy reloads.
+ * @callback: the function to call when the policy is reloaded
+ *
+ * This sets the function callback function that will update the rules
+ * upon policy reloads. This callback should rebuild all existing rules
+ * using selinux_audit_rule_init().
+ */
+void selinux_audit_set_callback(int (*callback)(void));
+
+/**
+ * selinux_task_ctxid - determine a context ID for a process.
+ * @tsk: the task object
+ * @ctxid: ID value returned via this
+ *
+ * On return, ctxid will contain an ID for the context. This value
+ * should only be used opaquely.
+ */
+void selinux_task_ctxid(struct task_struct *tsk, u32 *ctxid);
+
+#else
+
+static inline int selinux_audit_rule_init(u32 field, u32 op,
+ char *rulestr,
+ struct selinux_audit_rule **rule)
+{
+ return -ENOTSUPP;
+}
+
+static inline void selinux_audit_rule_free(struct selinux_audit_rule *rule)
+{
+ return;
+}
+
+static inline int selinux_audit_rule_match(u32 ctxid, u32 field, u32 op,
+ struct selinux_audit_rule *rule,
+ struct audit_context *actx)
+{
+ return 0;
+}
+
+static inline void selinux_audit_set_callback(int (*callback)(void))
+{
+ return;
+}
+
+static inline void selinux_task_ctxid(struct task_struct *tsk, u32 *ctxid)
+{
+ *ctxid = 0;
+}
+
+#endif /* CONFIG_SECURITY_SELINUX */
+
+#endif /* _LINUX_SELINUX_H */
diff --git a/security/selinux/Makefile b/security/selinux/Makefile
index 688c0a2..faf2e02 100644
--- a/security/selinux/Makefile
+++ b/security/selinux/Makefile
@@ -4,7 +4,7 @@

obj-$(CONFIG_SECURITY_SELINUX) := selinux.o ss/

-selinux-y := avc.o hooks.o selinuxfs.o netlink.o nlmsgtab.o netif.o
+selinux-y := avc.o hooks.o selinuxfs.o netlink.o nlmsgtab.o netif.o exports.o

selinux-$(CONFIG_SECURITY_NETWORK_XFRM) += xfrm.o

diff --git a/security/selinux/avc.c b/security/selinux/avc.c
index ac5d69b..a300702 100644
--- a/security/selinux/avc.c
+++ b/security/selinux/avc.c
@@ -800,7 +800,7 @@ out:
int avc_ss_reset(u32 seqno)
{
struct avc_callback_node *c;
- int i, rc = 0;
+ int i, rc = 0, tmprc;
unsigned long flag;
struct avc_node *node;

@@ -813,15 +813,16 @@ int avc_ss_reset(u32 seqno)

for (c = avc_callbacks; c; c = c->next) {
if (c->events & AVC_CALLBACK_RESET) {
- rc = c->callback(AVC_CALLBACK_RESET,
- 0, 0, 0, 0, NULL);
- if (rc)
- goto out;
+ tmprc = c->callback(AVC_CALLBACK_RESET,
+ 0, 0, 0, 0, NULL);
+ /* save the first error encountered for the return
+ value and continue processing the callbacks */
+ if (!rc)
+ rc = tmprc;
}
}

avc_latest_notif_update(seqno, 0);
-out:
return rc;
}

diff --git a/security/selinux/exports.c b/security/selinux/exports.c
new file mode 100644
index 0000000..26ff66d
--- /dev/null
+++ b/security/selinux/exports.c
@@ -0,0 +1,25 @@
+/*
+ * SELinux services exported to the rest of the kernel.
+ *
+ * Author: James Morris <***@redhat.com>
+ *
+ * Copyright (C) 2005 Red Hat, Inc., James Morris <***@redhat.com>
+ * Copyright (C) 2006 Trusted Computer Solutions, Inc. <***@trustedcs.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2,
+ * as published by the Free Software Foundation.
+ */
+#include <linux/types.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/selinux.h>
+
+#include "security.h"
+#include "objsec.h"
+
+void selinux_task_ctxid(struct task_struct *tsk, u32 *ctxid)
+{
+ struct task_security_struct *tsec = tsk->security;
+ *ctxid = tsec->sid;
+}
diff --git a/security/selinux/ss/mls.c b/security/selinux/ss/mls.c
index 640d0bf..df358be 100644
--- a/security/selinux/ss/mls.c
+++ b/security/selinux/ss/mls.c
@@ -8,7 +8,7 @@
*
* Support for enhanced MLS infrastructure.
*
- * Copyright (C) 2004-2005 Trusted Computer Solutions, Inc.
+ * Copyright (C) 2004-2006 Trusted Computer Solutions, Inc.
*/

#include <linux/kernel.h>
@@ -385,6 +385,31 @@ out:
}

/*
+ * Set the MLS fields in the security context structure
+ * `context' based on the string representation in
+ * the string `str'. This function will allocate temporary memory with the
+ * given constraints of gfp_mask.
+ */
+int mls_from_string(char *str, struct context *context, gfp_t gfp_mask)
+{
+ char *tmpstr, *freestr;
+ int rc;
+
+ /* we need freestr because mls_context_to_sid will change
+ the value of tmpstr */
+ tmpstr = freestr = kstrdup(str, gfp_mask);
+ if (!tmpstr) {
+ rc = -ENOMEM;
+ } else {
+ rc = mls_context_to_sid(':', &tmpstr, context,
+ NULL, SECSID_NULL);
+ kfree(freestr);
+ }
+
+ return rc;
+}
+
+/*
* Copies the effective MLS range from `src' into `dst'.
*/
static inline int mls_scopy_context(struct context *dst,
diff --git a/security/selinux/ss/mls.h b/security/selinux/ss/mls.h
index 03de697..fbb42f0 100644
--- a/security/selinux/ss/mls.h
+++ b/security/selinux/ss/mls.h
@@ -8,7 +8,7 @@
*
* Support for enhanced MLS infrastructure.
*
- * Copyright (C) 2004-2005 Trusted Computer Solutions, Inc.
+ * Copyright (C) 2004-2006 Trusted Computer Solutions, Inc.
*/

#ifndef _SS_MLS_H_
@@ -27,6 +27,8 @@ int mls_context_to_sid(char oldc,
struct sidtab *s,
u32 def_sid);

+int mls_from_string(char *str, struct context *context, gfp_t gfp_mask);
+
int mls_convert_context(struct policydb *oldp,
struct policydb *newp,
struct context *context);
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index d877cd1..a2ad2cd 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -7,12 +7,13 @@
* Updated: Trusted Computer Solutions, Inc. <***@trustedcs.com>
*
* Support for enhanced MLS infrastructure.
+ * Support for context based audit filters.
*
* Updated: Frank Mayer <***@tresys.com> and Karl MacMillan <***@tresys.com>
*
* Added conditional policy language extensions
*
- * Copyright (C) 2004-2005 Trusted Computer Solutions, Inc.
+ * Copyright (C) 2004-2006 Trusted Computer Solutions, Inc.
* Copyright (C) 2003 - 2004 Tresys Technology, LLC
* Copyright (C) 2003 Red Hat, Inc., James Morris <***@redhat.com>
* This program is free software; you can redistribute it and/or modify
@@ -1810,3 +1811,235 @@ out:
POLICY_RDUNLOCK;
return rc;
}
+
+struct selinux_audit_rule {
+ u32 au_seqno;
+ struct context au_ctxt;
+};
+
+void selinux_audit_rule_free(struct selinux_audit_rule *rule)
+{
+ if (rule) {
+ context_destroy(&rule->au_ctxt);
+ kfree(rule);
+ }
+}
+
+int selinux_audit_rule_init(u32 field, u32 op, char *rulestr,
+ struct selinux_audit_rule **rule)
+{
+ struct selinux_audit_rule *tmprule;
+ struct role_datum *roledatum;
+ struct type_datum *typedatum;
+ struct user_datum *userdatum;
+ int rc = 0;
+
+ *rule = NULL;
+
+ if (!ss_initialized)
+ return -ENOTSUPP;
+
+ switch (field) {
+ case AUDIT_SE_USER:
+ case AUDIT_SE_ROLE:
+ case AUDIT_SE_TYPE:
+ /* only 'equals' and 'not equals' fit user, role, and type */
+ if (op != AUDIT_EQUAL && op != AUDIT_NOT_EQUAL)
+ return -EINVAL;
+ break;
+ case AUDIT_SE_SEN:
+ case AUDIT_SE_CLR:
+ /* we do not allow a range, indicated by the presense of '-' */
+ if (strchr(rulestr, '-'))
+ return -EINVAL;
+ break;
+ default:
+ /* only the above fileds are valid */
+ return -EINVAL;
+ }
+
+ tmprule = kzalloc(sizeof(struct selinux_audit_rule), GFP_KERNEL);
+ if (!tmprule)
+ return -ENOMEM;
+
+ context_init(&tmprule->au_ctxt);
+
+ POLICY_RDLOCK;
+
+ tmprule->au_seqno = latest_granting;
+
+ switch (field) {
+ case AUDIT_SE_USER:
+ userdatum = hashtab_search(policydb.p_users.table, rulestr);
+ if (!userdatum)
+ rc = -EINVAL;
+ else
+ tmprule->au_ctxt.user = userdatum->value;
+ break;
+ case AUDIT_SE_ROLE:
+ roledatum = hashtab_search(policydb.p_roles.table, rulestr);
+ if (!roledatum)
+ rc = -EINVAL;
+ else
+ tmprule->au_ctxt.role = roledatum->value;
+ break;
+ case AUDIT_SE_TYPE:
+ typedatum = hashtab_search(policydb.p_types.table, rulestr);
+ if (!typedatum)
+ rc = -EINVAL;
+ else
+ tmprule->au_ctxt.type = typedatum->value;
+ break;
+ case AUDIT_SE_SEN:
+ case AUDIT_SE_CLR:
+ rc = mls_from_string(rulestr, &tmprule->au_ctxt, GFP_ATOMIC);
+ break;
+ }
+
+ POLICY_RDUNLOCK;
+
+ if (rc) {
+ selinux_audit_rule_free(tmprule);
+ tmprule = NULL;
+ }
+
+ *rule = tmprule;
+
+ return rc;
+}
+
+int selinux_audit_rule_match(u32 ctxid, u32 field, u32 op,
+ struct selinux_audit_rule *rule,
+ struct audit_context *actx)
+{
+ struct context *ctxt;
+ struct mls_level *level;
+ int match = 0;
+
+ if (!rule) {
+ audit_log(actx, GFP_ATOMIC, AUDIT_SELINUX_ERR,
+ "selinux_audit_rule_match: missing rule\n");
+ return -ENOENT;
+ }
+
+ POLICY_RDLOCK;
+
+ if (rule->au_seqno < latest_granting) {
+ audit_log(actx, GFP_ATOMIC, AUDIT_SELINUX_ERR,
+ "selinux_audit_rule_match: stale rule\n");
+ match = -ESTALE;
+ goto out;
+ }
+
+ ctxt = sidtab_search(&sidtab, ctxid);
+ if (!ctxt) {
+ audit_log(actx, GFP_ATOMIC, AUDIT_SELINUX_ERR,
+ "selinux_audit_rule_match: unrecognized SID %d\n",
+ ctxid);
+ match = -ENOENT;
+ goto out;
+ }
+
+ /* a field/op pair that is not caught here will simply fall through
+ without a match */
+ switch (field) {
+ case AUDIT_SE_USER:
+ switch (op) {
+ case AUDIT_EQUAL:
+ match = (ctxt->user == rule->au_ctxt.user);
+ break;
+ case AUDIT_NOT_EQUAL:
+ match = (ctxt->user != rule->au_ctxt.user);
+ break;
+ }
+ break;
+ case AUDIT_SE_ROLE:
+ switch (op) {
+ case AUDIT_EQUAL:
+ match = (ctxt->role == rule->au_ctxt.role);
+ break;
+ case AUDIT_NOT_EQUAL:
+ match = (ctxt->role != rule->au_ctxt.role);
+ break;
+ }
+ break;
+ case AUDIT_SE_TYPE:
+ switch (op) {
+ case AUDIT_EQUAL:
+ match = (ctxt->type == rule->au_ctxt.type);
+ break;
+ case AUDIT_NOT_EQUAL:
+ match = (ctxt->type != rule->au_ctxt.type);
+ break;
+ }
+ break;
+ case AUDIT_SE_SEN:
+ case AUDIT_SE_CLR:
+ level = (op == AUDIT_SE_SEN ?
+ &ctxt->range.level[0] : &ctxt->range.level[1]);
+ switch (op) {
+ case AUDIT_EQUAL:
+ match = mls_level_eq(&rule->au_ctxt.range.level[0],
+ level);
+ break;
+ case AUDIT_NOT_EQUAL:
+ match = !mls_level_eq(&rule->au_ctxt.range.level[0],
+ level);
+ break;
+ case AUDIT_LESS_THAN:
+ match = (mls_level_dom(&rule->au_ctxt.range.level[0],
+ level) &&
+ !mls_level_eq(&rule->au_ctxt.range.level[0],
+ level));
+ break;
+ case AUDIT_LESS_THAN_OR_EQUAL:
+ match = mls_level_dom(&rule->au_ctxt.range.level[0],
+ level);
+ break;
+ case AUDIT_GREATER_THAN:
+ match = (mls_level_dom(level,
+ &rule->au_ctxt.range.level[0]) &&
+ !mls_level_eq(level,
+ &rule->au_ctxt.range.level[0]));
+ break;
+ case AUDIT_GREATER_THAN_OR_EQUAL:
+ match = mls_level_dom(level,
+ &rule->au_ctxt.range.level[0]);
+ break;
+ }
+ }
+
+out:
+ POLICY_RDUNLOCK;
+ return match;
+}
+
+static int (*aurule_callback)(void) = NULL;
+
+static int aurule_avc_callback(u32 event, u32 ssid, u32 tsid,
+ u16 class, u32 perms, u32 *retained)
+{
+ int err = 0;
+
+ if (event == AVC_CALLBACK_RESET && aurule_callback)
+ err = aurule_callback();
+ return err;
+}
+
+static int __init aurule_init(void)
+{
+ int err;
+
+ err = avc_add_callback(aurule_avc_callback, AVC_CALLBACK_RESET,
+ SECSID_NULL, SECSID_NULL, SECCLASS_NULL, 0);
+ if (err)
+ panic("avc_add_callback() failed, error %d\n", err);
+
+ return err;
+}
+__initcall(aurule_init);
+
+void selinux_audit_set_callback(int (*callback)(void))
+{
+ aurule_callback = callback;
+}
--
Darrel
Stephen Smalley
2006-02-24 13:27:37 UTC
Permalink
Post by Darrel Goeddel
diff --git a/security/selinux/ss/mls.c b/security/selinux/ss/mls.c
index 640d0bf..df358be 100644
--- a/security/selinux/ss/mls.c
+++ b/security/selinux/ss/mls.c
}
/*
+ * Set the MLS fields in the security context structure
+ * `context' based on the string representation in
+ * the string `str'. This function will allocate temporary memory with the
+ * given constraints of gfp_mask.
+ */
+int mls_from_string(char *str, struct context *context, gfp_t gfp_mask)
+{
+ char *tmpstr, *freestr;
+ int rc;
Likely should be checking selinux_mls_enabled on entry and returning an
error in that case (mls_context_to_sid will just return 0 in that case).
Post by Darrel Goeddel
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index d877cd1..a2ad2cd 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
Need to #include <linux/selinux.h> now to pick up the function
prototypes for the selinux_audit_ functions to ensure that they are
checked against the real functions.
Post by Darrel Goeddel
+ /* only the above fileds are valid */
Nit: Typo in comment.

Otherwise, looks good.
--
Stephen Smalley
National Security Agency
Darrel Goeddel
2006-02-24 21:44:05 UTC
Permalink
Post by Stephen Smalley
Post by Darrel Goeddel
diff --git a/security/selinux/ss/mls.c b/security/selinux/ss/mls.c
index 640d0bf..df358be 100644
--- a/security/selinux/ss/mls.c
+++ b/security/selinux/ss/mls.c
}
/*
+ * Set the MLS fields in the security context structure
+ * `context' based on the string representation in
+ * the string `str'. This function will allocate temporary memory with the
+ * given constraints of gfp_mask.
+ */
+int mls_from_string(char *str, struct context *context, gfp_t gfp_mask)
+{
+ char *tmpstr, *freestr;
+ int rc;
Likely should be checking selinux_mls_enabled on entry and returning an
error in that case (mls_context_to_sid will just return 0 in that case).
Post by Darrel Goeddel
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index d877cd1..a2ad2cd 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
Need to #include <linux/selinux.h> now to pick up the function
prototypes for the selinux_audit_ functions to ensure that they are
checked against the real functions.
Post by Darrel Goeddel
+ /* only the above fileds are valid */
Nit: Typo in comment.
Otherwise, looks good.
Fixed up those two issues. I also fixed a compilation problem when selinux
support is not compiled in, and a possible oops when selinux is compiled in
but not initialized (including being disabled at runtime). The final
version is below.

--

The following patch provides selinux interfaces that will allow the audit
system to perform filtering based on the process context (user, role, type,
sensitivity, and clearance). These interfaces will allow the selinux
module to perform efficient matches based on lower level selinux constructs,
rather than relying on context retrievals and string comparisons within
the audit module. It also allows for dominance checks on the mls portion
of the contexts that are impossible with only string comparisons.


Signed-off-by: Darrel Goeddel <***@trustedcs.com>


diff --git a/include/linux/audit.h b/include/linux/audit.h
index 4bb4b9f..dd4f759 100644
--- 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_SEN 16 /* security label sensitivity label */
+#define AUDIT_SE_CLR 17 /* security label clearance label */

/* These are ONLY useful when checking
* at syscall exit time (AUDIT_AT_EXIT). */
diff --git a/include/linux/selinux.h b/include/linux/selinux.h
new file mode 100644
index 0000000..9d684b1
--- /dev/null
+++ b/include/linux/selinux.h
@@ -0,0 +1,112 @@
+/*
+ * SELinux services exported to the rest of the kernel.
+ *
+ * Author: James Morris <***@redhat.com>
+ *
+ * Copyright (C) 2005 Red Hat, Inc., James Morris <***@redhat.com>
+ * Copyright (C) 2006 Trusted Computer Solutions, Inc. <***@trustedcs.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2,
+ * as published by the Free Software Foundation.
+ */
+#ifndef _LINUX_SELINUX_H
+#define _LINUX_SELINUX_H
+
+struct selinux_audit_rule;
+struct audit_context;
+
+#ifdef CONFIG_SECURITY_SELINUX
+
+/**
+ * selinux_audit_rule_init - alloc/init an selinux audit rule structure.
+ * @field: the field this rule refers to
+ * @op: the operater the rule uses
+ * @rulestr: the text "target" of the rule
+ * @rule: pointer to the new rule structure returned via this
+ *
+ * Returns 0 if successful, -errno if not. On success, the rule structure
+ * will be allocated internally. The caller must free this structure with
+ * selinux_audit_rule_free() after use.
+ */
+int selinux_audit_rule_init(u32 field, u32 op, char *rulestr,
+ struct selinux_audit_rule **rule);
+
+/**
+ * selinux_audit_rule_free - free an selinux audit rule structure.
+ * @rule: pointer to the audit rule to be freed
+ *
+ * This will free all memory associated with the given rule.
+ * If @rule is NULL, no operation is performed.
+ */
+void selinux_audit_rule_free(struct selinux_audit_rule *rule);
+
+/**
+ * selinux_audit_rule_match - determine if a context ID matches a rule.
+ * @ctxid: the context ID to check
+ * @field: the field this rule refers to
+ * @op: the operater the rule uses
+ * @rule: pointer to the audit rule to check against
+ * @actx: the audit context (can be NULL) associated with the check
+ *
+ * Returns 1 if the context id matches the rule, 0 if it does not, and
+ * -errno on failure.
+ */
+int selinux_audit_rule_match(u32 ctxid, u32 field, u32 op,
+ struct selinux_audit_rule *rule,
+ struct audit_context *actx);
+
+/**
+ * selinux_audit_set_callback - set the callback for policy reloads.
+ * @callback: the function to call when the policy is reloaded
+ *
+ * This sets the function callback function that will update the rules
+ * upon policy reloads. This callback should rebuild all existing rules
+ * using selinux_audit_rule_init().
+ */
+void selinux_audit_set_callback(int (*callback)(void));
+
+/**
+ * selinux_task_ctxid - determine a context ID for a process.
+ * @tsk: the task object
+ * @ctxid: ID value returned via this
+ *
+ * On return, ctxid will contain an ID for the context. This value
+ * should only be used opaquely.
+ */
+void selinux_task_ctxid(struct task_struct *tsk, u32 *ctxid);
+
+#else
+
+static inline int selinux_audit_rule_init(u32 field, u32 op,
+ char *rulestr,
+ struct selinux_audit_rule **rule)
+{
+ return -ENOTSUPP;
+}
+
+static inline void selinux_audit_rule_free(struct selinux_audit_rule *rule)
+{
+ return;
+}
+
+static inline int selinux_audit_rule_match(u32 ctxid, u32 field, u32 op,
+ struct selinux_audit_rule *rule,
+ struct audit_context *actx)
+{
+ return 0;
+}
+
+static inline void selinux_audit_set_callback(int (*callback)(void))
+{
+ return;
+}
+
+static inline void selinux_task_ctxid(struct task_struct *tsk, u32 *ctxid)
+{
+ *ctxid = 0;
+}
+
+#endif /* CONFIG_SECURITY_SELINUX */
+
+#endif /* _LINUX_SELINUX_H */
diff --git a/security/selinux/Makefile b/security/selinux/Makefile
index 688c0a2..faf2e02 100644
--- a/security/selinux/Makefile
+++ b/security/selinux/Makefile
@@ -4,7 +4,7 @@

obj-$(CONFIG_SECURITY_SELINUX) := selinux.o ss/

-selinux-y := avc.o hooks.o selinuxfs.o netlink.o nlmsgtab.o netif.o
+selinux-y := avc.o hooks.o selinuxfs.o netlink.o nlmsgtab.o netif.o exports.o

selinux-$(CONFIG_SECURITY_NETWORK_XFRM) += xfrm.o

diff --git a/security/selinux/avc.c b/security/selinux/avc.c
index ac5d69b..a300702 100644
--- a/security/selinux/avc.c
+++ b/security/selinux/avc.c
@@ -800,7 +800,7 @@ out:
int avc_ss_reset(u32 seqno)
{
struct avc_callback_node *c;
- int i, rc = 0;
+ int i, rc = 0, tmprc;
unsigned long flag;
struct avc_node *node;

@@ -813,15 +813,16 @@ int avc_ss_reset(u32 seqno)

for (c = avc_callbacks; c; c = c->next) {
if (c->events & AVC_CALLBACK_RESET) {
- rc = c->callback(AVC_CALLBACK_RESET,
- 0, 0, 0, 0, NULL);
- if (rc)
- goto out;
+ tmprc = c->callback(AVC_CALLBACK_RESET,
+ 0, 0, 0, 0, NULL);
+ /* save the first error encountered for the return
+ value and continue processing the callbacks */
+ if (!rc)
+ rc = tmprc;
}
}

avc_latest_notif_update(seqno, 0);
-out:
return rc;
}

diff --git a/security/selinux/exports.c b/security/selinux/exports.c
new file mode 100644
index 0000000..333c4c7
--- /dev/null
+++ b/security/selinux/exports.c
@@ -0,0 +1,28 @@
+/*
+ * SELinux services exported to the rest of the kernel.
+ *
+ * Author: James Morris <***@redhat.com>
+ *
+ * Copyright (C) 2005 Red Hat, Inc., James Morris <***@redhat.com>
+ * Copyright (C) 2006 Trusted Computer Solutions, Inc. <***@trustedcs.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2,
+ * as published by the Free Software Foundation.
+ */
+#include <linux/types.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/selinux.h>
+
+#include "security.h"
+#include "objsec.h"
+
+void selinux_task_ctxid(struct task_struct *tsk, u32 *ctxid)
+{
+ struct task_security_struct *tsec = tsk->security;
+ if (selinux_enabled)
+ *ctxid = tsec->sid;
+ else
+ *ctxid = 0;
+}
diff --git a/security/selinux/ss/mls.c b/security/selinux/ss/mls.c
index 640d0bf..fc34f87 100644
--- a/security/selinux/ss/mls.c
+++ b/security/selinux/ss/mls.c
@@ -8,7 +8,7 @@
*
* Support for enhanced MLS infrastructure.
*
- * Copyright (C) 2004-2005 Trusted Computer Solutions, Inc.
+ * Copyright (C) 2004-2006 Trusted Computer Solutions, Inc.
*/

#include <linux/kernel.h>
@@ -385,6 +385,34 @@ out:
}

/*
+ * Set the MLS fields in the security context structure
+ * `context' based on the string representation in
+ * the string `str'. This function will allocate temporary memory with the
+ * given constraints of gfp_mask.
+ */
+int mls_from_string(char *str, struct context *context, gfp_t gfp_mask)
+{
+ char *tmpstr, *freestr;
+ int rc;
+
+ if (!selinux_mls_enabled)
+ return -EINVAL;
+
+ /* we need freestr because mls_context_to_sid will change
+ the value of tmpstr */
+ tmpstr = freestr = kstrdup(str, gfp_mask);
+ if (!tmpstr) {
+ rc = -ENOMEM;
+ } else {
+ rc = mls_context_to_sid(':', &tmpstr, context,
+ NULL, SECSID_NULL);
+ kfree(freestr);
+ }
+
+ return rc;
+}
+
+/*
* Copies the effective MLS range from `src' into `dst'.
*/
static inline int mls_scopy_context(struct context *dst,
diff --git a/security/selinux/ss/mls.h b/security/selinux/ss/mls.h
index 03de697..fbb42f0 100644
--- a/security/selinux/ss/mls.h
+++ b/security/selinux/ss/mls.h
@@ -8,7 +8,7 @@
*
* Support for enhanced MLS infrastructure.
*
- * Copyright (C) 2004-2005 Trusted Computer Solutions, Inc.
+ * Copyright (C) 2004-2006 Trusted Computer Solutions, Inc.
*/

#ifndef _SS_MLS_H_
@@ -27,6 +27,8 @@ int mls_context_to_sid(char oldc,
struct sidtab *s,
u32 def_sid);

+int mls_from_string(char *str, struct context *context, gfp_t gfp_mask);
+
int mls_convert_context(struct policydb *oldp,
struct policydb *newp,
struct context *context);
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index d877cd1..ce9378e 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -7,12 +7,13 @@
* Updated: Trusted Computer Solutions, Inc. <***@trustedcs.com>
*
* Support for enhanced MLS infrastructure.
+ * Support for context based audit filters.
*
* Updated: Frank Mayer <***@tresys.com> and Karl MacMillan <***@tresys.com>
*
* Added conditional policy language extensions
*
- * Copyright (C) 2004-2005 Trusted Computer Solutions, Inc.
+ * Copyright (C) 2004-2006 Trusted Computer Solutions, Inc.
* Copyright (C) 2003 - 2004 Tresys Technology, LLC
* Copyright (C) 2003 Red Hat, Inc., James Morris <***@redhat.com>
* This program is free software; you can redistribute it and/or modify
@@ -1810,3 +1811,235 @@ out:
POLICY_RDUNLOCK;
return rc;
}
+
+struct selinux_audit_rule {
+ u32 au_seqno;
+ struct context au_ctxt;
+};
+
+void selinux_audit_rule_free(struct selinux_audit_rule *rule)
+{
+ if (rule) {
+ context_destroy(&rule->au_ctxt);
+ kfree(rule);
+ }
+}
+
+int selinux_audit_rule_init(u32 field, u32 op, char *rulestr,
+ struct selinux_audit_rule **rule)
+{
+ struct selinux_audit_rule *tmprule;
+ struct role_datum *roledatum;
+ struct type_datum *typedatum;
+ struct user_datum *userdatum;
+ int rc = 0;
+
+ *rule = NULL;
+
+ if (!ss_initialized)
+ return -ENOTSUPP;
+
+ switch (field) {
+ case AUDIT_SE_USER:
+ case AUDIT_SE_ROLE:
+ case AUDIT_SE_TYPE:
+ /* only 'equals' and 'not equals' fit user, role, and type */
+ if (op != AUDIT_EQUAL && op != AUDIT_NOT_EQUAL)
+ return -EINVAL;
+ break;
+ case AUDIT_SE_SEN:
+ case AUDIT_SE_CLR:
+ /* we do not allow a range, indicated by the presense of '-' */
+ if (strchr(rulestr, '-'))
+ return -EINVAL;
+ break;
+ default:
+ /* only the above fields are valid */
+ return -EINVAL;
+ }
+
+ tmprule = kzalloc(sizeof(struct selinux_audit_rule), GFP_KERNEL);
+ if (!tmprule)
+ return -ENOMEM;
+
+ context_init(&tmprule->au_ctxt);
+
+ POLICY_RDLOCK;
+
+ tmprule->au_seqno = latest_granting;
+
+ switch (field) {
+ case AUDIT_SE_USER:
+ userdatum = hashtab_search(policydb.p_users.table, rulestr);
+ if (!userdatum)
+ rc = -EINVAL;
+ else
+ tmprule->au_ctxt.user = userdatum->value;
+ break;
+ case AUDIT_SE_ROLE:
+ roledatum = hashtab_search(policydb.p_roles.table, rulestr);
+ if (!roledatum)
+ rc = -EINVAL;
+ else
+ tmprule->au_ctxt.role = roledatum->value;
+ break;
+ case AUDIT_SE_TYPE:
+ typedatum = hashtab_search(policydb.p_types.table, rulestr);
+ if (!typedatum)
+ rc = -EINVAL;
+ else
+ tmprule->au_ctxt.type = typedatum->value;
+ break;
+ case AUDIT_SE_SEN:
+ case AUDIT_SE_CLR:
+ rc = mls_from_string(rulestr, &tmprule->au_ctxt, GFP_ATOMIC);
+ break;
+ }
+
+ POLICY_RDUNLOCK;
+
+ if (rc) {
+ selinux_audit_rule_free(tmprule);
+ tmprule = NULL;
+ }
+
+ *rule = tmprule;
+
+ return rc;
+}
+
+int selinux_audit_rule_match(u32 ctxid, u32 field, u32 op,
+ struct selinux_audit_rule *rule,
+ struct audit_context *actx)
+{
+ struct context *ctxt;
+ struct mls_level *level;
+ int match = 0;
+
+ if (!rule) {
+ audit_log(actx, GFP_ATOMIC, AUDIT_SELINUX_ERR,
+ "selinux_audit_rule_match: missing rule\n");
+ return -ENOENT;
+ }
+
+ POLICY_RDLOCK;
+
+ if (rule->au_seqno < latest_granting) {
+ audit_log(actx, GFP_ATOMIC, AUDIT_SELINUX_ERR,
+ "selinux_audit_rule_match: stale rule\n");
+ match = -ESTALE;
+ goto out;
+ }
+
+ ctxt = sidtab_search(&sidtab, ctxid);
+ if (!ctxt) {
+ audit_log(actx, GFP_ATOMIC, AUDIT_SELINUX_ERR,
+ "selinux_audit_rule_match: unrecognized SID %d\n",
+ ctxid);
+ match = -ENOENT;
+ goto out;
+ }
+
+ /* a field/op pair that is not caught here will simply fall through
+ without a match */
+ switch (field) {
+ case AUDIT_SE_USER:
+ switch (op) {
+ case AUDIT_EQUAL:
+ match = (ctxt->user == rule->au_ctxt.user);
+ break;
+ case AUDIT_NOT_EQUAL:
+ match = (ctxt->user != rule->au_ctxt.user);
+ break;
+ }
+ break;
+ case AUDIT_SE_ROLE:
+ switch (op) {
+ case AUDIT_EQUAL:
+ match = (ctxt->role == rule->au_ctxt.role);
+ break;
+ case AUDIT_NOT_EQUAL:
+ match = (ctxt->role != rule->au_ctxt.role);
+ break;
+ }
+ break;
+ case AUDIT_SE_TYPE:
+ switch (op) {
+ case AUDIT_EQUAL:
+ match = (ctxt->type == rule->au_ctxt.type);
+ break;
+ case AUDIT_NOT_EQUAL:
+ match = (ctxt->type != rule->au_ctxt.type);
+ break;
+ }
+ break;
+ case AUDIT_SE_SEN:
+ case AUDIT_SE_CLR:
+ level = (op == AUDIT_SE_SEN ?
+ &ctxt->range.level[0] : &ctxt->range.level[1]);
+ switch (op) {
+ case AUDIT_EQUAL:
+ match = mls_level_eq(&rule->au_ctxt.range.level[0],
+ level);
+ break;
+ case AUDIT_NOT_EQUAL:
+ match = !mls_level_eq(&rule->au_ctxt.range.level[0],
+ level);
+ break;
+ case AUDIT_LESS_THAN:
+ match = (mls_level_dom(&rule->au_ctxt.range.level[0],
+ level) &&
+ !mls_level_eq(&rule->au_ctxt.range.level[0],
+ level));
+ break;
+ case AUDIT_LESS_THAN_OR_EQUAL:
+ match = mls_level_dom(&rule->au_ctxt.range.level[0],
+ level);
+ break;
+ case AUDIT_GREATER_THAN:
+ match = (mls_level_dom(level,
+ &rule->au_ctxt.range.level[0]) &&
+ !mls_level_eq(level,
+ &rule->au_ctxt.range.level[0]));
+ break;
+ case AUDIT_GREATER_THAN_OR_EQUAL:
+ match = mls_level_dom(level,
+ &rule->au_ctxt.range.level[0]);
+ break;
+ }
+ }
+
+out:
+ POLICY_RDUNLOCK;
+ return match;
+}
+
+static int (*aurule_callback)(void) = NULL;
+
+static int aurule_avc_callback(u32 event, u32 ssid, u32 tsid,
+ u16 class, u32 perms, u32 *retained)
+{
+ int err = 0;
+
+ if (event == AVC_CALLBACK_RESET && aurule_callback)
+ err = aurule_callback();
+ return err;
+}
+
+static int __init aurule_init(void)
+{
+ int err;
+
+ err = avc_add_callback(aurule_avc_callback, AVC_CALLBACK_RESET,
+ SECSID_NULL, SECSID_NULL, SECCLASS_NULL, 0);
+ if (err)
+ panic("avc_add_callback() failed, error %d\n", err);
+
+ return err;
+}
+__initcall(aurule_init);
+
+void selinux_audit_set_callback(int (*callback)(void))
+{
+ aurule_callback = callback;
+}
--
Darrel
Darrel Goeddel
2006-02-24 22:26:13 UTC
Permalink
Darrel Goeddel wrote:
<snip>
Post by Darrel Goeddel
The following patch provides selinux interfaces that will allow the audit
system to perform filtering based on the process context (user, role, type,
sensitivity, and clearance). These interfaces will allow the selinux
module to perform efficient matches based on lower level selinux constructs,
rather than relying on context retrievals and string comparisons within
the audit module. It also allows for dominance checks on the mls portion
of the contexts that are impossible with only string comparisons.
I have updated the audit portion of the patch to match up with this latest selinux
patch.

The last update to that patch was here:
http://marc.theaimsgroup.com/?l=selinux&m=114056729028852&w=2

I have also reworked the update procedure of the se_rule fields to hopefully be
in line with the requirement of RCU. Consider this portion of the patch for
*testing only*. The other portions of the patch seem good to me so far.

--

diff --git a/kernel/audit.h b/kernel/audit.h
index eb33354..3ffc70a 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -59,9 +59,11 @@ struct audit_watch {
};

struct audit_field {
- u32 type;
- u32 val;
- u32 op;
+ u32 type;
+ u32 val;
+ u32 op;
+ char *se_str;
+ struct selinux_audit_rule *se_rule;
};

struct audit_krule {
diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index 3712295..752e2bb 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -25,6 +25,7 @@
#include <linux/fs.h>
#include <linux/namei.h>
#include <linux/netlink.h>
+#include <linux/selinux.h>
#include "audit.h"

/* There are three lists of rules -- one to search at task creation
@@ -50,6 +51,13 @@ static inline void audit_free_watch(stru

static inline void audit_free_rule(struct audit_entry *e)
{
+ int i;
+ if (e->rule.fields)
+ for (i = 0; i < e->rule.field_count; i++) {
+ struct audit_field *f = &e->rule.fields[i];
+ kfree(f->se_str);
+ selinux_audit_rule_free(f->se_rule);
+ }
kfree(e->rule.fields);
kfree(e);
}
@@ -192,7 +200,12 @@ static struct audit_entry *audit_rule_to
f->val = rule->values[i];

if (f->type & AUDIT_UNUSED_BITS ||
- f->type == AUDIT_WATCH) {
+ f->type == AUDIT_WATCH ||
+ f->type == AUDIT_SE_USER ||
+ f->type == AUDIT_SE_ROLE ||
+ f->type == AUDIT_SE_TYPE ||
+ f->type == AUDIT_SE_SEN ||
+ f->type == AUDIT_SE_CLR) {
err = -EINVAL;
goto exit_free;
}
@@ -222,7 +235,7 @@ static struct audit_entry *audit_data_to
void *bufp;
size_t remain = datasz - sizeof(struct audit_rule_data);
int i;
- char *path;
+ char *str;

entry = audit_to_entry_common((struct audit_rule *)data);
if (IS_ERR(entry))
@@ -241,16 +254,42 @@ static struct audit_entry *audit_data_to
f->op = data->fieldflags[i] & AUDIT_OPERATORS;
f->type = data->fields[i];
f->val = data->values[i];
+ f->se_str = NULL;
+ f->se_rule = NULL;
switch(f->type) {
+ case AUDIT_SE_USER:
+ case AUDIT_SE_ROLE:
+ case AUDIT_SE_TYPE:
+ case AUDIT_SE_SEN:
+ case AUDIT_SE_CLR:
+ str = audit_unpack_string(&bufp, &remain, f->val);
+ if (IS_ERR(str))
+ goto exit_free;
+ entry->rule.buflen += f->val;
+
+ err = selinux_audit_rule_init(f->type, f->op, str,
+ &f->se_rule);
+ /* Keep currently invalid fields around in case they
+ become valid after a policy reload. */
+ if (err == -EINVAL) {
+ printk(KERN_WARNING "selinux audit rule for item %s is invalid\n", str);
+ err = 0;
+ }
+ if (err) {
+ kfree(str);
+ goto exit_free;
+ } else
+ f->se_str = str;
+ break;
case AUDIT_WATCH:
- path = audit_unpack_string(&bufp, &remain, f->val);
- if (IS_ERR(path))
+ str = audit_unpack_string(&bufp, &remain, f->val);
+ if (IS_ERR(str))
goto exit_free;
entry->rule.buflen += f->val;

- err = audit_to_watch(path, &entry->rule, i);
+ err = audit_to_watch(str, &entry->rule, i);
if (err) {
- kfree(path);
+ kfree(str);
goto exit_free;
}
break;
@@ -333,6 +372,14 @@ static struct audit_rule_data *audit_kru
data->buflen += data->values[i] =
audit_pack_string(&bufp, krule->watch->path);
break;
+ case AUDIT_SE_USER:
+ case AUDIT_SE_ROLE:
+ case AUDIT_SE_TYPE:
+ case AUDIT_SE_SEN:
+ case AUDIT_SE_CLR:
+ data->buflen += data->values[i] =
+ audit_pack_string(&bufp, f->se_str);
+ break;
default:
data->values[i] = f->val;
}
@@ -370,6 +417,14 @@ static int audit_compare_rule(struct aud
if (audit_compare_watch(a->watch, b->watch))
return 1;
break;
+ case AUDIT_SE_USER:
+ case AUDIT_SE_ROLE:
+ case AUDIT_SE_TYPE:
+ case AUDIT_SE_SEN:
+ case AUDIT_SE_CLR:
+ if (strcmp(a->fields[i].se_str, b->fields[i].se_str))
+ return 1;
+ break;
default:
if (a->fields[i].val != b->fields[i].val)
return 1;
@@ -640,6 +695,9 @@ int audit_comparator(const u32 left, con
default:
return -EINVAL;
}
+ /* should NEVER get here */
+ BUG();
+ return 0;
}


@@ -726,3 +784,143 @@ unlock_and_return:
rcu_read_unlock();
return result;
}
+
+/* Check to see if the rule contains any selinux fields. Returns 1 if there
+ are selinux fields specified in the rule, 0 otherwise. */
+static inline int audit_rule_has_selinux(struct audit_krule *rule)
+{
+ int i;
+
+ for (i = 0; i < rule->field_count; i++) {
+ struct audit_field *f = &rule->fields[i];
+ switch (f->type) {
+ case AUDIT_SE_USER:
+ case AUDIT_SE_ROLE:
+ case AUDIT_SE_TYPE:
+ case AUDIT_SE_SEN:
+ case AUDIT_SE_CLR:
+ return 1;
+ }
+ }
+
+ return 0;
+}
+
+/* Make a copy of src in dest. This will be a deep copy with the exception
+ of the watch - that pointer is carried over. The selinux specific fields
+ will be updated in the copy. The point is to be able to replace the src
+ rule with the dest rule in the list, then free the dest rule. */
+static inline int selinux_audit_rule_update_helper(struct audit_krule *dest,
+ struct audit_krule *src)
+{
+ int i, err = 0;
+
+ dest->vers_ops = src->vers_ops;
+ dest->flags = src->flags;
+ dest->listnr = src->listnr;
+ dest->action = src->action;
+ for (i = 0; i < AUDIT_BITMASK_SIZE; i++)
+ dest->mask[i] = src->mask[i];
+ dest->buflen = src->buflen;
+ dest->field_count = src->field_count;
+
+ /* deep copy this information, updating the se_rule fields, because
+ the originals will all be freed when the old rule is freed. */
+ dest->fields = kzalloc(sizeof(struct audit_field) * dest->field_count,
+ GFP_ATOMIC);
+ if (!dest->fields)
+ return -ENOMEM;
+ memcpy(dest->fields, src->fields,
+ sizeof(struct audit_field) * dest->field_count);
+ for (i = 0; i < dest->field_count; i++) {
+ struct audit_field *df = &dest->fields[i];
+ struct audit_field *sf = &src->fields[i];
+ switch (df->type) {
+ case AUDIT_SE_USER:
+ case AUDIT_SE_ROLE:
+ case AUDIT_SE_TYPE:
+ case AUDIT_SE_SEN:
+ case AUDIT_SE_CLR:
+ /* our own copy of se_str */
+ df->se_str = kstrdup(sf->se_str, GFP_ATOMIC);
+ /* our own (refreshed) copy of se_rule */
+ err = selinux_audit_rule_init(df->type, df->op,
+ df->se_str, &df->se_rule);
+ /* Keep currently invalid fields around in case they
+ become valid after a policy reload. */
+ if (err == -EINVAL) {
+ printk(KERN_WARNING "selinux audit rule for item %s is invalid\n", df->se_str);
+ err = 0;
+ }
+ if (err)
+ return err;
+ }
+ }
+
+ /* we can shallow copy the watch because we will not be freeing it via
+ selinux_audit_rule_update (and we do nto modify it) */
+ dest->watch = src->watch;
+ dest->rlist = src->rlist;
+
+ return 0;
+}
+
+/* This function will re-initialize the se_rule field of all applicable rules.
+ It will traverse the filter lists serarching for rules that contain selinux
+ specific filter fields. When such a rule is found, it is copied, the
+ selinux field is re-initialized, and the old rule is replaced with the
+ updated rule. */
+/* XXX: is the error handling below appropriate */
+static int selinux_audit_rule_update(void)
+{
+ struct audit_entry *entry, *nentry;
+ int i, err = 0, tmperr;
+
+ /* audit_netlink_sem synchronizes the writers */
+ down(&audit_netlink_sem);
+
+ for (i = 0; i < AUDIT_NR_FILTERS; i++) {
+ list_for_each_entry(entry, &audit_filter_list[i], list) {
+ if (!audit_rule_has_selinux(&entry->rule))
+ continue;
+
+ nentry = kmalloc(sizeof(*entry), GFP_ATOMIC);
+ if (!nentry) {
+ /* save the first error encountered for the
+ return value */
+ if (!err)
+ err = -ENOMEM;
+ audit_panic("error updating selinux filters");
+ continue;
+ }
+
+ tmperr = selinux_audit_rule_update_helper(&nentry->rule,
+ &entry->rule);
+ if (!nentry) {
+ /* save the first error encountered for the
+ return value */
+ if (!err)
+ err = -ENOMEM;
+ audit_free_rule(nentry);
+ audit_panic("error updating selinux filters");
+ continue;
+ }
+ list_replace_rcu(&entry->list, &nentry->list);
+ call_rcu(&entry->rcu, audit_free_rule_rcu);
+ }
+ }
+
+ up(&audit_netlink_sem);
+
+ return err;
+}
+
+/* Register the callback with selinux. This callback will be invoked when a
+ new policy is loaded. */
+static int __init register_selinux_update_callback(void)
+{
+ selinux_audit_set_callback(&selinux_audit_rule_update);
+ return 0;
+}
+__initcall(register_selinux_update_callback);
+
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index cd83289..f117563 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -58,6 +58,7 @@
#include <linux/security.h>
#include <linux/list.h>
#include <linux/tty.h>
+#include <linux/selinux.h>

#include "audit.h"

@@ -168,6 +169,9 @@ static int audit_filter_rules(struct tas
enum audit_state *state)
{
int i, j;
+ u32 sid;
+
+ selinux_task_ctxid(tsk, &sid);

for (i = 0; i < rule->field_count; i++) {
struct audit_field *f = &rule->fields[i];
@@ -258,6 +262,22 @@ static int audit_filter_rules(struct tas
if (ctx)
result = audit_comparator(ctx->loginuid, f->op, f->val);
break;
+ case AUDIT_SE_USER:
+ case AUDIT_SE_ROLE:
+ case AUDIT_SE_TYPE:
+ case AUDIT_SE_SEN:
+ case AUDIT_SE_CLR:
+ /* NOTE: this may return negative values indicating
+ a temporary error. We simply treat this as a
+ match for now to avoid losing information that
+ may be wanted. An error message will also be
+ logged upon error */
+ if (f->se_rule)
+ result = selinux_audit_rule_match(sid, f->type,
+ f->op,
+ f->se_rule,
+ ctx);
+ break;
case AUDIT_ARG0:
case AUDIT_ARG1:
case AUDIT_ARG2:
--
Darrel
Amy Griffis
2006-03-07 18:02:30 UTC
Permalink
Hi Darrel,

I didn't notice this patch in my inbox until just recently. I've put
a few comments inline.
Post by Darrel Goeddel
diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index 3712295..752e2bb 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -25,6 +25,7 @@
#include <linux/fs.h>
#include <linux/namei.h>
#include <linux/netlink.h>
+#include <linux/selinux.h>
#include "audit.h"
/* There are three lists of rules -- one to search at task creation
@@ -50,6 +51,13 @@ static inline void audit_free_watch(stru
static inline void audit_free_rule(struct audit_entry *e)
{
+ int i;
+ if (e->rule.fields)
+ for (i = 0; i < e->rule.field_count; i++) {
+ struct audit_field *f = &e->rule.fields[i];
+ kfree(f->se_str);
+ selinux_audit_rule_free(f->se_rule);
+ }
kfree(e->rule.fields);
kfree(e);
}
@@ -192,7 +200,12 @@ static struct audit_entry *audit_rule_to
f->val = rule->values[i];
if (f->type & AUDIT_UNUSED_BITS ||
- f->type == AUDIT_WATCH) {
+ f->type == AUDIT_WATCH ||
+ f->type == AUDIT_SE_USER ||
+ f->type == AUDIT_SE_ROLE ||
+ f->type == AUDIT_SE_TYPE ||
+ f->type == AUDIT_SE_SEN ||
+ f->type == AUDIT_SE_CLR) {
err = -EINVAL;
goto exit_free;
}
@@ -222,7 +235,7 @@ static struct audit_entry *audit_data_to
void *bufp;
size_t remain = datasz - sizeof(struct audit_rule_data);
int i;
- char *path;
+ char *str;
entry = audit_to_entry_common((struct audit_rule *)data);
if (IS_ERR(entry))
@@ -241,16 +254,42 @@ static struct audit_entry *audit_data_to
f->op = data->fieldflags[i] & AUDIT_OPERATORS;
f->type = data->fields[i];
f->val = data->values[i];
+ f->se_str = NULL;
+ f->se_rule = NULL;
switch(f->type) {
+ str = audit_unpack_string(&bufp, &remain, f->val);
+ if (IS_ERR(str))
+ goto exit_free;
+ entry->rule.buflen += f->val;
+
+ err = selinux_audit_rule_init(f->type, f->op, str,
+ &f->se_rule);
+ /* Keep currently invalid fields around in case they
+ become valid after a policy reload. */
+ if (err == -EINVAL) {
+ printk(KERN_WARNING "selinux audit rule for
item %s is invalid\n", str);
+ err = 0;
+ }
+ if (err) {
+ kfree(str);
+ goto exit_free;
+ } else
+ f->se_str = str;
+ break;
- path = audit_unpack_string(&bufp, &remain, f->val);
- if (IS_ERR(path))
+ str = audit_unpack_string(&bufp, &remain, f->val);
+ if (IS_ERR(str))
goto exit_free;
entry->rule.buflen += f->val;
- err = audit_to_watch(path, &entry->rule, i);
+ err = audit_to_watch(str, &entry->rule, i);
if (err) {
- kfree(path);
+ kfree(str);
goto exit_free;
}
break;
@@ -333,6 +372,14 @@ static struct audit_rule_data *audit_kru
data->buflen += data->values[i] =
audit_pack_string(&bufp, krule->watch->path);
break;
+ data->buflen += data->values[i] =
+ audit_pack_string(&bufp, f->se_str);
+ break;
data->values[i] = f->val;
}
@@ -370,6 +417,14 @@ static int audit_compare_rule(struct aud
if (audit_compare_watch(a->watch, b->watch))
return 1;
break;
+ if (strcmp(a->fields[i].se_str, b->fields[i].se_str))
+ return 1;
+ break;
if (a->fields[i].val != b->fields[i].val)
return 1;
@@ -640,6 +695,9 @@ int audit_comparator(const u32 left, con
return -EINVAL;
}
+ /* should NEVER get here */
+ BUG();
+ return 0;
}
rcu_read_unlock();
return result;
}
+
+/* Check to see if the rule contains any selinux fields. Returns 1 if there
+ are selinux fields specified in the rule, 0 otherwise. */
+static inline int audit_rule_has_selinux(struct audit_krule *rule)
+{
+ int i;
+
+ for (i = 0; i < rule->field_count; i++) {
+ struct audit_field *f = &rule->fields[i];
+ switch (f->type) {
+ return 1;
+ }
+ }
+
+ return 0;
+}
+
+/* Make a copy of src in dest. This will be a deep copy with the exception
+ of the watch - that pointer is carried over. The selinux specific fields
+ will be updated in the copy. The point is to be able to replace the src
+ rule with the dest rule in the list, then free the dest rule. */
+static inline int selinux_audit_rule_update_helper(struct audit_krule *dest,
+ struct audit_krule *src)
+{
+ int i, err = 0;
+
+ dest->vers_ops = src->vers_ops;
+ dest->flags = src->flags;
+ dest->listnr = src->listnr;
+ dest->action = src->action;
+ for (i = 0; i < AUDIT_BITMASK_SIZE; i++)
+ dest->mask[i] = src->mask[i];
+ dest->buflen = src->buflen;
+ dest->field_count = src->field_count;
+
+ /* deep copy this information, updating the se_rule fields, because
+ the originals will all be freed when the old rule is freed. */
+ dest->fields = kzalloc(sizeof(struct audit_field) *
dest->field_count,
+ GFP_ATOMIC);
+ if (!dest->fields)
+ return -ENOMEM;
It seems like it would be cleaner to group the memory allocations for
the rule entry and the fields into a single function, as any audit
rule must always have both. This would allow you to retain the
assumption of the existence of fields in audit_free_rule.
Post by Darrel Goeddel
+ memcpy(dest->fields, src->fields,
+ sizeof(struct audit_field) * dest->field_count);
+ for (i = 0; i < dest->field_count; i++) {
+ struct audit_field *df = &dest->fields[i];
+ struct audit_field *sf = &src->fields[i];
+ switch (df->type) {
+ /* our own copy of se_str */
+ df->se_str = kstrdup(sf->se_str, GFP_ATOMIC);
I realize failure is unlikely here, but shouldn't you check the return
value? Later on you'll end up passing this to strcmp() which won't
like it if it's NULL.
Post by Darrel Goeddel
+ /* our own (refreshed) copy of se_rule */
+ err = selinux_audit_rule_init(df->type, df->op,
+ df->se_str,
&df->se_rule);
+ /* Keep currently invalid fields around in case they
+ become valid after a policy reload. */
+ if (err == -EINVAL) {
+ printk(KERN_WARNING "selinux audit rule for
item %s is invalid\n", df->se_str);
+ err = 0;
+ }
+ if (err)
+ return err;
+ }
+ }
+
+ /* we can shallow copy the watch because we will not be freeing it via
+ selinux_audit_rule_update (and we do nto modify it) */
On the flipside, when audit watches are updated, the se_str and
se_rule will need to be deep copied since they are freed in
audit_free_rule(). Is this what you intend?
Post by Darrel Goeddel
+ dest->watch = src->watch;
+ dest->rlist = src->rlist;
+
+ return 0;
+}
+
+/* This function will re-initialize the se_rule field of all applicable rules.
+ It will traverse the filter lists serarching for rules that contain selinux
+ specific filter fields. When such a rule is found, it is copied, the
+ selinux field is re-initialized, and the old rule is replaced with the
+ updated rule. */
+/* XXX: is the error handling below appropriate */
+static int selinux_audit_rule_update(void)
+{
+ struct audit_entry *entry, *nentry;
+ int i, err = 0, tmperr;
+
+ /* audit_netlink_sem synchronizes the writers */
+ down(&audit_netlink_sem);
+
+ for (i = 0; i < AUDIT_NR_FILTERS; i++) {
+ list_for_each_entry(entry, &audit_filter_list[i], list) {
+ if (!audit_rule_has_selinux(&entry->rule))
+ continue;
+
+ nentry = kmalloc(sizeof(*entry), GFP_ATOMIC);
+ if (!nentry) {
+ /* save the first error encountered for the
+ return value */
+ if (!err)
+ err = -ENOMEM;
+ audit_panic("error updating selinux
filters");
+ continue;
+ }
+
+ tmperr =
selinux_audit_rule_update_helper(&nentry->rule,
+
&entry->rule);
+ if (!nentry) {
How would we end up with !nentry here? Maybe you mean if (temperr) ?
Post by Darrel Goeddel
+ /* save the first error encountered for the
+ return value */
+ if (!err)
+ err = -ENOMEM;
You don't want to hardcode the -ENOMEM as selinux_audit_rule_init()
can also return an error.
Post by Darrel Goeddel
+ audit_free_rule(nentry);
+ audit_panic("error updating selinux
filters");
+ continue;
+ }
+ list_replace_rcu(&entry->list, &nentry->list);
+ call_rcu(&entry->rcu, audit_free_rule_rcu);
+ }
+ }
+
+ up(&audit_netlink_sem);
+
+ return err;
+}
+
+/* Register the callback with selinux. This callback will be invoked when a
+ new policy is loaded. */
+static int __init register_selinux_update_callback(void)
+{
+ selinux_audit_set_callback(&selinux_audit_rule_update);
+ return 0;
+}
+__initcall(register_selinux_update_callback);
+
I don't know about anyone else, but I would prefer to keep all of the
initialization for the audit subsystem in audit.c:audit_init(). This
makes the audit initialization path more easily synchronized and
readable.

Regards,
Amy
Darrel Goeddel
2006-03-08 18:46:51 UTC
Permalink
Post by Amy Griffis
Hi Darrel,
I didn't notice this patch in my inbox until just recently. I've put
a few comments inline.
Post by Darrel Goeddel
diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index 3712295..752e2bb 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
<snip>
Post by Amy Griffis
Post by Darrel Goeddel
rcu_read_unlock();
return result;
}
+
+/* Check to see if the rule contains any selinux fields. Returns 1 if there
+ are selinux fields specified in the rule, 0 otherwise. */
+static inline int audit_rule_has_selinux(struct audit_krule *rule)
+{
+ int i;
+
+ for (i = 0; i < rule->field_count; i++) {
+ struct audit_field *f = &rule->fields[i];
+ switch (f->type) {
+ return 1;
+ }
+ }
+
+ return 0;
+}
+
+/* Make a copy of src in dest. This will be a deep copy with the exception
+ of the watch - that pointer is carried over. The selinux specific fields
+ will be updated in the copy. The point is to be able to replace the src
+ rule with the dest rule in the list, then free the dest rule. */
+static inline int selinux_audit_rule_update_helper(struct audit_krule *dest,
+ struct audit_krule *src)
+{
+ int i, err = 0;
+
+ dest->vers_ops = src->vers_ops;
+ dest->flags = src->flags;
+ dest->listnr = src->listnr;
+ dest->action = src->action;
+ for (i = 0; i < AUDIT_BITMASK_SIZE; i++)
+ dest->mask[i] = src->mask[i];
+ dest->buflen = src->buflen;
+ dest->field_count = src->field_count;
+
+ /* deep copy this information, updating the se_rule fields, because
+ the originals will all be freed when the old rule is freed. */
+ dest->fields = kzalloc(sizeof(struct audit_field) *
dest->field_count,
+ GFP_ATOMIC);
+ if (!dest->fields)
+ return -ENOMEM;
It seems like it would be cleaner to group the memory allocations for
the rule entry and the fields into a single function, as any audit
rule must always have both. This would allow you to retain the
assumption of the existence of fields in audit_free_rule.
I originally went with that approach, but later decided to break it out
to make a generic copy function (which I scrapped in favor of the
specialized helper because I didn't know anything about the watches...).
I'm assuming that this whole thing will need modification based on your
continued fs auditing work. I'll keep this in mind for the next version.
Post by Amy Griffis
Post by Darrel Goeddel
+ memcpy(dest->fields, src->fields,
+ sizeof(struct audit_field) * dest->field_count);
+ for (i = 0; i < dest->field_count; i++) {
+ struct audit_field *df = &dest->fields[i];
+ struct audit_field *sf = &src->fields[i];
+ switch (df->type) {
+ /* our own copy of se_str */
+ df->se_str = kstrdup(sf->se_str, GFP_ATOMIC);
I realize failure is unlikely here, but shouldn't you check the return
value? Later on you'll end up passing this to strcmp() which won't
like it if it's NULL.
Yep. Thanks.
Post by Amy Griffis
Post by Darrel Goeddel
+ /* our own (refreshed) copy of se_rule */
+ err = selinux_audit_rule_init(df->type, df->op,
+ df->se_str,
&df->se_rule);
+ /* Keep currently invalid fields around in case they
+ become valid after a policy reload. */
+ if (err == -EINVAL) {
+ printk(KERN_WARNING "selinux audit rule for
item %s is invalid\n", df->se_str);
+ err = 0;
+ }
+ if (err)
+ return err;
+ }
+ }
+
+ /* we can shallow copy the watch because we will not be freeing it via
+ selinux_audit_rule_update (and we do nto modify it) */
On the flipside, when audit watches are updated, the se_str and
se_rule will need to be deep copied since they are freed in
audit_free_rule(). Is this what you intend?
As I mentioned before, I'm not really sure on the whole watch situation.
Sounds right though. se_str will need to be copied and se_rule would need
to be re-initialized using selinux_audit_rule_init() because it is an
opaque item.
Post by Amy Griffis
Post by Darrel Goeddel
+ dest->watch = src->watch;
+ dest->rlist = src->rlist;
+
+ return 0;
+}
+
+/* This function will re-initialize the se_rule field of all applicable rules.
+ It will traverse the filter lists serarching for rules that contain selinux
+ specific filter fields. When such a rule is found, it is copied, the
+ selinux field is re-initialized, and the old rule is replaced with the
+ updated rule. */
+/* XXX: is the error handling below appropriate */
+static int selinux_audit_rule_update(void)
+{
+ struct audit_entry *entry, *nentry;
+ int i, err = 0, tmperr;
+
+ /* audit_netlink_sem synchronizes the writers */
+ down(&audit_netlink_sem);
+
+ for (i = 0; i < AUDIT_NR_FILTERS; i++) {
+ list_for_each_entry(entry, &audit_filter_list[i], list) {
+ if (!audit_rule_has_selinux(&entry->rule))
+ continue;
+
+ nentry = kmalloc(sizeof(*entry), GFP_ATOMIC);
+ if (!nentry) {
+ /* save the first error encountered for the
+ return value */
+ if (!err)
+ err = -ENOMEM;
+ audit_panic("error updating selinux
filters");
+ continue;
+ }
+
+ tmperr =
selinux_audit_rule_update_helper(&nentry->rule,
+
&entry->rule);
+ if (!nentry) {
How would we end up with !nentry here? Maybe you mean if (temperr) ?
Yep.
Post by Amy Griffis
Post by Darrel Goeddel
+ /* save the first error encountered for the
+ return value */
+ if (!err)
+ err = -ENOMEM;
You don't want to hardcode the -ENOMEM as selinux_audit_rule_init()
can also return an error.
Yep. Should be tmperr.
Post by Amy Griffis
Post by Darrel Goeddel
+ audit_free_rule(nentry);
+ audit_panic("error updating selinux
filters");
+ continue;
+ }
+ list_replace_rcu(&entry->list, &nentry->list);
+ call_rcu(&entry->rcu, audit_free_rule_rcu);
+ }
+ }
+
+ up(&audit_netlink_sem);
+
+ return err;
+}
+
+/* Register the callback with selinux. This callback will be invoked when a
+ new policy is loaded. */
+static int __init register_selinux_update_callback(void)
+{
+ selinux_audit_set_callback(&selinux_audit_rule_update);
+ return 0;
+}
+__initcall(register_selinux_update_callback);
+
I don't know about anyone else, but I would prefer to keep all of the
initialization for the audit subsystem in audit.c:audit_init(). This
makes the audit initialization path more easily synchronized and
readable.
That can be done. I liked keeping this all scoped together. We could put
this part in audit_init (audit.c) and put the declaration for
selinux_audit_rule_update in kernel/audit.h.

Thanks for the feedback. I'm sure this stuff will look much better when
integrated with your audit fs work.
--
Darrel
Darrel Goeddel
2006-03-09 19:27:40 UTC
Permalink
Here is an update of this patch incorporating some of the feedback
(not all yet - just some bug fixes). This is most important feature
is that it is rebased to the latest audit-current git tree (with
some more help from Amy - thanks again).
--
Darrel


diff --git a/kernel/audit.h b/kernel/audit.h
index bc53920..051ac2a 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -54,9 +54,11 @@ enum audit_state {

/* Rule lists */
struct audit_field {
- u32 type;
- u32 val;
- u32 op;
+ u32 type;
+ u32 val;
+ u32 op;
+ char *se_str;
+ struct selinux_audit_rule *se_rule;
};

struct audit_krule {
diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index 45f3001..b115f51 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -23,6 +23,7 @@
#include <linux/audit.h>
#include <linux/kthread.h>
#include <linux/netlink.h>
+#include <linux/selinux.h>
#include "audit.h"

/* There are three lists of rules -- one to search at task creation
@@ -42,6 +43,13 @@ struct list_head audit_filter_list[AUDIT

static inline void audit_free_rule(struct audit_entry *e)
{
+ int i;
+ if (e->rule.fields)
+ for (i = 0; i < e->rule.field_count; i++) {
+ struct audit_field *f = &e->rule.fields[i];
+ kfree(f->se_str);
+ selinux_audit_rule_free(f->se_rule);
+ }
kfree(e->rule.fields);
kfree(e);
}
@@ -54,7 +62,7 @@ static inline void audit_free_rule_rcu(s

/* Unpack a filter field's string representation from user-space
* buffer. */
-static __attribute__((unused)) char *audit_unpack_string(void **bufp, size_t *remain, size_t len)
+static char *audit_unpack_string(void **bufp, size_t *remain, size_t len)
{
char *str;

@@ -150,15 +158,20 @@ static struct audit_entry *audit_rule_to
for (i = 0; i < rule->field_count; i++) {
struct audit_field *f = &entry->rule.fields[i];

- if (rule->fields[i] & AUDIT_UNUSED_BITS) {
- err = -EINVAL;
- goto exit_free;
- }
-
f->op = rule->fields[i] & (AUDIT_NEGATE|AUDIT_OPERATORS);
f->type = rule->fields[i] & ~(AUDIT_NEGATE|AUDIT_OPERATORS);
f->val = rule->values[i];

+ if (f->type & AUDIT_UNUSED_BITS ||
+ f->type == AUDIT_SE_USER ||
+ f->type == AUDIT_SE_ROLE ||
+ f->type == AUDIT_SE_TYPE ||
+ f->type == AUDIT_SE_SEN ||
+ f->type == AUDIT_SE_CLR) {
+ err = -EINVAL;
+ goto exit_free;
+ }
+
entry->rule.vers_ops = (f->op & AUDIT_OPERATORS) ? 2 : 1;

/* Support for legacy operators where
@@ -188,8 +201,9 @@ 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); */
+ size_t remain = datasz - sizeof(struct audit_rule_data);
int i;
+ char *str;

entry = audit_to_entry_common((struct audit_rule *)data);
if (IS_ERR(entry))
@@ -207,10 +221,34 @@ static struct audit_entry *audit_data_to

f->op = data->fieldflags[i] & AUDIT_OPERATORS;
f->type = data->fields[i];
+ f->val = data->values[i];
+ f->se_str = NULL;
+ f->se_rule = NULL;
switch(f->type) {
- /* call type-specific conversion routines here */
- default:
- f->val = data->values[i];
+ case AUDIT_SE_USER:
+ case AUDIT_SE_ROLE:
+ case AUDIT_SE_TYPE:
+ case AUDIT_SE_SEN:
+ case AUDIT_SE_CLR:
+ str = audit_unpack_string(&bufp, &remain, f->val);
+ if (IS_ERR(str))
+ goto exit_free;
+ entry->rule.buflen += f->val;
+
+ err = selinux_audit_rule_init(f->type, f->op, str,
+ &f->se_rule);
+ /* Keep currently invalid fields around in case they
+ * become valid after a policy reload. */
+ if (err == -EINVAL) {
+ printk(KERN_WARNING "selinux audit rule for item %s is invalid\n", str);
+ err = 0;
+ }
+ if (err) {
+ kfree(str);
+ goto exit_free;
+ } else
+ f->se_str = str;
+ break;
}
}

@@ -286,7 +324,14 @@ static struct audit_rule_data *audit_kru
data->fields[i] = f->type;
data->fieldflags[i] = f->op;
switch(f->type) {
- /* call type-specific conversion routines here */
+ case AUDIT_SE_USER:
+ case AUDIT_SE_ROLE:
+ case AUDIT_SE_TYPE:
+ case AUDIT_SE_SEN:
+ case AUDIT_SE_CLR:
+ data->buflen += data->values[i] =
+ audit_pack_string(&bufp, f->se_str);
+ break;
default:
data->values[i] = f->val;
}
@@ -314,7 +359,14 @@ static int audit_compare_rule(struct aud
return 1;

switch(a->fields[i].type) {
- /* call type-specific comparison routines here */
+ case AUDIT_SE_USER:
+ case AUDIT_SE_ROLE:
+ case AUDIT_SE_TYPE:
+ case AUDIT_SE_SEN:
+ case AUDIT_SE_CLR:
+ if (strcmp(a->fields[i].se_str, b->fields[i].se_str))
+ return 1;
+ break;
default:
if (a->fields[i].val != b->fields[i].val)
return 1;
@@ -649,3 +701,139 @@ unlock_and_return:
rcu_read_unlock();
return result;
}
+
+/* Check to see if the rule contains any selinux fields. Returns 1 if there
+ are selinux fields specified in the rule, 0 otherwise. */
+static inline int audit_rule_has_selinux(struct audit_krule *rule)
+{
+ int i;
+
+ for (i = 0; i < rule->field_count; i++) {
+ struct audit_field *f = &rule->fields[i];
+ switch (f->type) {
+ case AUDIT_SE_USER:
+ case AUDIT_SE_ROLE:
+ case AUDIT_SE_TYPE:
+ case AUDIT_SE_SEN:
+ case AUDIT_SE_CLR:
+ return 1;
+ }
+ }
+
+ return 0;
+}
+
+/* Make a copy of src in dest. This will be a deep copy with the exception
+ of the watch - that pointer is carried over. The selinux specific fields
+ will be updated in the copy. The point is to be able to replace the src
+ rule with the dest rule in the list, then free the dest rule. */
+static inline int selinux_audit_rule_update_helper(struct audit_krule *dest,
+ struct audit_krule *src)
+{
+ int i, err = 0;
+
+ dest->vers_ops = src->vers_ops;
+ dest->flags = src->flags;
+ dest->listnr = src->listnr;
+ dest->action = src->action;
+ for (i = 0; i < AUDIT_BITMASK_SIZE; i++)
+ dest->mask[i] = src->mask[i];
+ dest->buflen = src->buflen;
+ dest->field_count = src->field_count;
+
+ /* deep copy this information, updating the se_rule fields, because
+ the originals will all be freed when the old rule is freed. */
+ dest->fields = kzalloc(sizeof(struct audit_field) * dest->field_count,
+ GFP_ATOMIC);
+ if (!dest->fields)
+ return -ENOMEM;
+ memcpy(dest->fields, src->fields,
+ sizeof(struct audit_field) * dest->field_count);
+ for (i = 0; i < dest->field_count; i++) {
+ struct audit_field *df = &dest->fields[i];
+ struct audit_field *sf = &src->fields[i];
+ switch (df->type) {
+ case AUDIT_SE_USER:
+ case AUDIT_SE_ROLE:
+ case AUDIT_SE_TYPE:
+ case AUDIT_SE_SEN:
+ case AUDIT_SE_CLR:
+ /* our own copy of se_str */
+ df->se_str = kstrdup(sf->se_str, GFP_ATOMIC);
+ if (!df->se_str)
+ return -ENOMEM;
+ /* our own (refreshed) copy of se_rule */
+ err = selinux_audit_rule_init(df->type, df->op,
+ df->se_str, &df->se_rule);
+ /* Keep currently invalid fields around in case they
+ become valid after a policy reload. */
+ if (err == -EINVAL) {
+ printk(KERN_WARNING "selinux audit rule for item %s is invalid\n", df->se_str);
+ err = 0;
+ }
+ if (err)
+ return err;
+ }
+ }
+
+ return 0;
+}
+
+/* This function will re-initialize the se_rule field of all applicable rules.
+ It will traverse the filter lists serarching for rules that contain selinux
+ specific filter fields. When such a rule is found, it is copied, the
+ selinux field is re-initialized, and the old rule is replaced with the
+ updated rule. */
+static int selinux_audit_rule_update(void)
+{
+ struct audit_entry *entry, *nentry;
+ int i, err = 0, tmperr;
+
+ /* audit_netlink_mutex synchronizes the writers */
+ mutex_lock(&audit_netlink_mutex);
+
+ for (i = 0; i < AUDIT_NR_FILTERS; i++) {
+ list_for_each_entry(entry, &audit_filter_list[i], list) {
+ if (!audit_rule_has_selinux(&entry->rule))
+ continue;
+
+ nentry = kmalloc(sizeof(*entry), GFP_ATOMIC);
+ if (!nentry) {
+ /* save the first error encountered for the
+ return value */
+ if (!err)
+ err = -ENOMEM;
+ audit_panic("error updating selinux filters");
+ continue;
+ }
+
+ tmperr = selinux_audit_rule_update_helper(&nentry->rule,
+ &entry->rule);
+ if (tmperr) {
+ /* save the first error encountered for the
+ return value */
+ if (!err)
+ err = tmperr;
+ audit_free_rule(nentry);
+ audit_panic("error updating selinux filters");
+ continue;
+ }
+ list_replace_rcu(&entry->list, &nentry->list);
+ call_rcu(&entry->rcu, audit_free_rule_rcu);
+ }
+ }
+
+ mutex_unlock(&audit_netlink_mutex);
+
+ return err;
+}
+
+/* Register the callback with selinux. This callback will be invoked when a
+ new policy is loaded. */
+static int __init register_selinux_update_callback(void)
+{
+ selinux_audit_set_callback(&selinux_audit_rule_update);
+ return 0;
+}
+__initcall(register_selinux_update_callback);
+
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 0d1bda1..05a2dc1 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -58,6 +58,7 @@
#include <linux/security.h>
#include <linux/list.h>
#include <linux/tty.h>
+#include <linux/selinux.h>

#include "audit.h"

@@ -174,6 +175,9 @@ static int audit_filter_rules(struct tas
enum audit_state *state)
{
int i, j;
+ u32 sid;
+
+ selinux_task_ctxid(tsk, &sid);

for (i = 0; i < rule->field_count; i++) {
struct audit_field *f = &rule->fields[i];
@@ -263,6 +267,22 @@ static int audit_filter_rules(struct tas
if (ctx)
result = audit_comparator(ctx->loginuid, f->op, f->val);
break;
+ case AUDIT_SE_USER:
+ case AUDIT_SE_ROLE:
+ case AUDIT_SE_TYPE:
+ case AUDIT_SE_SEN:
+ case AUDIT_SE_CLR:
+ /* NOTE: this may return negative values indicating
+ a temporary error. We simply treat this as a
+ match for now to avoid losing information that
+ may be wanted. An error message will also be
+ logged upon error */
+ if (f->se_rule)
+ result = selinux_audit_rule_match(sid, f->type,
+ f->op,
+ f->se_rule,
+ ctx);
+ break;
case AUDIT_ARG0:
case AUDIT_ARG1:
case AUDIT_ARG2:
Amy Griffis
2006-03-09 19:35:40 UTC
Permalink
Hi Darrel,
Post by Darrel Goeddel
Post by Amy Griffis
It seems like it would be cleaner to group the memory allocations for
the rule entry and the fields into a single function, as any audit
rule must always have both. This would allow you to retain the
assumption of the existence of fields in audit_free_rule.
I originally went with that approach, but later decided to break it
out to make a generic copy function (which I scrapped in favor of
the specialized helper because I didn't know anything about the
watches...). I'm assuming that this whole thing will need
modification based on your continued fs auditing work. I'll keep
this in mind for the next version.
Since the audit watches don't exist as of current git, you don't have
to worry too much about them. I'll add the necessary code to handle
them in the policy update path as part of the patch that introduces
them.

My only real question is whether it is appropriate to re-initialize
the opaque selinux fields on events other than policy update, or if
there should be a copy routine as well. You seemed to indicate that
re-initializing was the right thing in all cases. I'm just wanting to
confirm that.

If so, here are a couple of routines that I believe would suffice for
copying audit rules with selinux portions as a result of either policy
reload or filesystem events. What do you think?

static inline struct audit_entry *audit_init_entry(u32 field_count,
gfp_t gfp_mask)
{
struct audit_entry *entry;
struct audit_field *fields;

entry = kzalloc(sizeof(*entry), gfp_mask);
if (unlikely(!entry))
return NULL;

fields = kzalloc(sizeof(*fields) * field_count, gfp_mask);
if (unlikely(!fields)) {
kfree(entry);
return NULL;
}
entry->rule.fields = fields;

return entry;
}

static inline int audit_dupe_selinux_field(struct audit_field *df,
struct audit_field *sf)
{
int ret = 0;
char *se_str;

/* our own copy of se_str */
se_str = kstrdup(sf->se_str, GFP_ATOMIC);
if (unlikely(IS_ERR(se_str)))
return -ENOMEM;
df->se_str = se_str;

/* our own (refreshed) copy of se_rule */
ret = selinux_audit_rule_init(df->type, df->op, df->se_str,
&df->se_rule);
/* Keep currently invalid fields around in case they
* become valid after a policy reload. */
if (ret == -EINVAL) {
printk(KERN_WARNING "audit rule for selinux \'%s\' is invalid\n",
df->se_str);
ret = 0;
}

return ret;
}

static struct audit_entry *audit_dupe_rule(struct audit_krule *old)
{
u32 fcount = old->field_count;
struct audit_entry *entry;
struct audit_krule *new;
int i, err = 0;

entry = audit_init_entry(fcount, GFP_ATOMIC);
if (unlikely(!entry))
return ERR_PTR(-ENOMEM);

new = &entry->rule;
new->vers_ops = old->vers_ops;
new->flags = old->flags;
new->listnr = old->listnr;
new->action = old->action;
for (i = 0; i < AUDIT_BITMASK_SIZE; i++)
new->mask[i] = old->mask[i];
new->buflen = old->buflen;
new->field_count = old->field_count;
memcpy(new->fields, old->fields, sizeof(struct audit_field) * fcount);

for (i = 0; i < fcount; i++) {
switch (new->fields[i].type) {
case AUDIT_SE_USER:
case AUDIT_SE_ROLE:
case AUDIT_SE_TYPE:
case AUDIT_SE_SEN:
case AUDIT_SE_CLR:
err = audit_dupe_selinux_field(&new->fields[i],
&old->fields[i]);
}
if (err) {
audit_free_rule(entry);
return ERR_PTR(err);
}
}

return entry;
}
Darrel Goeddel
2006-03-10 20:52:51 UTC
Permalink
Post by Amy Griffis
Hi Darrel,
Post by Darrel Goeddel
Post by Amy Griffis
It seems like it would be cleaner to group the memory allocations for
the rule entry and the fields into a single function, as any audit
rule must always have both. This would allow you to retain the
assumption of the existence of fields in audit_free_rule.
I originally went with that approach, but later decided to break it
out to make a generic copy function (which I scrapped in favor of
the specialized helper because I didn't know anything about the
watches...). I'm assuming that this whole thing will need
modification based on your continued fs auditing work. I'll keep
this in mind for the next version.
Since the audit watches don't exist as of current git, you don't have
to worry too much about them. I'll add the necessary code to handle
them in the policy update path as part of the patch that introduces
them.
My only real question is whether it is appropriate to re-initialize
the opaque selinux fields on events other than policy update, or if
there should be a copy routine as well. You seemed to indicate that
re-initializing was the right thing in all cases. I'm just wanting to
confirm that.
If so, here are a couple of routines that I believe would suffice for
copying audit rules with selinux portions as a result of either policy
reload or filesystem events. What do you think?
static inline struct audit_entry *audit_init_entry(u32 field_count,
gfp_t gfp_mask)
{
struct audit_entry *entry;
struct audit_field *fields;
entry = kzalloc(sizeof(*entry), gfp_mask);
if (unlikely(!entry))
return NULL;
fields = kzalloc(sizeof(*fields) * field_count, gfp_mask);
if (unlikely(!fields)) {
kfree(entry);
return NULL;
}
entry->rule.fields = fields;
return entry;
}
static inline int audit_dupe_selinux_field(struct audit_field *df,
struct audit_field *sf)
{
int ret = 0;
char *se_str;
/* our own copy of se_str */
se_str = kstrdup(sf->se_str, GFP_ATOMIC);
if (unlikely(IS_ERR(se_str)))
return -ENOMEM;
df->se_str = se_str;
/* our own (refreshed) copy of se_rule */
ret = selinux_audit_rule_init(df->type, df->op, df->se_str,
&df->se_rule);
/* Keep currently invalid fields around in case they
* become valid after a policy reload. */
if (ret == -EINVAL) {
printk(KERN_WARNING "audit rule for selinux \'%s\' is invalid\n",
df->se_str);
ret = 0;
}
return ret;
}
static struct audit_entry *audit_dupe_rule(struct audit_krule *old)
{
u32 fcount = old->field_count;
struct audit_entry *entry;
struct audit_krule *new;
int i, err = 0;
entry = audit_init_entry(fcount, GFP_ATOMIC);
if (unlikely(!entry))
return ERR_PTR(-ENOMEM);
new = &entry->rule;
new->vers_ops = old->vers_ops;
new->flags = old->flags;
new->listnr = old->listnr;
new->action = old->action;
for (i = 0; i < AUDIT_BITMASK_SIZE; i++)
new->mask[i] = old->mask[i];
new->buflen = old->buflen;
new->field_count = old->field_count;
memcpy(new->fields, old->fields, sizeof(struct audit_field) * fcount);
for (i = 0; i < fcount; i++) {
switch (new->fields[i].type) {
err = audit_dupe_selinux_field(&new->fields[i],
&old->fields[i]);
}
if (err) {
audit_free_rule(entry);
return ERR_PTR(err);
}
}
return entry;
}
I like 'em. Here is a new patch that incorporates them. It also moves the
initialization call to selinux into the audit_init function as you had
suggested earlier. Look right?
--
Darrel



diff --git a/kernel/audit.c b/kernel/audit.c
index 04fe2e3..65e1d03 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -55,6 +55,9 @@
#include <net/netlink.h>
#include <linux/skbuff.h>
#include <linux/netlink.h>
+#include <linux/selinux.h>
+
+#include "audit.h"

/* No auditing will take place until audit_initialized != 0.
* (Initialization happens after skb_init is called.) */
@@ -564,6 +567,11 @@ static int __init audit_init(void)
skb_queue_head_init(&audit_skb_queue);
audit_initialized = 1;
audit_enabled = audit_default;
+
+ /* Register the callback with selinux. This callback will be invoked
+ * when a new policy is loaded. */
+ selinux_audit_set_callback(&selinux_audit_rule_update);
+
audit_log(NULL, GFP_KERNEL, AUDIT_KERNEL, "initialized");
return 0;
}
diff --git a/kernel/audit.h b/kernel/audit.h
index bc53920..6f73392 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -54,9 +54,11 @@ enum audit_state {

/* Rule lists */
struct audit_field {
- u32 type;
- u32 val;
- u32 op;
+ u32 type;
+ u32 val;
+ u32 op;
+ char *se_str;
+ struct selinux_audit_rule *se_rule;
};

struct audit_krule {
@@ -86,3 +88,5 @@ extern void audit_send_reply(int pi
extern void audit_log_lost(const char *message);
extern void audit_panic(const char *message);
extern struct mutex audit_netlink_mutex;
+
+extern int selinux_audit_rule_update(void);
diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index 45f3001..0d349e0 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -23,6 +23,7 @@
#include <linux/audit.h>
#include <linux/kthread.h>
#include <linux/netlink.h>
+#include <linux/selinux.h>
#include "audit.h"

/* There are three lists of rules -- one to search at task creation
@@ -42,6 +43,13 @@ struct list_head audit_filter_list[AUDIT

static inline void audit_free_rule(struct audit_entry *e)
{
+ int i;
+ if (e->rule.fields)
+ for (i = 0; i < e->rule.field_count; i++) {
+ struct audit_field *f = &e->rule.fields[i];
+ kfree(f->se_str);
+ selinux_audit_rule_free(f->se_rule);
+ }
kfree(e->rule.fields);
kfree(e);
}
@@ -52,9 +60,30 @@ static inline void audit_free_rule_rcu(s
audit_free_rule(e);
}

+/* Initialize an audit filterlist entry. */
+static inline struct audit_entry *audit_init_entry(u32 field_count,
+ gfp_t gfp_mask)
+{
+ struct audit_entry *entry;
+ struct audit_field *fields;
+
+ entry = kzalloc(sizeof(*entry), gfp_mask);
+ if (unlikely(!entry))
+ return NULL;
+
+ fields = kzalloc(sizeof(*fields) * field_count, gfp_mask);
+ if (unlikely(!fields)) {
+ kfree(entry);
+ return NULL;
+ }
+ entry->rule.fields = fields;
+
+ return entry;
+}
+
/* Unpack a filter field's string representation from user-space
* buffer. */
-static __attribute__((unused)) char *audit_unpack_string(void **bufp, size_t *remain, size_t len)
+static char *audit_unpack_string(void **bufp, size_t *remain, size_t len)
{
char *str;

@@ -84,7 +113,6 @@ static inline struct audit_entry *audit_
{
unsigned listnr;
struct audit_entry *entry;
- struct audit_field *fields;
int i, err;

err = -EINVAL;
@@ -108,23 +136,14 @@ static inline struct audit_entry *audit_
goto exit_err;

err = -ENOMEM;
- entry = kmalloc(sizeof(*entry), GFP_KERNEL);
- if (unlikely(!entry))
- goto exit_err;
- fields = kmalloc(sizeof(*fields) * rule->field_count, GFP_KERNEL);
- if (unlikely(!fields)) {
- kfree(entry);
+ entry = audit_init_entry(rule->field_count, GFP_KERNEL);
+ if (!entry)
goto exit_err;
- }
-
- memset(&entry->rule, 0, sizeof(struct audit_krule));
- memset(fields, 0, sizeof(struct audit_field));

entry->rule.flags = rule->flags & AUDIT_FILTER_PREPEND;
entry->rule.listnr = listnr;
entry->rule.action = rule->action;
entry->rule.field_count = rule->field_count;
- entry->rule.fields = fields;

for (i = 0; i < AUDIT_BITMASK_SIZE; i++)
entry->rule.mask[i] = rule->mask[i];
@@ -150,15 +169,20 @@ static struct audit_entry *audit_rule_to
for (i = 0; i < rule->field_count; i++) {
struct audit_field *f = &entry->rule.fields[i];

- if (rule->fields[i] & AUDIT_UNUSED_BITS) {
- err = -EINVAL;
- goto exit_free;
- }
-
f->op = rule->fields[i] & (AUDIT_NEGATE|AUDIT_OPERATORS);
f->type = rule->fields[i] & ~(AUDIT_NEGATE|AUDIT_OPERATORS);
f->val = rule->values[i];

+ if (f->type & AUDIT_UNUSED_BITS ||
+ f->type == AUDIT_SE_USER ||
+ f->type == AUDIT_SE_ROLE ||
+ f->type == AUDIT_SE_TYPE ||
+ f->type == AUDIT_SE_SEN ||
+ f->type == AUDIT_SE_CLR) {
+ err = -EINVAL;
+ goto exit_free;
+ }
+
entry->rule.vers_ops = (f->op & AUDIT_OPERATORS) ? 2 : 1;

/* Support for legacy operators where
@@ -188,8 +212,9 @@ 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); */
+ size_t remain = datasz - sizeof(struct audit_rule_data);
int i;
+ char *str;

entry = audit_to_entry_common((struct audit_rule *)data);
if (IS_ERR(entry))
@@ -207,10 +232,35 @@ static struct audit_entry *audit_data_to

f->op = data->fieldflags[i] & AUDIT_OPERATORS;
f->type = data->fields[i];
+ f->val = data->values[i];
+ f->se_str = NULL;
+ f->se_rule = NULL;
switch(f->type) {
- /* call type-specific conversion routines here */
- default:
- f->val = data->values[i];
+ case AUDIT_SE_USER:
+ case AUDIT_SE_ROLE:
+ case AUDIT_SE_TYPE:
+ case AUDIT_SE_SEN:
+ case AUDIT_SE_CLR:
+ str = audit_unpack_string(&bufp, &remain, f->val);
+ if (IS_ERR(str))
+ goto exit_free;
+ entry->rule.buflen += f->val;
+
+ err = selinux_audit_rule_init(f->type, f->op, str,
+ &f->se_rule);
+ /* Keep currently invalid fields around in case they
+ * become valid after a policy reload. */
+ if (err == -EINVAL) {
+ printk(KERN_WARNING "audit rule for selinux "
+ "\'%s\' is invalid\n", str);
+ err = 0;
+ }
+ if (err) {
+ kfree(str);
+ goto exit_free;
+ } else
+ f->se_str = str;
+ break;
}
}

@@ -286,7 +336,14 @@ static struct audit_rule_data *audit_kru
data->fields[i] = f->type;
data->fieldflags[i] = f->op;
switch(f->type) {
- /* call type-specific conversion routines here */
+ case AUDIT_SE_USER:
+ case AUDIT_SE_ROLE:
+ case AUDIT_SE_TYPE:
+ case AUDIT_SE_SEN:
+ case AUDIT_SE_CLR:
+ data->buflen += data->values[i] =
+ audit_pack_string(&bufp, f->se_str);
+ break;
default:
data->values[i] = f->val;
}
@@ -314,7 +371,14 @@ static int audit_compare_rule(struct aud
return 1;

switch(a->fields[i].type) {
- /* call type-specific comparison routines here */
+ case AUDIT_SE_USER:
+ case AUDIT_SE_ROLE:
+ case AUDIT_SE_TYPE:
+ case AUDIT_SE_SEN:
+ case AUDIT_SE_CLR:
+ if (strcmp(a->fields[i].se_str, b->fields[i].se_str))
+ return 1;
+ break;
default:
if (a->fields[i].val != b->fields[i].val)
return 1;
@@ -328,6 +392,81 @@ static int audit_compare_rule(struct aud
return 0;
}

+/* Duplicate selinux field information. The se_rule is opaque, so must be
+ * re-initialized. */
+static inline int audit_dupe_selinux_field(struct audit_field *df,
+ struct audit_field *sf)
+{
+ int ret = 0;
+ char *se_str;
+
+ /* our own copy of se_str */
+ se_str = kstrdup(sf->se_str, GFP_ATOMIC);
+ if (unlikely(IS_ERR(se_str)))
+ return -ENOMEM;
+ df->se_str = se_str;
+
+ /* our own (refreshed) copy of se_rule */
+ ret = selinux_audit_rule_init(df->type, df->op, df->se_str,
+ &df->se_rule);
+ /* Keep currently invalid fields around in case they
+ * become valid after a policy reload. */
+ if (ret == -EINVAL) {
+ printk(KERN_WARNING "audit rule for selinux \'%s\' is "
+ "invalid\n", df->se_str);
+ ret = 0;
+ }
+
+ return ret;
+}
+
+/* Duplicate an audit rule. This will be a deep copy with the exception
+ * of the watch - that pointer is carried over. The selinux specific fields
+ * will be updated in the copy. The point is to be able to replace the old
+ * rule with the new rule in the filterlist, then free the old rule. */
+static struct audit_entry *audit_dupe_rule(struct audit_krule *old)
+{
+ u32 fcount = old->field_count;
+ struct audit_entry *entry;
+ struct audit_krule *new;
+ int i, err = 0;
+
+ entry = audit_init_entry(fcount, GFP_ATOMIC);
+ if (unlikely(!entry))
+ return ERR_PTR(-ENOMEM);
+
+ new = &entry->rule;
+ new->vers_ops = old->vers_ops;
+ new->flags = old->flags;
+ new->listnr = old->listnr;
+ new->action = old->action;
+ for (i = 0; i < AUDIT_BITMASK_SIZE; i++)
+ new->mask[i] = old->mask[i];
+ new->buflen = old->buflen;
+ new->field_count = old->field_count;
+ memcpy(new->fields, old->fields, sizeof(struct audit_field) * fcount);
+
+ /* deep copy this information, updating the se_rule fields, because
+ * the originals will all be freed when the old rule is freed. */
+ for (i = 0; i < fcount; i++) {
+ switch (new->fields[i].type) {
+ case AUDIT_SE_USER:
+ case AUDIT_SE_ROLE:
+ case AUDIT_SE_TYPE:
+ case AUDIT_SE_SEN:
+ case AUDIT_SE_CLR:
+ err = audit_dupe_selinux_field(&new->fields[i],
+ &old->fields[i]);
+ }
+ if (err) {
+ audit_free_rule(entry);
+ return ERR_PTR(err);
+ }
+ }
+
+ return entry;
+}
+
/* Add rule to given filterlist if not a duplicate. Protected by
* audit_netlink_mutex. */
static inline int audit_add_rule(struct audit_entry *entry,
@@ -649,3 +788,62 @@ unlock_and_return:
rcu_read_unlock();
return result;
}
+
+/* Check to see if the rule contains any selinux fields. Returns 1 if there
+ * are selinux fields specified in the rule, 0 otherwise. */
+static inline int audit_rule_has_selinux(struct audit_krule *rule)
+{
+ int i;
+
+ for (i = 0; i < rule->field_count; i++) {
+ struct audit_field *f = &rule->fields[i];
+ switch (f->type) {
+ case AUDIT_SE_USER:
+ case AUDIT_SE_ROLE:
+ case AUDIT_SE_TYPE:
+ case AUDIT_SE_SEN:
+ case AUDIT_SE_CLR:
+ return 1;
+ }
+ }
+
+ return 0;
+}
+
+/* This function will re-initialize the se_rule field of all applicable rules.
+ * It will traverse the filter lists serarching for rules that contain selinux
+ * specific filter fields. When such a rule is found, it is copied, the
+ * selinux field is re-initialized, and the old rule is replaced with the
+ * updated rule. */
+int selinux_audit_rule_update(void)
+{
+ struct audit_entry *entry, *nentry;
+ int i, err = 0;
+
+ /* audit_netlink_mutex synchronizes the writers */
+ mutex_lock(&audit_netlink_mutex);
+
+ for (i = 0; i < AUDIT_NR_FILTERS; i++) {
+ list_for_each_entry(entry, &audit_filter_list[i], list) {
+ if (!audit_rule_has_selinux(&entry->rule))
+ continue;
+
+ nentry = audit_dupe_rule(&entry->rule);
+ if (unlikely(IS_ERR(nentry))) {
+ /* save the first error encountered for the
+ * return value */
+ if (!err)
+ err = PTR_ERR(nentry);
+ audit_panic("error updating selinux filters");
+ list_del_rcu(&entry->list);
+ } else {
+ list_replace_rcu(&entry->list, &nentry->list);
+ }
+ call_rcu(&entry->rcu, audit_free_rule_rcu);
+ }
+ }
+
+ mutex_unlock(&audit_netlink_mutex);
+
+ return err;
+}
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 6297d96..3aea29b 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -58,6 +58,7 @@
#include <linux/security.h>
#include <linux/list.h>
#include <linux/tty.h>
+#include <linux/selinux.h>

#include "audit.h"

@@ -174,6 +175,9 @@ static int audit_filter_rules(struct tas
enum audit_state *state)
{
int i, j;
+ u32 sid;
+
+ selinux_task_ctxid(tsk, &sid);

for (i = 0; i < rule->field_count; i++) {
struct audit_field *f = &rule->fields[i];
@@ -263,6 +267,22 @@ static int audit_filter_rules(struct tas
if (ctx)
result = audit_comparator(ctx->loginuid, f->op, f->val);
break;
+ case AUDIT_SE_USER:
+ case AUDIT_SE_ROLE:
+ case AUDIT_SE_TYPE:
+ case AUDIT_SE_SEN:
+ case AUDIT_SE_CLR:
+ /* NOTE: this may return negative values indicating
+ * a temporary error. We simply treat this as a
+ * match for now to avoid losing information that
+ * may be wanted. An error message will also be
+ * logged upon error */
+ if (f->se_rule)
+ result = selinux_audit_rule_match(sid, f->type,
+ f->op,
+ f->se_rule,
+ ctx);
+ break;
case AUDIT_ARG0:
case AUDIT_ARG1:
case AUDIT_ARG2:
Amy Griffis
2006-03-10 21:57:17 UTC
Permalink
Post by Darrel Goeddel
I like 'em. Here is a new patch that incorporates them. It also
moves the initialization call to selinux into the audit_init
function as you had suggested earlier. Look right?
You may want to audit_log a message indicating that the audit rules
were updated due to policy reload. And in the case when you remove a
rule because you couldn't update it, you might want to log that too.

Otherwise, looks good to me.
Steve Grubb
2006-03-10 22:03:36 UTC
Permalink
Post by Amy Griffis
You may want to audit_log a message indicating that the audit rules
were updated due to policy reload.  And in the case when you remove a
rule because you couldn't update it, you might want to log that too.
Do we really need to audit_log that? I would think that syslog is enough. We
already have an event that a policy load occurred, can it be assumed that all
these were updated? We do not do audit_log for other things that may or may
not exist. For example, what if you put a rule in for uid=5000 when you meant
500. The kernel does not say anything.

-Steve
Amy Griffis
2006-03-10 22:48:28 UTC
Permalink
Post by Steve Grubb
Post by Amy Griffis
You may want to audit_log a message indicating that the audit rules
were updated due to policy reload. ?And in the case when you remove a
rule because you couldn't update it, you might want to log that too.
Do we really need to audit_log that? I would think that syslog is
enough.
Syslog is not a good solution when the audit log is available. The
reason to audit_log is to maintain the sequence of events, otherwise
it's necessary (and perhaps impossible) to piece together the audit
log and syslog to figure out in what order things happened.
Post by Steve Grubb
We already have an event that a policy load occurred, can it
be assumed that all these were updated?
Yes, that may be fine since the part of the rule that's being changed
is never indicated as part of other records (the string representation
is all you see).

However, in the case that a rule couldn't be properly updated and we
did not have an audit failure panic, I think the audit log should show
that a rule was removed.
Post by Steve Grubb
We do not do audit_log for other things that may or may not exist.
For example, what if you put a rule in for uid=5000 when you meant
500. The kernel does not say anything.
This is a little different from your example in that the kernel is
actually modifying what the user specified. But as I said above, the
extra verification in the successful case is probably meaningless.

Amy
Darrel Goeddel
2006-03-10 23:22:01 UTC
Permalink
Post by Amy Griffis
Post by Darrel Goeddel
I like 'em. Here is a new patch that incorporates them. It also
moves the initialization call to selinux into the audit_init
function as you had suggested earlier. Look right?
You may want to audit_log a message indicating that the audit rules
were updated due to policy reload. And in the case when you remove a
rule because you couldn't update it, you might want to log that too.
We really aren't updating (or removing) the rule. We are only updating
an implementation specific piece of information relating to the rule.
If a rule references the type badapp_t, it will always reference that
type. The hidden selinux cache of information may change, but the
"spirit of the rule" is always the same. So my opinion is that noting
the update is unnecessary (syslog was a compromise from earlier...).

The removal case is handled by audit_panic because it indicates an real
failure in the audit internals somewhere.
--
Darrel
Darrel Goeddel
2006-03-11 00:14:06 UTC
Permalink
Darrel Goeddel wrote:
<snip>
Post by Darrel Goeddel
I like 'em. Here is a new patch that incorporates them. It also moves the
initialization call to selinux into the audit_init function as you had
suggested earlier. Look right?
The GFP_ATOMIC allocations are not necessary (noted on IRC). This version
switches to GFP_KERNEL and gets rid of the gfp_mask argument to
audit_init_entry().
--
Darrel



diff --git a/kernel/audit.c b/kernel/audit.c
index 04fe2e3..65e1d03 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -55,6 +55,9 @@
#include <net/netlink.h>
#include <linux/skbuff.h>
#include <linux/netlink.h>
+#include <linux/selinux.h>
+
+#include "audit.h"

/* No auditing will take place until audit_initialized != 0.
* (Initialization happens after skb_init is called.) */
@@ -564,6 +567,11 @@ static int __init audit_init(void)
skb_queue_head_init(&audit_skb_queue);
audit_initialized = 1;
audit_enabled = audit_default;
+
+ /* Register the callback with selinux. This callback will be invoked
+ * when a new policy is loaded. */
+ selinux_audit_set_callback(&selinux_audit_rule_update);
+
audit_log(NULL, GFP_KERNEL, AUDIT_KERNEL, "initialized");
return 0;
}
diff --git a/kernel/audit.h b/kernel/audit.h
index bc53920..6f73392 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -54,9 +54,11 @@ enum audit_state {

/* Rule lists */
struct audit_field {
- u32 type;
- u32 val;
- u32 op;
+ u32 type;
+ u32 val;
+ u32 op;
+ char *se_str;
+ struct selinux_audit_rule *se_rule;
};

struct audit_krule {
@@ -86,3 +88,5 @@ extern void audit_send_reply(int pi
extern void audit_log_lost(const char *message);
extern void audit_panic(const char *message);
extern struct mutex audit_netlink_mutex;
+
+extern int selinux_audit_rule_update(void);
diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index 45f3001..c895de7 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -23,6 +23,7 @@
#include <linux/audit.h>
#include <linux/kthread.h>
#include <linux/netlink.h>
+#include <linux/selinux.h>
#include "audit.h"

/* There are three lists of rules -- one to search at task creation
@@ -42,6 +43,13 @@ struct list_head audit_filter_list[AUDIT

static inline void audit_free_rule(struct audit_entry *e)
{
+ int i;
+ if (e->rule.fields)
+ for (i = 0; i < e->rule.field_count; i++) {
+ struct audit_field *f = &e->rule.fields[i];
+ kfree(f->se_str);
+ selinux_audit_rule_free(f->se_rule);
+ }
kfree(e->rule.fields);
kfree(e);
}
@@ -52,9 +60,29 @@ static inline void audit_free_rule_rcu(s
audit_free_rule(e);
}

+/* Initialize an audit filterlist entry. */
+static inline struct audit_entry *audit_init_entry(u32 field_count)
+{
+ struct audit_entry *entry;
+ struct audit_field *fields;
+
+ entry = kzalloc(sizeof(*entry), GFP_KERNEL);
+ if (unlikely(!entry))
+ return NULL;
+
+ fields = kzalloc(sizeof(*fields) * field_count, GFP_KERNEL);
+ if (unlikely(!fields)) {
+ kfree(entry);
+ return NULL;
+ }
+ entry->rule.fields = fields;
+
+ return entry;
+}
+
/* Unpack a filter field's string representation from user-space
* buffer. */
-static __attribute__((unused)) char *audit_unpack_string(void **bufp, size_t *remain, size_t len)
+static char *audit_unpack_string(void **bufp, size_t *remain, size_t len)
{
char *str;

@@ -84,7 +112,6 @@ static inline struct audit_entry *audit_
{
unsigned listnr;
struct audit_entry *entry;
- struct audit_field *fields;
int i, err;

err = -EINVAL;
@@ -108,23 +135,14 @@ static inline struct audit_entry *audit_
goto exit_err;

err = -ENOMEM;
- entry = kmalloc(sizeof(*entry), GFP_KERNEL);
- if (unlikely(!entry))
- goto exit_err;
- fields = kmalloc(sizeof(*fields) * rule->field_count, GFP_KERNEL);
- if (unlikely(!fields)) {
- kfree(entry);
+ entry = audit_init_entry(rule->field_count);
+ if (!entry)
goto exit_err;
- }
-
- memset(&entry->rule, 0, sizeof(struct audit_krule));
- memset(fields, 0, sizeof(struct audit_field));

entry->rule.flags = rule->flags & AUDIT_FILTER_PREPEND;
entry->rule.listnr = listnr;
entry->rule.action = rule->action;
entry->rule.field_count = rule->field_count;
- entry->rule.fields = fields;

for (i = 0; i < AUDIT_BITMASK_SIZE; i++)
entry->rule.mask[i] = rule->mask[i];
@@ -150,15 +168,20 @@ static struct audit_entry *audit_rule_to
for (i = 0; i < rule->field_count; i++) {
struct audit_field *f = &entry->rule.fields[i];

- if (rule->fields[i] & AUDIT_UNUSED_BITS) {
- err = -EINVAL;
- goto exit_free;
- }
-
f->op = rule->fields[i] & (AUDIT_NEGATE|AUDIT_OPERATORS);
f->type = rule->fields[i] & ~(AUDIT_NEGATE|AUDIT_OPERATORS);
f->val = rule->values[i];

+ if (f->type & AUDIT_UNUSED_BITS ||
+ f->type == AUDIT_SE_USER ||
+ f->type == AUDIT_SE_ROLE ||
+ f->type == AUDIT_SE_TYPE ||
+ f->type == AUDIT_SE_SEN ||
+ f->type == AUDIT_SE_CLR) {
+ err = -EINVAL;
+ goto exit_free;
+ }
+
entry->rule.vers_ops = (f->op & AUDIT_OPERATORS) ? 2 : 1;

/* Support for legacy operators where
@@ -188,8 +211,9 @@ 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); */
+ size_t remain = datasz - sizeof(struct audit_rule_data);
int i;
+ char *str;

entry = audit_to_entry_common((struct audit_rule *)data);
if (IS_ERR(entry))
@@ -207,10 +231,35 @@ static struct audit_entry *audit_data_to

f->op = data->fieldflags[i] & AUDIT_OPERATORS;
f->type = data->fields[i];
+ f->val = data->values[i];
+ f->se_str = NULL;
+ f->se_rule = NULL;
switch(f->type) {
- /* call type-specific conversion routines here */
- default:
- f->val = data->values[i];
+ case AUDIT_SE_USER:
+ case AUDIT_SE_ROLE:
+ case AUDIT_SE_TYPE:
+ case AUDIT_SE_SEN:
+ case AUDIT_SE_CLR:
+ str = audit_unpack_string(&bufp, &remain, f->val);
+ if (IS_ERR(str))
+ goto exit_free;
+ entry->rule.buflen += f->val;
+
+ err = selinux_audit_rule_init(f->type, f->op, str,
+ &f->se_rule);
+ /* Keep currently invalid fields around in case they
+ * become valid after a policy reload. */
+ if (err == -EINVAL) {
+ printk(KERN_WARNING "audit rule for selinux "
+ "\'%s\' is invalid\n", str);
+ err = 0;
+ }
+ if (err) {
+ kfree(str);
+ goto exit_free;
+ } else
+ f->se_str = str;
+ break;
}
}

@@ -286,7 +335,14 @@ static struct audit_rule_data *audit_kru
data->fields[i] = f->type;
data->fieldflags[i] = f->op;
switch(f->type) {
- /* call type-specific conversion routines here */
+ case AUDIT_SE_USER:
+ case AUDIT_SE_ROLE:
+ case AUDIT_SE_TYPE:
+ case AUDIT_SE_SEN:
+ case AUDIT_SE_CLR:
+ data->buflen += data->values[i] =
+ audit_pack_string(&bufp, f->se_str);
+ break;
default:
data->values[i] = f->val;
}
@@ -314,7 +370,14 @@ static int audit_compare_rule(struct aud
return 1;

switch(a->fields[i].type) {
- /* call type-specific comparison routines here */
+ case AUDIT_SE_USER:
+ case AUDIT_SE_ROLE:
+ case AUDIT_SE_TYPE:
+ case AUDIT_SE_SEN:
+ case AUDIT_SE_CLR:
+ if (strcmp(a->fields[i].se_str, b->fields[i].se_str))
+ return 1;
+ break;
default:
if (a->fields[i].val != b->fields[i].val)
return 1;
@@ -328,6 +391,81 @@ static int audit_compare_rule(struct aud
return 0;
}

+/* Duplicate selinux field information. The se_rule is opaque, so must be
+ * re-initialized. */
+static inline int audit_dupe_selinux_field(struct audit_field *df,
+ struct audit_field *sf)
+{
+ int ret = 0;
+ char *se_str;
+
+ /* our own copy of se_str */
+ se_str = kstrdup(sf->se_str, GFP_KERNEL);
+ if (unlikely(IS_ERR(se_str)))
+ return -ENOMEM;
+ df->se_str = se_str;
+
+ /* our own (refreshed) copy of se_rule */
+ ret = selinux_audit_rule_init(df->type, df->op, df->se_str,
+ &df->se_rule);
+ /* Keep currently invalid fields around in case they
+ * become valid after a policy reload. */
+ if (ret == -EINVAL) {
+ printk(KERN_WARNING "audit rule for selinux \'%s\' is "
+ "invalid\n", df->se_str);
+ ret = 0;
+ }
+
+ return ret;
+}
+
+/* Duplicate an audit rule. This will be a deep copy with the exception
+ * of the watch - that pointer is carried over. The selinux specific fields
+ * will be updated in the copy. The point is to be able to replace the old
+ * rule with the new rule in the filterlist, then free the old rule. */
+static struct audit_entry *audit_dupe_rule(struct audit_krule *old)
+{
+ u32 fcount = old->field_count;
+ struct audit_entry *entry;
+ struct audit_krule *new;
+ int i, err = 0;
+
+ entry = audit_init_entry(fcount);
+ if (unlikely(!entry))
+ return ERR_PTR(-ENOMEM);
+
+ new = &entry->rule;
+ new->vers_ops = old->vers_ops;
+ new->flags = old->flags;
+ new->listnr = old->listnr;
+ new->action = old->action;
+ for (i = 0; i < AUDIT_BITMASK_SIZE; i++)
+ new->mask[i] = old->mask[i];
+ new->buflen = old->buflen;
+ new->field_count = old->field_count;
+ memcpy(new->fields, old->fields, sizeof(struct audit_field) * fcount);
+
+ /* deep copy this information, updating the se_rule fields, because
+ * the originals will all be freed when the old rule is freed. */
+ for (i = 0; i < fcount; i++) {
+ switch (new->fields[i].type) {
+ case AUDIT_SE_USER:
+ case AUDIT_SE_ROLE:
+ case AUDIT_SE_TYPE:
+ case AUDIT_SE_SEN:
+ case AUDIT_SE_CLR:
+ err = audit_dupe_selinux_field(&new->fields[i],
+ &old->fields[i]);
+ }
+ if (err) {
+ audit_free_rule(entry);
+ return ERR_PTR(err);
+ }
+ }
+
+ return entry;
+}
+
/* Add rule to given filterlist if not a duplicate. Protected by
* audit_netlink_mutex. */
static inline int audit_add_rule(struct audit_entry *entry,
@@ -649,3 +787,62 @@ unlock_and_return:
rcu_read_unlock();
return result;
}
+
+/* Check to see if the rule contains any selinux fields. Returns 1 if there
+ are selinux fields specified in the rule, 0 otherwise. */
+static inline int audit_rule_has_selinux(struct audit_krule *rule)
+{
+ int i;
+
+ for (i = 0; i < rule->field_count; i++) {
+ struct audit_field *f = &rule->fields[i];
+ switch (f->type) {
+ case AUDIT_SE_USER:
+ case AUDIT_SE_ROLE:
+ case AUDIT_SE_TYPE:
+ case AUDIT_SE_SEN:
+ case AUDIT_SE_CLR:
+ return 1;
+ }
+ }
+
+ return 0;
+}
+
+/* This function will re-initialize the se_rule field of all applicable rules.
+ * It will traverse the filter lists serarching for rules that contain selinux
+ * specific filter fields. When such a rule is found, it is copied, the
+ * selinux field is re-initialized, and the old rule is replaced with the
+ * updated rule. */
+int selinux_audit_rule_update(void)
+{
+ struct audit_entry *entry, *nentry;
+ int i, err = 0;
+
+ /* audit_netlink_mutex synchronizes the writers */
+ mutex_lock(&audit_netlink_mutex);
+
+ for (i = 0; i < AUDIT_NR_FILTERS; i++) {
+ list_for_each_entry(entry, &audit_filter_list[i], list) {
+ if (!audit_rule_has_selinux(&entry->rule))
+ continue;
+
+ nentry = audit_dupe_rule(&entry->rule);
+ if (unlikely(IS_ERR(nentry))) {
+ /* save the first error encountered for the
+ * return value */
+ if (!err)
+ err = PTR_ERR(nentry);
+ audit_panic("error updating selinux filters");
+ list_del_rcu(&entry->list);
+ } else {
+ list_replace_rcu(&entry->list, &nentry->list);
+ }
+ call_rcu(&entry->rcu, audit_free_rule_rcu);
+ }
+ }
+
+ mutex_unlock(&audit_netlink_mutex);
+
+ return err;
+}
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 6297d96..3aea29b 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -58,6 +58,7 @@
#include <linux/security.h>
#include <linux/list.h>
#include <linux/tty.h>
+#include <linux/selinux.h>

#include "audit.h"

@@ -174,6 +175,9 @@ static int audit_filter_rules(struct tas
enum audit_state *state)
{
int i, j;
+ u32 sid;
+
+ selinux_task_ctxid(tsk, &sid);

for (i = 0; i < rule->field_count; i++) {
struct audit_field *f = &rule->fields[i];
@@ -263,6 +267,22 @@ static int audit_filter_rules(struct tas
if (ctx)
result = audit_comparator(ctx->loginuid, f->op, f->val);
break;
+ case AUDIT_SE_USER:
+ case AUDIT_SE_ROLE:
+ case AUDIT_SE_TYPE:
+ case AUDIT_SE_SEN:
+ case AUDIT_SE_CLR:
+ /* NOTE: this may return negative values indicating
+ a temporary error. We simply treat this as a
+ match for now to avoid losing information that
+ may be wanted. An error message will also be
+ logged upon error */
+ if (f->se_rule)
+ result = selinux_audit_rule_match(sid, f->type,
+ f->op,
+ f->se_rule,
+ ctx);
+ break;
case AUDIT_ARG0:
case AUDIT_ARG1:
case AUDIT_ARG2:
Continue reading on narkive:
Loading...