Date: Fri, 26 May 2006 10:23:41 +0200 From: Ian FREISLICH <if@hetzner.co.za> To: FreeBSD-gnats-submit@FreeBSD.org Subject: kern/97951: [PATCH] ipfw does not tie interface details to state Message-ID: <E1FjXbh-0006TT-7T@hetzner.co.za> Resent-Message-ID: <200605260830.k4Q8UCYc028720@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>Number: 97951
>Category: kern
>Synopsis: [PATCH] ipfw does not tie interface details to state
>Confidential: no
>Severity: serious
>Priority: medium
>Responsible: freebsd-bugs
>State: open
>Quarter:
>Keywords:
>Date-Required:
>Class: sw-bug
>Submitter-Id: current-users
>Arrival-Date: Fri May 26 08:30:11 GMT 2006
>Closed-Date:
>Last-Modified:
>Originator: Ian FREISLICH
>Release: FreeBSD 7.0-CURRENT i386
>Organization:
Hetzner
>Environment:
System: FreeBSD firewall1a.your-server.co.za 7.0-CURRENT FreeBSD 7.0-CURRENT #5: Fri Mar 10 14:47:45 SAST 2006 ianf@firewall1a.your-server.co.za:/usr/obj/usr/src/sys/FIREWALL i386
FreeBSD running as a shared firewall for many clients using 802.1Q
vlan tagging and trunking to gain sufficient interfaces.
>Description:
The state rules do not record interface information alongside
protocol information. When doing stateful filtering a rule that
records state for an inbound packet on an interface (to allow the
return traffic) will allow that packet out of another interface
by way of the check-state, even if that other interface would not
have allowed the packet through.
>How-To-Repeat:
00301 skipto 1100 ip from any to any in recv vlan1
00301 skipto 1100 ip from any to any out xmit vlan1
...
00564 skipto 27400 ip from any to any in recv vlan264
00564 skipto 27400 ip from any to any out xmit vlan264
01000 allow ip from any to not me
#vlan1
01100 allow tcp from any to any dst-port 22 setup out xmit vlan1
01199 allow udp from any to any dst-port 53 in recv vlan1 keep-state
01199 allow icmp from any to any in recv vlan1 keep-state
01199 check-state
01199 allow tcp from any to any established out xmit vlan1
01199 allow tcp from any to any in recv vlan1
01199 deny log ip from any to any
...
#vlan264
27400 allow tcp from any to any dst-port 22 setup out xmit vlan264
27499 allow udp from any to any dst-port 53 in recv vlan264 keep-state
27499 allow icmp from any to any in recv vlan264 keep-state
27499 check-state
27499 allow tcp from any to any established out xmit vlan264
27499 allow tcp from any to any in recv vlan264
27499 deny log ip from any to any
...
Other rules that protect the firewall itself on the world facing interface.
A host on vlan1 will be able to ping or send dns queries to a host
on vlan264 because of side-effects of the way that state rules are
generated and matched.
>Fix:
This patch records interface context with each state rule. Dynamic
state rules are printed as follows, showing the interface on which
the rule was created:
27499 1 141 (5s) STATE (vlan264) udp 192.168.0.2 2191 <-> 192.168.1.9 53
Index: ipfw2.c
===================================================================
RCS file: /home/ncvs/src/sbin/ipfw/ipfw2.c,v
retrieving revision 1.87
diff -u -d -r1.87 ipfw2.c
--- ipfw2.c 31 Mar 2006 12:54:17 -0000 1.87
+++ ipfw2.c 8 May 2006 13:26:03 -0000
@@ -1950,7 +1950,7 @@
printf(" LIMIT");
break;
case O_KEEP_STATE: /* bidir, no mask */
- printf(" STATE");
+ printf(" STATE (%s)", d->if_name);
break;
}
Index: ip_fw.h
===================================================================
RCS file: /home/ncvs/src/sys/netinet/ip_fw.h,v
retrieving revision 1.104
diff -u -d -r1.104 ip_fw.h
--- ip_fw.h 14 Feb 2006 06:36:39 -0000 1.104
+++ ip_fw.h 8 May 2006 13:24:16 -0000
@@ -422,6 +422,8 @@
/* to generate keepalives) */
u_int16_t dyn_type; /* rule type */
u_int16_t count; /* refcount */
+ u_short if_index;
+ char if_name[IFNAMSIZ];
};
/*
Index: ip_fw2.c
===================================================================
RCS file: /home/ncvs/src/sys/netinet/ip_fw2.c,v
retrieving revision 1.127
diff -u -d -r1.127 ip_fw2.c
--- ip_fw2.c 3 Mar 2006 12:10:59 -0000 1.127
+++ ip_fw2.c 9 May 2006 07:08:49 -0000
@@ -1120,7 +1120,7 @@
*/
static ipfw_dyn_rule *
lookup_dyn_rule_locked(struct ipfw_flow_id *pkt, int *match_direction,
- struct tcphdr *tcp)
+ struct tcphdr *tcp, struct ifnet *ifp)
{
/*
* stateful ipfw extensions.
@@ -1139,7 +1139,9 @@
goto done; /* not found */
i = hash_packet( pkt );
for (prev=NULL, q = ipfw_dyn_v[i] ; q != NULL ; ) {
- if (q->dyn_type == O_LIMIT_PARENT && q->count)
+ if ((q->dyn_type == O_LIMIT_PARENT && q->count) ||
+ (q->dyn_type == O_KEEP_STATE &&
+ q->if_index != ifp->if_index))
goto next;
if (TIME_LEQ( q->expire, time_uptime)) { /* expire entry */
UNLINK_DYN_RULE(prev, ipfw_dyn_v[i], q);
@@ -1263,12 +1265,12 @@
static ipfw_dyn_rule *
lookup_dyn_rule(struct ipfw_flow_id *pkt, int *match_direction,
- struct tcphdr *tcp)
+ struct tcphdr *tcp, struct ifnet *ifp)
{
ipfw_dyn_rule *q;
IPFW_DYN_LOCK();
- q = lookup_dyn_rule_locked(pkt, match_direction, tcp);
+ q = lookup_dyn_rule_locked(pkt, match_direction, tcp, ifp);
if (q == NULL)
IPFW_DYN_UNLOCK();
/* NB: return table locked when q is not NULL */
@@ -1315,7 +1317,8 @@
* - "parent" rules for the above (O_LIMIT_PARENT).
*/
static ipfw_dyn_rule *
-add_dyn_rule(struct ipfw_flow_id *id, u_int8_t dyn_type, struct ip_fw *rule)
+add_dyn_rule(struct ipfw_flow_id *id, u_int8_t dyn_type, struct ip_fw *rule,
+ struct ifnet *ifp)
{
ipfw_dyn_rule *r;
int i;
@@ -1352,6 +1355,11 @@
r->dyn_type = dyn_type;
r->pcnt = r->bcnt = 0;
r->count = 0;
+ if (dyn_type == O_KEEP_STATE) {
+ r->if_index = ifp->if_index;
+ strncpy(r->if_name, ifp->if_xname, IFNAMSIZ);
+ r->if_name[IFNAMSIZ] = '\0';
+ }
r->bucket = i;
r->next = ipfw_dyn_v[i];
@@ -1402,7 +1410,7 @@
return q;
}
}
- return add_dyn_rule(pkt, O_LIMIT_PARENT, rule);
+ return add_dyn_rule(pkt, O_LIMIT_PARENT, rule, NULL);
}
/**
@@ -1413,7 +1421,7 @@
*/
static int
install_state(struct ip_fw *rule, ipfw_insn_limit *cmd,
- struct ip_fw_args *args)
+ struct ip_fw_args *args, struct ifnet *ifp)
{
static int last_log;
@@ -1426,7 +1434,7 @@
IPFW_DYN_LOCK();
- q = lookup_dyn_rule_locked(&args->f_id, NULL, NULL);
+ q = lookup_dyn_rule_locked(&args->f_id, NULL, NULL, ifp);
if (q != NULL) { /* should never occur */
if (last_log != time_uptime) {
@@ -1454,7 +1462,7 @@
switch (cmd->o.opcode) {
case O_KEEP_STATE: /* bidir rule */
- add_dyn_rule(&args->f_id, O_KEEP_STATE, rule);
+ add_dyn_rule(&args->f_id, O_KEEP_STATE, rule, ifp);
break;
case O_LIMIT: /* limit number of sessions */
@@ -1506,7 +1514,7 @@
return 1;
}
}
- add_dyn_rule(&args->f_id, O_LIMIT, (struct ip_fw *)parent);
+ add_dyn_rule(&args->f_id, O_LIMIT, (struct ip_fw *)parent, NULL
);
}
break;
default:
@@ -1514,7 +1522,7 @@
IPFW_DYN_UNLOCK();
return 1;
}
- lookup_dyn_rule_locked(&args->f_id, NULL, NULL); /* XXX just set lifeti
me */
+ lookup_dyn_rule_locked(&args->f_id, NULL, NULL, ifp); /* XXX just set l
ifetime */
IPFW_DYN_UNLOCK();
return 0;
}
@@ -2929,7 +2937,8 @@
case O_LIMIT:
case O_KEEP_STATE:
if (install_state(f,
- (ipfw_insn_limit *)cmd, args)) {
+ (ipfw_insn_limit *)cmd, args, oif ? oif :
+ m->m_pkthdr.rcvif)) {
retval = IP_FW_DENY;
goto done; /* error/limit violation */
}
@@ -2950,7 +2959,8 @@
if (dyn_dir == MATCH_UNKNOWN &&
(q = lookup_dyn_rule(&args->f_id,
&dyn_dir, proto == IPPROTO_TCP ?
- TCP(ulp) : NULL))
+ TCP(ulp) : NULL, oif ? oif :
+ m->m_pkthdr.rcvif))
!= NULL) {
/*
* Found dynamic entry, update stats
>Release-Note:
>Audit-Trail:
>Unformatted:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?E1FjXbh-0006TT-7T>