diff --git a/pkg/tcpip/stack/conntrack.go b/pkg/tcpip/stack/conntrack.go index 833843678..2440c34f7 100644 --- a/pkg/tcpip/stack/conntrack.go +++ b/pkg/tcpip/stack/conntrack.go @@ -95,6 +95,19 @@ func (ti tupleID) reply() tupleID { } } +type manipType int + +const ( + // manipNotPerformed indicates that NAT has not been performed. + manipNotPerformed manipType = iota + + // manipPerformed indicates that NAT was performed. + manipPerformed + + // manipPerformedNoop indicates that NAT was performed but it was a no-op. + manipPerformedNoop +) + // conn is a tracked connection. // // +stateify savable @@ -112,14 +125,14 @@ type conn struct { // // +checklocks:mu finalized bool - // sourceManip indicates the packet's source is manipulated. + // sourceManip indicates the source manipulation type. // // +checklocks:mu - sourceManip bool - // destinationManip indicates the packet's destination is manipulated. + sourceManip manipType + // destinationManip indicates the destination's manipulation type. // // +checklocks:mu - destinationManip bool + destinationManip manipType stateMu sync.RWMutex `state:"nosave"` // tcb is TCB control block. It is used to keep track of states @@ -539,13 +552,20 @@ func (cn *conn) finalize() { cn.ct.finalize(cn) } -// performNAT setups up the connection for the specified NAT. -// -// Generally, only the first packet of a connection reaches this method; other -// other packets will be manipulated without needing to modify the connection. -func (cn *conn) performNAT(pkt *PacketBuffer, hook Hook, r *Route, ports portRange, address tcpip.Address, dnat bool) { - cn.performNATIfNoop(ports, address, dnat) - cn.handlePacket(pkt, hook, r) +func (cn *conn) maybePerformNoopNAT(dnat bool) { + cn.mu.Lock() + defer cn.mu.Unlock() + + var manip *manipType + if dnat { + manip = &cn.destinationManip + } else { + manip = &cn.sourceManip + } + + if *manip == manipNotPerformed { + *manip = manipPerformedNoop + } } type portRange struct { @@ -553,36 +573,49 @@ type portRange struct { size uint16 } -func (cn *conn) performNATIfNoop(ports portRange, address tcpip.Address, dnat bool) { +// performNAT setups up the connection for the specified NAT and rewrites the +// packet. +// +// If NAT has already been performed on the connection, then the packet will +// be rewritten with the NAT performed on the connection, ignoring the passed +// address and port range. +// +// Generally, only the first packet of a connection reaches this method; other +// packets will be manipulated without needing to modify the connection. +func (cn *conn) performNAT(pkt *PacketBuffer, hook Hook, r *Route, ports portRange, natAddress tcpip.Address, dnat bool) { + // Make sure the packet is re-written after performing NAT. + defer func() { + // handlePacket returns true if the packet may skip the NAT table as the + // connection is already NATed, but if we reach this point we must be in the + // NAT table, so the return value is useless for us. + _ = cn.handlePacket(pkt, hook, r) + }() + cn.mu.Lock() defer cn.mu.Unlock() - if cn.finalized { - return - } - cn.reply.mu.Lock() defer cn.reply.mu.Unlock() + var manip *manipType + var address *tcpip.Address var port *uint16 if dnat { - if cn.destinationManip { - return - } - cn.destinationManip = true - - cn.reply.tupleID.srcAddr = address + manip = &cn.destinationManip + address = &cn.reply.tupleID.srcAddr port = &cn.reply.tupleID.srcPort } else { - if cn.sourceManip { - return - } - cn.sourceManip = true - - cn.reply.tupleID.dstAddr = address + manip = &cn.sourceManip + address = &cn.reply.tupleID.dstAddr port = &cn.reply.tupleID.dstPort } + if *manip != manipNotPerformed { + return + } + *manip = manipPerformed + *address = natAddress + // Does the current port fit in the range? if end := ports.start + ports.size - 1; *port >= ports.start && *port <= end { // Yes, is the current reply tuple unique? @@ -686,37 +719,34 @@ func (cn *conn) handlePacket(pkt *PacketBuffer, hook Hook, rt *Route) bool { reply := pkt.tuple.reply - tid, performManip := func() (tupleID, bool) { + tid, manip := func() (tupleID, manipType) { cn.mu.RLock() defer cn.mu.RUnlock() - var tuple *tuple if reply { - if dnat { - if !cn.sourceManip { - return tupleID{}, false - } - } else if !cn.destinationManip { - return tupleID{}, false - } + tid := cn.original.id() - tuple = &cn.original - } else { if dnat { - if !cn.destinationManip { - return tupleID{}, false - } - } else if !cn.sourceManip { - return tupleID{}, false + return tid, cn.sourceManip } - - tuple = &cn.reply + return tid, cn.destinationManip } - return tuple.id(), true + tid := cn.reply.id() + if dnat { + return tid, cn.destinationManip + } + return tid, cn.sourceManip }() - if !performManip { + switch manip { + case manipNotPerformed: return false + case manipPerformedNoop: + *natDone = true + return true + case manipPerformed: + default: + panic(fmt.Sprintf("unhandled manip = %d", manip)) } newPort := tid.dstPort @@ -939,7 +969,7 @@ func (ct *ConnTrack) originalDst(epID TransportEndpointID, netProto tcpip.Networ t.conn.mu.RLock() defer t.conn.mu.RUnlock() - if !t.conn.destinationManip { + if t.conn.destinationManip == manipNotPerformed { // Unmanipulated destination. return "", 0, &tcpip.ErrInvalidOptionValue{} } diff --git a/pkg/tcpip/stack/iptables.go b/pkg/tcpip/stack/iptables.go index 16f539c61..09188cdaf 100644 --- a/pkg/tcpip/stack/iptables.go +++ b/pkg/tcpip/stack/iptables.go @@ -425,11 +425,43 @@ func (it *IPTables) checkMangleRLocked(hook Hook, pkt *PacketBuffer, r *Route, a // // +checklocksread:it.mu func (it *IPTables) checkNATRLocked(hook Hook, pkt *PacketBuffer, r *Route, addressEP AddressableEndpoint, inNicName, outNicName string) bool { - if t := pkt.tuple; t != nil && t.conn.handlePacket(pkt, hook, r) { + t := pkt.tuple + if t != nil && t.conn.handlePacket(pkt, hook, r) { return true } - return it.checkRLocked(NATID, hook, pkt, r, addressEP, inNicName, outNicName) + if !it.checkRLocked(NATID, hook, pkt, r, addressEP, inNicName, outNicName) { + return false + } + + if t == nil { + return true + } + + var dnat bool + var natDone *bool + switch hook { + case Prerouting, Output: + dnat = true + natDone = &pkt.DNATDone + case Input, Postrouting: + dnat = false + natDone = &pkt.SNATDone + case Forward: + panic("should not attempt NAT in forwarding") + default: + panic(fmt.Sprintf("unhandled hook = %d", hook)) + } + + // Make sure the connection is NATed. + // + // If the packet was already NATed, the connection must be NATed. + if !*natDone { + t.conn.maybePerformNoopNAT(dnat) + _ = t.conn.handlePacket(pkt, hook, r) + } + + return true } // checkFilterRLocked runs the packet through the filter table. diff --git a/pkg/tcpip/stack/iptables_test.go b/pkg/tcpip/stack/iptables_test.go index f4542de0d..32ff6a9ce 100644 --- a/pkg/tcpip/stack/iptables_test.go +++ b/pkg/tcpip/stack/iptables_test.go @@ -18,27 +18,57 @@ import ( "math/rand" "testing" - "gvisor.dev/gvisor/pkg/tcpip" "gvisor.dev/gvisor/pkg/tcpip/faketime" "gvisor.dev/gvisor/pkg/tcpip/header" + "gvisor.dev/gvisor/pkg/tcpip/testutil" ) +const ( + nattedDstPort = 1 + srcPort = 2 + dstPort = 3 + + // The network protocol used for these tests doesn't matter as the tests are + // not targetting anything protocol specific. + ipv6 = true + netProto = header.IPv6ProtocolNumber +) + +var ( + nattedDstAddr = testutil.MustParse6("a::1") + srcAddr = testutil.MustParse6("b::2") + dstAddr = testutil.MustParse6("c::3") +) + +func v6PacketBuffer() *PacketBuffer { + pkt := NewPacketBuffer(PacketBufferOptions{ + ReserveHeaderBytes: header.IPv6MinimumSize + header.UDPMinimumSize, + }) + udp := header.UDP(pkt.TransportHeader().Push(header.UDPMinimumSize)) + udp.SetSourcePort(srcPort) + udp.SetDestinationPort(dstPort) + udp.SetChecksum(0) + udp.SetChecksum(^udp.CalculateChecksum(header.PseudoHeaderChecksum( + header.UDPProtocolNumber, + srcAddr, + dstAddr, + uint16(len(udp)), + ))) + pkt.TransportProtocolNumber = header.UDPProtocolNumber + ip := header.IPv6(pkt.NetworkHeader().Push(header.IPv6MinimumSize)) + ip.Encode(&header.IPv6Fields{ + PayloadLength: uint16(len(udp)), + TransportProtocol: header.UDPProtocolNumber, + HopLimit: 64, + SrcAddr: srcAddr, + DstAddr: dstAddr, + }) + pkt.NetworkProtocolNumber = header.IPv6ProtocolNumber + return pkt +} + // TestNATedConnectionReap tests that NATed connections are properly reaped. func TestNATedConnectionReap(t *testing.T) { - // Note that the network protocol used for this test doesn't matter as this - // test focuses on reaping, not anything related to a specific network - // protocol. - - const ( - nattedDstPort = 1 - srcPort = 2 - dstPort = 3 - - nattedDstAddr = tcpip.Address("\x0a\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01") - srcAddr = tcpip.Address("\x0b\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02") - dstAddr = tcpip.Address("\x0c\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x03") - ) - clock := faketime.NewManualClock() iptables := DefaultTables(clock, rand.New(rand.NewSource(0 /* seed */))) @@ -46,7 +76,7 @@ func TestNATedConnectionReap(t *testing.T) { Rules: []Rule{ // Prerouting { - Target: &DNATTarget{NetworkProtocol: header.IPv6ProtocolNumber, Addr: nattedDstAddr, Port: nattedDstPort}, + Target: &DNATTarget{NetworkProtocol: netProto, Addr: nattedDstAddr, Port: nattedDstPort}, }, { Target: &AcceptTarget{}, @@ -80,7 +110,7 @@ func TestNATedConnectionReap(t *testing.T) { Postrouting: 5, }, } - if err := iptables.ReplaceTable(NATID, table, true /* ipv6 */); err != nil { + if err := iptables.ReplaceTable(NATID, table, ipv6); err != nil { t.Fatalf("ipt.ReplaceTable(%d, _, true): %s", NATID, err) } @@ -88,29 +118,7 @@ func TestNATedConnectionReap(t *testing.T) { // on the first change to IPTables. iptables.reaperDone <- struct{}{} - pkt := NewPacketBuffer(PacketBufferOptions{ - ReserveHeaderBytes: header.IPv6MinimumSize + header.UDPMinimumSize, - }) - udp := header.UDP(pkt.TransportHeader().Push(header.UDPMinimumSize)) - udp.SetSourcePort(srcPort) - udp.SetDestinationPort(dstPort) - udp.SetChecksum(0) - udp.SetChecksum(^udp.CalculateChecksum(header.PseudoHeaderChecksum( - header.UDPProtocolNumber, - srcAddr, - dstAddr, - uint16(len(udp)), - ))) - pkt.TransportProtocolNumber = header.UDPProtocolNumber - ip := header.IPv6(pkt.NetworkHeader().Push(header.IPv6MinimumSize)) - ip.Encode(&header.IPv6Fields{ - PayloadLength: uint16(len(udp)), - TransportProtocol: header.UDPProtocolNumber, - HopLimit: 64, - SrcAddr: srcAddr, - DstAddr: dstAddr, - }) - pkt.NetworkProtocolNumber = header.IPv6ProtocolNumber + pkt := v6PacketBuffer() originalTID, _, ok := getTupleID(pkt) if !ok { @@ -219,3 +227,93 @@ func TestNATedConnectionReap(t *testing.T) { checkNoTupleInBucket(originalBkt, originalTID, false /* reply */) checkNoTupleInBucket(replyBkt, replyTID, true /* reply */) } + +// TestNATAlwaysPerformed tests that a connection will have a noop-NAT +// performed on it when no rule matches its associated packet. +func TestNATAlwaysPerformed(t *testing.T) { + tests := []struct { + name string + dnatHook func(*testing.T, *IPTables, *PacketBuffer) + snatHook func(*testing.T, *IPTables, *PacketBuffer) + }{ + { + name: "Prerouting and Input", + dnatHook: func(t *testing.T, iptables *IPTables, pkt *PacketBuffer) { + t.Helper() + + if !iptables.CheckPrerouting(pkt, nil /* addressEP */, "" /* inNicName */) { + t.Fatal("got iptables.CheckPrerouting(...) = false, want = true") + } + }, + snatHook: func(t *testing.T, iptables *IPTables, pkt *PacketBuffer) { + t.Helper() + + if !iptables.CheckInput(pkt, "" /* inNicName */) { + t.Fatal("got iptables.CheckInput(...) = false, want = true") + } + }, + }, + { + name: "Output and Postrouting", + dnatHook: func(t *testing.T, iptables *IPTables, pkt *PacketBuffer) { + t.Helper() + + // Output hook depends on a route but if the route is local, we don't + // need anything else from it. + r := Route{ + routeInfo: routeInfo{ + Loop: PacketLoop, + }, + } + if !iptables.CheckOutput(pkt, &r, "" /* outNicName */) { + t.Fatal("got iptables.CheckOutput(...) = false, want = true") + } + }, + snatHook: func(t *testing.T, iptables *IPTables, pkt *PacketBuffer) { + t.Helper() + + // Postrouting hook depends on a route but if the route is local, we + // don't need anything else from it. + r := Route{ + routeInfo: routeInfo{ + Loop: PacketLoop, + }, + } + if !iptables.CheckPostrouting(pkt, &r, nil /* addressEP */, "" /* outNicName */) { + t.Fatal("got iptables.CheckPostrouting(...) = false, want = true") + } + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + clock := faketime.NewManualClock() + iptables := DefaultTables(clock, rand.New(rand.NewSource(0 /* seed */))) + + // Just to make sure the iptables is not short circuited. + if err := iptables.ReplaceTable(NATID, iptables.GetTable(NATID, ipv6), ipv6); err != nil { + t.Fatalf("ipt.ReplaceTable(%d, _, true): %s", NATID, err) + } + + pkt := v6PacketBuffer() + + test.dnatHook(t, iptables, pkt) + conn := pkt.tuple.conn + conn.mu.RLock() + destManip := conn.destinationManip + conn.mu.RUnlock() + if destManip != manipPerformedNoop { + t.Errorf("got destManip = %d, want = %d", destManip, manipPerformedNoop) + } + + test.snatHook(t, iptables, pkt) + conn.mu.RLock() + srcManip := conn.sourceManip + conn.mu.RUnlock() + if srcManip != manipPerformedNoop { + t.Errorf("got destManip = %d, want = %d", destManip, manipPerformedNoop) + } + }) + } +}