From e18e311202b7869b3cde71ae678f968276791e19 Mon Sep 17 00:00:00 2001 From: Andrei Vagin Date: Thu, 13 Jun 2024 01:02:31 -0700 Subject: [PATCH] netstack/iptables: don't require ports for snat rules Ports will be set dynamically. It was implemented in d8772545113f ("Track UDP connections"). PiperOrigin-RevId: 642886517 --- pkg/sentry/socket/netfilter/snat.go | 5 --- test/iptables/nat.go | 57 +++++++++++++++++++++-------- 2 files changed, 41 insertions(+), 21 deletions(-) diff --git a/pkg/sentry/socket/netfilter/snat.go b/pkg/sentry/socket/netfilter/snat.go index 7f7a0e37e..2225e07bf 100644 --- a/pkg/sentry/socket/netfilter/snat.go +++ b/pkg/sentry/socket/netfilter/snat.go @@ -104,11 +104,6 @@ func (*snatTargetMakerV4) unmarshal(buf []byte, filter stack.IPHeaderFilter) (ta return nil, syserr.ErrInvalidArgument } - if nfRange.RangeIPV4.MinPort == 0 { - nflog("snatTargetMakerV4: snat target needs to specify a non-zero port") - return nil, syserr.ErrInvalidArgument - } - if nfRange.RangeIPV4.MinPort != nfRange.RangeIPV4.MaxPort { nflog("snatTargetMakerV4: MinPort != MaxPort (%d, %d)", nfRange.RangeIPV4.MinPort, nfRange.RangeIPV4.MaxPort) return nil, syserr.ErrInvalidArgument diff --git a/test/iptables/nat.go b/test/iptables/nat.go index d1ea2c926..8ca082df3 100644 --- a/test/iptables/nat.go +++ b/test/iptables/nat.go @@ -50,7 +50,9 @@ func init() { RegisterTestCase(&NATPreRECVORIGDSTADDR{}) RegisterTestCase(&NATOutRECVORIGDSTADDR{}) RegisterTestCase(&NATPostSNATUDP{}) + RegisterTestCase(&NATPostSNATUDP{withPort: true}) RegisterTestCase(&NATPostSNATTCP{}) + RegisterTestCase(&NATPostSNATTCP{withPort: true}) RegisterTestCase(&NATOutDNAT{}) RegisterTestCase(&NATOutDNATAddrOnly{}) RegisterTestCase(&NATOutDNATPortOnly{}) @@ -941,22 +943,31 @@ const ( // NATPostSNATUDP tests that the source port/IP in the packets are modified as // expected. It tests the latest-implemented revision of the SNAT target. -type NATPostSNATUDP struct{ localCase } +type NATPostSNATUDP struct { + localCase + withPort bool +} var _ TestCase = (*NATPostSNATUDP)(nil) // Name implements TestCase.Name. -func (*NATPostSNATUDP) Name() string { +func (t *NATPostSNATUDP) Name() string { + if t.withPort { + return "NATPostSNATUDPWithPort" + } return "NATPostSNATUDP" } // ContainerAction implements TestCase.ContainerAction. -func (*NATPostSNATUDP) ContainerAction(ctx context.Context, ip net.IP, ipv6 bool) error { +func (t *NATPostSNATUDP) ContainerAction(ctx context.Context, ip net.IP, ipv6 bool) error { var source string if ipv6 { - source = fmt.Sprintf("[%s]:%d", snatAddrV6, snatPort) + source = fmt.Sprintf("[%s]", snatAddrV6) } else { - source = fmt.Sprintf("%s:%d", snatAddrV4, snatPort) + source = fmt.Sprintf("%s", snatAddrV4) + } + if t.withPort { + source += fmt.Sprintf(":%d", snatPort) } if err := natTable(ipv6, "-A", "POSTROUTING", "-p", "udp", "-j", "SNAT", "--to-source", source); err != nil { @@ -966,7 +977,7 @@ func (*NATPostSNATUDP) ContainerAction(ctx context.Context, ip net.IP, ipv6 bool } // LocalAction implements TestCase.LocalAction. -func (*NATPostSNATUDP) LocalAction(ctx context.Context, ip net.IP, ipv6 bool) error { +func (t *NATPostSNATUDP) LocalAction(ctx context.Context, ip net.IP, ipv6 bool) error { remote, err := listenUDPFrom(ctx, acceptPort, ipv6) if err != nil { return err @@ -980,25 +991,33 @@ func (*NATPostSNATUDP) LocalAction(ctx context.Context, ip net.IP, ipv6 bool) er if got, want := remote.IP, net.ParseIP(snatAddr); !got.Equal(want) { return fmt.Errorf("got remote address = %s, want = %s", got, want) } - if got, want := remote.Port, snatPort; got != want { - return fmt.Errorf("got remote port = %d, want = %d", got, want) + if t.withPort { + if got, want := remote.Port, snatPort; got != want { + return fmt.Errorf("got remote port = %d, want = %d", got, want) + } } return nil } // NATPostSNATTCP tests that the source port/IP in the packets are modified as // expected. It tests the latest-implemented revision of the SNAT target. -type NATPostSNATTCP struct{ localCase } +type NATPostSNATTCP struct { + localCase + withPort bool +} var _ TestCase = (*NATPostSNATTCP)(nil) // Name implements TestCase.Name. -func (*NATPostSNATTCP) Name() string { +func (t *NATPostSNATTCP) Name() string { + if t.withPort { + return "NATPostSNATTCPWithPort" + } return "NATPostSNATTCP" } // ContainerAction implements TestCase.ContainerAction. -func (*NATPostSNATTCP) ContainerAction(ctx context.Context, ip net.IP, ipv6 bool) error { +func (t *NATPostSNATTCP) ContainerAction(ctx context.Context, ip net.IP, ipv6 bool) error { addrs, err := getInterfaceAddrs(ipv6) if err != nil { return err @@ -1007,16 +1026,20 @@ func (*NATPostSNATTCP) ContainerAction(ctx context.Context, ip net.IP, ipv6 bool for _, addr := range addrs { if addr.To4() != nil { if !ipv6 { - source = fmt.Sprintf("%s:%d", addr, snatPort) + source = fmt.Sprintf("%s", addr) } } else if ipv6 && addr.IsGlobalUnicast() { - source = fmt.Sprintf("[%s]:%d", addr, snatPort) + source = fmt.Sprintf("[%s]", addr) } } if source == "" { return fmt.Errorf("can't find any interface address to use") } + if t.withPort { + source += fmt.Sprintf(":%d", snatPort) + } + if err := natTable(ipv6, "-A", "POSTROUTING", "-p", "tcp", "-j", "SNAT", "--to-source", source); err != nil { return err } @@ -1024,7 +1047,7 @@ func (*NATPostSNATTCP) ContainerAction(ctx context.Context, ip net.IP, ipv6 bool } // LocalAction implements TestCase.LocalAction. -func (*NATPostSNATTCP) LocalAction(ctx context.Context, ip net.IP, ipv6 bool) error { +func (t *NATPostSNATTCP) LocalAction(ctx context.Context, ip net.IP, ipv6 bool) error { remote, err := listenTCPFrom(ctx, acceptPort, ipv6) if err != nil { return err @@ -1040,8 +1063,10 @@ func (*NATPostSNATTCP) LocalAction(ctx context.Context, ip net.IP, ipv6 bool) er if err != nil { return err } - if got, want := int(port), snatPort; got != want { - return fmt.Errorf("got remote port = %d, want = %d", got, want) + if t.withPort { + if got, want := int(port), snatPort; got != want { + return fmt.Errorf("got remote port = %d, want = %d", got, want) + } } return nil }