[PATCH] fix masking of capabilities over netlink in permissive mode

[PATCH] fix masking of capabilities over netlink in permissive mode

[Darrel Goeddel]

I think I ran across the problem described in this thread:

http://www.redhat.com/archives/linux-audit/2006-May/msg00059.html

The process' effective capabilities are always being masked with the
allowed vector of the avc decision (for self against the capability
security class) in netlink's copy of the process capabilities (eff_cap).
The allowed vector takes on a slightly different role when SELinux
is not in enforcing mode - it starts to track used-but-not-normally-
permitted actions in the allowed vector.  That is what is causing
the first attempt to fail (the allowed vector has not been "inflated")
and the following attempts to succeed (the vector has been inflated in
response to its previous use).  Does my reasoning (and patch) seem to
be on track?




This patch removes the masking of capabilities passed over netlink
socket when SELinux is in enforcing mode.

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


diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 21dad41..c7650bb 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -3599,7 +3599,8 @@ static int selinux_netlink_send(struct s
 	avd.allowed = 0;
 	avc_has_perm_noaudit(tsec->sid, tsec->sid,
 				SECCLASS_CAPABILITY, ~0, &avd);
-	cap_mask(NETLINK_CB(skb).eff_cap, avd.allowed);
+	if (selinux_enforcing)
+		cap_mask(NETLINK_CB(skb).eff_cap, avd.allowed);
 
 	if (policydb_loaded_version >= POLICYDB_VERSION_NLCLASS)
 		err = selinux_nlmsg_perm(sk, skb);

-- 

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.

Re: [PATCH] fix masking of capabilities over netlink in permissive mode

[Stephen Smalley]

On Wed, 2006-05-31 at 12:35 -0500, Darrel Goeddel wrote:
> I think I ran across the problem described in this thread:
> 
> http://www.redhat.com/archives/linux-audit/2006-May/msg00059.html
> 
> The process' effective capabilities are always being masked with the
> allowed vector of the avc decision (for self against the capability
> security class) in netlink's copy of the process capabilities (eff_cap).
> The allowed vector takes on a slightly different role when SELinux
> is not in enforcing mode - it starts to track used-but-not-normally-
> permitted actions in the allowed vector.  That is what is causing
> the first attempt to fail (the allowed vector has not been "inflated")
> and the following attempts to succeed (the vector has been inflated in
> response to its previous use).  Does my reasoning (and patch) seem to
> be on track?

Alternative:  Since the sending task SID is now saved in the netlink
control buffer, we could move the netlink checking entirely to the
receive side, and perform a normal avc_has_perm() check, via
task_has_capability, with corresponding auditing of netlink denials.
Similarly for audit_netlink_ok.  We couldn't do that in the past because
the sender SID wasn't available to us on the receive side.

> This patch removes the masking of capabilities passed over netlink
> socket when SELinux is in enforcing mode.

I assume that you meant "permissive mode" above.

-- 
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.

Re: [PATCH] fix masking of capabilities over netlink in permissive mode

[Darrel Goeddel]

Stephen Smalley wrote:
> On Wed, 2006-05-31 at 12:35 -0500, Darrel Goeddel wrote:
> 
>>I think I ran across the problem described in this thread:
>>
>>http://www.redhat.com/archives/linux-audit/2006-May/msg00059.html
>>
>>The process' effective capabilities are always being masked with the
>>allowed vector of the avc decision (for self against the capability
>>security class) in netlink's copy of the process capabilities (eff_cap).
>>The allowed vector takes on a slightly different role when SELinux
>>is not in enforcing mode - it starts to track used-but-not-normally-
>>permitted actions in the allowed vector.  That is what is causing
>>the first attempt to fail (the allowed vector has not been "inflated")
>>and the following attempts to succeed (the vector has been inflated in
>>response to its previous use).  Does my reasoning (and patch) seem to
>>be on track?
> 
> 
> Alternative:  Since the sending task SID is now saved in the netlink
> control buffer, we could move the netlink checking entirely to the
> receive side, and perform a normal avc_has_perm() check, via
> task_has_capability, with corresponding auditing of netlink denials.
> Similarly for audit_netlink_ok.  We couldn't do that in the past because
> the sender SID wasn't available to us on the receive side.

Good idea - I forgot about the sid being there now.  That approach would
have the benefit of actually getting the AVC denials for capability checks
that occur "over netlink".  However, this would involve replacing all of the
checks using eff_cap (thankfully not very many) with new lsm hook(s).  This
also will provide better encapsulation for the capability system.  I was
hoping that this simple patch would have a shot at making the release
of 2.6.17 to at least address the current problem.  I can work up patches
that creates the new lsm hook to replace the current instances of
cap_raised(eff_cap) and move the SELinux checking into that hook.  Would
a single security_netlink_capable(struct netlink_skb_params) hook suffice,
or would decomposition of the the actual actions be preferred
(and acceptable)?

Thanks for the idea.

> 
>>This patch removes the masking of capabilities passed over netlink
>>socket when SELinux is in enforcing mode.
> 
> 
> I assume that you meant "permissive mode" above.
>

Yep, looks like I was in a hurry when I sent the mail...

-- 

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.

Re: [PATCH] fix masking of capabilities over netlink in permissive mode

[Stephen Smalley]

On Thu, 2006-06-01 at 09:13 -0500, Darrel Goeddel wrote:
> Stephen Smalley wrote:
> > On Wed, 2006-05-31 at 12:35 -0500, Darrel Goeddel wrote:
> > 
> >>I think I ran across the problem described in this thread:
> >>
> >>http://www.redhat.com/archives/linux-audit/2006-May/msg00059.html
> >>
> >>The process' effective capabilities are always being masked with the
> >>allowed vector of the avc decision (for self against the capability
> >>security class) in netlink's copy of the process capabilities (eff_cap).
> >>The allowed vector takes on a slightly different role when SELinux
> >>is not in enforcing mode - it starts to track used-but-not-normally-
> >>permitted actions in the allowed vector.  That is what is causing
> >>the first attempt to fail (the allowed vector has not been "inflated")
> >>and the following attempts to succeed (the vector has been inflated in
> >>response to its previous use).  Does my reasoning (and patch) seem to
> >>be on track?
> > 
> > 
> > Alternative:  Since the sending task SID is now saved in the netlink
> > control buffer, we could move the netlink checking entirely to the
> > receive side, and perform a normal avc_has_perm() check, via
> > task_has_capability, with corresponding auditing of netlink denials.
> > Similarly for audit_netlink_ok.  We couldn't do that in the past because
> > the sender SID wasn't available to us on the receive side.
> 
> Good idea - I forgot about the sid being there now.  That approach would
> have the benefit of actually getting the AVC denials for capability checks
> that occur "over netlink".  However, this would involve replacing all of the
> checks using eff_cap (thankfully not very many) with new lsm hook(s).  This
> also will provide better encapsulation for the capability system.  I was
> hoping that this simple patch would have a shot at making the release
> of 2.6.17 to at least address the current problem.

If we think it is warranted, we can push up the simple patch now and
then proceed with the more general fix later.  But I wasn't clear that
the current behavior is a major problem, as it only affects permissive
mode operation.   

>   I can work up patches
> that creates the new lsm hook to replace the current instances of
> cap_raised(eff_cap) and move the SELinux checking into that hook.  Would
> a single security_netlink_capable(struct netlink_skb_params) hook suffice,
> or would decomposition of the the actual actions be preferred
> (and acceptable)?

Possibly generalize the existing security_netlink_recv() hook to also
take a capability to check as an argument, and then pass CAP_NET_ADMIN
from the net callers and CAP_AUDIT_xxx from audit_netlink_ok.
 
-- 
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.

Re: [PATCH] fix masking of capabilities over netlink in permissive mode

[Darrel Goeddel]

Stephen Smalley wrote:
> On Thu, 2006-06-01 at 09:13 -0500, Darrel Goeddel wrote:
> 
>>Stephen Smalley wrote:
>>
>>>On Wed, 2006-05-31 at 12:35 -0500, Darrel Goeddel wrote:
>>>
>>>
>>>>I think I ran across the problem described in this thread:
>>>>
>>>>http://www.redhat.com/archives/linux-audit/2006-May/msg00059.html
>>>>
>>>>The process' effective capabilities are always being masked with the
>>>>allowed vector of the avc decision (for self against the capability
>>>>security class) in netlink's copy of the process capabilities (eff_cap).
>>>>The allowed vector takes on a slightly different role when SELinux
>>>>is not in enforcing mode - it starts to track used-but-not-normally-
>>>>permitted actions in the allowed vector.  That is what is causing
>>>>the first attempt to fail (the allowed vector has not been "inflated")
>>>>and the following attempts to succeed (the vector has been inflated in
>>>>response to its previous use).  Does my reasoning (and patch) seem to
>>>>be on track?
>>>
>>>
>>>Alternative:  Since the sending task SID is now saved in the netlink
>>>control buffer, we could move the netlink checking entirely to the
>>>receive side, and perform a normal avc_has_perm() check, via
>>>task_has_capability, with corresponding auditing of netlink denials.
>>>Similarly for audit_netlink_ok.  We couldn't do that in the past because
>>>the sender SID wasn't available to us on the receive side.
>>
>>Good idea - I forgot about the sid being there now.  That approach would
>>have the benefit of actually getting the AVC denials for capability checks
>>that occur "over netlink".  However, this would involve replacing all of the
>>checks using eff_cap (thankfully not very many) with new lsm hook(s).  This
>>also will provide better encapsulation for the capability system.  I was
>>hoping that this simple patch would have a shot at making the release
>>of 2.6.17 to at least address the current problem.
> 
> 
> If we think it is warranted, we can push up the simple patch now and
> then proceed with the more general fix later.  But I wasn't clear that
> the current behavior is a major problem, as it only affects permissive
> mode operation.   

Now that we actually know what the problem is (and that it is only a
permissive mode issue), it does not seem very urgent to me.  Others may
view this as obstacle, but avoiding permissive mode altogether is
an even better fix ;)  Anyone *need* this?  We could also just carry this
as a temporary fix in the lspp kernel until a better fix is available.

> 
>>  I can work up patches
>>that creates the new lsm hook to replace the current instances of
>>cap_raised(eff_cap) and move the SELinux checking into that hook.  Would
>>a single security_netlink_capable(struct netlink_skb_params) hook suffice,
>>or would decomposition of the the actual actions be preferred
>>(and acceptable)?
> 
> 
> Possibly generalize the existing security_netlink_recv() hook to also
> take a capability to check as an argument, and then pass CAP_NET_ADMIN
> from the net callers and CAP_AUDIT_xxx from audit_netlink_ok.
>  

I'll look into that.  A quick glance seems to indicate that the current hook
and the usage of eff_cap are in very different places, but I'm not super
familiar with the netlink codepath.

-- 

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.

Re: [redhat-lspp] Re: [PATCH] fix masking of capabilities over netlink in permissive mode

[Steve Grubb]

On Thursday 01 June 2006 10:58, Darrel Goeddel wrote:
> We could also just carry this as a temporary fix in the lspp kernel until a
> better fix is available.

I'll put this in today's build since it does annoy some people.

-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.

Re: [PATCH] fix masking of capabilities over netlink in permissive mode

[Linda Knippers]

Stephen Smalley wrote:

> If we think it is warranted, we can push up the simple patch now and
> then proceed with the more general fix later.  But I wasn't clear that
> the current behavior is a major problem, as it only affects permissive
> mode operation.   

Since permissive mode is a supported mode and the error is rather
bizarre, I'd like to see it fixed.

-- 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.

Re: [PATCH] fix masking of capabilities over netlink in permissive mode

[Darrel Goeddel]

Stephen Smalley wrote:
> On Thu, 2006-06-01 at 09:13 -0500, Darrel Goeddel wrote:
> 
>>Stephen Smalley wrote:
>>
>>>On Wed, 2006-05-31 at 12:35 -0500, Darrel Goeddel wrote:
>>>
>>>
>>>>I think I ran across the problem described in this thread:
>>>>
>>>>http://www.redhat.com/archives/linux-audit/2006-May/msg00059.html
>>>>
>>>>The process' effective capabilities are always being masked with the
>>>>allowed vector of the avc decision (for self against the capability
>>>>security class) in netlink's copy of the process capabilities (eff_cap).
>>>>The allowed vector takes on a slightly different role when SELinux
>>>>is not in enforcing mode - it starts to track used-but-not-normally-
>>>>permitted actions in the allowed vector.  That is what is causing
>>>>the first attempt to fail (the allowed vector has not been "inflated")
>>>>and the following attempts to succeed (the vector has been inflated in
>>>>response to its previous use).  Does my reasoning (and patch) seem to
>>>>be on track?
>>>
>>>
>>>Alternative:  Since the sending task SID is now saved in the netlink
>>>control buffer, we could move the netlink checking entirely to the
>>>receive side, and perform a normal avc_has_perm() check, via
>>>task_has_capability, with corresponding auditing of netlink denials.
>>>Similarly for audit_netlink_ok.  We couldn't do that in the past because
>>>the sender SID wasn't available to us on the receive side.

Here's an attempt using the above approach.  What does everyone think?  The
only issue I can see is that there may be a possibility of getting some
incorrect audit information if many queued netlink messages are processed
at the same time.  The AVC message will include information about "current",
which may not be the sending process.  Stephen suggested that the AVC message
could be modified to include the pid from the netlink message instead of
information about current.  That would lead to an AVC message stating pid=XXX,
while the syscall message would state pid=YYY.  Anyone see a way to get correct
information, or at least suppress possibly incorrect information.  I really
like the idea of getting the AVC message because tracking down cases where
SELinux was denying CAP_AUDIT_XXX was a huge pain when userspace was starting to
become audit-aware...



This patch encapsulates the usage of eff_cap (in netlink_skb_params) within
the lsm framework by extending security_netlink_recv to include a required
capability parameter and converting all direct usage of eff_caps outside
of the lsm modules to use the interface.  It also updates the SELinux
implementation of the security_netlink_send and security_netlink_recv
hooks to take advantage of the sid in the netlink_skb_params struct.

diff --git a/include/linux/security.h b/include/linux/security.h
index 1bab48f..deb3968 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -67,7 +67,7 @@ struct xfrm_state;
 struct xfrm_user_sec_ctx;
 
 extern int cap_netlink_send(struct sock *sk, struct sk_buff *skb);
-extern int cap_netlink_recv(struct sk_buff *skb);
+extern int cap_netlink_recv(struct sk_buff *skb, int cap);
 
 /*
  * Values used in the task_security_ops calls
@@ -647,6 +647,7 @@ #ifdef CONFIG_SECURITY
  *	Check permission before processing the received netlink message in
  *	@skb.
  *	@skb contains the sk_buff structure for the netlink message.
+ *	@cap indicates the capability required
  *	Return 0 if permission is granted.
  *
  * Security hooks for Unix domain networking.
@@ -1248,7 +1249,7 @@ struct security_operations {
 			  struct sembuf * sops, unsigned nsops, int alter);
 
 	int (*netlink_send) (struct sock * sk, struct sk_buff * skb);
-	int (*netlink_recv) (struct sk_buff * skb);
+	int (*netlink_recv) (struct sk_buff * skb, int cap);
 
 	/* allow module stacking */
 	int (*register_security) (const char *name,
@@ -2002,9 +2003,9 @@ static inline int security_netlink_send(
 	return security_ops->netlink_send(sk, skb);
 }
 
-static inline int security_netlink_recv(struct sk_buff * skb)
+static inline int security_netlink_recv(struct sk_buff * skb, int cap)
 {
-	return security_ops->netlink_recv(skb);
+	return security_ops->netlink_recv(skb, cap);
 }
 
 /* prototypes */
@@ -2630,9 +2631,9 @@ static inline int security_netlink_send 
 	return cap_netlink_send (sk, skb);
 }
 
-static inline int security_netlink_recv (struct sk_buff *skb)
+static inline int security_netlink_recv (struct sk_buff *skb, int cap)
 {
-	return cap_netlink_recv (skb);
+	return cap_netlink_recv (skb, cap);
 }
 
 static inline struct dentry *securityfs_create_dir(const char *name,
diff --git a/kernel/audit.c b/kernel/audit.c
index df57b49..8c6b430 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -412,7 +412,7 @@ nlmsg_failure:			/* Used by NLMSG_PUT */
  * Check for appropriate CAP_AUDIT_ capabilities on incoming audit
  * control messages.
  */
-static int audit_netlink_ok(kernel_cap_t eff_cap, u16 msg_type)
+static int audit_netlink_ok(struct sk_buff *skb, u16 msg_type)
 {
 	int err = 0;
 
@@ -426,13 +426,13 @@ static int audit_netlink_ok(kernel_cap_t
 	case AUDIT_DEL:
 	case AUDIT_DEL_RULE:
 	case AUDIT_SIGNAL_INFO:
-		if (!cap_raised(eff_cap, CAP_AUDIT_CONTROL))
+		if (security_netlink_recv(skb, CAP_AUDIT_CONTROL))
 			err = -EPERM;
 		break;
 	case AUDIT_USER:
 	case AUDIT_FIRST_USER_MSG...AUDIT_LAST_USER_MSG:
 	case AUDIT_FIRST_USER_MSG2...AUDIT_LAST_USER_MSG2:
-		if (!cap_raised(eff_cap, CAP_AUDIT_WRITE))
+		if (security_netlink_recv(skb, CAP_AUDIT_WRITE))
 			err = -EPERM;
 		break;
 	default:  /* bad msg */
@@ -453,7 +453,7 @@ static int audit_receive_msg(struct sk_b
 	uid_t			loginuid; /* loginuid of sender */
 	struct audit_sig_info   sig_data;
 
-	err = audit_netlink_ok(NETLINK_CB(skb).eff_cap, msg_type);
+	err = audit_netlink_ok(skb, msg_type);
 	if (err)
 		return err;
 
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 3fcfa9c..f25aac1 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -663,7 +663,7 @@ rtnetlink_rcv_msg(struct sk_buff *skb, s
 	sz_idx = type>>2;
 	kind = type&3;
 
-	if (kind != 2 && security_netlink_recv(skb)) {
+	if (kind != 2 && security_netlink_recv(skb, CAP_NET_ADMIN)) {
 		*errp = -EPERM;
 		return -1;
 	}
diff --git a/net/decnet/netfilter/dn_rtmsg.c b/net/decnet/netfilter/dn_rtmsg.c
index 74133ec..8b99bd3 100644
--- a/net/decnet/netfilter/dn_rtmsg.c
+++ b/net/decnet/netfilter/dn_rtmsg.c
@@ -107,7 +107,7 @@ static inline void dnrmg_receive_user_sk
 	if (nlh->nlmsg_len < sizeof(*nlh) || skb->len < nlh->nlmsg_len)
 		return;
 
-	if (!cap_raised(NETLINK_CB(skb).eff_cap, CAP_NET_ADMIN))
+	if (security_netlink_recv(skb, CAP_NET_ADMIN))
 		RCV_SKB_FAIL(-EPERM);
 
 	/* Eventually we might send routing messages too */
diff --git a/net/ipv4/netfilter/ip_queue.c b/net/ipv4/netfilter/ip_queue.c
index b93f049..89a42af 100644
--- a/net/ipv4/netfilter/ip_queue.c
+++ b/net/ipv4/netfilter/ip_queue.c
@@ -507,7 +507,7 @@ ipq_rcv_skb(struct sk_buff *skb)
 	if (type <= IPQM_BASE)
 		return;
 		
-	if (security_netlink_recv(skb))
+	if (security_netlink_recv(skb, CAP_NET_ADMIN))
 		RCV_SKB_FAIL(-EPERM);
 	
 	write_lock_bh(&queue_lock);
diff --git a/net/ipv6/netfilter/ip6_queue.c b/net/ipv6/netfilter/ip6_queue.c
index b4b7d44..968a14b 100644
--- a/net/ipv6/netfilter/ip6_queue.c
+++ b/net/ipv6/netfilter/ip6_queue.c
@@ -505,7 +505,7 @@ ipq_rcv_skb(struct sk_buff *skb)
 	if (type <= IPQM_BASE)
 		return;
 	
-	if (security_netlink_recv(skb))
+	if (security_netlink_recv(skb, CAP_NET_ADMIN))
 		RCV_SKB_FAIL(-EPERM);	
 
 	write_lock_bh(&queue_lock);
diff --git a/net/netfilter/nfnetlink.c b/net/netfilter/nfnetlink.c
index b88e82a..ec9f0ef 100644
--- a/net/netfilter/nfnetlink.c
+++ b/net/netfilter/nfnetlink.c
@@ -229,7 +229,7 @@ static int nfnetlink_rcv_msg(struct sk_b
 		 NFNL_SUBSYS_ID(nlh->nlmsg_type),
 		 NFNL_MSG_TYPE(nlh->nlmsg_type));
 
-	if (!cap_raised(NETLINK_CB(skb).eff_cap, CAP_NET_ADMIN)) {
+	if (security_netlink_recv(skb, CAP_NET_ADMIN)) {
 		DEBUGP("missing CAP_NET_ADMIN\n");
 		*errp = -EPERM;
 		return -1;
diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
index f329b72..edf084b 100644
--- a/net/netlink/genetlink.c
+++ b/net/netlink/genetlink.c
@@ -320,7 +320,7 @@ static int genl_rcv_msg(struct sk_buff *
 		goto errout;
 	}
 
-	if ((ops->flags & GENL_ADMIN_PERM) && security_netlink_recv(skb)) {
+	if ((ops->flags & GENL_ADMIN_PERM) && security_netlink_recv(skb, CAP_NET_ADMIN)) {
 		err = -EPERM;
 		goto errout;
 	}
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 81d1005..d514b3b 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -1430,7 +1430,7 @@ static int xfrm_user_rcv_msg(struct sk_b
 	link = &xfrm_dispatch[type];
 
 	/* All operations require privileges, even GET */
-	if (security_netlink_recv(skb)) {
+	if (security_netlink_recv(skb, CAP_NET_ADMIN)) {
 		*errp = -EPERM;
 		return -1;
 	}
diff --git a/security/commoncap.c b/security/commoncap.c
index 841eb4e..57673ee 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -33,9 +33,9 @@ int cap_netlink_send(struct sock *sk, st
 
 EXPORT_SYMBOL(cap_netlink_send);
 
-int cap_netlink_recv(struct sk_buff *skb)
+int cap_netlink_recv(struct sk_buff *skb, int cap)
 {
-	if (!cap_raised(NETLINK_CB(skb).eff_cap, CAP_NET_ADMIN))
+	if (!cap_raised(NETLINK_CB(skb).eff_cap, cap))
 		return -EPERM;
 	return 0;
 }
diff --git a/security/dummy.c b/security/dummy.c
index 8cccccc..49ad82d 100644
--- a/security/dummy.c
+++ b/security/dummy.c
@@ -665,9 +665,9 @@ static int dummy_netlink_send (struct so
 	return 0;
 }
 
-static int dummy_netlink_recv (struct sk_buff *skb)
+static int dummy_netlink_recv (struct sk_buff *skb, int cap)
 {
-	if (!cap_raised (NETLINK_CB (skb).eff_cap, CAP_NET_ADMIN))
+	if (!cap_raised (NETLINK_CB (skb).eff_cap, cap))
 		return -EPERM;
 	return 0;
 }
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 21dad41..023da01 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -3586,32 +3586,32 @@ #endif	/* CONFIG_NETFILTER */
 
 static int selinux_netlink_send(struct sock *sk, struct sk_buff *skb)
 {
-	struct task_security_struct *tsec;
-	struct av_decision avd;
 	int err;
 
 	err = secondary_ops->netlink_send(sk, skb);
 	if (err)
 		return err;
 
-	tsec = current->security;
-
-	avd.allowed = 0;
-	avc_has_perm_noaudit(tsec->sid, tsec->sid,
-				SECCLASS_CAPABILITY, ~0, &avd);
-	cap_mask(NETLINK_CB(skb).eff_cap, avd.allowed);
-
 	if (policydb_loaded_version >= POLICYDB_VERSION_NLCLASS)
 		err = selinux_nlmsg_perm(sk, skb);
 
 	return err;
 }
 
-static int selinux_netlink_recv(struct sk_buff *skb)
+static int selinux_netlink_recv(struct sk_buff *skb, int capability)
 {
-	if (!cap_raised(NETLINK_CB(skb).eff_cap, CAP_NET_ADMIN))
-		return -EPERM;
-	return 0;
+	int err;
+	struct avc_audit_data ad;
+
+	err = secondary_ops->netlink_recv(skb, capability);
+	if (err)
+		return err;
+
+	AVC_AUDIT_DATA_INIT(&ad, CAP);
+	ad.u.cap = capability;
+
+	return avc_has_perm(NETLINK_CB(skb).sid, NETLINK_CB(skb).sid,
+	                    SECCLASS_CAPABILITY, CAP_TO_MASK(capability), &ad);
 }
 
 static int ipc_alloc_security(struct task_struct *task,

-- 

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.

Re: [PATCH] fix masking of capabilities over netlink in permissive mode

[Stephen Smalley]

On Fri, 2006-06-02 at 09:49 -0500, Darrel Goeddel wrote:
> Here's an attempt using the above approach.  What does everyone think?  The
> only issue I can see is that there may be a possibility of getting some
> incorrect audit information if many queued netlink messages are processed
> at the same time.  The AVC message will include information about "current",
> which may not be the sending process.  Stephen suggested that the AVC message
> could be modified to include the pid from the netlink message instead of
> information about current.  That would lead to an AVC message stating pid=XXX,
> while the syscall message would state pid=YYY.  Anyone see a way to get correct
> information, or at least suppress possibly incorrect information.

Note that this could involve more than just the pid; it could be any of
the information in the NETLINK_CB or even the NETLINK_CREDS, so it could
also include loginuid and possibly uid info.  You can propagate that
data to avc_audit by extending the avc audit data structure, and either
have it log it instead of the current task info or have it log it as
supplemental data in addition to the current task info.  You could
further propagate it to syscall audit by setting those values in the
audit context from the netlink information, but it isn't clear that is
what you want and I don't think that gives you the full task information
that is presently logged by audit_log_exit.

The other approach is to move all checking to send time, including
audit_netlink_ok and equivalent checking by other netlink handlers,
using a callback mechanism as proposed in patches by Chris Wright a long
time ago.  I'm not sure what happened with those.  Then the current task
info is valid for checking and for auditing.

>   I really
> like the idea of getting the AVC message because tracking down cases where
> SELinux was denying CAP_AUDIT_XXX was a huge pain when userspace was starting to
> become audit-aware...

Yes, and the same applies for CAP_NET_ADMIN check by various netlink
handlers - we don't get any audit messages presently since they are
checked at receive time.

> This patch encapsulates the usage of eff_cap (in netlink_skb_params) within
> the lsm framework by extending security_netlink_recv to include a required
> capability parameter and converting all direct usage of eff_caps outside
> of the lsm modules to use the interface.  It also updates the SELinux
> implementation of the security_netlink_send and security_netlink_recv
> hooks to take advantage of the sid in the netlink_skb_params struct.

The patch looks sane to me.  I'm not as concerned about the audit
confusion because the portions relevant to policy will be correct (i.e.
the SIDs/contexts).

-- 
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.

Re: [PATCH] fix masking of capabilities over netlink in permissive mode

[Darrel Goeddel]

Stephen Smalley wrote:
> On Fri, 2006-06-02 at 09:49 -0500, Darrel Goeddel wrote:
> 
>>Here's an attempt using the above approach.  What does everyone think?  The
>>only issue I can see is that there may be a possibility of getting some
>>incorrect audit information if many queued netlink messages are processed
>>at the same time.  The AVC message will include information about "current",
>>which may not be the sending process.  Stephen suggested that the AVC message
>>could be modified to include the pid from the netlink message instead of
>>information about current.  That would lead to an AVC message stating pid=XXX,
>>while the syscall message would state pid=YYY.  Anyone see a way to get correct
>>information, or at least suppress possibly incorrect information.
> 
> 
> Note that this could involve more than just the pid; it could be any of
> the information in the NETLINK_CB or even the NETLINK_CREDS, so it could
> also include loginuid and possibly uid info.  You can propagate that
> data to avc_audit by extending the avc audit data structure, and either
> have it log it instead of the current task info or have it log it as
> supplemental data in addition to the current task info.  You could
> further propagate it to syscall audit by setting those values in the
> audit context from the netlink information, but it isn't clear that is
> what you want and I don't think that gives you the full task information
> that is presently logged by audit_log_exit.
> 
> The other approach is to move all checking to send time, including
> audit_netlink_ok and equivalent checking by other netlink handlers,
> using a callback mechanism as proposed in patches by Chris Wright a long
> time ago.  I'm not sure what happened with those.  Then the current task
> info is valid for checking and for auditing

Looks like they just died...

http://marc.theaimsgroup.com/?l=linux-netdev&m=110819902809549&w=2
http://marc.theaimsgroup.com/?l=linux-netdev&m=110819906824642&w=2
http://marc.theaimsgroup.com/?l=linux-netdev&m=110819919430147&w=2
http://marc.theaimsgroup.com/?l=linux-netdev&m=110819926630794&w=2

Seems like a nice idea (wish it would have been completed and upstreamed...).
I have no idea how this fits into the current state of the kernel or what
may have been missing from the original implementation.  My limited knowledge
of netlink puts this approach out of my league for now (would be nice to learn
more about this in the future though).  I'd be happy to see this in the future.

>>  I really
>>like the idea of getting the AVC message because tracking down cases where
>>SELinux was denying CAP_AUDIT_XXX was a huge pain when userspace was starting to
>>become audit-aware...
> 
> 
> Yes, and the same applies for CAP_NET_ADMIN check by various netlink
> handlers - we don't get any audit messages presently since they are
> checked at receive time.
> 
> 
>>This patch encapsulates the usage of eff_cap (in netlink_skb_params) within
>>the lsm framework by extending security_netlink_recv to include a required
>>capability parameter and converting all direct usage of eff_caps outside
>>of the lsm modules to use the interface.  It also updates the SELinux
>>implementation of the security_netlink_send and security_netlink_recv
>>hooks to take advantage of the sid in the netlink_skb_params struct.
> 
> 
> The patch looks sane to me.  I'm not as concerned about the audit
> confusion because the portions relevant to policy will be correct (i.e.
> the SIDs/contexts).

I am also not really concerned about it.  Shall we give this a whirl in the
lspp kernel (since its users are currently exercising the problematic code
path) and include push it through the audit git tree if it works nicely?
Note that this would replace the band-aid patch that is currently included
in the lspp kernel.  Anyone object (or have a patch using the callback
method)?

-- 

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.

Re: [PATCH] fix masking of capabilities over netlink in permissive mode

[Stephen Smalley]

On Tue, 2006-06-13 at 14:00 -0500, Darrel Goeddel wrote:
> I am also not really concerned about it.  Shall we give this a whirl in the
> lspp kernel (since its users are currently exercising the problematic code
> path) and include push it through the audit git tree if it works nicely?
> Note that this would replace the band-aid patch that is currently included
> in the lspp kernel.  Anyone object (or have a patch using the callback
> method)?

No objection from me.

-- 
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.