diff --git a/pkg/tcpip/network/ipv4/icmp.go b/pkg/tcpip/network/ipv4/icmp.go index 2a024fe38..ce8ec96ab 100644 --- a/pkg/tcpip/network/ipv4/icmp.go +++ b/pkg/tcpip/network/ipv4/icmp.go @@ -20,6 +20,7 @@ import ( "gvisor.dev/gvisor/pkg/tcpip" "gvisor.dev/gvisor/pkg/tcpip/buffer" "gvisor.dev/gvisor/pkg/tcpip/header" + "gvisor.dev/gvisor/pkg/tcpip/header/parse" "gvisor.dev/gvisor/pkg/tcpip/stack" ) @@ -255,6 +256,12 @@ func (e *endpoint) handleICMP(pkt *stack.PacketBuffer) { e.dispatcher.DeliverTransportPacket(header.ICMPv4ProtocolNumber, pkt) pkt = nil + sent := e.stats.icmp.packetsSent + if !e.protocol.allowICMPReply(header.ICMPv4EchoReply, header.ICMPv4UnusedCode) { + sent.rateLimited.Increment() + return + } + // Take the base of the incoming request IP header but replace the options. replyHeaderLength := uint8(header.IPv4MinimumSize + len(newOptions)) replyIPHdr := header.IPv4(append(iph[:header.IPv4MinimumSize:header.IPv4MinimumSize], newOptions...)) @@ -275,9 +282,10 @@ func (e *endpoint) handleICMP(pkt *stack.PacketBuffer) { } defer r.Release() - sent := e.stats.icmp.packetsSent - if !e.protocol.allowICMPReply(header.ICMPv4EchoReply, header.ICMPv4UnusedCode) { - sent.rateLimited.Increment() + outgoingEP, ok := e.protocol.getEndpointForNIC(r.NICID()) + if !ok { + // The outgoing NIC went away. + sent.dropped.Increment() return } @@ -308,6 +316,9 @@ func (e *endpoint) handleICMP(pkt *stack.PacketBuffer) { replyIPHdr.SetSourceAddress(r.LocalAddress()) replyIPHdr.SetDestinationAddress(r.RemoteAddress()) replyIPHdr.SetTTL(r.DefaultTTL()) + replyIPHdr.SetTotalLength(uint16(len(replyIPHdr) + len(replyData))) + replyIPHdr.SetChecksum(0) + replyIPHdr.SetChecksum(^replyIPHdr.CalculateChecksum()) replyICMPHdr := header.ICMPv4(replyData) replyICMPHdr.SetType(header.ICMPv4EchoReply) @@ -321,9 +332,16 @@ func (e *endpoint) handleICMP(pkt *stack.PacketBuffer) { Data: replyVV, }) defer replyPkt.DecRef() - replyPkt.TransportProtocolNumber = header.ICMPv4ProtocolNumber + // Populate the network/transport headers in the packet buffer so the + // ICMP packet goes through IPTables. + if ok := parse.IPv4(replyPkt); !ok { + panic("expected to parse IPv4 header we just created") + } + if ok := parse.ICMPv4(replyPkt); !ok { + panic("expected to parse ICMPv4 header we just created") + } - if err := r.WriteHeaderIncludedPacket(replyPkt); err != nil { + if err := outgoingEP.writePacket(r, replyPkt); err != nil { sent.dropped.Increment() return } diff --git a/pkg/tcpip/network/ipv4/ipv4.go b/pkg/tcpip/network/ipv4/ipv4.go index 8afb52a99..475bb82cd 100644 --- a/pkg/tcpip/network/ipv4/ipv4.go +++ b/pkg/tcpip/network/ipv4/ipv4.go @@ -420,11 +420,17 @@ func (e *endpoint) handleFragments(_ *stack.Route, networkMTU uint32, pkt *stack // WritePacket writes a packet to the given destination address and protocol. func (e *endpoint) WritePacket(r *stack.Route, params stack.NetworkHeaderParams, pkt *stack.PacketBuffer) tcpip.Error { - dstAddr := r.RemoteAddress() - if err := e.addIPHeader(r.LocalAddress(), dstAddr, pkt, params, nil /* options */); err != nil { + if err := e.addIPHeader(r.LocalAddress(), r.RemoteAddress(), pkt, params, nil /* options */); err != nil { return err } + return e.writePacket(r, pkt) +} + +func (e *endpoint) writePacket(r *stack.Route, pkt *stack.PacketBuffer) tcpip.Error { + netHeader := header.IPv4(pkt.NetworkHeader().View()) + dstAddr := netHeader.DestinationAddress() + // iptables filtering. All packets that reach here are locally // generated. outNicName := e.protocol.stack.FindNICNameFromID(e.nic.ID()) @@ -441,8 +447,8 @@ func (e *endpoint) WritePacket(r *stack.Route, params stack.NetworkHeaderParams, // We should do this for every packet, rather than only DNATted packets, but // removing this check short circuits broadcasts before they are sent out to // other hosts. - if netHeader := header.IPv4(pkt.NetworkHeader().View()); dstAddr != netHeader.DestinationAddress() { - if ep := e.protocol.findEndpointWithAddress(netHeader.DestinationAddress()); ep != nil { + if newDstAddr := netHeader.DestinationAddress(); dstAddr != newDstAddr { + if ep := e.protocol.findEndpointWithAddress(newDstAddr); ep != nil { // Since we rewrote the packet but it is being routed back to us, we // can safely assume the checksum is valid. ep.handleLocalPacket(pkt, true /* canSkipRXChecksum */) @@ -450,10 +456,10 @@ func (e *endpoint) WritePacket(r *stack.Route, params stack.NetworkHeaderParams, } } - return e.writePacket(r, pkt, false /* headerIncluded */) + return e.writePacketPostRouting(r, pkt, false /* headerIncluded */) } -func (e *endpoint) writePacket(r *stack.Route, pkt *stack.PacketBuffer, headerIncluded bool) tcpip.Error { +func (e *endpoint) writePacketPostRouting(r *stack.Route, pkt *stack.PacketBuffer, headerIncluded bool) tcpip.Error { if r.Loop()&stack.PacketLoop != 0 { // If the packet was generated by the stack (not a raw/packet endpoint // where a packet may be written with the header included), then we can @@ -561,7 +567,7 @@ func (e *endpoint) WriteHeaderIncludedPacket(r *stack.Route, pkt *stack.PacketBu return &tcpip.ErrMalformedHeader{} } - return e.writePacket(r, pkt, true /* headerIncluded */) + return e.writePacketPostRouting(r, pkt, true /* headerIncluded */) } // forwardPacket attempts to forward a packet to its final destination. @@ -691,7 +697,7 @@ func (e *endpoint) forwardPacket(pkt *stack.PacketBuffer) ip.ForwardingError { return &ip.ErrOther{Err: &tcpip.ErrUnknownDevice{}} } - switch err := forwardToEp.writePacket(r, newPkt, true /* headerIncluded */); err.(type) { + switch err := forwardToEp.writePacketPostRouting(r, newPkt, true /* headerIncluded */); err.(type) { case nil: return nil case *tcpip.ErrMessageTooLong: diff --git a/pkg/tcpip/tests/integration/iptables_test.go b/pkg/tcpip/tests/integration/iptables_test.go index ccdc1f877..e84193de9 100644 --- a/pkg/tcpip/tests/integration/iptables_test.go +++ b/pkg/tcpip/tests/integration/iptables_test.go @@ -971,7 +971,7 @@ func TestForwardingHook(t *testing.T) { } } -func TestInputHookWithLocalForwarding(t *testing.T) { +func TestFilteringEchoPacketsWithLocalForwarding(t *testing.T) { const ( nicID1 = 1 nicID2 = 2 @@ -1018,37 +1018,58 @@ func TestInputHookWithLocalForwarding(t *testing.T) { }, } + type droppedEcho int + const ( + _ droppedEcho = iota + noneDropped + echoRequestDroppedAtInput + echoRequestDroppedAtForward + echoReplyDropped + ) + subTests := []struct { - name string - setupFilter func(*testing.T, *stack.Stack, tcpip.NetworkProtocolNumber) - expectDrop bool + name string + setupFilter func(*testing.T, *stack.Stack, tcpip.NetworkProtocolNumber) + expectResult droppedEcho }{ { - name: "Accept", - setupFilter: func(*testing.T, *stack.Stack, tcpip.NetworkProtocolNumber) { /* no filter */ }, - expectDrop: false, + name: "Accept", + setupFilter: func(*testing.T, *stack.Stack, tcpip.NetworkProtocolNumber) { /* no filter */ }, + expectResult: noneDropped, }, { - name: "Drop", - setupFilter: setupDropFilter(stack.Input, stack.IPHeaderFilter{}), - expectDrop: true, + name: "Input Drop", + setupFilter: setupDropFilter(stack.Input, stack.IPHeaderFilter{}), + expectResult: echoRequestDroppedAtInput, }, { - name: "Drop with input NIC filtering on arrival NIC", - setupFilter: setupDropFilter(stack.Input, stack.IPHeaderFilter{InputInterface: nic1Name}), - expectDrop: true, + name: "Input Drop with input NIC filtering on arrival NIC", + setupFilter: setupDropFilter(stack.Input, stack.IPHeaderFilter{InputInterface: nic1Name}), + expectResult: echoRequestDroppedAtInput, }, { - name: "Drop with input NIC filtering on delivered NIC", - setupFilter: setupDropFilter(stack.Input, stack.IPHeaderFilter{InputInterface: nic2Name}), - expectDrop: false, + name: "Input Drop with input NIC filtering on delivered NIC", + setupFilter: setupDropFilter(stack.Input, stack.IPHeaderFilter{InputInterface: nic2Name}), + expectResult: noneDropped, }, { - name: "Drop with input NIC filtering on other NIC", - setupFilter: setupDropFilter(stack.Input, stack.IPHeaderFilter{InputInterface: otherNICName}), - expectDrop: false, + name: "Input Drop with input NIC filtering on other NIC", + setupFilter: setupDropFilter(stack.Input, stack.IPHeaderFilter{InputInterface: otherNICName}), + expectResult: noneDropped, + }, + + { + name: "Forward Drop", + setupFilter: setupDropFilter(stack.Forward, stack.IPHeaderFilter{}), + expectResult: echoRequestDroppedAtForward, + }, + + { + name: "Output Drop", + setupFilter: setupDropFilter(stack.Output, stack.IPHeaderFilter{}), + expectResult: echoReplyDropped, }, } @@ -1121,8 +1142,34 @@ func TestInputHookWithLocalForwarding(t *testing.T) { if got := ip1Stats.ValidPacketsReceived.Value(); got != 1 { t.Errorf("got ip1Stats.ValidPacketsReceived.Value() = %d, want = 1", got) } - if got, want := ip1Stats.PacketsSent.Value(), boolToInt(!subTest.expectDrop); got != want { - t.Errorf("got ip1Stats.PacketsSent.Value() = %d, want = %d", got, want) + + expectedIP1StatIPTablesForawrdDropped := uint64(0) + expectedIP1StatIPTablesOutputDropped := uint64(0) + expectedIP1StatPacketsSent := uint64(0) + expectedIP2StatValidPacketsReceived := uint64(1) + expectedIP2StatIPTablesInputDropped := uint64(0) + switch subTest.expectResult { + case noneDropped: + expectedIP1StatPacketsSent = 1 + case echoRequestDroppedAtInput: + expectedIP2StatIPTablesInputDropped = 1 + case echoRequestDroppedAtForward: + expectedIP1StatIPTablesForawrdDropped = 1 + expectedIP2StatValidPacketsReceived = 0 + case echoReplyDropped: + expectedIP1StatIPTablesOutputDropped = 1 + default: + t.Fatalf("unhandled expectResult = %d", subTest.expectResult) + } + + if got := ip1Stats.IPTablesForwardDropped.Value(); got != expectedIP1StatIPTablesForawrdDropped { + t.Errorf("got ip1Stats.IPTablesForwardDropped.Value() = %d, want = %d", got, expectedIP1StatIPTablesForawrdDropped) + } + if got := ip1Stats.IPTablesOutputDropped.Value(); got != expectedIP1StatIPTablesOutputDropped { + t.Errorf("got ip1Stats.IPTablesOutputDropped.Value() = %d, want = %d", got, expectedIP1StatIPTablesOutputDropped) + } + if got := ip1Stats.PacketsSent.Value(); got != expectedIP1StatPacketsSent { + t.Errorf("got ip1Stats.PacketsSent.Value() = %d, want = %d", got, expectedIP1StatPacketsSent) } ep2, err := s.GetNetworkEndpoint(nicID2, test.netProto) @@ -1138,19 +1185,20 @@ func TestInputHookWithLocalForwarding(t *testing.T) { if got := ip2Stats.PacketsReceived.Value(); got != 0 { t.Errorf("got ip2Stats.PacketsReceived.Value() = %d, want = 0", got) } - if got := ip2Stats.ValidPacketsReceived.Value(); got != 1 { - t.Errorf("got ip2Stats.ValidPacketsReceived.Value() = %d, want = 1", got) + if got := ip2Stats.ValidPacketsReceived.Value(); got != expectedIP2StatValidPacketsReceived { + t.Errorf("got ip2Stats.ValidPacketsReceived.Value() = %d, want = %d", got, expectedIP2StatValidPacketsReceived) } - if got, want := ip2Stats.IPTablesInputDropped.Value(), boolToInt(subTest.expectDrop); got != want { - t.Errorf("got ip2Stats.IPTablesInputDropped.Value() = %d, want = %d", got, want) + if got := ip2Stats.IPTablesInputDropped.Value(); got != expectedIP2StatIPTablesInputDropped { + t.Errorf("got ip2Stats.IPTablesInputDropped.Value() = %d, want = %d", got, expectedIP2StatIPTablesInputDropped) } if got := ip2Stats.PacketsSent.Value(); got != 0 { t.Errorf("got ip2Stats.PacketsSent.Value() = %d, want = 0", got) } - if p := e1.Read(); (p != nil) == subTest.expectDrop { - t.Errorf("got e1.Read() = %#v, want = (_ == nil) = %t", p, !subTest.expectDrop) - } else if !subTest.expectDrop { + expectPacket := subTest.expectResult == noneDropped + if p := e1.Read(); (p != nil) != expectPacket { + t.Errorf("got e1.Read() = %#v, want = (_ == nil) = %t", p, expectPacket) + } else if expectPacket { test.checker(t, stack.PayloadSince(p.NetworkHeader())) } if p := e2.Read(); p != nil {