k8snetworkplumbingwg / multi-networkpolicy-iptables

MultiNetworkPolicy iptable based implementation
Apache License 2.0
13 stars 19 forks source link

Not match multiple ingress policy #45

Closed mimuret closed 1 year ago

mimuret commented 1 year ago

Set multiple ingress rules, but only matches the last rule. For example, This policy only allows from 172.17.0.0/16.

apiVersion: k8s.cni.cncf.io/v1beta1
kind: MultiNetworkPolicy
metadata:
  name: test-network-policy
  namespace: default
  annotations:
    k8s.v1.cni.cncf.io/policy-for: macvlan-conf-1 
spec:
  policyTypes:
  - Ingress
  ingress:
  - from:
    - ipBlock:
        cidr: 172.16.0.0/16
  - from:
    - ipBlock:
        cidr: 172.17.0.0/16

Maybe need a return rule before the next rule. https://github.com/k8snetworkplumbingwg/multi-networkpolicy-iptables/blob/0c6df81c1bf67c0812be0be94b4932388586b4df/pkg/server/policyrules.go#L237

s1061123 commented 1 year ago

Thank you for issue report!

Multi-networkpolicy(and k8s NetworkPolicy as well) from can have multiple ipBlocks. So I suppose following config may satisfy your requirements. Could you please check that?

apiVersion: k8s.cni.cncf.io/v1beta1
kind: MultiNetworkPolicy
(snip)
spec:
  policyTypes:
  - Ingress
  ingress:
  - from:
    - ipBlock:
        cidr: 172.16.0.0/16
    - ipBlock:
        cidr: 172.17.0.0/16
s1061123 commented 1 year ago

I add two e2e testcases for ipblock cases and it seems to work, in kind environment so far. So iptables rules seems to be valid. (see below)

https://github.com/k8snetworkplumbingwg/multi-networkpolicy-iptables/blob/639a712cecee7d2f4abad1bdb3c0c55d10e4ebec/e2e/tests/ipblock-stacked.yml#L115-L160

https://github.com/k8snetworkplumbingwg/multi-networkpolicy-iptables/blob/639a712cecee7d2f4abad1bdb3c0c55d10e4ebec/e2e/tests/ipblock.yml#L134-L139

In addition, iptables tutorial describes

When/If we reach the end of that chain, we get dropped back to the INPUT chain and the packet starts traversing from the rule one step below where it jumped to the other chain (tcp_packets in this case).

Hence we don't need to add 'RETURN'.

mimuret commented 1 year ago

Thanks you for you response.

I think need "RETURN" multiple from rule.

for example,

apiVersion: k8s.cni.cncf.io/v1beta1
kind: MultiNetworkPolicy
(snip)
spec:
  policyTypes:
  - Ingress
  ingress:
  - from:
    - ipBlock:
        cidr: 172.16.0.0/16
    ports:
    # for exporter
    - port: 8080
      protocol: TCP
  - from:
    - ipBlock:
        cidr: 172.17.0.0/16
    ports:
    # for service port
    - port: 443
      protocol: TCP

Output iptables rules below. 10.138.140.81 is NIC ip.

If src ip is 172.16.0.1 and dest port 8080. Request matches MULTI-0-INGRESS-0-FROM and MULTI-0-INGRESS-0-PORTS. But mark is resetting before MULTI-0-INGRESS-1-PORTS. I think "RETURN" before reset mark.

(SNIP)
-A MULTI-0-INGRESS -j MARK --set-xmark 0x0/0x30000
-A MULTI-0-INGRESS -j MULTI-0-INGRESS-0-PORTS
-A MULTI-0-INGRESS -j MULTI-0-INGRESS-0-FROM
-A MULTI-0-INGRESS -j MARK --set-xmark 0x0/0x30000
-A MULTI-0-INGRESS -j MULTI-0-INGRESS-1-PORTS
-A MULTI-0-INGRESS -j MULTI-0-INGRESS-1-FROM
-A MULTI-0-INGRESS-0-FROM -s 172.16.0.0/16 -i net2 -j MARK --set-xmark 0x20000/0x20000
-A MULTI-0-INGRESS-0-FROM -s 10.138.140.81/32 -i net2 -j MARK --set-xmark 0x20000/0x20000
-A MULTI-0-INGRESS-0-PORTS -m comment --comment "no ingress ports, skipped" -j MARK --set-xmark 0x10000/0x10000
-A MULTI-0-INGRESS-1-FROM -s 172.17.0.0/16 -i net2 -j MARK --set-xmark 0x20000/0x20000
-A MULTI-0-INGRESS-1-FROM -s 10.138.140.81/32 -i net2 -j MARK --set-xmark 0x20000/0x20000
-A MULTI-0-INGRESS-1-PORTS -m comment --comment "no ingress ports, skipped" -j MARK --set-xmark 0x10000/0x10000
-A MULTI-INGRESS -j MULTI-INGRESS-COMMON
-A MULTI-INGRESS -i net2 -m comment --comment "hogehoge" -j MULTI-0-INGRESS
-A MULTI-INGRESS -m mark --mark 0x30000/0x30000 -j RETURN
-A MULTI-INGRESS -j DROP
-A MULTI-INGRESS-COMMON -p icmp -j ACCEPT
-A MULTI-INGRESS-COMMON -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT

workaround is split from resource level.

```yaml
apiVersion: k8s.cni.cncf.io/v1beta1
kind: MultiNetworkPolicy
(snip)
spec:
  policyTypes:
  - Ingress
  ingress:
  - from:
    - ipBlock:
        cidr: 172.16.0.0/16
    ports:
    # for exporter
    - port: 8080
      protocol: TCP
---
apiVersion: k8s.cni.cncf.io/v1beta1
kind: MultiNetworkPolicy
(snip)
spec:
  policyTypes:
  - Ingress
  ingress:
  - from:
    - ipBlock:
        cidr: 172.17.0.0/16
    ports:
    # for service port
    - port: 443
      protocol: TCP

It seems to be working fine.

(SNIP)
-A MULTI-0-INGRESS -j MARK --set-xmark 0x0/0x30000
-A MULTI-0-INGRESS -j MULTI-0-INGRESS-0-PORTS
-A MULTI-0-INGRESS -j MULTI-0-INGRESS-0-FROM
-A MULTI-0-INGRESS-0-FROM -s 172.16.0.0/16 -i net2 -j MARK --set-xmark 0x20000/0x20000
-A MULTI-0-INGRESS-0-FROM -s 10.138.140.81/32 -i net2 -j MARK --set-xmark 0x20000/0x20000
-A MULTI-0-INGRESS-0-PORTS -i net2 -p tcp -m tcp --dport 8080 -j MARK --set-xmark 0x10000/0x10000
-A MULTI-1-INGRESS -j MARK --set-xmark 0x0/0x30000
-A MULTI-1-INGRESS -j MULTI-1-INGRESS-0-PORTS
-A MULTI-1-INGRESS -j MULTI-1-INGRESS-0-FROM
-A MULTI-1-INGRESS-0-FROM -s 172.17.0.0/16 -i net2 -j MARK --set-xmark 0x20000/0x20000
-A MULTI-1-INGRESS-0-FROM -s 10.138.140.81/32 -i net2 -j MARK --set-xmark 0x20000/0x20000
-A MULTI-1-INGRESS-0-PORTS -i net2 -p tcp -m tcp --dport 443 -j MARK --set-xmark 0x10000/0x10000
-A MULTI-INGRESS -j MULTI-INGRESS-COMMON
-A MULTI-INGRESS -i net2 -m comment --comment "hogehoge" -j MULTI-0-INGRESS
-A MULTI-INGRESS -m mark --mark 0x30000/0x30000 -j RETURN
-A MULTI-INGRESS -j MULTI-INGRESS-COMMON
-A MULTI-INGRESS -i net2 -m comment --comment "hugahuga" -j MULTI-1-INGRESS
-A MULTI-INGRESS -m mark --mark 0x30000/0x30000 -j RETURN
-A MULTI-INGRESS -j DROP
(SNIP)
s1061123 commented 1 year ago

Got your point. Let me double check that.