diff options
-rw-r--r-- | pkg/tcpip/header/ipv4.go | 19 | ||||
-rw-r--r-- | pkg/tcpip/network/ipv4/icmp.go | 4 | ||||
-rw-r--r-- | pkg/tcpip/network/ipv4/ipv4.go | 4 | ||||
-rw-r--r-- | test/packetimpact/testbench/connections.go | 2 | ||||
-rw-r--r-- | test/packetimpact/testbench/layers.go | 8 |
5 files changed, 19 insertions, 18 deletions
diff --git a/pkg/tcpip/header/ipv4.go b/pkg/tcpip/header/ipv4.go index 713d476d6..91fe7b6a5 100644 --- a/pkg/tcpip/header/ipv4.go +++ b/pkg/tcpip/header/ipv4.go @@ -89,7 +89,17 @@ type IPv4Fields struct { // DstAddr is the "destination ip address" of an IPv4 packet. DstAddr tcpip.Address - // Options is between 0 and 40 bytes or nil if empty. + // Options must be 40 bytes or less as they must fit along with the + // rest of the IPv4 header into the maximum size describable in the + // IHL field. RFC 791 section 3.1 says: + // IHL: 4 bits + // + // Internet Header Length is the length of the internet header in 32 + // bit words, and thus points to the beginning of the data. Note that + // the minimum value for a correct header is 5. + // + // That leaves ten 32 bit (4 byte) fields for options. An attempt to encode + // more will fail. Options IPv4Options } @@ -284,13 +294,10 @@ func (o IPv4Options) SizeWithPadding() int { return (len(o) + IPv4IHLStride - 1) & ^(IPv4IHLStride - 1) } -// Options returns a buffer holding the options or nil. +// Options returns a buffer holding the options. func (b IPv4) Options() IPv4Options { hdrLen := b.HeaderLength() - if hdrLen > IPv4MinimumSize { - return IPv4Options(b[options:hdrLen:hdrLen]) - } - return nil + return IPv4Options(b[options:hdrLen:hdrLen]) } // TransportProtocol implements Network.TransportProtocol. diff --git a/pkg/tcpip/network/ipv4/icmp.go b/pkg/tcpip/network/ipv4/icmp.go index 58a19e74a..204b182e6 100644 --- a/pkg/tcpip/network/ipv4/icmp.go +++ b/pkg/tcpip/network/ipv4/icmp.go @@ -90,7 +90,7 @@ func (e *endpoint) handleICMP(pkt *stack.PacketBuffer) { iph := header.IPv4(pkt.NetworkHeader().View()) var newOptions header.IPv4Options - if len(iph) > header.IPv4MinimumSize { + if opts := iph.Options(); len(opts) != 0 { // RFC 1122 section 3.2.2.6 (page 43) (and similar for other round trip // type ICMP packets): // If a Record Route and/or Time Stamp option is received in an @@ -106,7 +106,7 @@ func (e *endpoint) handleICMP(pkt *stack.PacketBuffer) { } else { op = &optionUsageReceive{} } - aux, tmp, err := e.processIPOptions(pkt, iph.Options(), op) + aux, tmp, err := e.processIPOptions(pkt, opts, op) if err != nil { switch { case diff --git a/pkg/tcpip/network/ipv4/ipv4.go b/pkg/tcpip/network/ipv4/ipv4.go index b4f21d61e..a9a38b851 100644 --- a/pkg/tcpip/network/ipv4/ipv4.go +++ b/pkg/tcpip/network/ipv4/ipv4.go @@ -713,11 +713,11 @@ func (e *endpoint) handlePacket(pkt *stack.PacketBuffer) { e.handleICMP(pkt) return } - if len(h.Options()) != 0 { + if opts := h.Options(); len(opts) != 0 { // TODO(gvisor.dev/issue/4586): // When we add forwarding support we should use the verified options // rather than just throwing them away. - aux, _, err := e.processIPOptions(pkt, h.Options(), &optionUsageReceive{}) + aux, _, err := e.processIPOptions(pkt, opts, &optionUsageReceive{}) if err != nil { switch { case diff --git a/test/packetimpact/testbench/connections.go b/test/packetimpact/testbench/connections.go index 030a73c3c..919b4fd25 100644 --- a/test/packetimpact/testbench/connections.go +++ b/test/packetimpact/testbench/connections.go @@ -72,7 +72,7 @@ func pickPort(domain, typ int) (fd int, port uint16, err error) { } sa, err = unix.Getsockname(fd) if err != nil { - return -1, 0, fmt.Errorf("fail in Getsocketname(%d): %w", fd, err) + return -1, 0, fmt.Errorf("unix.Getsocketname(%d): %w", fd, err) } port, err = portFromSockaddr(sa) if err != nil { diff --git a/test/packetimpact/testbench/layers.go b/test/packetimpact/testbench/layers.go index 62473c577..7401a1991 100644 --- a/test/packetimpact/testbench/layers.go +++ b/test/packetimpact/testbench/layers.go @@ -410,13 +410,7 @@ func Address(v tcpip.Address) *tcpip.Address { // continues parsing further encapsulations. func parseIPv4(b []byte) (Layer, layerParser) { h := header.IPv4(b) - hdrLen := h.HeaderLength() - // Even if there are no options, we set an empty options field instead of nil - // so that the decision to compare is up to the caller of that comparison. - var options header.IPv4Options - if hdrLen > header.IPv4MinimumSize { - options = append(options, h.Options()...) - } + options := h.Options() tos, _ := h.TOS() ipv4 := IPv4{ IHL: Uint8(h.HeaderLength()), |