Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
yndai opened this issue Apr 9, 2024 · 2 comments
Open

Comments

@yndai
Copy link

yndai commented Apr 9, 2024

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
Copy link
Contributor

haouc commented Apr 26, 2024

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
Copy link
Author

yndai commented May 2, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants