Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ipmasq: fix nftables backend #1120

Merged
merged 1 commit into from
Nov 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 27 additions & 8 deletions pkg/ip/ipmasq_iptables_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,26 +15,33 @@
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"
)

// setupIPMasqIPTables is the iptables-based implementation of SetupIPMasqForNetwork
func setupIPMasqIPTables(ipn *net.IPNet, network, _, containerID string) error {
// setupIPMasqIPTables is the iptables-based implementation of SetupIPMasqForNetworks
func setupIPMasqIPTables(ipns []*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)
for _, ip := range ipns {
if err := SetupIPMasq(ip, chain, comment); err != nil {
return err
}
}
return nil
}

// 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
// Deprecated: This function only supports iptables. Use SetupIPMasqForNetworks, which
// supports both iptables and nftables.
func SetupIPMasq(ipn *net.IPNet, chain string, comment string) error {
isV6 := ipn.IP.To4() == nil
Expand Down Expand Up @@ -87,16 +94,28 @@ func SetupIPMasq(ipn *net.IPNet, chain string, comment string) error {
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 {
// teardownIPMasqIPTables is the iptables-based implementation of TeardownIPMasqForNetworks
func teardownIPMasqIPTables(ipns []*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)

var errs []string
for _, ipn := range ipns {
err := TeardownIPMasq(ipn, chain, comment)
if err != nil {
errs = append(errs, err.Error())
}
}

if errs == nil {
return nil
}
return errors.New(strings.Join(errs, "\n"))
}

// TeardownIPMasq undoes the effects of SetupIPMasq.
// Deprecated: This function only supports iptables. Use TeardownIPMasqForNetwork, which
// Deprecated: This function only supports iptables. Use TeardownIPMasqForNetworks, which
// supports both iptables and nftables.
func TeardownIPMasq(ipn *net.IPNet, chain string, comment string) error {
isV6 := ipn.IP.To4() == nil
Expand Down
18 changes: 9 additions & 9 deletions pkg/ip/ipmasq_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,11 @@ import (
"github.com/containernetworking/plugins/pkg/utils"
)

// 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
// SetupIPMasqForNetworks installs rules to masquerade traffic coming from ips of ipns and
// going outside of ipns, 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 {
func SetupIPMasqForNetworks(backend *string, ipns []*net.IPNet, network, ifname, containerID string) error {
if backend == nil {
// Prefer iptables, unless only nftables is available
defaultBackend := "iptables"
Expand All @@ -40,27 +40,27 @@ func SetupIPMasqForNetwork(backend *string, ipn *net.IPNet, network, ifname, con

switch *backend {
case "iptables":
return setupIPMasqIPTables(ipn, network, ifname, containerID)
return setupIPMasqIPTables(ipns, network, ifname, containerID)
case "nftables":
return setupIPMasqNFTables(ipn, network, ifname, containerID)
return setupIPMasqNFTables(ipns, network, ifname, containerID)
default:
return fmt.Errorf("unknown ipmasq backend %q", *backend)
}
}

// TeardownIPMasqForNetwork undoes the effects of SetupIPMasqForNetwork
func TeardownIPMasqForNetwork(ipn *net.IPNet, network, ifname, containerID string) error {
// TeardownIPMasqForNetworks undoes the effects of SetupIPMasqForNetworks
func TeardownIPMasqForNetworks(ipns []*net.IPNet, network, ifname, containerID string) error {
var errs []string

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

err := teardownIPMasqIPTables(ipn, network, ifname, containerID)
err := teardownIPMasqIPTables(ipns, network, ifname, containerID)
if err != nil && utils.SupportsIPTables() {
errs = append(errs, err.Error())
}

err = teardownIPMasqNFTables(ipn, network, ifname, containerID)
err = teardownIPMasqNFTables(ipns, network, ifname, containerID)
if err != nil && utils.SupportsNFTables() {
errs = append(errs, err.Error())
}
Expand Down
50 changes: 26 additions & 24 deletions pkg/ip/ipmasq_nftables_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,16 +72,16 @@ func commentForInstance(network, ifname, containerID string) string {
return comment
}

// setupIPMasqNFTables is the nftables-based implementation of SetupIPMasqForNetwork
func setupIPMasqNFTables(ipn *net.IPNet, network, ifname, containerID string) error {
// setupIPMasqNFTables is the nftables-based implementation of SetupIPMasqForNetworks
func setupIPMasqNFTables(ipns []*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)
return setupIPMasqNFTablesWithInterface(nft, ipns, network, ifname, containerID)
}

func setupIPMasqNFTablesWithInterface(nft knftables.Interface, ipn *net.IPNet, network, ifname, containerID string) error {
func setupIPMasqNFTablesWithInterface(nft knftables.Interface, ipns []*net.IPNet, network, ifname, containerID string) error {
staleRules, err := findRules(nft, hashForInstance(network, ifname, containerID))
if err != nil {
return err
Expand Down Expand Up @@ -128,37 +128,39 @@ func setupIPMasqNFTablesWithInterface(nft knftables.Interface, ipn *net.IPNet, n
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}
for _, ipn := range ipns {
ip := "ip"
if ipn.IP.To4() == nil {
ip = "ip6"
}

tx.Add(&knftables.Rule{
Chain: ipMasqChainName,
Rule: knftables.Concat(
ip, "saddr", "==", ipn.IP,
ip, "daddr", "!=", dstNet,
"masquerade",
),
Comment: knftables.PtrTo(commentForInstance(network, ifname, containerID)),
})
// 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 {
// teardownIPMasqNFTables is the nftables-based implementation of TeardownIPMasqForNetworks
func teardownIPMasqNFTables(ipns []*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)
return teardownIPMasqNFTablesWithInterface(nft, ipns, network, ifname, containerID)
}

func teardownIPMasqNFTablesWithInterface(nft knftables.Interface, _ *net.IPNet, network, ifname, containerID string) error {
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
Expand Down
61 changes: 44 additions & 17 deletions pkg/ip/ipmasq_nftables_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package ip

import (
"net"
"strings"
"testing"

Expand All @@ -31,43 +32,55 @@ func Test_setupIPMasqNFTables(t *testing.T) {
network string
ifname string
containerID string
addr string
addrs []string
}{
{
network: "unit-test",
ifname: "eth0",
containerID: "one",
addr: "192.168.1.1/24",
addrs: []string{"192.168.1.1/24"},
},
{
network: "unit-test",
ifname: "eth0",
containerID: "two",
addr: "192.168.1.2/24",
addrs: []string{"192.168.1.2/24", "2001:db8::2/64"},
},
{
network: "unit-test",
ifname: "eth0",
containerID: "three",
addr: "192.168.99.5/24",
addrs: []string{"192.168.99.5/24"},
},
{
network: "alternate",
ifname: "net1",
containerID: "three",
addr: "10.0.0.5/24",
addrs: []string{
"10.0.0.5/24",
"10.0.0.6/24",
"10.0.1.7/24",
"2001:db8::5/64",
"2001:db8::6/64",
"2001:db8:1::7/64",
},
},
}

for _, c := range containers {
addr, err := netlink.ParseAddr(c.addr)
if err != nil {
t.Fatalf("failed to parse test addr: %v", err)
ipns := []*net.IPNet{}
for _, addr := range c.addrs {
nladdr, err := netlink.ParseAddr(addr)
if err != nil {
t.Fatalf("failed to parse test addr: %v", err)
}
ipns = append(ipns, nladdr.IPNet)
}
err = setupIPMasqNFTablesWithInterface(nft, addr.IPNet, c.network, c.ifname, c.containerID)
err := setupIPMasqNFTablesWithInterface(nft, ipns, c.network, c.ifname, c.containerID)
if err != nil {
t.Fatalf("error from setupIPMasqNFTables: %v", err)
}

}

expected := strings.TrimSpace(`
Expand All @@ -76,8 +89,14 @@ add chain inet cni_plugins_masquerade masq_checks { comment "Masquerade traffic
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 ip6 saddr == 2001:db8::2 ip6 daddr != 2001:db8::/64 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 == 10.0.0.6 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 == 10.0.1.7 ip daddr != 10.0.1.0/24 masquerade comment "82783ef24bdc7036-acb19d111858e348, net: alternate, if: net1, id: three"
add rule inet cni_plugins_masquerade masq_checks ip6 saddr == 2001:db8::5 ip6 daddr != 2001:db8::/64 masquerade comment "82783ef24bdc7036-acb19d111858e348, net: alternate, if: net1, id: three"
add rule inet cni_plugins_masquerade masq_checks ip6 saddr == 2001:db8::6 ip6 daddr != 2001:db8::/64 masquerade comment "82783ef24bdc7036-acb19d111858e348, net: alternate, if: net1, id: three"
add rule inet cni_plugins_masquerade masq_checks ip6 saddr == 2001:db8:1::7 ip6 daddr != 2001:db8:1::/64 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
Expand All @@ -88,22 +107,18 @@ add rule inet cni_plugins_masquerade postrouting goto masq_checks
}

// Add a new container reusing "one"'s address, before deleting "one"
addr, err := netlink.ParseAddr(containers[0].addr)
c := containers[0]
addr, err := netlink.ParseAddr(c.addrs[0])
if err != nil {
t.Fatalf("failed to parse test addr: %v", err)
}
err = setupIPMasqNFTablesWithInterface(nft, addr.IPNet, "unit-test", "eth0", "four")
err = setupIPMasqNFTablesWithInterface(nft, []*net.IPNet{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)
err = teardownIPMasqNFTablesWithInterface(nft, []*net.IPNet{addr.IPNet}, c.network, c.ifname, c.containerID)
if err != nil {
t.Fatalf("error from teardownIPMasqNFTables: %v", err)
}
Expand All @@ -114,8 +129,14 @@ add table inet cni_plugins_masquerade { comment "Masquerading for plugins from g
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 ip6 saddr == 2001:db8::2 ip6 daddr != 2001:db8::/64 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 == 10.0.0.6 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 == 10.0.1.7 ip daddr != 10.0.1.0/24 masquerade comment "82783ef24bdc7036-acb19d111858e348, net: alternate, if: net1, id: three"
add rule inet cni_plugins_masquerade masq_checks ip6 saddr == 2001:db8::5 ip6 daddr != 2001:db8::/64 masquerade comment "82783ef24bdc7036-acb19d111858e348, net: alternate, if: net1, id: three"
add rule inet cni_plugins_masquerade masq_checks ip6 saddr == 2001:db8::6 ip6 daddr != 2001:db8::/64 masquerade comment "82783ef24bdc7036-acb19d111858e348, net: alternate, if: net1, id: three"
add rule inet cni_plugins_masquerade masq_checks ip6 saddr == 2001:db8:1::7 ip6 daddr != 2001:db8:1::/64 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
Expand Down Expand Up @@ -150,8 +171,14 @@ add table inet cni_plugins_masquerade { comment "Masquerading for plugins from g
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 ip6 saddr == 2001:db8::2 ip6 daddr != 2001:db8::/64 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 == 10.0.0.6 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 == 10.0.1.7 ip daddr != 10.0.1.0/24 masquerade comment "82783ef24bdc7036-acb19d111858e348, net: alternate, if: net1, id: three"
add rule inet cni_plugins_masquerade masq_checks ip6 saddr == 2001:db8::5 ip6 daddr != 2001:db8::/64 masquerade comment "82783ef24bdc7036-acb19d111858e348, net: alternate, if: net1, id: three"
add rule inet cni_plugins_masquerade masq_checks ip6 saddr == 2001:db8::6 ip6 daddr != 2001:db8::/64 masquerade comment "82783ef24bdc7036-acb19d111858e348, net: alternate, if: net1, id: three"
add rule inet cni_plugins_masquerade masq_checks ip6 saddr == 2001:db8:1::7 ip6 daddr != 2001:db8:1::/64 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
Expand Down
14 changes: 7 additions & 7 deletions plugins/main/bridge/bridge.go
Original file line number Diff line number Diff line change
Expand Up @@ -668,10 +668,12 @@ func cmdAdd(args *skel.CmdArgs) error {
}

if n.IPMasq {
ipns := []*net.IPNet{}
for _, ipc := range result.IPs {
if err = ip.SetupIPMasqForNetwork(n.IPMasqBackend, &ipc.Address, n.Name, args.IfName, args.ContainerID); err != nil {
return err
}
ipns = append(ipns, &ipc.Address)
}
if err = ip.SetupIPMasqForNetworks(n.IPMasqBackend, ipns, n.Name, args.IfName, args.ContainerID); err != nil {
return err
}
}
} else if !n.DisableContainerInterface {
Expand Down Expand Up @@ -807,10 +809,8 @@ func cmdDel(args *skel.CmdArgs) error {
}

if isLayer3 && n.IPMasq {
for _, ipn := range ipnets {
if err := ip.TeardownIPMasqForNetwork(ipn, n.Name, args.IfName, args.ContainerID); err != nil {
return err
}
if err := ip.TeardownIPMasqForNetworks(ipnets, n.Name, args.IfName, args.ContainerID); err != nil {
return err
}
}

Expand Down
Loading