From 7243860557731efe55b8dcb7cdeaff0dd18e4274 Mon Sep 17 00:00:00 2001 From: Rob Murray Date: Wed, 12 Feb 2025 11:29:21 +0000 Subject: [PATCH] Include per-port rules in iptablesNetwork Signed-off-by: Rob Murray --- libnetwork/drivers/bridge/bridge_linux.go | 23 ---- .../drivers/bridge/port_mapping_linux.go | 103 ++++++++++++------ .../drivers/bridge/port_mapping_linux_test.go | 5 + .../bridge/setup_ip_tables_linux_test.go | 19 +--- 4 files changed, 80 insertions(+), 70 deletions(-) diff --git a/libnetwork/drivers/bridge/bridge_linux.go b/libnetwork/drivers/bridge/bridge_linux.go index 834d3e3060..24fd34471d 100644 --- a/libnetwork/drivers/bridge/bridge_linux.go +++ b/libnetwork/drivers/bridge/bridge_linux.go @@ -391,21 +391,6 @@ func parseErr(label, value, errString string) error { return types.InvalidParameterErrorf("failed to parse %s value: %v (%s)", label, value, errString) } -func (n *bridgeNetwork) iptablesEnabled(version iptables.IPVersion) (bool, error) { - n.Lock() - defer n.Unlock() - if n.driver == nil { - return false, types.InvalidParameterErrorf("no driver found") - } - - n.driver.Lock() - defer n.driver.Unlock() - if version == iptables.IPv6 { - return n.driver.config.EnableIP6Tables, nil - } - return n.driver.config.EnableIPTables, nil -} - func (n *bridgeNetwork) newIptablesNetwork() (*iptablesNetwork, error) { config4, err := makeNetworkConfigFam(n.config.HostIPv4, n.bridge.bridgeIPv4, n.gwMode(iptables.IPv4)) if err != nil { @@ -451,14 +436,6 @@ func makeNetworkConfigFam(hostIP net.IP, bridgePrefix *net.IPNet, gwm gwMode) (n return c, nil } -func (n *bridgeNetwork) getNetworkBridgeName() string { - n.Lock() - config := n.config - n.Unlock() - - return config.BridgeName -} - func (n *bridgeNetwork) getNATDisabled() (ipv4, ipv6 bool) { n.Lock() defer n.Unlock() diff --git a/libnetwork/drivers/bridge/port_mapping_linux.go b/libnetwork/drivers/bridge/port_mapping_linux.go index f1dd719b1f..e70795045e 100644 --- a/libnetwork/drivers/bridge/port_mapping_linux.go +++ b/libnetwork/drivers/bridge/port_mapping_linux.go @@ -203,9 +203,10 @@ func (n *bridgeNetwork) addPortMappings( return nil, err } } - if err := n.setPerPortIptables(ctx, b, true); err != nil { - return nil, err - } + } + + if err := n.iptablesNetwork.AddPorts(ctx, mergeChildHostIPs(bindings)); err != nil { + return nil, err } // Now the iptables rules are set up, it's safe to start the userland proxy. @@ -322,6 +323,21 @@ func needSamePort(a, b types.PortBinding) bool { a.IP.Equal(b.IP) } +// 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 []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. @@ -745,9 +761,11 @@ func (n *bridgeNetwork) releasePortBindings(pbs []portBinding) error { errs = append(errs, fmt.Errorf("failed to stop userland proxy for port mapping %s: %w", pb, err)) } } - if err := n.setPerPortIptables(context.TODO(), pb, false); err != nil { - errs = append(errs, fmt.Errorf("failed to remove iptables rules for port mapping %s: %w", pb, err)) - } + } + if err := n.iptablesNetwork.DelPorts(context.TODO(), mergeChildHostIPs(pbs)); err != nil { + errs = append(errs, err) + } + for _, pb := range pbs { if pb.HostPort > 0 { portallocator.Get().ReleasePort(pb.childHostIP, pb.Proto.String(), int(pb.HostPort)) } @@ -755,17 +773,39 @@ func (n *bridgeNetwork) releasePortBindings(pbs []portBinding) error { return errors.Join(errs...) } -func (n *bridgeNetwork) setPerPortIptables(ctx context.Context, b portBinding, enable bool) error { +func (n *iptablesNetwork) AddPorts(ctx context.Context, pbs []types.PortBinding) error { + return n.modPorts(ctx, pbs, true) +} + +func (n *iptablesNetwork) DelPorts(ctx context.Context, pbs []types.PortBinding) error { + return n.modPorts(ctx, pbs, false) +} + +func (n *iptablesNetwork) modPorts(ctx context.Context, pbs []types.PortBinding, enable bool) error { + for _, pb := range pbs { + if err := n.setPerPortIptables(ctx, pb, enable); err != nil { + return err + } + } + return nil +} + +func (n *iptablesNetwork) setPerPortIptables(ctx context.Context, b types.PortBinding, enable bool) error { v := iptables.IPv4 + enabled := n.Enable4 + config := n.Config4 if b.IP.To4() == nil { v = iptables.IPv6 + enabled = n.Enable6 + config = n.Config6 } - if enabled, err := n.iptablesEnabled(v); err != nil || !enabled { + + if !enabled { // Nothing to do, iptables/ip6tables is not enabled. return nil } - if err := n.filterPortMappedOnLoopback(ctx, b, enable); err != nil { + if err := filterPortMappedOnLoopback(ctx, b, b.HostIP, enable); err != nil { return err } @@ -780,21 +820,19 @@ func (n *bridgeNetwork) setPerPortIptables(ctx context.Context, b portBinding, e return nil } - bridgeName := n.getNetworkBridgeName() - proxyPath := n.userlandProxyPath() - if err := setPerPortNAT(b, v, proxyPath, bridgeName, enable); err != nil { + if err := n.setPerPortNAT(v, b, enable); err != nil { return err } - if !n.gwMode(v).unprotected() { - if err := setPerPortForwarding(b, v, bridgeName, enable); err != nil { + if !config.Unprotected { + if err := setPerPortForwarding(b, v, n.IfName, enable); err != nil { return err } } return nil } -func setPerPortNAT(b portBinding, ipv iptables.IPVersion, proxyPath string, bridgeName string, enable bool) error { +func (n *iptablesNetwork) setPerPortNAT(ipv iptables.IPVersion, b types.PortBinding, enable bool) error { if b.HostPort == 0 { // NAT is disabled. return nil @@ -803,8 +841,8 @@ func setPerPortNAT(b portBinding, ipv iptables.IPVersion, proxyPath string, brid // want "0.0.0.0/0". "0/0" is correctly interpreted as "any // value" by both iptables and ip6tables. hostIP := "0/0" - if !b.childHostIP.IsUnspecified() { - hostIP = b.childHostIP.String() + if !b.HostIP.IsUnspecified() { + hostIP = b.HostIP.String() } args := []string{ "-p", b.Proto.String(), @@ -813,9 +851,8 @@ func setPerPortNAT(b portBinding, ipv iptables.IPVersion, proxyPath string, brid "-j", "DNAT", "--to-destination", net.JoinHostPort(b.IP.String(), strconv.Itoa(int(b.Port))), } - hairpinMode := proxyPath == "" - if !hairpinMode { - args = append(args, "!", "-i", bridgeName) + if !n.Hairpin { + args = append(args, "!", "-i", n.IfName) } if ipv == iptables.IPv6 { args = append(args, "!", "-s", "fe80::/10") @@ -832,14 +869,14 @@ func setPerPortNAT(b portBinding, ipv iptables.IPVersion, proxyPath string, brid "--dport", strconv.Itoa(int(b.Port)), "-j", "MASQUERADE", }} - if err := appendOrDelChainRule(rule, "MASQUERADE", hairpinMode && enable); err != nil { + if err := appendOrDelChainRule(rule, "MASQUERADE", n.Hairpin && enable); err != nil { return err } return nil } -func setPerPortForwarding(b portBinding, ipv iptables.IPVersion, bridgeName string, enable bool) error { +func setPerPortForwarding(b types.PortBinding, ipv iptables.IPVersion, bridgeName string, enable bool) error { // Insert rules for open ports at the top of the filter table's DOCKER // chain (a per-network DROP rule, which must come after these per-port // per-container ACCEPT rules, is appended to the chain when the network @@ -881,15 +918,14 @@ func setPerPortForwarding(b portBinding, ipv iptables.IPVersion, bridgeName stri // filterPortMappedOnLoopback adds an iptables rule that drops remote // connections to ports mapped on loopback addresses. // -// This is a no-ip if the portBinding is for IPv6 (IPv6 loopback address is +// This is a no-op if the portBinding is for IPv6 (IPv6 loopback address is // non-routable), or over a network with gw_mode=routed (PBs in routed mode // don't map ports on the host). -func (n *bridgeNetwork) filterPortMappedOnLoopback(ctx context.Context, b portBinding, enable bool) error { +func filterPortMappedOnLoopback(ctx context.Context, b types.PortBinding, hostIP net.IP, enable bool) error { if rawRulesDisabled(ctx) { return nil } - hostIP := b.childHostIP - if b.HostPort == 0 || !hostIP.IsLoopback() || b.childHostIP.To4() == nil { + if b.HostPort == 0 || !hostIP.IsLoopback() || hostIP.To4() == nil { return nil } @@ -924,27 +960,28 @@ func (n *bridgeNetwork) filterPortMappedOnLoopback(ctx context.Context, b portBi // mode is "nat". // // This is a no-op if the gw_mode is "nat-unprotected" or "routed". -func (n *bridgeNetwork) filterDirectAccess(ctx context.Context, b portBinding, enable bool) error { +func (n *iptablesNetwork) filterDirectAccess(ctx context.Context, b types.PortBinding, enable bool) error { if rawRulesDisabled(ctx) { return nil } ipv := iptables.IPv4 + config := n.Config4 if b.IP.To4() == nil { ipv = iptables.IPv6 + config = n.Config6 } // gw_mode=nat-unprotected means there's minimal security for NATed ports, // so don't filter direct access. - if n.gwMode(ipv).unprotected() || n.gwMode(ipv).routed() { + if config.Unprotected || config.Routed { return nil } - bridgeName := n.getNetworkBridgeName() drop := iptables.Rule{IPVer: ipv, Table: iptables.Raw, Chain: "PREROUTING", Args: []string{ "-p", b.Proto.String(), "-d", b.IP.String(), // Container IP address "--dport", strconv.Itoa(int(b.Port)), // Container port - "!", "-i", bridgeName, + "!", "-i", n.IfName, "-j", "DROP", }} if err := appendOrDelChainRule(drop, "DIRECT ACCESS FILTERING - DROP", enable); err != nil { @@ -970,9 +1007,7 @@ func (n *bridgeNetwork) reapplyPerPortIptables() { } n.Unlock() - for _, b := range allPBs { - if err := n.setPerPortIptables(context.Background(), b, true); err != nil { - log.G(context.TODO()).Warnf("Failed to reconfigure iptables on firewalld reload %s: %s", b, err) - } + if err := n.iptablesNetwork.AddPorts(context.Background(), mergeChildHostIPs(allPBs)); err != nil { + log.G(context.TODO()).Warnf("Failed to reconfigure NAT: %s", err) } } diff --git a/libnetwork/drivers/bridge/port_mapping_linux_test.go b/libnetwork/drivers/bridge/port_mapping_linux_test.go index d2ef930e89..13bfc2a463 100644 --- a/libnetwork/drivers/bridge/port_mapping_linux_test.go +++ b/libnetwork/drivers/bridge/port_mapping_linux_test.go @@ -830,6 +830,7 @@ func TestAddPortMappings(t *testing.T) { GwModeIPv4: tc.gwMode4, GwModeIPv6: tc.gwMode6, }, + bridge: &bridgeInterface{}, driver: newDriver(storeutils.NewTempStore(t)), } genericOption := map[string]interface{}{ @@ -843,6 +844,10 @@ func TestAddPortMappings(t *testing.T) { } err := n.driver.configure(genericOption) assert.NilError(t, err) + fwn, err := n.newIptablesNetwork() + assert.NilError(t, err) + assert.Check(t, fwn != nil, "no firewaller network") + n.iptablesNetwork = fwn assert.Check(t, is.Equal(n.driver.portDriverClient == nil, !tc.rootless)) expChildIP := func(hostIP net.IP) net.IP { diff --git a/libnetwork/drivers/bridge/setup_ip_tables_linux_test.go b/libnetwork/drivers/bridge/setup_ip_tables_linux_test.go index 4d4ae06583..7e845b369e 100644 --- a/libnetwork/drivers/bridge/setup_ip_tables_linux_test.go +++ b/libnetwork/drivers/bridge/setup_ip_tables_linux_test.go @@ -531,19 +531,12 @@ func TestMirroredWSL2LoopbackFiltering(t *testing.T) { restoreWslinfoPath := simulateWSL2MirroredMode(t, tc.loopback0, tc.wslinfoPerm) defer restoreWslinfoPath() - nw := bridgeNetwork{ - driver: &driver{ - config: configuration{EnableIPTables: true}, - }, - } - err := nw.filterPortMappedOnLoopback(context.TODO(), portBinding{ - PortBinding: types.PortBinding{ - Proto: types.TCP, - IP: net.ParseIP("127.0.0.1"), - HostPort: 8000, - }, - childHostIP: net.ParseIP("127.0.0.1"), - }, true) + hostIP := net.ParseIP("127.0.0.1") + err := filterPortMappedOnLoopback(context.TODO(), types.PortBinding{ + Proto: types.TCP, + IP: hostIP, + HostPort: 8000, + }, hostIP, true) assert.NilError(t, err) // Checking this after trying to create rules, to make sure the init code in iptables/firewalld.go has run.