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

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

@@ -77,6 +77,9 @@ func cmdAdd(args *skel.CmdArgs) error {
 	if err != nil {
 		return fmt.Errorf("failed to parse config: %v", err)
 	}
+	if netConf.ContIPv4.IP == nil || netConf.ContIPv4.Mask == nil {
+		return fmt.Errorf(" parsed netConf doesn't have an IPv4 field: %v", netConf)
+	}
 
 	if netConf.PrevResult == nil {
 		return fmt.Errorf("must be called as chained plugin")
@@ -121,18 +124,27 @@ func cmdDel(args *skel.CmdArgs) error {
 	if err != nil {
 		return fmt.Errorf("failed to parse config: %v", err)
 	}
+	if netConf.ContIPv4.IP == nil || netConf.ContIPv4.Mask == nil {
+		return fmt.Errorf(" parsed netConf doesn't have an IPv4 field: %v", netConf)
+	}
 
 	if len(netConf.RuntimeConfig.PortMaps) == 0 {
 		return nil
 	}
 
 	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
 }
 
@@ -141,30 +153,33 @@ func main() {
 }
 
 func cmdCheck(args *skel.CmdArgs) error {
-	conf, result, err := parseConfig(args.StdinData, args.IfName)
+	netConf, result, err := parseConfig(args.StdinData, args.IfName)
 	if err != nil {
 		return err
 	}
+	if netConf.ContIPv4.IP == nil || netConf.ContIPv4.Mask == nil {
+		return fmt.Errorf(" parsed netConf doesn't have an IPv4 field: %v", netConf)
+	}
 
 	// Ensure we have previous result.
 	if result == nil {
 		return fmt.Errorf("Required prevResult missing")
 	}
 
-	if len(conf.RuntimeConfig.PortMaps) == 0 {
+	if len(netConf.RuntimeConfig.PortMaps) == 0 {
 		return nil
 	}
 
-	conf.ContainerID = args.ContainerID
+	netConf.ContainerID = args.ContainerID
 
-	if conf.ContIPv4.IP != nil {
-		if err := checkPorts(conf, conf.ContIPv4); err != nil {
+	if netConf.ContIPv4.IP != nil {
+		if err := checkPorts(netConf, netConf.ContIPv4); err != nil {
 			return err
 		}
 	}
 
-	if conf.ContIPv6.IP != nil {
-		if err := checkPorts(conf, conf.ContIPv6); err != nil {
+	if netConf.ContIPv6.IP != nil {
+		if err := checkPorts(netConf, netConf.ContIPv6); err != nil {
 			return err
 		}
 	}

plugins/meta/portmap/portmap.go

@@ -63,11 +63,16 @@ func forwardPorts(config *PortMapConf, containerNet net.IPNet) error {
 		ipt, err = iptables.NewWithProtocol(iptables.ProtocolIPv6)
 	} else {
 		ipt, err = iptables.NewWithProtocol(iptables.ProtocolIPv4)
+
 	}
 	if err != nil {
 		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 +122,21 @@ 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)
+	// check is called for each address, not once for all addresses
+	var ip4t *iptables.IPTables
+	var err4 error
+	var ip6t *iptables.IPTables
+	var err6 error
+	if isV6 {
+		ip6t, err6 = maybeGetIptables(true)
+	} else {
+		ip4t, err4 = maybeGetIptables(false)
+	}
+
 	if ip4t == nil && ip6t == nil {
 		err := fmt.Errorf("neither iptables nor ip6tables is usable")
 		err = fmt.Errorf("%v, (iptables) %v", err, err4)
@@ -360,14 +375,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 unforwardPortsOld(config *PortMapConf, containerNet net.IPNet) error {
 	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)
+
 	if ip4t == nil && ip6t == nil {
 		err := fmt.Errorf("neither iptables nor ip6tables is usable")
 		err = fmt.Errorf("%v, (iptables) %v", err, err4)
@@ -376,20 +392,65 @@ func unforwardPorts(config *PortMapConf) error {
 	}
 
 	if ip4t != 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 with containerNet-ContIPv4: %v err: %v", containerNet, err)
+		}
+
 		if err := dnatChain.teardown(ip4t); err != nil {
 			return fmt.Errorf("could not teardown ipv4 dnat: %v", err)
 		}
 		oldSnatChain.teardown(ip4t)
 	}
 
 	if ip6t != nil {
+		err := ip6t.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 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)
 	}
 	return nil
 }
+func unforwardPorts(config *PortMapConf, containerNet net.IPNet) error {
+	isV6 := (containerNet.IP.To4() == nil)
+	dnatChain := genDnatChain(config.Name, config.ContainerID)
+	//fillDnatRules(&dnatChain, config, containerNet)
+
+	// Might be lying around from old versions
+	oldSnatChain := genOldSnatChain(config.Name, config.ContainerID)
+
+	// unforward is called for each address, not once for all addresses
+	var ipt *iptables.IPTables
+	ipt, err := maybeGetIptables(false)
+
+	if isV6 {
+		ipt, err = iptables.NewWithProtocol(iptables.ProtocolIPv6)
+	} else {
+		ipt, err = iptables.NewWithProtocol(iptables.ProtocolIPv4)
+	}
+	if err != nil {
+		return fmt.Errorf("failed to open iptables: %v", err)
+	}
+
+	if ipt != nil {
+		err = ipt.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 FORWARD dnat: %v", err)
+		}
+
+		if err = dnatChain.teardown(ipt); err != nil {
+			return fmt.Errorf("could not teardown dnat: %v", err)
+		}
+		oldSnatChain.teardown(ipt)
+	}
+
+	return nil
+}
 
 // maybeGetIptables implements the soft error swallowing. If iptables is
 // usable for the given protocol, returns a handle, otherwise nil
@@ -427,3 +488,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()
+}

plugins/meta/portmap/portmap_integ_test.go

@@ -94,7 +94,8 @@ var _ = Describe("portmap integration tests", func() {
 		testutils.UnmountNS(targetNS)
 	})
 
-	for _, ver := range []string{"0.3.0", "0.3.1", "0.4.0", "1.0.0"} {
+	//for _, ver := range []string{"0.3.0", "0.3.1", "0.4.0", "1.0.0"} {
+	for _, ver := range []string{"0.4.0", "1.0.0"} {
 		// Redefine ver inside for scope so real value is picked up by each dynamically defined It()
 		// See Gingkgo's "Patterns for dynamically generating tests" documentation.
 		ver := ver
@@ -115,6 +116,11 @@ var _ = Describe("portmap integration tests", func() {
 								"containerPort": containerPort,
 								"protocol":      "tcp",
 							},
+							{
+								"hostPort":      hostPort,
+								"containerPort": containerPort,
+								"protocol":      "udp",
+							},
 						},
 					},
 				}
@@ -205,6 +211,12 @@ var _ = Describe("portmap integration tests", func() {
 				err = deleteNetwork()
 				Expect(err).NotTo(HaveOccurred())
 
+				// dump iptables-save output for debugging
+				cmd = exec.Command("iptables-save")
+				cmd.Stderr = GinkgoWriter
+				cmd.Stdout = GinkgoWriter
+				Expect(cmd.Run()).To(Succeed())
+
 				// Verify iptables rules are gone
 				_, err = ipt.List("nat", dnatChainName)
 				Expect(err).To(HaveOccurred())
@@ -287,22 +299,22 @@ var _ = Describe("portmap integration tests", func() {
 					hostIP, hostPort, contIP, containerPort)
 
 				// dump iptables-save output for debugging
-				cmd = exec.Command("iptables-save")
-				cmd.Stderr = GinkgoWriter
-				cmd.Stdout = GinkgoWriter
-				Expect(cmd.Run()).To(Succeed())
+				//cmd = exec.Command("iptables-save")
+				//cmd.Stderr = GinkgoWriter
+				//cmd.Stdout = GinkgoWriter
+				//Expect(cmd.Run()).To(Succeed())
 
 				// dump ip routes output for debugging
-				cmd = exec.Command("ip", "route")
-				cmd.Stderr = GinkgoWriter
-				cmd.Stdout = GinkgoWriter
-				Expect(cmd.Run()).To(Succeed())
+				//cmd = exec.Command("ip", "route")
+				//cmd.Stderr = GinkgoWriter
+				//cmd.Stdout = GinkgoWriter
+				//Expect(cmd.Run()).To(Succeed())
 
 				// dump ip addresses output for debugging
-				cmd = exec.Command("ip", "addr")
-				cmd.Stderr = GinkgoWriter
-				cmd.Stdout = GinkgoWriter
-				Expect(cmd.Run()).To(Succeed())
+				//cmd = exec.Command("ip", "addr")
+				//cmd.Stderr = GinkgoWriter
+				//cmd.Stdout = GinkgoWriter
+				//Expect(cmd.Run()).To(Succeed())
 
 				// Sanity check: verify that the container is reachable directly
 				fmt.Fprintln(GinkgoWriter, "Connect to container:", contIP.String(), containerPort)
@@ -382,22 +394,22 @@ var _ = Describe("portmap integration tests", func() {
 					hostIP, hostPort, contIP2, containerPort)
 
 				// dump iptables-save output for debugging
-				cmd = exec.Command("iptables-save")
-				cmd.Stderr = GinkgoWriter
-				cmd.Stdout = GinkgoWriter
-				Expect(cmd.Run()).To(Succeed())
+				//cmd = exec.Command("iptables-save")
+				//cmd.Stderr = GinkgoWriter
+				//cmd.Stdout = GinkgoWriter
+				//Expect(cmd.Run()).To(Succeed())
 
 				// dump ip routes output for debugging
-				cmd = exec.Command("ip", "route")
-				cmd.Stderr = GinkgoWriter
-				cmd.Stdout = GinkgoWriter
-				Expect(cmd.Run()).To(Succeed())
+				//cmd = exec.Command("ip", "route")
+				//cmd.Stderr = GinkgoWriter
+				//cmd.Stdout = GinkgoWriter
+				//Expect(cmd.Run()).To(Succeed())
 
 				// dump ip addresses output for debugging
-				cmd = exec.Command("ip", "addr")
-				cmd.Stderr = GinkgoWriter
-				cmd.Stdout = GinkgoWriter
-				Expect(cmd.Run()).To(Succeed())
+				//cmd = exec.Command("ip", "addr")
+				//cmd.Stderr = GinkgoWriter
+				//cmd.Stdout = GinkgoWriter
+				//Expect(cmd.Run()).To(Succeed())
 
 				// Sanity check: verify that the container is reachable directly
 				fmt.Fprintln(GinkgoWriter, "Connect to container:", contIP2.String(), containerPort)