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..5b52540d3e 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, n.firewallerNetwork) + if err != nil { + return nil, err + } + defer func() { + if retErr != nil { + if err := pm.UnmapPorts(ctx, bindings, n.firewallerNetwork); 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. @@ -343,6 +416,15 @@ func (n *bridgeNetwork) unmapPBs(ctx context.Context, bindings []portmapperapi.P if err := pm.UnmapPorts(ctx, []portmapperapi.PortBinding{b}, n.firewallerNetwork); 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..3bd5725adf 100644 --- a/daemon/libnetwork/drivers/bridge/port_mapping_linux_test.go +++ b/daemon/libnetwork/drivers/bridge/port_mapping_linux_test.go @@ -563,7 +563,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 +572,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 +716,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 +766,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 +787,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 +888,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 +901,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. 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/portmappers/nat/mapper_linux.go b/daemon/libnetwork/portmappers/nat/mapper_linux.go index e411e68f45..002ba4c8ee 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,24 +29,18 @@ 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, } } @@ -102,42 +93,16 @@ 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 } @@ -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).