whitelist traffic to cluster IP and node ports in INPUT chain to bypass netwrok policy enforcement #914

murali-reddy · 2020-05-27T11:59:08Z

Fixes #905

Following rules will be prepended in KUBE-ROUTER-INPUT chain. So traffic to cluster IP's and node port services are targetted to RETURN.

Chain KUBE-ROUTER-INPUT (1 references)
 pkts bytes target     prot opt in     out     source               destination         
    0     0 RETURN     tcp  --  *      *       0.0.0.0/0            0.0.0.0/0            /* allow LOCAL traffic to node ports */ ADDRTYPE match dst-type LOCAL multiport dports 30000:32767
    0     0 RETURN     udp  --  *      *       0.0.0.0/0            0.0.0.0/0            /* allow LOCAL traffic to node ports */ ADDRTYPE match dst-type LOCAL multiport dports 30000:32767
   15  1464 RETURN     all  --  *      *       0.0.0.0/0            10.96.0.0/12         /* allow traffic to cluster IP */
    0     0 KUBE-POD-FW-VTNZYDWW3NHVYEUD  all  --  *      *       10.1.1.4             0.0.0.0/0            /* rule to jump traffic from POD name:frontend-56fc5b6b47-dvksf namespace: default to chain KUBE-POD-FW-VTNZYDWW3NHVYEUD */

Note: there is no dependency/assumptions on service proxy. Service proxy could add its filtering rules to filter traffic to services. Below is e.g. in case where kube-router is acting as service proxy and has setup filter rules to restrict traffic to service VIP's.

# iptables -t filter -L INPUT -n -v
Chain INPUT (policy ACCEPT 1483 packets, 419K bytes)
 pkts bytes target     prot opt in     out     source               destination         
 265K   75M KUBE-ROUTER-INPUT  all  --  *      *       0.0.0.0/0            0.0.0.0/0            /* kube-router netpol */
 265K   75M KUBE-ROUTER-SERVICES  all  --  *      *       0.0.0.0/0            0.0.0.0/0            /* handle traffic to IPVS service IPs in custom chain */ match-set kube-router-service-ips dst

netwrok policy enforcement Fixes #905

…all rules

murali-reddy · 2020-06-09T11:06:01Z

This PR has been tested through following test cases:

Four services are used: service A, service B, service C and service D. Service B and C are of type NodePort and service D is running in host network. Network policies are appliced such that pods in service A can only access service B.

pod A corresponding to service A should be able to access pod B in service B (in both cases where pod B is on the same node and different node) directly
pod A corresponding to service A should be able to access cluster IP of service B and get load balanced to one of the pods in service B
pod A corresponding to service A should be able to access NodePort of service B and get load balanced to one of the pods in service B
pod A corresponding to service A should be able to access cluster IP of service C but load balanced traffic to one of the pods in service C should be dropped
pod A corresponding to service A should be able to access NodePort of service C but load balanced traffic to one of the pods in service C should be dropped
pod A should NOT be able to access the host network service D
verified following order of chains is ensured on restart of kube-router pods and periodic syncs

# iptables -t filter -L INPUT -n -v
Chain INPUT (policy ACCEPT 1483 packets, 419K bytes)
 pkts bytes target     prot opt in     out     source               destination         
 265K   75M KUBE-ROUTER-INPUT  all  --  *      *       0.0.0.0/0            0.0.0.0/0            /* kube-router netpol */
 265K   75M KUBE-ROUTER-SERVICES  all  --  *      *       0.0.0.0/0            0.0.0.0/0            /* handle traffic to IPVS service IPs in custom chain */ match-set kube-router-service-ips dst

verified firewall rules configured for kube-router IPVS service proxy are honoured

Merging the PR based on the testing done.

aauren · 2020-06-13T02:41:17Z

pkg/controllers/netpol/network_policy_controller.go

-		args := []string{"-m", "comment", "--comment", "kube-router netpol", "-j", customChain}
-		exists, err := iptablesCmdHandler.Exists("filter", builtinChain, args...)
+
+		exists, err := iptablesCmdHandler.Exists("filter", chain, ruleSpec...)


Really minor nitpic, but we should probably do this existence checking before the List call above. If the iptables rule doesn't exist, then there is no reason to do the above list and it would save us an iptables call.

Agree. Will fix it.

aauren · 2020-06-13T02:43:01Z

pkg/controllers/netpol/network_policy_controller.go

-				if err != nil {
-					glog.Fatalf("Failed to delete wrong rule to jump to chain %s in %s chain due to %s", customChain, builtinChain, err.Error())
-				}
+			err = iptablesCmdHandler.Delete("filter", chain, strconv.Itoa(ruleNo+1))


I know that you've talked about this other places in the code, but at some point in the future we should really not rely on deleting iptables rules by number (I know we do it all over the place). But there exists a possibility that since we've listed the rules, that some other process has come in and manipulated iptables causing us to potentially delete the wrong rule.

Yes. There is a possibility. Perhaps holding the iptables lock followed by iptables-save and iptables-restore would be a safe solution. But if there are non-cooperating processes which does not use locks we just have no way to control it with iptables. Such races has been problem:

weaveworks/weave#3230
kubernetes/kubernetes#20391

aauren · 2020-06-13T02:43:42Z

pkg/options/options.go

@@ -74,6 +76,8 @@ func NewKubeRouterConfig() *KubeRouterConfig {
 		BGPGracefulRestartDeferralTime: 360 * time.Second,
 		EnableOverlay:                  true,
 		OverlayType:                    "subnet",
+		ClusterIPCIDR:                  "10.96.0.0/12",


Is this a safe guess for all of the user's that use our daemonset's that we publish?

Its default value kubernetes uses.

aauren · 2020-06-13T02:53:12Z

I know my review comes late. Overall, LGTM.

For posterity, as we discussed offline earlier, this will affect the ability to route to ExternalIPs or LoadBalancer type services from within the cluster. Granted, I don't think this was the intention of Kubernetes for inter-cluster traffic (i.e. it should be using a ClusterIP), but it does have the potential to affect users (I know that our current users use ExternalIPs for inter-cluster traffic routing because that's what they get back from Consul).

Other than that, I had one nit and one question, but neither are pressing.

murali-reddy · 2020-06-15T11:24:42Z

@aauren thanks for the review

For posterity, as we discussed offline earlier, this will affect the ability to route to ExternalIPs or LoadBalancer type services from within the cluster. Granted, I don't think this was the intention of Kubernetes for inter-cluster traffic (i.e. it should be using a ClusterIP), but it does have the potential to affect users (I know that our current users use ExternalIPs for inter-cluster traffic routing because that's what they get back from Consul).

Please open an issue for the use-case. We should discuss and see how to evolve it.

aauren · 2020-06-16T01:02:19Z

@aauren thanks for the review

For posterity, as we discussed offline earlier, this will affect the ability to route to ExternalIPs or LoadBalancer type services from within the cluster. Granted, I don't think this was the intention of Kubernetes for inter-cluster traffic (i.e. it should be using a ClusterIP), but it does have the potential to affect users (I know that our current users use ExternalIPs for inter-cluster traffic routing because that's what they get back from Consul).

Please open an issue for the use-case. We should discuss and see how to evolve it.

Added here: #934

…ss netwrok policy enforcement (cloudnativelabs#914) * whitelist traffic to cluster IP and node ports in INPUT chain to bypass netwrok policy enforcement Fixes cloudnativelabs#905 * fix unit test failure * ensure netpol firewall rules are configured after service proxy firewall rules

murali-reddy force-pushed the issue-905 branch from e5e0330 to e5be745 Compare May 27, 2020 12:10

murali-reddy mentioned this pull request May 28, 2020

fix(network_policy): consolidate policy chains #906

Closed

murali-reddy force-pushed the npc-refactor branch from 97b022f to e5a9900 Compare June 5, 2020 05:48

murali-reddy force-pushed the issue-905 branch from 6613f82 to 5df5267 Compare June 5, 2020 09:04

murali-reddy changed the base branch from npc-refactor to master June 8, 2020 17:28

murali-reddy added 2 commits June 8, 2020 23:00

whitelist traffic to cluster IP and node ports in INPUT chain to bypass

f285ea3

netwrok policy enforcement Fixes #905

fix unit test failure

49bdbf5

murali-reddy force-pushed the issue-905 branch from 5df5267 to 49bdbf5 Compare June 8, 2020 17:51

ensure netpol firewall rules are configured after service proxy firew…

a97b8cb

…all rules

murali-reddy merged commit 4d6b0b8 into master Jun 9, 2020

aauren reviewed Jun 13, 2020

View reviewed changes

murali-reddy mentioned this pull request Jun 15, 2020

avoide listing a chain if the rule already exists #931

Merged

aauren mentioned this pull request Jun 16, 2020

ExternalIPs and LoadBalancer IPs No Longer Work with NetworkPolicy without Manual Specification #934

Closed

aauren deleted the issue-905 branch September 4, 2023 15:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

whitelist traffic to cluster IP and node ports in INPUT chain to bypass netwrok policy enforcement #914

whitelist traffic to cluster IP and node ports in INPUT chain to bypass netwrok policy enforcement #914

murali-reddy commented May 27, 2020 •

edited

Loading

murali-reddy commented Jun 9, 2020 •

edited

Loading

aauren Jun 13, 2020

murali-reddy Jun 15, 2020

aauren Jun 13, 2020

murali-reddy Jun 15, 2020

aauren Jun 13, 2020

murali-reddy Jun 15, 2020

aauren commented Jun 13, 2020

murali-reddy commented Jun 15, 2020

aauren commented Jun 16, 2020

whitelist traffic to cluster IP and node ports in INPUT chain to bypass netwrok policy enforcement #914

whitelist traffic to cluster IP and node ports in INPUT chain to bypass netwrok policy enforcement #914

Conversation

murali-reddy commented May 27, 2020 • edited Loading

murali-reddy commented Jun 9, 2020 • edited Loading

aauren Jun 13, 2020

Choose a reason for hiding this comment

murali-reddy Jun 15, 2020

Choose a reason for hiding this comment

aauren Jun 13, 2020

Choose a reason for hiding this comment

murali-reddy Jun 15, 2020

Choose a reason for hiding this comment

aauren Jun 13, 2020

Choose a reason for hiding this comment

murali-reddy Jun 15, 2020

Choose a reason for hiding this comment

aauren commented Jun 13, 2020

murali-reddy commented Jun 15, 2020

aauren commented Jun 16, 2020

murali-reddy commented May 27, 2020 •

edited

Loading

murali-reddy commented Jun 9, 2020 •

edited

Loading