-
Notifications
You must be signed in to change notification settings - Fork 798
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
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>
1 parent
8632ace
commit 84e9132
Showing
5 changed files
with
425 additions
and
20 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,203 @@ | ||
// 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" | ||
types100 "github.com/containernetworking/cni/pkg/types/100" | ||
"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]*types100.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 = types100.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) | ||
timeoutSec := 1 | ||
if err := testutils.Ping(saddr, daddr, 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()) | ||
}) | ||
}) | ||
}) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,175 @@ | ||
// 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. | ||
|
||
// This is a sample chained plugin that supports multiple CNI versions. It | ||
// parses prevResult according to the cniVersion | ||
package main | ||
|
||
import ( | ||
"fmt" | ||
|
||
types100 "github.com/containernetworking/cni/pkg/types/100" | ||
"github.com/containernetworking/plugins/pkg/utils" | ||
"github.com/coreos/go-iptables/iptables" | ||
) | ||
|
||
func setupIngressPolicy(conf *FirewallNetConf, prevResult *types100.Result) error { | ||
switch conf.IngressPolicy { | ||
case "", IngressPolicyOpen: | ||
// NOP | ||
return nil | ||
case IngressPolicySameNetwork: | ||
return setupIngressPolicySameNetwork(conf, prevResult) | ||
default: | ||
return fmt.Errorf("unknown ingress policy: %q", conf.IngressPolicy) | ||
} | ||
} | ||
|
||
func setupIngressPolicySameNetwork(conf *FirewallNetConf, prevResult *types100.Result) error { | ||
if len(prevResult.Interfaces) == 0 { | ||
return fmt.Errorf("interface needs to be set for ingress policy %q, make sure to chain \"firewall\" plugin with \"bridge\"", | ||
conf.IngressPolicy) | ||
} | ||
intf := prevResult.Interfaces[0] | ||
if intf == nil { | ||
return fmt.Errorf("got nil interface") | ||
} | ||
bridgeName := intf.Name | ||
if bridgeName == "" { | ||
return fmt.Errorf("got empty bridge name") | ||
} | ||
for _, iptProto := range findProtos(conf) { | ||
ipt, err := iptables.NewWithProtocol(iptProto) | ||
if err != nil { | ||
return err | ||
} | ||
if err := setupIsolationChains(ipt, bridgeName); err != nil { | ||
return err | ||
} | ||
} | ||
return nil | ||
} | ||
|
||
func teardownIngressPolicy(conf *FirewallNetConf, prevResult *types100.Result) error { | ||
switch conf.IngressPolicy { | ||
case "", IngressPolicyOpen: | ||
// NOP | ||
return nil | ||
case IngressPolicySameNetwork: | ||
// NOP | ||
// | ||
// We can't be sure whether conf.bridgeName is still in use by other containers. | ||
// So we do not remove the iptable rules that are created per bridge. | ||
return nil | ||
default: | ||
return fmt.Errorf("unknown ingress policy: %q", conf.IngressPolicy) | ||
} | ||
} | ||
|
||
const ( | ||
filterTableName = "filter" // built-in | ||
forwardChainName = "FORWARD" // built-in | ||
) | ||
|
||
// setupIsolationChains executes the following iptables commands for isolating networks: | ||
// ``` | ||
// iptables -N CNI-ISOLATION-STAGE-1 | ||
// iptables -N CNI-ISOLATION-STAGE-2 | ||
// # NOTE: "-j CNI-ISOLATION-STAGE-1" needs to be before "CNI-FORWARD" chain. So we use -I here. | ||
// iptables -I FORWARD -j CNI-ISOLATION-STAGE-1 | ||
// iptables -A CNI-ISOLATION-STAGE-1 -i ${bridgeName} ! -o ${bridgeName} -j CNI-ISOLATION-STAGE-2 | ||
// iptables -A CNI-ISOLATION-STAGE-1 -j RETURN | ||
// iptables -A CNI-ISOLATION-STAGE-2 -o ${bridgeName} -j DROP | ||
// iptables -A CNI-ISOLATION-STAGE-2 -j RETURN | ||
// ``` | ||
func setupIsolationChains(ipt *iptables.IPTables, bridgeName string) error { | ||
const ( | ||
// Future version may support custom chain names | ||
stage1Chain = "CNI-ISOLATION-STAGE-1" | ||
stage2Chain = "CNI-ISOLATION-STAGE-2" | ||
) | ||
// Commands: | ||
// ``` | ||
// iptables -N CNI-ISOLATION-STAGE-1 | ||
// iptables -N CNI-ISOLATION-STAGE-2 | ||
// ``` | ||
for _, chain := range []string{stage1Chain, stage2Chain} { | ||
if err := utils.EnsureChain(ipt, filterTableName, chain); err != nil { | ||
return err | ||
} | ||
} | ||
|
||
// Commands: | ||
// ``` | ||
// iptables -I FORWARD -j CNI-ISOLATION-STAGE-1 | ||
// ``` | ||
jumpToStage1 := withDefaultComment([]string{"-j", stage1Chain}) | ||
// NOTE: "-j CNI-ISOLATION-STAGE-1" needs to be before "CNI-FORWARD" created by CNI firewall plugin. | ||
// So we specify prepend = true . | ||
const jumpToStage1Prepend = true | ||
if err := utils.InsertUnique(ipt, filterTableName, forwardChainName, jumpToStage1Prepend, jumpToStage1); err != nil { | ||
return err | ||
} | ||
|
||
// Commands: | ||
// ``` | ||
// iptables -A CNI-ISOLATION-STAGE-1 -i ${bridgeName} ! -o ${bridgeName} -j CNI-ISOLATION-STAGE-2 | ||
// iptables -A CNI-ISOLATION-STAGE-1 -j RETURN | ||
// ``` | ||
stage1Bridge := withDefaultComment(isolationStage1BridgeRule(bridgeName, stage2Chain)) | ||
// prepend = true because this needs to be before "-j RETURN" | ||
const stage1BridgePrepend = true | ||
if err := utils.InsertUnique(ipt, filterTableName, stage1Chain, stage1BridgePrepend, stage1Bridge); err != nil { | ||
return err | ||
} | ||
stage1Return := withDefaultComment([]string{"-j", "RETURN"}) | ||
if err := utils.InsertUnique(ipt, filterTableName, stage1Chain, false, stage1Return); err != nil { | ||
return err | ||
} | ||
|
||
// Commands: | ||
// ``` | ||
// iptables -A CNI-ISOLATION-STAGE-2 -o ${bridgeName} -j DROP | ||
// iptables -A CNI-ISOLATION-STAGE-2 -j RETURN | ||
// ``` | ||
stage2Bridge := withDefaultComment(isolationStage2BridgeRule(bridgeName)) | ||
// prepend = true because this needs to be before "-j RETURN" | ||
const stage2BridgePrepend = true | ||
if err := utils.InsertUnique(ipt, filterTableName, stage2Chain, stage2BridgePrepend, stage2Bridge); err != nil { | ||
return err | ||
} | ||
stage2Return := withDefaultComment([]string{"-j", "RETURN"}) | ||
if err := utils.InsertUnique(ipt, filterTableName, stage2Chain, false, stage2Return); err != nil { | ||
return err | ||
} | ||
|
||
return nil | ||
} | ||
|
||
func isolationStage1BridgeRule(bridgeName, stage2Chain string) []string { | ||
return []string{"-i", bridgeName, "!", "-o", bridgeName, "-j", stage2Chain} | ||
} | ||
|
||
func isolationStage2BridgeRule(bridgeName string) []string { | ||
return []string{"-o", bridgeName, "-j", "DROP"} | ||
} | ||
|
||
func withDefaultComment(rule []string) []string { | ||
defaultComment := fmt.Sprintf("CNI firewall plugin rules (ingressPolicy: same-network)") | ||
return withComment(rule, defaultComment) | ||
} | ||
|
||
func withComment(rule []string, comment string) []string { | ||
return append(rule, []string{"-m", "comment", "--comment", comment}...) | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters