Merge pull request #50706 from akerouanton/move-back-fw-setup-into-bridge-driver

libnet/pmapi: let portmappers specify NAT/fwding rules

daemon/daemon_unix.go

@@ -933,7 +933,8 @@ func networkPlatformOptions(conf *config.Config) []nwconfig.Option {
 				"DisableFilterForwardDrop": conf.BridgeConfig.DisableFilterForwardDrop,
 				"EnableIPTables":           conf.BridgeConfig.EnableIPTables,
 				"EnableIP6Tables":          conf.BridgeConfig.EnableIP6Tables,
-				"Hairpin":                  !conf.EnableUserlandProxy || conf.UserlandProxyPath == "",
+				"EnableProxy":              conf.EnableUserlandProxy && conf.UserlandProxyPath != "",
+				"ProxyPath":                conf.UserlandProxyPath,
 				"AllowDirectRouting":       conf.BridgeConfig.AllowDirectRouting,
 				"AcceptFwMark":             conf.BridgeConfig.BridgeAcceptFwMark,
 			},

daemon/libnetwork/drivers/bridge/bridge_linux.go

@@ -69,10 +69,11 @@ type configuration struct {
 	DisableFilterForwardDrop bool
 	EnableIPTables           bool
 	EnableIP6Tables          bool
-	// Hairpin indicates whether packets sent from a container to a host port
-	// published by another container on the same bridge network should be
-	// hairpinned.
-	Hairpin            bool
+	// EnableProxy indicates whether the userland proxy should be used for NAT
+	// port-mappings that can't be fulfilled with firewall rules alone. This
+	// must not be true if ProxyPath is empty.
+	EnableProxy        bool
+	ProxyPath          string
 	AllowDirectRouting bool
 	AcceptFwMark       string
 }
@@ -472,15 +473,6 @@ func (n *bridgeNetwork) gwMode(v firewaller.IPVersion) gwMode {
 	return n.config.GwModeIPv6
 }
 
-func (n *bridgeNetwork) hairpin() bool {
-	n.Lock()
-	defer n.Unlock()
-	if n.driver == nil {
-		return false
-	}
-	return n.driver.config.Hairpin
-}
-
 func (n *bridgeNetwork) portMappers() *drvregistry.PortMappers {
 	n.Lock()
 	defer n.Unlock()
@@ -525,7 +517,7 @@ func (d *driver) configure(option map[string]any) error {
 	d.firewaller, err = newFirewaller(context.Background(), firewaller.Config{
 		IPv4:               config.EnableIPTables,
 		IPv6:               config.EnableIP6Tables,
-		Hairpin:            config.Hairpin,
+		Hairpin:            !config.EnableProxy,
 		AllowDirectRouting: config.AllowDirectRouting,
 		WSL2Mirrored:       isRunningUnderWSL2MirroredMode(context.Background()),
 	})
@@ -853,8 +845,8 @@ func (d *driver) createNetwork(ctx context.Context, config *networkConfiguration
 	}
 
 	// Module br_netfilter needs to be loaded with net.bridge.bridge-nf-call-ip[6]tables
-	// enabled to implement icc=false, or DNAT when hairpin mode is enabled.
-	enableBrNfCallIptables := !config.EnableICC || d.config.Hairpin
+	// enabled to implement icc=false, or DNAT when the userland-proxy is disabled.
+	enableBrNfCallIptables := !config.EnableICC || !d.config.EnableProxy
 
 	// Conditionally queue setup steps depending on configuration values.
 	for _, step := range []struct {
@@ -906,7 +898,7 @@ func (d *driver) createNetwork(ctx context.Context, config *networkConfiguration
 		},
 
 		// Setup Loopback Addresses Routing
-		{d.config.Hairpin, "setupLoopbackAddressesRouting", setupLoopbackAddressesRouting},
+		{!d.config.EnableProxy, "setupLoopbackAddressesRouting", setupLoopbackAddressesRouting},
 
 		// Setup DefaultGatewayIPv4
 		{config.DefaultGatewayIPv4 != nil, "setupGatewayIPv4", setupGatewayIPv4},
@@ -1185,7 +1177,7 @@ func (d *driver) CreateEndpoint(ctx context.Context, nid, eid string, ifInfo dri
 		return fmt.Errorf("adding interface %s to bridge %s failed: %v", hostIfName, config.BridgeName, err)
 	}
 
-	if dconfig.Hairpin {
+	if !dconfig.EnableProxy {
 		err = setHairpinMode(d.nlh, host, true)
 		if err != nil {
 			return err

daemon/libnetwork/drivers/bridge/port_mapping_linux.go

@@ -5,15 +5,22 @@ import (
 	"errors"
 	"fmt"
 	"net"
+	"net/netip"
+	"os"
 	"slices"
 
 	"github.com/containerd/log"
-	"github.com/moby/moby/v2/daemon/internal/sliceutil"
+	"github.com/moby/moby/v2/daemon/libnetwork/drvregistry"
 	"github.com/moby/moby/v2/daemon/libnetwork/netutils"
+	"github.com/moby/moby/v2/daemon/libnetwork/portallocator"
+	"github.com/moby/moby/v2/daemon/libnetwork/portmapper"
 	"github.com/moby/moby/v2/daemon/libnetwork/portmapperapi"
 	"github.com/moby/moby/v2/daemon/libnetwork/types"
 )
 
+// Allow unit tests to supply a dummy StartProxy.
+var startProxy = portmapper.StartProxy
+
 // addPortMappings takes cfg, the configuration for port mappings, selects host
 // ports when ranges are given, binds host ports to check they're available and
 // reserve them, starts docker-proxy if required, and sets up iptables
@@ -72,19 +79,11 @@ func (n *bridgeNetwork) addPortMappings(
 			continue
 		}
 
-		pm, err := pms.Get(c.Mapper)
+		newB, err := n.mapPorts(ctx, pms, toBind)
 		if err != nil {
 			return nil, err
 		}
-
-		newB, err := pm.MapPorts(ctx, toBind, n.firewallerNetwork)
-		if err != nil {
-			return nil, err
-		}
-		bindings = append(bindings, sliceutil.Map(newB, func(b portmapperapi.PortBinding) portmapperapi.PortBinding {
-			b.Mapper = c.Mapper
-			return b
-		})...)
+		bindings = append(bindings, newB...)
 
 		// Reset toBind now the ports are bound.
 		toBind = toBind[:0]
@@ -93,6 +92,96 @@ func (n *bridgeNetwork) addPortMappings(
 	return bindings, nil
 }
 
+// mapPorts calls the port mapper used to map the ports in reqs, applies the firewall rules requested by that portmapper,
+// and starts userland proxies if needed. It returns an error if it fails on any of these steps, and rolls back any
+// changes it made. Caller must ensure that reqs is non-empty and all requests have the same Mapper set.
+func (n *bridgeNetwork) mapPorts(ctx context.Context, pms *drvregistry.PortMappers, reqs []portmapperapi.PortBindingReq) (_ []portmapperapi.PortBinding, retErr error) {
+	mapper := reqs[0].Mapper
+	pm, err := pms.Get(mapper)
+	if err != nil {
+		return nil, err
+	}
+
+	bindings, err := pm.MapPorts(ctx, reqs)
+	if err != nil {
+		return nil, err
+	}
+	defer func() {
+		if retErr != nil {
+			if err := pm.UnmapPorts(ctx, bindings); err != nil {
+				log.G(ctx).WithFields(log.Fields{
+					"bindings": bindings,
+					"error":    err,
+					"origErr":  retErr,
+				}).Warn("Failed to unmap port bindings after error")
+			}
+			return
+		}
+	}()
+
+	for i := range bindings {
+		// Make sure that Mapper is correctly set such that UnmapPorts call the right portmapper.
+		bindings[i].Mapper = mapper
+	}
+
+	fwPorts := collectFirewallPorts(bindings)
+	if err := n.firewallerNetwork.AddPorts(ctx, fwPorts); err != nil {
+		return nil, err
+	}
+	defer func() {
+		if retErr != nil {
+			if err := n.firewallerNetwork.DelPorts(ctx, fwPorts); err != nil {
+				log.G(ctx).WithFields(log.Fields{
+					"bindings": bindings,
+					"error":    err,
+					"origErr":  retErr,
+				}).Warn("Failed to remove firewall rules after error")
+			}
+		}
+	}()
+
+	// Start userland proxy processes.
+	defer func() {
+		if retErr != nil {
+			for _, pb := range bindings {
+				if pb.StopProxy == nil {
+					continue
+				}
+				if err := pb.StopProxy(); err != nil {
+					log.G(ctx).WithFields(log.Fields{
+						"binding": pb.PortBinding,
+						"error":   err,
+					}).Warnf("failed to stop userland proxy for port mapping")
+				}
+			}
+		}
+	}()
+	if n.driver.config.EnableProxy {
+		for i, pb := range bindings {
+			if pb.BoundSocket == nil || pb.RootlesskitUnsupported || pb.StopProxy != nil {
+				continue
+			}
+			if err := portallocator.DetachSocketFilter(bindings[i].BoundSocket); err != nil {
+				return nil, fmt.Errorf("failed to detach socket filter for port mapping %s: %w", bindings[i].PortBinding, err)
+			}
+			var err error
+			bindings[i].StopProxy, err = startProxy(pb.ChildPortBinding(), n.driver.config.ProxyPath, pb.BoundSocket)
+			if err != nil {
+				return nil, fmt.Errorf("failed to start userland proxy for port mapping %s: %w", pb.PortBinding, err)
+			}
+			if err := bindings[i].BoundSocket.Close(); err != nil {
+				log.G(ctx).WithFields(log.Fields{
+					"error":   err,
+					"mapping": pb.PortBinding,
+				}).Warnf("failed to close proxy socket")
+			}
+			bindings[i].BoundSocket = nil
+		}
+	}
+
+	return bindings, nil
+}
+
 // sortAndNormPBs transforms cfg into a list of portBindingReq, with all fields
 // normalized:
 //
@@ -123,7 +212,6 @@ func (n *bridgeNetwork) sortAndNormPBs(
 		containerIPv6 = ep.addrv6.IP
 	}
 
-	hairpin := n.hairpin()
 	disableNAT4, disableNAT6 := n.getNATDisabled()
 
 	add4 := !ep.portBindingState.ipv4 && pbmReq.ipv4 || (disableNAT4 && !ep.portBindingState.routed && pbmReq.routed)
@@ -146,7 +234,7 @@ func (n *bridgeNetwork) sortAndNormPBs(
 		// This change was added to keep backward compatibility
 		containerIP := containerIPv6
 		if containerIPv6 == nil && pbmReq.ipv4 && add6 {
-			if hairpin {
+			if !n.driver.config.EnableProxy {
 				// There's no way to map from host-IPv6 to container-IPv4 with the userland proxy
 				// disabled.
 				// If that is required, don't treat it as an error because, as networks are
@@ -189,21 +277,6 @@ func needSamePort(a, b portmapperapi.PortBindingReq) bool {
 		a.HostPortEnd == b.HostPortEnd
 }
 
-// mergeChildHostIPs take a slice of PortBinding and returns a slice of
-// types.PortBinding, where the HostIP in each of the results has the
-// value of ChildHostIP from the input (if present).
-func mergeChildHostIPs(pbs []portmapperapi.PortBinding) []types.PortBinding {
-	res := make([]types.PortBinding, 0, len(pbs))
-	for _, b := range pbs {
-		pb := b.PortBinding
-		if b.ChildHostIP != nil {
-			pb.HostIP = b.ChildHostIP
-		}
-		res = append(res, pb)
-	}
-	return res
-}
-
 // configurePortBindingIPv4 returns a new port binding with the HostIP field
 // populated and true, if a binding is required. Else, false and an empty
 // binding.
@@ -340,9 +413,18 @@ func (n *bridgeNetwork) unmapPBs(ctx context.Context, bindings []portmapperapi.P
 			continue
 		}
 
-		if err := pm.UnmapPorts(ctx, []portmapperapi.PortBinding{b}, n.firewallerNetwork); err != nil {
+		if err := pm.UnmapPorts(ctx, []portmapperapi.PortBinding{b}); err != nil {
 			errs = append(errs, fmt.Errorf("unmapping port binding %s: %w", b.PortBinding, err))
 		}
+		if b.StopProxy != nil {
+			if err := b.StopProxy(); err != nil && !errors.Is(err, os.ErrProcessDone) {
+				errs = append(errs, fmt.Errorf("unmapping port binding %s: failed to stop userland proxy: %w", b.PortBinding, err))
+			}
+		}
+	}
+
+	if err := n.firewallerNetwork.DelPorts(ctx, collectFirewallPorts(bindings)); err != nil {
+		return err
 	}
 
 	return errors.Join(errs...)
@@ -365,7 +447,55 @@ func (n *bridgeNetwork) reapplyPerPortIptables() {
 		}
 	}
 
-	if err := n.firewallerNetwork.AddPorts(context.Background(), mergeChildHostIPs(allPBs)); err != nil {
+	if err := n.firewallerNetwork.AddPorts(context.Background(), collectFirewallPorts(allPBs)); err != nil {
 		log.G(context.TODO()).Warnf("Failed to reconfigure NAT: %s", err)
 	}
 }
+
+// collectFirewallPorts collects all the types.PortBinding needed to
+// reconfigure the host firewall for a given list of port bindings. If one of
+// the pbs is NATed, but has an invalid NAT field (i.e. multicast address, or a
+// port 0), an error is returned.
+func collectFirewallPorts(pbs []portmapperapi.PortBinding) []types.PortBinding {
+	var fwPBs []types.PortBinding
+	for _, pb := range pbs {
+		if pb.NAT.IsValid() {
+			if pb.NAT.Addr().IsMulticast() || pb.NAT.Port() == 0 {
+				log.G(context.Background()).WithFields(log.Fields{"pb": pb}).Error("invalid NAT address")
+				continue
+			}
+			fwPBs = append(fwPBs, toNATBinding(pb))
+		} else if pb.Forwarding {
+			fwPBs = append(fwPBs, toFwdBinding(pb))
+		}
+	}
+	return fwPBs
+}
+
+// toNATBinding converts a portmapperapi.PortBinding to a types.PortBinding
+// that can be passed to firewaller.Network for setting up a NAT rule.
+func toNATBinding(pb portmapperapi.PortBinding) types.PortBinding {
+	return types.PortBinding{
+		IP:          pb.IP,
+		Port:        pb.Port,
+		Proto:       pb.Proto,
+		HostIP:      pb.NAT.Addr().AsSlice(),
+		HostPort:    pb.NAT.Port(),
+		HostPortEnd: pb.NAT.Port(),
+	}
+}
+
+// toFwdBinding converts a portmapperapi.PortBinding to a types.PortBinding
+// that can be passed to firewaller.Network for setting up forwarding.
+func toFwdBinding(pb portmapperapi.PortBinding) types.PortBinding {
+	unspecAddr := netip.IPv4Unspecified()
+	if pb.IP.To4() == nil {
+		unspecAddr = netip.IPv6Unspecified()
+	}
+	return types.PortBinding{
+		IP:     pb.IP,
+		Port:   pb.Port,
+		Proto:  pb.Proto,
+		HostIP: unspecAddr.AsSlice(),
+	}
+}

daemon/libnetwork/drivers/bridge/port_mapping_linux_test.go

@@ -45,7 +45,6 @@ func TestPortMappingConfig(t *testing.T) {
 
 	config := &configuration{
 		EnableIPTables: true,
-		Hairpin:        true,
 	}
 	genericOption := make(map[string]any)
 	genericOption[netlabel.GenericData] = config
@@ -563,7 +562,6 @@ func TestAddPortMappings(t *testing.T) {
 			cfg: []portmapperapi.PortBindingReq{
 				{PortBinding: types.PortBinding{Proto: types.TCP, Port: 22, HostIP: net.IPv6loopback}},
 			},
-			hairpin: true,
 			expLogs: []string{"Cannot map from IPv6 to an IPv4-only container because the userland proxy is disabled"},
 		},
 		{
@@ -573,7 +571,6 @@ func TestAddPortMappings(t *testing.T) {
 			cfg: []portmapperapi.PortBindingReq{
 				{PortBinding: types.PortBinding{Proto: types.TCP, Port: 22}},
 			},
-			hairpin: true,
 			expLogs: []string{"Cannot map from default host binding address to an IPv4-only container because the userland proxy is disabled"},
 		},
 		{
@@ -718,7 +715,7 @@ func TestAddPortMappings(t *testing.T) {
 
 			// Mock the startProxy function used by the code under test.
 			proxies := map[proxyCall]bool{} // proxy -> is not stopped
-			startProxy := func(pb types.PortBinding, listenSock *os.File) (stop func() error, retErr error) {
+			startProxy = func(pb types.PortBinding, _ string, listenSock *os.File) (stop func() error, retErr error) {
 				if tc.busyPortIPv4 > 0 && tc.busyPortIPv4 == int(pb.HostPort) && pb.HostIP.To4() != nil {
 					return nil, errors.New("busy port")
 				}
@@ -768,9 +765,7 @@ func TestAddPortMappings(t *testing.T) {
 
 			pms := &drvregistry.PortMappers{}
 			err := nat.Register(pms, nat.Config{
-				RlkClient:   pdc,
-				EnableProxy: tc.enableProxy,
-				StartProxy:  startProxy,
+				RlkClient: pdc,
 			})
 			assert.NilError(t, err)
 			err = routed.Register(pms)
@@ -791,7 +786,7 @@ func TestAddPortMappings(t *testing.T) {
 				netlabel.GenericData: &configuration{
 					EnableIPTables:  true,
 					EnableIP6Tables: true,
-					Hairpin:         tc.hairpin,
+					EnableProxy:     tc.enableProxy,
 				},
 			}
 			err = n.driver.configure(genericOption)
@@ -892,8 +887,8 @@ func TestAddPortMappings(t *testing.T) {
 			}
 
 			// Check a docker-proxy was started and stopped for each expected port binding.
+			expProxies := map[proxyCall]bool{}
 			if tc.enableProxy {
-				expProxies := map[proxyCall]bool{}
 				for _, expPB := range tc.expPBs {
 					hip := expChildIP(expPB.HostIP)
 					is4 := hip.To4() != nil
@@ -905,8 +900,8 @@ func TestAddPortMappings(t *testing.T) {
 						expPB.IP, int(expPB.Port))
 					expProxies[p] = tc.expReleaseErr != ""
 				}
-				assert.Check(t, is.DeepEqual(expProxies, proxies))
 			}
+			assert.Check(t, is.DeepEqual(expProxies, proxies))
 
 			// Check the port driver has seen the expected port mappings and no others,
 			// and that they have all been closed.
@@ -995,7 +990,7 @@ type stubPortMapper struct {
 	mapped []portmapperapi.PortBinding
 }
 
-func (pm *stubPortMapper) MapPorts(_ context.Context, reqs []portmapperapi.PortBindingReq, _ portmapperapi.Firewaller) ([]portmapperapi.PortBinding, error) {
+func (pm *stubPortMapper) MapPorts(_ context.Context, reqs []portmapperapi.PortBindingReq) ([]portmapperapi.PortBinding, error) {
 	if len(reqs) == 0 {
 		return []portmapperapi.PortBinding{}, nil
 	}
@@ -1007,7 +1002,7 @@ func (pm *stubPortMapper) MapPorts(_ context.Context, reqs []portmapperapi.PortB
 	return pbs, nil
 }
 
-func (pm *stubPortMapper) UnmapPorts(_ context.Context, reqs []portmapperapi.PortBinding, _ portmapperapi.Firewaller) error {
+func (pm *stubPortMapper) UnmapPorts(_ context.Context, reqs []portmapperapi.PortBinding) error {
 	for _, req := range reqs {
 		// We're only checking for the PortBinding here, not any other
 		// property of [portmapperapi.PortBinding].

daemon/libnetwork/drivers_linux.go

@@ -3,7 +3,6 @@ package libnetwork
 import (
 	"context"
 	"fmt"
-	"os"
 
 	"github.com/moby/moby/v2/daemon/libnetwork/config"
 	"github.com/moby/moby/v2/daemon/libnetwork/datastore"
@@ -16,10 +15,8 @@ import (
 	"github.com/moby/moby/v2/daemon/libnetwork/drivers/overlay"
 	"github.com/moby/moby/v2/daemon/libnetwork/drvregistry"
 	"github.com/moby/moby/v2/daemon/libnetwork/internal/rlkclient"
-	"github.com/moby/moby/v2/daemon/libnetwork/portmapper"
 	"github.com/moby/moby/v2/daemon/libnetwork/portmappers/nat"
 	"github.com/moby/moby/v2/daemon/libnetwork/portmappers/routed"
-	"github.com/moby/moby/v2/daemon/libnetwork/types"
 )
 
 func registerNetworkDrivers(r driverapi.Registerer, store *datastore.Store, pms *drvregistry.PortMappers, driverConfig func(string) map[string]any) error {
@@ -60,13 +57,7 @@ func registerPortMappers(ctx context.Context, r *drvregistry.PortMappers, cfg *c
 		}
 	}
 
-	if err := nat.Register(r, nat.Config{
-		RlkClient: pdc,
-		StartProxy: func(pb types.PortBinding, file *os.File) (func() error, error) {
-			return portmapper.StartProxy(pb, cfg.UserlandProxyPath, file)
-		},
-		EnableProxy: cfg.EnableUserlandProxy && cfg.UserlandProxyPath != "",
-	}); err != nil {
+	if err := nat.Register(r, nat.Config{RlkClient: pdc}); err != nil {
 		return fmt.Errorf("registering nat portmapper: %w", err)
 	}
 

daemon/libnetwork/drvregistry/portmappers_test.go

@@ -10,11 +10,11 @@ import (
 
 type fakePortMapper struct{}
 
-func (f fakePortMapper) MapPorts(_ context.Context, _ []portmapperapi.PortBindingReq, _ portmapperapi.Firewaller) ([]portmapperapi.PortBinding, error) {
+func (f fakePortMapper) MapPorts(_ context.Context, _ []portmapperapi.PortBindingReq) ([]portmapperapi.PortBinding, error) {
 	return nil, nil
 }
 
-func (f fakePortMapper) UnmapPorts(_ context.Context, _ []portmapperapi.PortBinding, _ portmapperapi.Firewaller) error {
+func (f fakePortMapper) UnmapPorts(_ context.Context, _ []portmapperapi.PortBinding) error {
 	return nil
 }
 

daemon/libnetwork/portmapperapi/api.go

@@ -29,10 +29,10 @@ type PortMapper interface {
 	// When an ephemeral port, or a single port from a range is requested
 	// MapPorts should attempt a few times to find a free port available
 	// across all IP addresses.
-	MapPorts(ctx context.Context, reqs []PortBindingReq, fwn Firewaller) ([]PortBinding, error)
+	MapPorts(ctx context.Context, reqs []PortBindingReq) ([]PortBinding, error)
 
 	// UnmapPorts takes a list of port bindings to unmap.
-	UnmapPorts(ctx context.Context, pbs []PortBinding, fwn Firewaller) error
+	UnmapPorts(ctx context.Context, pbs []PortBinding) error
 }
 
 type PortBindingReq struct {
@@ -92,9 +92,25 @@ func (pbReq PortBindingReq) Compare(other PortBindingReq) int {
 }
 
 type PortBinding struct {
+	// PortBinding contains the port binding information reported through the
+	// Engine API.
 	types.PortBinding
 	// Mapper is the name of the port mapper used to process this PortBinding.
 	Mapper string
+
+	// NAT represents the host IP and port that should be NATed to the
+	// container IP and port specified in types.PortBinding. When set, callers
+	// of the port mapper should reconfigure the host firewall. When it's not
+	// set, callers won't reconfigure the host firewall.
+	//
+	// If the address is invalid, or a non-unicast address, or the port is 0,
+	// it's treated as an error. If both Forwarding and NAT are specified, NAT
+	// takes precedence.
+	NAT netip.AddrPort
+	// Forwarding indicates whether callers of the port mapper should update
+	// the host firewall to allow traffic forwarding to IP:Port.
+	Forwarding bool
+
 	// BoundSocket is used to reserve a host port for the binding. If the
 	// userland proxy is in-use, it's passed to the proxy when the proxy is
 	// started, then it's closed and set to nil here.

daemon/libnetwork/portmapperapi/firewaller.go

@@ -1,14 +0,0 @@
-package portmapperapi
-
-import (
-	"context"
-
-	"github.com/moby/moby/v2/daemon/libnetwork/types"
-)
-
-type Firewaller interface {
-	// AddPorts adds the configuration needed for NATing ports.
-	AddPorts(ctx context.Context, pbs []types.PortBinding) error
-	// DelPorts deletes the configuration needed for NATing ports.
-	DelPorts(ctx context.Context, pbs []types.PortBinding) error
-}

daemon/libnetwork/portmappers/nat/mapper_linux.go

@@ -6,7 +6,6 @@ import (
 	"fmt"
 	"net"
 	"net/netip"
-	"os"
 	"strconv"
 
 	"github.com/containerd/log"
@@ -23,8 +22,6 @@ type PortDriverClient interface {
 	AddPort(ctx context.Context, proto string, hostIP, childIP netip.Addr, hostPort int) (func() error, error)
 }
 
-type proxyStarter func(types.PortBinding, *os.File) (func() error, error)
-
 // Register the "nat" port-mapper with libnetwork.
 func Register(r portmapperapi.Registerer, cfg Config) error {
 	return r.Register(driverName, NewPortMapper(cfg))
@@ -32,31 +29,25 @@ func Register(r portmapperapi.Registerer, cfg Config) error {
 
 type PortMapper struct {
 	// pdc is used to interact with rootlesskit port driver.
-	pdc         PortDriverClient
-	startProxy  proxyStarter
-	enableProxy bool
+	pdc PortDriverClient
 }
 
 type Config struct {
 	// RlkClient is called by MapPorts to determine the ChildHostIP and ask
 	// rootlesskit to map ports in its netns.
-	RlkClient   PortDriverClient
-	StartProxy  proxyStarter
-	EnableProxy bool
+	RlkClient PortDriverClient
 }
 
 func NewPortMapper(cfg Config) PortMapper {
 	return PortMapper{
-		pdc:         cfg.RlkClient,
-		startProxy:  cfg.StartProxy,
-		enableProxy: cfg.EnableProxy,
+		pdc: cfg.RlkClient,
 	}
 }
 
 // MapPorts allocates and binds host ports for the given cfg. The caller is
 // responsible for ensuring that all entries in cfg have the same proto,
 // container port, and host port range (their host addresses must differ).
-func (pm PortMapper) MapPorts(ctx context.Context, cfg []portmapperapi.PortBindingReq, fwn portmapperapi.Firewaller) (_ []portmapperapi.PortBinding, retErr error) {
+func (pm PortMapper) MapPorts(ctx context.Context, cfg []portmapperapi.PortBindingReq) (_ []portmapperapi.PortBinding, retErr error) {
 	if len(cfg) == 0 {
 		return nil, nil
 	}
@@ -73,7 +64,7 @@ func (pm PortMapper) MapPorts(ctx context.Context, cfg []portmapperapi.PortBindi
 	bindings := make([]portmapperapi.PortBinding, 0, len(cfg))
 	defer func() {
 		if retErr != nil {
-			if err := pm.UnmapPorts(ctx, bindings, fwn); err != nil {
+			if err := pm.UnmapPorts(ctx, bindings); err != nil {
 				log.G(ctx).WithFields(log.Fields{
 					"pbs":   bindings,
 					"error": err,
@@ -102,47 +93,21 @@ func (pm PortMapper) MapPorts(ctx context.Context, cfg []portmapperapi.PortBindi
 		}
 		pb.PortBinding.HostPort = uint16(allocatedPort)
 		pb.PortBinding.HostPortEnd = pb.HostPort
+
+		childHIP, _ := netip.AddrFromSlice(cfg[i].ChildHostIP)
+		pb.NAT = netip.AddrPortFrom(childHIP, pb.PortBinding.HostPort)
+
 		bindings = append(bindings, pb)
 	}
 
 	if err := configPortDriver(ctx, bindings, pm.pdc); err != nil {
 		return nil, err
 	}
-	if err := fwn.AddPorts(ctx, mergeChildHostIPs(bindings)); err != nil {
-		return nil, err
-	}
-
-	// Start userland proxy processes.
-	if pm.enableProxy {
-		for i := range bindings {
-			if bindings[i].BoundSocket == nil || bindings[i].RootlesskitUnsupported || bindings[i].StopProxy != nil {
-				continue
-			}
-			if err := portallocator.DetachSocketFilter(bindings[i].BoundSocket); err != nil {
-				return nil, fmt.Errorf("failed to detach socket filter for port mapping %s: %w", bindings[i].PortBinding, err)
-			}
-			var err error
-			bindings[i].StopProxy, err = pm.startProxy(
-				bindings[i].ChildPortBinding(), bindings[i].BoundSocket,
-			)
-			if err != nil {
-				return nil, fmt.Errorf("failed to start userland proxy for port mapping %s: %w",
-					bindings[i].PortBinding, err)
-			}
-			if err := bindings[i].BoundSocket.Close(); err != nil {
-				log.G(ctx).WithFields(log.Fields{
-					"error":   err,
-					"mapping": bindings[i].PortBinding,
-				}).Warnf("failed to close proxy socket")
-			}
-			bindings[i].BoundSocket = nil
-		}
-	}
 
 	return bindings, nil
 }
 
-func (pm PortMapper) UnmapPorts(ctx context.Context, pbs []portmapperapi.PortBinding, fwn portmapperapi.Firewaller) error {
+func (pm PortMapper) UnmapPorts(ctx context.Context, pbs []portmapperapi.PortBinding) error {
 	var errs []error
 	for _, pb := range pbs {
 		if pb.BoundSocket != nil {
@@ -155,14 +120,6 @@ func (pm PortMapper) UnmapPorts(ctx context.Context, pbs []portmapperapi.PortBin
 				errs = append(errs, err)
 			}
 		}
-		if pb.StopProxy != nil {
-			if err := pb.StopProxy(); err != nil && !errors.Is(err, os.ErrProcessDone) {
-				errs = append(errs, fmt.Errorf("failed to stop userland proxy: %w", err))
-			}
-		}
-	}
-	if err := fwn.DelPorts(ctx, mergeChildHostIPs(pbs)); err != nil {
-		errs = append(errs, err)
 	}
 	for _, pb := range pbs {
 		portallocator.Get().ReleasePort(pb.ChildHostIP, pb.Proto.String(), int(pb.HostPort))
@@ -180,21 +137,6 @@ func setChildHostIP(pdc PortDriverClient, req portmapperapi.PortBindingReq) port
 	return req
 }
 
-// mergeChildHostIPs take a slice of PortBinding and returns a slice of
-// types.PortBinding, where the HostIP in each of the results has the
-// value of ChildHostIP from the input (if present).
-func mergeChildHostIPs(pbs []portmapperapi.PortBinding) []types.PortBinding {
-	res := make([]types.PortBinding, 0, len(pbs))
-	for _, b := range pbs {
-		pb := b.PortBinding
-		if b.ChildHostIP != nil {
-			pb.HostIP = b.ChildHostIP
-		}
-		res = append(res, pb)
-	}
-	return res
-}
-
 // configPortDriver passes the port binding's details to rootlesskit, and updates the
 // port binding with callbacks to remove the rootlesskit config (or marks the binding as
 // unsupported by rootlesskit).

daemon/libnetwork/portmappers/nat/mapper_linux_test.go

@@ -30,7 +30,7 @@ func TestBindHostPortsError(t *testing.T) {
 		},
 	}
 	pm := &PortMapper{}
-	pbs, err := pm.MapPorts(context.Background(), cfg, nil)
+	pbs, err := pm.MapPorts(context.Background(), cfg)
 	assert.Check(t, is.Error(err, "port binding mismatch 80/tcp:8080-8080, 80/tcp:8080-8081"))
 	assert.Check(t, is.Nil(pbs))
 }

daemon/libnetwork/portmappers/routed/mapper_linux.go

@@ -4,9 +4,7 @@ import (
 	"context"
 
 	"github.com/containerd/log"
-	"github.com/moby/moby/v2/daemon/internal/sliceutil"
 	"github.com/moby/moby/v2/daemon/libnetwork/portmapperapi"
-	"github.com/moby/moby/v2/daemon/libnetwork/types"
 )
 
 const driverName = "routed"
@@ -22,16 +20,19 @@ func NewPortMapper() PortMapper {
 	return PortMapper{}
 }
 
-// MapPorts sets up firewall rules to allow direct remote access to pbs.
-func (pm PortMapper) MapPorts(ctx context.Context, reqs []portmapperapi.PortBindingReq, fwn portmapperapi.Firewaller) ([]portmapperapi.PortBinding, error) {
+// MapPorts returns a PortBinding for every PortBindingReq received, with Forwarding enabled for each. If a HostPort is
+// specified, it's logged and ignored.
+func (pm PortMapper) MapPorts(ctx context.Context, reqs []portmapperapi.PortBindingReq) ([]portmapperapi.PortBinding, error) {
 	if len(reqs) == 0 {
 		return nil, nil
 	}
 
 	res := make([]portmapperapi.PortBinding, 0, len(reqs))
-	bindings := make([]types.PortBinding, 0, len(reqs))
 	for _, c := range reqs {
-		pb := portmapperapi.PortBinding{PortBinding: c.Copy()}
+		pb := portmapperapi.PortBinding{
+			PortBinding: c.Copy(),
+			Forwarding:  true,
+		}
 		if pb.HostPort != 0 || pb.HostPortEnd != 0 {
 			log.G(ctx).WithFields(log.Fields{"mapping": pb}).Infof(
 				"Host port ignored, because NAT is disabled")
@@ -39,19 +40,11 @@ func (pm PortMapper) MapPorts(ctx context.Context, reqs []portmapperapi.PortBind
 			pb.HostPortEnd = 0
 		}
 		res = append(res, pb)
-		bindings = append(bindings, pb.PortBinding)
-	}
-
-	if err := fwn.AddPorts(ctx, bindings); err != nil {
-		return nil, err
 	}
 
 	return res, nil
 }
 
-// UnmapPorts removes firewall rules allowing direct remote access to the pbs.
-func (pm PortMapper) UnmapPorts(ctx context.Context, pbs []portmapperapi.PortBinding, fwn portmapperapi.Firewaller) error {
-	return fwn.DelPorts(ctx, sliceutil.Map(pbs, func(pb portmapperapi.PortBinding) types.PortBinding {
-		return pb.PortBinding
-	}))
+func (pm PortMapper) UnmapPorts(_ context.Context, _ []portmapperapi.PortBinding) error {
+	return nil
 }