12:28 p.m.
Hi,
This is a first draft patch to change the auditing of inodes for lspp.
Previously, we were gathering the context instead of the sid. Now in this patch,
we gather just the sid and convert to context only if an audit event is being
output. This patch makes no effort to account for policy_load. It also inserts
some functions that are likely going upstream via Se Linux kernel people. So,
that will need to be resolved before this patch is final. In any event its
good enough to test with. This patch brings the performance hit from
146% down to 11%. We need a similar patch for IPC syscall auditing.
-Steve
diff -urp linux-2.6.16.x86_64.orig/include/linux/selinux.h
linux-2.6.16.x86_64/include/linux/selinux.h
--- linux-2.6.16.x86_64.orig/include/linux/selinux.h 2006-03-29 10:40:42.000000000 -0500
+++ linux-2.6.16.x86_64/include/linux/selinux.h 2006-03-29 10:27:06.000000000 -0500
@@ -13,6 +13,8 @@
#ifndef _LINUX_SELINUX_H
#define _LINUX_SELINUX_H
+#include <linux/fs.h>
+
struct selinux_audit_rule;
struct audit_context;
@@ -76,6 +78,26 @@ void selinux_audit_set_callback(int (*ca
*/
void selinux_task_ctxid(struct task_struct *tsk, u32 *ctxid);
+/**
+ * selinux_ctxid_to_string - map a security context ID to a string
+ * @ctxid: security context ID to be converted.
+ * @ctx: address of context string to be returned
+ * @ctxlen: length of returned context string.
+ *
+ * Returns 0 if successful, -errno if not. On success, the context
+ * string will be allocated internally, and the caller must call
+ * kfree() on it after use.
+ */
+int selinux_ctxid_to_string(u32 ctxid, char **ctx, u32 *ctxlen);
+
+/**
+ * selinux_get_inode_sid - get the inode's security context ID
+ * @inode: inode structure to get the sid from.
+ *
+ * Returns the sid if successful and 0 if unset
+ */
+u32 selinux_get_inode_sid(const struct inode *inode);
+
#else
static inline int selinux_audit_rule_init(u32 field, u32 op,
@@ -107,6 +129,18 @@ static inline void selinux_task_ctxid(st
*ctxid = 0;
}
+static inline int selinux_ctxid_to_string(u32 ctxid, char **ctx, u32 *ctxlen)
+{
+ *ctx = NULL;
+ *ctxlen = 0;
+ return 0;
+}
+
+static inline u32 selinux_get_inode_sid(const struct inode *inode)
+{
+ return 0;
+}
+
#endif /* CONFIG_SECURITY_SELINUX */
#endif /* _LINUX_SELINUX_H */
diff -urp linux-2.6.16.x86_64.orig/kernel/auditsc.c linux-2.6.16.x86_64/kernel/auditsc.c
--- linux-2.6.16.x86_64.orig/kernel/auditsc.c 2006-03-29 10:40:48.000000000 -0500
+++ linux-2.6.16.x86_64/kernel/auditsc.c 2006-03-29 10:26:45.000000000 -0500
@@ -90,7 +90,7 @@ struct audit_names {
uid_t uid;
gid_t gid;
dev_t rdev;
- char *ctx;
+ u32 osid;
};
struct audit_aux_data {
@@ -435,9 +435,6 @@ static inline void audit_free_names(stru
#endif
for (i = 0; i < context->name_count; i++) {
- char *p = context->names[i].ctx;
- context->names[i].ctx = NULL;
- kfree(p);
if (context->names[i].name)
__putname(context->names[i].name);
}
@@ -729,9 +726,24 @@ static void audit_log_exit(struct audit_
context->names[i].gid,
MAJOR(context->names[i].rdev),
MINOR(context->names[i].rdev));
- if (context->names[i].ctx) {
- audit_log_format(ab, " obj=%s",
- context->names[i].ctx);
+ if (context->names[i].osid != 0) {
+ char *ctx = NULL;
+ int len = 0;
+ if (selinux_ctxid_to_string(
+ context->names[i].osid, &ctx, &len) == 0) {
+ ctx = kmalloc(len, gfp_mask);
+ if (ctx) {
+ selinux_ctxid_to_string(
+ context->names[i].osid,
+ &ctx, &len);
+ }
+ }
+ if (ctx)
+ audit_log_format(ab, " obj=%s", ctx);
+ else
+ audit_log_format(ab, " obj=%u",
+ context->names[i].osid);
+ kfree(ctx);
}
audit_log_end(ab);
@@ -983,37 +995,10 @@ void audit_putname(const char *name)
void audit_inode_context(int idx, const struct inode *inode)
{
struct audit_context *context = current->audit_context;
- const char *suffix = security_inode_xattr_getsuffix();
- char *ctx = NULL;
- int len = 0;
-
- if (!suffix)
- goto ret;
-
- len = security_inode_getsecurity(inode, suffix, NULL, 0, 0);
- if (len == -EOPNOTSUPP)
- goto ret;
- if (len < 0)
- goto error_path;
-
- ctx = kmalloc(len, GFP_KERNEL);
- if (!ctx)
- goto error_path;
-
- len = security_inode_getsecurity(inode, suffix, ctx, len, 0);
- if (len < 0)
- goto error_path;
-
- kfree(context->names[idx].ctx);
- context->names[idx].ctx = ctx;
- goto ret;
-
-error_path:
- if (ctx)
- kfree(ctx);
- audit_panic("error in audit_inode_context");
-ret:
- return;
+ if (security_inode_xattr_getsuffix())
+ context->names[idx].osid = selinux_get_inode_sid(inode);
+ else
+ context->names[idx].osid = 0;
}
diff -urp linux-2.6.16.x86_64.orig/security/selinux/exports.c
linux-2.6.16.x86_64/security/selinux/exports.c
--- linux-2.6.16.x86_64.orig/security/selinux/exports.c 2006-03-29 10:40:51.000000000
-0500
+++ linux-2.6.16.x86_64/security/selinux/exports.c 2006-03-29 10:26:45.000000000 -0500
@@ -26,3 +26,24 @@ void selinux_task_ctxid(struct task_stru
else
*ctxid = 0;
}
+
+extern int ss_initialized;
+
+int selinux_ctxid_to_string(u32 ctxid, char **ctx, u32 *ctxlen)
+{
+ if (ss_initialized)
+ return security_sid_to_context(ctxid, ctx, ctxlen);
+ else {
+ *ctx = NULL;
+ *ctxlen = 0;
+ }
+
+ return 0;
+}
+
+u32 selinux_get_inode_sid(const struct inode *inode)
+{
+ struct inode_security_struct *isec = inode->i_security;
+ return isec->sid;
+}
+
1:01 p.m.
On Wed, 2006-03-29 at 13:28 -0500, Steve Grubb wrote:
Hi,
This is a first draft patch to change the auditing of inodes for lspp.
Previously, we were gathering the context instead of the sid. Now in this patch,
we gather just the sid and convert to context only if an audit event is being
output. This patch makes no effort to account for policy_load. It also inserts
some functions that are likely going upstream via Se Linux kernel people. So,
that will need to be resolved before this patch is final. In any event its
good enough to test with. This patch brings the performance hit from
146% down to 11%. We need a similar patch for IPC syscall auditing.
Not that I disagree with this change in approach, but I think that when
it has come up in the past, there has been concern expressed about the
fact that we could end up not being able to generate the context from
the SID when the audit record is being emitted (due to OOM condition),
and the operation has already occurred at that point. Of course, there
are also other potential failure cases at the point, so I'm not sure it
is crucial, as long as audit_panic is called as appropriate. Just
wanted to make sure that this point was understood by everyone. But I
agree that pre-allocating the contexts is insane.
diff -urp linux-2.6.16.x86_64.orig/include/linux/selinux.h
linux-2.6.16.x86_64/include/linux/selinux.h
--- linux-2.6.16.x86_64.orig/include/linux/selinux.h 2006-03-29 10:40:42.000000000 -0500
+++ linux-2.6.16.x86_64/include/linux/selinux.h 2006-03-29 10:27:06.000000000 -0500
@@ -13,6 +13,8 @@
#ifndef _LINUX_SELINUX_H
#define _LINUX_SELINUX_H
+#include <linux/fs.h>
Just put an empty decl for struct inode here, to avoid header
inter-dependencies:
struct inode;
@@ -76,6 +78,26 @@ void selinux_audit_set_callback(int (*ca
*/
void selinux_task_ctxid(struct task_struct *tsk, u32 *ctxid);
+/**
+ * selinux_ctxid_to_string - map a security context ID to a string
+ * @ctxid: security context ID to be converted.
+ * @ctx: address of context string to be returned
+ * @ctxlen: length of returned context string.
+ *
+ * Returns 0 if successful, -errno if not. On success, the context
+ * string will be allocated internally, and the caller must call
+ * kfree() on it after use.
+ */
+int selinux_ctxid_to_string(u32 ctxid, char **ctx, u32 *ctxlen);
Didn't Tim's patch for saving and auditing the netlink sender
SID/context have a similar interface, based on James' proposed API for
iptables? Just need to make sure that we settle on a single interface
used by them all. From later comments below, I think we'll want a
gfp_mask provided and I'm not sure we need the *ctxlen at all, as
SELinux handles the allocation internally.
+/**
+ * selinux_get_inode_sid - get the inode's security context ID
+ * @inode: inode structure to get the sid from.
+ *
+ * Returns the sid if successful and 0 if unset
+ */
+u32 selinux_get_inode_sid(const struct inode *inode);
I'd favor returning an int (0 == success, -errno for failure) and
providing the SID via pointer arg like other interfaces. Or if there
are no possible error cases, make it void, but still supply the SID via
argument.
diff -urp linux-2.6.16.x86_64.orig/kernel/auditsc.c
linux-2.6.16.x86_64/kernel/auditsc.c
--- linux-2.6.16.x86_64.orig/kernel/auditsc.c 2006-03-29 10:40:48.000000000 -0500
+++ linux-2.6.16.x86_64/kernel/auditsc.c 2006-03-29 10:26:45.000000000 -0500
@@ -729,9 +726,24 @@ static void audit_log_exit(struct audit_
context->names[i].gid,
MAJOR(context->names[i].rdev),
MINOR(context->names[i].rdev));
- if (context->names[i].ctx) {
- audit_log_format(ab, " obj=%s",
- context->names[i].ctx);
+ if (context->names[i].osid != 0) {
+ char *ctx = NULL;
+ int len = 0;
+ if (selinux_ctxid_to_string(
+ context->names[i].osid, &ctx, &len) == 0) {
+ ctx = kmalloc(len, gfp_mask);
+ if (ctx) {
+ selinux_ctxid_to_string(
+ context->names[i].osid,
+ &ctx, &len);
+ }
+ }
Unless I'm confused (quite possible ;), the above sequence shouldn't be
necessary and will actually leak the allocated buffer because SELinux
will overwrite the pointer with its own. The SELinux internal functions
(e.g. security_sid_to_context) handle the allocation of a context buffer
to the right size and set *ctx to it, so the caller never needs to play
this game. Some of the hook interfaces unfortunately require the
caller to guess and provide a buffer that they allocate, but I don't
think we want to continue that trend. SELinux should just set *ctx to
the context buffer it allocates and you are done. You should likely
pass the gfp_mask down into the SELinux interface and propagate it down
to the internal code so that we can conform with whatever the caller
needs.
diff -urp linux-2.6.16.x86_64.orig/security/selinux/exports.c
linux-2.6.16.x86_64/security/selinux/exports.c
--- linux-2.6.16.x86_64.orig/security/selinux/exports.c 2006-03-29 10:40:51.000000000
-0500
+++ linux-2.6.16.x86_64/security/selinux/exports.c 2006-03-29 10:26:45.000000000 -0500
+u32 selinux_get_inode_sid(const struct inode *inode)
+{
+ struct inode_security_struct *isec = inode->i_security;
+ return isec->sid;
+}
I think you need to check for selinux_enabled here, c.f. Darrel's
patches for audit-by-context. Keep in mind that SELinux can be runtime
disabled by init (if SELINUX=disabled in /etc/selinux/config).
--
Stephen Smalley
National Security Agency
1:15 p.m.
On Wednesday 29 March 2006 14:01, Stephen Smalley wrote:
> This patch brings the performance hit from 146% down to 11%. We
need a
> similar patch for IPC syscall auditing.
Not that I disagree with this change in approach, but I think that when
it has come up in the past, there has been concern expressed about the
fact that we could end up not being able to generate the context from
the SID when the audit record is being emitted (due to OOM condition),
and the operation has already occurred at that point.
In that case, the patch writes out the sid number. Given a sid, is there a way
to find it in the policy on disk? If not, that might be useful to have.
Of course, there are also other potential failure cases at the point,
so I'm
not sure it is crucial, as long as audit_panic is called as
appropriate.
If we record the sid number, do we really need to call audit_panic?
> @@ -76,6 +78,26 @@ void selinux_audit_set_callback(int (*ca
> */
> void selinux_task_ctxid(struct task_struct *tsk, u32 *ctxid);
>
> +/**
> + * selinux_ctxid_to_string - map a security context ID to a string
> + * @ctxid: security context ID to be converted.
> + * @ctx: address of context string to be returned
> + * @ctxlen: length of returned context string.
> + *
> + * Returns 0 if successful, -errno if not. On success, the context
> + * string will be allocated internally, and the caller must call
> + * kfree() on it after use.
> + */
> +int selinux_ctxid_to_string(u32 ctxid, char **ctx, u32 *ctxlen);
Didn't Tim's patch for saving and auditing the netlink sender
SID/context have a similar interface, based on James' proposed API for
iptables?
Yes, I copy and pasted and changed the name based on a suggestion from Darrel.
What is the status of that API? Did it go into 2.6.17 tree? I'd like to code
to that API if it were available.
> + if (context->names[i].osid != 0) {
> + char *ctx = NULL;
> + int len = 0;
> + if (selinux_ctxid_to_string(
> + context->names[i].osid, &ctx, &len) == 0)
{
> + ctx = kmalloc(len, gfp_mask);
> + if (ctx) {
> + selinux_ctxid_to_string(
> + context->names[i].osid,
> + &ctx, &len);
> + }
> + }
Unless I'm confused (quite possible ;), the above sequence shouldn't be
necessary and will actually leak the allocated buffer because SELinux
will overwrite the pointer with its own.
OK, will look into this.
Some of the hook interfaces unfortunately require the caller to guess
and
provide a buffer that they allocate, but I don't think we want to continue
that trend.
Agreed, that was messy.
I'll make changes as you suggested and we can try this again. Is there a place
I can grab James' iptables SE Linux interface to patch the lspp kernel with?
I'd like to use that if its accepted/done. It'll make merging Tim's patch
easier.
-Steve
1:34 p.m.
On Wed, 29 Mar 2006 14:15:33 EST, Steve Grubb said:
On Wednesday 29 March 2006 14:01, Stephen Smalley wrote:
>> This patch brings the performance hit from 146% down to 11%. We need a
>> similar patch for IPC syscall auditing.
>
> Not that I disagree with this change in approach, but I think that when
> it has come up in the past, there has been concern expressed about the
> fact that we could end up not being able to generate the context from
> the SID when the audit record is being emitted (due to OOM condition),
> and the operation has already occurred at that point.
In that case, the patch writes out the sid number. Given a sid, is there a way
to find it in the policy on disk? If not, that might be useful to have.
The problem is that by the time you go to snarf it out of the policy on disk,
it may no longer match the policy in effect at the time of the record generation.
The hole probably isn't *that* bad if auditd is doing the grovelling. It's
almost
certainly an issue if ausearch is doing the correlation after the fact....
1:44 p.m.
On Wednesday 29 March 2006 14:34, Valdis.Kletnieks(a)vt.edu wrote:
> In that case, the patch writes out the sid number. Given a sid,
is there
> a way to find it in the policy on disk? If not, that might be useful to
> have.
The problem is that by the time you go to snarf it out of the policy on
disk, it may no longer match the policy in effect at the time of the record
generation.
That should be handled by site configuration control. Assuming that they are
careful to keep old policy around...can it be correlated?
The hole probably isn't *that* bad if auditd is doing the
grovelling.
Auditd has no time to do any correlation. This would have to be done
post-mortem just like uid conversion is done. I think this is an exceptional
condition and just want to make sure we can close the loop manually if this
ever happened.
-Steve
7:24 a.m.
New subject: [redhat-lspp] Re: [PATCH] change lspp inode auditing
On Wed, 2006-03-29 at 14:15 -0500, Steve Grubb wrote:
In that case, the patch writes out the sid number. Given a sid, is
there a way
to find it in the policy on disk? If not, that might be useful to have.
SIDs aren't persistent identifiers. Just runtime values generated on
demand. There is no persistent mapping of them to contexts. That is by
design.
If we record the sid number, do we really need to call audit_panic?
See above. The SID is useless for off-line analysis, and you'd have to
inspect kernel memory to even map it to a context - kernel SIDs aren't
exported to userspace. Again, by design.
Yes, I copy and pasted and changed the name based on a suggestion
from Darrel.
What is the status of that API? Did it go into 2.6.17 tree? I'd like to code
to that API if it were available.
I assumed that Darrel's patch was feeding into the audit tree. It
looked sane to me, but I assumed that there would be a final resolution
on integrating it with Tim's patch and with Amy's work, and a final
version of the patch would then be posted?
Agreed, that was messy.
I think that the existing hook interfaces were based on the existing
pattern for proc and xattrs, where the caller always supplied a buffer,
as those were the initial users of those hooks. But I would prefer the
normal SELinux model of allocating the buffer internally and returning
it to the caller.
I'll make changes as you suggested and we can try this again. Is
there a place
I can grab James' iptables SE Linux interface to patch the lspp kernel with?
I'd like to use that if its accepted/done. It'll make merging Tim's patch
easier.
James' patches are still considered experimental as they rely on
infrastructure changes to iptables that aren't likely to get in very
soon. So I'd just go with Darrel's patch, try to merge with Tim's work,
and use that.
--
Stephen Smalley
National Security Agency
8:22 a.m.
New subject: [redhat-lspp] Re: [PATCH] change lspp inode auditing
On Thursday 30 March 2006 08:24, Stephen Smalley wrote:
> In that case, the patch writes out the sid number. Given a sid,
is there
> a way to find it in the policy on disk? If not, that might be useful to
> have.
SIDs aren't persistent identifiers.
Do 2 back to back loads of the same policy produce the same sids?
> If we record the sid number, do we really need to call
audit_panic?
See above. The SID is useless for off-line analysis, and you'd have to
inspect kernel memory to even map it to a context - kernel SIDs aren't
exported to userspace. Again, by design.
I have a feeling that we may need to close the loop somehow. I really don't
anticipate this being a normal condition at all. But just in case...
-Steve
8:36 a.m.
New subject: [redhat-lspp] Re: [PATCH] change lspp inode auditing
On Thu, 2006-03-30 at 09:22 -0500, Steve Grubb wrote:
On Thursday 30 March 2006 08:24, Stephen Smalley wrote:
> > In that case, the patch writes out the sid number. Given a sid, is there
> > a way to find it in the policy on disk? If not, that might be useful to
> > have.
>
> SIDs aren't persistent identifiers.
Do 2 back to back loads of the same policy produce the same sids?
Policy reload doesn't flush the SID table; it just walks the SID table
and remaps the context if necessary, or marks them invalid if they are
no longer legal in the current policy. So the SIDs stay the same across
the reload. And when it invalidates a SID, it logs a warning from the
tail of convert_context() in services.c. Easy enough to convert that
printk to an audit message and to include the key (sid) value along with
it for later use, I suppose.
I have a feeling that we may need to close the loop somehow. I really
don't
anticipate this being a normal condition at all. But just in case...
--
Stephen Smalley
National Security Agency
10:04 a.m.
New subject: [redhat-lspp] Re: [PATCH] change lspp inode auditing
Stephen Smalley wrote: [Thu Mar 30 2006, 08:24:41AM EST]
On Wed, 2006-03-29 at 14:15 -0500, Steve Grubb wrote:
I assumed that Darrel's patch was feeding into the audit tree. It
looked sane to me, but I assumed that there would be a final resolution
on integrating it with Tim's patch and with Amy's work, and a final
version of the patch would then be posted?
AFAICT the integration of Darrel's patch with my stuff is done.
10:28 a.m.
New subject: [redhat-lspp] Re: [PATCH] change lspp inode auditing
Amy Griffis wrote:
Stephen Smalley wrote: [Thu Mar 30 2006, 08:24:41AM EST]
>On Wed, 2006-03-29 at 14:15 -0500, Steve Grubb wrote:
>I assumed that Darrel's patch was feeding into the audit tree. It
>looked sane to me, but I assumed that there would be a final resolution
>on integrating it with Tim's patch and with Amy's work, and a final
>version of the patch would then be posted?
AFAICT the integration of Darrel's patch with my stuff is done.
I agree. I think my patches have been stable for a while and Amy's
stuff sits nicely on top of it (making a few mods to my code for a new
lock). Tim's patch should fit in nicely right on top when rebased
without needing to change any of my stuff. If I'm wrong, let me know
and I'll make the appropriate adjustments. Looks like my patches will
be going into -mm soon (as seen on IRC).
--
Darrel
1:18 p.m.
Quoting Stephen Smalley (sds(a)tycho.nsa.gov):
On Wed, 2006-03-29 at 13:28 -0500, Steve Grubb wrote:
> Hi,
>
> This is a first draft patch to change the auditing of inodes for lspp.
> Previously, we were gathering the context instead of the sid. Now in this patch,
> we gather just the sid and convert to context only if an audit event is being
> output. This patch makes no effort to account for policy_load. It also inserts
> some functions that are likely going upstream via Se Linux kernel people. So,
> that will need to be resolved before this patch is final. In any event its
> good enough to test with. This patch brings the performance hit from
> 146% down to 11%. We need a similar patch for IPC syscall auditing.
Not that I disagree with this change in approach, but I think that when
it has come up in the past, there has been concern expressed about the
fact that we could end up not being able to generate the context from
the SID when the audit record is being emitted (due to OOM condition),
and the operation has already occurred at that point. Of course, there
are also other potential failure cases at the point, so I'm not sure it
is crucial, as long as audit_panic is called as appropriate. Just
wanted to make sure that this point was understood by everyone. But I
agree that pre-allocating the contexts is insane.
Maybe this is a silly idea... but what about just somehow hashing on
(sid,policy_version), where uint policy_version is incremented on each
selinux policy load?
The audit code would fill in an entry for au_ctx_hash(sid,policy_version),
if it isn't already filled in, when the context would previously have been
preallocated. But it stores (sid, policy_version) in the audit record,
and grabs the value from the table when it's time to actually log the
entry, i.e. where Steve's current patch fills in the string.
I guess whether this is worth it depends on how likely we are to lose
information with this current patch on a live system.
Anyway, just a thought.
-serge
7:28 a.m.
New subject: [redhat-lspp] Re: [PATCH] change lspp inode auditing
On Wed, 2006-03-29 at 13:18 -0600, Serge E. Hallyn wrote:
Maybe this is a silly idea... but what about just somehow hashing
on
(sid,policy_version), where uint policy_version is incremented on each
selinux policy load?
The audit code would fill in an entry for au_ctx_hash(sid,policy_version),
if it isn't already filled in, when the context would previously have been
preallocated. But it stores (sid, policy_version) in the audit record,
and grabs the value from the table when it's time to actually log the
entry, i.e. where Steve's current patch fills in the string.
I guess whether this is worth it depends on how likely we are to lose
information with this current patch on a live system.
People seem to constantly forget that SIDs aren't persistent and that
kernel SIDs aren't exported to userspace. So it doesn't help much to
augment them with a policy version.
However, that does bring up a separate issue beyond the inability to
allocate the context; the SID may be invalidated by a policy load, at
which point you'll get back the unlabeled context upon subsequent
attempts to map it to a context. Hence, if you have a policy reload
between the time you collect the SID and the time you generate the
context, you could "lose" the actual context information. But I think
that is mostly a documentation/procedural issue - don't ever remove
labels from the policy upon policy updates.
--
Stephen Smalley
National Security Agency
9:21 a.m.
New subject: [redhat-lspp] Re: [PATCH] change lspp inode auditing
Quoting Stephen Smalley (sds(a)tycho.nsa.gov):
On Wed, 2006-03-29 at 13:18 -0600, Serge E. Hallyn wrote:
> Maybe this is a silly idea... but what about just somehow hashing on
> (sid,policy_version), where uint policy_version is incremented on each
> selinux policy load?
>
> The audit code would fill in an entry for au_ctx_hash(sid,policy_version),
> if it isn't already filled in, when the context would previously have been
> preallocated. But it stores (sid, policy_version) in the audit record,
> and grabs the value from the table when it's time to actually log the
> entry, i.e. where Steve's current patch fills in the string.
>
> I guess whether this is worth it depends on how likely we are to lose
> information with this current patch on a live system.
People seem to constantly forget that SIDs aren't persistent and that
kernel SIDs aren't exported to userspace. So it doesn't help much to
augment them with a policy version.
I wasn't suggesting exporting the sid to userspace.
I was suggesting a way to help ensure we never do.
However, that does bring up a separate issue beyond the inability to
allocate the context; the SID may be invalidated by a policy load, at
That was what I was addressing.
which point you'll get back the unlabeled context upon
subsequent
attempts to map it to a context. Hence, if you have a policy reload
You couldn't end up with a completely wrong context this way?
between the time you collect the SID and the time you generate the
context, you could "lose" the actual context information. But I think
that is mostly a documentation/procedural issue - don't ever remove
labels from the policy upon policy updates.
-serge
10:15 a.m.
New subject: [redhat-lspp] Re: [PATCH] change lspp inode auditing
On Thu, 2006-03-30 at 09:21 -0600, Serge E. Hallyn wrote:
> However, that does bring up a separate issue beyond the
inability to
> allocate the context; the SID may be invalidated by a policy load, at
That was what I was addressing.
> which point you'll get back the unlabeled context upon subsequent
> attempts to map it to a context. Hence, if you have a policy reload
You couldn't end up with a completely wrong context this way?
No, at policy reload time, the SID table is remapped, with each context
either re-translated to the new representation or dropped entirely if
invalid. In the latter case, later lookups will return the unlabeled
SID's context instead.
--
Stephen Smalley
National Security Agency
7:57 a.m.
On Wednesday 29 March 2006 14:01, Stephen Smalley wrote:
Not that I disagree with this change in approach, but I think that
when
it has come up in the past, there has been concern expressed about the
fact that we could end up not being able to generate the context from
the SID when the audit record is being emitted (due to OOM condition),
OK, attached below is a patch that I think addresses all your comments.
In testing this version, I noticed that I now have a 30% performace hit.
Somewhere between the two patches I lost 19% of the performance.
I'm reviewing the differences to see what caused that.
There is one last loose end to this patch. I am adding a function,
selinux_ctxid_to_string. Is there an official SE Linux kernel API that
provides this? I'll fix Tim's patch to use the same API.
-Steve
diff -urp linux-2.6.16.x86_64.orig/include/linux/selinux.h
linux-2.6.16.x86_64/include/linux/selinux.h
--- linux-2.6.16.x86_64.orig/include/linux/selinux.h 2006-03-29 18:08:04.000000000 -0500
+++ linux-2.6.16.x86_64/include/linux/selinux.h 2006-03-29 20:29:32.000000000 -0500
@@ -15,6 +15,7 @@
struct selinux_audit_rule;
struct audit_context;
+struct inode;
#ifdef CONFIG_SECURITY_SELINUX
@@ -76,6 +77,28 @@ void selinux_audit_set_callback(int (*ca
*/
void selinux_task_ctxid(struct task_struct *tsk, u32 *ctxid);
+/**
+ * selinux_ctxid_to_string - map a security context ID to a string
+ * @ctxid: security context ID to be converted.
+ * @ctx: address of context string to be returned
+ * @ctxlen: length of returned context string.
+ * @gfp_mask: memory type mask
+ *
+ * Returns 0 if successful, -errno if not. On success, the context
+ * string will be allocated internally, and the caller must call
+ * kfree() on it after use.
+ */
+int selinux_ctxid_to_string(u32 ctxid, char **ctx, u32 *ctxlen, gfp_t gfp_mask);
+
+/**
+ * selinux_get_inode_sid - get the inode's security context ID
+ * @inode: inode structure to get the sid from.
+ * @sid: security context ID returned.
+ *
+ * Returns nothing
+ */
+void selinux_get_inode_sid(const struct inode *inode, u32 *sid);
+
#else
static inline int selinux_audit_rule_init(u32 field, u32 op,
@@ -107,6 +130,18 @@ static inline void selinux_task_ctxid(st
*ctxid = 0;
}
+static inline void selinux_ctxid_to_string(u32 ctxid, char **ctx, u32 *ctxlen, gfp_t
gfp_mask)
+{
+ *ctx = NULL;
+ *ctxlen = 0;
+ return 0;
+}
+
+static inline void selinux_get_inode_sid(const struct inode *inode, u32 *sid)
+{
+ *sid = 0;
+}
+
#endif /* CONFIG_SECURITY_SELINUX */
#endif /* _LINUX_SELINUX_H */
diff -urp linux-2.6.16.x86_64.orig/kernel/auditsc.c linux-2.6.16.x86_64/kernel/auditsc.c
--- linux-2.6.16.x86_64.orig/kernel/auditsc.c 2006-03-29 18:08:07.000000000 -0500
+++ linux-2.6.16.x86_64/kernel/auditsc.c 2006-03-29 20:17:41.000000000 -0500
@@ -90,7 +90,7 @@ struct audit_names {
uid_t uid;
gid_t gid;
dev_t rdev;
- char *ctx;
+ u32 osid;
};
struct audit_aux_data {
@@ -435,9 +435,6 @@ static inline void audit_free_names(stru
#endif
for (i = 0; i < context->name_count; i++) {
- char *p = context->names[i].ctx;
- context->names[i].ctx = NULL;
- kfree(p);
if (context->names[i].name)
__putname(context->names[i].name);
}
@@ -729,9 +726,16 @@ static void audit_log_exit(struct audit_
context->names[i].gid,
MAJOR(context->names[i].rdev),
MINOR(context->names[i].rdev));
- if (context->names[i].ctx) {
- audit_log_format(ab, " obj=%s",
- context->names[i].ctx);
+ if (context->names[i].osid != 0) {
+ char *ctx;
+ int len;
+ if (selinux_ctxid_to_string(
+ context->names[i].osid, &ctx, &len, gfp_mask)){
+ audit_log_format(ab, " obj=%u",
+ context->names[i].osid);
+ } else
+ audit_log_format(ab, " obj=%s", ctx);
+ kfree(ctx);
}
audit_log_end(ab);
@@ -983,37 +987,10 @@ void audit_putname(const char *name)
void audit_inode_context(int idx, const struct inode *inode)
{
struct audit_context *context = current->audit_context;
- const char *suffix = security_inode_xattr_getsuffix();
- char *ctx = NULL;
- int len = 0;
-
- if (!suffix)
- goto ret;
-
- len = security_inode_getsecurity(inode, suffix, NULL, 0, 0);
- if (len == -EOPNOTSUPP)
- goto ret;
- if (len < 0)
- goto error_path;
-
- ctx = kmalloc(len, GFP_KERNEL);
- if (!ctx)
- goto error_path;
-
- len = security_inode_getsecurity(inode, suffix, ctx, len, 0);
- if (len < 0)
- goto error_path;
-
- kfree(context->names[idx].ctx);
- context->names[idx].ctx = ctx;
- goto ret;
-
-error_path:
- if (ctx)
- kfree(ctx);
- audit_panic("error in audit_inode_context");
-ret:
- return;
+ if (security_inode_xattr_getsuffix())
+ selinux_get_inode_sid(inode, &context->names[idx].osid);
+ else
+ context->names[idx].osid = 0;
}
diff -urp linux-2.6.16.x86_64.orig/security/selinux/exports.c
linux-2.6.16.x86_64/security/selinux/exports.c
--- linux-2.6.16.x86_64.orig/security/selinux/exports.c 2006-03-29 18:08:08.000000000
-0500
+++ linux-2.6.16.x86_64/security/selinux/exports.c 2006-03-29 20:24:55.000000000 -0500
@@ -14,6 +14,7 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/selinux.h>
+#include <linux/fs.h>
#include "security.h"
#include "objsec.h"
@@ -26,3 +27,27 @@ void selinux_task_ctxid(struct task_stru
else
*ctxid = 0;
}
+
+extern int ss_initialized;
+
+int selinux_ctxid_to_string(u32 ctxid, char **ctx, u32 *ctxlen, gfp_t gfp_mask)
+{
+ if (ss_initialized)
+ return security_sid_to_context(ctxid, ctx, ctxlen);
+ else {
+ *ctx = NULL;
+ *ctxlen = 0;
+ }
+
+ return 0;
+}
+
+void selinux_get_inode_sid(const struct inode *inode, u32 *sid)
+{
+ if (selinux_enabled) {
+ struct inode_security_struct *isec = inode->i_security;
+ *sid = isec->sid;
+ } else
+ *sid = 0;
+}
+
8:30 a.m.
On Thu, 2006-03-30 at 08:57 -0500, Steve Grubb wrote:
There is one last loose end to this patch. I am adding a function,
selinux_ctxid_to_string. Is there an official SE Linux kernel API that
provides this? I'll fix Tim's patch to use the same API.
Nothing official; James' experimental patches for the iptables
integration used:
int selinux_id_to_ctx(u32 ctxid, char **ctx, u32 *ctxlen);
But see comments below on your proposed interface.
diff -urp linux-2.6.16.x86_64.orig/include/linux/selinux.h
linux-2.6.16.x86_64/include/linux/selinux.h
--- linux-2.6.16.x86_64.orig/include/linux/selinux.h 2006-03-29 18:08:04.000000000 -0500
+++ linux-2.6.16.x86_64/include/linux/selinux.h 2006-03-29 20:29:32.000000000 -0500
@@ -76,6 +77,28 @@ void selinux_audit_set_callback(int (*ca
*/
void selinux_task_ctxid(struct task_struct *tsk, u32 *ctxid);
+/**
+ * selinux_ctxid_to_string - map a security context ID to a string
+ * @ctxid: security context ID to be converted.
+ * @ctx: address of context string to be returned
+ * @ctxlen: length of returned context string.
+ * @gfp_mask: memory type mask
+ *
+ * Returns 0 if successful, -errno if not. On success, the context
+ * string will be allocated internally, and the caller must call
+ * kfree() on it after use.
+ */
+int selinux_ctxid_to_string(u32 ctxid, char **ctx, u32 *ctxlen, gfp_t gfp_mask);
On second look, I think you can drop the gfp_mask because we have to use
GFP_ATOMIC in context_struct_to_string regardless of what the caller is
using, due to taking the policy rdlock (and looking later in the patch,
you don't pass it down any further, so it serves no purpose). Also,
since you explicitly identify this as a _to_string interface, I think
you can drop the ctxlen argument; the returned string is NUL-terminated
anyway - unless the caller needs the length for some purpose.
#else
static inline int selinux_audit_rule_init(u32 field, u32 op,
@@ -107,6 +130,18 @@ static inline void selinux_task_ctxid(st
*ctxid = 0;
}
+static inline void selinux_ctxid_to_string(u32 ctxid, char **ctx, u32 *ctxlen, gfp_t
gfp_mask)
Mismatch in the return type with the #if clause, should be int here.
Make sure you compile with and without SELinux enabled, and also test
with SELinux runtime-disabled via SELINUX=disabled
in /etc/selinux/config and by booting with selinux=0 (sorry, too many
options).
+ if (selinux_ctxid_to_string(
+ context->names[i].osid, &ctx, &len, gfp_mask)){
+ audit_log_format(ab, " obj=%u",
+ context->names[i].osid);
+ } else
+ audit_log_format(ab, " obj=%s", ctx);
+ kfree(ctx);
Not much value in displaying the SID, although we do it elsewhere as
well (e.g. in the AVC) as a fallback - mapping it will then require a
dump of kernel memory at that time. Likely have to call audit_panic in
this scenario to meet the criteria, but the admin can always set
audit_panic to not actually panic the machine.
BTW, you kfree(ctx) unconditionally above, so you better initialize it
to NULL prior to calling selinux_ctxid_to_string(). len has the wrong
type too (int vs. u32), but I think you can drop it altogether.
+ if (security_inode_xattr_getsuffix())
+ selinux_get_inode_sid(inode, &context->names[idx].osid);
+ else
+ context->names[idx].osid = 0;
Why not just unconditionally call the interface, which will set the SID
to zero if selinux isn't enabled? Then you can also remove
security_inode_xattr_getsuffix() altogether IIUC.
diff -urp linux-2.6.16.x86_64.orig/security/selinux/exports.c
linux-2.6.16.x86_64/security/selinux/exports.c
--- linux-2.6.16.x86_64.orig/security/selinux/exports.c 2006-03-29 18:08:08.000000000
-0500
+++ linux-2.6.16.x86_64/security/selinux/exports.c 2006-03-29 20:24:55.000000000 -0500
@@ -26,3 +27,27 @@ void selinux_task_ctxid(struct task_stru
else
*ctxid = 0;
}
+
+extern int ss_initialized;
+
+int selinux_ctxid_to_string(u32 ctxid, char **ctx, u32 *ctxlen, gfp_t gfp_mask)
+{
+ if (ss_initialized)
+ return security_sid_to_context(ctxid, ctx, ctxlen);
security_sid_to_context actually has an internal handler for !
ss_initialized that should handle all initial SIDs (and you shouldn't
have any other SIDs until policy is loaded), so I suspect that the test
here should be for selinux_enabled instead.
--
Stephen Smalley
National Security Agency
8:50 a.m.
On Thu, 2006-03-30 at 09:30 -0500, Stephen Smalley wrote:
> +int selinux_ctxid_to_string(u32 ctxid, char **ctx, u32 *ctxlen,
gfp_t gfp_mask);
On second look, I think you can drop the gfp_mask because we have to use
GFP_ATOMIC in context_struct_to_string regardless of what the caller is
using, due to taking the policy rdlock (and looking later in the patch,
you don't pass it down any further, so it serves no purpose). Also,
since you explicitly identify this as a _to_string interface, I think
you can drop the ctxlen argument; the returned string is NUL-terminated
anyway - unless the caller needs the length for some purpose.
Actually, I take the last point back - keep the ctxlen argument please.
Catherine Zhang's patch for Unix datagram SO_PASSSEC support requires
the length information, and she needs this interface too.
--
Stephen Smalley
National Security Agency
9:48 a.m.
On Thursday 30 March 2006 09:30, Stephen Smalley wrote:
Not much value in displaying the SID, although we do it elsewhere as
well (e.g. in the AVC) as a fallback - mapping it will then require a
dump of kernel memory at that time. Likely have to call audit_panic in
this scenario to meet the criteria, but the admin can always set
audit_panic to not actually panic the machine.
I'll patch it to call audit_panic, but that is not the solution. For example,
suppose the syscall was to rename, sendfile, unlink, or link to a file...even
though we panic they were able to do the action. If it was rename a file,
they can now access the file when it comes back up and not be audited. I
think that the correct course of action is to log the number and figure out
how to close the loop on the mapping the sid to context post-mortem.
BTW, you kfree(ctx) unconditionally above, so you better initialize
it
to NULL prior to calling selinux_ctxid_to_string(). len has the wrong
type too (int vs. u32), but I think you can drop it altogether.
The called function initializes it. I'll add another init to NULL just in case
something it calls doesn't on error.
Thanks,
-Steve
9:58 a.m.
New subject: FC5 MLS Policy: auditctl permission denied
Hey Steve,
Under the FC5 MLS policy, what is the magic incantation of SELinux role
and MLS range that will make auditctl go? I've tried staff_r, with staff_t
and SystemLow, which I did not expect to work (and it didn't). I've also
tried sysadm_[rt] and secadm_[rt] with both SystemHigh and SystemLow. So
far, no combination has lead to auditctl being usable. secadm & sysadm
attempts resolve in a direct bash denial message, whereas staff _can_
execute audit, but I get the messages:
"Error sending (rule/watch) list request (Permission denied)"
Anyone know the magic or is this a policy bug?
Thanks,
Mike
10:06 a.m.
New subject: FC5 MLS Policy: auditctl permission denied
Michael C Thompson wrote:
Hey Steve,
Under the FC5 MLS policy, what is the magic incantation of SELinux
role and MLS range that will make auditctl go? I've tried staff_r,
with staff_t and SystemLow, which I did not expect to work (and it
didn't). I've also tried sysadm_[rt] and secadm_[rt] with both
SystemHigh and SystemLow. So far, no combination has lead to auditctl
being usable. secadm & sysadm attempts resolve in a direct bash denial
message, whereas staff _can_ execute audit, but I get the messages:
"Error sending (rule/watch) list request (Permission denied)"
Anyone know the magic or is this a policy bug?
secadm_r
newrole -r secadm_r -l SystemHigh
Thanks,
Mike
------------------------------------------------------------------------
--
Linux-audit mailing list
Linux-audit(a)redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
10:27 a.m.
New subject: FC5 MLS Policy: auditctl permission denied
Daniel J Walsh <dwalsh(a)redhat.com> wrote on 03/30/2006 10:06:30 AM:
Michael C Thompson wrote:
>
> Hey Steve,
>
> Under the FC5 MLS policy, what is the magic incantation of SELinux
> role and MLS range that will make auditctl go? I've tried staff_r,
> with staff_t and SystemLow, which I did not expect to work (and it
> didn't). I've also tried sysadm_[rt] and secadm_[rt] with both
> SystemHigh and SystemLow. So far, no combination has lead to auditctl
> being usable. secadm & sysadm attempts resolve in a direct bash denial
> message, whereas staff _can_ execute audit, but I get the
messages:
> "Error sending (rule/watch) list request (Permission denied)"
>
> Anyone know the magic or is this a policy bug?
>
secadm_r
newrole -r secadm_r -l SystemHigh
Transcript:
-bash-3.1# newrole -r secadm_r -l SystemHigh
Authenticating root.
Password:
[root@dyn94141107 ~]# auditctl -l
bash: /sbin/auditctl: Permission denied
[root@dyn94141107 ~]# ls -alZ /sbin/auditctl
-rwxr-x--- root root system_u:object_r:auditctl_exec_t:SystemLow
/sbin/auditctl
[root@dyn94141107 ~]# id
uid=0(root) gid=0(root)
groups=0(root),1(bin),2(daemon),3(sys),4(adm),6(disk),10(wheel)
context=root:secadm_r:secadm_t:SystemHigh
Its clear from here this is not a DAC issue, but at this point my grasp of
the policy is lacking. My policy packages are:
selinux-policy-2.2.23-15
selinux-policy-targeted-2.2.23-15
selinux-policy-mls-2.2.23-15
Am I out of date with policy?
Thanks,
Mike
12:04 p.m.
New subject: [redhat-lspp] Re: FC5 MLS Policy: auditctl permission denied
On Thu, Mar 30, 2006 at 10:27:04AM -0600, Michael C Thompson wrote:
Daniel J Walsh <dwalsh(a)redhat.com> wrote on 03/30/2006 10:06:30
AM:
> Michael C Thompson wrote:
> >
> > Hey Steve,
> >
> > Under the FC5 MLS policy, what is the magic incantation of SELinux
> > role and MLS range that will make auditctl go? I've tried staff_r,
> > with staff_t and SystemLow, which I did not expect to work (and it
> > didn't). I've also tried sysadm_[rt] and secadm_[rt] with both
> > SystemHigh and SystemLow. So far, no combination has lead to auditctl
> > being usable. secadm & sysadm attempts resolve in a direct bash denial
> > message, whereas staff _can_ execute audit, but I get the messages:
> > "Error sending (rule/watch) list request (Permission denied)"
> >
> > Anyone know the magic or is this a policy bug?
> >
> secadm_r
>
> newrole -r secadm_r -l SystemHigh
Transcript:
-bash-3.1# newrole -r secadm_r -l SystemHigh
Authenticating root.
Password:
[root@dyn94141107 ~]# auditctl -l
bash: /sbin/auditctl: Permission denied
[root@dyn94141107 ~]# ls -alZ /sbin/auditctl
-rwxr-x--- root root system_u:object_r:auditctl_exec_t:SystemLow
/sbin/auditctl
[root@dyn94141107 ~]# id
uid=0(root) gid=0(root)
groups=0(root),1(bin),2(daemon),3(sys),4(adm),6(disk),10(wheel)
context=root:secadm_r:secadm_t:SystemHigh
Its clear from here this is not a DAC issue, but at this point my grasp of
the policy is lacking. My policy packages are:
selinux-policy-2.2.23-15
selinux-policy-targeted-2.2.23-15
selinux-policy-mls-2.2.23-15
Am I out of date with policy?
Thanks,
Mike
I get the same results on pSeries. No, your policy shouldn't be out-of-date.
If I set permissive mode and look at dmesg, I see this:
audit(1143741108.405:2485): security_compute_sid: invalid context
root:secadm_r:auditctl_t:s15:c0.c255 for scontext=root:secadm_r:secadm_t:s15:c0.c255
tcontext=system_u:object_r:auditctl_exec_t:s0 tclass=process
This is FC5 gold with the lspp.13 kernel. I'll doublecheck to make sure
I didn't do anything unusual.
--
George Wilson <ltcgcw(a)us.ibm.com>
IBM Linux Technology Center
4:39 p.m.
New subject: FC5 MLS Policy: auditctl permission denied
New policy on ftp://people.redhat.com/dwalsh/SELinux/Fedora
Made the changes to allow the transition. I don't have access to an MLS
machine til tomorrow but this should fix it.
selinux-policy-targeted-2.2.28-3
Dan
9:36 a.m.
New subject: FC5 MLS Policy: auditctl permission denied
Daniel J Walsh <dwalsh(a)redhat.com> wrote on 03/30/2006 04:39:06 PM:
New policy on ftp://people.redhat.com/dwalsh/SELinux/Fedora
Made the changes to allow the transition. I don't have access to an MLS
machine til tomorrow but this should fix it.
selinux-policy-targeted-2.2.28-3
Dan
Hi Dan,
I just tried the recent policy files available from the above listed
people page, and I'm being DoS'd from my own machine with a stream of
audit avc denied messages. From what I can gleam from the stream of text,
itklogd is being denied {search}.
Mike
9:51 a.m.
New subject: FC5 MLS Policy: auditctl permission denied
linux-audit-bounces(a)redhat.com wrote on 03/31/2006 09:36:38 AM:
Daniel J Walsh <dwalsh(a)redhat.com> wrote on 03/30/2006 04:39:06 PM:
> New policy on ftp://people.redhat.com/dwalsh/SELinux/Fedora
>
> Made the changes to allow the transition. I don't have access to an
MLS
> machine til tomorrow but this should fix it.
> selinux-policy-targeted-2.2.28-3
> Dan
Hi Dan,
I just tried the recent policy files available from the above listed
people page, and I'm being DoS'd from my own machine with a stream
of audit avc denied messages. From what I can gleam from the stream
of text, itklogd is being denied {search}.
Sorry, that should read "its 'klogd' being denied {search}". I never got
that logged so I can't send you the exact message. If I manage to trap it,
I'll send it on. Having the system boot the MLS policy with enforcing mode
on as default will cause this DoS to appear for me. Booting with
permissive mode is fine. On the plus side, auditctl works for secadm_r,
but has lost access to the /root directory, which secadm_r previous had
access to.
Mike
12:47 p.m.
On Thursday 30 March 2006 10:48, Steve Grubb wrote:
> Not much value in displaying the SID, although we do it
elsewhere as
> well (e.g. in the AVC) as a fallback - mapping it will then require a
> dump of kernel memory at that time. Likely have to call audit_panic in
> this scenario to meet the criteria, but the admin can always set
> audit_panic to not actually panic the machine.
I'll patch it to call audit_panic, but that is not the solution. For
example, suppose the syscall was to rename, sendfile, unlink, or link to a
file...even though we panic they were able to do the action. If it was
rename a file, they can now access the file when it comes back up and not
be audited. I think that the correct course of action is to log the number
and figure out how to close the loop on the mapping the sid to context
post-mortem.
OK, below is a patch that I think addresses all issues pointed out. I've
benchmarked it and we are at 23% performance hit. That's much better
than before.
Signed-off-by: Steve Grubb <sgrubb(a)redhat.com>
diff -urp linux-2.6.16.x86_64.orig/include/linux/selinux.h
linux-2.6.16.x86_64/include/linux/selinux.h
--- linux-2.6.16.x86_64.orig/include/linux/selinux.h 2006-03-30 12:01:15.000000000 -0500
+++ linux-2.6.16.x86_64/include/linux/selinux.h 2006-03-30 12:04:42.000000000 -0500
@@ -15,6 +15,7 @@
struct selinux_audit_rule;
struct audit_context;
+struct inode;
#ifdef CONFIG_SECURITY_SELINUX
@@ -76,6 +77,27 @@ void selinux_audit_set_callback(int (*ca
*/
void selinux_task_ctxid(struct task_struct *tsk, u32 *ctxid);
+/**
+ * selinux_ctxid_to_string - map a security context ID to a string
+ * @ctxid: security context ID to be converted.
+ * @ctx: address of context string to be returned
+ * @ctxlen: length of returned context string.
+ *
+ * Returns 0 if successful, -errno if not. On success, the context
+ * string will be allocated internally, and the caller must call
+ * kfree() on it after use.
+ */
+int selinux_ctxid_to_string(u32 ctxid, char **ctx, u32 *ctxlen);
+
+/**
+ * selinux_get_inode_sid - get the inode's security context ID
+ * @inode: inode structure to get the sid from.
+ * @sid: pointer to security context ID to be filled in.
+ *
+ * Returns nothing
+ */
+void selinux_get_inode_sid(const struct inode *inode, u32 *sid);
+
#else
static inline int selinux_audit_rule_init(u32 field, u32 op,
@@ -107,6 +128,18 @@ static inline void selinux_task_ctxid(st
*ctxid = 0;
}
+static inline int selinux_ctxid_to_string(u32 ctxid, char **ctx, u32 *ctxlen)
+{
+ *ctx = NULL;
+ *ctxlen = 0;
+ return 0;
+}
+
+static inline void selinux_get_inode_sid(const struct inode *inode, u32 *sid)
+{
+ *sid = 0;
+}
+
#endif /* CONFIG_SECURITY_SELINUX */
#endif /* _LINUX_SELINUX_H */
diff -urp linux-2.6.16.x86_64.orig/kernel/auditsc.c linux-2.6.16.x86_64/kernel/auditsc.c
--- linux-2.6.16.x86_64.orig/kernel/auditsc.c 2006-03-30 12:01:21.000000000 -0500
+++ linux-2.6.16.x86_64/kernel/auditsc.c 2006-03-30 12:06:01.000000000 -0500
@@ -90,7 +90,7 @@ struct audit_names {
uid_t uid;
gid_t gid;
dev_t rdev;
- char *ctx;
+ u32 osid;
};
struct audit_aux_data {
@@ -435,9 +435,6 @@ static inline void audit_free_names(stru
#endif
for (i = 0; i < context->name_count; i++) {
- char *p = context->names[i].ctx;
- context->names[i].ctx = NULL;
- kfree(p);
if (context->names[i].name)
__putname(context->names[i].name);
}
@@ -700,6 +697,7 @@ static void audit_log_exit(struct audit_
}
}
for (i = 0; i < context->name_count; i++) {
+ int call_panic = 0;
unsigned long ino = context->names[i].ino;
unsigned long pino = context->names[i].pino;
@@ -729,12 +727,22 @@ static void audit_log_exit(struct audit_
context->names[i].gid,
MAJOR(context->names[i].rdev),
MINOR(context->names[i].rdev));
- if (context->names[i].ctx) {
- audit_log_format(ab, " obj=%s",
- context->names[i].ctx);
+ if (context->names[i].osid != 0) {
+ char *ctx = NULL;
+ int len;
+ if (selinux_ctxid_to_string(
+ context->names[i].osid, &ctx, &len)) {
+ audit_log_format(ab, " obj=%u",
+ context->names[i].osid);
+ call_panic = 1;
+ } else
+ audit_log_format(ab, " obj=%s", ctx);
+ kfree(ctx);
}
audit_log_end(ab);
+ if (call_panic)
+ audit_panic("error converting sid to string");
}
}
@@ -983,37 +991,8 @@ void audit_putname(const char *name)
void audit_inode_context(int idx, const struct inode *inode)
{
struct audit_context *context = current->audit_context;
- const char *suffix = security_inode_xattr_getsuffix();
- char *ctx = NULL;
- int len = 0;
-
- if (!suffix)
- goto ret;
-
- len = security_inode_getsecurity(inode, suffix, NULL, 0, 0);
- if (len == -EOPNOTSUPP)
- goto ret;
- if (len < 0)
- goto error_path;
-
- ctx = kmalloc(len, GFP_KERNEL);
- if (!ctx)
- goto error_path;
- len = security_inode_getsecurity(inode, suffix, ctx, len, 0);
- if (len < 0)
- goto error_path;
-
- kfree(context->names[idx].ctx);
- context->names[idx].ctx = ctx;
- goto ret;
-
-error_path:
- if (ctx)
- kfree(ctx);
- audit_panic("error in audit_inode_context");
-ret:
- return;
+ selinux_get_inode_sid(inode, &context->names[idx].osid);
}
diff -urp linux-2.6.16.x86_64.orig/security/selinux/exports.c
linux-2.6.16.x86_64/security/selinux/exports.c
--- linux-2.6.16.x86_64.orig/security/selinux/exports.c 2006-03-30 12:01:23.000000000
-0500
+++ linux-2.6.16.x86_64/security/selinux/exports.c 2006-03-30 12:05:30.000000000 -0500
@@ -14,6 +14,7 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/selinux.h>
+#include <linux/fs.h>
#include "security.h"
#include "objsec.h"
@@ -26,3 +27,26 @@ void selinux_task_ctxid(struct task_stru
else
*ctxid = 0;
}
+
+int selinux_ctxid_to_string(u32 ctxid, char **ctx, u32 *ctxlen)
+{
+ if (selinux_enabled)
+ return security_sid_to_context(ctxid, ctx, ctxlen);
+ else {
+ *ctx = NULL;
+ *ctxlen = 0;
+ }
+
+ return 0;
+}
+
+void selinux_get_inode_sid(const struct inode *inode, u32 *sid)
+{
+ if (selinux_enabled) {
+ struct inode_security_struct *isec = inode->i_security;
+ *sid = isec->sid;
+ return;
+ }
+ *sid = 0;
+}
+
1:02 p.m.
On Thu, 2006-03-30 at 13:47 -0500, Steve Grubb wrote:
OK, below is a patch that I think addresses all issues pointed out.
I've
benchmarked it and we are at 23% performance hit. That's much better
than before.
Signed-off-by: Steve Grubb <sgrubb(a)redhat.com>
+ if (context->names[i].osid != 0) {
+ char *ctx = NULL;
+ int len;
Needs to be u32 to match the interface.
Otherwise, looks sane to me. Make sure you compile without SELinux too,
and also try to boot with selinux disabled to be sure.
--
Stephen Smalley
National Security Agency
11:20 a.m.
Stephen Smalley wrote:
On Thu, 2006-03-30 at 08:57 -0500, Steve Grubb wrote:
>There is one last loose end to this patch. I am adding a function,
>selinux_ctxid_to_string. Is there an official SE Linux kernel API that
>provides this? I'll fix Tim's patch to use the same API.
Nothing official; James' experimental patches for the iptables
integration used:
int selinux_id_to_ctx(u32 ctxid, char **ctx, u32 *ctxlen);
But see comments below on your proposed interface.
Looks like I gave some bad advice here... I had suggested that the
function be renamed to used ctxid from instead of the one name
from Tim's patch because I remembered that I used ctxid because it
was in James' patch. I didn't realize that that func actually
cam from James' patch as well... I do think we should be consistent
with the naming for all of the exported selinux functions like
selinux_task_XXX
selinux_inode_XXX
selinux_ipc_XXX
selinux_XXX_to_string
I used ctxid in my patch, so I vote for that ;) Sorry bout the
misguided advice.
--
Darrel
6840
days inactive
6842
days old
linux-audit@lists.linux-audit.osci.io
27 comments
9 participants
participants (9)
-
Amy Griffis -
Daniel J Walsh -
Darrel Goeddel -
George C. Wilson -
Michael C Thompson -
Serge E. Hallyn -
Stephen Smalley -
Steve Grubb -
Valdis.Kletnieks＠vt.edu