firewall: support ingressPolicy=(open|same-network) for isolating bri…

…dges as in Docker

This commit adds a new parameter `ingressPolicy` (`string`) to the `firewall` plugin.
The supported values are `open` and `same-network`.

- `open` is the default and does NOP.

- `same-network` creates "CNI-ISOLATION-STAGE-1" and "CNI-ISOLATION-STAGE-2"
that are similar to Docker libnetwork's "DOCKER-ISOLATION-STAGE-1" and
"DOCKER-ISOLATION-STAGE-2" rules.

e.g., when `ns1` and `ns2` are connected to bridge `cni1`, and `ns3` is
connected to bridge `cni2`, the `same-network` ingress policy disallows
communications between `ns1` and `ns3`, while allowing communications
between `ns1` and `ns2`.

Please refer to the comment lines in `ingresspolicy.go` for the actual iptables rules.

The `same-network` ingress policy is expected to be used in conjunction
with `bridge` plugin. May not work as expected with other "main" plugins.

It should be also noted that the `same-network` ingress policy executes
raw `iptables` commands directly, even when the `backend` is set to `firewalld`.
We could potentially use the "direct" API of firewalld [1] to execute
iptables via firewalld, but it doesn't seem to have a clear benefit over just directly
executing raw iptables commands.
(Anyway, we have been already executing raw iptables commands in the `portmap` plugin)

[1] https://firewalld.org/documentation/direct/options.html

This commit replaces the `isolation` plugin proposal (issue 573, PR 574).
The design of `ingressPolicy` was discussed in the comments of the withdrawn PR 574 .

Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>

pkg/utils/iptables.go

@@ -119,3 +119,21 @@ func ClearChain(ipt *iptables.IPTables, table, chain string) error {
 		return err
 	}
 }
+
+// InsertUnique will add a rule to a chain if it does not already exist.
+// By default the rule is appended, unless prepend is true.
+func InsertUnique(ipt *iptables.IPTables, table, chain string, prepend bool, rule []string) error {
+	exists, err := ipt.Exists(table, chain, rule...)
+	if err != nil {
+		return err
+	}
+	if exists {
+		return nil
+	}
+
+	if prepend {
+		return ipt.Insert(table, chain, 1, rule...)
+	} else {
+		return ipt.Append(table, chain, rule...)
+	}
+}

plugins/meta/firewall/firewall.go

@@ -46,8 +46,27 @@ type FirewallNetConf struct {
 	// the firewalld backend is used but the zone is not given, it defaults
 	// to 'trusted'
 	FirewalldZone string `json:"firewalldZone,omitempty"`
+
+	// IngressPolicy is an optional ingress policy.
+	// Defaults to "open".
+	IngressPolicy IngressPolicy `json:"ingressPolicy,omitempty"`
 }
 
+// IngressPolicy is an ingress policy string.
+type IngressPolicy = string
+
+const (
+	// IngressPolicyOpen ("open"): all inbound connections to the container are accepted.
+	// IngressPolicyOpen is the default ingress policy.
+	IngressPolicyOpen IngressPolicy = "open"
+
+	// IngressPolicySameNetwork ("same-network"): connections from the same network are accepted, others are blocked.
+	// This is similar to how Docker libnetwork works.
+	// IngressPolicySameNetwork executes `iptables` regardless to the value of `Backend`.
+	// IngressPolicySameNetwork may not work as expected for non-bridge networks.
+	IngressPolicySameNetwork IngressPolicy = "same-network"
+)
+
 type FirewallBackend interface {
 	Add(*FirewallNetConf, *current.Result) error
 	Del(*FirewallNetConf, *current.Result) error
@@ -129,6 +148,10 @@ func cmdAdd(args *skel.CmdArgs) error {
 		return err
 	}
 
+	if err := setupIngressPolicy(conf, result); err != nil {
+		return err
+	}
+
 	if result == nil {
 		result = &current.Result{}
 	}
@@ -151,6 +174,10 @@ func cmdDel(args *skel.CmdArgs) error {
 		return err
 	}
 
+	if err := teardownIngressPolicy(conf, result); err != nil {
+		return err
+	}
+
 	return nil
 }
 

plugins/meta/firewall/firewall_integ_test.go

@@ -0,0 +1,204 @@
+// Copyright 2021 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 main
+
+import (
+	"context"
+	"fmt"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"strings"
+
+	"github.com/containernetworking/cni/libcni"
+	"github.com/containernetworking/cni/pkg/types/current"
+	"github.com/containernetworking/plugins/pkg/ns"
+	"github.com/containernetworking/plugins/pkg/testutils"
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+)
+
+// The integration tests expect the "firewall" binary to be present in $PATH.
+// To run test, e.g, : go test -exec "sudo -E PATH=$(pwd):/opt/cni/bin:$PATH" -v -ginkgo.v
+var _ = Describe("firewall integration tests (ingressPolicy: same-network)", func() {
+	// ns0: foo (10.88.3.0/24)
+	// ns1: foo (10.88.3.0/24)
+	// ns2: bar (10.88.4.0/24)
+	//
+	// ns0@foo can talk to ns1@foo, but cannot talk to ns2@bar
+	const nsCount = 3
+	var (
+		configListFoo *libcni.NetworkConfigList // "foo", 10.88.3.0/24
+		configListBar *libcni.NetworkConfigList // "bar", 10.88.4.0/24
+		cniConf       *libcni.CNIConfig
+		namespaces    [nsCount]ns.NetNS
+	)
+
+	BeforeEach(func() {
+		var err error
+		rawConfigFoo := `
+{
+   "cniVersion": "0.4.0",
+   "name": "foo",
+   "plugins": [
+      {
+         "type": "bridge",
+         "bridge": "foo",
+         "isGateway": true,
+         "ipMasq": true,
+         "hairpinMode": true,
+         "ipam": {
+            "type": "host-local",
+            "routes": [
+               {
+                  "dst": "0.0.0.0/0"
+               }
+            ],
+            "ranges": [
+               [
+                  {
+                     "subnet": "10.88.3.0/24",
+                     "gateway": "10.88.3.1"
+                  }
+               ]
+            ]
+         }
+      },
+      {
+         "type": "firewall",
+	 "backend": "iptables",
+         "ingressPolicy": "same-network"
+      }
+   ]
+}
+`
+		configListFoo, err = libcni.ConfListFromBytes([]byte(rawConfigFoo))
+		Expect(err).NotTo(HaveOccurred())
+
+		rawConfigBar := strings.ReplaceAll(rawConfigFoo, "foo", "bar")
+		rawConfigBar = strings.ReplaceAll(rawConfigBar, "10.88.3.", "10.88.4.")
+
+		configListBar, err = libcni.ConfListFromBytes([]byte(rawConfigBar))
+		Expect(err).NotTo(HaveOccurred())
+
+		// turn PATH in to CNI_PATH.
+		_, err = exec.LookPath("firewall")
+		Expect(err).NotTo(HaveOccurred())
+		dirs := filepath.SplitList(os.Getenv("PATH"))
+		cniConf = &libcni.CNIConfig{Path: dirs}
+
+		for i := 0; i < nsCount; i++ {
+			targetNS, err := testutils.NewNS()
+			Expect(err).NotTo(HaveOccurred())
+			fmt.Fprintf(GinkgoWriter, "namespace %d:%s\n", i, targetNS.Path())
+			namespaces[i] = targetNS
+		}
+	})
+
+	AfterEach(func() {
+		for _, targetNS := range namespaces {
+			if targetNS != nil {
+				targetNS.Close()
+			}
+		}
+	})
+
+	Describe("Testing with network foo and bar", func() {
+		It("should isolate foo from bar", func() {
+			var results [nsCount]*current.Result
+			for i := 0; i < nsCount; i++ {
+				runtimeConfig := libcni.RuntimeConf{
+					ContainerID: fmt.Sprintf("test-cni-firewall-%d", i),
+					NetNS:       namespaces[i].Path(),
+					IfName:      "eth0",
+				}
+
+				configList := configListFoo
+				switch i {
+				case 0, 1:
+				// leave foo
+				default:
+					configList = configListBar
+				}
+
+				// Clean up garbages produced during past failed executions
+				_ = cniConf.DelNetworkList(context.TODO(), configList, &runtimeConfig)
+
+				// Make delete idempotent, so we can clean up on failure
+				netDeleted := false
+				deleteNetwork := func() error {
+					if netDeleted {
+						return nil
+					}
+					netDeleted = true
+					return cniConf.DelNetworkList(context.TODO(), configList, &runtimeConfig)
+				}
+				// Create the network
+				res, err := cniConf.AddNetworkList(context.TODO(), configList, &runtimeConfig)
+				Expect(err).NotTo(HaveOccurred())
+				// nolint: errcheck
+				defer deleteNetwork()
+
+				results[i], err = current.NewResultFromResult(res)
+				Expect(err).NotTo(HaveOccurred())
+				fmt.Fprintf(GinkgoWriter, "results[%d]: %+v\n", i, results[i])
+			}
+			ping := func(src, dst int) error {
+				return namespaces[src].Do(func(ns.NetNS) error {
+					defer GinkgoRecover()
+					saddr := results[src].IPs[0].Address.IP.String()
+					daddr := results[dst].IPs[0].Address.IP.String()
+					srcNetName := results[src].Interfaces[0].Name
+					dstNetName := results[dst].Interfaces[0].Name
+
+					fmt.Fprintf(GinkgoWriter, "ping %s (ns%d@%s) -> %s (ns%d@%s)...",
+						saddr, src, srcNetName, daddr, dst, dstNetName)
+					v6 := results[dst].IPs[0].Version == "6"
+					timeoutSec := 1
+					if err := testutils.Ping(saddr, daddr, v6, timeoutSec); err != nil {
+						fmt.Fprintln(GinkgoWriter, "unpingable")
+						return err
+					}
+					fmt.Fprintln(GinkgoWriter, "pingable")
+					return nil
+				})
+			}
+
+			// ns0@foo can ping to ns1@foo
+			err := ping(0, 1)
+			Expect(err).NotTo(HaveOccurred())
+
+			// ns1@foo can ping to ns0@foo
+			err = ping(1, 0)
+			Expect(err).NotTo(HaveOccurred())
+
+			// ns0@foo cannot ping to ns2@bar
+			err = ping(0, 2)
+			Expect(err).To(HaveOccurred())
+
+			// ns1@foo cannot ping to ns2@bar
+			err = ping(1, 2)
+			Expect(err).To(HaveOccurred())
+
+			// ns2@bar cannot ping to ns0@foo
+			err = ping(2, 0)
+			Expect(err).To(HaveOccurred())
+
+			// ns2@bar cannot ping to ns1@foo
+			err = ping(2, 1)
+			Expect(err).To(HaveOccurred())
+		})
+	})
+})