aws / amazon-network-policy-controller-k8s

Apache License 2.0
38 stars 10 forks source link

Allow-all CIDR rule conflicts with more restrictive rule on the same CIDR #98

Open yndai opened 7 months ago

yndai commented 7 months ago

What happened:

Summary: When creating a NetworkPolicy with a rule that allows all traffic to a CIDR and another rule for the same CIDR, but with a port specified, the allow all rule is missing in the resulting PolicyEndpoint

Additional details:

Specifying an allow all rule without the more restrictive rule on the same CIDR works as expected. This seems to be related to some merging mechanism to collect all allowed ports/protocols on the same CIDR.

This use-case is important because there are 3rd party Helm charts with policies that are configured additively so we would have to override them entirely in order to allow all traffic on egress, for example.

According to the NetworkPolicy spec: https://kubernetes.io/docs/concepts/services-networking/network-policies The effects of those egress lists combine additively so this is unexpected behavior. We have had this working as expected in the past with Cilium network policies, as well.

How to reproduce it (as minimally and precisely as possible):

Example:

kubectl apply -f on:

apiVersion: networking.k8s.io/v1
kind: NetworkPolicy
metadata:
  name: allow-web-traffic-egress
spec:
  podSelector:
    matchLabels:
      app: target
  policyTypes:
  - Egress
  egress:
    - to:
      - ipBlock:
          cidr: 10.0.0.0/16
      ports:
        - protocol: TCP
          port: 443
    - to:
      - ipBlock:
          cidr: 10.0.0.0/16

We expect this in the resulting PolicyEndpoint:

apiVersion: networking.k8s.aws/v1alpha1
[...]
spec:
  egress:
  - cidr: 10.0.0.0/16
    ports:
    - port: 443
      protocol: TCP
  - cidr: 10.0.0.0/16

but we get:

apiVersion: networking.k8s.aws/v1alpha1
[...]
spec:
  egress:
  - cidr: 10.0.0.0/16
    ports:
    - port: 443
      protocol: TCP

which is more restrictive

Environment:

Kubernetes version (use kubectl version): Server Version: version.Info{Major:"1", Minor:"27+", GitVersion:"v1.27.10-eks-508b6b3", GitCommit:"e99f7c75641f738090d483d988dc4a70001e01cf", GitTreeState:"clean", BuildDate:"2024-01-29T20:59:05Z", GoVersion:"go1.20.13", Compiler:"gc", Platform:"linux/amd64"} CNI Version: amazon-k8s-cni:v1.16.0-eksbuild.1 Network Policy Agent Version: aws-network-policy-agent:v1.0.7-eksbuild.1 OS (e.g: cat /etc/os-release): Amazon Linux 2 Kernel (e.g. uname -a): Linux ip-10-1-61-179.ec2.internal 5.10.192-183.736.amzn2.x86_64 aws/aws-network-policy-agent#1 SMP Wed Sep 6 21:15:41 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux

haouc commented 7 months ago

Hey @yndai, sorry for late update. We have discussed on this behavior and didn't see a good reason to support all ports if no ports are defined in this case. Is this also supposed to support some static IP ranges in your use case without explicitly targeting specific ports?

yndai commented 7 months ago

Hi,

didn't see a good reason to support all ports if no ports are defined in this case

I would say that a good reason is that the NetworkPolicy doc/spec (as I linked in the initial statement) states that:

[...]
The effects of those egress lists combine additively
[...]
Network policies do not conflict; they are additive. If any policy or policies apply to a given pod for a given direction, the connections allowed in that direction from that pod is the union of what the applicable policies allow. Thus, order of evaluation does not affect the policy result.

In other words, having a policy rule allowing egress to 10.0.0.0/16 should allow egress to 10.0.0.0/16 regardless of what other policy rules restrict on the 10.0.0.0/16 IP block. This assumption is made by some public Helm charts, for example, and can result in unexpected behavior.

support some static IP ranges in your use case without explicitly targeting specific ports

I think I need some clarification on what you mean here, but an IP block rule without specifying ports is already currently supported from what I can tell:

apiVersion: networking.k8s.io/v1
kind: NetworkPolicy
[...]
  egress:
    - to:
      - ipBlock:
          cidr: 10.0.0.0/16

Results in PolicyEndpoint of

spec:
  egress:
  - cidr: 10.0.0.0/16

Again, I am mainly talking about the case where two policy rules acting on the same IP block (one of which allows all ports) conflict with each other when the NetworkPolicy spec states that they should not.