From 61d078645a6d2a2391a1555ecda3d0a080a45831 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Tue, 4 Apr 2023 08:49:14 -0400 Subject: [PATCH] Add nftables implementation of ipmasq Signed-off-by: Dan Winship --- pkg/ip/ipmasq_iptables_linux.go | 161 +++++++++++++++++++ pkg/ip/ipmasq_linux.go | 139 +++++++--------- pkg/ip/ipmasq_nftables_linux.go | 229 +++++++++++++++++++++++++++ pkg/ip/ipmasq_nftables_linux_test.go | 186 ++++++++++++++++++++++ plugins/main/bridge/bridge.go | 10 +- plugins/main/bridge/bridge_test.go | 117 ++++++++++---- plugins/main/ptp/ptp.go | 14 +- plugins/main/ptp/ptp_test.go | 57 +++++++ 8 files changed, 780 insertions(+), 133 deletions(-) create mode 100644 pkg/ip/ipmasq_iptables_linux.go create mode 100644 pkg/ip/ipmasq_nftables_linux.go create mode 100644 pkg/ip/ipmasq_nftables_linux_test.go diff --git a/pkg/ip/ipmasq_iptables_linux.go b/pkg/ip/ipmasq_iptables_linux.go new file mode 100644 index 00000000..5c1fcfa8 --- /dev/null +++ b/pkg/ip/ipmasq_iptables_linux.go @@ -0,0 +1,161 @@ +// Copyright 2015 CNI authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package ip + +import ( + "fmt" + "net" + + "github.com/coreos/go-iptables/iptables" + + "github.com/containernetworking/cni/pkg/types" + "github.com/containernetworking/plugins/pkg/utils" +) + +// setupIPMasqIPTables is the iptables-based implementation of SetupIPMasqForNetwork +func setupIPMasqIPTables(ipn *net.IPNet, network, _, containerID string) error { + // Note: for historical reasons, the iptables implementation ignores ifname. + chain := utils.FormatChainName(network, containerID) + comment := utils.FormatComment(network, containerID) + return SetupIPMasq(ipn, chain, comment) +} + +// SetupIPMasq installs iptables rules to masquerade traffic +// coming from ip of ipn and going outside of ipn. +// Deprecated: This function only supports iptables. Use SetupIPMasqForNetwork, which +// supports both iptables and nftables. +func SetupIPMasq(ipn *net.IPNet, chain string, comment string) error { + isV6 := ipn.IP.To4() == nil + + var ipt *iptables.IPTables + var err error + var multicastNet string + + if isV6 { + ipt, err = iptables.NewWithProtocol(iptables.ProtocolIPv6) + multicastNet = "ff00::/8" + } else { + ipt, err = iptables.NewWithProtocol(iptables.ProtocolIPv4) + multicastNet = "224.0.0.0/4" + } + if err != nil { + return fmt.Errorf("failed to locate iptables: %v", err) + } + + // Create chain if doesn't exist + exists := false + chains, err := ipt.ListChains("nat") + if err != nil { + return fmt.Errorf("failed to list chains: %v", err) + } + for _, ch := range chains { + if ch == chain { + exists = true + break + } + } + if !exists { + if err = ipt.NewChain("nat", chain); err != nil { + return err + } + } + + // Packets to this network should not be touched + if err := ipt.AppendUnique("nat", chain, "-d", ipn.String(), "-j", "ACCEPT", "-m", "comment", "--comment", comment); err != nil { + return err + } + + // Don't masquerade multicast - pods should be able to talk to other pods + // on the local network via multicast. + if err := ipt.AppendUnique("nat", chain, "!", "-d", multicastNet, "-j", "MASQUERADE", "-m", "comment", "--comment", comment); err != nil { + return err + } + + // Packets from the specific IP of this network will hit the chain + return ipt.AppendUnique("nat", "POSTROUTING", "-s", ipn.IP.String(), "-j", chain, "-m", "comment", "--comment", comment) +} + +// teardownIPMasqIPTables is the iptables-based implementation of TeardownIPMasqForNetwork +func teardownIPMasqIPTables(ipn *net.IPNet, network, _, containerID string) error { + // Note: for historical reasons, the iptables implementation ignores ifname. + chain := utils.FormatChainName(network, containerID) + comment := utils.FormatComment(network, containerID) + return TeardownIPMasq(ipn, chain, comment) +} + +// TeardownIPMasq undoes the effects of SetupIPMasq. +// Deprecated: This function only supports iptables. Use TeardownIPMasqForNetwork, which +// supports both iptables and nftables. +func TeardownIPMasq(ipn *net.IPNet, chain string, comment string) error { + isV6 := ipn.IP.To4() == nil + + var ipt *iptables.IPTables + var err error + + if isV6 { + ipt, err = iptables.NewWithProtocol(iptables.ProtocolIPv6) + } else { + ipt, err = iptables.NewWithProtocol(iptables.ProtocolIPv4) + } + if err != nil { + return fmt.Errorf("failed to locate iptables: %v", err) + } + + err = ipt.Delete("nat", "POSTROUTING", "-s", ipn.IP.String(), "-j", chain, "-m", "comment", "--comment", comment) + if err != nil && !isNotExist(err) { + return err + } + + // for downward compatibility + err = ipt.Delete("nat", "POSTROUTING", "-s", ipn.String(), "-j", chain, "-m", "comment", "--comment", comment) + if err != nil && !isNotExist(err) { + return err + } + + err = ipt.ClearChain("nat", chain) + if err != nil && !isNotExist(err) { + return err + } + + err = ipt.DeleteChain("nat", chain) + if err != nil && !isNotExist(err) { + return err + } + + return nil +} + +// gcIPMasqIPTables is the iptables-based implementation of GCIPMasqForNetwork +func gcIPMasqIPTables(_ string, _ []types.GCAttachment) error { + // FIXME: The iptables implementation does not support GC. + // + // (In theory, it _could_ backward-compatibly support it, by adding a no-op rule + // with a comment indicating the network to each chain it creates, so that it + // could later figure out which chains corresponded to which networks; older + // implementations would ignore the extra rule but would still correctly delete + // the chain on teardown (because they ClearChain() before doing DeleteChain()). + + return nil +} + +// isNotExist returnst true if the error is from iptables indicating +// that the target does not exist. +func isNotExist(err error) bool { + e, ok := err.(*iptables.Error) + if !ok { + return false + } + return e.IsNotExist() +} diff --git a/pkg/ip/ipmasq_linux.go b/pkg/ip/ipmasq_linux.go index aa59a8db..bad83541 100644 --- a/pkg/ip/ipmasq_linux.go +++ b/pkg/ip/ipmasq_linux.go @@ -15,111 +15,78 @@ package ip import ( + "errors" "fmt" "net" + "strings" - "github.com/coreos/go-iptables/iptables" + "github.com/containernetworking/cni/pkg/types" + "github.com/containernetworking/plugins/pkg/utils" ) -// SetupIPMasq installs iptables rules to masquerade traffic -// coming from ip of ipn and going outside of ipn -func SetupIPMasq(ipn *net.IPNet, chain string, comment string) error { - isV6 := ipn.IP.To4() == nil - - var ipt *iptables.IPTables - var err error - var multicastNet string - - if isV6 { - ipt, err = iptables.NewWithProtocol(iptables.ProtocolIPv6) - multicastNet = "ff00::/8" - } else { - ipt, err = iptables.NewWithProtocol(iptables.ProtocolIPv4) - multicastNet = "224.0.0.0/4" - } - if err != nil { - return fmt.Errorf("failed to locate iptables: %v", err) - } - - // Create chain if doesn't exist - exists := false - chains, err := ipt.ListChains("nat") - if err != nil { - return fmt.Errorf("failed to list chains: %v", err) - } - for _, ch := range chains { - if ch == chain { - exists = true - break - } - } - if !exists { - if err = ipt.NewChain("nat", chain); err != nil { - return err +// SetupIPMasqForNetwork installs rules to masquerade traffic coming from ip of ipn and +// going outside of ipn, using a chain name based on network, ifname, and containerID. The +// backend can be either "iptables" or "nftables"; if it is nil, then a suitable default +// implementation will be used. +func SetupIPMasqForNetwork(backend *string, ipn *net.IPNet, network, ifname, containerID string) error { + if backend == nil { + // Prefer iptables, unless only nftables is available + defaultBackend := "iptables" + if !utils.SupportsIPTables() && utils.SupportsNFTables() { + defaultBackend = "nftables" } + backend = &defaultBackend } - // Packets to this network should not be touched - if err := ipt.AppendUnique("nat", chain, "-d", ipn.String(), "-j", "ACCEPT", "-m", "comment", "--comment", comment); err != nil { - return err + switch *backend { + case "iptables": + return setupIPMasqIPTables(ipn, network, ifname, containerID) + case "nftables": + return setupIPMasqNFTables(ipn, network, ifname, containerID) + default: + return fmt.Errorf("unknown ipmasq backend %q", *backend) } - - // Don't masquerade multicast - pods should be able to talk to other pods - // on the local network via multicast. - if err := ipt.AppendUnique("nat", chain, "!", "-d", multicastNet, "-j", "MASQUERADE", "-m", "comment", "--comment", comment); err != nil { - return err - } - - // Packets from the specific IP of this network will hit the chain - return ipt.AppendUnique("nat", "POSTROUTING", "-s", ipn.IP.String(), "-j", chain, "-m", "comment", "--comment", comment) } -// TeardownIPMasq undoes the effects of SetupIPMasq -func TeardownIPMasq(ipn *net.IPNet, chain string, comment string) error { - isV6 := ipn.IP.To4() == nil +// TeardownIPMasqForNetwork undoes the effects of SetupIPMasqForNetwork +func TeardownIPMasqForNetwork(ipn *net.IPNet, network, ifname, containerID string) error { + var errs []string - var ipt *iptables.IPTables - var err error + // Do both the iptables and the nftables cleanup, since the pod may have been + // created with a different version of this plugin or a different configuration. - if isV6 { - ipt, err = iptables.NewWithProtocol(iptables.ProtocolIPv6) - } else { - ipt, err = iptables.NewWithProtocol(iptables.ProtocolIPv4) - } - if err != nil { - return fmt.Errorf("failed to locate iptables: %v", err) + err := teardownIPMasqIPTables(ipn, network, ifname, containerID) + if err != nil && utils.SupportsIPTables() { + errs = append(errs, err.Error()) } - err = ipt.Delete("nat", "POSTROUTING", "-s", ipn.IP.String(), "-j", chain, "-m", "comment", "--comment", comment) - if err != nil && !isNotExist(err) { - return err + err = teardownIPMasqNFTables(ipn, network, ifname, containerID) + if err != nil && utils.SupportsNFTables() { + errs = append(errs, err.Error()) } - // for downward compatibility - err = ipt.Delete("nat", "POSTROUTING", "-s", ipn.String(), "-j", chain, "-m", "comment", "--comment", comment) - if err != nil && !isNotExist(err) { - return err + if errs == nil { + return nil } - - err = ipt.ClearChain("nat", chain) - if err != nil && !isNotExist(err) { - return err - } - - err = ipt.DeleteChain("nat", chain) - if err != nil && !isNotExist(err) { - return err - } - - return nil + return errors.New(strings.Join(errs, "\n")) } -// isNotExist returnst true if the error is from iptables indicating -// that the target does not exist. -func isNotExist(err error) bool { - e, ok := err.(*iptables.Error) - if !ok { - return false +// GCIPMasqForNetwork garbage collects stale IPMasq entries for network +func GCIPMasqForNetwork(network string, attachments []types.GCAttachment) error { + var errs []string + + err := gcIPMasqIPTables(network, attachments) + if err != nil && utils.SupportsIPTables() { + errs = append(errs, err.Error()) } - return e.IsNotExist() + + err = gcIPMasqNFTables(network, attachments) + if err != nil && utils.SupportsNFTables() { + errs = append(errs, err.Error()) + } + + if errs == nil { + return nil + } + return errors.New(strings.Join(errs, "\n")) } diff --git a/pkg/ip/ipmasq_nftables_linux.go b/pkg/ip/ipmasq_nftables_linux.go new file mode 100644 index 00000000..5c7458c9 --- /dev/null +++ b/pkg/ip/ipmasq_nftables_linux.go @@ -0,0 +1,229 @@ +// Copyright 2023 CNI authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package ip + +import ( + "context" + "fmt" + "net" + "strings" + + "sigs.k8s.io/knftables" + + "github.com/containernetworking/cni/pkg/types" + "github.com/containernetworking/plugins/pkg/utils" +) + +const ( + ipMasqTableName = "cni_plugins_masquerade" + ipMasqChainName = "masq_checks" +) + +// The nftables ipmasq implementation is mostly like the iptables implementation, with +// minor updates to fix a bug (adding `ifname`) and to allow future GC support. +// +// We add a rule for each mapping, with a comment containing a hash of its identifiers, +// so that we can later reliably delete the rules we want. (This is important because in +// edge cases, it's possible the plugin might see "ADD container A with IP 192.168.1.3", +// followed by "ADD container B with IP 192.168.1.3" followed by "DEL container A with IP +// 192.168.1.3", and we need to make sure that the DEL causes us to delete the rule for +// container A, and not the rule for container B.) +// +// It would be more nftables-y to have a chain with a single rule doing a lookup against a +// set with an element per mapping, rather than having a chain with a rule per mapping. +// But there's no easy, non-racy way to say "delete the element 192.168.1.3 from the set, +// but only if it was added for container A, not if it was added for container B". + +// hashForNetwork returns a unique hash for this network +func hashForNetwork(network string) string { + return utils.MustFormatHashWithPrefix(16, "", network) +} + +// hashForInstance returns a unique hash identifying the rules for this +// network/ifname/containerID +func hashForInstance(network, ifname, containerID string) string { + return hashForNetwork(network) + "-" + utils.MustFormatHashWithPrefix(16, "", ifname+":"+containerID) +} + +// commentForInstance returns a comment string that begins with a unique hash and +// ends with a (possibly-truncated) human-readable description. +func commentForInstance(network, ifname, containerID string) string { + comment := fmt.Sprintf("%s, net: %s, if: %s, id: %s", + hashForInstance(network, ifname, containerID), + strings.ReplaceAll(network, `"`, ``), + strings.ReplaceAll(ifname, `"`, ``), + strings.ReplaceAll(containerID, `"`, ``), + ) + if len(comment) > knftables.CommentLengthMax { + comment = comment[:knftables.CommentLengthMax] + } + return comment +} + +// setupIPMasqNFTables is the nftables-based implementation of SetupIPMasqForNetwork +func setupIPMasqNFTables(ipn *net.IPNet, network, ifname, containerID string) error { + nft, err := knftables.New(knftables.InetFamily, ipMasqTableName) + if err != nil { + return err + } + return setupIPMasqNFTablesWithInterface(nft, ipn, network, ifname, containerID) +} + +func setupIPMasqNFTablesWithInterface(nft knftables.Interface, ipn *net.IPNet, network, ifname, containerID string) error { + staleRules, err := findRules(nft, hashForInstance(network, ifname, containerID)) + if err != nil { + return err + } + + tx := nft.NewTransaction() + + // Ensure that our table and chains exist. + tx.Add(&knftables.Table{ + Comment: knftables.PtrTo("Masquerading for plugins from github.com/containernetworking/plugins"), + }) + tx.Add(&knftables.Chain{ + Name: ipMasqChainName, + Comment: knftables.PtrTo("Masquerade traffic from certain IPs to any (non-multicast) IP outside their subnet"), + }) + + // Ensure that the postrouting chain exists and has the correct rules. (Has to be + // done after creating ipMasqChainName, so we can jump to it.) + tx.Add(&knftables.Chain{ + Name: "postrouting", + Type: knftables.PtrTo(knftables.NATType), + Hook: knftables.PtrTo(knftables.PostroutingHook), + Priority: knftables.PtrTo(knftables.SNATPriority), + }) + tx.Flush(&knftables.Chain{ + Name: "postrouting", + }) + tx.Add(&knftables.Rule{ + Chain: "postrouting", + Rule: "ip daddr == 224.0.0.0/4 return", + }) + tx.Add(&knftables.Rule{ + Chain: "postrouting", + Rule: "ip6 daddr == ff00::/8 return", + }) + tx.Add(&knftables.Rule{ + Chain: "postrouting", + Rule: knftables.Concat( + "goto", ipMasqChainName, + ), + }) + + // Delete stale rules, add new rules to masquerade chain + for _, rule := range staleRules { + tx.Delete(rule) + } + ip := "ip" + if ipn.IP.To4() == nil { + ip = "ip6" + } + + // e.g. if ipn is "192.168.1.4/24", then dstNet is "192.168.1.0/24" + dstNet := &net.IPNet{IP: ipn.IP.Mask(ipn.Mask), Mask: ipn.Mask} + + tx.Add(&knftables.Rule{ + Chain: ipMasqChainName, + Rule: knftables.Concat( + ip, "saddr", "==", ipn.IP, + ip, "daddr", "!=", dstNet, + "masquerade", + ), + Comment: knftables.PtrTo(commentForInstance(network, ifname, containerID)), + }) + + return nft.Run(context.TODO(), tx) +} + +// teardownIPMasqNFTables is the nftables-based implementation of TeardownIPMasqForNetwork +func teardownIPMasqNFTables(ipn *net.IPNet, network, ifname, containerID string) error { + nft, err := knftables.New(knftables.InetFamily, ipMasqTableName) + if err != nil { + return err + } + return teardownIPMasqNFTablesWithInterface(nft, ipn, network, ifname, containerID) +} + +func teardownIPMasqNFTablesWithInterface(nft knftables.Interface, _ *net.IPNet, network, ifname, containerID string) error { + rules, err := findRules(nft, hashForInstance(network, ifname, containerID)) + if err != nil { + return err + } else if len(rules) == 0 { + return nil + } + + tx := nft.NewTransaction() + for _, rule := range rules { + tx.Delete(rule) + } + return nft.Run(context.TODO(), tx) +} + +// gcIPMasqNFTables is the nftables-based implementation of GCIPMasqForNetwork +func gcIPMasqNFTables(network string, attachments []types.GCAttachment) error { + nft, err := knftables.New(knftables.InetFamily, ipMasqTableName) + if err != nil { + return err + } + return gcIPMasqNFTablesWithInterface(nft, network, attachments) +} + +func gcIPMasqNFTablesWithInterface(nft knftables.Interface, network string, attachments []types.GCAttachment) error { + // Find all rules for the network + rules, err := findRules(nft, hashForNetwork(network)) + if err != nil { + return err + } else if len(rules) == 0 { + return nil + } + + // Compute the comments for all elements of attachments + validAttachments := map[string]bool{} + for _, attachment := range attachments { + validAttachments[commentForInstance(network, attachment.IfName, attachment.ContainerID)] = true + } + + // Delete anything in rules that isn't in validAttachments + tx := nft.NewTransaction() + for _, rule := range rules { + if !validAttachments[*rule.Comment] { + tx.Delete(rule) + } + } + return nft.Run(context.TODO(), tx) +} + +// findRules finds rules with comments that start with commentPrefix. +func findRules(nft knftables.Interface, commentPrefix string) ([]*knftables.Rule, error) { + rules, err := nft.ListRules(context.TODO(), ipMasqChainName) + if err != nil { + if knftables.IsNotFound(err) { + // If ipMasqChainName doesn't exist yet, that's fine + return nil, nil + } + return nil, err + } + + matchingRules := make([]*knftables.Rule, 0, 1) + for _, rule := range rules { + if rule.Comment != nil && strings.HasPrefix(*rule.Comment, commentPrefix) { + matchingRules = append(matchingRules, rule) + } + } + + return matchingRules, nil +} diff --git a/pkg/ip/ipmasq_nftables_linux_test.go b/pkg/ip/ipmasq_nftables_linux_test.go new file mode 100644 index 00000000..08b8bbe5 --- /dev/null +++ b/pkg/ip/ipmasq_nftables_linux_test.go @@ -0,0 +1,186 @@ +// Copyright 2023 CNI authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package ip + +import ( + "strings" + "testing" + + "github.com/vishvananda/netlink" + "sigs.k8s.io/knftables" + + "github.com/containernetworking/cni/pkg/types" +) + +func Test_setupIPMasqNFTables(t *testing.T) { + nft := knftables.NewFake(knftables.InetFamily, ipMasqTableName) + + containers := []struct { + network string + ifname string + containerID string + addr string + }{ + { + network: "unit-test", + ifname: "eth0", + containerID: "one", + addr: "192.168.1.1/24", + }, + { + network: "unit-test", + ifname: "eth0", + containerID: "two", + addr: "192.168.1.2/24", + }, + { + network: "unit-test", + ifname: "eth0", + containerID: "three", + addr: "192.168.99.5/24", + }, + { + network: "alternate", + ifname: "net1", + containerID: "three", + addr: "10.0.0.5/24", + }, + } + + for _, c := range containers { + addr, err := netlink.ParseAddr(c.addr) + if err != nil { + t.Fatalf("failed to parse test addr: %v", err) + } + err = setupIPMasqNFTablesWithInterface(nft, addr.IPNet, c.network, c.ifname, c.containerID) + if err != nil { + t.Fatalf("error from setupIPMasqNFTables: %v", err) + } + } + + expected := strings.TrimSpace(` +add table inet cni_plugins_masquerade { comment "Masquerading for plugins from github.com/containernetworking/plugins" ; } +add chain inet cni_plugins_masquerade masq_checks { comment "Masquerade traffic from certain IPs to any (non-multicast) IP outside their subnet" ; } +add chain inet cni_plugins_masquerade postrouting { type nat hook postrouting priority 100 ; } +add rule inet cni_plugins_masquerade masq_checks ip saddr == 192.168.1.1 ip daddr != 192.168.1.0/24 masquerade comment "6fd94d501e58f0aa-287fc69eff0574a2, net: unit-test, if: eth0, id: one" +add rule inet cni_plugins_masquerade masq_checks ip saddr == 192.168.1.2 ip daddr != 192.168.1.0/24 masquerade comment "6fd94d501e58f0aa-d750b2c8f0f25d5f, net: unit-test, if: eth0, id: two" +add rule inet cni_plugins_masquerade masq_checks ip saddr == 192.168.99.5 ip daddr != 192.168.99.0/24 masquerade comment "6fd94d501e58f0aa-a4d4adb82b669cfe, net: unit-test, if: eth0, id: three" +add rule inet cni_plugins_masquerade masq_checks ip saddr == 10.0.0.5 ip daddr != 10.0.0.0/24 masquerade comment "82783ef24bdc7036-acb19d111858e348, net: alternate, if: net1, id: three" +add rule inet cni_plugins_masquerade postrouting ip daddr == 224.0.0.0/4 return +add rule inet cni_plugins_masquerade postrouting ip6 daddr == ff00::/8 return +add rule inet cni_plugins_masquerade postrouting goto masq_checks +`) + dump := strings.TrimSpace(nft.Dump()) + if dump != expected { + t.Errorf("expected nftables state:\n%s\n\nactual:\n%s\n\n", expected, dump) + } + + // Add a new container reusing "one"'s address, before deleting "one" + addr, err := netlink.ParseAddr(containers[0].addr) + if err != nil { + t.Fatalf("failed to parse test addr: %v", err) + } + err = setupIPMasqNFTablesWithInterface(nft, addr.IPNet, "unit-test", "eth0", "four") + if err != nil { + t.Fatalf("error from setupIPMasqNFTables: %v", err) + } + + // Remove "one" + c := containers[0] + addr, err = netlink.ParseAddr(c.addr) + if err != nil { + t.Fatalf("failed to parse test addr: %v", err) + } + err = teardownIPMasqNFTablesWithInterface(nft, addr.IPNet, c.network, c.ifname, c.containerID) + if err != nil { + t.Fatalf("error from teardownIPMasqNFTables: %v", err) + } + + // Check that "one" was deleted (and "four" wasn't) + expected = strings.TrimSpace(` +add table inet cni_plugins_masquerade { comment "Masquerading for plugins from github.com/containernetworking/plugins" ; } +add chain inet cni_plugins_masquerade masq_checks { comment "Masquerade traffic from certain IPs to any (non-multicast) IP outside their subnet" ; } +add chain inet cni_plugins_masquerade postrouting { type nat hook postrouting priority 100 ; } +add rule inet cni_plugins_masquerade masq_checks ip saddr == 192.168.1.2 ip daddr != 192.168.1.0/24 masquerade comment "6fd94d501e58f0aa-d750b2c8f0f25d5f, net: unit-test, if: eth0, id: two" +add rule inet cni_plugins_masquerade masq_checks ip saddr == 192.168.99.5 ip daddr != 192.168.99.0/24 masquerade comment "6fd94d501e58f0aa-a4d4adb82b669cfe, net: unit-test, if: eth0, id: three" +add rule inet cni_plugins_masquerade masq_checks ip saddr == 10.0.0.5 ip daddr != 10.0.0.0/24 masquerade comment "82783ef24bdc7036-acb19d111858e348, net: alternate, if: net1, id: three" +add rule inet cni_plugins_masquerade masq_checks ip saddr == 192.168.1.1 ip daddr != 192.168.1.0/24 masquerade comment "6fd94d501e58f0aa-e766de567ef6c543, net: unit-test, if: eth0, id: four" +add rule inet cni_plugins_masquerade postrouting ip daddr == 224.0.0.0/4 return +add rule inet cni_plugins_masquerade postrouting ip6 daddr == ff00::/8 return +add rule inet cni_plugins_masquerade postrouting goto masq_checks +`) + dump = strings.TrimSpace(nft.Dump()) + if dump != expected { + t.Errorf("expected nftables state:\n%s\n\nactual:\n%s\n\n", expected, dump) + } + + // GC "four" from the "unit-test" network + err = gcIPMasqNFTablesWithInterface(nft, "unit-test", []types.GCAttachment{ + {IfName: "eth0", ContainerID: "two"}, + {IfName: "eth0", ContainerID: "three"}, + // (irrelevant extra element) + {IfName: "eth0", ContainerID: "one"}, + }) + if err != nil { + t.Fatalf("error from gcIPMasqNFTables: %v", err) + } + // GC the "alternate" network without removing anything + err = gcIPMasqNFTablesWithInterface(nft, "alternate", []types.GCAttachment{ + {IfName: "net1", ContainerID: "three"}, + }) + if err != nil { + t.Fatalf("error from gcIPMasqNFTables: %v", err) + } + + // Re-dump + expected = strings.TrimSpace(` +add table inet cni_plugins_masquerade { comment "Masquerading for plugins from github.com/containernetworking/plugins" ; } +add chain inet cni_plugins_masquerade masq_checks { comment "Masquerade traffic from certain IPs to any (non-multicast) IP outside their subnet" ; } +add chain inet cni_plugins_masquerade postrouting { type nat hook postrouting priority 100 ; } +add rule inet cni_plugins_masquerade masq_checks ip saddr == 192.168.1.2 ip daddr != 192.168.1.0/24 masquerade comment "6fd94d501e58f0aa-d750b2c8f0f25d5f, net: unit-test, if: eth0, id: two" +add rule inet cni_plugins_masquerade masq_checks ip saddr == 192.168.99.5 ip daddr != 192.168.99.0/24 masquerade comment "6fd94d501e58f0aa-a4d4adb82b669cfe, net: unit-test, if: eth0, id: three" +add rule inet cni_plugins_masquerade masq_checks ip saddr == 10.0.0.5 ip daddr != 10.0.0.0/24 masquerade comment "82783ef24bdc7036-acb19d111858e348, net: alternate, if: net1, id: three" +add rule inet cni_plugins_masquerade postrouting ip daddr == 224.0.0.0/4 return +add rule inet cni_plugins_masquerade postrouting ip6 daddr == ff00::/8 return +add rule inet cni_plugins_masquerade postrouting goto masq_checks +`) + dump = strings.TrimSpace(nft.Dump()) + if dump != expected { + t.Errorf("expected nftables state:\n%s\n\nactual:\n%s\n\n", expected, dump) + } + + // GC everything + err = gcIPMasqNFTablesWithInterface(nft, "unit-test", []types.GCAttachment{}) + if err != nil { + t.Fatalf("error from gcIPMasqNFTables: %v", err) + } + err = gcIPMasqNFTablesWithInterface(nft, "alternate", []types.GCAttachment{}) + if err != nil { + t.Fatalf("error from gcIPMasqNFTables: %v", err) + } + + expected = strings.TrimSpace(` +add table inet cni_plugins_masquerade { comment "Masquerading for plugins from github.com/containernetworking/plugins" ; } +add chain inet cni_plugins_masquerade masq_checks { comment "Masquerade traffic from certain IPs to any (non-multicast) IP outside their subnet" ; } +add chain inet cni_plugins_masquerade postrouting { type nat hook postrouting priority 100 ; } +add rule inet cni_plugins_masquerade postrouting ip daddr == 224.0.0.0/4 return +add rule inet cni_plugins_masquerade postrouting ip6 daddr == ff00::/8 return +add rule inet cni_plugins_masquerade postrouting goto masq_checks +`) + dump = strings.TrimSpace(nft.Dump()) + if dump != expected { + t.Errorf("expected nftables state:\n%s\n\nactual:\n%s\n\n", expected, dump) + } +} diff --git a/plugins/main/bridge/bridge.go b/plugins/main/bridge/bridge.go index 49d7b04d..1c4fa59a 100644 --- a/plugins/main/bridge/bridge.go +++ b/plugins/main/bridge/bridge.go @@ -35,7 +35,6 @@ import ( "github.com/containernetworking/plugins/pkg/ipam" "github.com/containernetworking/plugins/pkg/link" "github.com/containernetworking/plugins/pkg/ns" - "github.com/containernetworking/plugins/pkg/utils" bv "github.com/containernetworking/plugins/pkg/utils/buildversion" "github.com/containernetworking/plugins/pkg/utils/sysctl" ) @@ -52,6 +51,7 @@ type NetConf struct { IsDefaultGW bool `json:"isDefaultGateway"` ForceAddress bool `json:"forceAddress"` IPMasq bool `json:"ipMasq"` + IPMasqBackend *string `json:"ipMasqBackend,omitempty"` MTU int `json:"mtu"` HairpinMode bool `json:"hairpinMode"` PromiscMode bool `json:"promiscMode"` @@ -673,10 +673,8 @@ func cmdAdd(args *skel.CmdArgs) error { } if n.IPMasq { - chain := utils.FormatChainName(n.Name, args.ContainerID) - comment := utils.FormatComment(n.Name, args.ContainerID) for _, ipc := range result.IPs { - if err = ip.SetupIPMasq(&ipc.Address, chain, comment); err != nil { + if err = ip.SetupIPMasqForNetwork(n.IPMasqBackend, &ipc.Address, n.Name, args.IfName, args.ContainerID); err != nil { return err } } @@ -814,10 +812,8 @@ func cmdDel(args *skel.CmdArgs) error { } if isLayer3 && n.IPMasq { - chain := utils.FormatChainName(n.Name, args.ContainerID) - comment := utils.FormatComment(n.Name, args.ContainerID) for _, ipn := range ipnets { - if err := ip.TeardownIPMasq(ipn, chain, comment); err != nil { + if err := ip.TeardownIPMasqForNetwork(ipn, n.Name, args.IfName, args.ContainerID); err != nil { return err } } diff --git a/plugins/main/bridge/bridge_test.go b/plugins/main/bridge/bridge_test.go index 15d41911..c625ab09 100644 --- a/plugins/main/bridge/bridge_test.go +++ b/plugins/main/bridge/bridge_test.go @@ -15,6 +15,7 @@ package main import ( + "context" "encoding/json" "fmt" "net" @@ -27,6 +28,7 @@ import ( . "github.com/onsi/gomega" "github.com/vishvananda/netlink" "github.com/vishvananda/netlink/nl" + "sigs.k8s.io/knftables" "github.com/containernetworking/cni/pkg/skel" "github.com/containernetworking/cni/pkg/types" @@ -77,6 +79,7 @@ type testCase struct { vlanTrunk []*VlanTrunk removeDefaultVlan bool ipMasq bool + ipMasqBackend string macspoofchk bool disableContIface bool @@ -172,6 +175,9 @@ const ( ipMasqConfStr = `, "ipMasq": %t` + ipMasqBackendConfStr = `, + "ipMasqBackend": "%s"` + // Single subnet configuration (legacy) subnetConfStr = `, "subnet": "%s"` @@ -243,6 +249,9 @@ func (tc testCase) netConfJSON(dataDir string) string { if tc.ipMasq { conf += tc.ipMasqConfig() } + if tc.ipMasqBackend != "" { + conf += tc.ipMasqBackendConfig() + } if tc.args.cni.mac != "" { conf += fmt.Sprintf(argsFormat, tc.args.cni.mac) } @@ -295,6 +304,11 @@ func (tc testCase) ipMasqConfig() string { return conf } +func (tc testCase) ipMasqBackendConfig() string { + conf := fmt.Sprintf(ipMasqBackendConfStr, tc.ipMasqBackend) + return conf +} + func (tc testCase) rangesConfig() string { conf := rangesStartStr for i, tcRange := range tc.ranges { @@ -2390,41 +2404,82 @@ var _ = Describe("bridge Operations", func() { }) if testutils.SpecVersionHasChaining(ver) { - It(fmt.Sprintf("[%s] configures a bridge and ipMasq rules", ver), func() { - err := originalNS.Do(func(ns.NetNS) error { - defer GinkgoRecover() - tc := testCase{ - ranges: []rangeInfo{{ - subnet: "10.1.2.0/24", - }}, - ipMasq: true, - cniVersion: ver, - } + for _, tc := range []testCase{ + { + ranges: []rangeInfo{{ + subnet: "10.1.2.0/24", + }}, + ipMasq: true, + cniVersion: ver, + }, + { + ranges: []rangeInfo{{ + subnet: "10.1.2.0/24", + }}, + ipMasq: true, + ipMasqBackend: "iptables", + cniVersion: ver, + }, + { + ranges: []rangeInfo{{ + subnet: "10.1.2.0/24", + }}, + ipMasq: true, + ipMasqBackend: "nftables", + cniVersion: ver, + }, + } { + tc := tc + It(fmt.Sprintf("[%s] configures a bridge and ipMasq rules with ipMasqBackend %q", ver, tc.ipMasqBackend), func() { + err := originalNS.Do(func(ns.NetNS) error { + defer GinkgoRecover() - args := tc.createCmdArgs(originalNS, dataDir) - r, _, err := testutils.CmdAddWithArgs(args, func() error { - return cmdAdd(args) + args := tc.createCmdArgs(originalNS, dataDir) + r, _, err := testutils.CmdAddWithArgs(args, func() error { + return cmdAdd(args) + }) + Expect(err).NotTo(HaveOccurred()) + result, err := types100.GetResult(r) + Expect(err).NotTo(HaveOccurred()) + Expect(result.IPs).Should(HaveLen(1)) + + ip := result.IPs[0].Address.IP.String() + + // Update this if the default ipmasq backend changes + switch tc.ipMasqBackend { + case "iptables", "": + ipt, err := iptables.NewWithProtocol(iptables.ProtocolIPv4) + Expect(err).NotTo(HaveOccurred()) + + rules, err := ipt.List("nat", "POSTROUTING") + Expect(err).NotTo(HaveOccurred()) + Expect(rules).Should(ContainElement(ContainSubstring(ip))) + case "nftables": + nft, err := knftables.New(knftables.InetFamily, "cni_plugins_masquerade") + Expect(err).NotTo(HaveOccurred()) + rules, err := nft.ListRules(context.TODO(), "masq_checks") + Expect(err).NotTo(HaveOccurred()) + // FIXME: ListRules() doesn't return the actual rule strings, + // and we can't easily compute the ipmasq plugin's comment. + comments := 0 + for _, r := range rules { + if r.Comment != nil { + comments++ + break + } + } + Expect(comments).To(Equal(1), "expected to find exactly one Rule with a comment") + } + + err = testutils.CmdDelWithArgs(args, func() error { + return cmdDel(args) + }) + Expect(err).NotTo(HaveOccurred()) + return nil }) Expect(err).NotTo(HaveOccurred()) - result, err := types100.GetResult(r) - Expect(err).NotTo(HaveOccurred()) - Expect(result.IPs).Should(HaveLen(1)) - - ipt, err := iptables.NewWithProtocol(iptables.ProtocolIPv4) - Expect(err).NotTo(HaveOccurred()) - - rules, err := ipt.List("nat", "POSTROUTING") - Expect(err).NotTo(HaveOccurred()) - Expect(rules).Should(ContainElement(ContainSubstring(result.IPs[0].Address.IP.String()))) - - err = testutils.CmdDelWithArgs(args, func() error { - return cmdDel(args) - }) - Expect(err).NotTo(HaveOccurred()) - return nil }) - Expect(err).NotTo(HaveOccurred()) - }) + } for i, tc := range []testCase{ { diff --git a/plugins/main/ptp/ptp.go b/plugins/main/ptp/ptp.go index 7213b9c6..0ac5e609 100644 --- a/plugins/main/ptp/ptp.go +++ b/plugins/main/ptp/ptp.go @@ -31,7 +31,6 @@ import ( "github.com/containernetworking/plugins/pkg/ip" "github.com/containernetworking/plugins/pkg/ipam" "github.com/containernetworking/plugins/pkg/ns" - "github.com/containernetworking/plugins/pkg/utils" bv "github.com/containernetworking/plugins/pkg/utils/buildversion" ) @@ -44,8 +43,9 @@ func init() { type NetConf struct { types.NetConf - IPMasq bool `json:"ipMasq"` - MTU int `json:"mtu"` + IPMasq bool `json:"ipMasq"` + IPMasqBackend *string `json:"ipMasqBackend,omitempty"` + MTU int `json:"mtu"` } func setupContainerVeth(netns ns.NetNS, ifName string, mtu int, pr *current.Result) (*current.Interface, *current.Interface, error) { @@ -229,10 +229,8 @@ func cmdAdd(args *skel.CmdArgs) error { } if conf.IPMasq { - chain := utils.FormatChainName(conf.Name, args.ContainerID) - comment := utils.FormatComment(conf.Name, args.ContainerID) for _, ipc := range result.IPs { - if err = ip.SetupIPMasq(&ipc.Address, chain, comment); err != nil { + if err = ip.SetupIPMasqForNetwork(conf.IPMasqBackend, &ipc.Address, conf.Name, args.IfName, args.ContainerID); err != nil { return err } } @@ -293,10 +291,8 @@ func cmdDel(args *skel.CmdArgs) error { } if len(ipnets) != 0 && conf.IPMasq { - chain := utils.FormatChainName(conf.Name, args.ContainerID) - comment := utils.FormatComment(conf.Name, args.ContainerID) for _, ipn := range ipnets { - err = ip.TeardownIPMasq(ipn, chain, comment) + err = ip.TeardownIPMasqForNetwork(ipn, conf.Name, args.IfName, args.ContainerID) } } diff --git a/plugins/main/ptp/ptp_test.go b/plugins/main/ptp/ptp_test.go index 0ede4409..bdc78b13 100644 --- a/plugins/main/ptp/ptp_test.go +++ b/plugins/main/ptp/ptp_test.go @@ -39,6 +39,7 @@ type Net struct { CNIVersion string `json:"cniVersion"` Type string `json:"type,omitempty"` IPMasq bool `json:"ipMasq"` + IPMasqBackend *string `json:"ipMasqBackend,omitempty"` MTU int `json:"mtu"` IPAM *allocator.IPAMConfig `json:"ipam"` DNS types.DNS `json:"dns"` @@ -368,6 +369,62 @@ var _ = Describe("ptp Operations", func() { doTest(conf, ver, 1, dnsConf, targetNS) }) + It(fmt.Sprintf("[%s] configures and deconfigures a ptp link when specifying ipMasqBackend: iptables", ver), func() { + dnsConf := types.DNS{ + Nameservers: []string{"10.1.2.123"}, + Domain: "some.domain.test", + Search: []string{"search.test"}, + Options: []string{"option1:foo"}, + } + dnsConfBytes, err := json.Marshal(dnsConf) + Expect(err).NotTo(HaveOccurred()) + + conf := fmt.Sprintf(`{ + "cniVersion": "%s", + "name": "mynet", + "type": "ptp", + "ipMasq": true, + "ipMasqBackend": "iptables", + "mtu": 5000, + "ipam": { + "type": "host-local", + "subnet": "10.1.2.0/24", + "dataDir": "%s" + }, + "dns": %s + }`, ver, dataDir, string(dnsConfBytes)) + + doTest(conf, ver, 1, dnsConf, targetNS) + }) + + It(fmt.Sprintf("[%s] configures and deconfigures a ptp link when specifying ipMasqBackend: nftables", ver), func() { + dnsConf := types.DNS{ + Nameservers: []string{"10.1.2.123"}, + Domain: "some.domain.test", + Search: []string{"search.test"}, + Options: []string{"option1:foo"}, + } + dnsConfBytes, err := json.Marshal(dnsConf) + Expect(err).NotTo(HaveOccurred()) + + conf := fmt.Sprintf(`{ + "cniVersion": "%s", + "name": "mynet", + "type": "ptp", + "ipMasq": true, + "ipMasqBackend": "nftables", + "mtu": 5000, + "ipam": { + "type": "host-local", + "subnet": "10.1.2.0/24", + "dataDir": "%s" + }, + "dns": %s + }`, ver, dataDir, string(dnsConfBytes)) + + doTest(conf, ver, 1, dnsConf, targetNS) + }) + It(fmt.Sprintf("[%s] configures and deconfigures a dual-stack ptp link + routes with ADD/DEL", ver), func() { conf := fmt.Sprintf(`{ "cniVersion": "%s",