Merge pull request moby#50501 from robmry/nftables_interface

Rework the interface to libnet/internal/nftables

Author: robmry

daemon/libnetwork/drivers/bridge/internal/nftabler/endpoint.go

@@ -25,18 +25,24 @@ func (n *network) DelEndpoint(ctx context.Context, epIPv4, epIPv6 netip.Addr) er
 
 func (n *network) modEndpoint(ctx context.Context, epIPv4, epIPv6 netip.Addr, enable bool) error {
 	if n.fw.config.IPv4 && epIPv4.IsValid() {
-		if err := n.filterDirectAccess(ctx, n.fw.table4, n.config.Config4, epIPv4, enable); err != nil {
-			return err
+		tm := nftables.Modifier{}
+		updater := tm.Create
+		if !enable {
+			updater = tm.Delete
 		}
-		if err := nftApply(ctx, n.fw.table4); err != nil {
+		n.filterDirectAccess(updater, nftables.IPv4, n.config.Config4, epIPv4)
+		if err := n.fw.table4.Apply(ctx, tm); err != nil {
 			return fmt.Errorf("adding rules for bridge %s: %w", n.config.IfName, err)
 		}
 	}
 	if n.fw.config.IPv6 && epIPv6.IsValid() {
-		if err := n.filterDirectAccess(ctx, n.fw.table6, n.config.Config6, epIPv6, enable); err != nil {
-			return err
+		tm := nftables.Modifier{}
+		updater := tm.Create
+		if !enable {
+			updater = tm.Delete
 		}
-		if err := nftApply(ctx, n.fw.table6); err != nil {
+		n.filterDirectAccess(updater, nftables.IPv6, n.config.Config6, epIPv6)
+		if err := n.fw.table6.Apply(ctx, tm); err != nil {
 			return fmt.Errorf("adding rules for bridge %s: %w", n.config.IfName, err)
 		}
 	}
@@ -53,17 +59,21 @@ func (n *network) modEndpoint(ctx context.Context, epIPv4, epIPv6 netip.Addr, en
 //     kernel support).
 //
 // Packets originating on the bridge's own interface and addressed directly to the
-// container are allowed - the host always has direct access to its own containers
-// (it doesn't need to use the port mapped to its own addresses, although it can).
+// container are allowed - the host always has direct access to its own containers.
+// (It doesn't need to use the port mapped to its own addresses, although it can.)
 //
 // "Trusted interfaces" are treated in the same way as the bridge itself.
-func (n *network) filterDirectAccess(ctx context.Context, table nftables.TableRef, conf firewaller.NetworkConfigFam, epIP netip.Addr, enable bool) error {
+func (n *network) filterDirectAccess(updater func(nftables.Obj), fam nftables.Family, conf firewaller.NetworkConfigFam, epIP netip.Addr) {
 	if n.config.Internal || conf.Unprotected || conf.Routed || n.fw.config.AllowDirectRouting {
-		return nil
+		return
 	}
-	updater := table.ChainUpdateFunc(ctx, rawPreroutingChain, enable)
 	ifNames := strings.Join(n.config.TrustedHostInterfaces, ", ")
-	return updater(ctx, rawPreroutingPortsRuleGroup,
-		`%s daddr %s iifname != { %s, %s } counter drop comment "DROP DIRECT ACCESS"`,
-		table.Family(), epIP, n.config.IfName, ifNames)
+	updater(nftables.Rule{
+		Chain: rawPreroutingChain,
+		Group: rawPreroutingPortsRuleGroup,
+		Rule: []string{
+			string(fam), "daddr", epIP.String(),
+			"iifname != {", n.config.IfName, ",", ifNames, `} counter drop comment "DROP DIRECT ACCESS"`,
+		},
+	})
 }

daemon/libnetwork/drivers/bridge/internal/nftabler/link.go

@@ -7,8 +7,10 @@ import (
 	"errors"
 	"fmt"
 	"net/netip"
+	"strconv"
 
 	"github.com/containerd/log"
+	"github.com/moby/moby/v2/daemon/libnetwork/internal/nftables"
 	"github.com/moby/moby/v2/daemon/libnetwork/types"
 )
 
@@ -20,44 +22,47 @@ func (n *network) AddLink(ctx context.Context, parentIP, childIP netip.Addr, por
 		return errors.New("cannot link to a container with an empty child IP address")
 	}
 
-	chain := n.fw.table4.Chain(ctx, chainFilterFwdIn(n.config.IfName))
+	tm := nftables.Modifier{}
 	for _, port := range ports {
-		for _, rule := range legacyLinkRules(parentIP, childIP, port) {
-			if err := chain.AppendRule(ctx, fwdInLegacyLinksRuleGroup, rule); err != nil {
-				return err
-			}
-		}
+		updateLegacyLinkRules(tm.Create, chainFilterFwdIn(n.config.IfName), parentIP, childIP, port)
 	}
-	if err := nftApply(ctx, n.fw.table4); err != nil {
+	if err := n.fw.table4.Apply(ctx, tm); err != nil {
 		return fmt.Errorf("adding rules for bridge %s: %w", n.config.IfName, err)
 	}
 	return nil
 }
 
 func (n *network) DelLink(ctx context.Context, parentIP, childIP netip.Addr, ports []types.TransportPort) {
-	chain := n.fw.table4.Chain(ctx, chainFilterFwdIn(n.config.IfName))
+	tm := nftables.Modifier{}
 	for _, port := range ports {
-		for _, rule := range legacyLinkRules(parentIP, childIP, port) {
-			if err := chain.DeleteRule(ctx, fwdInLegacyLinksRuleGroup, rule); err != nil {
-				log.G(ctx).WithFields(log.Fields{
-					"rule":  rule,
-					"error": err,
-				}).Warn("Failed to remove link between containers")
-			}
-		}
+		updateLegacyLinkRules(tm.Delete, chainFilterFwdIn(n.config.IfName), parentIP, childIP, port)
 	}
-	if err := nftApply(ctx, n.fw.table4); err != nil {
+	if err := n.fw.table4.Apply(ctx, tm); err != nil {
 		log.G(ctx).WithError(err).Warn("Removing link, failed to update nftables")
 	}
 }
 
-func legacyLinkRules(parentIP, childIP netip.Addr, port types.TransportPort) []string {
+func updateLegacyLinkRules(updater func(command nftables.Obj), chainName string, parentIP, childIP netip.Addr, port types.TransportPort) {
 	// TODO(robmry) - could combine rules for each proto by using an anonymous set.
-	return []string{
-		// Match the iptables implementation, but without checking iifname/oifname (not needed
-		// because the addresses belong to the bridge).
-		fmt.Sprintf("ip saddr %s ip daddr %s %s dport %d counter accept", parentIP.Unmap(), childIP.Unmap(), port.Proto, port.Port),
-		// Conntrack will allow responses. So, this must be to allow unsolicited packets from an exposed port.
-		fmt.Sprintf("ip daddr %s ip saddr %s %s sport %d counter accept", parentIP.Unmap(), childIP.Unmap(), port.Proto, port.Port),
-	}
+	// Match the iptables implementation, but without checking iifname/oifname (not needed
+	// because the addresses belong to the bridge).
+	updater(nftables.Rule{
+		Chain: chainName,
+		Group: fwdInLegacyLinksRuleGroup,
+		Rule: []string{
+			"ip saddr", parentIP.Unmap().String(),
+			"ip daddr", childIP.Unmap().String(), port.Proto.String(), "dport", strconv.Itoa(int(port.Port)),
+			"counter accept",
+		},
+	})
+	// Conntrack will allow responses. So, this must be to allow unsolicited packets from an exposed port.
+	updater(nftables.Rule{
+		Chain: chainName,
+		Group: fwdInLegacyLinksRuleGroup,
+		Rule: []string{
+			"ip saddr", childIP.Unmap().String(), port.Proto.String(), "sport", strconv.Itoa(int(port.Port)),
+			"ip daddr", parentIP.Unmap().String(),
+			"counter accept",
+		},
+	})
 }