From c26544aa7f9234a4c0e5363f5bdfe926f0b910d8 Mon Sep 17 00:00:00 2001 From: Gleb Smirnoff Date: Mon, 1 Sep 2014 14:04:51 +0000 Subject: [PATCH] Make SOCK_RAW sockets to be truly raw, not modifying received and sent packets at all. Swapping byte order on SOCK_RAW was actually a bug, an artifact from the BSD network stack, that used to convert a packet to native byte order once it is received by kernel. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Other operating systems didn't follow this, and later other BSD descendants fixed this, leaving us alone with the bug. Now it is clear that we should fix the bug. In collaboration with: Olivier Cochard-Labbé See also: https://wiki.freebsd.org/SOCK_RAW Sponsored by: Nginx, Inc. --- share/man/man4/ip.4 | 36 +++++++++++++++++++++++------------- sys/netinet/raw_ip.c | 16 ++-------------- sys/sys/param.h | 2 +- usr.sbin/traceroute/Makefile | 2 +- 4 files changed, 27 insertions(+), 29 deletions(-) diff --git a/share/man/man4/ip.4 b/share/man/man4/ip.4 index b95a350b34e2..68b817d6fad3 100644 --- a/share/man/man4/ip.4 +++ b/share/man/man4/ip.4 @@ -28,7 +28,7 @@ .\" @(#)ip.4 8.2 (Berkeley) 11/30/93 .\" $FreeBSD$ .\" -.Dd October 12, 2012 +.Dd September 1, 2014 .Dt IP 4 .Os .Sh NAME @@ -755,13 +755,11 @@ number the socket is created with), unless the .Dv IP_HDRINCL option has been set. -Incoming packets are received with +Unlike in previous +.Bx +releases, incoming packets are received with .Tn IP -header and options intact, except for -.Va ip_len -and -.Va ip_off -fields converted to host byte order. +header and options intact, leaving all fields in network byte order. .Pp .Dv IP_HDRINCL indicates the complete IP header is included with the data @@ -784,17 +782,16 @@ the fields of the IP header, including the following: ip->ip_v = IPVERSION; ip->ip_hl = hlen >> 2; ip->ip_id = 0; /* 0 means kernel set appropriate value */ -ip->ip_off = offset; +ip->ip_off = htons(offset); +ip->ip_len = htons(len); .Ed .Pp -The +The packet should be provided as is to be sent over wire. +This implies all fields, including .Va ip_len and .Va ip_off -fields -.Em must -be provided in host byte order. -All other fields must be provided in network byte order. +to be in network byte order. See .Xr byteorder 3 for more information on network byte order. @@ -891,3 +888,16 @@ packets received on raw IP sockets had the subtracted from the .Va ip_len field. +.Pp +Before +.Fx 11.0 +packets received on raw IP sockets had the +.Va ip_len +and +.Va ip_off +fields converted to host byte order. +Packets written to raw IP sockets were expected to have +.Va ip_len +and +.Va ip_off +in host byte order. diff --git a/sys/netinet/raw_ip.c b/sys/netinet/raw_ip.c index 99706312db0f..f394f01d9a58 100644 --- a/sys/netinet/raw_ip.c +++ b/sys/netinet/raw_ip.c @@ -290,11 +290,6 @@ rip_input(struct mbuf **mp, int *offp, int proto) last = NULL; ifp = m->m_pkthdr.rcvif; - /* - * Applications on raw sockets expect host byte order. - */ - ip->ip_len = ntohs(ip->ip_len); - ip->ip_off = ntohs(ip->ip_off); hash = INP_PCBHASH_RAW(proto, ip->ip_src.s_addr, ip->ip_dst.s_addr, V_ripcbinfo.ipi_hashmask); @@ -504,8 +499,8 @@ rip_output(struct mbuf *m, struct socket *so, ...) * and don't allow packet length sizes that will crash. */ if (((ip->ip_hl != (sizeof (*ip) >> 2)) && inp->inp_options) - || (ip->ip_len > m->m_pkthdr.len) - || (ip->ip_len < (ip->ip_hl << 2))) { + || (ntohs(ip->ip_len) > m->m_pkthdr.len) + || (ntohs(ip->ip_len) < (ip->ip_hl << 2))) { INP_RUNLOCK(inp); m_freem(m); return (EINVAL); @@ -513,13 +508,6 @@ rip_output(struct mbuf *m, struct socket *so, ...) if (ip->ip_id == 0) ip->ip_id = ip_newid(); - /* - * Applications on raw sockets pass us packets - * in host byte order. - */ - ip->ip_len = htons(ip->ip_len); - ip->ip_off = htons(ip->ip_off); - /* * XXX prevent ip_output from overwriting header fields. */ diff --git a/sys/sys/param.h b/sys/sys/param.h index 264a38aa3ee0..50be879486d4 100644 --- a/sys/sys/param.h +++ b/sys/sys/param.h @@ -58,7 +58,7 @@ * in the range 5 to 9. */ #undef __FreeBSD_version -#define __FreeBSD_version 1100029 /* Master, propagated to newvers */ +#define __FreeBSD_version 1100030 /* Master, propagated to newvers */ /* * __FreeBSD_kernel__ indicates that this system uses the kernel of FreeBSD, diff --git a/usr.sbin/traceroute/Makefile b/usr.sbin/traceroute/Makefile index 103d2065f796..12f9a0b7cddf 100644 --- a/usr.sbin/traceroute/Makefile +++ b/usr.sbin/traceroute/Makefile @@ -13,7 +13,7 @@ CLEANFILES= version.c CFLAGS+= -DHAVE_SYS_SELECT_H=1 -DHAVE_SYS_SOCKIO_H=1 \ -DHAVE_NET_ROUTE_H=1 -DHAVE_NET_IF_DL_H=1 \ -DHAVE_STRERROR=1 -DHAVE_USLEEP=1 \ - -DHAVE_SYS_SYSCTL_H=1 \ + -DHAVE_SYS_SYSCTL_H=1 -DBYTESWAP_IP_HDR=1 \ -DHAVE_SETLINEBUF=1 -DHAVE_RAW_OPTIONS=1 \ -DHAVE_SOCKADDR_SA_LEN=1 -DHAVE_ICMP_NEXTMTU=1 .if !defined(TRACEROUTE_NO_IPSEC)