* [PATCH 4/7] secid reconciliation-v02: Invoke LSM hook for outbound traffic
@ 2006-09-08 16:50 Venkat Yekkirala
2006-09-18 18:19 ` James Morris
2006-09-18 19:12 ` James Morris
0 siblings, 2 replies; 3+ messages in thread
From: Venkat Yekkirala @ 2006-09-08 16:50 UTC (permalink / raw)
To: netdev, selinux; +Cc: jmorris, sds, chanson
Invoke the skb_netfilter_check LSM hook for outbound (OUTPUT/FORWARD)
traffic for secid reconciliation and flow control.
Signed-off-by: Venkat Yekkirala <vyekkirala@TrustedCS.com>
---
net/netfilter/xt_CONNSECMARK.c | 44 ++++++++++++++++++++++---------
net/netfilter/xt_SECMARK.c | 20 ++++++++++++--
2 files changed, 50 insertions(+), 14 deletions(-)
diff --git a/net/netfilter/xt_CONNSECMARK.c b/net/netfilter/xt_CONNSECMARK.c
index 4673862..a79bd20 100644
--- a/net/netfilter/xt_CONNSECMARK.c
+++ b/net/netfilter/xt_CONNSECMARK.c
@@ -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,
},
};
diff --git a/net/netfilter/xt_SECMARK.c b/net/netfilter/xt_SECMARK.c
index add7521..de1de45 100644
--- a/net/netfilter/xt_SECMARK.c
+++ b/net/netfilter/xt_SECMARK.c
@@ -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 <jmorris@redhat.com>");
@@ -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,
},
};
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH 4/7] secid reconciliation-v02: Invoke LSM hook for outbound traffic
2006-09-08 16:50 [PATCH 4/7] secid reconciliation-v02: Invoke LSM hook for outbound traffic Venkat Yekkirala
@ 2006-09-18 18:19 ` James Morris
2006-09-18 19:12 ` James Morris
1 sibling, 0 replies; 3+ messages in thread
From: James Morris @ 2006-09-18 18:19 UTC (permalink / raw)
To: Venkat Yekkirala; +Cc: netdev, selinux, sds, chanson
On Fri, 8 Sep 2006, Venkat Yekkirala wrote:
> @@ -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),
Why have you added constraints on the hooks?
This breaks a bunch of things.
> @@ -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,
Ditto...
> @@ -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,
> },
> };
>
--
James Morris
<jmorris@namei.org>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 4/7] secid reconciliation-v02: Invoke LSM hook for outbound traffic
2006-09-08 16:50 [PATCH 4/7] secid reconciliation-v02: Invoke LSM hook for outbound traffic Venkat Yekkirala
2006-09-18 18:19 ` James Morris
@ 2006-09-18 19:12 ` James Morris
1 sibling, 0 replies; 3+ messages in thread
From: James Morris @ 2006-09-18 19:12 UTC (permalink / raw)
To: Venkat Yekkirala; +Cc: netdev, selinux, sds, chanson
On Fri, 8 Sep 2006, Venkat Yekkirala wrote:
> -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;
> }
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
<jmorris@namei.org>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2006-09-18 19:12 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-09-08 16:50 [PATCH 4/7] secid reconciliation-v02: Invoke LSM hook for outbound traffic Venkat Yekkirala
2006-09-18 18:19 ` James Morris
2006-09-18 19:12 ` James Morris
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).