ovn-org / ovn-kubernetes

A robust Kubernetes networking platform
https://ovn-kubernetes.io/
Apache License 2.0
777 stars 334 forks source link

ACLs using wrong IP + Mask in network policy #1660

Open trozet opened 3 years ago

trozet commented 3 years ago

We can see in the ovn-controller logs:

2020-09-02T21:07:32.482555025Z stdout F 2020-09-02T21:07:32.478Z|00195|lflow|WARN|error parsing match "((ct.new && !ct.est) || (!ct.new && ct.est && !ct.rpl && ct_label.blocked == 1)) && (ip4.dst == {10.244.2.81/24} && inport == @a6662893649192231225)": Value contains unmasked 1-bits.

@dceara pointed to a feature to handle this for a user in OVN: https://github.com/ovn-org/ovn/commit/2104f67aacd62f62a31f4e23a6720aeeaa751154

However we should fix this in ovn-k8s ourselves

trozet commented 3 years ago

I0902 21:07:32.329191 54 policy.go:668] Network policy egress is {Ports:[] To:[{PodSelector:nil NamespaceSelector:nil IPBlock:&IPBlock{CIDR:10.244.2.81/24,Except:[10.244.2.81/32],}}]}

trozet commented 3 years ago

I0902 21:07:32.368492 54 ovs.go:162] exec(2094): /usr/bin/ovn-nbctl --timeout=15 --id=@acl create acl priority=1001 direction=to-lport match="ip4.dst == {10.244.2.81/24} && inport == @a6662893649192231225" action=allow external-ids:l4Match="None" external-ids:ipblock_cidr=true external-ids:namespace=network-policy-2143 external-ids:policy=deny-client-a-via-except-cidr-egress-rule external-ids:Egress_num=1 external-ids:policy_type=Egress -- add port_group 43e0a602-548a-4d21-aea3-487e5c5a8d78 acls @acl

danwinship commented 3 years ago

@dceara pointed to a feature to handle this for a user in OVN:

I disagree with "handling" it; if the user says to block access to 10.244.2.81/24, you don't know whether they really meant 10.244.2.0/24 or 10.244.2.81/32, and it's better to treat it as an error than to silently end up doing the wrong thing

trozet commented 3 years ago

@danwinship exactly, i'm trying to figure that out now

danwinship commented 3 years ago

one source of these just got fixed in the networkpolicy e2e tests upstream: https://github.com/kubernetes/kubernetes/pull/93583#pullrequestreview-458806141