When masquerade is set on bridge, add an iptables rule to drop any pa…

…ckets which conntrack consideres invalid.

When portmap is used in chain, do likewise.

Use container specific IP addresses in rules so that only this rule is removed in cniDel

Allow for portmap and ipMasq to co-exist or used independently

Fixes containernetworking#816

Signed-off-by: Michael Cambria <mcambria@redhat.com>

pkg/ip/ipmasq_linux.go

@@ -41,6 +41,10 @@ func SetupIPMasq(ipn *net.IPNet, chain string, comment string) error {
 		return fmt.Errorf("failed to locate iptables: %v", err)
 	}
 
+	if err := ipt.AppendUnique("filter", "FORWARD", "-s", ipn.IP.String(), "-m", "conntrack", "--ctstate", "INVALID", "-j", "DROP", "-m", "comment", "--comment", "bridge masq rule"); err != nil {
+		return err
+	}
+
 	// Create chain if doesn't exist
 	exists := false
 	chains, err := ipt.ListChains("nat")
@@ -90,6 +94,10 @@ func TeardownIPMasq(ipn *net.IPNet, chain string, comment string) error {
 		return fmt.Errorf("failed to locate iptables: %v", err)
 	}
 
+	if err := ipt.Delete("filter", "FORWARD", "-s", ipn.IP.String(), "-m", "conntrack", "--ctstate", "INVALID", "-j", "DROP", "-m", "comment", "--comment", "bridge masq rule"); err != nil {
+		return err
+	}
+
 	err = ipt.Delete("nat", "POSTROUTING", "-s", ipn.IP.String(), "-j", chain, "-m", "comment", "--comment", comment)
 	if err != nil && !isNotExist(err) {
 		return err

plugins/meta/portmap/main.go

@@ -128,11 +128,17 @@ func cmdDel(args *skel.CmdArgs) error {
 
 	netConf.ContainerID = args.ContainerID
 
-	// We don't need to parse out whether or not we're using v6 or snat,
-	// deletion is idempotent
-	if err := unforwardPorts(netConf); err != nil {
-		return err
+	if netConf.ContIPv4.IP != nil {
+		if err := unforwardPorts(netConf, netConf.ContIPv4); err != nil {
+			return err
+		}
 	}
+	if netConf.ContIPv6.IP != nil {
+		if err := unforwardPorts(netConf, netConf.ContIPv6); err != nil {
+			return err
+		}
+	}
+
 	return nil
 }
 

plugins/meta/portmap/portmap.go

@@ -68,6 +68,10 @@ func forwardPorts(config *PortMapConf, containerNet net.IPNet) error {
 		return fmt.Errorf("failed to open iptables: %v", err)
 	}
 
+	if err := ipt.AppendUnique("filter", "FORWARD", "-s", containerNet.IP.String(), "-m", "conntrack", "--ctstate", "INVALID", "-j", "DROP", "-m", "comment", "--comment", "portmap rule"); err != nil {
+		return err
+	}
+
 	// Enable masquerading for traffic as necessary.
 	// The DNAT chain sets a mark bit for traffic that needs masq:
 	// - connections from localhost
@@ -117,11 +121,12 @@ func forwardPorts(config *PortMapConf, containerNet net.IPNet) error {
 }
 
 func checkPorts(config *PortMapConf, containerNet net.IPNet) error {
+	isV6 := (containerNet.IP.To4() == nil)
 	dnatChain := genDnatChain(config.Name, config.ContainerID)
 	fillDnatRules(&dnatChain, config, containerNet)
 
 	ip4t, err4 := maybeGetIptables(false)
-	ip6t, err6 := maybeGetIptables(true)
+	ip6t, err6 := maybeGetIptables(isV6)
 	if ip4t == nil && ip6t == nil {
 		err := fmt.Errorf("neither iptables nor ip6tables is usable")
 		err = fmt.Errorf("%v, (iptables) %v", err, err4)
@@ -360,14 +365,15 @@ func genOldSnatChain(netName, containerID string) chain {
 // don't know which protocols were used.
 // So, we first check that iptables is "generally OK" by doing a check. If
 // not, we ignore the error, unless neither v4 nor v6 are OK.
-func unforwardPorts(config *PortMapConf) error {
+func unforwardPorts(config *PortMapConf, containerNet net.IPNet) error {
+	isV6 := (containerNet.IP.To4() == nil)
 	dnatChain := genDnatChain(config.Name, config.ContainerID)
 
 	// Might be lying around from old versions
 	oldSnatChain := genOldSnatChain(config.Name, config.ContainerID)
 
 	ip4t, err4 := maybeGetIptables(false)
-	ip6t, err6 := maybeGetIptables(true)
+	ip6t, err6 := maybeGetIptables(isV6)
 	if ip4t == nil && ip6t == nil {
 		err := fmt.Errorf("neither iptables nor ip6tables is usable")
 		err = fmt.Errorf("%v, (iptables) %v", err, err4)
@@ -376,14 +382,24 @@ func unforwardPorts(config *PortMapConf) error {
 	}
 
 	if ip4t != nil {
-		if err := dnatChain.teardown(ip4t); err != nil {
+		err := ip4t.Delete("filter", "FORWARD", "-s", containerNet.IP.String(), "-m", "conntrack", "--ctstate", "INVALID", "-j", "DROP", "-m", "comment", "--comment", "portmap rule")
+		if err != nil && !isNotExist(err) {
+			return fmt.Errorf("could not teardown ipv4 FORWARD dnat: %v", err)
+		}
+
+		if err = dnatChain.teardown(ip4t); err != nil {
 			return fmt.Errorf("could not teardown ipv4 dnat: %v", err)
 		}
 		oldSnatChain.teardown(ip4t)
 	}
 
 	if ip6t != nil {
-		if err := dnatChain.teardown(ip6t); err != nil {
+		err := ip6t.Delete("filter", "FORWARD", "-m", "conntrack", "--ctstate", "INVALID", "-j", "DROP")
+		if err != nil && !isNotExist(err) {
+			return fmt.Errorf("could not teardown ipv6 FORWARD dnat: %v", err)
+		}
+
+		if err = dnatChain.teardown(ip6t); err != nil {
 			return fmt.Errorf("could not teardown ipv6 dnat: %v", err)
 		}
 		oldSnatChain.teardown(ip6t)
@@ -427,3 +443,13 @@ func deletePortmapStaleConnections(portMappings []PortMapEntry, family netlink.I
 	}
 	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()
+}