Thanks for the bug report. I've just done an audit of the unclean
code: patch against 2.4.5 is below. There were some bad thinkos
there, two fatal.
It'd be nice if someone else audits it, as I wrote it in the first
place...
Untested, please report back.
Cheers,
Rusty.
-- Premature optmztion is rt of all evl. --DK--- linux-2.4.5-official/net/ipv4/netfilter/ipt_unclean.c Fri Apr 28 08:43:15 2000 +++ working-2.4.5-hotplug/net/ipv4/netfilter/ipt_unclean.c Tue Jun 19 19:30:32 2001 @@ -76,7 +76,7 @@ = { 12, 12, ICMP_NOT_ERROR, 0, 0 } }; /* Can't do anything if it's a fragment. */ - if (!offset) + if (offset) return 1; /* Must cover type and code. */ @@ -87,7 +87,7 @@ /* If not embedded. */ if (!embedded) { - /* Bad checksum? Don't print, just drop. */ + /* Bad checksum? Don't print, just ignore. */ if (!more_frags && ip_compute_csum((unsigned char *) icmph, datalen) != 0) return 0; @@ -108,6 +108,8 @@ length of iph + 8 bytes. */ struct iphdr *inner = (void *)icmph + 8; + /* datalen > 8 since all ICMP_IS_ERROR types + have min length > 8 */ if (datalen - 8 < sizeof(struct iphdr)) { limpk("ICMP error internal way too short\n"); return 0; @@ -155,6 +157,8 @@ u_int32_t arg = ntohl(icmph->un.gateway); if (icmph->code == 0) { + /* Code 0 means that upper 8 bits is pointer + to problem. */ if ((arg >> 24) >= iph->ihl*4) { limpk("ICMP PARAMETERPROB ptr = %u\n", ntohl(icmph->un.gateway) >> 24); @@ -196,7 +200,7 @@ int embedded) { /* Can't do anything if it's a fragment. */ - if (!offset) + if (offset) return 1; /* CHECK: Must cover UDP header. */ @@ -205,7 +209,7 @@ return 0; } - /* Bad checksum? Don't print, just drop. */ + /* Bad checksum? Don't print, just say it's unclean. */ /* FIXME: SRC ROUTE packets won't match checksum --RR */ if (!more_frags && !embedded && csum_tcpudp_magic(iph->saddr, iph->daddr, datalen, IPPROTO_UDP, @@ -272,7 +276,7 @@ /* In fact, this is caught below (offset < 516). */ /* Can't do anything if it's a fragment. */ - if (!offset) + if (offset) return 1; /* CHECK: Smaller than minimal TCP hdr. */ @@ -281,7 +285,8 @@ limpk("Packet length %u < TCP header.\n", datalen); return 0; } - /* Must have ports available (datalen >= 8). */ + /* Must have ports available (datalen >= 8), from + check_icmp which set embedded = 1 */ /* CHECK: TCP ports inside ICMP error */ if (!tcph->source || !tcph->dest) { limpk("Zero TCP ports %u/%u.\n", @@ -301,7 +306,7 @@ return 1; } - /* Bad checksum? Don't print, just drop. */ + /* Bad checksum? Don't print, just say it's unclean. */ /* FIXME: SRC ROUTE packets won't match checksum --RR */ if (!more_frags && !embedded && csum_tcpudp_magic(iph->saddr, iph->daddr, datalen, IPPROTO_TCP, @@ -373,6 +378,8 @@ (unsigned int) opt[i], i); return 0; } + /* Move to next option */ + i += opt[i+1]; } } @@ -442,6 +449,8 @@ opt[i], i); return 0; } + /* Move to next option */ + i += opt[i+1]; } } @@ -495,10 +504,10 @@ return 0; } - /* CHECK: Min offset of frag = 128 - 60 (max IP hdr len). */ - if (offset && offset * 8 < MIN_LIKELY_MTU - 60) { + /* CHECK: Min offset of frag = 128 - IP hdr len. */ + if (offset && offset * 8 < MIN_LIKELY_MTU - iph->ihl * 4) { limpk("Fragment starts at %u < %u\n", offset * 8, - MIN_LIKELY_MTU-60); + MIN_LIKELY_MTU - iph->ihl * 4); return 0; } - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/