From owner-freebsd-bugs Fri Sep 8 7:50:11 2000 Delivered-To: freebsd-bugs@freebsd.org Received: from freefall.freebsd.org (freefall.FreeBSD.org [216.136.204.21]) by hub.freebsd.org (Postfix) with ESMTP id BFA2537B440 for ; Fri, 8 Sep 2000 07:50:01 -0700 (PDT) Received: (from gnats@localhost) by freefall.freebsd.org (8.9.3/8.9.2) id HAA85047; Fri, 8 Sep 2000 07:50:00 -0700 (PDT) (envelope-from gnats@FreeBSD.org) Received: from sneakerz.org (sneakerz.org [207.154.226.254]) by hub.freebsd.org (Postfix) with ESMTP id EBF2137B42C for ; Fri, 8 Sep 2000 07:45:40 -0700 (PDT) Received: by sneakerz.org (Postfix, from userid 1023) id 988115D006; Fri, 8 Sep 2000 09:45:35 -0500 (CDT) Message-Id: <20000908144535.988115D006@sneakerz.org> Date: Fri, 8 Sep 2000 09:45:35 -0500 (CDT) From: missnglnk@sneakerz.org Reply-To: missnglnk@sneakerz.org To: FreeBSD-gnats-submit@freebsd.org X-Send-Pr-Version: 3.2 Subject: kern/21118: Multiple problems in ipfw's stateful code Sender: owner-freebsd-bugs@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.org >Number: 21118 >Category: kern >Synopsis: Multiple problems in ipfw's stateful code >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 Sep 08 07:50:00 PDT 2000 >Closed-Date: >Last-Modified: >Originator: Lawrence N. King >Release: FreeBSD 4.1-FEARSOME-20000818 i386 >Organization: >Environment: FreeBSD 4.0-RELEASE FreeBSD 4.1-STABLE FreeBSD 5.0-CURRENT Above version with IPFIREWALL defined in kernel >Description: When a connection is made and kept track of (keep-state): 1) Connections expire prematurely if dynamic ACK lifetime is is greater than the keepalive interval. 2) Expired connections aren't actually expired, but are allowed to continue (TCP connections are extended by the dynamic RST lifetime even though they have expired, and all other protocol connections are extended by the dynamic short lifetime). 3) Expired connections aren't cleaned from the state table unless the next dynamic rule added reaches the maximum amount of dynamic rules. 4) A new state is installed for any packet which matches the rule (with keep-state), regardless of whether its not a TCP packet with the SYN flag set, or if its an ICMP query packet. >How-To-Repeat: ipfw -f flush ipfw add check-state ipfw add allow ip from any to any via fxp0 out keep-state ipfw add deny ip from any to any Now issue a couple of connections, I did SSH, a new state was installed, the rule expired, but I still could continue my SSH connection since its expiration time was being extended by the dynamic RST lifetime. Instead of issuing a TCP packet with the SYN flag set, issue a TCP packet with any other flag set, and a new state is installed for this packet. Let a couple of dynamic rules timeout (you can check via ipfw show), and watch as they linger around, they aren't cleaned out until the amount of dynamic rules is equal to the maximum and a new rule is being installed. >Fix: Nothing for problem 1, but solutions for 2-4 are in the below patch: -- snip -- --- sys/netinet/ip_fw.c.orig Wed Sep 6 21:01:07 2000 +++ sys/netinet/ip_fw.c Wed Sep 6 21:40:55 2000 @@ -735,4 +735,3 @@ break ; - default: -#if 0 + case TH_RST | (TH_RST << 8) : /* @@ -741,7 +740,18 @@ */ - if ( (q->state & ((TH_RST << 8)|TH_RST)) == 0) - printf("invalid state: 0x%x\n", q->state); -#endif + printf("invalid state: 0x%x\n", q->state); q->expire = time_second + dyn_rst_lifetime ; break ; + default: + /* + * A TCP packet found in unknown state, drop it. + */ + DEB(printf("packet should be dropped (state: 0x%x)\n", q->state)); + old_q = q ; + if (prev != NULL) + prev->next = q = q->next ; + else + ipfw_dyn_v[i] = q = q->next ; + dyn_count-- ; + free(old_q, M_IPFW); + break ; } @@ -838,4 +848,7 @@ } - if (dyn_count >= dyn_max) /* try remove old ones... */ - remove_dyn_rule(NULL, 0 /* expire */); + /* + * Unconditionally remove expired states. + */ + remove_dyn_rule(NULL, 0 /* expire */); + if (dyn_count >= dyn_max) { @@ -1277,4 +1290,43 @@ */ - if (q == NULL && f->fw_flg & IP_FW_F_KEEP_S) - install_state(chain); + if (q == NULL && f->fw_flg & IP_FW_F_KEEP_S) { + /* + * Instead of unconditionally adding a new state, + * check the protocol and flags, and add a new state + * or ignore packet. + */ + switch(proto) { + case IPPROTO_TCP: + if (flags & TH_SYN) { + DEB(printf("-- installing state for TCP packet\n")); + install_state(chain); + } else { + DEB(printf("-- invalid TCP connection state\n")); + } + break; + case IPPROTO_UDP: + DEB(printf("-- installing state for UDP packet\n")); + install_state(chain); + break; + case IPPROTO_ICMP: + if (is_icmp_query(ip)) { + DEB(printf("-- installing state for ICMP packet\n")); + install_state(chain); + } else { + DEB(printf("-- invalid ICMP connection state\n")); + } + break; + default: + /* + * Unknown packet, if default is to accept all + * packets, add a new state, otherwise ignore. + */ +#ifdef IPFIREWALL_DEFAULT_TO_ACCEPT + DEB(printf("-- installing state for unknown packet\n")); + install_state(chain); +#else + DEB(printf("invalid unknown protocol connection state\n")); +#endif + break; + } + } #endif -- snip -- >Release-Note: >Audit-Trail: >Unformatted: To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-bugs" in the body of the message