[PATCH] Add audit uid to netlink credentials

[PATCH] Add audit uid to netlink credentials

[Serge E. Hallyn]

Most audit control messages are sent over netlink.  In order to properly
log the identity of the sender of audit control messages, we would like
to add the loginuid to the netlink_creds structure, as per the attached
patch.

thanks,
-serge

Signed-off-by: Serge Hallyn <serue@us.ibm.com>
Index: linux-2.6.10/include/linux/audit.h
===================================================================
--- linux-2.6.10.orig/include/linux/audit.h	2005-01-27 10:46:57.887036520 -0600
+++ linux-2.6.10/include/linux/audit.h	2005-01-27 10:51:37.408542792 -0600
@@ -145,7 +145,7 @@ extern void audit_inode(const char *name
 
 				/* Private API (for audit.c only) */
 extern int  audit_receive_filter(int type, int pid, int uid, int seq,
-				 void *data);
+				 void *data, uid_t loginuid);
 extern void audit_get_stamp(struct audit_context *ctx,
 			    struct timespec *t, int *serial);
 extern int  audit_set_loginuid(struct audit_context *ctx, uid_t loginuid);
@@ -179,10 +179,10 @@ extern void		    audit_log_d_path(struct
 					     const char *prefix,
 					     struct dentry *dentry,
 					     struct vfsmount *vfsmnt);
-extern int		    audit_set_rate_limit(int limit);
-extern int		    audit_set_backlog_limit(int limit);
-extern int		    audit_set_enabled(int state);
-extern int		    audit_set_failure(int state);
+extern int		    audit_set_rate_limit(int limit, uid_t loginuid);
+extern int		    audit_set_backlog_limit(int limit, uid_t loginuid);
+extern int		    audit_set_enabled(int state, uid_t loginuid);
+extern int		    audit_set_failure(int state, uid_t loginuid);
 
 				/* Private API (for auditsc.c only) */
 extern void		    audit_send_reply(int pid, int seq, int type,
Index: linux-2.6.10/include/linux/netlink.h
===================================================================
--- linux-2.6.10.orig/include/linux/netlink.h	2005-01-27 10:46:57.888036368 -0600
+++ linux-2.6.10/include/linux/netlink.h	2005-01-27 10:51:37.409542640 -0600
@@ -110,6 +110,7 @@ struct netlink_skb_parms
 	__u32			dst_pid;
 	__u32			dst_groups;
 	kernel_cap_t		eff_cap;
+	__u32			loginuid;	/* Login (audit) uid */
 };
 
 #define NETLINK_CB(skb)		(*(struct netlink_skb_parms*)&((skb)->cb))
Index: linux-2.6.10/kernel/audit.c
===================================================================
--- linux-2.6.10.orig/kernel/audit.c	2005-01-27 10:46:57.888036368 -0600
+++ linux-2.6.10/kernel/audit.c	2005-01-27 10:52:28.753737136 -0600
@@ -236,36 +236,36 @@ void audit_log_lost(const char *message)
 
 }
 
-int audit_set_rate_limit(int limit)
+int audit_set_rate_limit(int limit, uid_t loginuid)
 {
 	int old		 = audit_rate_limit;
 	audit_rate_limit = limit;
-	audit_log(current->audit_context, "audit_rate_limit=%d old=%d",
-		  audit_rate_limit, old);
+	audit_log(NULL, "audit_rate_limit=%d old=%d by loginuid %u",
+			audit_rate_limit, old, loginuid);
 	return old;
 }
 
-int audit_set_backlog_limit(int limit)
+int audit_set_backlog_limit(int limit, uid_t loginuid)
 {
 	int old		 = audit_backlog_limit;
 	audit_backlog_limit = limit;
-	audit_log(current->audit_context, "audit_backlog_limit=%d old=%d",
-		  audit_backlog_limit, old);
+	audit_log(NULL, "audit_backlog_limit=%d old=%d by loginuid %u",
+			audit_backlog_limit, old, loginuid);
 	return old;
 }
 
-int audit_set_enabled(int state)
+int audit_set_enabled(int state, uid_t loginuid)
 {
 	int old		 = audit_enabled;
 	if (state != 0 && state != 1)
 		return -EINVAL;
 	audit_enabled = state;
-	audit_log(current->audit_context, "audit_enabled=%d old=%d",
-		  audit_enabled, old);
+	audit_log(NULL, "audit_enabled=%d old=%d by loginuid %u",
+		  audit_enabled, old, loginuid);
 	return old;
 }
 
-int audit_set_failure(int state)
+int audit_set_failure(int state, uid_t loginuid)
 {
 	int old		 = audit_failure;
 	if (state != AUDIT_FAIL_SILENT
@@ -273,8 +273,8 @@ int audit_set_failure(int state)
 	    && state != AUDIT_FAIL_PANIC)
 		return -EINVAL;
 	audit_failure = state;
-	audit_log(current->audit_context, "audit_failure=%d old=%d",
-		  audit_failure, old);
+	audit_log(NULL, "audit_failure=%d old=%d by loginuid %u",
+		  audit_failure, old, loginuid);
 	return old;
 }
 
@@ -341,6 +341,7 @@ static int audit_receive_msg(struct sk_b
 	int			err;
 	struct audit_buffer	*ab;
 	u16			msg_type = nlh->nlmsg_type;
+	uid_t			loginuid; /* loginuid of sender */
 
 	err = audit_netlink_ok(NETLINK_CB(skb).eff_cap, msg_type);
 	if (err)
@@ -348,6 +349,7 @@ static int audit_receive_msg(struct sk_b
 
 	pid  = NETLINK_CREDS(skb)->pid;
 	uid  = NETLINK_CREDS(skb)->uid;
+	loginuid = NETLINK_CB(skb).loginuid;
 	seq  = nlh->nlmsg_seq;
 	data = NLMSG_DATA(nlh);
 
@@ -368,31 +370,33 @@ static int audit_receive_msg(struct sk_b
 			return -EINVAL;
 		status_get   = (struct audit_status *)data;
 		if (status_get->mask & AUDIT_STATUS_ENABLED) {
-			err = audit_set_enabled(status_get->enabled);
+			err = audit_set_enabled(status_get->enabled, loginuid);
 			if (err < 0) return err;
 		}
 		if (status_get->mask & AUDIT_STATUS_FAILURE) {
-			err = audit_set_failure(status_get->failure);
+			err = audit_set_failure(status_get->failure, loginuid);
 			if (err < 0) return err;
 		}
 		if (status_get->mask & AUDIT_STATUS_PID) {
 			int old   = audit_pid;
 			audit_pid = status_get->pid;
-			audit_log(current->audit_context,
-				  "audit_pid=%d old=%d", audit_pid, old);
+			audit_log(NULL, "audit_pid=%d old=%d by loginuid %u",
+				  audit_pid, old, loginuid);
 		}
 		if (status_get->mask & AUDIT_STATUS_RATE_LIMIT)
-			audit_set_rate_limit(status_get->rate_limit);
+			audit_set_rate_limit(status_get->rate_limit, loginuid);
 		if (status_get->mask & AUDIT_STATUS_BACKLOG_LIMIT)
-			audit_set_backlog_limit(status_get->backlog_limit);
+			audit_set_backlog_limit(status_get->backlog_limit,
+							loginuid);
 		break;
 	case AUDIT_USER:
 		ab = audit_log_start(NULL);
 		if (!ab)
 			break;	/* audit_panic has been called */
 		audit_log_format(ab,
-				 "user pid=%d uid=%d length=%d msg='%.1024s'",
-				 pid, uid,
+				 "user pid=%d uid=%d loginuid=%u length=%d"
+				 " msg='%.1024s'",
+				 pid, uid, loginuid,
 				 (int)(nlh->nlmsg_len
 				       - ((char *)data - (char *)nlh)),
 				 (char *)data);
@@ -408,7 +412,7 @@ static int audit_receive_msg(struct sk_b
 	case AUDIT_LIST:
 #ifdef CONFIG_AUDITSYSCALL
 		err = audit_receive_filter(nlh->nlmsg_type, pid, uid, seq,
-					   data);
+					   data, loginuid);
 #else
 		err = -EOPNOTSUPP;
 #endif
Index: linux-2.6.10/kernel/auditsc.c
===================================================================
--- linux-2.6.10.orig/kernel/auditsc.c	2005-01-27 10:46:57.890036064 -0600
+++ linux-2.6.10/kernel/auditsc.c	2005-01-27 10:52:53.776933032 -0600
@@ -228,7 +228,8 @@ static int audit_copy_rule(struct audit_
 	return 0;
 }
 
-int audit_receive_filter(int type, int pid, int uid, int seq, void *data)
+int audit_receive_filter(int type, int pid, int uid, int seq, void *data,
+							uid_t loginuid)
 {
 	u32		   flags;
 	struct audit_entry *entry;
@@ -263,6 +264,7 @@ int audit_receive_filter(int type, int p
 			err = audit_add_rule(entry, &audit_entlist);
 		if (!err && (flags & AUDIT_AT_EXIT))
 			err = audit_add_rule(entry, &audit_extlist);
+		audit_log(NULL, "loginuid %u added an audit rule\n", loginuid);
 		break;
 	case AUDIT_DEL:
 		flags =((struct audit_rule *)data)->flags;
@@ -272,6 +274,8 @@ int audit_receive_filter(int type, int p
 			err = audit_del_rule(data, &audit_entlist);
 		if (!err && (flags & AUDIT_AT_EXIT))
 			err = audit_del_rule(data, &audit_extlist);
+		audit_log(NULL, "loginuid %u removed an audit rule\n",
+							loginuid);
 		break;
 	default:
 		return -EINVAL;
Index: linux-2.6.10/net/netlink/af_netlink.c
===================================================================
--- linux-2.6.10.orig/net/netlink/af_netlink.c	2005-01-27 10:46:57.891035912 -0600
+++ linux-2.6.10/net/netlink/af_netlink.c	2005-01-27 10:51:37.411542336 -0600
@@ -928,6 +928,7 @@ static int netlink_sendmsg(struct kiocb 
 	NETLINK_CB(skb).groups	= nlk->groups;
 	NETLINK_CB(skb).dst_pid = dst_pid;
 	NETLINK_CB(skb).dst_groups = dst_groups;
+	NETLINK_CB(skb).loginuid = audit_get_loginuid(current->audit_context);
 	memcpy(NETLINK_CREDS(skb), &siocb->scm->creds, sizeof(struct ucred));
 
 	/* What can I do? Netlink is asynchronous, so that

Re: [PATCH] Add audit uid to netlink credentials

[Patrick McHardy]

On Fri, 4 Feb 2005, Serge E. Hallyn wrote:
> Most audit control messages are sent over netlink.  In order to properly
> log the identity of the sender of audit control messages, we would like
> to add the loginuid to the netlink_creds structure, as per the attached
> patch.

Reception of netlink messages in the kernel happens in the context
of the sending process, so you can simply call
audit_get_loginuid(current->audit_context) in audit_receive_msg().

Regards
Patrick

Re: [PATCH] Add audit uid to netlink credentials

[Stephen Smalley]

On Tue, 2005-02-08 at 01:04, Patrick McHardy wrote:
> Reception of netlink messages in the kernel happens in the context
> of the sending process, so you can simply call
> audit_get_loginuid(current->audit_context) in audit_receive_msg().

Then why does netlink_sendmsg() need to save the effective capability
set of the sender in the control buffer (via security_netlink_send) for
later checking by other receive functions in the kernel (via
security_netlink_recv)?  What prevents audit_receive() or other similar
receive functions in the kernel from processing messages sent by
multiple senders?

-- 
Stephen Smalley <sds@epoch.ncsc.mil>
National Security Agency

Re: [PATCH] Add audit uid to netlink credentials

[Patrick McHardy]

Stephen Smalley wrote:

>On Tue, 2005-02-08 at 01:04, Patrick McHardy wrote:
>  
>
>>Reception of netlink messages in the kernel happens in the context
>>of the sending process, so you can simply call
>>audit_get_loginuid(current->audit_context) in audit_receive_msg().
>>    
>>
>
>Then why does netlink_sendmsg() need to save the effective capability
>set of the sender in the control buffer (via security_netlink_send) for
>later checking by other receive functions in the kernel (via
>security_netlink_recv)?
>
It looks like it doesn't need to, I guess it was copied from 
netlink_sendmsg.
netlink transmission to userspace is asynchronous, some values need to be
saved, but userspace->kernel transmission is synchronous.

>What prevents audit_receive() or other similar
>receive functions in the kernel from processing messages sent by
>multiple senders?
>
Multiple messages from multiple senders are handled by multiple calls to
the input function. Check netlink_kernel_create() and netlink_data_ready().

Regards
Patrick

Re: [PATCH] Add audit uid to netlink credentials

[David Woodhouse]

On Fri, 2005-02-04 at 10:58 -0600, Serge E. Hallyn wrote:
> Most audit control messages are sent over netlink.  In order to properly
> log the identity of the sender of audit control messages, we would like
> to add the loginuid to the netlink_creds structure, as per the attached
> patch.

I think it would be better to leave the loginuid in the payload of the
audit packets, not put it into generic netlink structures.

In the common case where audit messages are being generated by the
kernel, the loginuid can be trusted anyway, and doesn't need to be
handled by netlink. 

The only time it's possibly worth verifying it is for the case where
userspace is sending AUDIT_USER messages -- for which the process needs
CAP_AUDIT_WRITE anyway. And if you're then going to trust the rest of
what that process sends, what's wrong with trusting the loginuid which
it provides too?

Why should it be impossible for a trusted logging dæmon to log actions
of another process, running with a loginuid other than the loginuid of
the dæmon?

-- 
dwmw2

Re: [PATCH] Add audit uid to netlink credentials

[Alexey Kuznetsov]

Hello!

> > Reception of netlink messages in the kernel happens in the context
> > of the sending process, so you can simply call
> > audit_get_loginuid(current->audit_context) in audit_receive_msg().
> 
> Then why does netlink_sendmsg() need to save the effective capability

Yes, when kernel receives a message, it can be processed in context
of another process. This happens with rtnetlink, which queues messages
when someone holds netadmin semaphore and processing of backlog happens
in context of process which holds the semaphore.

Unfortunately, audit uses the same twisted way. Actually, if people
expected synchronous processing, it is better to replace

if (down_trylock(&audit_netlink_sem))
	return;

with plain down(&audit_netlink_sem);

Alexey

Re: [PATCH] Add audit uid to netlink credentials

[Serge Hallyn]

On Wed, 2005-02-09 at 14:17 +0000, David Woodhouse wrote:
> The only time it's possibly worth verifying it is for the case where
> userspace is sending AUDIT_USER messages -- for which the process needs
> CAP_AUDIT_WRITE anyway.

CAP_AUDIT_WRITE is needed, but not CAP_AUDIT_CONTROL, which is needed to
set the loginuid.  Of course, an LSM could check at
security_netlink_send whether the login_uid in the payload is the same
as the real loginuid.  Otherwise, we're wasting a (very precious)
capability bit.

In either case, have we decided we don't want it in the netlink
credentials after all?

thanks,
-serge 
-- 
Serge Hallyn <serue@us.ibm.com>

Re: [PATCH] Add audit uid to netlink credentials

[Alexey Kuznetsov]

Hello!

> if (down_trylock(&audit_netlink_sem))
> 	return;
> 
> with plain down(&audit_netlink_sem);

I am sorry, this is wrong. Dequeue may happen in another process context
in any case.

Alexey

Re: [PATCH] Add audit uid to netlink credentials

[Stephen Smalley]

On Wed, 2005-02-09 at 09:50, Serge Hallyn wrote:
> CAP_AUDIT_WRITE is needed, but not CAP_AUDIT_CONTROL, which is needed to
> set the loginuid.  Of course, an LSM could check at
> security_netlink_send whether the login_uid in the payload is the same
> as the real loginuid.  Otherwise, we're wasting a (very precious)
> capability bit.
> 
> In either case, have we decided we don't want it in the netlink
> credentials after all?

If the audit subsystem truly needs to include the loginuid in audit
messages generated upon processing netlink messages, then I think it
belongs in the control buffer as per your patch.  Alexey has confirmed
that we cannot use the current task's audit context regardless.

As a side bar, a similar security field in the control buffer would
likewise be very useful so that SELinux could set the SID for use in
permission checks by receive functions.

-- 
Stephen Smalley <sds@epoch.ncsc.mil>
National Security Agency

Re: [PATCH] Add audit uid to netlink credentials

[Chris Wright]

* Stephen Smalley (sds@epoch.ncsc.mil) wrote:
> On Wed, 2005-02-09 at 09:50, Serge Hallyn wrote:
> > CAP_AUDIT_WRITE is needed, but not CAP_AUDIT_CONTROL, which is needed to
> > set the loginuid.  Of course, an LSM could check at
> > security_netlink_send whether the login_uid in the payload is the same
> > as the real loginuid.  Otherwise, we're wasting a (very precious)
> > capability bit.
> > 
> > In either case, have we decided we don't want it in the netlink
> > credentials after all?
> 
> If the audit subsystem truly needs to include the loginuid in audit
> messages generated upon processing netlink messages, then I think it
> belongs in the control buffer as per your patch.  Alexey has confirmed
> that we cannot use the current task's audit context regardless.
> 
> As a side bar, a similar security field in the control buffer would
> likewise be very useful so that SELinux could set the SID for use in
> permission checks by receive functions.

This means sendmsg hook would set the SID?  And in that case, you'd
stomp on loginuid for audit messages unless they are special cased.
The loginuid is special case to audit, it doesn't make sense to me that
it is in generic netlink_skb_parms structure unless it's used by more
netlink users.

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net

Re: [PATCH] Add audit uid to netlink credentials

[Stephen Smalley]

On Wed, 2005-02-09 at 13:37, Chris Wright wrote:
> This means sendmsg hook would set the SID?  And in that case, you'd
> stomp on loginuid for audit messages unless they are special cased.

I was referring to a separate field for use by security modules, not
re-use of the same field being proposed for the loginuid.  Yes, it would
be set by the security_netlink_send hook.  The principal problem with
such a security field is that unless we mandate it to be a simple
integer value (like a SELinux SID), we have to deal with lifecycle
management for it, i.e. a set of hooks that starts to look like the
sk_buff security hooks from the old LSM patch.  But if we can limit it
to a simple value, then it would be useful for such security
identifiers, and allow receiver-side permission checks based on the
sender SID.

> The loginuid is special case to audit, it doesn't make sense to me that
> it is in generic netlink_skb_parms structure unless it's used by more
> netlink users.

So you also think it should be in the payload?  That would require
security_netlink_send to dig into the payload if we wanted to control
who can specify other loginuids, as Serge noted.

-- 
Stephen Smalley <sds@epoch.ncsc.mil>
National Security Agency

Re: [PATCH] Add audit uid to netlink credentials

[Patrick McHardy]

Alexey Kuznetsov wrote:

>Hello!
>
>  
>
>>if (down_trylock(&audit_netlink_sem))
>>	return;
>>
>>with plain down(&audit_netlink_sem);
>>
>
>I am sorry, this is wrong. Dequeue may happen in another process context
>in any case.
>
Could you explain how this can happen ? From what I can see whenever data
is queued to the receive queue the input function is called immediately
through sk->sk_data_ready() -> netlink_data_ready() -> nlk->data_ready()
and processes all queued packets, except in the case you pointed out,
when audit_netlink_sem is already taken.

Regards
Patrick

Re: [PATCH] Add audit uid to netlink credentials

[Stephen Smalley]

On Wed, 2005-02-09 at 13:52, Patrick McHardy wrote:
> Could you explain how this can happen ? From what I can see whenever data
> is queued to the receive queue the input function is called immediately
> through sk->sk_data_ready() -> netlink_data_ready() -> nlk->data_ready()
> and processes all queued packets, except in the case you pointed out,
> when audit_netlink_sem is already taken.

More packets may be queued by another sender while audit_receive() is
still processing the original one, so it will process them too.

-- 
Stephen Smalley <sds@epoch.ncsc.mil>
National Security Agency

Re: [PATCH] Add audit uid to netlink credentials

[Chris Wright]

* Stephen Smalley (sds@epoch.ncsc.mil) wrote:
> On Wed, 2005-02-09 at 13:37, Chris Wright wrote:
> > This means sendmsg hook would set the SID?  And in that case, you'd
> > stomp on loginuid for audit messages unless they are special cased.
> 
> I was referring to a separate field for use by security modules, not
> re-use of the same field being proposed for the loginuid.  Yes, it would
> be set by the security_netlink_send hook.  The principal problem with
> such a security field is that unless we mandate it to be a simple
> integer value (like a SELinux SID), we have to deal with lifecycle
> management for it, i.e. a set of hooks that starts to look like the
> sk_buff security hooks from the old LSM patch.  But if we can limit it
> to a simple value, then it would be useful for such security
> identifiers, and allow receiver-side permission checks based on the
> sender SID.

This makes sense to me.  Just an extension of existing eff_cap and would
be used by security modules for each netlink packet.

> > The loginuid is special case to audit, it doesn't make sense to me that
> > it is in generic netlink_skb_parms structure unless it's used by more
> > netlink users.
> 
> So you also think it should be in the payload?  That would require
> security_netlink_send to dig into the payload if we wanted to control
> who can specify other loginuids, as Serge noted.

I just don't see it making sense to add another credential for a special
case.  The signal code already peaks into the siginfo struct when queueing
a signal to make sure some user isn't trying to send si_code == SI_KERNEL
or similar.  Perhaps audit could do that with it's own payload during send.
No matter how we slice it, it's a special case.

Hmm, perhaps we could eliminate the whole asynchronous issue by allowing
registration of a netlink link specific security handler.  Something like:

netlink_kernel_create_sec(unit, rx, sec_handler)

Then the check would be done before the packet was ever queued.  This
would eliminate the if (NETLINK_CREDS(skb)->$cred == bad) on receipt
side, and push it to sender side.  It would also be link specific so
audit could do it's audit payload loginuid check here.  I think it would
also eliminate SELinux's need to tag the packet for later checking on
receipt.  Thoughts?

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net

Re: [PATCH] Add audit uid to netlink credentials

[David Woodhouse]

On Wed, 2005-02-09 at 15:38 -0800, Chris Wright wrote:
>> So you also think it should be in the payload?  That would require
>> security_netlink_send to dig into the payload if we wanted to control
>> who can specify other loginuids, as Serge noted.
>
>I just don't see it making sense to add another credential for a special
>case.  The signal code already peaks into the siginfo struct when queueing
>a signal to make sure some user isn't trying to send si_code == SI_KERNEL
>or similar.  Perhaps audit could do that with it's own payload during send.
>No matter how we slice it, it's a special case.

I'm not entirely sure the check is needed anyway. This is a trusted
application sending audit messages. Why shouldn't it be permitted to log
auditable events which were triggered by someone _else_? 

If we want to audit the actions of the userspace logging dæmon itself
and see what it sends, then we can quite happily do so within the audit
framework. That's a _different_ issue, surely?

-- 
dwmw2

Re: [PATCH] Add audit uid to netlink credentials

[Chris Wright]

* David Woodhouse (dwmw2@infradead.org) wrote:
> On Wed, 2005-02-09 at 15:38 -0800, Chris Wright wrote:
> >I just don't see it making sense to add another credential for a special
> >case.  The signal code already peaks into the siginfo struct when queueing
> >a signal to make sure some user isn't trying to send si_code == SI_KERNEL
> >or similar.  Perhaps audit could do that with it's own payload during send.
> >No matter how we slice it, it's a special case.
> 
> I'm not entirely sure the check is needed anyway. This is a trusted
> application sending audit messages. Why shouldn't it be permitted to log
> auditable events which were triggered by someone _else_? 

Then it comes back to the question of how to protect loginuid.  If it
can be spoofed by someone with CAP_AUDIT_WRITE, then it shouldn't be
write protected by CAP_AUDIT_CONTROL.

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net

Re: [PATCH] Add audit uid to netlink credentials

[Chris Wright]

* Chris Wright (chrisw@osdl.org) wrote:
> Hmm, perhaps we could eliminate the whole asynchronous issue by allowing
> registration of a netlink link specific security handler.  Something like:
> 
> netlink_kernel_create_sec(unit, rx, sec_handler)
> 
> Then the check would be done before the packet was ever queued.  This
> would eliminate the if (NETLINK_CREDS(skb)->$cred == bad) on receipt
> side, and push it to sender side.  It would also be link specific so
> audit could do it's audit payload loginuid check here.  I think it would
> also eliminate SELinux's need to tag the packet for later checking on
> receipt.  Thoughts?

Here's an example of what I mean.  It's quite rough, doesn't yet eliminate
any of the code that it eventually could, and doesn't deal with broadcast.
I gave it a quick test with audit netlink, and it does what I expect.

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net

===== kernel/audit.c 1.9 vs edited =====
--- 1.9/kernel/audit.c	2005-01-30 22:33:47 -08:00
+++ edited/kernel/audit.c	2005-02-09 17:07:22 -08:00
@@ -333,6 +333,15 @@ static int audit_netlink_ok(kernel_cap_t
 	return err;
 }
 
+static int audit_check_sender(struct sk_buff *skb)
+{
+	struct nlmsghdr *nlh = (struct nlmsghdr *)skb->data;
+	u16		msg_type = nlh->nlmsg_type;
+
+	return audit_netlink_ok(NETLINK_CB(skb).eff_cap, msg_type);
+
+}
+
 static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 {
 	u32			uid, pid, seq;
@@ -551,7 +560,7 @@ int __init audit_init(void)
 {
 	printk(KERN_INFO "audit: initializing netlink socket (%s)\n",
 	       audit_default ? "enabled" : "disabled");
-	audit_sock = netlink_kernel_create(NETLINK_AUDIT, audit_receive);
+	audit_sock = netlink_kernel_create_check(NETLINK_AUDIT, audit_receive, audit_check_sender);
 	if (!audit_sock)
 		audit_panic("cannot initialize netlink socket");
 
===== net/netlink/af_netlink.c 1.69 vs edited =====
--- 1.69/net/netlink/af_netlink.c	2005-01-21 12:25:32 -08:00
+++ edited/net/netlink/af_netlink.c	2005-02-09 16:39:15 -08:00
@@ -71,6 +71,7 @@ struct netlink_opt
 	struct netlink_callback	*cb;
 	spinlock_t		cb_lock;
 	void			(*data_ready)(struct sock *sk, int bytes);
+	int			(*send_check)(struct sk_buff *skb);
 };
 
 #define nlk_sk(__sk) ((struct netlink_opt *)(__sk)->sk_protinfo)
@@ -639,6 +640,14 @@ int netlink_sendskb(struct sock *sk, str
 	int len = skb->len;
 
 	nlk = nlk_sk(sk);
+
+	if (nlk->send_check)
+		if (nlk->send_check(skb)) {
+			kfree_skb(skb);
+			sock_put(sk);
+			return -EPERM;
+		}
+
 #ifdef NL_EMULATE_DEV
 	if (nlk->handler) {
 		skb_orphan(skb);
@@ -1033,7 +1042,8 @@ static void netlink_data_ready(struct so
  */
 
 struct sock *
-netlink_kernel_create(int unit, void (*input)(struct sock *sk, int len))
+netlink_kernel_create_check(int unit, void (*input)(struct sock *sk, int len),
+			    int (*check)(struct sk_buff *skb))
 {
 	struct socket *sock;
 	struct sock *sk;
@@ -1053,8 +1063,10 @@ netlink_kernel_create(int unit, void (*i
 	}
 	sk = sock->sk;
 	sk->sk_data_ready = netlink_data_ready;
-	if (input)
+	if (input) {
 		nlk_sk(sk)->data_ready = input;
+		nlk_sk(sk)->send_check = check;
+	}
 
 	if (netlink_insert(sk, 0)) {
 		sock_release(sock);
@@ -1459,7 +1471,7 @@ MODULE_ALIAS_NETPROTO(PF_NETLINK);
 EXPORT_SYMBOL(netlink_ack);
 EXPORT_SYMBOL(netlink_broadcast);
 EXPORT_SYMBOL(netlink_dump_start);
-EXPORT_SYMBOL(netlink_kernel_create);
+EXPORT_SYMBOL(netlink_kernel_create_check);
 EXPORT_SYMBOL(netlink_register_notifier);
 EXPORT_SYMBOL(netlink_set_err);
 EXPORT_SYMBOL(netlink_set_nonroot);
===== include/linux/netlink.h 1.23 vs edited =====
--- 1.23/include/linux/netlink.h	2005-02-06 21:59:39 -08:00
+++ edited/include/linux/netlink.h	2005-02-09 16:36:45 -08:00
@@ -116,7 +116,11 @@ struct netlink_skb_parms
 #define NETLINK_CREDS(skb)	(&NETLINK_CB((skb)).creds)
 
 
-extern struct sock *netlink_kernel_create(int unit, void (*input)(struct sock *sk, int len));
+extern struct sock *netlink_kernel_create_check(int unit, void (*input)(struct sock *sk, int len), int (*check)(struct sk_buff *skb));
+static inline struct sock *netlink_kernel_create(int unit, void (*input)(struct sock *sk, int len)) 
+{
+	return netlink_kernel_create_check(unit, input, NULL);
+}
 extern void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr *nlh, int err);
 extern int netlink_unicast(struct sock *ssk, struct sk_buff *skb, __u32 pid, int nonblock);
 extern int netlink_broadcast(struct sock *ssk, struct sk_buff *skb, __u32 pid,

Re: [PATCH] Add audit uid to netlink credentials

[David Woodhouse]

On Wed, 2005-02-09 at 16:19 -0800, Chris Wright wrote:
> Then it comes back to the question of how to protect loginuid.  If it
> can be spoofed by someone with CAP_AUDIT_WRITE, then it shouldn't be
> write protected by CAP_AUDIT_CONTROL.

I'm not sure I agree with that. With CAP_AUDIT_WRITE you _can't_ modify
the loginuid of the audit logs of your own actions. You can only modify
the loginuid on the messages you pull out of thin air and send. You can
already make up the rest of the payload -- why shouldn't you be allowed
to make up the loginuid too? You could be reporting something that
someone _else_ has done, after all.

Or am I misunderstanding the intended use of CAP_AUDIT_WRITE?

-- 
dwmw2

Re: [PATCH] Add audit uid to netlink credentials

[Stephen Smalley]

On Wed, 2005-02-09 at 20:11, Chris Wright wrote:
> * Chris Wright (chrisw@osdl.org) wrote:
> > Hmm, perhaps we could eliminate the whole asynchronous issue by allowing
> > registration of a netlink link specific security handler.  Something like:
> > 
> > netlink_kernel_create_sec(unit, rx, sec_handler)
> > 
> > Then the check would be done before the packet was ever queued.  This
> > would eliminate the if (NETLINK_CREDS(skb)->$cred == bad) on receipt
> > side, and push it to sender side.  It would also be link specific so
> > audit could do it's audit payload loginuid check here.  I think it would
> > also eliminate SELinux's need to tag the packet for later checking on
> > receipt.  Thoughts?
> 
> Here's an example of what I mean.  It's quite rough, doesn't yet eliminate
> any of the code that it eventually could, and doesn't deal with broadcast.
> I gave it a quick test with audit netlink, and it does what I expect.

Why not just call the security handler from the security_netlink_send()
function, which is already called by netlink_sendmsg()?  Note that
SELinux (thanks to work by James Morris a while back) does apply
fine-grained netlink controls using its selinux_netlink_send() hook
function, but the problem with this approach is that it requires
maintaining a netlink message type table within SELinux itself that maps
netlink operations to generic flow operations (read, write), effectively
duplicating state between SELinux and the netlink protocol
implementations.  It also isn't necessarily sufficiently granular for
all needs.  It would be preferable to have the netlink protocol
implementations to maintain such mappings and just invoke security
modules to check the generic permissions based on its own internal
mapping (same idea applies for ioctls, where the device driver or
filesystem code should handle the mapping to generic permissions and
only invoke the security module to check the generic permission).

-- 
Stephen Smalley <sds@epoch.ncsc.mil>
National Security Agency

Re: [PATCH] Add audit uid to netlink credentials

[Stephen Smalley]

On Wed, 2005-02-09 at 19:19, Chris Wright wrote:
> Then it comes back to the question of how to protect loginuid.  If it
> can be spoofed by someone with CAP_AUDIT_WRITE, then it shouldn't be
> write protected by CAP_AUDIT_CONTROL.

To be precise, isn't it true that someone with only CAP_AUDIT_WRITE
would only be able to spoof loginuids in the AUDIT_USER messages they
generate?  The loginuid on any syscall audit messages for the task would
still be the one associated with the task's audit context, so that would
not be spoofable.

-- 
Stephen Smalley <sds@epoch.ncsc.mil>
National Security Agency

Re: [PATCH] Add audit uid to netlink credentials

[David Woodhouse]

On Thu, 2005-02-10 at 07:40 -0500, Stephen Smalley wrote:
> To be precise, isn't it true that someone with only CAP_AUDIT_WRITE
> would only be able to spoof loginuids in the AUDIT_USER messages they
> generate?  The loginuid on any syscall audit messages for the task would
> still be the one associated with the task's audit context, so that would
> not be spoofable.

Correct.

-- 
dwmw2

Re: [PATCH] Add audit uid to netlink credentials

[Stephen Smalley]

On Thu, 2005-02-10 at 07:36, Stephen Smalley wrote:
> Why not just call the security handler from the security_netlink_send()
> function, which is already called by netlink_sendmsg()?

Ok, I can see why you wouldn't want to do that, but why not just call
the handler immediately after security_netlink_send() then in
netlink_sendmsg()?  What is the advantage of deferring to
netlink_sendskb (and some other location for broadcasts)?

-- 
Stephen Smalley <sds@tycho.nsa.gov>
National Security Agency

RE: [PATCH] Add audit uid to netlink credentials

[Chad Hanson]

David Woodhouse wrote:
> 
> On Wed, 2005-02-09 at 16:19 -0800, Chris Wright wrote:
> > Then it comes back to the question of how to protect loginuid.  If it
> > can be spoofed by someone with CAP_AUDIT_WRITE, then it shouldn't be
> > write protected by CAP_AUDIT_CONTROL.
> 
> I'm not sure I agree with that. With CAP_AUDIT_WRITE you _can't_ modify
> the loginuid of the audit logs of your own actions. You can only modify
> the loginuid on the messages you pull out of thin air and send. You can
> already make up the rest of the payload -- why shouldn't you be allowed
> to make up the loginuid too? You could be reporting something that
> someone _else_ has done, after all.
> 

I'm not sure I understand this logic. 

Let me start with some background. The purpose of the loginuid is to record
the original creator of the process regardless of credential changes since
login. We use a capability to protect this, so it cannot be altered by most
programs, even those which write audit records. Placing the loginuid in the
payload effectively removes the purpose of CAP_AUDIT_CONTROL from all
userland audit messages. A program may be privileged to write an audit
record, but a granular security approach would not let them have the ability
to change the loginuid as well.

In your example of a process watching daemon, why would this daemon want to
spoof the credentials of the watched process? I can think of two examples.
One you are recording information for IDS like purposes of system and
process state. This could be a good use of audit, however, I don't
understand the need to make the loginuid of the audit logs match the process
you are watching. If you really did, y0ou are a heavily privileged process
already to watch all of these other processes, simply change your loginuid
through CAP_AUDIT_CONTROL and add that to the other privileges you already
have in monitoring the system state.

-Chad

RE: [PATCH] Add audit uid to netlink credentials

[David Woodhouse]

On Thu, 2005-02-10 at 09:37 -0500, Chad Hanson wrote:
> In your example of a process watching daemon, why would this daemon want to
> spoof the credentials of the watched process? I can think of two examples.

Perhaps I misunderstand the intent of userspace AUDIT_WRITE. Can you
provide examples of why you _wouldn't_ want to let a dæmon which is
already sending random unvetted AUDIT_WRITE messages also specify the
loginuid on _those_ messages?

-- 
dwmw2

RE: [PATCH] Add audit uid to netlink credentials

[Chad Hanson]

David Woodhouse wrote:
> 
> Perhaps I misunderstand the intent of userspace AUDIT_WRITE. Can you
> provide examples of why you _wouldn't_ want to let a dæmon which is
> already sending random unvetted AUDIT_WRITE messages also specify the
> loginuid on _those_ messages?

The loginuid is part of the process state. This is the reason you do not
want to write out this information from a userspace application, as the
process state portions of the audit record are recorded by the kernel. 

-Chad

Re: [PATCH] Add audit uid to netlink credentials

[Chris Wright]

* Stephen Smalley (sds@epoch.ncsc.mil) wrote:
> On Wed, 2005-02-09 at 19:19, Chris Wright wrote:
> > Then it comes back to the question of how to protect loginuid.  If it
> > can be spoofed by someone with CAP_AUDIT_WRITE, then it shouldn't be
> > write protected by CAP_AUDIT_CONTROL.
> 
> To be precise, isn't it true that someone with only CAP_AUDIT_WRITE
> would only be able to spoof loginuids in the AUDIT_USER messages they
> generate?  The loginuid on any syscall audit messages for the task would
> still be the one associated with the task's audit context, so that would
> not be spoofable.

Yes, that's true.

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net

Re: [PATCH] Add audit uid to netlink credentials

[Klaus Weidner]

On Thu, Feb 10, 2005 at 02:56:36PM +0000, David Woodhouse wrote:
> On Thu, 2005-02-10 at 09:37 -0500, Chad Hanson wrote:
> > In your example of a process watching daemon, why would this daemon want to
> > spoof the credentials of the watched process? I can think of two examples.
> 
> Perhaps I misunderstand the intent of userspace AUDIT_WRITE. Can you
> provide examples of why you _wouldn't_ want to let a dæmon which is
> already sending random unvetted AUDIT_WRITE messages also specify the
> loginuid on _those_ messages?

A few comments on this issue from the point of view of common criteria
evaluations... Briefly, either choice of implementation would be okay.

Both CAPP and LSPP assume trustworthy administrators, and those
protection profiles don't really support the concept of fine grained
capabilities for not-quite-administrator tasks.

The CAPP and LSPP audit requirements include that audit records contain
the subject identity, this corresponds to the login UID. The point of the
user messages is to support proper auditing of actions that aren't
directly related to system calls, such as authenticating users, modifying
security databases, and similar things. This is always done by trusted
processes, so the technical method used to get the login UID into the
audit record for user messages doesn't matter as long as it can't be
falsified by non-administrators.

A CAPP/LSPP compliant implementation could for example completely bypass
the kernel and append user messages from trusted processes directly to
the audit log file - I'm not saying that would be a good idea, but it
could be compliant.

-Klaus

Re: [PATCH] Add audit uid to netlink credentials

[Casey Schaufler]

--- Klaus Weidner <klaus@atsec.com> wrote:

> Both CAPP and LSPP assume trustworthy
> administrators, and those
> protection profiles don't really support the concept
> of fine grained
> capabilities for not-quite-administrator tasks.

I don't know where you get that idea, as it is
inconsistent with the evaluations I have done.
A fine grained capability model that is used
will always make an evaluation easier. A process
that runs with partial privilege requires much
less supporting evidence for evaluation than
one that has superuser privilege. The Criteria
may not require fine grained privilege, but
that does not mean it doesn't help.

> The CAPP and LSPP audit requirements include that
> audit records contain
> the subject identity, this corresponds to the login
> UID.

The subject identity is the name of the subject,
that is the process ID. The user associated with
the session is the login UID. The user is not the
subject.

> The point of the
> user messages is to support proper auditing of
> actions that aren't
> directly related to system calls, such as
> authenticating users, modifying
> security databases, and similar things. This is
> always done by trusted
> processes, so the technical method used to get the
> login UID into the
> audit record for user messages doesn't matter as
> long as it can't be
> falsified by non-administrators.

This is correct as far as it goes. One of the
requirements is to correctly audit attempts made
by unprivileged users to write to the audit trail.
This is one reason you need the login UID of the
process attempting to write to the audit trail.

> A CAPP/LSPP compliant implementation could for
> example completely bypass
> the kernel and append user messages from trusted
> processes directly to
> the audit log file - I'm not saying that would be a
> good idea, but it
> could be compliant.

It is actually a very good idea to maintain
modifications to system security databases outside
the audit trail proper. In the Orange Book days
the NSA went so far as to declare a that a
reasonable procedure for keeping them under
revision control would be sufficient to meet the
audit requirements.

Another mechanism that would work well is to
send the user space records directly to the
audit daemon using a protected socket.


=====
Casey Schaufler
casey@schaufler-ca.com


	
		
__________________________________ 
Do you Yahoo!? 
Yahoo! Mail - You care about security. So do we. 
http://promotions.yahoo.com/new_mail

Re: [PATCH] Add audit uid to netlink credentials

[Klaus Weidner]

On Thu, Feb 10, 2005 at 10:10:23AM -0800, Casey Schaufler wrote:
> --- Klaus Weidner <klaus@atsec.com> wrote:
> > Both CAPP and LSPP assume trustworthy administrators, and those
> > protection profiles don't really support the concept of fine grained
> > capabilities for not-quite-administrator tasks.
> 
> I don't know where you get that idea, as it is inconsistent with the
> evaluations I have done.  A fine grained capability model that is used
> will always make an evaluation easier. A process that runs with partial
> privilege requires much less supporting evidence for evaluation than
> one that has superuser privilege. The Criteria may not require fine
> grained privilege, but that does not mean it doesn't help.

My point was that the security functional requirements in CAPP and LSPP
related to audit and user management only differentiate between
authorized users and authorized administrators, and adding a finer
grained distinction to those SFRs would be incompatible with CAPP/LSPP.

CAPP and LSPP permit extending the DAC and MAC policies by adding
additional rules such as capability-based ones, but I'd normally expect
that to cause more work instead of less when evaluating since all claims
related to that policy would then need to be sufficiently tested and
verified. But I think that's getting off-topic for this list.

> > The CAPP and LSPP audit requirements include that audit records
> > contain the subject identity, this corresponds to the login UID.
> 
> The subject identity is the name of the subject, that is the process
> ID. The user associated with the session is the login UID. The user is
> not the subject.

Sorry, I blame lack of caffeine for that mixup. The requirement I meant
was that each auditable event needs to be associated with the identity of
the user that caused the event. In the context of this discussion, that
means that the login UID in audit records needs to correspond to the user
identity of the user on whose behalf the process (subject) runs.

For cron/at and other indirect execution methods, the daemon running the
jobs needs to be able to correctly associate the login UID of the user
who submitted the job with the audit records generated by that job.
Usually such daemons will need the capability to change the login UID for
forked processes in addition to generating user messages with a specific
login UID included in the record, so the distinction between those two
capabilities isn't all that useful in this case.

> It is actually a very good idea to maintain modifications to system
> security databases outside the audit trail proper. In the Orange Book
> days the NSA went so far as to declare a that a reasonable procedure
> for keeping them under revision control would be sufficient to meet the
> audit requirements.

Hmmm, this doesn't match the application note for FAU_SAR.1 in CAPP and
LSPP that mentions that it is expected that a single tool will exist that
satisfies all the various requirements related to audit information
access. Of course application notes aren't normative, and you could claim
that "grep" is the single tool usable for all types of audit trail
access...

-Klaus