[hipl-commit] [trunk] Rev 3990: fixed major security whole in the firewall

From: Rene Hummen <rene.hummen@xxxxxxxxxxxxxxxxx>
To: hipl-commit@xxxxxxxxxxxxx
Date: Wed, 17 Mar 2010 18:28:41 +0200
Committer: Rene Hummen <rene.hummen@xxxxxxxxxxxxxxxxx>
Date: 17/03/2010 at 18:28:41
Revision: 3990
Revision-id: rene.hummen@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
Branch nick: trunk

Log:
  fixed major security whole in the firewall
  
  before ALL HIP packets matching a rule were treated with the configured
  verdict instead of incorporating signature verification results, etc
  into the overall verdict. This means that as long as e.g. the HIT of a
  packet matched a rule in the hipfw, it was accepted...even if the HOST_ID
  did actually NOT match the HIT or if the signature was plain wrong.
  
  NOTE: NOTIFY messages MUST contain a HIP signature. However, they don't
  for HIPL and thus are blocked by the firewall! This needs to be fixed.

Modified:
  M  firewall/conntrack.c
  M  firewall/conntrack.h
  M  firewall/firewall.c

=== modified file 'firewall/conntrack.c'
--- firewall/conntrack.c        2010-03-17 15:27:26 +0000
+++ firewall/conntrack.c        2010-03-17 16:22:48 +0000
@@ -2213,13 +2213,14 @@
  * @param buf the control packet
  * @param ctx context for the control packet
  */
-void conntrack(const struct in6_addr *ip6_src,
+int conntrack(const struct in6_addr *ip6_src,
                const struct in6_addr *ip6_dst,
                struct hip_common *buf,
                hip_fw_context_t *ctx)
 {
     struct hip_data *data = NULL;
     struct tuple *tuple   = NULL;
+    int verdict = 0;
 
     _HIP_DEBUG("\n");
     //g_mutex_lock(connectionTableMutex);
@@ -2234,12 +2235,14 @@
 
     // the accept_mobile parameter is true as packets
     // are not filtered here
-    check_packet(ip6_src, ip6_dst, buf, tuple, 0, 1, ctx);
+    verdict = check_packet(ip6_src, ip6_dst, buf, tuple, 0, 1, ctx);
 
     //g_mutex_unlock(connectionTableMutex);
     _HIP_DEBUG("unlocked mutex\n");
 
     free(data);
+
+    return verdict;
 }
 
 /**

=== modified file 'firewall/conntrack.h'
--- firewall/conntrack.h        2010-03-14 15:12:47 +0000
+++ firewall/conntrack.h        2010-03-17 16:22:48 +0000
@@ -24,7 +24,7 @@
                  struct hip_common *buf,
                  const struct state_option *option,
                  const int accept, hip_fw_context_t *ctx);
-void conntrack(const struct in6_addr *ip6_src,
+int conntrack(const struct in6_addr *ip6_src,
                const struct in6_addr *ip6_dst,
                struct hip_common *buf, hip_fw_context_t *ctx);
 

=== modified file 'firewall/firewall.c'
--- firewall/firewall.c 2010-03-14 15:12:47 +0000
+++ firewall/firewall.c 2010-03-17 16:22:48 +0000
@@ -1215,13 +1215,8 @@
     //release rule list
     read_rules_exit(0);
 
-    /* FIXME this actually verifies the packet and should be incorporated in 
the
-     *       resulting verdict!!! */
-    // if packet will be accepted and connection tracking is used
-    // but there is no state for the packet in the conntrack module
-    // yet -> show the packet to conntracking
     if (statefulFiltering && verdict && !conntracked) {
-        conntrack(ip6_src, ip6_dst, buf, ctx);
+        verdict = conntrack(ip6_src, ip6_dst, buf, ctx);
     }
 
     return verdict;
@@ -1307,8 +1302,6 @@
         verdict = ACCEPT;
     }
 
-    HIP_INFO("\n");
-
     /* zero return value means that the packet should be dropped */
     return verdict;
 }
[hipl-commit] [trunk] Rev 3990: fixed major security whole in the firewall

Other related posts:
