secid reconciliation-v02: Invoke LSM hook for outbound traffic

Invoke the skb_netfilter_check LSM hook for outbound (OUTPUT/FORWARD) traffic for secid reconciliation and flow control.

---

net/netfilter/xt_CONNSECMARK.c |   44 ++++++++++++++++++++++---------
net/netfilter/xt_SECMARK.c     |   20 ++++++++++++--
2 files changed, 50 insertions(+), 14 deletions(-)

index 4673862..a79bd20 100644
@@ -17,6 +17,8 @@
 */

#include <linux/module.h>
#include <linux/skbuff.h>
+#include <linux/security.h>
+#include <linux/netfilter_ipv6.h>
#include <linux/netfilter/x_tables.h>
#include <linux/netfilter/xt_CONNSECMARK.h> #include <net/netfilter/nf_conntrack_compat.h>
@@ -47,20 +49,32 @@ static void secmark_save(struct sk_buff }

/*
- * If packet has no security mark, and the connection does, restore the
- * security mark from the connection to the packet.
+ * On the inbound, restore the security mark from the connection to the packet.
+ * On the outbound, filter based on the current secmark.
*/
-static void secmark_restore(struct sk_buff *skb)
+static unsigned int secmark_restore(struct sk_buff *skb, unsigned int hooknum,
+ const struct xt_target *target)
{
- if (!skb->secmark) {
- u32 *connsecmark;
- enum ip_conntrack_info ctinfo;
+ u32 *psecmark;
+ u32 secmark = 0;
+ enum ip_conntrack_info ctinfo;

- connsecmark = nf_ct_get_secmark(skb, &ctinfo);
- if (connsecmark && *connsecmark)
- if (skb->secmark != *connsecmark)
- skb->secmark = *connsecmark;
- }
+ psecmark = nf_ct_get_secmark(skb, &ctinfo);
+ if (psecmark)
+ secmark = *psecmark;
+
+ if (!secmark)
+ return XT_CONTINUE;
+
+ /* Set secmark on inbound and filter it on outbound */
+ if (hooknum == NF_IP_POST_ROUTING || hooknum == NF_IP6_POST_ROUTING) {
+ if (!security_skb_netfilter_check(skb, secmark))
+ return NF_DROP;
+ } else
+ if (skb->secmark != secmark)
+ skb->secmark = secmark;
+
+ return XT_CONTINUE;
}

static unsigned int target(struct sk_buff **pskb, const struct net_device *in, @@ -77,7 +91,7 @@ static unsigned int target(struct sk_buf

		break;

	case CONNSECMARK_RESTORE:
-		secmark_restore(skb);
+		return secmark_restore(skb, hooknum, target);
		break;

	default:
@@ -114,6 +128,9 @@ static struct xt_target xt_connsecmark_t
		.target		= target,
		.targetsize	= sizeof(struct xt_connsecmark_target_info),
		.table		= "mangle",
+		.hooks		= (1 << NF_IP_LOCAL_IN) |
+				  (1 << NF_IP_FORWARD) |
+				  (1 << NF_IP_POST_ROUTING),
		.me		= THIS_MODULE,
	},
	{
@@ -123,6 +140,9 @@ static struct xt_target xt_connsecmark_t
		.target		= target,
		.targetsize	= sizeof(struct xt_connsecmark_target_info),
		.table		= "mangle",
+		.hooks		= (1 << NF_IP6_LOCAL_IN) |
+				  (1 << NF_IP6_FORWARD) |
+				  (1 << NF_IP6_POST_ROUTING),
		.me		= THIS_MODULE,
	},
};

index add7521..de1de45 100644
@@ -15,8 +15,10 @@

#include <linux/module.h>
#include <linux/skbuff.h>
#include <linux/selinux.h>
+#include <linux/security.h>
#include <linux/netfilter/x_tables.h>
#include <linux/netfilter/xt_SECMARK.h>
+#include <linux/netfilter_ipv6.h>

MODULE_LICENSE("GPL");
MODULE_AUTHOR("James Morris <[email protected]>"); @@ -47,8 +49,16 @@ static unsigned int target(struct sk_buf

		BUG();
	}

- if ((*pskb)->secmark != secmark)
- (*pskb)->secmark = secmark;
+ if (!secmark)
+ return XT_CONTINUE;
+
+ /* Set secmark on inbound and filter it on outbound */
+ if (hooknum == NF_IP_POST_ROUTING || hooknum == NF_IP6_POST_ROUTING) {
+ if (!security_skb_netfilter_check(*pskb, secmark))
+ return NF_DROP;
+ } else
+ if ((*pskb)->secmark != secmark)
+ (*pskb)->secmark = secmark;

	return XT_CONTINUE;
}
@@ -119,6 +129,9 @@ static struct xt_target xt_secmark_targe
		.target		= target,
		.targetsize	= sizeof(struct xt_secmark_target_info),
		.table		= "mangle",
+		.hooks		= (1 << NF_IP_LOCAL_IN) |
+				  (1 << NF_IP_FORWARD) |
+				  (1 << NF_IP_POST_ROUTING),
		.me		= THIS_MODULE,
	},
	{
@@ -128,6 +141,9 @@ static struct xt_target xt_secmark_targe
		.target		= target,
		.targetsize	= sizeof(struct xt_secmark_target_info),
		.table		= "mangle",
+		.hooks		= (1 << NF_IP6_LOCAL_IN) |
+				  (1 << NF_IP6_FORWARD) |
+				  (1 << NF_IP6_POST_ROUTING),
		.me		= THIS_MODULE,
	},
};

--
This message was distributed to subscribers of the selinux mailing list. If you no longer wish to subscribe, send mail to [email protected] with the words "unsubscribe selinux" without quotes as the message.

Quite a lot of logic has changed here.

With the original code, we only restored a secmark once for the lifetime of a packet or connetcion (to make behavior deterministic and security marks immutable in the face of arbitrarily complex iptables rules).

With your patch, secmarks are always writable.

What about packets on the OUTPUT hook?

Also, we did not restore a 'null' (zero) secmark to the skb (while this should never happen with the current SECMARK target, there may be non-SELinux extensions later which set a null marking).

Why not just do something like:

	psecmark = nf_ct_get_secmark(skb, &ctinfo);
	if (psecmark && *psecmark) {

		... core of function ...

	}

	return XT_CONTINUE;

I don't think you need the new secmark variable.

You've also changed the logic for the dummy case of security_skb_netfilter_check()

+static inline int security_skb_netfilter_check(struct sk_buff *skb,
+ u32 nf_secid)
+{
+ return 1;
+}
+

This code does not now behave as it did originally. Keep in mind that SELinux is not the only user of SECMARK.

(The documentation of the hook in security.h doesn't match the behavior, either -- it's (re-)labeling, not just filtering).

I really don't know if connection tracking is the right place to be doing policy enforcment, either. Perhaps you should just do the relabeling here and enforcement later.

The xt_SECMARK.c case has similar issues to all of the above.

- James

James Morris
<[email protected]>