diff --git a/integration/networking/port_mapping_linux_test.go b/integration/networking/port_mapping_linux_test.go index deb110dcc9..f1405f4f65 100644 --- a/integration/networking/port_mapping_linux_test.go +++ b/integration/networking/port_mapping_linux_test.go @@ -1282,3 +1282,25 @@ func TestSkipRawRules(t *testing.T) { }) } } + +// Regression test for https://github.com/docker/compose/issues/12846 +func TestMixAnyWithSpecificHostAddrs(t *testing.T) { + ctx := setupTest(t) + // Start a new daemon, so the port allocator will start with new/empty ephemeral port ranges, + // making a clash more likely. + d := daemon.New(t) + d.StartWithBusybox(ctx, t) + defer d.Stop(t) + c := d.NewClientT(t) + defer c.Close() + + ctrId := container.Run(ctx, t, c, + container.WithExposedPorts("80/tcp", "81/tcp", "82/tcp"), + container.WithPortMap(nat.PortMap{ + "81/tcp": {{}}, + "82/tcp": {{}}, + "80/tcp": {{HostIP: "127.0.0.1"}}, + }), + ) + defer c.ContainerRemove(ctx, ctrId, containertypes.RemoveOptions{Force: true}) +} diff --git a/libnetwork/drivers/bridge/port_mapping_linux.go b/libnetwork/drivers/bridge/port_mapping_linux.go index 18fb80fa9f..c3ceb228d1 100644 --- a/libnetwork/drivers/bridge/port_mapping_linux.go +++ b/libnetwork/drivers/bridge/port_mapping_linux.go @@ -15,6 +15,7 @@ import ( "syscall" "github.com/containerd/log" + "github.com/docker/docker/libnetwork/drivers/bridge/internal/firewaller" "github.com/docker/docker/libnetwork/drivers/bridge/internal/rlkclient" "github.com/docker/docker/libnetwork/netutils" "github.com/docker/docker/libnetwork/portallocator" @@ -103,7 +104,7 @@ func (n *bridgeNetwork) addPortMappings( defer func() { if retErr != nil { - if err := n.releasePortBindings(bindings); err != nil { + if err := releasePortBindings(bindings, n.firewallerNetwork); err != nil { log.G(ctx).Warnf("Release port bindings: %s", err.Error()) } } @@ -167,7 +168,7 @@ func (n *bridgeNetwork) addPortMappings( } // Allocate and bind a host port. - newB, err := bindHostPorts(ctx, toBind, proxyPath) + newB, err := bindHostPorts(ctx, toBind, proxyPath, pdc, n.firewallerNetwork) if err != nil { return nil, err } @@ -177,66 +178,12 @@ func (n *bridgeNetwork) addPortMappings( toBind = toBind[:0] } - for i := range bindings { - b := bindings[i] - if pdc != nil && b.HostPort != 0 { - var err error - hip, ok := netip.AddrFromSlice(b.HostIP) - if !ok { - return nil, fmt.Errorf("invalid host IP address in %s", b) - } - chip, ok := netip.AddrFromSlice(b.childHostIP) - if !ok { - return nil, fmt.Errorf("invalid child host IP address %s in %s", b.childHostIP, b) - } - bindings[i].portDriverRemove, err = pdc.AddPort(ctx, b.Proto.String(), hip, chip, int(b.HostPort)) - if err != nil { - var pErr *rlkclient.ProtocolUnsupportedError - if errors.As(err, &pErr) { - log.G(ctx).WithFields(log.Fields{ - "error": pErr, - }).Warnf("discarding request for %q", net.JoinHostPort(hip.String(), strconv.Itoa(int(b.HostPort)))) - bindings[i].rootlesskitUnsupported = true - continue - } - return nil, err - } - } - } - - if err := n.firewallerNetwork.AddPorts(ctx, mergeChildHostIPs(bindings)); err != nil { - return nil, err - } - - // Now the iptables rules are set up, it's safe to start the userland proxy. - // (If it was started before the iptables rules were created, it may have - // accepted a connection, then become unreachable due to NAT rules sending - // packets directly to the container.) - // If not starting the proxy, nothing will ever accept a connection on the - // socket. But, listen anyway so that the binding shows up in "netstat -at". - somaxconn := 0 + // Start userland proxy processes. if proxyPath != "" { - somaxconn = -1 // silently capped to "/proc/sys/net/core/somaxconn" - } - for i := range bindings { - if bindings[i].boundSocket == nil || bindings[i].rootlesskitUnsupported { - continue - } - if bindings[i].Proto == types.TCP { - rc, err := bindings[i].boundSocket.SyscallConn() - if err != nil { - return nil, fmt.Errorf("raw conn not available on TCP socket: %w", err) + for i := range bindings { + if bindings[i].boundSocket == nil || bindings[i].rootlesskitUnsupported || bindings[i].stopProxy != nil { + continue } - if errC := rc.Control(func(fd uintptr) { - err = syscall.Listen(int(fd), somaxconn) - }); errC != nil { - return nil, fmt.Errorf("failed to Control TCP socket: %w", err) - } - if err != nil { - return nil, fmt.Errorf("failed to listen on TCP socket: %w", err) - } - } - if proxyPath != "" { var err error bindings[i].stopProxy, err = startProxy( bindings[i].childPortBinding(), proxyPath, bindings[i].boundSocket, @@ -468,6 +415,8 @@ func bindHostPorts( ctx context.Context, cfg []portBindingReq, proxyPath string, + pdc portDriverClient, + fwn firewaller.Network, ) ([]portBinding, error) { if len(cfg) == 0 { return nil, nil @@ -486,16 +435,19 @@ func bindHostPorts( var err error for i := 0; i < maxAllocatePortAttempts; i++ { var b []portBinding - b, err = attemptBindHostPorts(ctx, cfg, proto.String(), hostPort, hostPortEnd, proxyPath) + b, err = attemptBindHostPorts(ctx, cfg, proto.String(), hostPort, hostPortEnd, proxyPath, pdc, fwn) if err == nil { return b, nil } // There is no point in immediately retrying to map an explicitly chosen port. if hostPort != 0 && hostPort == hostPortEnd { - log.G(ctx).Warnf("Failed to allocate and map port: %s", err) + log.G(ctx).WithError(err).Warnf("Failed to allocate and map port") break } - log.G(ctx).Warnf("Failed to allocate and map port: %s, retry: %d", err, i+1) + log.G(ctx).WithFields(log.Fields{ + "error": err, + "attempt": i + 1, + }).Warn("Failed to allocate and map port") } return nil, err } @@ -517,6 +469,8 @@ func attemptBindHostPorts( proto string, hostPortStart, hostPortEnd uint16, proxyPath string, + pdc portDriverClient, + fwn firewaller.Network, ) (_ []portBinding, retErr error) { var err error var port int @@ -546,20 +500,8 @@ func attemptBindHostPorts( res := make([]portBinding, 0, len(cfg)) defer func() { if retErr != nil { - for _, pb := range res { - if pb.boundSocket != nil { - if err := pb.boundSocket.Close(); err != nil { - log.G(ctx).Warnf("Failed to close port binding for %s: %s", pb, err) - } - } - // TODO(robmry) - this is only needed because the userland proxy may have - // been started for SCTP. If a bound socket is passed to the proxy after - // iptables rules have been configured (as it is for TCP/UDP), remove this. - if pb.stopProxy != nil { - if err := pb.stopProxy(); err != nil { - log.G(ctx).Warnf("Failed to stop proxy for %s: %s", pb, err) - } - } + if err := releasePortBindings(res, fwn); err != nil { + log.G(ctx).WithError(err).Warn("Failed to release port bindings") } } }() @@ -588,11 +530,10 @@ func attemptBindHostPorts( // to the userland proxy, because the proxy is not able to convert the // file descriptor into an sctp.SCTPListener (fd is an unexported member // of the struct, and ListenSCTP is the only constructor). - // So, it is possible for the proxy to start listening and accept + // If that changes, remove this. + // Until then, it is possible for the proxy to start listening and accept // connections before iptables rules are created that would bypass // the proxy for external connections. - // Remove this and pb.stopProxy() from the cleanup function above if - // this is fixed. pb, err = startSCTPProxy(c, port, proxyPath) } default: @@ -604,6 +545,24 @@ func attemptBindHostPorts( } res = append(res, pb) } + + if err := configPortDriver(ctx, res, pdc); err != nil { + return nil, err + } + if err := fwn.AddPorts(ctx, mergeChildHostIPs(res)); err != nil { + return nil, err + } + // Now the firewall rules are set up, it's safe to listen on the socket. (Listening + // earlier could result in dropped connections if the proxy becomes unreachable due + // to NAT rules sending packets directly to the container.) + // + // If not starting the proxy, nothing will ever accept a connection on the + // socket. Listen here anyway because SO_REUSEADDR is set, so bind() won't notice + // the problem if a port's bound to both INADDR_ANY and a specific address. (Also + // so the binding shows up in "netstat -at".) + if err := tcpListenBoundPorts(res, proxyPath); err != nil { + return nil, err + } return res, nil } @@ -732,6 +691,64 @@ func startSCTPProxy(cfg portBindingReq, port int, proxyPath string) (_ portBindi return pb, nil } +// 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). +func configPortDriver(ctx context.Context, pbs []portBinding, pdc portDriverClient) error { + for i := range pbs { + b := pbs[i] + if pdc != nil && b.HostPort != 0 { + var err error + hip, ok := netip.AddrFromSlice(b.HostIP) + if !ok { + return fmt.Errorf("invalid host IP address in %s", b) + } + chip, ok := netip.AddrFromSlice(b.childHostIP) + if !ok { + return fmt.Errorf("invalid child host IP address %s in %s", b.childHostIP, b) + } + pbs[i].portDriverRemove, err = pdc.AddPort(ctx, b.Proto.String(), hip, chip, int(b.HostPort)) + if err != nil { + var pErr *rlkclient.ProtocolUnsupportedError + if errors.As(err, &pErr) { + log.G(ctx).WithFields(log.Fields{ + "error": pErr, + }).Warnf("discarding request for %q", net.JoinHostPort(hip.String(), strconv.Itoa(int(b.HostPort)))) + pbs[i].rootlesskitUnsupported = true + continue + } + return err + } + } + } + return nil +} + +func tcpListenBoundPorts(pbs []portBinding, proxyPath string) error { + somaxconn := 0 + if proxyPath != "" { + somaxconn = -1 // silently capped to "/proc/sys/net/core/somaxconn" + } + for i := range pbs { + if pbs[i].boundSocket == nil || pbs[i].rootlesskitUnsupported || pbs[i].Proto != types.TCP { + continue + } + rc, err := pbs[i].boundSocket.SyscallConn() + if err != nil { + return fmt.Errorf("raw conn not available on TCP socket: %w", err) + } + if errC := rc.Control(func(fd uintptr) { + err = syscall.Listen(int(fd), somaxconn) + }); errC != nil { + return fmt.Errorf("failed to Control TCP socket: %w", err) + } + if err != nil { + return fmt.Errorf("failed to listen on TCP socket: %w", err) + } + } + return nil +} + // releasePorts attempts to release all port bindings, does not stop on failure func (n *bridgeNetwork) releasePorts(ep *bridgeEndpoint) error { n.Lock() @@ -739,10 +756,10 @@ func (n *bridgeNetwork) releasePorts(ep *bridgeEndpoint) error { ep.portMapping = nil n.Unlock() - return n.releasePortBindings(pbs) + return releasePortBindings(pbs, n.firewallerNetwork) } -func (n *bridgeNetwork) releasePortBindings(pbs []portBinding) error { +func releasePortBindings(pbs []portBinding, fwn firewaller.Network) error { var errs []error for _, pb := range pbs { if pb.boundSocket != nil { @@ -761,7 +778,7 @@ func (n *bridgeNetwork) releasePortBindings(pbs []portBinding) error { } } } - if err := n.firewallerNetwork.DelPorts(context.TODO(), mergeChildHostIPs(pbs)); err != nil { + if err := fwn.DelPorts(context.TODO(), mergeChildHostIPs(pbs)); err != nil { errs = append(errs, err) } for _, pb := range pbs { diff --git a/libnetwork/drivers/bridge/port_mapping_linux_test.go b/libnetwork/drivers/bridge/port_mapping_linux_test.go index c2bf4db55f..28f3aeaf9a 100644 --- a/libnetwork/drivers/bridge/port_mapping_linux_test.go +++ b/libnetwork/drivers/bridge/port_mapping_linux_test.go @@ -263,7 +263,7 @@ func TestBindHostPortsError(t *testing.T) { }, }, } - pbs, err := bindHostPorts(context.Background(), cfg, "") + pbs, err := bindHostPorts(context.Background(), cfg, "", nil, nil) assert.Check(t, is.Error(err, "port binding mismatch 80/tcp:8080-8080, 80/tcp:8080-8081")) assert.Check(t, is.Nil(pbs)) }