Perform Output hook for ICMPv4 Reply

Before this change, locally generated ICMPv4 replies would not
perform the Output hook so NAT will not be performed for locally
generated ICMPv4 replies. This change fixes that bug.

PiperOrigin-RevId: 418513563
This commit is contained in:
Ghanan Gowripalan
2021-12-27 11:20:10 -08:00
committed by gVisor bot
parent 715f9b6539
commit 76776aad8b
3 changed files with 113 additions and 41 deletions
+23 -5
View File
@@ -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
}
+14 -8
View File
@@ -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:
+76 -28
View File
@@ -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 {