diff --git a/daemon/daemon_unix.go b/daemon/daemon_unix.go index eb74c88b55..9ee29059cb 100644 --- a/daemon/daemon_unix.go +++ b/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, }, diff --git a/daemon/libnetwork/drivers/bridge/bridge_linux.go b/daemon/libnetwork/drivers/bridge/bridge_linux.go index 32f002e20d..7e193e41a8 100644 --- a/daemon/libnetwork/drivers/bridge/bridge_linux.go +++ b/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 diff --git a/daemon/libnetwork/drivers/bridge/port_mapping_linux.go b/daemon/libnetwork/drivers/bridge/port_mapping_linux.go index f0cb7a117f..81307cb84a 100644 --- a/daemon/libnetwork/drivers/bridge/port_mapping_linux.go +++ b/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(), + } +} diff --git a/daemon/libnetwork/drivers/bridge/port_mapping_linux_test.go b/daemon/libnetwork/drivers/bridge/port_mapping_linux_test.go index 3b33186b49..fa7e77071d 100644 --- a/daemon/libnetwork/drivers/bridge/port_mapping_linux_test.go +++ b/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]. diff --git a/daemon/libnetwork/drivers_linux.go b/daemon/libnetwork/drivers_linux.go index d5037b86a0..a4be36ed51 100644 --- a/daemon/libnetwork/drivers_linux.go +++ b/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) } diff --git a/daemon/libnetwork/drvregistry/portmappers_test.go b/daemon/libnetwork/drvregistry/portmappers_test.go index 30c3a8a183..ad05f952c7 100644 --- a/daemon/libnetwork/drvregistry/portmappers_test.go +++ b/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 } diff --git a/daemon/libnetwork/portmapperapi/api.go b/daemon/libnetwork/portmapperapi/api.go index 932a70feac..40531248b0 100644 --- a/daemon/libnetwork/portmapperapi/api.go +++ b/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. diff --git a/daemon/libnetwork/portmapperapi/firewaller.go b/daemon/libnetwork/portmapperapi/firewaller.go deleted file mode 100644 index e09484147e..0000000000 --- a/daemon/libnetwork/portmapperapi/firewaller.go +++ /dev/null @@ -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 -} diff --git a/daemon/libnetwork/portmappers/nat/mapper_linux.go b/daemon/libnetwork/portmappers/nat/mapper_linux.go index e411e68f45..aa175e018c 100644 --- a/daemon/libnetwork/portmappers/nat/mapper_linux.go +++ b/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). diff --git a/daemon/libnetwork/portmappers/nat/mapper_linux_test.go b/daemon/libnetwork/portmappers/nat/mapper_linux_test.go index 2edf6c0489..e5b1882841 100644 --- a/daemon/libnetwork/portmappers/nat/mapper_linux_test.go +++ b/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)) } diff --git a/daemon/libnetwork/portmappers/routed/mapper_linux.go b/daemon/libnetwork/portmappers/routed/mapper_linux.go index c3ed62d886..5c4ad5d1d5 100644 --- a/daemon/libnetwork/portmappers/routed/mapper_linux.go +++ b/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 }