diff options
| -rw-r--r-- | pkg/tcpip/network/ipv4/icmp.go | 53 | ||||
| -rw-r--r-- | pkg/tcpip/network/ipv4/ipv4.go | 114 | ||||
| -rw-r--r-- | pkg/tcpip/network/ipv4/ipv4_test.go | 127 |
3 files changed, 241 insertions, 53 deletions
diff --git a/pkg/tcpip/network/ipv4/icmp.go b/pkg/tcpip/network/ipv4/icmp.go index 2b7bc0dd0..b44304cee 100644 --- a/pkg/tcpip/network/ipv4/icmp.go +++ b/pkg/tcpip/network/ipv4/icmp.go @@ -213,7 +213,8 @@ func (e *endpoint) handleICMP(pkt *stack.PacketBuffer) { } else { op = &optionUsageReceive{} } - tmp, optProblem := e.processIPOptions(pkt, opts, op) + var optProblem *header.IPv4OptParameterProblem + newOptions, optProblem = e.processIPOptions(pkt, opts, op) if optProblem != nil { if optProblem.NeedICMP { _ = e.protocol.returnError(&icmpReasonParamProblem{ @@ -224,7 +225,14 @@ func (e *endpoint) handleICMP(pkt *stack.PacketBuffer) { } return } - newOptions = tmp + copied := copy(opts, newOptions) + if copied != len(newOptions) { + panic(fmt.Sprintf("copied %d bytes of new options, expected %d bytes", copied, len(newOptions))) + } + for i := copied; i < len(opts); i++ { + // Pad with 0 (EOL). RFC 791 page 23 says "The padding is zero". + opts[i] = byte(header.IPv4OptionListEndType) + } } // TODO(b/112892170): Meaningfully handle all ICMP types. @@ -375,6 +383,7 @@ func (e *endpoint) handleICMP(pkt *stack.PacketBuffer) { // icmpReason is a marker interface for IPv4 specific ICMP errors. type icmpReason interface { isICMPReason() + isForwarding() bool } // icmpReasonPortUnreachable is an error where the transport protocol has no @@ -382,12 +391,18 @@ type icmpReason interface { type icmpReasonPortUnreachable struct{} func (*icmpReasonPortUnreachable) isICMPReason() {} +func (*icmpReasonPortUnreachable) isForwarding() bool { + return false +} // icmpReasonProtoUnreachable is an error where the transport protocol is // not supported. type icmpReasonProtoUnreachable struct{} func (*icmpReasonProtoUnreachable) isICMPReason() {} +func (*icmpReasonProtoUnreachable) isForwarding() bool { + return false +} // icmpReasonTTLExceeded is an error where a packet's time to live exceeded in // transit to its final destination, as per RFC 792 page 6, Time Exceeded @@ -395,6 +410,15 @@ func (*icmpReasonProtoUnreachable) isICMPReason() {} type icmpReasonTTLExceeded struct{} func (*icmpReasonTTLExceeded) isICMPReason() {} +func (*icmpReasonTTLExceeded) isForwarding() bool { + // If we hit a TTL Exceeded error, then we know we are operating as a router. + // As per RFC 792 page 6, Time Exceeded Message, + // + // If the gateway processing a datagram finds the time to live field + // is zero it must discard the datagram. The gateway may also notify + // the source host via the time exceeded message. + return true +} // icmpReasonReassemblyTimeout is an error where insufficient fragments are // received to complete reassembly of a packet within a configured time after @@ -402,14 +426,21 @@ func (*icmpReasonTTLExceeded) isICMPReason() {} type icmpReasonReassemblyTimeout struct{} func (*icmpReasonReassemblyTimeout) isICMPReason() {} +func (*icmpReasonReassemblyTimeout) isForwarding() bool { + return false +} // icmpReasonParamProblem is an error to use to request a Parameter Problem // message to be sent. type icmpReasonParamProblem struct { - pointer byte + pointer byte + forwarding bool } func (*icmpReasonParamProblem) isICMPReason() {} +func (r *icmpReasonParamProblem) isForwarding() bool { + return r.forwarding +} // returnError takes an error descriptor and generates the appropriate ICMP // error packet for IPv4 and sends it back to the remote device that sent @@ -448,26 +479,14 @@ func (p *protocol) returnError(reason icmpReason, pkt *stack.PacketBuffer) tcpip return nil } - // If we hit a TTL Exceeded error, then we know we are operating as a router. - // As per RFC 792 page 6, Time Exceeded Message, - // - // If the gateway processing a datagram finds the time to live field - // is zero it must discard the datagram. The gateway may also notify - // the source host via the time exceeded message. - // - // ... - // - // Code 0 may be received from a gateway. ... - // - // Note, Code 0 is the TTL exceeded error. - // // If we are operating as a router/gateway, don't use the packet's destination // address as the response's source address as we should not not own the // destination address of a packet we are forwarding. localAddr := origIPHdrDst - if _, ok := reason.(*icmpReasonTTLExceeded); ok { + if reason.isForwarding() { localAddr = "" } + // Even if we were able to receive a packet from some remote, we may not have // a route to it - the remote may be blocked via routing rules. We must always // consult our routing table and find a route to the remote before sending any diff --git a/pkg/tcpip/network/ipv4/ipv4.go b/pkg/tcpip/network/ipv4/ipv4.go index 7de438fe3..c5e4034ce 100644 --- a/pkg/tcpip/network/ipv4/ipv4.go +++ b/pkg/tcpip/network/ipv4/ipv4.go @@ -561,6 +561,34 @@ func (e *endpoint) forwardPacket(pkt *stack.PacketBuffer) tcpip.Error { return e.protocol.returnError(&icmpReasonTTLExceeded{}, pkt) } + if opts := h.Options(); len(opts) != 0 { + newOpts, optProblem := e.processIPOptions(pkt, opts, &optionUsageForward{}) + if optProblem != nil { + if optProblem.NeedICMP { + _ = e.protocol.returnError(&icmpReasonParamProblem{ + pointer: optProblem.Pointer, + forwarding: true, + }, pkt) + e.protocol.stack.Stats().MalformedRcvdPackets.Increment() + e.stats.ip.MalformedPacketsReceived.Increment() + } + return nil // option problems are not reported locally. + } + copied := copy(opts, newOpts) + if copied != len(newOpts) { + panic(fmt.Sprintf("copied %d bytes of new options, expected %d bytes", copied, len(newOpts))) + } + // Since in forwarding we handle all options, including copying those we + // do not recognise, the options region should remain the same size which + // simplifies processing. As we MAY receive a packet with a lot of padded + // bytes after the "end of options list" byte, make sure we copy + // them as the legal padding value (0). + for i := copied; i < len(opts); i++ { + // Pad with 0 (EOL). RFC 791 page 23 says "The padding is zero". + opts[i] = byte(header.IPv4OptionListEndType) + } + } + dstAddr := h.DestinationAddress() // Check if the destination is owned by the stack. @@ -711,8 +739,9 @@ func (e *endpoint) handlePacket(pkt *stack.PacketBuffer) { } } - // The destination address should be an address we own or a group we joined - // for us to receive the packet. Otherwise, attempt to forward the packet. + // Before we do any processing, note if the packet was received as some + // sort of broadcast. The destination address should be an address we own + // or a group we joined. if addressEndpoint := e.AcquireAssignedAddress(dstAddr, e.nic.Promiscuous(), stack.CanBePrimaryEndpoint); addressEndpoint != nil { subnet := addressEndpoint.AddressWithPrefix().Subnet() addressEndpoint.DecRef() @@ -722,7 +751,6 @@ func (e *endpoint) handlePacket(pkt *stack.PacketBuffer) { stats.ip.InvalidDestinationAddressesReceived.Increment() return } - _ = e.forwardPacket(pkt) return } @@ -744,6 +772,21 @@ func (e *endpoint) handlePacket(pkt *stack.PacketBuffer) { stats.ip.MalformedFragmentsReceived.Increment() return } + if opts := h.Options(); len(opts) != 0 { + // If there are options we need to check them before we do assembly + // or we could be assembling errant packets. However we do not change the + // options as that could lead to double processing later. + if _, optProblem := e.processIPOptions(pkt, opts, &optionUsageVerify{}); optProblem != nil { + if optProblem.NeedICMP { + _ = e.protocol.returnError(&icmpReasonParamProblem{ + pointer: optProblem.Pointer, + }, pkt) + e.protocol.stack.Stats().MalformedRcvdPackets.Increment() + e.stats.ip.MalformedPacketsReceived.Increment() + } + return + } + } // The packet is a fragment, let's try to reassemble it. start := h.FragmentOffset() // Drop the fragment if the size of the reassembled payload would exceed the @@ -802,17 +845,10 @@ func (e *endpoint) handlePacket(pkt *stack.PacketBuffer) { e.handleICMP(pkt) return } - if p == header.IGMPProtocolNumber { - e.mu.Lock() - e.mu.igmp.handleIGMP(pkt) - e.mu.Unlock() - return - } + // ICMP handles options itself but do it here for all remaining destinations. 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. - if _, optProblem := e.processIPOptions(pkt, opts, &optionUsageReceive{}); optProblem != nil { + newOpts, optProblem := e.processIPOptions(pkt, opts, &optionUsageReceive{}) + if optProblem != nil { if optProblem.NeedICMP { _ = e.protocol.returnError(&icmpReasonParamProblem{ pointer: optProblem.Pointer, @@ -822,6 +858,20 @@ func (e *endpoint) handlePacket(pkt *stack.PacketBuffer) { } return } + copied := copy(opts, newOpts) + if copied != len(newOpts) { + panic(fmt.Sprintf("copied %d bytes of new options, expected %d bytes", copied, len(newOpts))) + } + for i := copied; i < len(opts); i++ { + // Pad with 0 (EOL). RFC 791 page 23 says "The padding is zero". + opts[i] = byte(header.IPv4OptionListEndType) + } + } + if p == header.IGMPProtocolNumber { + e.mu.Lock() + e.mu.igmp.handleIGMP(pkt) + e.mu.Unlock() + return } switch res := e.dispatcher.DeliverTransportPacket(p, pkt); res { @@ -1254,23 +1304,49 @@ type optionsUsage interface { actions() optionActions } -// optionUsageReceive implements optionsUsage for received packets. -type optionUsageReceive struct{} +// optionUsageVerify implements optionsUsage for when we just want to check +// fragments. Don't change anything, just check and reject if bad. No +// replacement options are generated. +type optionUsageVerify struct{} // actions implements optionsUsage. -func (*optionUsageReceive) actions() optionActions { +func (*optionUsageVerify) actions() optionActions { return optionActions{ timestamp: optionVerify, recordRoute: optionVerify, + unknown: optionRemove, + } +} + +// optionUsageReceive implements optionsUsage for packets we will pass +// to the transport layer (with the exception of Echo requests). +type optionUsageReceive struct{} + +// actions implements optionsUsage. +func (*optionUsageReceive) actions() optionActions { + return optionActions{ + timestamp: optionProcess, + recordRoute: optionProcess, unknown: optionPass, } } -// TODO(gvisor.dev/issue/4586): Add an entry here for forwarding when it -// is enabled (Process, Process, Pass) and for fragmenting (Process, Process, -// Pass for frag1, but Remove,Remove,Remove for all other frags). +// optionUsageForward implements optionsUsage for packets about to be forwarded. +// All options are passed on regardless of whether we recognise them, however +// we do process the Timestamp and Record Route options. +type optionUsageForward struct{} + +// actions implements optionsUsage. +func (*optionUsageForward) actions() optionActions { + return optionActions{ + timestamp: optionProcess, + recordRoute: optionProcess, + unknown: optionPass, + } +} // optionUsageEcho implements optionsUsage for echo packet processing. +// Only Timestamp and RecordRoute are processed and sent back. type optionUsageEcho struct{} // actions implements optionsUsage. diff --git a/pkg/tcpip/network/ipv4/ipv4_test.go b/pkg/tcpip/network/ipv4/ipv4_test.go index a296bed79..c57d7f616 100644 --- a/pkg/tcpip/network/ipv4/ipv4_test.go +++ b/pkg/tcpip/network/ipv4/ipv4_test.go @@ -111,10 +111,11 @@ func TestExcludeBroadcast(t *testing.T) { func TestForwarding(t *testing.T) { const ( - nicID1 = 1 - nicID2 = 2 - randomSequence = 123 - randomIdent = 42 + nicID1 = 1 + nicID2 = 2 + randomSequence = 123 + randomIdent = 42 + randomTimeOffset = 0x10203040 ) ipv4Addr1 := tcpip.AddressWithPrefix{ @@ -129,14 +130,20 @@ func TestForwarding(t *testing.T) { remoteIPv4Addr2 := tcpip.Address(net.ParseIP("11.0.0.2").To4()) tests := []struct { - name string - TTL uint8 - expectErrorICMP bool + name string + TTL uint8 + expectErrorICMP bool + options header.IPv4Options + forwardedOptions header.IPv4Options + icmpType header.ICMPv4Type + icmpCode header.ICMPv4Code }{ { name: "TTL of zero", TTL: 0, expectErrorICMP: true, + icmpType: header.ICMPv4TimeExceeded, + icmpCode: header.ICMPv4TTLExceeded, }, { name: "TTL of one", @@ -153,14 +160,78 @@ func TestForwarding(t *testing.T) { TTL: math.MaxUint8, expectErrorICMP: false, }, + { + name: "four EOL options", + TTL: 2, + expectErrorICMP: false, + options: header.IPv4Options{0, 0, 0, 0}, + forwardedOptions: header.IPv4Options{0, 0, 0, 0}, + }, + { + name: "TS type 1 full", + TTL: 2, + options: header.IPv4Options{ + 68, 12, 13, 0xF1, + 192, 168, 1, 12, + 1, 2, 3, 4, + }, + expectErrorICMP: true, + icmpType: header.ICMPv4ParamProblem, + icmpCode: header.ICMPv4UnusedCode, + }, + { + name: "TS type 0", + TTL: 2, + options: header.IPv4Options{ + 68, 24, 21, 0x00, + 1, 2, 3, 4, + 5, 6, 7, 8, + 9, 10, 11, 12, + 13, 14, 15, 16, + 0, 0, 0, 0, + }, + forwardedOptions: header.IPv4Options{ + 68, 24, 25, 0x00, + 1, 2, 3, 4, + 5, 6, 7, 8, + 9, 10, 11, 12, + 13, 14, 15, 16, + 0x00, 0xad, 0x1c, 0x40, // time we expect from fakeclock + }, + }, + { + name: "end of options list", + TTL: 2, + options: header.IPv4Options{ + 68, 12, 13, 0x11, + 192, 168, 1, 12, + 1, 2, 3, 4, + 0, 10, 3, 99, // EOL followed by junk + 1, 2, 3, 4, + }, + forwardedOptions: header.IPv4Options{ + 68, 12, 13, 0x21, + 192, 168, 1, 12, + 1, 2, 3, 4, + 0, // End of Options hides following bytes. + 0, 0, 0, // 7 bytes unknown option removed. + 0, 0, 0, 0, + }, + }, } - for _, test := range tests { t.Run(test.name, func(t *testing.T) { + clock := faketime.NewManualClock() s := stack.New(stack.Options{ NetworkProtocols: []stack.NetworkProtocolFactory{ipv4.NewProtocol}, TransportProtocols: []stack.TransportProtocolFactory{icmp.NewProtocol4}, + Clock: clock, }) + + // Advance the clock by some unimportant amount to make + // it give a more recognisable signature than 00,00,00,00. + clock.Advance(time.Millisecond * randomTimeOffset) + // We expect at most a single packet in response to our ICMP Echo Request. e1 := channel.New(1, ipv4.MaxTotalSize, "") if err := s.CreateNIC(nicID1, e1); err != nil { @@ -195,7 +266,11 @@ func TestForwarding(t *testing.T) { t.Fatalf("SetForwarding(%d, true): %s", header.IPv4ProtocolNumber, err) } - totalLen := uint16(header.IPv4MinimumSize + header.ICMPv4MinimumSize) + ipHeaderLength := header.IPv4MinimumSize + len(test.options) + if ipHeaderLength > header.IPv4MaximumHeaderSize { + t.Fatalf("got ipHeaderLength = %d, want <= %d ", ipHeaderLength, header.IPv4MaximumHeaderSize) + } + totalLen := uint16(ipHeaderLength + header.ICMPv4MinimumSize) hdr := buffer.NewPrependable(int(totalLen)) icmp := header.ICMPv4(hdr.Prepend(header.ICMPv4MinimumSize)) icmp.SetIdent(randomIdent) @@ -204,7 +279,7 @@ func TestForwarding(t *testing.T) { icmp.SetCode(header.ICMPv4UnusedCode) icmp.SetChecksum(0) icmp.SetChecksum(^header.Checksum(icmp, 0)) - ip := header.IPv4(hdr.Prepend(header.IPv4MinimumSize)) + ip := header.IPv4(hdr.Prepend(ipHeaderLength)) ip.Encode(&header.IPv4Fields{ TotalLength: totalLen, Protocol: uint8(header.ICMPv4ProtocolNumber), @@ -212,6 +287,14 @@ func TestForwarding(t *testing.T) { SrcAddr: remoteIPv4Addr1, DstAddr: remoteIPv4Addr2, }) + if len(test.options) != 0 { + ip.SetHeaderLength(uint8(ipHeaderLength)) + // Copy options manually. We do not use Encode for options so we can + // verify malformed options with handcrafted payloads. + if want, got := copy(ip.Options(), test.options), len(test.options); want != got { + t.Fatalf("got copy(ip.Options(), test.options) = %d, want = %d", got, want) + } + } ip.SetChecksum(0) ip.SetChecksum(^ip.CalculateChecksum()) requestPkt := stack.NewPacketBuffer(stack.PacketBufferOptions{ @@ -222,7 +305,7 @@ func TestForwarding(t *testing.T) { if test.expectErrorICMP { reply, ok := e1.Read() if !ok { - t.Fatal("expected ICMP TTL Exceeded packet through incoming NIC") + t.Fatalf("expected ICMP packet type %d through incoming NIC", test.icmpType) } checker.IPv4(t, header.IPv4(stack.PayloadSince(reply.Pkt.NetworkHeader())), @@ -231,8 +314,8 @@ func TestForwarding(t *testing.T) { checker.TTL(ipv4.DefaultTTL), checker.ICMPv4( checker.ICMPv4Checksum(), - checker.ICMPv4Type(header.ICMPv4TimeExceeded), - checker.ICMPv4Code(header.ICMPv4TTLExceeded), + checker.ICMPv4Type(test.icmpType), + checker.ICMPv4Code(test.icmpCode), checker.ICMPv4Payload([]byte(hdr.View())), ), ) @@ -250,6 +333,7 @@ func TestForwarding(t *testing.T) { checker.SrcAddr(remoteIPv4Addr1), checker.DstAddr(remoteIPv4Addr2), checker.TTL(test.TTL-1), + checker.IPv4Options(test.forwardedOptions), checker.ICMPv4( checker.ICMPv4Checksum(), checker.ICMPv4Type(header.ICMPv4Echo), @@ -279,6 +363,7 @@ func TestIPv4Sanity(t *testing.T) { // (offset 1). For compatibility we must do the same. Use this constant // to indicate where this happens. pointerOffsetForInvalidLength = 0 + randomTimeOffset = 0x10203040 ) var ( ipv4Addr = tcpip.AddressWithPrefix{ @@ -893,6 +978,10 @@ func TestIPv4Sanity(t *testing.T) { TransportProtocols: []stack.TransportProtocolFactory{icmp.NewProtocol4}, Clock: clock, }) + // Advance the clock by some unimportant amount to make + // it give a more recognisable signature than 00,00,00,00. + clock.Advance(time.Millisecond * randomTimeOffset) + // We expect at most a single packet in response to our ICMP Echo Request. e := channel.New(1, ipv4.MaxTotalSize, "") if err := s.CreateNIC(nicID, e); err != nil { @@ -902,9 +991,6 @@ func TestIPv4Sanity(t *testing.T) { if err := s.AddProtocolAddress(nicID, ipv4ProtoAddr); err != nil { t.Fatalf("AddProtocolAddress(%d, %#v): %s", nicID, ipv4ProtoAddr, err) } - // Advance the clock by some unimportant amount to make - // sure it's all set up. - clock.Advance(time.Millisecond * 0x10203040) // Default routes for IPv4 so ICMP can find a route to the remote // node when attempting to send the ICMP Echo Reply. @@ -1739,12 +1825,19 @@ func TestInvalidFragments(t *testing.T) { ip := header.IPv4(hdr.Prepend(pktSize)) ip.Encode(&f.ipv4fields) + if want, got := len(f.payload), copy(ip[header.IPv4MinimumSize:], f.payload); want != got { + t.Fatalf("copied %d bytes, expected %d bytes.", got, want) + } // Encode sets this up correctly. If we want a different value for // testing then we need to overwrite the good value. if f.overrideIHL != 0 { ip.SetHeaderLength(uint8(f.overrideIHL)) + // If we are asked to add options (type not specified) then pad + // with 0 (EOL). RFC 791 page 23 says "The padding is zero". + for i := header.IPv4MinimumSize; i < f.overrideIHL; i++ { + ip[i] = byte(header.IPv4OptionListEndType) + } } - copy(ip[header.IPv4MinimumSize:], f.payload) if f.autoChecksum { ip.SetChecksum(0) |