All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC][PATCH] collect security labels on user processes generating audit messages
@ 2006-02-09  1:32 Timothy R. Chavez
  2006-02-09 14:58 ` James Morris
  0 siblings, 1 reply; 40+ messages in thread
From: Timothy R. Chavez @ 2006-02-09  1:32 UTC (permalink / raw)
  To: selinux, Linux Audit Discussion
  Cc: Serge Hallyn, James Morris, Stephen Smalley

Hello,

This patch collects the security label for a user process generating
audit messages.  Traditionally, obtaining the security label of a
process in the kernel could be done using security_getprocattr().
However, due to the asynchronous nature of netlink, by the time the
kernel gets the audit message, the 'pid' packaged with it, may have
already been recycled.  Thus, if a task is found with 'pid', it may not
be the same task that issued the audit message, and the wrong security
label could be collected.  One solution to this problem and the solution
implemented here, is to collect the 'sid' of the process, while we are
running in that process' context.  This can be accomplished by attaching
the 'sid' to the audit message on the client-side of the netlink
interaction much like 'loginuid'.  When the message arrives to the
kernel and is processed by the audit subsystem, the 'sid' can then be
resolved to the correct security label.  One thing to keep in mind, is
that should the policy be changed and reloaded while an audit message is
in transit, the security label the 'sid' associated with that audit
message resolves to, may be invalid or incorrect.  

Notable details about this implementation:

1) A new SELinux interface was introduced to give other parts of the
kernel the ability to resolve 'sids' into security labels.  

2) SELinux and LSM were augmented to obtain the 'sid' of a task in a
similar manner to ipc and inode.

I've posted this message and patch as an RFC to solicit feedback from
the SELinux and audit communities.

I personally think that the Kconfig option could be a bit more
descriptive and selinux_getsecurity() could probably find a new home
outside of selinux/include/objsec.h, though that seemed to make sense
for me, at the time of coding.

Thank you.

-tim

diff --git a/include/linux/netlink.h b/include/linux/netlink.h
index 6a2ccf7..ccd5905 100644
--- a/include/linux/netlink.h
+++ b/include/linux/netlink.h
@@ -143,6 +143,7 @@ struct netlink_skb_parms
 	__u32			dst_group;
 	kernel_cap_t		eff_cap;
 	__u32			loginuid;	/* Login (audit) uid */
+	__u32			secid;		/* SELinux security id */
 };
 
 #define NETLINK_CB(skb)		(*(struct netlink_skb_parms*)&((skb)->cb))
diff --git a/include/linux/security.h b/include/linux/security.h
index b4fe8aa..4178175 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -625,6 +625,11 @@ struct swap_info_struct;
  *	@p contains the task_struct for the task.
  *	@inode contains the inode structure for the inode.
  *
+ * @task_getsecurity:
+ *      Copy the security label associated with the task object into
+ *      @buffer.  @buffer may be NULL to request the size of the buffer
+ *      required.  @size indicates the size of @buffer in bytes. Return
+ *      number of bytes used/required on success.
  * Security hooks for Netlink messaging.
  *
  * @netlink_send:
@@ -1169,6 +1174,7 @@ struct security_operations {
 			   unsigned long arg5);
 	void (*task_reparent_to_init) (struct task_struct * p);
 	void (*task_to_inode)(struct task_struct *p, struct inode *inode);
+	int (*task_getsecurity)(struct task_struct *tsk, void *buffer, size_t size);
 
 	int (*ipc_permission) (struct kern_ipc_perm * ipcp, short flag);
 	int (*ipc_getsecurity)(struct kern_ipc_perm *ipcp, void *buffer, size_t size);
@@ -1817,6 +1823,11 @@ static inline void security_task_to_inod
 	security_ops->task_to_inode(p, inode);
 }
 
+static inline int security_task_getsecurity(struct task_struct *tsk,void *buffer, size_t size)
+{
+	return security_ops->task_getsecurity(tsk, buffer, size);
+}
+
 static inline int security_ipc_permission (struct kern_ipc_perm *ipcp,
 					   short flag)
 {
@@ -2457,6 +2468,11 @@ static inline void security_task_reparen
 static inline void security_task_to_inode(struct task_struct *p, struct inode *inode)
 { }
 
+static inline int security_task_getsecurity(struct task_struct *tsk, void *buffer, size_t size)
+{
+	return -EOPNOTSUPP;
+}
+
 static inline int security_ipc_permission (struct kern_ipc_perm *ipcp,
 					   short flag)
 {
diff --git a/include/linux/selinux_api.h b/include/linux/selinux_api.h
new file mode 100644
index 0000000..54102ab
--- /dev/null
+++ b/include/linux/selinux_api.h
@@ -0,0 +1,38 @@
+/*
+ * External SELinux API
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+ *
+ * Copyright (C) IBM Corporation, 2006
+ *
+ * Author: Timothy R. Chavez <tinytim@us.ibm.com>
+ *
+ */
+
+#ifndef _LINUX_SELINUX_API_H
+#define _LINUX_SELINUX_API_H
+
+#include <linux/kernel.h>
+#include <linux/errno.h>
+
+#ifdef CONFIG_SECURITY_SELINUX_API
+int selinux_sid_to_context(u32 sid, void *ctx, size_t size);
+#else
+static inline int selinux_sid_to_context(u32 sid, void *ctx, size_t size)
+{
+	return -EOPNOTSUPP;
+}
+#endif /* CONFIG_SELINUX_API */
+#endif /* _LINUX_SELINUX_API_H */
diff --git a/kernel/audit.c b/kernel/audit.c
index d95efd6..0f158f0 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -49,6 +49,7 @@
 #include <linux/err.h>
 #include <linux/kthread.h>
 
+#include <linux/selinux_api.h>
 #include <linux/audit.h>
 
 #include <net/sock.h>
@@ -383,7 +384,7 @@ static int audit_netlink_ok(kernel_cap_t
 
 static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 {
-	u32			uid, pid, seq;
+	u32			uid, pid, sid, seq;
 	void			*data;
 	struct audit_status	*status_get, status_set;
 	int			err;
@@ -391,6 +392,8 @@ static int audit_receive_msg(struct sk_b
 	u16			msg_type = nlh->nlmsg_type;
 	uid_t			loginuid; /* loginuid of sender */
 	struct audit_sig_info   sig_data;
+	int			len;
+	char 			*ctx = NULL;
 
 	err = audit_netlink_ok(NETLINK_CB(skb).eff_cap, msg_type);
 	if (err)
@@ -409,6 +412,7 @@ static int audit_receive_msg(struct sk_b
 	pid  = NETLINK_CREDS(skb)->pid;
 	uid  = NETLINK_CREDS(skb)->uid;
 	loginuid = NETLINK_CB(skb).loginuid;
+	sid = NETLINK_CB(skb).secid;
 	seq  = nlh->nlmsg_seq;
 	data = NLMSG_DATA(nlh);
 
@@ -460,11 +464,26 @@ static int audit_receive_msg(struct sk_b
 			err = 0;
 			ab = audit_log_start(NULL, GFP_KERNEL, msg_type);
 			if (ab) {
+				len = selinux_sid_to_context(sid, NULL, 0);
+				if (len < 0 && len != -EOPNOTSUPP)
+					return len;
+				else if (len > 0) {
+					ctx = (char *)kmalloc(len, GFP_KERNEL);
+					if (!ctx)
+						return -ENOMEM;
+					len = selinux_sid_to_context(sid, ctx, len);
+					if (len < 0) {
+						kfree(ctx);
+						return len;
+					}
+				}
 				audit_log_format(ab,
-						 "user pid=%d uid=%u auid=%u msg='%.1024s'",
-						 pid, uid, loginuid, (char *)data);
+						 "user pid=%d uid=%u auid=%u subj=%s msg='%.1024s'",
+						 pid, uid, loginuid, (!ctx ? "null" : ctx),
+						 (char *)data);
 				audit_set_pid(ab, pid);
 				audit_log_end(ab);
+				kfree(ctx);
 			}
 		}
 		break;
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 96020d7..a516453 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -1120,6 +1120,7 @@ static int netlink_sendmsg(struct kiocb 
 	NETLINK_CB(skb).dst_pid = dst_pid;
 	NETLINK_CB(skb).dst_group = dst_group;
 	NETLINK_CB(skb).loginuid = audit_get_loginuid(current->audit_context);
+	NETLINK_CB(skb).secid = security_task_getsid(current);
 	memcpy(NETLINK_CREDS(skb), &siocb->scm->creds, sizeof(struct ucred));
 
 	/* What can I do? Netlink is asynchronous, so that
diff --git a/security/dummy.c b/security/dummy.c
index 75e7c4a..768a39a 100644
--- a/security/dummy.c
+++ b/security/dummy.c
@@ -557,6 +557,11 @@ static void dummy_task_reparent_to_init 
 static void dummy_task_to_inode(struct task_struct *p, struct inode *inode)
 { }
 
+static int dummy_task_getsecurity(struct task_struct *tsk, void *buffer, size_t size)
+{
+	return -EOPNOTSUPP;
+}
+
 static int dummy_ipc_permission (struct kern_ipc_perm *ipcp, short flag)
 {
 	return 0;
@@ -934,6 +939,7 @@ void security_fixup_ops (struct security
 	set_to_dummy_if_null(ops, task_prctl);
 	set_to_dummy_if_null(ops, task_reparent_to_init);
  	set_to_dummy_if_null(ops, task_to_inode);
+	set_to_dummy_if_null(ops, task_getsecurity);
 	set_to_dummy_if_null(ops, ipc_permission);
 	set_to_dummy_if_null(ops, ipc_getsecurity);
 	set_to_dummy_if_null(ops, msg_msg_alloc_security);
diff --git a/security/selinux/Kconfig b/security/selinux/Kconfig
index b59582b..424c24e 100644
--- a/security/selinux/Kconfig
+++ b/security/selinux/Kconfig
@@ -95,3 +95,11 @@ config SECURITY_SELINUX_CHECKREQPROT_VAL
 	  via /selinux/checkreqprot if authorized by policy.
 
 	  If you are unsure how to answer this question, answer 1.
+
+config SECURITY_SELINUX_API
+	bool "NSA SELinux API"
+	depends on SECURITY_SELINUX
+	default n
+	help
+	 This option exposes internal SELinux concepts such as a 'security id'
+	 to other parts of the kernel.
diff --git a/security/selinux/Makefile b/security/selinux/Makefile
index b038cd0..4cf0782 100644
--- a/security/selinux/Makefile
+++ b/security/selinux/Makefile
@@ -8,5 +8,7 @@ selinux-y := avc.o hooks.o selinuxfs.o n
 
 selinux-$(CONFIG_SECURITY_NETWORK) += netif.o
 
+selinux-$(CONFIG_SECURITY_SELINUX_API) += selinux_api.o
+
 EXTRA_CFLAGS += -Isecurity/selinux/include
 
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 21c8aa6..88e9d87 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -118,7 +118,7 @@ static DEFINE_SPINLOCK(sb_security_lock)
 
 /* Return security context for a given sid or just the context 
    length if the buffer is null or length is 0 */
-static int selinux_getsecurity(u32 sid, void *buffer, size_t size)
+int selinux_getsecurity(u32 sid, void *buffer, size_t size)
 {
 	char *context;
 	unsigned len;
@@ -2769,6 +2769,13 @@ static void selinux_task_to_inode(struct
 	return;
 }
 
+static int selinux_task_getsecurity(struct task_struct *tsk, void *buffer, size_t size)
+{
+	struct task_security_struct *tsec = tsk->security;
+	
+	return selinux_getsecurity(tsec->sid, buffer, size);
+}
+
 #ifdef CONFIG_SECURITY_NETWORK
 
 /* Returns error only if unable to parse addresses */
@@ -4319,6 +4326,7 @@ static struct security_operations selinu
 	.task_prctl =			selinux_task_prctl,
 	.task_reparent_to_init =	selinux_task_reparent_to_init,
 	.task_to_inode =                selinux_task_to_inode,
+	.task_getsecurity =		selinux_task_getsecurity,
 
 	.ipc_permission =		selinux_ipc_permission,
 	.ipc_getsecurity =		selinux_ipc_getsecurity,
diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h
index 887937c..9fc4692 100644
--- a/security/selinux/include/objsec.h
+++ b/security/selinux/include/objsec.h
@@ -109,4 +109,6 @@ struct sk_security_struct {
 
 extern unsigned int selinux_checkreqprot;
 
+int selinux_getsecurity(u32 sid, void *buffer, size_t size);
+
 #endif /* _SELINUX_OBJSEC_H_ */
diff --git a/security/selinux/selinux_api.c b/security/selinux/selinux_api.c
new file mode 100644
index 0000000..7604766
--- /dev/null
+++ b/security/selinux/selinux_api.c
@@ -0,0 +1,8 @@
+#include <linux/selinux_api.h>
+
+extern int selinux_getsecurity(u32 sid, void *buffer, size_t size);
+
+int selinux_sid_to_context(u32 sid, void *ctx, size_t size)
+{
+	return selinux_getsecurity(sid, ctx, size);
+}



--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* Re: [RFC][PATCH] collect security labels on user processes generating audit messages
  2006-02-09  1:32 [RFC][PATCH] collect security labels on user processes generating audit messages Timothy R. Chavez
@ 2006-02-09 14:58 ` James Morris
  2006-02-09 15:10   ` Darrel Goeddel
                     ` (3 more replies)
  0 siblings, 4 replies; 40+ messages in thread
From: James Morris @ 2006-02-09 14:58 UTC (permalink / raw)
  To: Timothy R. Chavez
  Cc: selinux, Linux Audit Discussion, James Morris, Stephen Smalley

On Wed, 8 Feb 2006, Timothy R. Chavez wrote:

> 1) A new SELinux interface was introduced to give other parts of the
> kernel the ability to resolve 'sids' into security labels.  

Please look at the way I intend to export SELinux APIs in:
http://people.redhat.com/jmorris/selinux/skfilter/kernel/12-skfilter-selinux-exports.patch

> +++ b/include/linux/netlink.h
> @@ -143,6 +143,7 @@ struct netlink_skb_parms
>  	__u32			dst_group;
>  	kernel_cap_t		eff_cap;
>  	__u32			loginuid;	/* Login (audit) uid */
> +	__u32			secid;		/* SELinux security id */
>  };

You also need to verify the policy serial number.

I wonder if it might be better to use the security context directly.


> @@ -460,11 +464,26 @@ static int audit_receive_msg(struct sk_b
>  			err = 0;
>  			ab = audit_log_start(NULL, GFP_KERNEL, msg_type);
>  			if (ab) {
> +				len = selinux_sid_to_context(sid, NULL, 0);

This is embedding SELinux specific code into the audit code.  I think you 
need to add some audit/SELinux glue code which disappears if SELinux is 
not enabled.

> +	NETLINK_CB(skb).secid = security_task_getsid(current);

security_task_getsid() doesn't exist.

You created security_task_getsecurity(), which retrieves the security
context.



- James
-- 
James Morris
<jmorris@redhat.com>




--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC][PATCH] collect security labels on user processes generating audit messages
  2006-02-09 14:58 ` James Morris
@ 2006-02-09 15:10   ` Darrel Goeddel
  2006-02-09 15:15   ` James Morris
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 40+ messages in thread
From: Darrel Goeddel @ 2006-02-09 15:10 UTC (permalink / raw)
  To: James Morris
  Cc: Timothy R. Chavez, Linux Audit Discussion, James Morris, selinux

James Morris wrote:
> On Wed, 8 Feb 2006, Timothy R. Chavez wrote:
> 
> 
>>1) A new SELinux interface was introduced to give other parts of the
>>kernel the ability to resolve 'sids' into security labels.  
> 
> 
> Please look at the way I intend to export SELinux APIs in:
> http://people.redhat.com/jmorris/selinux/skfilter/kernel/12-skfilter-selinux-exports.patch

This looks good.  I'm also working on some interfaces to export from selinux to
enable efficient audit selection based on SELinux context and I was contemplating
on exactly where to put the goods.  Can we get a consensus on declaration/definition
locations?  "include/linux/selinux.h" and "security/selinux/exports.c" seem good to me.

--

Darrel

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC][PATCH] collect security labels on user processes generating audit messages
  2006-02-09 14:58 ` James Morris
  2006-02-09 15:10   ` Darrel Goeddel
@ 2006-02-09 15:15   ` James Morris
  2006-02-09 17:43     ` Stephen Smalley
  2006-02-09 16:13   ` Timothy R. Chavez
  2006-02-10  0:14   ` Timothy R. Chavez
  3 siblings, 1 reply; 40+ messages in thread
From: James Morris @ 2006-02-09 15:15 UTC (permalink / raw)
  To: James Morris
  Cc: Timothy R. Chavez, selinux, Linux Audit Discussion, Stephen Smalley

On Thu, 9 Feb 2006, James Morris wrote:

> You also need to verify the policy serial number.
> 
> I wonder if it might be better to use the security context directly.

Also consider adding a security field to netlink messages, to managed by 
the LSM.


- James
-- 
James Morris
<jmorris@namei.org>

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC][PATCH] collect security labels on user processes generating audit messages
  2006-02-09 14:58 ` James Morris
  2006-02-09 15:10   ` Darrel Goeddel
  2006-02-09 15:15   ` James Morris
@ 2006-02-09 16:13   ` Timothy R. Chavez
  2006-02-09 17:03     ` James Morris
                       ` (2 more replies)
  2006-02-10  0:14   ` Timothy R. Chavez
  3 siblings, 3 replies; 40+ messages in thread
From: Timothy R. Chavez @ 2006-02-09 16:13 UTC (permalink / raw)
  To: James Morris
  Cc: selinux, Linux Audit Discussion, James Morris, Stephen Smalley

Hi James,

Thank you for the response (and putting Stephen on the CC list,
evolution flubbered my original CC list, hrm).  My response below.

On Thu, 2006-02-09 at 09:58 -0500, James Morris wrote:
> On Wed, 8 Feb 2006, Timothy R. Chavez wrote:
> 
> > 1) A new SELinux interface was introduced to give other parts of the
> > kernel the ability to resolve 'sids' into security labels.  
> 
> Please look at the way I intend to export SELinux APIs in:
> http://people.redhat.com/jmorris/selinux/skfilter/kernel/12-skfilter-selinux-exports.patch

This looks good.  Do you have a schedule for releasing this?  I could
probably wait until it becomes available in -mm before changing out the
API plumbing.

> 
> > +++ b/include/linux/netlink.h
> > @@ -143,6 +143,7 @@ struct netlink_skb_parms
> >  	__u32			dst_group;
> >  	kernel_cap_t		eff_cap;
> >  	__u32			loginuid;	/* Login (audit) uid */
> > +	__u32			secid;		/* SELinux security id */
> >  };
> 
> You also need to verify the policy serial number.

Ah, thanks.

> 
> I wonder if it might be better to use the security context directly.
>

I think it'd be the simplest solution, but I was a bit weary about
adding a string param... I thought using an integer might be the path of
least resistance :)

> 
> > @@ -460,11 +464,26 @@ static int audit_receive_msg(struct sk_b
> >  			err = 0;
> >  			ab = audit_log_start(NULL, GFP_KERNEL, msg_type);
> >  			if (ab) {
> > +				len = selinux_sid_to_context(sid, NULL, 0);
> 
> This is embedding SELinux specific code into the audit code.  I think you 
> need to add some audit/SELinux glue code which disappears if SELinux is 
> not enabled.
> 
> > +	NETLINK_CB(skb).secid = security_task_getsid(current);
> 
> security_task_getsid() doesn't exist.
> 
> You created security_task_getsecurity(), which retrieves the security
> context.
> 
> 
> 
> - James

Actually, security_task_getsid() does exist (or did exist last time I
updated the viro/audit-2.6 git tree).

http://www.promethos.org/lxr/http/ident?i=security_task_getsid


Thanks again for the feedback James.

-tim


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC][PATCH] collect security labels on user processes generating audit messages
  2006-02-09 16:13   ` Timothy R. Chavez
@ 2006-02-09 17:03     ` James Morris
  2006-02-09 17:39       ` Timothy R. Chavez
  2006-02-09 17:29     ` Stephen Smalley
  2006-02-09 18:13     ` Stephen Smalley
  2 siblings, 1 reply; 40+ messages in thread
From: James Morris @ 2006-02-09 17:03 UTC (permalink / raw)
  To: Timothy R. Chavez
  Cc: James Morris, selinux, Linux Audit Discussion, Stephen Smalley

On Thu, 9 Feb 2006, Timothy R. Chavez wrote:

> > Please look at the way I intend to export SELinux APIs in:
> > http://people.redhat.com/jmorris/selinux/skfilter/kernel/12-skfilter-selinux-exports.patch
> 
> This looks good.  Do you have a schedule for releasing this?

No, it's blocked on some core netfilter changes.  I suggest following its 
format, though, if needed.

> > I wonder if it might be better to use the security context directly.
> >
> 
> I think it'd be the simplest solution, but I was a bit weary about
> adding a string param... I thought using an integer might be the path of
> least resistance :)

As previousl mentioned, also consider adding a security blob to the 
netlink params.

> > security_task_getsid() doesn't exist.
> > 
> > You created security_task_getsecurity(), which retrieves the security
> > context.
> 
> Actually, security_task_getsid() does exist (or did exist last time I
> updated the viro/audit-2.6 git tree).
> 
> http://www.promethos.org/lxr/http/ident?i=security_task_getsid

Oh, ok.

Where is security_task_getsecurity() used, then?


- James
-- 
James Morris
<jmorris@namei.org>

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC][PATCH] collect security labels on user processes generating audit messages
  2006-02-09 16:13   ` Timothy R. Chavez
  2006-02-09 17:03     ` James Morris
@ 2006-02-09 17:29     ` Stephen Smalley
  2006-02-09 18:13     ` Stephen Smalley
  2 siblings, 0 replies; 40+ messages in thread
From: Stephen Smalley @ 2006-02-09 17:29 UTC (permalink / raw)
  To: Timothy R. Chavez
  Cc: James Morris, selinux, Linux Audit Discussion, James Morris

On Thu, 2006-02-09 at 10:13 -0600, Timothy R. Chavez wrote:
> > You also need to verify the policy serial number.
> 
> Ah, thanks.

Not clear actually - the context structs and integer index values for
the components need to be tagged with a policy serial number if exported
outside of the security server, but the SID itself remains stable across
policy reloads; only the context struct contents are remapped.  If
invalidated, subsequent lookup of the SID will be remapped to the
unlabeled SID's context.

> I think it'd be the simplest solution, but I was a bit weary about
> adding a string param... I thought using an integer might be the path of
> least resistance :)

Yes, a SID makes sense here and avoids the allocation/lifecycle pain of
strings or generic security blobs.

> Actually, security_task_getsid() does exist (or did exist last time I
> updated the viro/audit-2.6 git tree).

It doesn't do what you think it does.   Look at the inline documentation
for it in security.h.

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC][PATCH] collect security labels on user processes generating audit messages
  2006-02-09 17:03     ` James Morris
@ 2006-02-09 17:39       ` Timothy R. Chavez
  0 siblings, 0 replies; 40+ messages in thread
From: Timothy R. Chavez @ 2006-02-09 17:39 UTC (permalink / raw)
  To: James Morris
  Cc: James Morris, selinux, Linux Audit Discussion, Stephen Smalley

On Thu, 2006-02-09 at 12:03 -0500, James Morris wrote:
<snip>
> > > security_task_getsid() doesn't exist.
> > > 
> > > You created security_task_getsecurity(), which retrieves the security
> > > context.
> > 
> > Actually, security_task_getsid() does exist (or did exist last time I
> > updated the viro/audit-2.6 git tree).
> > 
> > http://www.promethos.org/lxr/http/ident?i=security_task_getsid
> 
> Oh, ok.
> 
> Where is security_task_getsecurity() used, then?
> 
> 
> - James

Hm... :) I made it at first because I was just going to extract the
context directly and store it with the skb_params, but then decided to
go the 'sid' route.  I left it in to expand the LSM API a little, but if
it's not being used currently, it probably shouldn't be included, eh?

Thanks.

-tim


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC][PATCH] collect security labels on user processes generating audit messages
  2006-02-09 15:15   ` James Morris
@ 2006-02-09 17:43     ` Stephen Smalley
  0 siblings, 0 replies; 40+ messages in thread
From: Stephen Smalley @ 2006-02-09 17:43 UTC (permalink / raw)
  To: James Morris
  Cc: James Morris, Timothy R. Chavez, selinux, Linux Audit Discussion

On Thu, 2006-02-09 at 10:15 -0500, James Morris wrote:
> On Thu, 9 Feb 2006, James Morris wrote:
> 
> > You also need to verify the policy serial number.
> > 
> > I wonder if it might be better to use the security context directly.
> 
> Also consider adding a security field to netlink messages, to managed by 
> the LSM.

A generic security blob would require full lifecycle maintenance,
including the old sk hooks that were rejected for 2.5.  I also think it
is a mistake to generalize the audit system's use of SELinux, as I don't
think that any other LSM is likely to provide LSPP functionality, so we
might as well just convert all of these audit-SELinux interfaces into
direct SELinux calls IMHO.  At least in the long term if not
immediately.

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC][PATCH] collect security labels on user processes generating audit messages
  2006-02-09 16:13   ` Timothy R. Chavez
  2006-02-09 17:03     ` James Morris
  2006-02-09 17:29     ` Stephen Smalley
@ 2006-02-09 18:13     ` Stephen Smalley
  2 siblings, 0 replies; 40+ messages in thread
From: Stephen Smalley @ 2006-02-09 18:13 UTC (permalink / raw)
  To: Timothy R. Chavez
  Cc: James Morris, selinux, Linux Audit Discussion, James Morris

On Thu, 2006-02-09 at 10:13 -0600, Timothy R. Chavez wrote:
> On Thu, 2006-02-09 at 09:58 -0500, James Morris wrote:
> > Please look at the way I intend to export SELinux APIs in:
> > http://people.redhat.com/jmorris/selinux/skfilter/kernel/12-skfilter-selinux-exports.patch
> 
> This looks good.  Do you have a schedule for releasing this?  I could
> probably wait until it becomes available in -mm before changing out the
> API plumbing.

Note btw that the advantage of the security_sid_to_context() interface
(wrapped by James' selinux_id_to_ctx interface) is that it internally
allocates a buffer of the right length for you.  You don't have to query
for a length and allocate one yourself, unlike the selinux_getsecurity
interface.  You do still need to free it when done.

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC][PATCH] collect security labels on user processes generating audit messages
  2006-02-09 14:58 ` James Morris
                     ` (2 preceding siblings ...)
  2006-02-09 16:13   ` Timothy R. Chavez
@ 2006-02-10  0:14   ` Timothy R. Chavez
  2006-02-10  4:00     ` James Morris
  2006-02-13 19:12     ` Stephen Smalley
  3 siblings, 2 replies; 40+ messages in thread
From: Timothy R. Chavez @ 2006-02-10  0:14 UTC (permalink / raw)
  To: James Morris
  Cc: selinux, Linux Audit Discussion, James Morris, Stephen Smalley

Hello,

This is an updated patch to collect the SID of a user process sending
audit messages while running in its context and then converting it into
the corresponding security label once the audit message is received in a
kernel context for logging purposes.

This patch: 

1) Augments the LSM interface to retrieve a task's SID and scraps the
incorrect usage of security_task_getsid (Thanks Stephen ;)).

2) Uses a portion of the James Morris' SELinux API to give the ability
to easily convert a SID into a security label / SELinux context. 

http://people.redhat.com/jmorris/selinux/skfilter/kernel/12-skfilter-selinux-exports.patch


Thanks.  Comments and feedback welcome.

diff --git a/include/linux/netlink.h b/include/linux/netlink.h
index 6a2ccf7..ccd5905 100644
--- a/include/linux/netlink.h
+++ b/include/linux/netlink.h
@@ -143,6 +143,7 @@ struct netlink_skb_parms
 	__u32			dst_group;
 	kernel_cap_t		eff_cap;
 	__u32			loginuid;	/* Login (audit) uid */
+	__u32			secid;		/* SELinux security id */
 };
 
 #define NETLINK_CB(skb)		(*(struct netlink_skb_parms*)&((skb)->cb))
diff --git a/include/linux/security.h b/include/linux/security.h
index b4fe8aa..bed528c 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -625,6 +625,11 @@ struct swap_info_struct;
  *	@p contains the task_struct for the task.
  *	@inode contains the inode structure for the inode.
  *
+ * @task_getsecid:
+ * 	Get the SID (security id) of a task.
+ *	@tsk contains the task_struct for the task
+ *	@sid is storage for task SID
+ *
  * Security hooks for Netlink messaging.
  *
  * @netlink_send:
@@ -1169,6 +1174,7 @@ struct security_operations {
 			   unsigned long arg5);
 	void (*task_reparent_to_init) (struct task_struct * p);
 	void (*task_to_inode)(struct task_struct *p, struct inode *inode);
+	int (*task_getsecid)(struct task_struct *tsk, __u32 *sid);
 
 	int (*ipc_permission) (struct kern_ipc_perm * ipcp, short flag);
 	int (*ipc_getsecurity)(struct kern_ipc_perm *ipcp, void *buffer, size_t size);
@@ -1817,6 +1823,11 @@ static inline void security_task_to_inod
 	security_ops->task_to_inode(p, inode);
 }
 
+static inline int security_task_getsecid(struct task_struct *tsk, __u32 *sid)
+{
+	return security_ops->task_getsecid(tsk, sid);
+}
+
 static inline int security_ipc_permission (struct kern_ipc_perm *ipcp,
 					   short flag)
 {
@@ -2457,6 +2468,11 @@ static inline void security_task_reparen
 static inline void security_task_to_inode(struct task_struct *p, struct inode *inode)
 { }
 
+static inline int security_task_getsecid(struct task_struct *tsk, __u32 *sid)
+{
+	return -EOPNOTSUPP;
+}
+
 static inline int security_ipc_permission (struct kern_ipc_perm *ipcp,
 					   short flag)
 {
diff --git a/include/linux/selinux.h b/include/linux/selinux.h
new file mode 100644
index 0000000..7da2da3
--- /dev/null
+++ b/include/linux/selinux.h
@@ -0,0 +1,70 @@
+/*
+ * SELinux services exported to the rest of the kernel.
+ *
+ * Author: James Morris <jmorris@redhat.com>
+ *
+ * Copyright (C) 2005 Red Hat, Inc., James Morris <jmorris@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.
+ */
+#ifndef _LINUX_SELINUX_H
+#define _LINUX_SELINUX_H
+
+#ifdef CONFIG_SECURITY_SELINUX
+
+struct sock;
+
+/**
+ *	selinux_available - check if SELinux is available for use.
+ *
+ *	Returns true if configured, enabled, not disabled and policy loaded.
+ */
+int selinux_available(void);
+
+/**
+ *	selinux_ctx_to_id - map a security context string to an ID
+ *	@ctx: the security context string to be mapped
+ *	@ctxid: ID value returned via this.
+ *
+ *	Returns 0 if successful, with the ID stored in ctxid.  A value
+ *	of zero for the ctxid indicates no ID could be determined (but
+ *	no error occurred).  Otherwise, this value should only be used
+ *	opaquely (e.g. compare with value from selinux_sk_ctxid())
+ */
+int selinux_ctx_to_id(const char *ctx, u32 *ctxid);
+
+/**
+ *	selinux_id_to_ctx - 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_id_to_ctx(u32 ctxid, char **ctx, u32 *ctxlen);
+
+#else
+
+static inline int selinux_available(void)
+{
+	return 0;
+}
+
+static inline int selinux_ctx_to_id(const char *ctx, u32 *ctxid)
+{
+	*ctxid = 0;
+	return 0;
+}
+
+static inline int selinux_id_to_ctx(u32 ctxid, char **ctx, u32 *ctxlen)
+{
+	return 0;
+}
+
+#endif /* CONFIG_SECURITY_SELINUX */
+
+#endif /* _LINUX_SELINUX_H */
diff --git a/kernel/audit.c b/kernel/audit.c
index d95efd6..129b3da 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -50,6 +50,7 @@
 #include <linux/kthread.h>
 
 #include <linux/audit.h>
+#include <linux/selinux.h>
 
 #include <net/sock.h>
 #include <linux/skbuff.h>
@@ -383,7 +384,7 @@ static int audit_netlink_ok(kernel_cap_t
 
 static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 {
-	u32			uid, pid, seq;
+	u32			uid, pid, sid, seq;
 	void			*data;
 	struct audit_status	*status_get, status_set;
 	int			err;
@@ -391,6 +392,8 @@ static int audit_receive_msg(struct sk_b
 	u16			msg_type = nlh->nlmsg_type;
 	uid_t			loginuid; /* loginuid of sender */
 	struct audit_sig_info   sig_data;
+	char *			ctx = NULL;
+	u32			len;
 
 	err = audit_netlink_ok(NETLINK_CB(skb).eff_cap, msg_type);
 	if (err)
@@ -409,6 +412,7 @@ static int audit_receive_msg(struct sk_b
 	pid  = NETLINK_CREDS(skb)->pid;
 	uid  = NETLINK_CREDS(skb)->uid;
 	loginuid = NETLINK_CB(skb).loginuid;
+	sid = NETLINK_CB(skb).secid;
 	seq  = nlh->nlmsg_seq;
 	data = NLMSG_DATA(nlh);
 
@@ -460,11 +464,17 @@ static int audit_receive_msg(struct sk_b
 			err = 0;
 			ab = audit_log_start(NULL, GFP_KERNEL, msg_type);
 			if (ab) {
+				if (selinux_available()) {
+					err = selinux_id_to_ctx(sid, &ctx, &len);
+					if (err < 0)
+						return err;
+				}
 				audit_log_format(ab,
-						 "user pid=%d uid=%u auid=%u msg='%.1024s'",
-						 pid, uid, loginuid, (char *)data);
+						 "user pid=%d uid=%u auid=%u subj=%s msg='%.1024s'",
+						 pid, uid, loginuid, ctx ? ctx : "null", (char *)data);
 				audit_set_pid(ab, pid);
 				audit_log_end(ab);
+				kfree(ctx);
 			}
 		}
 		break;
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 96020d7..8b9eff4 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -1120,6 +1120,9 @@ static int netlink_sendmsg(struct kiocb 
 	NETLINK_CB(skb).dst_pid = dst_pid;
 	NETLINK_CB(skb).dst_group = dst_group;
 	NETLINK_CB(skb).loginuid = audit_get_loginuid(current->audit_context);
+	err = security_task_getsecid(current, &NETLINK_CB(skb).secid);
+	if (err < 0 && err != -EOPNOTSUPP)
+		goto out;
 	memcpy(NETLINK_CREDS(skb), &siocb->scm->creds, sizeof(struct ucred));
 
 	/* What can I do? Netlink is asynchronous, so that
diff --git a/security/dummy.c b/security/dummy.c
index 75e7c4a..ef32c87 100644
--- a/security/dummy.c
+++ b/security/dummy.c
@@ -557,6 +557,11 @@ static void dummy_task_reparent_to_init 
 static void dummy_task_to_inode(struct task_struct *p, struct inode *inode)
 { }
 
+static int dummy_task_getsecid(struct task_struct *tsk, __u32 *sid)
+{
+	return -EOPNOTSUPP;
+}
+
 static int dummy_ipc_permission (struct kern_ipc_perm *ipcp, short flag)
 {
 	return 0;
@@ -934,6 +939,7 @@ void security_fixup_ops (struct security
 	set_to_dummy_if_null(ops, task_prctl);
 	set_to_dummy_if_null(ops, task_reparent_to_init);
  	set_to_dummy_if_null(ops, task_to_inode);
+	set_to_dummy_if_null(ops, task_getsecid);
 	set_to_dummy_if_null(ops, ipc_permission);
 	set_to_dummy_if_null(ops, ipc_getsecurity);
 	set_to_dummy_if_null(ops, msg_msg_alloc_security);
diff --git a/security/selinux/Makefile b/security/selinux/Makefile
index b038cd0..3e3d4eb 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
+selinux-y := avc.o hooks.o selinuxfs.o netlink.o nlmsgtab.o exports.o
 
 selinux-$(CONFIG_SECURITY_NETWORK) += netif.o
 
diff --git a/security/selinux/exports.c b/security/selinux/exports.c
new file mode 100644
index 0000000..4dc7405
--- /dev/null
+++ b/security/selinux/exports.c
@@ -0,0 +1,38 @@
+/*
+ * SELinux services exported to the rest of the kernel.
+ *
+ * Author: James Morris <jmorris@redhat.com>
+ *
+ * Copyright (C) 2005 Red Hat, Inc., James Morris <jmorris@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"
+
+extern int ss_initialized;
+
+int selinux_available(void)
+{
+	return ss_initialized;
+}
+
+int selinux_ctx_to_id(const char *ctx, u32 *ctxid)
+{
+	return security_context_to_sid(ctx, strlen(ctx), ctxid);
+}
+
+int selinux_id_to_ctx(u32 ctxid, char **ctx, u32 *ctxlen)
+{
+	return security_sid_to_context(ctxid, ctx, ctxlen);
+}
+
+EXPORT_SYMBOL_GPL(selinux_available);
+EXPORT_SYMBOL_GPL(selinux_ctx_to_id);
+EXPORT_SYMBOL_GPL(selinux_id_to_ctx); 
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 21c8aa6..ae9b097 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2769,6 +2769,13 @@ static void selinux_task_to_inode(struct
 	return;
 }
 
+static int selinux_task_getsecid(struct task_struct *tsk)
+{
+	struct task_security_struct *tsec = tsk->security;
+	
+	return tsec->sid;
+}
+
 #ifdef CONFIG_SECURITY_NETWORK
 
 /* Returns error only if unable to parse addresses */
@@ -4319,6 +4326,7 @@ static struct security_operations selinu
 	.task_prctl =			selinux_task_prctl,
 	.task_reparent_to_init =	selinux_task_reparent_to_init,
 	.task_to_inode =                selinux_task_to_inode,
+	.task_getsecid =		selinux_task_getsecid,
 
 	.ipc_permission =		selinux_ipc_permission,
 	.ipc_getsecurity =		selinux_ipc_getsecurity,
diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
index 5f016c9..a2e5bb0 100644
--- a/security/selinux/include/security.h
+++ b/security/selinux/include/security.h
@@ -63,7 +63,7 @@ int security_change_sid(u32 ssid, u32 ts
 int security_sid_to_context(u32 sid, char **scontext,
 	u32 *scontext_len);
 
-int security_context_to_sid(char *scontext, u32 scontext_len,
+int security_context_to_sid(const char *scontext, u32 scontext_len,
 	u32 *out_sid);
 
 int security_context_to_sid_default(char *scontext, u32 scontext_len, u32 *out_sid, u32 def_sid);
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index 2311255..c66f765 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -617,7 +617,7 @@ out:
 
 }
 
-static int security_context_to_sid_core(char *scontext, u32 scontext_len, u32 *sid, u32 def_sid)
+static int security_context_to_sid_core(const char *scontext, u32 scontext_len, u32 *sid, u32 def_sid)
 {
 	char *scontext2;
 	struct context context;
@@ -743,7 +743,7 @@ out:
  * Returns -%EINVAL if the context is invalid, -%ENOMEM if insufficient
  * memory is available, or 0 on success.
  */
-int security_context_to_sid(char *scontext, u32 scontext_len, u32 *sid)
+int security_context_to_sid(const char *scontext, u32 scontext_len, u32 *sid)
 {
 	return security_context_to_sid_core(scontext, scontext_len,
 	                                    sid, SECSID_NULL);




--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* Re: [RFC][PATCH] collect security labels on user processes generating audit messages
  2006-02-10  0:14   ` Timothy R. Chavez
@ 2006-02-10  4:00     ` James Morris
  2006-02-13 19:12     ` Stephen Smalley
  1 sibling, 0 replies; 40+ messages in thread
From: James Morris @ 2006-02-10  4:00 UTC (permalink / raw)
  To: Timothy R. Chavez
  Cc: James Morris, selinux, Linux Audit Discussion, Stephen Smalley

On Thu, 9 Feb 2006, Timothy R. Chavez wrote:

> +int selinux_ctx_to_id(const char *ctx, u32 *ctxid);

You're not using this function, so don't include it for upstream.


- James
-- 
James Morris
<jmorris@namei.org>

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC][PATCH] collect security labels on user processes generating audit messages
  2006-02-10  0:14   ` Timothy R. Chavez
  2006-02-10  4:00     ` James Morris
@ 2006-02-13 19:12     ` Stephen Smalley
  2006-02-14 23:48       ` Timothy R. Chavez
  1 sibling, 1 reply; 40+ messages in thread
From: Stephen Smalley @ 2006-02-13 19:12 UTC (permalink / raw)
  To: Timothy R. Chavez
  Cc: James Morris, selinux, Linux Audit Discussion, James Morris

On Thu, 2006-02-09 at 18:14 -0600, Timothy R. Chavez wrote:
> Thanks.  Comments and feedback welcome.

> diff --git a/include/linux/selinux.h b/include/linux/selinux.h
> new file mode 100644
> index 0000000..7da2da3
> --- /dev/null
> +++ b/include/linux/selinux.h
> @@ -0,0 +1,70 @@
> +/*
> + * SELinux services exported to the rest of the kernel.
> + *
> + * Author: James Morris <jmorris@redhat.com>
> + *
> + * Copyright (C) 2005 Red Hat, Inc., James Morris <jmorris@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.
> + */
> +#ifndef _LINUX_SELINUX_H
> +#define _LINUX_SELINUX_H
> +
> +#ifdef CONFIG_SECURITY_SELINUX
> +
> +struct sock;

You can omit the above struct declaration for your purposes, since you
aren't upstreaming the socket-related interfaces.

> +static inline int selinux_id_to_ctx(u32 ctxid, char **ctx, u32 *ctxlen)
> +{
> +	return 0;
> +}

I'd be inclined to return an error here; otherwise, the caller may
proceed to use *ctx and *ctxlen.  Or, if you want this to return 0 for a
graceful degenerate case (e.g. so that you don't need to check
selinux_available separately each time), I think you need to initialize
*ctx and *ctxlen as well to sane values.

BTW, traditionally, SELinux has viewed the terminating NUL as part of
the context string and thus as part of its length in bytes.  A
concession was made in the security server code to allow context strings
that lacked the terminating NUL when we migrated to xattrs because the
existing attr package did not include the terminating NUL (e.g. for
setfattr).

> diff --git a/kernel/audit.c b/kernel/audit.c
> index d95efd6..129b3da 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -460,11 +464,17 @@ static int audit_receive_msg(struct sk_b
>  			err = 0;
>  			ab = audit_log_start(NULL, GFP_KERNEL, msg_type);
>  			if (ab) {
> +				if (selinux_available()) {
> +					err = selinux_id_to_ctx(sid, &ctx, &len);
> +					if (err < 0)
> +						return err;
> +				}

Is simply returning an error sufficient here?  What about the ab that
has already been allocated?

> diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
> index 96020d7..8b9eff4 100644
> --- a/net/netlink/af_netlink.c
> +++ b/net/netlink/af_netlink.c
> @@ -1120,6 +1120,9 @@ static int netlink_sendmsg(struct kiocb 
>  	NETLINK_CB(skb).dst_pid = dst_pid;
>  	NETLINK_CB(skb).dst_group = dst_group;
>  	NETLINK_CB(skb).loginuid = audit_get_loginuid(current->audit_context);
> +	err = security_task_getsecid(current, &NETLINK_CB(skb).secid);
> +	if (err < 0 && err != -EOPNOTSUPP)
> +		goto out;

Don't you need to kfree_skb(skb) too?  And perhaps the dummy function
should just set the sid to 0 and return 0 to avoid the extra test here
for -EOPNOTSUPP?  In fact, perhaps this hook should just return void,
thereby avoiding any need for error handling here?  In what case could
this fail, as no allocation is happening and no permission check occurs
from that hook (happens from security_netlink_send instead)?

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC][PATCH] collect security labels on user processes generating audit messages
  2006-02-13 19:12     ` Stephen Smalley
@ 2006-02-14 23:48       ` Timothy R. Chavez
  2006-02-15 13:47         ` Stephen Smalley
  2006-02-15 21:05         ` Darrel Goeddel
  0 siblings, 2 replies; 40+ messages in thread
From: Timothy R. Chavez @ 2006-02-14 23:48 UTC (permalink / raw)
  To: Stephen Smalley
  Cc: James Morris, selinux, Linux Audit Discussion, James Morris

James & Stephen,

Thank you for the comments.  While implementing your feedback I came
across a pretty severe bug.  I was basically obtaining the sid and then
throwing it away (I was returning it from the function, but not actually
assigning it to anything).  New patch below.  I still need to test this
a little more.  Thanks!

-tim

diff --git a/include/linux/netlink.h b/include/linux/netlink.h
index 6a2ccf7..ccd5905 100644
--- a/include/linux/netlink.h
+++ b/include/linux/netlink.h
@@ -143,6 +143,7 @@ struct netlink_skb_parms
 	__u32			dst_group;
 	kernel_cap_t		eff_cap;
 	__u32			loginuid;	/* Login (audit) uid */
+	__u32			secid;		/* SELinux security id */
 };
 
 #define NETLINK_CB(skb)		(*(struct netlink_skb_parms*)&((skb)->cb))
diff --git a/include/linux/security.h b/include/linux/security.h
index b4fe8aa..c6fe5fe 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -625,6 +625,11 @@ struct swap_info_struct;
  *	@p contains the task_struct for the task.
  *	@inode contains the inode structure for the inode.
  *
+ * @task_getsecid:
+ * 	Get the SID (security id) of a task.
+ *	@tsk contains the task_struct for the task
+ *	@sid is storage for task SID
+ *
  * Security hooks for Netlink messaging.
  *
  * @netlink_send:
@@ -1169,6 +1174,7 @@ struct security_operations {
 			   unsigned long arg5);
 	void (*task_reparent_to_init) (struct task_struct * p);
 	void (*task_to_inode)(struct task_struct *p, struct inode *inode);
+	void (*task_getsecid)(struct task_struct *tsk, __u32 *sid);
 
 	int (*ipc_permission) (struct kern_ipc_perm * ipcp, short flag);
 	int (*ipc_getsecurity)(struct kern_ipc_perm *ipcp, void *buffer, size_t size);
@@ -1817,6 +1823,11 @@ static inline void security_task_to_inod
 	security_ops->task_to_inode(p, inode);
 }
 
+static inline void security_task_getsecid(struct task_struct *tsk, __u32 *sid)
+{
+	security_ops->task_getsecid(tsk, sid);
+}
+
 static inline int security_ipc_permission (struct kern_ipc_perm *ipcp,
 					   short flag)
 {
@@ -2457,6 +2468,9 @@ static inline void security_task_reparen
 static inline void security_task_to_inode(struct task_struct *p, struct inode *inode)
 { }
 
+static inline void security_task_getsecid(struct task_struct *tsk, __u32 *sid)
+{ }
+
 static inline int security_ipc_permission (struct kern_ipc_perm *ipcp,
 					   short flag)
 {
diff --git a/include/linux/selinux.h b/include/linux/selinux.h
new file mode 100644
index 0000000..c2e0e20
--- /dev/null
+++ b/include/linux/selinux.h
@@ -0,0 +1,52 @@
+/*
+ * SELinux services exported to the rest of the kernel.
+ *
+ * Author: James Morris <jmorris@redhat.com>
+ *
+ * Copyright (C) 2005 Red Hat, Inc., James Morris <jmorris@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.
+ */
+#ifndef _LINUX_SELINUX_H
+#define _LINUX_SELINUX_H
+
+#ifdef CONFIG_SECURITY_SELINUX
+
+/**
+ *	selinux_available - check if SELinux is available for use.
+ *
+ *	Returns true if configured, enabled, not disabled and policy loaded.
+ */
+int selinux_available(void);
+
+/**
+ *	selinux_id_to_ctx - 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_id_to_ctx(u32 ctxid, char **ctx, u32 *ctxlen);
+
+#else
+
+static inline int selinux_available(void)
+{
+	return 0;
+}
+
+static inline int selinux_id_to_ctx(u32 ctxid, char **ctx, u32 *ctxlen)
+{
+	*ctx = NULL;
+	*ctxlen = 0;
+	return 0;
+}
+
+#endif /* CONFIG_SECURITY_SELINUX */
+
+#endif /* _LINUX_SELINUX_H */
diff --git a/kernel/audit.c b/kernel/audit.c
index d95efd6..4ca77dd 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -50,6 +50,7 @@
 #include <linux/kthread.h>
 
 #include <linux/audit.h>
+#include <linux/selinux.h>
 
 #include <net/sock.h>
 #include <linux/skbuff.h>
@@ -383,7 +384,7 @@ static int audit_netlink_ok(kernel_cap_t
 
 static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 {
-	u32			uid, pid, seq;
+	u32			uid, pid, sid, seq;
 	void			*data;
 	struct audit_status	*status_get, status_set;
 	int			err;
@@ -391,6 +392,8 @@ static int audit_receive_msg(struct sk_b
 	u16			msg_type = nlh->nlmsg_type;
 	uid_t			loginuid; /* loginuid of sender */
 	struct audit_sig_info   sig_data;
+	char *			ctx = NULL;
+	u32			len;
 
 	err = audit_netlink_ok(NETLINK_CB(skb).eff_cap, msg_type);
 	if (err)
@@ -409,6 +412,7 @@ static int audit_receive_msg(struct sk_b
 	pid  = NETLINK_CREDS(skb)->pid;
 	uid  = NETLINK_CREDS(skb)->uid;
 	loginuid = NETLINK_CB(skb).loginuid;
+	sid = NETLINK_CB(skb).secid;
 	seq  = nlh->nlmsg_seq;
 	data = NLMSG_DATA(nlh);
 
@@ -458,14 +462,20 @@ static int audit_receive_msg(struct sk_b
 		err = audit_filter_user(&NETLINK_CB(skb), msg_type);
 		if (err == 1) {
 			err = 0;
+			if (selinux_available()) {
+				err = selinux_id_to_ctx(sid, &ctx, &len);
+				if (err < 0)
+					return err;
+			}
 			ab = audit_log_start(NULL, GFP_KERNEL, msg_type);
 			if (ab) {
 				audit_log_format(ab,
-						 "user pid=%d uid=%u auid=%u msg='%.1024s'",
-						 pid, uid, loginuid, (char *)data);
+						 "user pid=%d uid=%u auid=%u subj=%s msg='%.1024s'",
+						 pid, uid, loginuid, ctx ? ctx : "null", (char *)data);
 				audit_set_pid(ab, pid);
 				audit_log_end(ab);
 			}
+			kfree(ctx);
 		}
 		break;
 	case AUDIT_ADD:
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 96020d7..1f4b241 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -1120,6 +1120,7 @@ static int netlink_sendmsg(struct kiocb 
 	NETLINK_CB(skb).dst_pid = dst_pid;
 	NETLINK_CB(skb).dst_group = dst_group;
 	NETLINK_CB(skb).loginuid = audit_get_loginuid(current->audit_context);
+	security_task_getsecid(current, &NETLINK_CB(skb).secid);
 	memcpy(NETLINK_CREDS(skb), &siocb->scm->creds, sizeof(struct ucred));
 
 	/* What can I do? Netlink is asynchronous, so that
diff --git a/security/dummy.c b/security/dummy.c
index 75e7c4a..2325823 100644
--- a/security/dummy.c
+++ b/security/dummy.c
@@ -557,6 +557,12 @@ static void dummy_task_reparent_to_init 
 static void dummy_task_to_inode(struct task_struct *p, struct inode *inode)
 { }
 
+static void dummy_task_getsecid(struct task_struct *tsk, __u32 *sid)
+{
+	*sid = 0;
+	return;
+}
+
 static int dummy_ipc_permission (struct kern_ipc_perm *ipcp, short flag)
 {
 	return 0;
@@ -934,6 +940,7 @@ void security_fixup_ops (struct security
 	set_to_dummy_if_null(ops, task_prctl);
 	set_to_dummy_if_null(ops, task_reparent_to_init);
  	set_to_dummy_if_null(ops, task_to_inode);
+	set_to_dummy_if_null(ops, task_getsecid);
 	set_to_dummy_if_null(ops, ipc_permission);
 	set_to_dummy_if_null(ops, ipc_getsecurity);
 	set_to_dummy_if_null(ops, msg_msg_alloc_security);
diff --git a/security/selinux/Makefile b/security/selinux/Makefile
index b038cd0..3e3d4eb 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
+selinux-y := avc.o hooks.o selinuxfs.o netlink.o nlmsgtab.o exports.o
 
 selinux-$(CONFIG_SECURITY_NETWORK) += netif.o
 
diff --git a/security/selinux/exports.c b/security/selinux/exports.c
new file mode 100644
index 0000000..c4707fc
--- /dev/null
+++ b/security/selinux/exports.c
@@ -0,0 +1,32 @@
+/*
+ * SELinux services exported to the rest of the kernel.
+ *
+ * Author: James Morris <jmorris@redhat.com>
+ *
+ * Copyright (C) 2005 Red Hat, Inc., James Morris <jmorris@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"
+
+extern int ss_initialized;
+
+int selinux_available(void)
+{
+	return ss_initialized;
+}
+
+int selinux_id_to_ctx(u32 ctxid, char **ctx, u32 *ctxlen)
+{
+	return security_sid_to_context(ctxid, ctx, ctxlen);
+}
+
+EXPORT_SYMBOL_GPL(selinux_available);
+EXPORT_SYMBOL_GPL(selinux_id_to_ctx);
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 21c8aa6..d2356ad 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2769,6 +2769,14 @@ static void selinux_task_to_inode(struct
 	return;
 }
 
+static void selinux_task_getsecid(struct task_struct *tsk, __u32 *sid)
+{
+	struct task_security_struct *tsec = tsk->security;
+	
+	*sid = tsec->sid;
+	return;
+}
+
 #ifdef CONFIG_SECURITY_NETWORK
 
 /* Returns error only if unable to parse addresses */
@@ -4319,6 +4327,7 @@ static struct security_operations selinu
 	.task_prctl =			selinux_task_prctl,
 	.task_reparent_to_init =	selinux_task_reparent_to_init,
 	.task_to_inode =                selinux_task_to_inode,
+	.task_getsecid =		selinux_task_getsecid,
 
 	.ipc_permission =		selinux_ipc_permission,
 	.ipc_getsecurity =		selinux_ipc_getsecurity,
diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
index 5f016c9..a2e5bb0 100644
--- a/security/selinux/include/security.h
+++ b/security/selinux/include/security.h
@@ -63,7 +63,7 @@ int security_change_sid(u32 ssid, u32 ts
 int security_sid_to_context(u32 sid, char **scontext,
 	u32 *scontext_len);
 
-int security_context_to_sid(char *scontext, u32 scontext_len,
+int security_context_to_sid(const char *scontext, u32 scontext_len,
 	u32 *out_sid);
 
 int security_context_to_sid_default(char *scontext, u32 scontext_len, u32 *out_sid, u32 def_sid);
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index 2311255..c66f765 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -617,7 +617,7 @@ out:
 
 }
 
-static int security_context_to_sid_core(char *scontext, u32 scontext_len, u32 *sid, u32 def_sid)
+static int security_context_to_sid_core(const char *scontext, u32 scontext_len, u32 *sid, u32 def_sid)
 {
 	char *scontext2;
 	struct context context;
@@ -743,7 +743,7 @@ out:
  * Returns -%EINVAL if the context is invalid, -%ENOMEM if insufficient
  * memory is available, or 0 on success.
  */
-int security_context_to_sid(char *scontext, u32 scontext_len, u32 *sid)
+int security_context_to_sid(const char *scontext, u32 scontext_len, u32 *sid)
 {
 	return security_context_to_sid_core(scontext, scontext_len,
 	                                    sid, SECSID_NULL);



--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* Re: [RFC][PATCH] collect security labels on user processes generating audit messages
  2006-02-14 23:48       ` Timothy R. Chavez
@ 2006-02-15 13:47         ` Stephen Smalley
  2006-02-15 15:49           ` Timothy R. Chavez
  2006-02-15 21:05         ` Darrel Goeddel
  1 sibling, 1 reply; 40+ messages in thread
From: Stephen Smalley @ 2006-02-15 13:47 UTC (permalink / raw)
  To: Timothy R. Chavez
  Cc: James Morris, selinux, Linux Audit Discussion, James Morris

On Tue, 2006-02-14 at 17:48 -0600, Timothy R. Chavez wrote:
> diff --git a/include/linux/netlink.h b/include/linux/netlink.h
> index 6a2ccf7..ccd5905 100644
> --- a/include/linux/netlink.h
> +++ b/include/linux/netlink.h
> @@ -143,6 +143,7 @@ struct netlink_skb_parms
>  	__u32			dst_group;
>  	kernel_cap_t		eff_cap;
>  	__u32			loginuid;	/* Login (audit) uid */
> +	__u32			secid;		/* SELinux security id */
>  };
>  
>  #define NETLINK_CB(skb)		(*(struct netlink_skb_parms*)&((skb)->cb))

As a minor nit, does anyone know why '__u32' is used here vs. 'u32'?
The definition is already wrapped with an #ifdef __KERNEL__.  I see that
you are being consistent with the existing fields, but then we use just
'u32' in the audit code and in the SELinux interfaces and code.  Seems
like we should be consistent throughout, and I don't see a real reason
to use __u32 vs. just u32 if it is all kernel code and not included in
userland (or protected by #ifdef __KERNEL__).

> diff --git a/include/linux/security.h b/include/linux/security.h
> index b4fe8aa..c6fe5fe 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -1169,6 +1174,7 @@ struct security_operations {
>  			   unsigned long arg5);
>  	void (*task_reparent_to_init) (struct task_struct * p);
>  	void (*task_to_inode)(struct task_struct *p, struct inode *inode);
> +	void (*task_getsecid)(struct task_struct *tsk, __u32 *sid);

Same issue for the security hook interfaces.

> @@ -2457,6 +2468,9 @@ static inline void security_task_reparen
>  static inline void security_task_to_inode(struct task_struct *p, struct inode *inode)
>  { }
>  
> +static inline void security_task_getsecid(struct task_struct *tsk, __u32 *sid)
> +{ }
> +

Should we set *sid = 0 here as in the dummy function, just to make sure
it doesn't contain garbage?

> diff --git a/kernel/audit.c b/kernel/audit.c
> index d95efd6..4ca77dd 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -458,14 +462,20 @@ static int audit_receive_msg(struct sk_b
>  		err = audit_filter_user(&NETLINK_CB(skb), msg_type);
>  		if (err == 1) {
>  			err = 0;
> +			if (selinux_available()) {
> +				err = selinux_id_to_ctx(sid, &ctx, &len);
> +				if (err < 0)
> +					return err;
> +			}

It seems unfortunate to have to wrap each call to a public SELinux
interface with a selinux_available() check, and the !
defined(CONFIG_SECURITY_SELINUX) case would actually work without such a
check since it just sets (ctx, len) to (NULL, 0).  So the
selinux_available() check is only necessary for the case where SELinux
is disabled at runtime (selinux=0 or /selinux/disable).  Possibly it
should be moved within selinux_id_to_ctx() so that callers can just call
selinux_id_to_ctx() unconditionally?

>  			ab = audit_log_start(NULL, GFP_KERNEL, msg_type);
>  			if (ab) {
>  				audit_log_format(ab,
> -						 "user pid=%d uid=%u auid=%u msg='%.1024s'",
> -						 pid, uid, loginuid, (char *)data);
> +						 "user pid=%d uid=%u auid=%u subj=%s msg='%.1024s'",
> +						 pid, uid, loginuid, ctx ? ctx : "null", (char *)data);

Do you want those "subj=null" items in the output when SELinux is
disabled, or should there be a different audit_log_format call in the !
ctx case that completely omits "subj="? 

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC][PATCH] collect security labels on user processes generating audit messages
  2006-02-15 13:47         ` Stephen Smalley
@ 2006-02-15 15:49           ` Timothy R. Chavez
  2006-02-15 16:14             ` Linda Knippers
  2006-02-15 16:17             ` Stephen Smalley
  0 siblings, 2 replies; 40+ messages in thread
From: Timothy R. Chavez @ 2006-02-15 15:49 UTC (permalink / raw)
  To: Stephen Smalley
  Cc: James Morris, selinux, Linux Audit Discussion, James Morris

On Wed, 2006-02-15 at 08:47 -0500, Stephen Smalley wrote:
> On Tue, 2006-02-14 at 17:48 -0600, Timothy R. Chavez wrote:
> > diff --git a/include/linux/netlink.h b/include/linux/netlink.h
> > index 6a2ccf7..ccd5905 100644
> > --- a/include/linux/netlink.h
> > +++ b/include/linux/netlink.h
> > @@ -143,6 +143,7 @@ struct netlink_skb_parms
> >  	__u32			dst_group;
> >  	kernel_cap_t		eff_cap;
> >  	__u32			loginuid;	/* Login (audit) uid */
> > +	__u32			secid;		/* SELinux security id */
> >  };
> >  
> >  #define NETLINK_CB(skb)		(*(struct netlink_skb_parms*)&((skb)->cb))
> 
> As a minor nit, does anyone know why '__u32' is used here vs. 'u32'?
> The definition is already wrapped with an #ifdef __KERNEL__.  I see that
> you are being consistent with the existing fields, but then we use just
> 'u32' in the audit code and in the SELinux interfaces and code.  Seems
> like we should be consistent throughout, and I don't see a real reason
> to use __u32 vs. just u32 if it is all kernel code and not included in
> userland (or protected by #ifdef __KERNEL__).
> 

Yeah.. I was wondering 'bout this myself.  I'll just change secid to u32
for the time being.

[..]
> > diff --git a/include/linux/security.h b/include/linux/security.h
> > index b4fe8aa..c6fe5fe 100644
> > --- a/include/linux/security.h
> > +++ b/include/linux/security.h
> > @@ -1169,6 +1174,7 @@ struct security_operations {
> >  			   unsigned long arg5);
> >  	void (*task_reparent_to_init) (struct task_struct * p);
> >  	void (*task_to_inode)(struct task_struct *p, struct inode *inode);
> > +	void (*task_getsecid)(struct task_struct *tsk, __u32 *sid);
> 
> Same issue for the security hook interfaces.

And s/__u32/u32 on all the relevant functions too.

[..]
> 
> > @@ -2457,6 +2468,9 @@ static inline void security_task_reparen
> >  static inline void security_task_to_inode(struct task_struct *p, struct inode *inode)
> >  { }
> >  
> > +static inline void security_task_getsecid(struct task_struct *tsk, __u32 *sid)
> > +{ }
> > +
> 
> Should we set *sid = 0 here as in the dummy function, just to make sure
> it doesn't contain garbage?

Yep.

[..]
> 
> > diff --git a/kernel/audit.c b/kernel/audit.c
> > index d95efd6..4ca77dd 100644
> > --- a/kernel/audit.c
> > +++ b/kernel/audit.c
> > @@ -458,14 +462,20 @@ static int audit_receive_msg(struct sk_b
> >  		err = audit_filter_user(&NETLINK_CB(skb), msg_type);
> >  		if (err == 1) {
> >  			err = 0;
> > +			if (selinux_available()) {
> > +				err = selinux_id_to_ctx(sid, &ctx, &len);
> > +				if (err < 0)
> > +					return err;
> > +			}
> 
> It seems unfortunate to have to wrap each call to a public SELinux
> interface with a selinux_available() check, and the !
> defined(CONFIG_SECURITY_SELINUX) case would actually work without such a
> check since it just sets (ctx, len) to (NULL, 0).  So the
> selinux_available() check is only necessary for the case where SELinux
> is disabled at runtime (selinux=0 or /selinux/disable).  Possibly it
> should be moved within selinux_id_to_ctx() so that callers can just call
> selinux_id_to_ctx() unconditionally?

This makes sense to me.  I'll go ahead and make the change.  I wouldn't
even technically need the function or function call in my patch since
selinux_available() simply returns ss_initialized.

[..]
> 
> >  			ab = audit_log_start(NULL, GFP_KERNEL, msg_type);
> >  			if (ab) {
> >  				audit_log_format(ab,
> > -						 "user pid=%d uid=%u auid=%u msg='%.1024s'",
> > -						 pid, uid, loginuid, (char *)data);
> > +						 "user pid=%d uid=%u auid=%u subj=%s msg='%.1024s'",
> > +						 pid, uid, loginuid, ctx ? ctx : "null", (char *)data);
> 
> Do you want those "subj=null" items in the output when SELinux is
> disabled, or should there be a different audit_log_format call in the !
> ctx case that completely omits "subj="? 
> 

I remember a while back, Steve wanting to reduce / remove the
conditional tokens in records (I think the argument had to do with
performance impact and parsing).  Did I remember that correctly Steve?
Assuming we do want to print the token if ctx == NULL, is there a
standard way of printing NULL in the record?  I should probably make
sure kernel/auditsc.c:audit_log_task_context() is consistent with
whatever we decide.

Thanks.

-tim


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC][PATCH] collect security labels on user processes generating audit messages
  2006-02-15 15:49           ` Timothy R. Chavez
@ 2006-02-15 16:14             ` Linda Knippers
  2006-02-15 16:22               ` Steve Grubb
  2006-02-15 16:17             ` Stephen Smalley
  1 sibling, 1 reply; 40+ messages in thread
From: Linda Knippers @ 2006-02-15 16:14 UTC (permalink / raw)
  To: Timothy R. Chavez
  Cc: Stephen Smalley, Linux Audit Discussion, James Morris, selinux

> [..]
> 
>>> 
>>
>>>> >  			ab = audit_log_start(NULL, GFP_KERNEL, msg_type);
>>>> >  			if (ab) {
>>>> >  				audit_log_format(ab,
>>>> > -						 "user pid=%d uid=%u auid=%u msg='%.1024s'",
>>>> > -						 pid, uid, loginuid, (char *)data);
>>>> > +						 "user pid=%d uid=%u auid=%u subj=%s msg='%.1024s'",
>>>> > +						 pid, uid, loginuid, ctx ? ctx : "null", (char *)data);
>>
>>> 
>>> Do you want those "subj=null" items in the output when SELinux is
>>> disabled, or should there be a different audit_log_format call in the !
>>> ctx case that completely omits "subj="? 
>>> 
> 
> 
> I remember a while back, Steve wanting to reduce / remove the
> conditional tokens in records (I think the argument had to do with
> performance impact and parsing).  Did I remember that correctly Steve?
> Assuming we do want to print the token if ctx == NULL, is there a
> standard way of printing NULL in the record?  I should probably make
> sure kernel/auditsc.c:audit_log_task_context() is consistent with
> whatever we decide.
> 

Amy submitted a patch a while back to eliminate the "name=" field
to avoid "name=(null)" from the audit records if there was no name
but I don't think the patch went anywhere.
https://www.redhat.com/archives/linux-audit/2005-November/msg00093.html

It looks like there's a new case (for tty) where "(none)" is used.

It would be nice to avoid having this in the audit records, especially
in this case where the value might never be set on a particular system.

-- ljk

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC][PATCH] collect security labels on user processes generating audit messages
  2006-02-15 15:49           ` Timothy R. Chavez
  2006-02-15 16:14             ` Linda Knippers
@ 2006-02-15 16:17             ` Stephen Smalley
  2006-02-15 16:41               ` Timothy R. Chavez
  1 sibling, 1 reply; 40+ messages in thread
From: Stephen Smalley @ 2006-02-15 16:17 UTC (permalink / raw)
  To: Timothy R. Chavez
  Cc: James Morris, selinux, Linux Audit Discussion, James Morris

On Wed, 2006-02-15 at 09:49 -0600, Timothy R. Chavez wrote:
> This makes sense to me.  I'll go ahead and make the change.  I wouldn't
> even technically need the function or function call in my patch since
> selinux_available() simply returns ss_initialized.

Well, I think we want to keep that variable private to the SELinux
"module".  In the future, we'll likely add proper namespace prefixes to
all non-static SELinux symbols to avoid polluting the kernel namespace.
 
-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC][PATCH] collect security labels on user processes generating audit messages
  2006-02-15 16:14             ` Linda Knippers
@ 2006-02-15 16:22               ` Steve Grubb
  2006-02-15 16:37                 ` Stephen Smalley
  2006-02-15 17:17                 ` Linda Knippers
  0 siblings, 2 replies; 40+ messages in thread
From: Steve Grubb @ 2006-02-15 16:22 UTC (permalink / raw)
  To: linux-audit; +Cc: Linda Knippers, Timothy R. Chavez, James Morris, selinux

This should be a separate thread since the topic is different.

On Wednesday 15 February 2006 11:14, Linda Knippers wrote:
> Amy submitted a patch a while back to eliminate the "name=" field
> to avoid "name=(null)" from the audit records if there was no name
> but I don't think the patch went anywhere.

Right. I want all audit fields to have name=value. If we have %s in the 
message and pass NULL to it, snprintf is already going to put "(null)" so 
what's wrong with just using this precedent?

> It looks like there's a new case (for tty) where "(none)" is used.

Yes for the same reason.


> It would be nice to avoid having this in the audit records, especially
> in this case where the value might never be set on a particular system.

It creates parsing problems without a value. If I saw "tty="  and that's all, 
I'd think the audit system malfunctioned and file a bugzilla. I don't want 
that.

-Steve

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC][PATCH] collect security labels on user processes generating audit messages
  2006-02-15 16:22               ` Steve Grubb
@ 2006-02-15 16:37                 ` Stephen Smalley
  2006-02-15 16:41                   ` Steve Grubb
  2006-02-15 18:33                   ` Timothy R. Chavez
  2006-02-15 17:17                 ` Linda Knippers
  1 sibling, 2 replies; 40+ messages in thread
From: Stephen Smalley @ 2006-02-15 16:37 UTC (permalink / raw)
  To: Steve Grubb; +Cc: linux-audit, James Morris, selinux

On Wed, 2006-02-15 at 11:22 -0500, Steve Grubb wrote:
> This should be a separate thread since the topic is different.
> 
> On Wednesday 15 February 2006 11:14, Linda Knippers wrote:
> > Amy submitted a patch a while back to eliminate the "name=" field
> > to avoid "name=(null)" from the audit records if there was no name
> > but I don't think the patch went anywhere.
> 
> Right. I want all audit fields to have name=value. If we have %s in the 
> message and pass NULL to it, snprintf is already going to put "(null)" so 
> what's wrong with just using this precedent?

In that case, Tim doesn't need a special check for !ctx in his code at
all.  But see below.

> It creates parsing problems without a value. If I saw "tty="  and that's all, 
> I'd think the audit system malfunctioned and file a bugzilla. I don't want 
> that.

OTOH, if I see (null), I tend to assume a bug in the code.  Isn't it
saner to just omit the name=value pair altogether if the value is NULL?
Otherwise, you are adding extra processing on the generation and parsing
side for no benefit, along with wasting space in the audit message.

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC][PATCH] collect security labels on user processes generating audit messages
  2006-02-15 16:41               ` Timothy R. Chavez
@ 2006-02-15 16:38                 ` Stephen Smalley
  0 siblings, 0 replies; 40+ messages in thread
From: Stephen Smalley @ 2006-02-15 16:38 UTC (permalink / raw)
  To: Timothy R. Chavez
  Cc: James Morris, selinux, Linux Audit Discussion, James Morris

On Wed, 2006-02-15 at 10:41 -0600, Timothy R. Chavez wrote:
> On Wed, 2006-02-15 at 11:17 -0500, Stephen Smalley wrote:
> > On Wed, 2006-02-15 at 09:49 -0600, Timothy R. Chavez wrote:
> > > This makes sense to me.  I'll go ahead and make the change.  I wouldn't
> > > even technically need the function or function call in my patch since
> > > selinux_available() simply returns ss_initialized.
> > 
> > Well, I think we want to keep that variable private to the SELinux
> > "module".  In the future, we'll likely add proper namespace prefixes to
> > all non-static SELinux symbols to avoid polluting the kernel namespace.
> >  
> 
> I think maybe I miscommunicated my intentions.  If I move the check to
> determine whether or not SELinux is enabled into selinux_id_to_ctx(),
> then I can simply use ss_initialized directly rather then calling
> selinux_available(), as I'll be making the check within the SELinux
> "module" (selinux/exports.c).

Ah, ok - no problem then.

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC][PATCH] collect security labels on user processes generating audit messages
  2006-02-15 16:17             ` Stephen Smalley
@ 2006-02-15 16:41               ` Timothy R. Chavez
  2006-02-15 16:38                 ` Stephen Smalley
  0 siblings, 1 reply; 40+ messages in thread
From: Timothy R. Chavez @ 2006-02-15 16:41 UTC (permalink / raw)
  To: Stephen Smalley
  Cc: James Morris, selinux, Linux Audit Discussion, James Morris

On Wed, 2006-02-15 at 11:17 -0500, Stephen Smalley wrote:
> On Wed, 2006-02-15 at 09:49 -0600, Timothy R. Chavez wrote:
> > This makes sense to me.  I'll go ahead and make the change.  I wouldn't
> > even technically need the function or function call in my patch since
> > selinux_available() simply returns ss_initialized.
> 
> Well, I think we want to keep that variable private to the SELinux
> "module".  In the future, we'll likely add proper namespace prefixes to
> all non-static SELinux symbols to avoid polluting the kernel namespace.
>  

I think maybe I miscommunicated my intentions.  If I move the check to
determine whether or not SELinux is enabled into selinux_id_to_ctx(),
then I can simply use ss_initialized directly rather then calling
selinux_available(), as I'll be making the check within the SELinux
"module" (selinux/exports.c).

-tim


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC][PATCH] collect security labels on user processes generating audit messages
  2006-02-15 16:37                 ` Stephen Smalley
@ 2006-02-15 16:41                   ` Steve Grubb
  2006-02-15 16:58                     ` Timothy R. Chavez
  2006-02-15 18:33                   ` Timothy R. Chavez
  1 sibling, 1 reply; 40+ messages in thread
From: Steve Grubb @ 2006-02-15 16:41 UTC (permalink / raw)
  To: Stephen Smalley; +Cc: linux-audit, James Morris, selinux

On Wednesday 15 February 2006 11:37, Stephen Smalley wrote:
> > It creates parsing problems without a value. If I saw "tty="  and that's
> > all, I'd think the audit system malfunctioned and file a bugzilla. I
> > don't want that.
>
> OTOH, if I see (null), I tend to assume a bug in the code.  Isn't it
> saner to just omit the name=value pair altogether if the value is NULL?

No, cause then I have non-normalized records.

> Otherwise, you are adding extra processing on the generation and parsing
> side for no benefit, along with wasting space in the audit message.

There is a benefit...no missing fields means that the record is normalized. 
This is a required step before we create a binary format record. 

There are performance benefits in the kernel as well as user space. The kernel 
doesn't have to have an "if" statement with 2 nearly identical calls to 
audit_log_format or 2 back to back calls to the same function adding a new 
piece of info. 

In userspace, I can parse it faster since I don't have to backtrack and 
re-parse from the last good token to look for the next field after deciding 
one is missing.

-Steve

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC][PATCH] collect security labels on user processes generating audit messages
  2006-02-15 16:41                   ` Steve Grubb
@ 2006-02-15 16:58                     ` Timothy R. Chavez
  0 siblings, 0 replies; 40+ messages in thread
From: Timothy R. Chavez @ 2006-02-15 16:58 UTC (permalink / raw)
  To: Steve Grubb; +Cc: Stephen Smalley, linux-audit, James Morris, selinux

On Wed, 2006-02-15 at 11:41 -0500, Steve Grubb wrote:
> On Wednesday 15 February 2006 11:37, Stephen Smalley wrote:
> > > It creates parsing problems without a value. If I saw "tty="  and that's
> > > all, I'd think the audit system malfunctioned and file a bugzilla. I
> > > don't want that.
> >
> > OTOH, if I see (null), I tend to assume a bug in the code.  Isn't it
> > saner to just omit the name=value pair altogether if the value is NULL?
> 
> No, cause then I have non-normalized records.
> 
> > Otherwise, you are adding extra processing on the generation and parsing
> > side for no benefit, along with wasting space in the audit message.
> 
> There is a benefit...no missing fields means that the record is normalized. 
> This is a required step before we create a binary format record. 
> 
> There are performance benefits in the kernel as well as user space. The kernel 
> doesn't have to have an "if" statement with 2 nearly identical calls to 
> audit_log_format or 2 back to back calls to the same function adding a new 
> piece of info. 
> 
> In userspace, I can parse it faster since I don't have to backtrack and 
> re-parse from the last good token to look for the next field after deciding 
> one is missing.
> 
> -Steve

Steve,

I agree with you.  Also, we could have 'ausearch' interpret a (null)
differently depending on token.  So perhaps an ausearch on a record
where subj=(null), would return subj=disabled.

-tim


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC][PATCH] collect security labels on user processes generating audit messages
  2006-02-15 16:22               ` Steve Grubb
  2006-02-15 16:37                 ` Stephen Smalley
@ 2006-02-15 17:17                 ` Linda Knippers
  2006-02-15 18:14                   ` Steve Grubb
                                     ` (2 more replies)
  1 sibling, 3 replies; 40+ messages in thread
From: Linda Knippers @ 2006-02-15 17:17 UTC (permalink / raw)
  To: Steve Grubb; +Cc: linux-audit, Timothy R. Chavez, James Morris, selinux

Steve Grubb wrote:
> This should be a separate thread since the topic is different.
> 
> On Wednesday 15 February 2006 11:14, Linda Knippers wrote:
> 
>>Amy submitted a patch a while back to eliminate the "name=" field
>>to avoid "name=(null)" from the audit records if there was no name
>>but I don't think the patch went anywhere.
> 
> 
> Right. I want all audit fields to have name=value. If we have %s in the 
> message and pass NULL to it, snprintf is already going to put "(null)" so 
> what's wrong with just using this precedent?

The problem is that "(null)" is a valid file name.

[ljk@cert-e2 ~]$ touch "(null)"
[ljk@cert-e2 ~]$ ls -l "(null)"
-rw-rw-r--  1 ljk ljk 0 Feb 17 11:14 (null)

When I look at audit records generated by those commands I see records
like this:

type=SYSCALL msg=audit(1140192875.311:3789): arch=c000003e syscall=132
success=yes exit=0 a0=7fbffffc51 a1=0 a2=1b6 a3=0 items=1 pid=2116
auid=501 uid=501 gid=501 euid=501 suid=501 fsuid=501 egid=501 sgid=501
fsgid=501 comm="touch" exe="/bin/touch"
type=CWD msg=audit(1140192875.311:3789):  cwd="/home/ljk"
type=PATH msg=audit(1140192875.311:3789): name="(null)" flags=1
inode=6537222 dev=fd:01 mode=0100664 ouid=501 ogid=501 rdev=00:00

How can I tell from the audit records that the file name was "(null)"
vs. having "(null)" manufactured by the audit system?

-- ljk

> 
>>It looks like there's a new case (for tty) where "(none)" is used.
> 
> 
> Yes for the same reason.
> 
> 
> 
>>It would be nice to avoid having this in the audit records, especially
>>in this case where the value might never be set on a particular system.
> 
> 
> It creates parsing problems without a value. If I saw "tty="  and that's all, 
> I'd think the audit system malfunctioned and file a bugzilla. I don't want 
> that.
> 
> -Steve
> 


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC][PATCH] collect security labels on user processes generating audit messages
  2006-02-15 17:17                 ` Linda Knippers
@ 2006-02-15 18:14                   ` Steve Grubb
  2006-02-15 18:20                   ` Steve Grubb
  2006-02-16 19:03                   ` Lamont R. Peterson
  2 siblings, 0 replies; 40+ messages in thread
From: Steve Grubb @ 2006-02-15 18:14 UTC (permalink / raw)
  To: Linda Knippers; +Cc: linux-audit, Timothy R. Chavez, James Morris, selinux

On Wednesday 15 February 2006 12:17, Linda Knippers wrote:
> How can I tell from the audit records that the file name was "(null)"
> vs. having "(null)" manufactured by the audit system?

ls -i "(null)"

and then compare inode values.

-Steve

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC][PATCH] collect security labels on user processes generating audit messages
  2006-02-15 17:17                 ` Linda Knippers
  2006-02-15 18:14                   ` Steve Grubb
@ 2006-02-15 18:20                   ` Steve Grubb
  2006-02-16 14:56                     ` Steve Grubb
  2006-02-16 19:03                   ` Lamont R. Peterson
  2 siblings, 1 reply; 40+ messages in thread
From: Steve Grubb @ 2006-02-15 18:20 UTC (permalink / raw)
  To: Linda Knippers; +Cc: linux-audit, Timothy R. Chavez, James Morris, selinux

On Wednesday 15 February 2006 12:17, Linda Knippers wrote:
> type=PATH msg=audit(1140192875.311:3789): name="(null)" flags=1
> inode=6537222 dev=fd:01 mode=0100664 ouid=501 ogid=501 rdev=00:00

Wait a second...notice the quote marks around (null). When you have a genuine 
(null) they are not there.

type=PATH msg=audit(02/14/2006 08:54:27.096:24) : item=1 name=(null) 
inode=34681 dev=03:06 mode=dir,700 ouid=root ogid=root rdev=00:00 
obj=system_u:object_r:automount_tmp_t:s0


-Steve

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC][PATCH] collect security labels on user processes generating audit messages
  2006-02-15 16:37                 ` Stephen Smalley
  2006-02-15 16:41                   ` Steve Grubb
@ 2006-02-15 18:33                   ` Timothy R. Chavez
  1 sibling, 0 replies; 40+ messages in thread
From: Timothy R. Chavez @ 2006-02-15 18:33 UTC (permalink / raw)
  To: Stephen Smalley; +Cc: Steve Grubb, linux-audit, James Morris, selinux

On Wed, 2006-02-15 at 11:37 -0500, Stephen Smalley wrote:
> On Wed, 2006-02-15 at 11:22 -0500, Steve Grubb wrote:
> > This should be a separate thread since the topic is different.
> > 
> > On Wednesday 15 February 2006 11:14, Linda Knippers wrote:
> > > Amy submitted a patch a while back to eliminate the "name=" field
> > > to avoid "name=(null)" from the audit records if there was no name
> > > but I don't think the patch went anywhere.
> > 
> > Right. I want all audit fields to have name=value. If we have %s in the 
> > message and pass NULL to it, snprintf is already going to put "(null)" so 
> > what's wrong with just using this precedent?
> 
> In that case, Tim doesn't need a special check for !ctx in his code at
> all.

FYI, if I just pass NULL to audit_log_format(), then <NULL> is printed
to the log, not (null).  I just tried this.

-tim


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC][PATCH] collect security labels on user processes generating audit messages
  2006-02-14 23:48       ` Timothy R. Chavez
  2006-02-15 13:47         ` Stephen Smalley
@ 2006-02-15 21:05         ` Darrel Goeddel
  2006-02-17 20:58           ` Timothy R. Chavez
  1 sibling, 1 reply; 40+ messages in thread
From: Darrel Goeddel @ 2006-02-15 21:05 UTC (permalink / raw)
  To: Timothy R. Chavez
  Cc: Stephen Smalley, Linux Audit Discussion, James Morris, selinux

Timothy R. Chavez wrote:
> James & Stephen,
> 
> Thank you for the comments.  While implementing your feedback I came
> across a pretty severe bug.  I was basically obtaining the sid and then
> throwing it away (I was returning it from the function, but not actually
> assigning it to anything).  New patch below.  I still need to test this
> a little more.  Thanks!
> 
> -tim

Should you really be using an lsm interface for getting the sid?  The
patch is currently allowing any security module to put a secid (whose
comment says SELinux security id) into the netlink_skb_params struct.
This generic item is then only used in SELinux specific calls.  It
seems that the getsecid functionality could just fit into an SELinux
specific API just like selinux_id_to_ctx and friends.  That would also
avoid the overhead of lsm and all of the associated code changes.  Of
course this is probably moot if there are other planned uses for
security_task_getsecid().

-- 

Darrel

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC][PATCH] collect security labels on user processes generating audit messages
  2006-02-15 18:20                   ` Steve Grubb
@ 2006-02-16 14:56                     ` Steve Grubb
  2006-02-16 15:29                       ` Stephen Smalley
  0 siblings, 1 reply; 40+ messages in thread
From: Steve Grubb @ 2006-02-16 14:56 UTC (permalink / raw)
  To: linux-audit; +Cc: Linda Knippers, James Morris, selinux

On Wednesday 15 February 2006 13:20, Steve Grubb wrote:
> > type=PATH msg=audit(1140192875.311:3789): name="(null)" flags=1
> > inode=6537222 dev=fd:01 mode=0100664 ouid=501 ogid=501 rdev=00:00
>
> Wait a second...notice the quote marks around (null). When you have a
> genuine (null) they are not there.
>
> type=PATH msg=audit(02/14/2006 08:54:27.096:24) : item=1 name=(null)
> inode=34681 dev=03:06 mode=dir,700 ouid=root ogid=root rdev=00:00
> obj=system_u:object_r:automount_tmp_t:s0

OK, I chased this down to make sure of what is happening. The audit working 
group has a test kernel, lspp.8, that has all the future audit and lspp 
patches in it for testing. (it can be found at 
http://people.redhat.com/sgrubb/files/lspp). There is a patch 
linux-2.6-audit-git.patch, which is not upstream, but should be in the next 
kernel. That changes the code in audit_log_exit of auditsc.c to:

                if (context->names[i].name)
                        audit_log_untrustedstring(ab, context->names[i].name);
                else
                        audit_log_format(ab, "(null)");

The code in audit_log_untrustedstring does this:

        while (*p) {
                if (*p == '"' || *p == '(' || *p < 0x21 || *p > 0x7f) {
                        audit_log_hex(ab, string, strlen(string));
                        return;
                }
                p++;
        }
        audit_log_format(ab, "\"%s\"", string);

This means that a real NULL will never have the double-quote marks around it, 
where a file named \(null\) will always have double-quote marks around it. I 
confirmed this by looking in the audit logs. 

However...ausearch does not make this distinction in its output. I will see 
what I can do to make the necessary adjustments to ausearch so that its more 
obvious. So, I think that puts this issue to bed...

-Steve

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC][PATCH] collect security labels on user processes generating audit messages
  2006-02-16 14:56                     ` Steve Grubb
@ 2006-02-16 15:29                       ` Stephen Smalley
  2006-02-16 15:35                         ` Steve Grubb
  0 siblings, 1 reply; 40+ messages in thread
From: Stephen Smalley @ 2006-02-16 15:29 UTC (permalink / raw)
  To: Steve Grubb; +Cc: linux-audit, selinux, James Morris

On Thu, 2006-02-16 at 09:56 -0500, Steve Grubb wrote:
> OK, I chased this down to make sure of what is happening. The audit working 
> group has a test kernel, lspp.8, that has all the future audit and lspp 
> patches in it for testing. (it can be found at 
> http://people.redhat.com/sgrubb/files/lspp). There is a patch 
> linux-2.6-audit-git.patch, which is not upstream, but should be in the next 
> kernel. That changes the code in audit_log_exit of auditsc.c to:
> 
>                 if (context->names[i].name)
>                         audit_log_untrustedstring(ab, context->names[i].name);
>                 else
>                         audit_log_format(ab, "(null)");
> 
> The code in audit_log_untrustedstring does this:
> 
>         while (*p) {
>                 if (*p == '"' || *p == '(' || *p < 0x21 || *p > 0x7f) {
>                         audit_log_hex(ab, string, strlen(string));
>                         return;
>                 }
>                 p++;
>         }
>         audit_log_format(ab, "\"%s\"", string);
> 
> This means that a real NULL will never have the double-quote marks around it, 
> where a file named \(null\) will always have double-quote marks around it. I 
> confirmed this by looking in the audit logs. 
> 
> However...ausearch does not make this distinction in its output. I will see 
> what I can do to make the necessary adjustments to ausearch so that its more 
> obvious. So, I think that puts this issue to bed...

Except for what other code should do about NULL pointers in output.  If
they defer it to vsnprintf, they will end up with <NULL> in the output.
So should Tim's code be checking for !ctx and outputting (null) there as
well?

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC][PATCH] collect security labels on user processes generating audit messages
  2006-02-16 15:29                       ` Stephen Smalley
@ 2006-02-16 15:35                         ` Steve Grubb
  2006-02-16 16:27                           ` Timothy R. Chavez
  0 siblings, 1 reply; 40+ messages in thread
From: Steve Grubb @ 2006-02-16 15:35 UTC (permalink / raw)
  To: Stephen Smalley; +Cc: linux-audit, selinux, James Morris

On Thursday 16 February 2006 10:29, Stephen Smalley wrote:
> Except for what other code should do about NULL pointers in output.  If
> they defer it to vsnprintf, they will end up with <NULL> in the output.
> So should Tim's code be checking for !ctx and outputting (null) there as
> well?

Yes, I think that would be consistent will all the other audit code.

-Steve


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC][PATCH] collect security labels on user processes generating audit messages
  2006-02-16 15:35                         ` Steve Grubb
@ 2006-02-16 16:27                           ` Timothy R. Chavez
  0 siblings, 0 replies; 40+ messages in thread
From: Timothy R. Chavez @ 2006-02-16 16:27 UTC (permalink / raw)
  To: Steve Grubb; +Cc: Stephen Smalley, linux-audit, selinux, James Morris

On Thu, 2006-02-16 at 10:35 -0500, Steve Grubb wrote:
> On Thursday 16 February 2006 10:29, Stephen Smalley wrote:
> > Except for what other code should do about NULL pointers in output.  If
> > they defer it to vsnprintf, they will end up with <NULL> in the output.
> > So should Tim's code be checking for !ctx and outputting (null) there as
> > well?
> 
> Yes, I think that would be consistent will all the other audit code.
> 
> -Steve
> 

Yes.  I've already made this change.  I just saw that Darren's recent
patch submission included the API work I just did (that he suggested
yesterday)... in the process of testing now.

Thanks.

-tim


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC][PATCH] collect security labels on user processes generating audit messages
  2006-02-15 17:17                 ` Linda Knippers
  2006-02-15 18:14                   ` Steve Grubb
  2006-02-15 18:20                   ` Steve Grubb
@ 2006-02-16 19:03                   ` Lamont R. Peterson
  2006-02-16 20:44                     ` Timothy R. Chavez
  2 siblings, 1 reply; 40+ messages in thread
From: Lamont R. Peterson @ 2006-02-16 19:03 UTC (permalink / raw)
  To: SELinux

[-- Attachment #1: Type: text/plain, Size: 1800 bytes --]

On Wednesday 15 February 2006 10:17am, Linda Knippers wrote:
> Steve Grubb wrote:
> > This should be a separate thread since the topic is different.
> >
> > On Wednesday 15 February 2006 11:14, Linda Knippers wrote:
> >>Amy submitted a patch a while back to eliminate the "name=" field
> >>to avoid "name=(null)" from the audit records if there was no name
> >>but I don't think the patch went anywhere.
> >
> > Right. I want all audit fields to have name=value. If we have %s in the
> > message and pass NULL to it, snprintf is already going to put "(null)" so
> > what's wrong with just using this precedent?
>
> The problem is that "(null)" is a valid file name.
>
> [ljk@cert-e2 ~]$ touch "(null)"
> [ljk@cert-e2 ~]$ ls -l "(null)"
> -rw-rw-r--  1 ljk ljk 0 Feb 17 11:14 (null)
>
> When I look at audit records generated by those commands I see records
> like this:
>
> type=SYSCALL msg=audit(1140192875.311:3789): arch=c000003e syscall=132
> success=yes exit=0 a0=7fbffffc51 a1=0 a2=1b6 a3=0 items=1 pid=2116
> auid=501 uid=501 gid=501 euid=501 suid=501 fsuid=501 egid=501 sgid=501
> fsgid=501 comm="touch" exe="/bin/touch"
> type=CWD msg=audit(1140192875.311:3789):  cwd="/home/ljk"
> type=PATH msg=audit(1140192875.311:3789): name="(null)" flags=1
> inode=6537222 dev=fd:01 mode=0100664 ouid=501 ogid=501 rdev=00:00
>
> How can I tell from the audit records that the file name was "(null)"
> vs. having "(null)" manufactured by the audit system?

How about:

type=PATH msg=audit(1140192875.311:3789): name=NULL flags=1

in cases where it truly is NULL?  The double-quotes "" are used to quote 
file-names and without them, we have some kind of meta-value, instead.

[snip]
-- 
Lamont R. Peterson <lrp@xmission.com>
[ http://www.xmission.com/~lrp/ ]

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC][PATCH] collect security labels on user processes generating audit messages
  2006-02-16 19:03                   ` Lamont R. Peterson
@ 2006-02-16 20:44                     ` Timothy R. Chavez
  0 siblings, 0 replies; 40+ messages in thread
From: Timothy R. Chavez @ 2006-02-16 20:44 UTC (permalink / raw)
  To: Lamont R. Peterson; +Cc: Linux Audit Discussion, SELinux

On Thu, 2006-02-16 at 12:03 -0700, Lamont R. Peterson wrote:
> On Wednesday 15 February 2006 10:17am, Linda Knippers wrote:
> > Steve Grubb wrote:
> > > This should be a separate thread since the topic is different.
> > >
> > > On Wednesday 15 February 2006 11:14, Linda Knippers wrote:
> > >>Amy submitted a patch a while back to eliminate the "name=" field
> > >>to avoid "name=(null)" from the audit records if there was no name
> > >>but I don't think the patch went anywhere.
> > >
> > > Right. I want all audit fields to have name=value. If we have %s in the
> > > message and pass NULL to it, snprintf is already going to put "(null)" so
> > > what's wrong with just using this precedent?
> >
> > The problem is that "(null)" is a valid file name.
> >
> > [ljk@cert-e2 ~]$ touch "(null)"
> > [ljk@cert-e2 ~]$ ls -l "(null)"
> > -rw-rw-r--  1 ljk ljk 0 Feb 17 11:14 (null)
> >
> > When I look at audit records generated by those commands I see records
> > like this:
> >
> > type=SYSCALL msg=audit(1140192875.311:3789): arch=c000003e syscall=132
> > success=yes exit=0 a0=7fbffffc51 a1=0 a2=1b6 a3=0 items=1 pid=2116
> > auid=501 uid=501 gid=501 euid=501 suid=501 fsuid=501 egid=501 sgid=501
> > fsgid=501 comm="touch" exe="/bin/touch"
> > type=CWD msg=audit(1140192875.311:3789):  cwd="/home/ljk"
> > type=PATH msg=audit(1140192875.311:3789): name="(null)" flags=1
> > inode=6537222 dev=fd:01 mode=0100664 ouid=501 ogid=501 rdev=00:00
> >
> > How can I tell from the audit records that the file name was "(null)"
> > vs. having "(null)" manufactured by the audit system?
> 
> How about:
> 
> type=PATH msg=audit(1140192875.311:3789): name=NULL flags=1
> 
> in cases where it truly is NULL?  The double-quotes "" are used to quote 
> file-names and without them, we have some kind of meta-value, instead.
> 
> [snip]

The difference is too subtle.  I imagine that will get confusing.  What
we use to represent a NULL value isn't as important is how we
distinguish it.  For instance, simply encoding "NULL" the filename will
make the distinction between 'name=NULL' and
name="NULL" (name="78857676") , clearer. 

If we "strongly suggest" the admin use ausearch to read the log, then we
could let ausearch make the distinction between quoted and unquoted
NULL's clearer, rather than the kernel.

-tim


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC][PATCH] collect security labels on user processes generating audit messages
  2006-02-15 21:05         ` Darrel Goeddel
@ 2006-02-17 20:58           ` Timothy R. Chavez
  2006-02-22 14:21             ` Stephen Smalley
  2006-02-22 14:26             ` Stephen Smalley
  0 siblings, 2 replies; 40+ messages in thread
From: Timothy R. Chavez @ 2006-02-17 20:58 UTC (permalink / raw)
  To: Darrel Goeddel
  Cc: Stephen Smalley, Linux Audit Discussion, James Morris, selinux

On Wed, 2006-02-15 at 15:05 -0600, Darrel Goeddel wrote:
<snip>
> 
> Should you really be using an lsm interface for getting the sid?  The
> patch is currently allowing any security module to put a secid (whose
> comment says SELinux security id) into the netlink_skb_params struct.
> This generic item is then only used in SELinux specific calls.  It
> seems that the getsecid functionality could just fit into an SELinux
> specific API just like selinux_id_to_ctx and friends.  That would also
> avoid the overhead of lsm and all of the associated code changes.  Of
> course this is probably moot if there are other planned uses for
> security_task_getsecid().
> 

Thanks Darrel!  New patch attached... so... assuming this is good... how
are we going to do this API merger :] ?

diff --git a/include/linux/netlink.h b/include/linux/netlink.h
index 6a2ccf7..a2538b4 100644
--- a/include/linux/netlink.h
+++ b/include/linux/netlink.h
@@ -143,6 +143,7 @@ struct netlink_skb_parms
 	__u32			dst_group;
 	kernel_cap_t		eff_cap;
 	__u32			loginuid;	/* Login (audit) uid */
+	u32			secid;		/* SELinux security id */
 };
 
 #define NETLINK_CB(skb)		(*(struct netlink_skb_parms*)&((skb)->cb))
diff --git a/include/linux/selinux.h b/include/linux/selinux.h
new file mode 100644
index 0000000..4d67711
--- /dev/null
+++ b/include/linux/selinux.h
@@ -0,0 +1,55 @@
+/*
+ * SELinux services exported to the rest of the kernel.
+ *
+ * Author: James Morris <jmorris@redhat.com>
+ *	   Timothy R. Chavez <tinytim@us.ibm.com>
+ *
+ * Copyright (C) 2005 Red Hat, Inc., James Morris <jmorris@redhat.com>
+ * Copyright (C) IBM Corporation, 2006
+ *
+ * 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_id_to_ctx - 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_id_to_ctx(u32 ctxid, char **ctx, u32 *ctxlen);
+
+/**
+ *     selinux_task_getsecid - return the SID of task
+ *	@tsk: the task whose SID will be returned
+ *
+ * 	Returns 0 if SELinux is disabled, otherwise the SID is returned.
+ */
+int selinux_task_getsecid(struct task_struct *tsk);
+
+#else
+
+static inline int selinux_id_to_ctx(u32 ctxid, char **ctx, u32 *ctxlen)
+{
+	*ctx = NULL;
+	*ctxlen = 0;
+	return 0;
+}
+
+static inline u32 selinux_task_getsecid(struct task_struct *tsk)
+{
+	return 0;
+}
+
+#endif /* CONFIG_SECURITY_SELINUX */
+
+#endif /* _LINUX_SELINUX_H */
diff --git a/kernel/audit.c b/kernel/audit.c
index d95efd6..334340d 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -50,6 +50,7 @@
 #include <linux/kthread.h>
 
 #include <linux/audit.h>
+#include <linux/selinux.h>
 
 #include <net/sock.h>
 #include <linux/skbuff.h>
@@ -383,7 +384,7 @@ static int audit_netlink_ok(kernel_cap_t
 
 static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 {
-	u32			uid, pid, seq;
+	u32			uid, pid, sid, seq;
 	void			*data;
 	struct audit_status	*status_get, status_set;
 	int			err;
@@ -391,6 +392,8 @@ static int audit_receive_msg(struct sk_b
 	u16			msg_type = nlh->nlmsg_type;
 	uid_t			loginuid; /* loginuid of sender */
 	struct audit_sig_info   sig_data;
+	char *			ctx;
+	u32			len;
 
 	err = audit_netlink_ok(NETLINK_CB(skb).eff_cap, msg_type);
 	if (err)
@@ -409,6 +412,7 @@ static int audit_receive_msg(struct sk_b
 	pid  = NETLINK_CREDS(skb)->pid;
 	uid  = NETLINK_CREDS(skb)->uid;
 	loginuid = NETLINK_CB(skb).loginuid;
+	sid = NETLINK_CB(skb).secid;
 	seq  = nlh->nlmsg_seq;
 	data = NLMSG_DATA(nlh);
 
@@ -457,15 +461,18 @@ static int audit_receive_msg(struct sk_b
 
 		err = audit_filter_user(&NETLINK_CB(skb), msg_type);
 		if (err == 1) {
-			err = 0;
+			err = selinux_id_to_ctx(sid, &ctx, &len);
+			if (err < 0)
+				return err;
 			ab = audit_log_start(NULL, GFP_KERNEL, msg_type);
 			if (ab) {
 				audit_log_format(ab,
-						 "user pid=%d uid=%u auid=%u msg='%.1024s'",
-						 pid, uid, loginuid, (char *)data);
+						 "user pid=%d uid=%u auid=%u subj=%s msg='%.1024s'",
+						 pid, uid, loginuid, ctx ? ctx : "(null)", (char *)data);
 				audit_set_pid(ab, pid);
 				audit_log_end(ab);
 			}
+			kfree(ctx);
 		}
 		break;
 	case AUDIT_ADD:
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 96020d7..f6a47a4 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -55,6 +55,7 @@
 #include <linux/mm.h>
 #include <linux/types.h>
 #include <linux/audit.h>
+#include <linux/selinux.h>
 
 #include <net/sock.h>
 #include <net/scm.h>
@@ -1120,6 +1121,7 @@ static int netlink_sendmsg(struct kiocb 
 	NETLINK_CB(skb).dst_pid = dst_pid;
 	NETLINK_CB(skb).dst_group = dst_group;
 	NETLINK_CB(skb).loginuid = audit_get_loginuid(current->audit_context);
+	NETLINK_CB(skb).secid = selinux_task_getsecid(current);
 	memcpy(NETLINK_CREDS(skb), &siocb->scm->creds, sizeof(struct ucred));
 
 	/* What can I do? Netlink is asynchronous, so that
diff --git a/security/selinux/Makefile b/security/selinux/Makefile
index b038cd0..3e3d4eb 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
+selinux-y := avc.o hooks.o selinuxfs.o netlink.o nlmsgtab.o exports.o
 
 selinux-$(CONFIG_SECURITY_NETWORK) += netif.o
 
diff --git a/security/selinux/exports.c b/security/selinux/exports.c
new file mode 100644
index 0000000..29755ba
--- /dev/null
+++ b/security/selinux/exports.c
@@ -0,0 +1,47 @@
+/*
+ * SELinux services exported to the rest of the kernel.
+ *
+ * Author: James Morris <jmorris@redhat.com>
+ * 	   Timothy R. Chavez <tinytim@us.ibm.com>
+ *
+ * Copyright (C) 2005 Red Hat, Inc., James Morris <jmorris@redhat.com>
+ * Copyright (C) IBM Corporation, 2006
+ *
+ * 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"
+
+extern int ss_initialized;
+
+int selinux_id_to_ctx(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_task_getsecid(struct task_struct *tsk)
+{
+	u32 sid = 0;
+
+	if (ss_initialized)
+		sid = ((struct task_security_struct *)tsk->security)->sid;
+	
+	return sid;
+}
+
+EXPORT_SYMBOL_GPL(selinux_id_to_ctx);
+EXPORT_SYMBOL_GPL(selinux_task_getsecid);



--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* Re: [RFC][PATCH] collect security labels on user processes generating audit messages
  2006-02-17 20:58           ` Timothy R. Chavez
@ 2006-02-22 14:21             ` Stephen Smalley
  2006-02-22 17:14               ` Timothy R. Chavez
  2006-02-22 14:26             ` Stephen Smalley
  1 sibling, 1 reply; 40+ messages in thread
From: Stephen Smalley @ 2006-02-22 14:21 UTC (permalink / raw)
  To: Timothy R. Chavez
  Cc: Darrel Goeddel, Linux Audit Discussion, James Morris, selinux

On Fri, 2006-02-17 at 14:58 -0600, Timothy R. Chavez wrote:
> Thanks Darrel!  New patch attached... so... assuming this is good... how
> are we going to do this API merger :] ?

> +/**
> + *     selinux_task_getsecid - return the SID of task
> + *	@tsk: the task whose SID will be returned
> + *
> + * 	Returns 0 if SELinux is disabled, otherwise the SID is returned.
> + */
> +int selinux_task_getsecid(struct task_struct *tsk);

Ryan noticed that you didn't update this to return u32 yet, unlike the
#else case.

> +u32 selinux_task_getsecid(struct task_struct *tsk)
> +{
> +	u32 sid = 0;
> +
> +	if (ss_initialized)
> +		sid = ((struct task_security_struct *)tsk->security)->sid;
> +	
> +	return sid;
> +}

You don't strictly need to check ss_initialized in this function; all
tasks are assigned the kernel SID until policy is loaded, so you can
always access the SID.  As a matter of style, I'd prefer an explicit
task_security_struct* local variable with separate assignment, i.e.
	struct task_security_struct *tsec = tsk->security;
	sid = tsec->sid;

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC][PATCH] collect security labels on user processes generating audit messages
  2006-02-17 20:58           ` Timothy R. Chavez
  2006-02-22 14:21             ` Stephen Smalley
@ 2006-02-22 14:26             ` Stephen Smalley
  2006-02-22 17:13               ` Timothy R. Chavez
  1 sibling, 1 reply; 40+ messages in thread
From: Stephen Smalley @ 2006-02-22 14:26 UTC (permalink / raw)
  To: Timothy R. Chavez
  Cc: Darrel Goeddel, Linux Audit Discussion, James Morris, selinux

On Fri, 2006-02-17 at 14:58 -0600, Timothy R. Chavez wrote:
> Thanks Darrel!  New patch attached... so... assuming this is good... how
> are we going to do this API merger :] ?

Looks like your selinux_task_getsecid could be dropped in favor of
Darrel's selinux_task_ctxid.

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC][PATCH] collect security labels on user processes generating audit messages
  2006-02-22 14:26             ` Stephen Smalley
@ 2006-02-22 17:13               ` Timothy R. Chavez
  0 siblings, 0 replies; 40+ messages in thread
From: Timothy R. Chavez @ 2006-02-22 17:13 UTC (permalink / raw)
  To: Stephen Smalley
  Cc: Darrel Goeddel, Linux Audit Discussion, James Morris, selinux

On Wed, 2006-02-22 at 09:26 -0500, Stephen Smalley wrote:
> On Fri, 2006-02-17 at 14:58 -0600, Timothy R. Chavez wrote:
> > Thanks Darrel!  New patch attached... so... assuming this is good... how
> > are we going to do this API merger :] ?
> 
> Looks like your selinux_task_getsecid could be dropped in favor of
> Darrel's selinux_task_ctxid.
> 

Yeah.  I'll post an updated patch against his work when it goes
upstream.  That seems to be the easiest solution.  Thanks.

-tim


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC][PATCH] collect security labels on user processes generating audit messages
  2006-02-22 14:21             ` Stephen Smalley
@ 2006-02-22 17:14               ` Timothy R. Chavez
  0 siblings, 0 replies; 40+ messages in thread
From: Timothy R. Chavez @ 2006-02-22 17:14 UTC (permalink / raw)
  To: Stephen Smalley
  Cc: Darrel Goeddel, Linux Audit Discussion, James Morris, selinux

On Wed, 2006-02-22 at 09:21 -0500, Stephen Smalley wrote:
<snip>
> 
> > +u32 selinux_task_getsecid(struct task_struct *tsk)
> > +{
> > +	u32 sid = 0;
> > +
> > +	if (ss_initialized)
> > +		sid = ((struct task_security_struct *)tsk->security)->sid;
> > +	
> > +	return sid;
> > +}
> 
> You don't strictly need to check ss_initialized in this function; all
> tasks are assigned the kernel SID until policy is loaded, so you can
> always access the SID.  As a matter of style, I'd prefer an explicit
> task_security_struct* local variable with separate assignment, i.e.
> 	struct task_security_struct *tsec = tsk->security;
> 	sid = tsec->sid;
> 

Ok.  That change will appear in the patch I post against Darrel's work
once it goes upstream.

-tim


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 40+ messages in thread

end of thread, other threads:[~2006-02-22 17:14 UTC | newest]

Thread overview: 40+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-02-09  1:32 [RFC][PATCH] collect security labels on user processes generating audit messages Timothy R. Chavez
2006-02-09 14:58 ` James Morris
2006-02-09 15:10   ` Darrel Goeddel
2006-02-09 15:15   ` James Morris
2006-02-09 17:43     ` Stephen Smalley
2006-02-09 16:13   ` Timothy R. Chavez
2006-02-09 17:03     ` James Morris
2006-02-09 17:39       ` Timothy R. Chavez
2006-02-09 17:29     ` Stephen Smalley
2006-02-09 18:13     ` Stephen Smalley
2006-02-10  0:14   ` Timothy R. Chavez
2006-02-10  4:00     ` James Morris
2006-02-13 19:12     ` Stephen Smalley
2006-02-14 23:48       ` Timothy R. Chavez
2006-02-15 13:47         ` Stephen Smalley
2006-02-15 15:49           ` Timothy R. Chavez
2006-02-15 16:14             ` Linda Knippers
2006-02-15 16:22               ` Steve Grubb
2006-02-15 16:37                 ` Stephen Smalley
2006-02-15 16:41                   ` Steve Grubb
2006-02-15 16:58                     ` Timothy R. Chavez
2006-02-15 18:33                   ` Timothy R. Chavez
2006-02-15 17:17                 ` Linda Knippers
2006-02-15 18:14                   ` Steve Grubb
2006-02-15 18:20                   ` Steve Grubb
2006-02-16 14:56                     ` Steve Grubb
2006-02-16 15:29                       ` Stephen Smalley
2006-02-16 15:35                         ` Steve Grubb
2006-02-16 16:27                           ` Timothy R. Chavez
2006-02-16 19:03                   ` Lamont R. Peterson
2006-02-16 20:44                     ` Timothy R. Chavez
2006-02-15 16:17             ` Stephen Smalley
2006-02-15 16:41               ` Timothy R. Chavez
2006-02-15 16:38                 ` Stephen Smalley
2006-02-15 21:05         ` Darrel Goeddel
2006-02-17 20:58           ` Timothy R. Chavez
2006-02-22 14:21             ` Stephen Smalley
2006-02-22 17:14               ` Timothy R. Chavez
2006-02-22 14:26             ` Stephen Smalley
2006-02-22 17:13               ` Timothy R. Chavez

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.