RE: [RFC] [PATCH 4/4] SELinux changes

RE: [RFC] [PATCH 4/4] SELinux changes

[Chad Hanson]

I agree performance could be an issue. I would suggest these checks are
performed based on value /counter instead of the presence of labeled
networking.

I agree it may not be intersting for most, thus the conditional check, but
certainly some people would like to make sure that traffic labeled via
fallback measures is validated per interface controls.  Venkat's example of
forwarding unlabeled packets is prime example of something that should be
enforced. 

-Chad
> 
> > Just for clarification James, what is the motivation for making the 
> > permission checks conditional?  Performance?  Compatibility?
> 
> Performance -- per packet checks which are not doing anything 
> for the vast majority of users are undesirable.
> 
> 
> -- 
> 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.

Re: [RFC] [PATCH 4/4] SELinux changes

[Paul Moore]

On Thursday, September 20 2007 2:50:55 pm Chad Hanson wrote:
> I agree performance could be an issue. I would suggest these checks are
> performed based on value /counter instead of the presence of labeled
> networking.

Personally I like the idea of using a counter/flag/toggle/etc to 
enable/disable the checks but I believe there might be push back due to the 
added user configuration burden (even though it would be rather small).

-- 
paul moore
linux security @ hp

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

[RFC] [PATCH 4/4] SELinux changes

[Venkat Yekkirala]

This implements the skb_flow_out LSM hook for SELinux. This
also defines a new forward_first netfilter hook to perform
flow-control of forwarded traffic on the way into the system.
Locally destined traffic is flow-controlled inside the existing
rcv_skb LSM hook.

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 3694662..5434d7f 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -3519,6 +3519,124 @@ static int selinux_socket_unix_may_send(struct socket *sock,
 	return 0;
 }
 
+static int selinux_skb_flow_in(struct sk_buff *skb, struct net_device *in,
+			       unsigned short family)
+{
+	u32 node_sid, if_sid, secid = SECSID_NULL;
+	int err;
+	struct avc_audit_data ad;
+	char *addrp;
+	int len;
+
+	if (!in) {
+		if (skb->dev && skb->dev->ifindex == skb->iif)
+			in = skb->dev;
+		else
+			in = __dev_get_by_index(skb->iif);
+
+		if (!in) {
+			err = -EACCES;
+			goto out;
+		}
+	}
+
+	AVC_AUDIT_DATA_INIT(&ad, NET);
+	ad.u.net.netif = in->name;
+	ad.u.net.family = family;
+	err = selinux_parse_skb(skb, &ad, &addrp, &len, 1, NULL);
+	if (err)
+		goto out;
+
+	if (in != &loopback_dev) { /* Non-localhost packet */
+		err = selinux_xfrm_decode_session(skb, &secid, 0);
+		BUG_ON(err);
+		/* TODO: Retrieve and check any NetLabel for agreement with
+		   any Xfrm; also retrieve fallback if necessary */
+	}
+#ifdef TODO
+	else /* localhost packet */
+		/* TODO: Retrieve special IP Option set for localhost traffic */
+#endif
+
+	err = security_node_sid(family, addrp, len, &node_sid);
+	if (err)
+		goto out;
+
+	err = avc_has_perm(secid, node_sid,
+				  SECCLASS_NODE,
+				  NODE__FLOW_IN, &ad);
+	if (err)
+		goto out;
+
+	/* See if skb can flow in thru the interface */
+	err = sel_netif_sids(in, &if_sid, NULL);
+	if (err)
+		goto out;
+
+	err = avc_has_perm(secid, if_sid,
+				  SECCLASS_NETIF,
+				  NETIF__FLOW_IN, &ad);
+
+out:
+	return err;
+};
+
+static int selinux_skb_flow_out(struct sk_buff *skb, int family)
+{
+	u32 node_sid, if_sid, secid = SECSID_NULL;
+	int err;
+	struct avc_audit_data ad;
+	char *addrp;
+	int len;
+	struct sock *sk;
+	struct sk_security_struct *sksec;
+
+	if (!skb->dev) {
+		err = -EACCES;
+		goto out;
+	}
+
+	AVC_AUDIT_DATA_INIT(&ad, NET);
+	ad.u.net.netif = skb->dev->name;
+	ad.u.net.family = family;
+	err = selinux_parse_skb(skb, &ad, &addrp, &len, 1, NULL);
+	if (err)
+		goto out;
+
+	/* TODO: Glean the secid for forwarded packets
+	   from any incoming xfrms, NetLabel/Fallback, etc. */
+
+	if (!secid) {
+		sk = skb->sk;
+		if (sk) {
+			sksec = sk->sk_security;
+			secid = sksec->sid;
+		}
+	}
+
+	err = security_node_sid(family, addrp, len, &node_sid);
+	if (err)
+		goto out;
+
+	err = avc_has_perm(secid, node_sid,
+				  SECCLASS_NODE,
+				  NODE__FLOW_OUT, &ad);
+	if (err)
+		goto out;
+
+	/* See if skb can flow out thru the interface */
+	err = sel_netif_sids(skb->dev, &if_sid, NULL);
+	if (err)
+		goto out;
+
+	err = avc_has_perm(secid, if_sid,
+				  SECCLASS_NETIF,
+				  NETIF__FLOW_OUT, &ad);
+
+out:
+	return err;
+}
+
 static int selinux_sock_rcv_skb_compat(struct sock *sk, struct sk_buff *skb,
 		struct avc_audit_data *ad, u16 family, char *addrp, int len)
 {
@@ -3629,10 +3747,14 @@ static int selinux_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
 	if (err)
 		goto out;
 
-	if (selinux_compat_net)
+	if (selinux_compat_net) {
+		err = selinux_skb_flow_in(skb, NULL, family);
+		if (err)
+			goto out;
+
 		err = selinux_sock_rcv_skb_compat(sk, skb, &ad, family,
 						  addrp, len);
-	else
+	} else
 		err = avc_has_perm(sksec->sid, skb->secmark, SECCLASS_PACKET,
 				   PACKET__RECV, &ad);
 	if (err)
@@ -3924,6 +4046,20 @@ out:
 	return err;
 }
 
+static unsigned int selinux_ip_forward_first(unsigned int hooknum,
+                                              struct sk_buff **pskb,
+                                              const struct net_device *in,
+                                              const struct net_device *out,
+                                              int (*okfn)(struct sk_buff *),
+                                              u16 family)
+{
+	if (selinux_skb_flow_in(*pskb, (struct net_device *)in, family))
+		return NF_DROP;
+
+	return NF_ACCEPT;
+}
+
+
 static unsigned int selinux_ip_postroute_last(unsigned int hooknum,
                                               struct sk_buff **pskb,
                                               const struct net_device *in,
@@ -3969,6 +4105,15 @@ out:
 	return err ? NF_DROP : NF_ACCEPT;
 }
 
+static unsigned int selinux_ipv4_forward_first(unsigned int hooknum,
+                                              struct sk_buff **pskb,
+                                              const struct net_device *in,
+                                              const struct net_device *out,
+                                              int (*okfn)(struct sk_buff *))
+{
+	return selinux_ip_forward_first(hooknum, pskb, in, out, okfn, PF_INET);
+}
+
 static unsigned int selinux_ipv4_postroute_last(unsigned int hooknum,
 						struct sk_buff **pskb,
 						const struct net_device *in,
@@ -3980,6 +4125,15 @@ static unsigned int selinux_ipv4_postroute_last(unsigned int hooknum,
 
 #if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
 
+static unsigned int selinux_ipv6_forward_first(unsigned int hooknum,
+                                              struct sk_buff **pskb,
+                                              const struct net_device *in,
+                                              const struct net_device *out,
+                                              int (*okfn)(struct sk_buff *))
+{
+	return selinux_ip_forward_first(hooknum, pskb, in, out, okfn, PF_INET6);
+}
+
 static unsigned int selinux_ipv6_postroute_last(unsigned int hooknum,
 						struct sk_buff **pskb,
 						const struct net_device *in,
@@ -4875,6 +5029,7 @@ static struct security_operations selinux_ops = {
 	.inet_csk_clone =		selinux_inet_csk_clone,
 	.inet_conn_established =	selinux_inet_conn_established,
 	.req_classify_flow =		selinux_req_classify_flow,
+	.skb_flow_out =			selinux_skb_flow_out,
 
 #ifdef CONFIG_SECURITY_NETWORK_XFRM
 	.xfrm_policy_alloc_security =	selinux_xfrm_policy_alloc,
@@ -4978,22 +5133,40 @@ security_initcall(selinux_init);
 
 #if defined(CONFIG_NETFILTER)
 
-static struct nf_hook_ops selinux_ipv4_op = {
+static struct nf_hook_ops selinux_ipv4_ops[] = {
+	{
 	.hook =		selinux_ipv4_postroute_last,
 	.owner =	THIS_MODULE,
 	.pf =		PF_INET,
 	.hooknum =	NF_IP_POST_ROUTING,
 	.priority =	NF_IP_PRI_SELINUX_LAST,
+	},
+	{
+	.hook =		selinux_ipv4_forward_first,
+	.owner =	THIS_MODULE,
+	.pf =		PF_INET,
+	.hooknum =	NF_IP_FORWARD,
+	.priority =	NF_IP_PRI_SELINUX_FIRST,
+	}
 };
 
 #if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
 
-static struct nf_hook_ops selinux_ipv6_op = {
+static struct nf_hook_ops selinux_ipv6_ops[] = {
+	{
 	.hook =		selinux_ipv6_postroute_last,
 	.owner =	THIS_MODULE,
 	.pf =		PF_INET6,
 	.hooknum =	NF_IP6_POST_ROUTING,
 	.priority =	NF_IP6_PRI_SELINUX_LAST,
+	},
+	{
+	.hook =		selinux_ipv6_forward_first,
+	.owner =	THIS_MODULE,
+	.pf =		PF_INET6,
+	.hooknum =	NF_IP6_FORWARD,
+	.priority =	NF_IP6_PRI_SELINUX_FIRST,
+	}
 };
 
 #endif	/* IPV6 */
@@ -5001,21 +5174,26 @@ static struct nf_hook_ops selinux_ipv6_op = {
 static int __init selinux_nf_ip_init(void)
 {
 	int err = 0;
+	int i;
 
 	if (!selinux_enabled)
 		goto out;
 
 	printk(KERN_DEBUG "SELinux:  Registering netfilter hooks\n");
 
-	err = nf_register_hook(&selinux_ipv4_op);
-	if (err)
-		panic("SELinux: nf_register_hook for IPv4: error %d\n", err);
+	for (i = 0; i < sizeof(selinux_ipv4_ops)/sizeof(struct nf_hook_ops); i++) {
+		err = nf_register_hook(&selinux_ipv4_ops[i]);
+		if (err)
+			panic("SELinux: nf_register_hook for IPv4: error %d\n", err);
+	}
 
 #if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
 
-	err = nf_register_hook(&selinux_ipv6_op);
-	if (err)
-		panic("SELinux: nf_register_hook for IPv6: error %d\n", err);
+	for (i = 0; i < sizeof(selinux_ipv6_ops)/sizeof(struct nf_hook_ops); i++) {
+		err = nf_register_hook(&selinux_ipv6_ops[i]);
+		if (err)
+			panic("SELinux: nf_register_hook for IPv6: error %d\n", err);
+	}
 
 #endif	/* IPV6 */
 
@@ -5028,11 +5206,14 @@ __initcall(selinux_nf_ip_init);
 #ifdef CONFIG_SECURITY_SELINUX_DISABLE
 static void selinux_nf_ip_exit(void)
 {
+	int i;
 	printk(KERN_DEBUG "SELinux:  Unregistering netfilter hooks\n");
 
-	nf_unregister_hook(&selinux_ipv4_op);
+	for (i = 0; i < sizeof(selinux_ipv4_ops)/sizeof(struct nf_hook_ops); i++)
+		nf_unregister_hook(&selinux_ipv4_ops[i]);
 #if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
-	nf_unregister_hook(&selinux_ipv6_op);
+	for (i = 0; i < sizeof(selinux_ipv6_ops)/sizeof(struct nf_hook_ops); i++)
+		nf_unregister_hook(&selinux_ipv6_ops[i]);
 #endif	/* IPV6 */
 }
 #endif

--
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: [RFC] [PATCH 4/4] SELinux changes

[Stephen Smalley]

On Tue, 2007-09-18 at 12:32 -0500, Venkat Yekkirala wrote:
> This implements the skb_flow_out LSM hook for SELinux. This
> also defines a new forward_first netfilter hook to perform
> flow-control of forwarded traffic on the way into the system.
> Locally destined traffic is flow-controlled inside the existing
> rcv_skb LSM hook.
> 
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 3694662..5434d7f 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -3519,6 +3519,124 @@ static int selinux_socket_unix_may_send(struct socket *sock,
>  	return 0;
>  }
>  
> +static int selinux_skb_flow_in(struct sk_buff *skb, struct net_device *in,
> +			       unsigned short family)
> +{
> +	u32 node_sid, if_sid, secid = SECSID_NULL;
> +	int err;
> +	struct avc_audit_data ad;
> +	char *addrp;
> +	int len;
> +
> +	if (!in) {
> +		if (skb->dev && skb->dev->ifindex == skb->iif)
> +			in = skb->dev;
> +		else
> +			in = __dev_get_by_index(skb->iif);
> +
> +		if (!in) {
> +			err = -EACCES;
> +			goto out;
> +		}
> +	}
> +
> +	AVC_AUDIT_DATA_INIT(&ad, NET);
> +	ad.u.net.netif = in->name;
> +	ad.u.net.family = family;
> +	err = selinux_parse_skb(skb, &ad, &addrp, &len, 1, NULL);
> +	if (err)
> +		goto out;
> +
> +	if (in != &loopback_dev) { /* Non-localhost packet */
> +		err = selinux_xfrm_decode_session(skb, &secid, 0);
> +		BUG_ON(err);
> +		/* TODO: Retrieve and check any NetLabel for agreement with
> +		   any Xfrm; also retrieve fallback if necessary */
> +	}
> +#ifdef TODO
> +	else /* localhost packet */
> +		/* TODO: Retrieve special IP Option set for localhost traffic */
> +#endif
> +
> +	err = security_node_sid(family, addrp, len, &node_sid);
> +	if (err)
> +		goto out;
> +
> +	err = avc_has_perm(secid, node_sid,
> +				  SECCLASS_NODE,
> +				  NODE__FLOW_IN, &ad);
> +	if (err)
> +		goto out;

Side note:  If we are going to keep using node SIDs in new network
controls (vs. just the compat ones), then we will need to a) introduce
some kind of node SID cache to avoid the overhead of policy lookup on
each packet, and b) extend semanage to manage node contexts.  There was
work on both in the past but nothing ever made it to completion (see
prior postings by Joy Latten and Rodrigo Vivi).

We thought we were eliminating the need for these per-packet
per-node/netif checks by way of secmark, but I guess not if we are
keeping secmark separate from labeled networking.

> +
> +	/* See if skb can flow in thru the interface */
> +	err = sel_netif_sids(in, &if_sid, NULL);
> +	if (err)
> +		goto out;
> +
> +	err = avc_has_perm(secid, if_sid,
> +				  SECCLASS_NETIF,
> +				  NETIF__FLOW_IN, &ad);
> +
> +out:
> +	return err;
> +};
> +
> +static int selinux_skb_flow_out(struct sk_buff *skb, int family)
> +{
> +	u32 node_sid, if_sid, secid = SECSID_NULL;
> +	int err;
> +	struct avc_audit_data ad;
> +	char *addrp;
> +	int len;
> +	struct sock *sk;
> +	struct sk_security_struct *sksec;
> +
> +	if (!skb->dev) {
> +		err = -EACCES;
> +		goto out;
> +	}
> +
> +	AVC_AUDIT_DATA_INIT(&ad, NET);
> +	ad.u.net.netif = skb->dev->name;
> +	ad.u.net.family = family;
> +	err = selinux_parse_skb(skb, &ad, &addrp, &len, 1, NULL);
> +	if (err)
> +		goto out;
> +
> +	/* TODO: Glean the secid for forwarded packets
> +	   from any incoming xfrms, NetLabel/Fallback, etc. */
> +
> +	if (!secid) {
> +		sk = skb->sk;
> +		if (sk) {
> +			sksec = sk->sk_security;
> +			secid = sksec->sid;
> +		}
> +	}
> +
> +	err = security_node_sid(family, addrp, len, &node_sid);
> +	if (err)
> +		goto out;
> +
> +	err = avc_has_perm(secid, node_sid,
> +				  SECCLASS_NODE,
> +				  NODE__FLOW_OUT, &ad);
> +	if (err)
> +		goto out;
> +
> +	/* See if skb can flow out thru the interface */
> +	err = sel_netif_sids(skb->dev, &if_sid, NULL);
> +	if (err)
> +		goto out;
> +
> +	err = avc_has_perm(secid, if_sid,
> +				  SECCLASS_NETIF,
> +				  NETIF__FLOW_OUT, &ad);
> +
> +out:
> +	return err;
> +}
> +
>  static int selinux_sock_rcv_skb_compat(struct sock *sk, struct sk_buff *skb,
>  		struct avc_audit_data *ad, u16 family, char *addrp, int len)
>  {
> @@ -3629,10 +3747,14 @@ static int selinux_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
>  	if (err)
>  		goto out;
>  
> -	if (selinux_compat_net)
> +	if (selinux_compat_net) {
> +		err = selinux_skb_flow_in(skb, NULL, family);
> +		if (err)
> +			goto out;
> +
>  		err = selinux_sock_rcv_skb_compat(sk, skb, &ad, family,
>  						  addrp, len);
> -	else
> +	} else
>  		err = avc_has_perm(sksec->sid, skb->secmark, SECCLASS_PACKET,
>  				   PACKET__RECV, &ad);
>  	if (err)
> @@ -3924,6 +4046,20 @@ out:
>  	return err;
>  }
>  
> +static unsigned int selinux_ip_forward_first(unsigned int hooknum,
> +                                              struct sk_buff **pskb,
> +                                              const struct net_device *in,
> +                                              const struct net_device *out,
> +                                              int (*okfn)(struct sk_buff *),
> +                                              u16 family)
> +{
> +	if (selinux_skb_flow_in(*pskb, (struct net_device *)in, family))
> +		return NF_DROP;
> +
> +	return NF_ACCEPT;
> +}
> +
> +
>  static unsigned int selinux_ip_postroute_last(unsigned int hooknum,
>                                                struct sk_buff **pskb,
>                                                const struct net_device *in,
> @@ -3969,6 +4105,15 @@ out:
>  	return err ? NF_DROP : NF_ACCEPT;
>  }
>  
> +static unsigned int selinux_ipv4_forward_first(unsigned int hooknum,
> +                                              struct sk_buff **pskb,
> +                                              const struct net_device *in,
> +                                              const struct net_device *out,
> +                                              int (*okfn)(struct sk_buff *))
> +{
> +	return selinux_ip_forward_first(hooknum, pskb, in, out, okfn, PF_INET);
> +}
> +
>  static unsigned int selinux_ipv4_postroute_last(unsigned int hooknum,
>  						struct sk_buff **pskb,
>  						const struct net_device *in,
> @@ -3980,6 +4125,15 @@ static unsigned int selinux_ipv4_postroute_last(unsigned int hooknum,
>  
>  #if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
>  
> +static unsigned int selinux_ipv6_forward_first(unsigned int hooknum,
> +                                              struct sk_buff **pskb,
> +                                              const struct net_device *in,
> +                                              const struct net_device *out,
> +                                              int (*okfn)(struct sk_buff *))
> +{
> +	return selinux_ip_forward_first(hooknum, pskb, in, out, okfn, PF_INET6);
> +}
> +
>  static unsigned int selinux_ipv6_postroute_last(unsigned int hooknum,
>  						struct sk_buff **pskb,
>  						const struct net_device *in,
> @@ -4875,6 +5029,7 @@ static struct security_operations selinux_ops = {
>  	.inet_csk_clone =		selinux_inet_csk_clone,
>  	.inet_conn_established =	selinux_inet_conn_established,
>  	.req_classify_flow =		selinux_req_classify_flow,
> +	.skb_flow_out =			selinux_skb_flow_out,
>  
>  #ifdef CONFIG_SECURITY_NETWORK_XFRM
>  	.xfrm_policy_alloc_security =	selinux_xfrm_policy_alloc,
> @@ -4978,22 +5133,40 @@ security_initcall(selinux_init);
>  
>  #if defined(CONFIG_NETFILTER)
>  
> -static struct nf_hook_ops selinux_ipv4_op = {
> +static struct nf_hook_ops selinux_ipv4_ops[] = {
> +	{
>  	.hook =		selinux_ipv4_postroute_last,
>  	.owner =	THIS_MODULE,
>  	.pf =		PF_INET,
>  	.hooknum =	NF_IP_POST_ROUTING,
>  	.priority =	NF_IP_PRI_SELINUX_LAST,
> +	},
> +	{
> +	.hook =		selinux_ipv4_forward_first,
> +	.owner =	THIS_MODULE,
> +	.pf =		PF_INET,
> +	.hooknum =	NF_IP_FORWARD,
> +	.priority =	NF_IP_PRI_SELINUX_FIRST,
> +	}
>  };
>  
>  #if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
>  
> -static struct nf_hook_ops selinux_ipv6_op = {
> +static struct nf_hook_ops selinux_ipv6_ops[] = {
> +	{
>  	.hook =		selinux_ipv6_postroute_last,
>  	.owner =	THIS_MODULE,
>  	.pf =		PF_INET6,
>  	.hooknum =	NF_IP6_POST_ROUTING,
>  	.priority =	NF_IP6_PRI_SELINUX_LAST,
> +	},
> +	{
> +	.hook =		selinux_ipv6_forward_first,
> +	.owner =	THIS_MODULE,
> +	.pf =		PF_INET6,
> +	.hooknum =	NF_IP6_FORWARD,
> +	.priority =	NF_IP6_PRI_SELINUX_FIRST,
> +	}
>  };
>  
>  #endif	/* IPV6 */
> @@ -5001,21 +5174,26 @@ static struct nf_hook_ops selinux_ipv6_op = {
>  static int __init selinux_nf_ip_init(void)
>  {
>  	int err = 0;
> +	int i;
>  
>  	if (!selinux_enabled)
>  		goto out;
>  
>  	printk(KERN_DEBUG "SELinux:  Registering netfilter hooks\n");
>  
> -	err = nf_register_hook(&selinux_ipv4_op);
> -	if (err)
> -		panic("SELinux: nf_register_hook for IPv4: error %d\n", err);
> +	for (i = 0; i < sizeof(selinux_ipv4_ops)/sizeof(struct nf_hook_ops); i++) {
> +		err = nf_register_hook(&selinux_ipv4_ops[i]);
> +		if (err)
> +			panic("SELinux: nf_register_hook for IPv4: error %d\n", err);
> +	}
>  
>  #if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
>  
> -	err = nf_register_hook(&selinux_ipv6_op);
> -	if (err)
> -		panic("SELinux: nf_register_hook for IPv6: error %d\n", err);
> +	for (i = 0; i < sizeof(selinux_ipv6_ops)/sizeof(struct nf_hook_ops); i++) {
> +		err = nf_register_hook(&selinux_ipv6_ops[i]);
> +		if (err)
> +			panic("SELinux: nf_register_hook for IPv6: error %d\n", err);
> +	}
>  
>  #endif	/* IPV6 */
>  
> @@ -5028,11 +5206,14 @@ __initcall(selinux_nf_ip_init);
>  #ifdef CONFIG_SECURITY_SELINUX_DISABLE
>  static void selinux_nf_ip_exit(void)
>  {
> +	int i;
>  	printk(KERN_DEBUG "SELinux:  Unregistering netfilter hooks\n");
>  
> -	nf_unregister_hook(&selinux_ipv4_op);
> +	for (i = 0; i < sizeof(selinux_ipv4_ops)/sizeof(struct nf_hook_ops); i++)
> +		nf_unregister_hook(&selinux_ipv4_ops[i]);
>  #if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
> -	nf_unregister_hook(&selinux_ipv6_op);
> +	for (i = 0; i < sizeof(selinux_ipv6_ops)/sizeof(struct nf_hook_ops); i++)
> +		nf_unregister_hook(&selinux_ipv6_ops[i]);
>  #endif	/* IPV6 */
>  }
>  #endif
-- 
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: [RFC] [PATCH 4/4] SELinux changes

[James Morris]

On Wed, 19 Sep 2007, Stephen Smalley wrote:

> We thought we were eliminating the need for these per-packet
> per-node/netif checks by way of secmark, but I guess not if we are
> keeping secmark separate from labeled networking.

The checks should only be made if labeled networking is active.

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

RE: [RFC] [PATCH 4/4] SELinux changes

[Venkatesh Yekkirala]

> -----Original Message-----
> From: James Morris [mailto:jmorris@namei.org]
> Sent: Wednesday, September 19, 2007 4:13 PM
> To: Stephen Smalley
> Cc: Venkat Yekkirala; selinux@tycho.nsa.gov; paul.moore@hp.com; Karl
> MacMillan; Joshua Brindle
> Subject: Re: [RFC] [PATCH 4/4] SELinux changes
> 
> 
> On Wed, 19 Sep 2007, Stephen Smalley wrote:
> 
> > We thought we were eliminating the need for these per-packet
> > per-node/netif checks by way of secmark, but I guess not if we are
> > keeping secmark separate from labeled networking.
> 
> The checks should only be made if labeled networking is active.

Actually even when we aren't using labeled networking, we would
want to prevent packets arriving on a top-secret interface from
being forwarded onto a secret interface. So, the checks would be
in order here as well.

--
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: [RFC] [PATCH 4/4] SELinux changes

[Paul Moore]

On Wednesday, September 19 2007 5:22:24 pm Venkatesh Yekkirala wrote:
> > -----Original Message-----
> > From: James Morris [mailto:jmorris@namei.org]
> > On Wed, 19 Sep 2007, Stephen Smalley wrote:
> > > We thought we were eliminating the need for these per-packet
> > > per-node/netif checks by way of secmark, but I guess not if we are
> > > keeping secmark separate from labeled networking.
> >
> > The checks should only be made if labeled networking is active.
>
> Actually even when we aren't using labeled networking, we would
> want to prevent packets arriving on a top-secret interface from
> being forwarded onto a secret interface. So, the checks would be
> in order here as well.

[Sorry to be quiet on the patches but I'm still looking/thinking]

Just for clarification James, what is the motivation for making the permission 
checks conditional?  Performance?  Compatibility?

Compatibility is an issue that we are going to have to deal with for both flow 
control and peer label reconciliation.  My current thinking is that we 
introduce a new functionality version flag which is set by the policy at load 
time and is used to determine code paths.  It's similar to the compat_net 
flag but more general purpose.

-- 
paul moore
linux security @ hp

--
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: [RFC] [PATCH 4/4] SELinux changes

[James Morris]

On Wed, 19 Sep 2007, Paul Moore wrote:

> Just for clarification James, what is the motivation for making the permission 
> checks conditional?  Performance?  Compatibility?

Performance -- per packet checks which are not doing anything for the vast 
majority of users are undesirable.


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

Re: [RFC] [PATCH 4/4] SELinux changes

[Paul Moore]

On Wednesday, September 19 2007 6:52:13 pm James Morris wrote:
> On Wed, 19 Sep 2007, Paul Moore wrote:
> > Just for clarification James, what is the motivation for making the
> > permission checks conditional?  Performance?  Compatibility?
>
> Performance -- per packet checks which are not doing anything for the vast
> majority of users are undesirable.

All right, we'll throw that requirement on the pile too ...

-- 
paul moore
linux security @ hp

--
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: [RFC] [PATCH 4/4] SELinux changes

[Venkatesh Yekkirala]

> [Sorry to be quiet on the patches but I'm still looking/thinking]

No problem. I also wanted to ping on any further thinking on using
the IP option space (versus split secmark) for carrying the loopback
label as well as the label when a forwarded packet has used NetLabel/cipso
when coming in, but is going out using a non-labeled (plain) IPsec tunnel.
In the latter case, we would have the label unavailable for use in
the outgoing filter checks unless the ip option in the inner "tunneled"
packet is copied into the outer "tunnel" packet as well. I suggested
using the special localhost IP option to carry this label, but stripping
it out right after the flow_out checks. But on further discussions here
on our end, it seems like this would be extremely fragile, even if made
somehow workable in all cases. For example, this could potentially fail
when using AH on the tunnel packet. Which all makes us believe going
the split secmark route might be the most reliable/robust route under
the circumstances.

I know we wanted to hash out the flow control stuff first which I believe
we have a good handle on at this point. So my above query.

--
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: [RFC] [PATCH 4/4] SELinux changes

[Paul Moore]

On Thursday, September 20 2007 10:42:29 am Venkatesh Yekkirala wrote:
> I know we wanted to hash out the flow control stuff first which I believe
> we have a good handle on at this point. So my above query.

I know you guys are very anxious to get this done, I appreciate that, but I'm 
not to the point where I consider the flow control bits sorted quite yet.  
You've done a great job with the patches and we are definitely moving forward 
but I think we still have a ways to go.

FWIW, my main concern right now is sorting out a way to get this all upstream 
in a manner which doesn't break anything and doesn't totally screw 
performance.  Until we can address these issues all of the new functionality 
is a no-go.  I'll send out more info later, but right now this is the list of 
items we need in descending priority:

 1.  SELinux functionality counter/flag
     (similar to compat_net but useable for more than just networking and
      the value would be a "version" not simply a boolean on/off)
 2a. Reconcile NetLabel and labeled IPsec labels into a single, unified peer
     label with a single avc_has_perm() call for each check, create new
     peer object class/permissions
 2b. Introduce runtime activation of new peer access checks
     (to solve James performance concerns - when no labeled networking
      configuration is present do not perform any peer label access checks)
 3.  Flow control hooks
 ?*.  Fallback peer labels
 ?*.  Loopback peer labels

 * the order of the last two probably isn't important

My current plan is to create a new labeled networking git tree where we can 
slowly collect all of these patches in one place for testing and when we are 
happy I'll push the whole mess up to James/Dave/etc.  

[Okay, I just created the repo but it's currently just an empty clone of
 Linus' tree] 
 * git://git.infradead.org/users/pcmoore/lblnet-2.6_testing

Now, that doesn't mean I'm going to make you wait on the flow control patches 
(we _are_ getting close here), I'll go ahead and merge them in to the testing 
tree so they are available while I work on the items #1 and #2.  I think once 
we get items #1 through #3 done and tested we can probably push those 
upstream for comment/review.

I'll get you some comments on your latest flow control patchset before the end 
of the week - I promise.

> No problem. I also wanted to ping on any further thinking on using
> the IP option space (versus split secmark) for carrying the loopback
> label as well as the label when a forwarded packet has used NetLabel/cipso
> when coming in, but is going out using a non-labeled (plain) IPsec tunnel.
> In the latter case, we would have the label unavailable for use in
> the outgoing filter checks unless the ip option in the inner "tunneled"
> packet is copied into the outer "tunnel" packet as well. I suggested
> using the special localhost IP option to carry this label, but stripping
> it out right after the flow_out checks. But on further discussions here
> on our end, it seems like this would be extremely fragile, even if made
> somehow workable in all cases. For example, this could potentially fail
> when using AH on the tunnel packet. Which all makes us believe going
> the split secmark route might be the most reliable/robust route under
> the circumstances.

Okay, let's see.  Like I said, I'm a little preoccupied with items #1 and #2 
right now, but you make a valid point.  Just to be clear, I consider 
splitting the secmark field to be a last resort option and while I'm not 
giving a NO vote on it, I want to make sure we have examined all of the other 
possibilities before going down that road.

-- 
paul moore
linux security @ hp

--
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: [RFC] [PATCH 4/4] SELinux changes

[Paul Moore]

On Thursday, September 20 2007 11:31:50 am Paul Moore wrote:
> On Thursday, September 20 2007 10:42:29 am Venkatesh Yekkirala wrote:
> > No problem. I also wanted to ping on any further thinking on using
> > the IP option space (versus split secmark) for carrying the loopback
> > label as well as the label when a forwarded packet has used
> > NetLabel/cipso when coming in, but is going out using a non-labeled
> > (plain) IPsec tunnel. In the latter case, we would have the label
> > unavailable for use in the outgoing filter checks unless the ip option in
> > the inner "tunneled" packet is copied into the outer "tunnel" packet as
> > well. I suggested using the special localhost IP option to carry this
> > label, but stripping it out right after the flow_out checks. But on
> > further discussions here on our end, it seems like this would be
> > extremely fragile, even if made somehow workable in all cases. For
> > example, this could potentially fail when using AH on the tunnel packet.
> > Which all makes us believe going the split secmark route might be the
> > most reliable/robust route under the circumstances.
>
> Okay, let's see.  Like I said, I'm a little preoccupied with items #1 and
> #2 right now, but you make a valid point.  Just to be clear, I consider
> splitting the secmark field to be a last resort option and while I'm not
> giving a NO vote on it, I want to make sure we have examined all of the
> other possibilities before going down that road.

If it's only an issue for forwarding, it might be possible to add another 
entry to the inet[6]_skb_param struct to store the peer label.  According to 
my quick calculations both structs are under the 48 byte limit.

-- 
paul moore
linux security @ hp

--
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: [RFC] [PATCH 4/4] SELinux changes

[Venkatesh Yekkirala]

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

> -----Original Message-----
> From: Stephen Smalley [mailto:sds@tycho.nsa.gov]
> Sent: Wednesday, September 19, 2007 9:18 AM
> To: Venkat Yekkirala
> Cc: selinux@tycho.nsa.gov; paul.moore@hp.com; 
> jmorris@namei.org; Karl MacMillan; Joshua Brindle
> Subject: Re: [RFC] [PATCH 4/4] SELinux changes
> 
> 
> On Tue, 2007-09-18 at 12:32 -0500, Venkat Yekkirala wrote: 
> > This implements the skb_flow_out LSM hook for SELinux. This 
> > also defines a new forward_first netfilter hook to perform 
> > flow-control of forwarded traffic on the way into the system. 
> > Locally destined traffic is flow-controlled inside the existing 
> > rcv_skb LSM hook. 
> > 
> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c 
> > index 3694662..5434d7f 100644 
> > --- a/security/selinux/hooks.c 
> > +++ b/security/selinux/hooks.c 
> > @@ -3519,6 +3519,124 @@ static int 
> selinux_socket_unix_may_send(struct socket *sock, 
> >       return 0; 
> >  } 
> >  
> > +static int selinux_skb_flow_in(struct sk_buff *skb, struct 
> net_device *in, 
> > +                            unsigned short family) 
> > +{ 
> > +     u32 node_sid, if_sid, secid = SECSID_NULL; 
> > +     int err; 
> > +     struct avc_audit_data ad; 
> > +     char *addrp; 
> > +     int len; 
> > + 
> > +     if (!in) { 
> > +             if (skb->dev && skb->dev->ifindex == skb->iif) 
> > +                     in = skb->dev; 
> > +             else 
> > +                     in = __dev_get_by_index(skb->iif); 
> > + 
> > +             if (!in) { 
> > +                     err = -EACCES; 
> > +                     goto out; 
> > +             } 
> > +     } 
> > + 
> > +     AVC_AUDIT_DATA_INIT(&ad, NET); 
> > +     ad.u.net.netif = in->name; 
> > +     ad.u.net.family = family; 
> > +     err = selinux_parse_skb(skb, &ad, &addrp, &len, 1, NULL); 
> > +     if (err) 
> > +             goto out; 
> > + 
> > +     if (in != &loopback_dev) { /* Non-localhost packet */ 
> > +             err = selinux_xfrm_decode_session(skb, &secid, 0); 
> > +             BUG_ON(err); 
> > +             /* TODO: Retrieve and check any NetLabel for 
> agreement with 
> > +                any Xfrm; also retrieve fallback if necessary */ 
> > +     } 
> > +#ifdef TODO 
> > +     else /* localhost packet */ 
> > +             /* TODO: Retrieve special IP Option set for 
> localhost traffic */ 
> > +#endif 
> > + 
> > +     err = security_node_sid(family, addrp, len, &node_sid); 
> > +     if (err) 
> > +             goto out; 
> > + 
> > +     err = avc_has_perm(secid, node_sid, 
> > +                               SECCLASS_NODE, 
> > +                               NODE__FLOW_IN, &ad); 
> > +     if (err) 
> > +             goto out; 
> Side note:  If we are going to keep using node SIDs in new network 
> controls (vs. just the compat ones), then we will need to a) 
> introduce 
> some kind of node SID cache to avoid the overhead of policy lookup on 
> each packet, and b) extend semanage to manage node contexts.  
> There was 
> work on both in the past but nothing ever made it to completion (see 
> prior postings by Joy Latten and Rodrigo Vivi). 

Paul once wondered if it made sense to replace the individual netif
and node flow lookup/checks with a single interface/network based
label lookup and check. I initially felt it made sense but I was
discussing this with Chad and Darrel this afternoon
and the thinking on this end is that it would be best to leave the
boundary-defining labels in the policy itself. So unless we want to
invent a way to define and lookup the interface/network labels in policy,
we could continue with the individual checks. In which case, we will
certainly need to work on the 2 issues you mention above.

Also, another idea that has come up here is to make the default message
sid on netif's useable again and make them fallbacks to the NetLabel
fallbacks. So the resolution, in order of priority would be:

1. NetLabel(external/cipso)/Xfrm
2. NetLabel Fallback
3. netif default context
4. Unlabeled

> We thought we were eliminating the need for these per-packet 
> per-node/netif checks by way of secmark, but I guess not if we are 
> keeping secmark separate from labeled networking. 

At least that's my current understanding of what we were going to do
(keeping secmark separate).

[-- Attachment #2: winmail.dat --]
[-- Type: application/ms-tnef, Size: 3952 bytes --]

Re: [RFC] [PATCH 4/4] SELinux changes

[Paul Moore]

On Wednesday, September 19 2007 5:20:05 pm Venkatesh Yekkirala wrote:
> > -----Original Message-----
> > From: Stephen Smalley [mailto:sds@tycho.nsa.gov]
> >
> > Side note:  If we are going to keep using node SIDs in new network
> > controls (vs. just the compat ones), then we will need to a)
> > introduce
> > some kind of node SID cache to avoid the overhead of policy lookup on
> > each packet, and b) extend semanage to manage node contexts.
> > There was
> > work on both in the past but nothing ever made it to completion (see
> > prior postings by Joy Latten and Rodrigo Vivi).
>
> Paul once wondered if it made sense to replace the individual netif
> and node flow lookup/checks with a single interface/network based
> label lookup and check. I initially felt it made sense but I was
> discussing this with Chad and Darrel this afternoon
> and the thinking on this end is that it would be best to leave the
> boundary-defining labels in the policy itself.

Okay, just a thought.

> Also, another idea that has come up here is to make the default message
> sid on netif's useable again and make them fallbacks to the NetLabel
> fallbacks. So the resolution, in order of priority would be:
>
> 1. NetLabel(external/cipso)/Xfrm
> 2. NetLabel Fallback
> 3. netif default context
> 4. Unlabeled

I vote a strong NO on this, multiple fallback peer labels is a bad, confusing 
idea.  However, let's try to stay focused here and stick with flow control 
right now; we can tackle this later.

> > We thought we were eliminating the need for these per-packet
> > per-node/netif checks by way of secmark, but I guess not if we are
> > keeping secmark separate from labeled networking.
>
> At least that's my current understanding of what we were going to do
> (keeping secmark separate).

Yes, SECMARK is separate.  Let's pleeease not go down this road again right 
now.

SECMARK replaced the node/netif lookups when checking the socket against the 
node/netif.  The flow control checks use the node/netif lookups to check the 
peer label against the node/netif.

-- 
paul moore
linux security @ hp

--
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: [RFC] [PATCH 4/4] SELinux changes

[Paul Moore]

On Tuesday, September 18 2007 1:32:28 pm Venkat Yekkirala wrote:
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 3694662..5434d7f 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -3519,6 +3519,124 @@ static int selinux_socket_unix_may_send(struct
> socket *sock, return 0;
>  }
>
> +static int selinux_skb_flow_in(struct sk_buff *skb, struct net_device *in,
> +			       unsigned short family)
> +{
> +	u32 node_sid, if_sid, secid = SECSID_NULL;
> +	int err;
> +	struct avc_audit_data ad;
> +	char *addrp;
> +	int len;
> +
> +	if (!in) {
> +		if (skb->dev && skb->dev->ifindex == skb->iif)

When, if ever, will the skb->dev->ifindex != skb->iif?  Doesn't it have to be 
the same?

> +			in = skb->dev;

Please don't reuse the "in" parameter here (or below), create a new variable 
("dev" perhaps?).  I know this is a nit, but I think it helps make things 
easier to read/debug.

> +		else
> +			in = __dev_get_by_index(skb->iif);

Are you certain it's safe to use __dev_get_by_index() here?  I have a hunch we 
need to use the regular dev_get_by_index() function and the matching 
dev_put() call later.

> +		if (!in) {
> +			err = -EACCES;
> +			goto out;
> +		}
> +	}
> +	AVC_AUDIT_DATA_INIT(&ad, NET);
> +	ad.u.net.netif = in->name;
> +	ad.u.net.family = family;
> +	err = selinux_parse_skb(skb, &ad, &addrp, &len, 1, NULL);
> +	if (err)
> +		goto out;
> +
> +	if (in != &loopback_dev) { /* Non-localhost packet */
> +		err = selinux_xfrm_decode_session(skb, &secid, 0);
> +		BUG_ON(err);
> +		/* TODO: Retrieve and check any NetLabel for agreement with
> +		   any Xfrm; also retrieve fallback if necessary */
> +	}
> +#ifdef TODO
> +	else /* localhost packet */
> +		/* TODO: Retrieve special IP Option set for localhost traffic */
> +#endif

Remove the check for loopback and the #ifdef ... #endif block, this stuff 
should/will be handled in a different set of patches.  Stick with the 
selinux_xfrm_decode_session() call and the TODO comment for right now.

> +	err = security_node_sid(family, addrp, len, &node_sid);
> +	if (err)
> +		goto out;
> +
> +	err = avc_has_perm(secid, node_sid,
> +				  SECCLASS_NODE,
> +				  NODE__FLOW_IN, &ad);
> +	if (err)
> +		goto out;
> +
> +	/* See if skb can flow in thru the interface */
> +	err = sel_netif_sids(in, &if_sid, NULL);
> +	if (err)
> +		goto out;
> +
> +	err = avc_has_perm(secid, if_sid,
> +				  SECCLASS_NETIF,
> +				  NETIF__FLOW_IN, &ad);

Move the NETIF check in front of the NODE check.  I think it makes more sense 
conceptually to check the interface first.

> +out:
> +	return err;
> +};
> +
> +static int selinux_skb_flow_out(struct sk_buff *skb, int family)

If we change the other functions from "flow_out" to "output" it might make 
sense to do the same here and below.

> +{
> +	u32 node_sid, if_sid, secid = SECSID_NULL;
> +	int err;
> +	struct avc_audit_data ad;
> +	char *addrp;
> +	int len;
> +	struct sock *sk;
> +	struct sk_security_struct *sksec;
> +
> +	if (!skb->dev) {
> +		err = -EACCES;
> +		goto out;
> +	}
> +
> +	AVC_AUDIT_DATA_INIT(&ad, NET);
> +	ad.u.net.netif = skb->dev->name;
> +	ad.u.net.family = family;
> +	err = selinux_parse_skb(skb, &ad, &addrp, &len, 1, NULL);
> +	if (err)
> +		goto out;
> +
> +	/* TODO: Glean the secid for forwarded packets
> +	   from any incoming xfrms, NetLabel/Fallback, etc. */
> +
> +	if (!secid) {

Make use of the #define where possible, change this to:

	if (secid != SECSID_NULL) {

> +		sk = skb->sk;
> +		if (sk) {
> +			sksec = sk->sk_security;
> +			secid = sksec->sid;
> +		}
> +	}
> +
> +	err = security_node_sid(family, addrp, len, &node_sid);
> +	if (err)
> +		goto out;
> +
> +	err = avc_has_perm(secid, node_sid,
> +				  SECCLASS_NODE,
> +				  NODE__FLOW_OUT, &ad);
> +	if (err)
> +		goto out;
> +
> +	/* See if skb can flow out thru the interface */
> +	err = sel_netif_sids(skb->dev, &if_sid, NULL);
> +	if (err)
> +		goto out;
> +
> +	err = avc_has_perm(secid, if_sid,
> +				  SECCLASS_NETIF,
> +				  NETIF__FLOW_OUT, &ad);

Here I think it is "correct" to check the node first as you are currently 
doing.

> +out:
> +	return err;
> +}
> +
>  static int selinux_sock_rcv_skb_compat(struct sock *sk, struct sk_buff
> *skb, struct avc_audit_data *ad, u16 family, char *addrp, int len)
>  {
> @@ -3629,10 +3747,14 @@ static int selinux_socket_sock_rcv_skb(struct sock
> *sk, struct sk_buff *skb) if (err)
>  		goto out;
>
> -	if (selinux_compat_net)
> +	if (selinux_compat_net) {
> +		err = selinux_skb_flow_in(skb, NULL, family);
> +		if (err)
> +			goto out;

You just lost me here ... why only do the flow_in check when compat_net == 1?

>  		err = selinux_sock_rcv_skb_compat(sk, skb, &ad, family,
>  						  addrp, len);
> -	else
> +	} else
>  		err = avc_has_perm(sksec->sid, skb->secmark, SECCLASS_PACKET,
>  				   PACKET__RECV, &ad);
>  	if (err)
> @@ -3924,6 +4046,20 @@ out:
>  	return err;
>  }
>
> +static unsigned int selinux_ip_forward_first(unsigned int hooknum,
> +                                              struct sk_buff **pskb,
> +                                              const struct net_device *in,
> +                                              const struct net_device
> *out, +                                              int (*okfn)(struct
> sk_buff *), +                                              u16 family)
> +{
> +	if (selinux_skb_flow_in(*pskb, (struct net_device *)in, family))
> +		return NF_DROP;
> +
> +	return NF_ACCEPT;
> +}
> +
> +
>  static unsigned int selinux_ip_postroute_last(unsigned int hooknum,
>                                                struct sk_buff **pskb,
>                                                const struct net_device *in,
> @@ -3969,6 +4105,15 @@ out:
>  	return err ? NF_DROP : NF_ACCEPT;
>  }
>
> +static unsigned int selinux_ipv4_forward_first(unsigned int hooknum,
> +                                              struct sk_buff **pskb,
> +                                              const struct net_device *in,
> +                                              const struct net_device
> *out, +                                              int (*okfn)(struct
> sk_buff *)) +{
> +	return selinux_ip_forward_first(hooknum, pskb, in, out, okfn, PF_INET);
> +}
> +
>  static unsigned int selinux_ipv4_postroute_last(unsigned int hooknum,
>  						struct sk_buff **pskb,
>  						const struct net_device *in,
> @@ -3980,6 +4125,15 @@ static unsigned int
> selinux_ipv4_postroute_last(unsigned int hooknum,
>
>  #if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
>
> +static unsigned int selinux_ipv6_forward_first(unsigned int hooknum,
> +                                              struct sk_buff **pskb,
> +                                              const struct net_device *in,
> +                                              const struct net_device
> *out, +                                              int (*okfn)(struct
> sk_buff *)) +{
> +	return selinux_ip_forward_first(hooknum, pskb, in, out, okfn, PF_INET6);
> +}
> +
>  static unsigned int selinux_ipv6_postroute_last(unsigned int hooknum,
>  						struct sk_buff **pskb,
>  						const struct net_device *in,
> @@ -4875,6 +5029,7 @@ static struct security_operations selinux_ops = {
>  	.inet_csk_clone =		selinux_inet_csk_clone,
>  	.inet_conn_established =	selinux_inet_conn_established,
>  	.req_classify_flow =		selinux_req_classify_flow,
> +	.skb_flow_out =			selinux_skb_flow_out,
>
>  #ifdef CONFIG_SECURITY_NETWORK_XFRM
>  	.xfrm_policy_alloc_security =	selinux_xfrm_policy_alloc,
> @@ -4978,22 +5133,40 @@ security_initcall(selinux_init);
>
>  #if defined(CONFIG_NETFILTER)
>
> -static struct nf_hook_ops selinux_ipv4_op = {
> +static struct nf_hook_ops selinux_ipv4_ops[] = {
> +	{
>  	.hook =		selinux_ipv4_postroute_last,
>  	.owner =	THIS_MODULE,
>  	.pf =		PF_INET,
>  	.hooknum =	NF_IP_POST_ROUTING,
>  	.priority =	NF_IP_PRI_SELINUX_LAST,
> +	},
> +	{
> +	.hook =		selinux_ipv4_forward_first,
> +	.owner =	THIS_MODULE,
> +	.pf =		PF_INET,
> +	.hooknum =	NF_IP_FORWARD,
> +	.priority =	NF_IP_PRI_SELINUX_FIRST,
> +	}
>  };
>
>  #if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
>
> -static struct nf_hook_ops selinux_ipv6_op = {
> +static struct nf_hook_ops selinux_ipv6_ops[] = {
> +	{
>  	.hook =		selinux_ipv6_postroute_last,
>  	.owner =	THIS_MODULE,
>  	.pf =		PF_INET6,
>  	.hooknum =	NF_IP6_POST_ROUTING,
>  	.priority =	NF_IP6_PRI_SELINUX_LAST,
> +	},
> +	{
> +	.hook =		selinux_ipv6_forward_first,
> +	.owner =	THIS_MODULE,
> +	.pf =		PF_INET6,
> +	.hooknum =	NF_IP6_FORWARD,
> +	.priority =	NF_IP6_PRI_SELINUX_FIRST,
> +	}
>  };
>
>  #endif	/* IPV6 */
> @@ -5001,21 +5174,26 @@ static struct nf_hook_ops selinux_ipv6_op = {
>  static int __init selinux_nf_ip_init(void)
>  {
>  	int err = 0;
> +	int i;
>
>  	if (!selinux_enabled)
>  		goto out;
>
>  	printk(KERN_DEBUG "SELinux:  Registering netfilter hooks\n");
>
> -	err = nf_register_hook(&selinux_ipv4_op);
> -	if (err)
> -		panic("SELinux: nf_register_hook for IPv4: error %d\n", err);
> +	for (i = 0; i < sizeof(selinux_ipv4_ops)/sizeof(struct nf_hook_ops); i++)
> { +		err = nf_register_hook(&selinux_ipv4_ops[i]);
> +		if (err)
> +			panic("SELinux: nf_register_hook for IPv4: error %d\n", err);
> +	}
>
>  #if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
>
> -	err = nf_register_hook(&selinux_ipv6_op);
> -	if (err)
> -		panic("SELinux: nf_register_hook for IPv6: error %d\n", err);
> +	for (i = 0; i < sizeof(selinux_ipv6_ops)/sizeof(struct nf_hook_ops); i++)
> { +		err = nf_register_hook(&selinux_ipv6_ops[i]);
> +		if (err)
> +			panic("SELinux: nf_register_hook for IPv6: error %d\n", err);
> +	}
>
>  #endif	/* IPV6 */
>
> @@ -5028,11 +5206,14 @@ __initcall(selinux_nf_ip_init);
>  #ifdef CONFIG_SECURITY_SELINUX_DISABLE
>  static void selinux_nf_ip_exit(void)
>  {
> +	int i;
>  	printk(KERN_DEBUG "SELinux:  Unregistering netfilter hooks\n");
>
> -	nf_unregister_hook(&selinux_ipv4_op);
> +	for (i = 0; i < sizeof(selinux_ipv4_ops)/sizeof(struct nf_hook_ops); i++)
> +		nf_unregister_hook(&selinux_ipv4_ops[i]);
>  #if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
> -	nf_unregister_hook(&selinux_ipv6_op);
> +	for (i = 0; i < sizeof(selinux_ipv6_ops)/sizeof(struct nf_hook_ops); i++)
> +		nf_unregister_hook(&selinux_ipv6_ops[i]);
>  #endif	/* IPV6 */
>  }
>  #endif

Once we sort these changes/issues out send me a fresh set of patches backed 
against Linus' tree and I'll put them in the lblnet testing tree so we can 
start collecting all of these things in one place.

Thanks for sticking with this.

-- 
paul moore
linux security @ hp

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