From owner-freebsd-net Tue Aug 29 9:29:57 2000 Delivered-To: freebsd-net@freebsd.org Received: from whale.sunbay.crimea.ua (whale.sunbay.crimea.ua [212.110.138.65]) by hub.freebsd.org (Postfix) with ESMTP id 685F437B424; Tue, 29 Aug 2000 09:29:35 -0700 (PDT) Received: (from ru@localhost) by whale.sunbay.crimea.ua (8.9.3/1.13) id TAA39938; Tue, 29 Aug 2000 19:29:13 +0300 (EEST) Date: Tue, 29 Aug 2000 19:29:13 +0300 From: Ruslan Ermilov To: net@FreeBSD.org Cc: Garrett Wollman , Bill Fenner , Darren Reed , Kannan Varadhan , Frank Volf Subject: CFR: patch for ICMP error generation bugs Message-ID: <20000829192913.A39253@sunbay.com> Mail-Followup-To: net@FreeBSD.org, Garrett Wollman , Bill Fenner , Darren Reed , Kannan Varadhan , Frank Volf Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="5vNYLRcllDrimb99" X-Mailer: Mutt 1.0i Sender: owner-freebsd-net@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.org --5vNYLRcllDrimb99 Content-Type: text/plain; charset=us-ascii Hi! There are at least two problem reports PR 16240 and PR 20877 that this patch addresses. You can easily see yourself what gets wrong by monitoring ICMP error messages containing part of original datagram with `tcpdump -vvnx icmp' and comparing the original datagram with one in generated ICMP error. You will notice that sometimes fields are in host byte order, or TTL field is decremented. At least one case is not fixed by this patch -- in an IPFW based firewall, when we have a `unreach foo' rule matching `out'going packets, the ip_ttl field is still decremented. Please note that `udp_usrreq.c,v 1.71 by darrenr' should be backed out in order to apply and test this patch. Comments please, -- Ruslan Ermilov Oracle Developer/DBA, ru@sunbay.com Sunbay Software AG, ru@FreeBSD.org FreeBSD committer, +380.652.512.251 Simferopol, Ukraine http://www.FreeBSD.org The Power To Serve http://www.oracle.com Enabling The Information Age --5vNYLRcllDrimb99 Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename=p Index: ip_icmp.c =================================================================== RCS file: /home/ncvs/src/sys/netinet/ip_icmp.c,v retrieving revision 1.43 diff -u -p -r1.43 ip_icmp.c --- ip_icmp.c 2000/06/02 20:18:38 1.43 +++ ip_icmp.c 2000/08/29 15:50:18 @@ -191,7 +191,13 @@ icmp_error(n, type, code, dest, destifp) icp->icmp_code = code; bcopy((caddr_t)oip, (caddr_t)&icp->icmp_ip, icmplen); nip = &icp->icmp_ip; - nip->ip_len = htons((u_short)(nip->ip_len + oiplen)); + + /* + * Convert fields to network representation. + */ + HTONS(nip->ip_len); + HTONS(nip->ip_id); + HTONS(nip->ip_off); /* * Now, copy old ip header (without options) Index: ip_input.c =================================================================== RCS file: /home/ncvs/src/sys/netinet/ip_input.c,v retrieving revision 1.138 diff -u -p -r1.138 ip_input.c --- ip_input.c 2000/07/31 23:41:47 1.138 +++ ip_input.c 2000/08/29 15:50:19 @@ -1290,7 +1290,6 @@ nosourcerouting: } return (0); bad: - ip->ip_len -= IP_VHL_HL(ip->ip_vhl) << 2; /* XXX icmp_error adds in hdr length */ icmp_error(m, type, code, 0, 0); ipstat.ips_badoptions++; return (1); @@ -1496,19 +1495,14 @@ ip_forward(m, srcrt) m_freem(m); return; } - HTONS(ip->ip_id); #ifdef IPSTEALTH - if (!ipstealth) { + if (!ipstealth) #endif if (ip->ip_ttl <= IPTTLDEC) { icmp_error(m, ICMP_TIMXCEED, ICMP_TIMXCEED_INTRANS, dest, 0); return; } - ip->ip_ttl -= IPTTLDEC; -#ifdef IPSTEALTH - } -#endif sin = (struct sockaddr_in *)&ipforward_rt.ro_dst; if ((rt = ipforward_rt.ro_rt) == 0 || @@ -1534,7 +1528,14 @@ ip_forward(m, srcrt) * we need to generate an ICMP message to the src. */ mcopy = m_copy(m, 0, imin((int)ip->ip_len, 64)); + if (mcopy && (mcopy->m_flags & M_EXT)) + m_copydata(mcopy, 0, sizeof(struct ip), mtod(mcopy, caddr_t)); +#ifdef IPSTEALTH + if (!ipstealth) +#endif + ip->ip_ttl -= IPTTLDEC; + /* * If forwarding packet using same interface that it came in on, * perhaps should send a redirect to sender to shortcut a hop. @@ -1567,6 +1568,7 @@ ip_forward(m, srcrt) } } + HTONS(ip->ip_id); error = ip_output(m, (struct mbuf *)0, &ipforward_rt, IP_FORWARDING, 0); if (error) @@ -1672,6 +1674,8 @@ ip_forward(m, srcrt) m_freem(mcopy); return; } + if (mcopy->m_flags & M_EXT) + m_copyback(mcopy, 0, sizeof(struct ip), mtod(mcopy, caddr_t)); icmp_error(mcopy, type, code, dest, destifp); } Index: ip_output.c =================================================================== RCS file: /home/ncvs/src/sys/netinet/ip_output.c,v retrieving revision 1.109 diff -u -p -r1.109 ip_output.c --- ip_output.c 2000/07/31 13:11:41 1.109 +++ ip_output.c 2000/08/29 15:50:19 @@ -462,6 +462,7 @@ sendit: if (fw_enable && ip_fw_chk_ptr) { struct sockaddr_in *old = dst; + NTOHS(ip->ip_id); off = (*ip_fw_chk_ptr)(&ip, hlen, ifp, &divert_cookie, &m, &rule, &dst); /* @@ -482,6 +483,7 @@ sendit: error = EACCES; goto done; } + HTONS(ip->ip_id); if (off == 0 && dst == old) /* common case */ goto pass ; #ifdef DUMMYNET Index: udp_usrreq.c =================================================================== RCS file: /home/ncvs/src/sys/netinet/udp_usrreq.c,v retrieving revision 1.70 diff -u -p -r1.70 udp_usrreq.c --- udp_usrreq.c 2000/07/04 16:35:05 1.70 +++ udp_usrreq.c 2000/08/29 15:50:19 @@ -353,11 +353,12 @@ udp_input(m, off, proto) udpstat.udps_noportbcast++; goto bad; } - *ip = save_ip; if (badport_bandlim(0) < 0) goto bad; if (blackhole) goto bad; + *ip = save_ip; + ip->ip_len += iphlen; icmp_error(m, ICMP_UNREACH, ICMP_UNREACH_PORT, 0, 0); return; } --5vNYLRcllDrimb99-- To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-net" in the body of the message