Re: [PATCH 4/7] secid reconciliation-v03: Invoke LSM hook for outbound traffic

[James Morris]

On Thu, 28 Sep 2006, Venkat Yekkirala wrote:

> + }
> + else/* inbound */
> + /* loopback traffic should already be labeled
> +and any filtering on outbound should suffice */
> + if (in == &loopback_dev)
> + goto out;

Minor nit that akpm will point out, the else belongs on the same line as 
the brace.  Also, the comment style is:

/*
 * Comment...
 * etc.
 */




- James
-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/7] secid reconciliation-v03: Invoke LSM hook for outbound traffic

[James Morris]

On Thu, 28 Sep 2006, Venkat Yekkirala wrote:

> + if (connsecmark)
> + if (*connsecmark != skb->secmark) {
>   *connsecmark = skb->secmark;
> + }

Please remove the braces if re-submitting.

> +printk(KERN_ERR "IN HOOK (%d) (%u) (%u)\n", hooknum, skb->secmark, 
> *psecmark);\

Please remove debugging (here and elsewhere).



> + /* Set secmark on inbound and filter it on outbound */
> + if ((target->family == AF_INET &&
> + (hooknum == NF_IP_POST_ROUTING ||
> +  hooknum == NF_IP_LOCAL_OUT ||
> +  hooknum == NF_IP_FORWARD)) ||
> + (target->family == AF_INET6 &&
> + (hooknum == NF_IP6_POST_ROUTING ||
> +  hooknum == NF_IP6_LOCAL_OUT ||
> +  hooknum == NF_IP6_FORWARD))) {

I think this should be a separate helper function, so the logic can be 
changed/evaluated in isolation (preferred, but not a blocker).

> + secmark_save(skb, hooknum, target);

It seems that the target parameter is not needed.


> + return secmark_restore(skb, hooknum, in, target);

Please pass a family parameter instead of target.



-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/7] secid reconciliation-v03: Invoke LSM hook for outbound traffic

[Venkat Yekkirala]

Invoke the skb_flow_out LSM hook for outbound
traffic for secid reconciliation and flow control.

Signed-off-by: Venkat Yekkirala <[EMAIL PROTECTED]>
---
 net/netfilter/xt_CONNSECMARK.c |   70 +++
 net/netfilter/xt_SECMARK.c |   33 +-
 2 files changed, 85 insertions(+), 18 deletions(-)

diff --git a/net/netfilter/xt_CONNSECMARK.c b/net/netfilter/xt_CONNSECMARK.c
index 4673862..40b4150 100644
--- a/net/netfilter/xt_CONNSECMARK.c
+++ b/net/netfilter/xt_CONNSECMARK.c
@@ -17,6 +17,8 @@
  */
 #include 
 #include 
+#include 
+#include 
 #include 
 #include 
 #include 
@@ -33,34 +35,70 @@ MODULE_ALIAS("ip6t_CONNSECMARK");
  * If the packet has a security mark and the connection does not, copy
  * the security mark from the packet to the connection.
  */
-static void secmark_save(struct sk_buff *skb)
+static void secmark_save(struct sk_buff *skb, unsigned int hooknum,
+   const struct xt_target *target)
 {
if (skb->secmark) {
u32 *connsecmark;
enum ip_conntrack_info ctinfo;
 
connsecmark = nf_ct_get_secmark(skb, &ctinfo);
-   if (connsecmark && !*connsecmark)
-   if (*connsecmark != skb->secmark)
+   if (connsecmark)
+   if (*connsecmark != skb->secmark) {
*connsecmark = skb->secmark;
+   }
}
 }
 
 /*
- * 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 net_device *in, const struct xt_target *target)
 {
-   if (!skb->secmark) {
-   u32 *connsecmark;
-   enum ip_conntrack_info ctinfo;
-
-   connsecmark = nf_ct_get_secmark(skb, &ctinfo);
-   if (connsecmark && *connsecmark)
-   if (skb->secmark != *connsecmark)
-   skb->secmark = *connsecmark;
+   u32 *psecmark;
+   enum ip_conntrack_info ctinfo;
+
+   psecmark = nf_ct_get_secmark(skb, &ctinfo);
+
+   if (psecmark && *psecmark) {
+
+printk(KERN_ERR "IN HOOK (%d) (%u) (%u)\n", hooknum, skb->secmark, *psecmark);
+   /* Set secmark on inbound and filter it on outbound */
+   if ((target->family == AF_INET &&
+   (hooknum == NF_IP_POST_ROUTING ||
+hooknum == NF_IP_LOCAL_OUT ||
+hooknum == NF_IP_FORWARD)) ||
+   (target->family == AF_INET6 &&
+   (hooknum == NF_IP6_POST_ROUTING ||
+hooknum == NF_IP6_LOCAL_OUT ||
+hooknum == NF_IP6_FORWARD))) {
+   /* outbound */
+   int err;
+
+   err = security_skb_flow_out(skb, *psecmark);
+printk(KERN_ERR "ERR (%d) (%u)\n", err, skb->secmark);
+   if (!err)
+   return NF_DROP;
+   }
+   else/* inbound */
+   /* loopback traffic should already be labeled
+  and any filtering on outbound should suffice */
+   if (in == &loopback_dev)
+   goto out;
+
+   /* inbound or done with outbound check or no LSM hook
+  for outbound */
+   if (skb->secmark != *psecmark)
+{
+printk(KERN_ERR "RESTORING FROM (%u) TO (%u) (%u)\n", skb->secmark, *psecmark, 
0x);
+   skb->secmark = *psecmark;
+}
}
+
+out:
+   return XT_CONTINUE;
 }
 
 static unsigned int target(struct sk_buff **pskb, const struct net_device *in,
@@ -73,11 +111,11 @@ static unsigned int target(struct sk_buf
 
switch (info->mode) {
case CONNSECMARK_SAVE:
-   secmark_save(skb);
+   secmark_save(skb, hooknum, target);
break;
 
case CONNSECMARK_RESTORE:
-   secmark_restore(skb);
+   return secmark_restore(skb, hooknum, in, target);
break;
 
default:
diff --git a/net/netfilter/xt_SECMARK.c b/net/netfilter/xt_SECMARK.c
index add7521..28dc059 100644
--- a/net/netfilter/xt_SECMARK.c
+++ b/net/netfilter/xt_SECMARK.c
@@ -15,8 +15,10 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
+#include 
 
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("James Morris <[EMAIL PROTECTED]>");
@@ -47,9 +49,36 @@ static unsigned int target(struct sk_buf
BUG();
}
 
-   if ((*pskb)->secmark != secmark)
-   (*pskb)->secmark = secmark;
+   if (secmark) {
+
+