Replace existing iptables handler with firewall code

Use the new firewall code vendored from CNI to replace the
existing iptables rule addition handler we had in place. This
adds proper support for firewalld and should be much better at
interacting with the firewall.

Signed-off-by: Matthew Heon <matthew.heon@gmail.com>

Closes: #1431
Approved by: baude

5 files changed, 29 insertions, 54 deletions

diff --git a/libpod/networking_linux.go b/libpod/networking_linux.go
index 77ab97910..17e79aa62 100644
--- a/libpod/networking_linux.go
+++ b/libpod/networking_linux.go
@@ -14,9 +14,9 @@ import (
 
 	cnitypes "github.com/containernetworking/cni/pkg/types/current"
 	"github.com/containernetworking/plugins/pkg/ns"
+	"github.com/containers/libpod/pkg/firewall"
 	"github.com/containers/libpod/pkg/inspect"
 	"github.com/containers/libpod/pkg/netns"
-	"github.com/containers/libpod/utils"
 	"github.com/cri-o/ocicni/pkg/ocicni"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
@@ -63,14 +63,15 @@ func (r *Runtime) configureNetNS(ctr *Container, ctrNS ns.NetNS) (err error) {
 		ctr.state.NetworkStatus = append(ctr.state.NetworkStatus, resultCurrent)
 	}
 
-	for _, r := range ctr.state.NetworkStatus {
-		// We need to temporarily use iptables to allow the container
-		// to resolve DNS until this issue is fixed upstream.
-		// https://github.com/containernetworking/plugins/pull/75
-		for _, ip := range r.IPs {
-			if ip.Address.IP.To4() != nil {
-				iptablesDNS("-I", ip.Address.IP.String())
-			}
+	// Add firewall rules to ensure the container has network access.
+	// Will not be necessary once CNI firewall plugin merges upstream.
+	// https://github.com/containernetworking/plugins/pull/75
+	for _, netStatus := range ctr.state.NetworkStatus {
+		firewallConf := &firewall.FirewallNetConf{
+			PrevResult: netStatus,
+		}
+		if err := r.firewallBackend.Add(firewallConf); err != nil {
+			return errors.Wrapf(err, "error adding firewall rules for container %s", ctr.ID())
 		}
 	}
 
@@ -164,18 +165,6 @@ func (r *Runtime) setupNetNS(ctr *Container) (err error) {
 	return r.configureNetNS(ctr, netNS)
 }
 
-// iptablesDNS accepts an arg (-I|-D) and IP address of the container and then
-// generates an iptables command to either add or subtract the needed rule
-func iptablesDNS(arg, ip string) error {
-	iptablesCmd := []string{"-t", "filter", arg, "FORWARD", "-s", ip, "!", "-o", ip, "-j", "ACCEPT"}
-	logrus.Debug("Running iptables command: ", strings.Join(iptablesCmd, " "))
-	_, err := utils.ExecCmd("iptables", iptablesCmd...)
-	if err != nil {
-		logrus.Error(err)
-	}
-	return err
-}
-
 // Join an existing network namespace
 func joinNetNS(path string) (ns.NetNS, error) {
 	ns, err := ns.GetNS(path)
@@ -213,15 +202,15 @@ func (r *Runtime) teardownNetNS(ctr *Container) error {
 		return nil
 	}
 
-	// Because we are using iptables to allow the container to resolve DNS
-	// on per IP address, we also need to try to remove the iptables rule
-	// on cleanup. Remove when https://github.com/containernetworking/plugins/pull/75
-	// is merged.
-	for _, r := range ctr.state.NetworkStatus {
-		for _, ip := range r.IPs {
-			if ip.Address.IP.To4() != nil {
-				iptablesDNS("-D", ip.Address.IP.String())
-			}
+	// Remove firewall rules we added on configuring the container.
+	// Will not be necessary once CNI firewall plugin merges upstream.
+	// https://github.com/containernetworking/plugins/pull/75
+	for _, netStatus := range ctr.state.NetworkStatus {
+		firewallConf := &firewall.FirewallNetConf{
+			PrevResult: netStatus,
+		}
+		if err := r.firewallBackend.Del(firewallConf); err != nil {
+			return errors.Wrapf(err, "error removing firewall rules for container %s", ctr.ID())
 		}
 	}
 
diff --git a/pkg/firewall/common.go b/pkg/firewall/common.go
index 993c691cd..a65d4f03d 100644
--- a/pkg/firewall/common.go
+++ b/pkg/firewall/common.go
@@ -21,6 +21,8 @@ import (
 )
 
 // FirewallNetConf represents the firewall configuration.
+// Nolint applied for firewall.Firewall... name duplication notice.
+//nolint
 type FirewallNetConf struct {
 	//types.NetConf
 
@@ -33,11 +35,13 @@ type FirewallNetConf struct {
 	// to 'trusted'
 	FirewalldZone string
 
-	PrevResult    *current.Result
+	PrevResult *current.Result
 }
 
 // FirewallBackend is an interface to the system firewall, allowing addition and
 // removal of firewall rules.
+// Nolint applied for firewall.Firewall... name duplication notice.
+//nolint
 type FirewallBackend interface {
 	Add(*FirewallNetConf) error
 	Del(*FirewallNetConf) error
diff --git a/pkg/firewall/firewall_none.go b/pkg/firewall/firewall_none.go
index 9f9594b4a..9add24842 100644
--- a/pkg/firewall/firewall_none.go
+++ b/pkg/firewall/firewall_none.go
@@ -19,8 +19,10 @@ import (
 )
 
 // FirewallNone is a firewall backend for environments where manipulating the
-// system firewall is unsupported (for example, when running without root)
-type FirewallNone struct {}
+// system firewall is unsupported (for example, when running without root).
+// Nolint applied to avoid firewall.FirewallNone name duplication notes.
+//nolint
+type FirewallNone struct{}
 
 func newNoneBackend() (FirewallBackend, error) {
 	return &FirewallNone{}, nil
diff --git a/pkg/firewall/iptables.go b/pkg/firewall/iptables.go
index 9f065dbcf..59d81b287 100644
--- a/pkg/firewall/iptables.go
+++ b/pkg/firewall/iptables.go
@@ -51,10 +51,6 @@ func generateFilterRule(privChainName string) []string {
 	return []string{"-m", "comment", "--comment", "CNI firewall plugin rules", "-j", privChainName}
 }
 
-func generateAdminRule(adminChainName string) []string {
-	return []string{"-m", "comment", "--comment", "CNI firewall plugin admin overrides", "-j", adminChainName}
-}
-
 func cleanupRules(ipt *iptables.IPTables, privChainName string, rules [][]string) {
 	for _, rule := range rules {
 		ipt.Delete("filter", privChainName, rule...)
@@ -148,23 +144,6 @@ func (ib *iptablesBackend) delRules(conf *FirewallNetConf, ipt *iptables.IPTable
 	return nil
 }
 
-func findProtos(conf *FirewallNetConf) []iptables.Protocol {
-	protos := []iptables.Protocol{iptables.ProtocolIPv4, iptables.ProtocolIPv6}
-	if conf.PrevResult != nil {
-		// If PrevResult is given, scan all IP addresses to figure out
-		// which IP versions to use
-		protos = []iptables.Protocol{}
-		for _, addr := range conf.PrevResult.IPs {
-			if addr.Address.IP.To4() != nil {
-				protos = append(protos, iptables.ProtocolIPv4)
-			} else {
-				protos = append(protos, iptables.ProtocolIPv6)
-			}
-		}
-	}
-	return protos
-}
-
 type iptablesBackend struct {
 	protos         map[iptables.Protocol]*iptables.IPTables
 	privChainName  string
diff --git a/vendor.conf b/vendor.conf
index 682b597b9..a50d3822e 100644
--- a/vendor.conf
+++ b/vendor.conf
@@ -96,3 +96,4 @@ github.com/fsouza/go-dockerclient master
 github.com/openshift/imagebuilder master
 github.com/ulikunitz/xz v0.5.4
 github.com/mailru/easyjson 03f2033d19d5860aef995fe360ac7d395cd8ce65
+github.com/coreos/go-iptables 25d087f3cffd9aedc0c2b7eff25f23cbf3c20fe1