libnet/pm/nat: move back fw / proxy steps into the bridge driver

Since commit 4e246efcd, individual portmappers are responsible for
setting up firewall rules and proxying according to their needs.

This change moves the responsibility back to the bridge driver, removing
unnecessary code duplication across portmappers. For now, only the nat
portmapper takes advantage of this.

This partially reverts commit 4e246efcd.

Signed-off-by: Albin Kerouanton <albinker@gmail.com>
This commit is contained in:
Albin Kerouanton
2025-08-08 17:36:14 +02:00
parent 268e636b2b
commit 9d9b05446c
6 changed files with 184 additions and 132 deletions

View File

@@ -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,
},

View File

@@ -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

View File

@@ -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(),
}
}

View File

@@ -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.

View File

@@ -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)
}

View File

@@ -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).