Merge pull request #18804 from marun/fix-nodeport-services

Auto commit by PR queue bot

pkg/proxy/userspace/proxier.go

@@ -213,18 +213,28 @@ func CleanupLeftovers(ipt iptables.Interface) (encounteredError bool) {
 		glog.Errorf("Error removing userspace rule: %v", err)
 		encounteredError = true
 	}
+	args = []string{"-m", "comment", "--comment", "Ensure that non-local NodePort traffic can flow"}
+	if err := ipt.DeleteRule(iptables.TableFilter, iptables.ChainInput, append(args, "-j", string(iptablesNonLocalNodePortChain))...); err != nil {
+		glog.Errorf("Error removing userspace rule: %v", err)
+		encounteredError = true
+	}
 
 	// flush and delete chains.
-	chains := []iptables.Chain{iptablesContainerPortalChain, iptablesHostPortalChain, iptablesHostNodePortChain, iptablesContainerNodePortChain}
+	tableChains := map[iptables.Table][]iptables.Chain{
-	for _, c := range chains {
+		iptables.TableNAT:    {iptablesContainerPortalChain, iptablesHostPortalChain, iptablesHostNodePortChain, iptablesContainerNodePortChain},
-		// flush chain, then if successful delete, delete will fail if flush fails.
+		iptables.TableFilter: {iptablesNonLocalNodePortChain},
-		if err := ipt.FlushChain(iptables.TableNAT, c); err != nil {
+	}
-			glog.Errorf("Error flushing userspace chain: %v", err)
+	for table, chains := range tableChains {
-			encounteredError = true
+		for _, c := range chains {
-		} else {
+			// flush chain, then if successful delete, delete will fail if flush fails.
-			if err = ipt.DeleteChain(iptables.TableNAT, c); err != nil {
+			if err := ipt.FlushChain(table, c); err != nil {
-				glog.Errorf("Error deleting userspace chain: %v", err)
+				glog.Errorf("Error flushing userspace chain: %v", err)
 				encounteredError = true
+			} else {
+				if err = ipt.DeleteChain(table, c); err != nil {
+					glog.Errorf("Error deleting userspace chain: %v", err)
+					encounteredError = true
+				}
 			}
 		}
 	}
@@ -626,6 +636,17 @@ func (proxier *Proxier) openNodePort(nodePort int, protocol api.Protocol, proxyI
 	if !existed {
 		glog.Infof("Opened iptables from-host public port for service %q on %s port %d", name, protocol, nodePort)
 	}
+
+	args = proxier.iptablesNonLocalNodePortArgs(nodePort, protocol, proxyIP, proxyPort, name)
+	existed, err = proxier.iptables.EnsureRule(iptables.Append, iptables.TableFilter, iptablesNonLocalNodePortChain, args...)
+	if err != nil {
+		glog.Errorf("Failed to install iptables %s rule for service %q", iptablesNonLocalNodePortChain, name)
+		return err
+	}
+	if !existed {
+		glog.Infof("Opened iptables from-non-local public port for service %q on %s port %d", name, protocol, nodePort)
+	}
+
 	return nil
 }
 
@@ -711,6 +732,13 @@ func (proxier *Proxier) closeNodePort(nodePort int, protocol api.Protocol, proxy
 		el = append(el, err)
 	}
 
+	// Handle traffic not local to the host
+	args = proxier.iptablesNonLocalNodePortArgs(nodePort, protocol, proxyIP, proxyPort, name)
+	if err := proxier.iptables.DeleteRule(iptables.TableFilter, iptablesNonLocalNodePortChain, args...); err != nil {
+		glog.Errorf("Failed to delete iptables %s rule for service %q", iptablesNonLocalNodePortChain, name)
+		el = append(el, err)
+	}
+
 	if err := proxier.releaseNodePort(nil, nodePort, protocol, name); err != nil {
 		el = append(el, err)
 	}
@@ -743,6 +771,7 @@ var iptablesHostPortalChain iptables.Chain = "KUBE-PORTALS-HOST"
 // Chains for NodePort services
 var iptablesContainerNodePortChain iptables.Chain = "KUBE-NODEPORT-CONTAINER"
 var iptablesHostNodePortChain iptables.Chain = "KUBE-NODEPORT-HOST"
+var iptablesNonLocalNodePortChain iptables.Chain = "KUBE-NODEPORT-NON-LOCAL"
 
 // Ensure that the iptables infrastructure we use is set up.  This can safely be called periodically.
 func iptablesInit(ipt iptables.Interface) error {
@@ -798,6 +827,17 @@ func iptablesInit(ipt iptables.Interface) error {
 		return err
 	}
 
+	// Create a chain intended to explicitly allow non-local NodePort
+	// traffic to work around default-deny iptables configurations
+	// that would otherwise reject such traffic.
+	args = []string{"-m", "comment", "--comment", "Ensure that non-local NodePort traffic can flow"}
+	if _, err := ipt.EnsureChain(iptables.TableFilter, iptablesNonLocalNodePortChain); err != nil {
+		return err
+	}
+	if _, err := ipt.EnsureRule(iptables.Prepend, iptables.TableFilter, iptables.ChainInput, append(args, "-j", string(iptablesNonLocalNodePortChain))...); err != nil {
+		return err
+	}
+
 	// TODO: Verify order of rules.
 	return nil
 }
@@ -817,6 +857,9 @@ func iptablesFlush(ipt iptables.Interface) error {
 	if err := ipt.FlushChain(iptables.TableNAT, iptablesHostNodePortChain); err != nil {
 		el = append(el, err)
 	}
+	if err := ipt.FlushChain(iptables.TableFilter, iptablesNonLocalNodePortChain); err != nil {
+		el = append(el, err)
+	}
 	if len(el) != 0 {
 		glog.Errorf("Some errors flushing old iptables portals: %v", el)
 	}
@@ -974,6 +1017,13 @@ func (proxier *Proxier) iptablesHostNodePortArgs(nodePort int, protocol api.Prot
 	return args
 }
 
+// Build a slice of iptables args for an from-non-local public-port rule.
+func (proxier *Proxier) iptablesNonLocalNodePortArgs(nodePort int, protocol api.Protocol, proxyIP net.IP, proxyPort int, service proxy.ServicePortName) []string {
+	args := iptablesCommonPortalArgs(nil, false, false, proxyPort, protocol, service)
+	args = append(args, "-m", "comment", "--comment", service.String(), "-m", "state", "--state", "NEW", "-j", "ACCEPT")
+	return args
+}
+
 func isTooManyFDsError(err error) bool {
 	return strings.Contains(err.Error(), "too many open files")
 }

pkg/util/iptables/iptables.go

@@ -84,7 +84,8 @@ const (
 type Table string
 
 const (
-	TableNAT Table = "nat"
+	TableNAT    Table = "nat"
+	TableFilter Table = "filter"
 )
 
 type Chain string
@@ -93,6 +94,7 @@ const (
 	ChainPostrouting Chain = "POSTROUTING"
 	ChainPrerouting  Chain = "PREROUTING"
 	ChainOutput      Chain = "OUTPUT"
+	ChainInput       Chain = "INPUT"
 )
 
 const (

test/e2e/service.go

@@ -356,6 +356,9 @@ var _ = Describe("Services", func() {
 		expectNoError(verifyServeHostnameServiceUp(c, ns, host, podNames2, svc2IP, servicePort))
 	})
 
+	// TODO: Run this test against the userspace proxy and nodes
+	// configured with a default deny firewall to validate that the
+	// proxy whitelists NodePort traffic.
 	It("should be able to create a functioning NodePort service", func() {
 		serviceName := "nodeportservice-test"
 		ns := f.Namespace.Name