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

NetworkPolicy tests for blocking north/south traffic #114369

Open
danwinship opened this issue Dec 8, 2022 · 21 comments
Open

NetworkPolicy tests for blocking north/south traffic #114369

danwinship opened this issue Dec 8, 2022 · 21 comments
Assignees
Labels
area/network-policy Issues or PRs related to Network Policy subproject good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. priority/backlog Higher priority than priority/awaiting-more-evidence. sig/network Categorizes an issue or PR as relevant to SIG Network.

Comments

@danwinship
Copy link
Contributor

The NP docs point out that NP's semantics for north/south traffic are not very clearly defined:

Cluster ingress and egress mechanisms often require rewriting the source or destination IP of packets. In cases where this happens, it is not defined whether this happens before or after NetworkPolicy processing, and the behavior may be different for different combinations of network plugin, cloud provider, Service implementation, etc.

In the case of ingress, this means that in some cases you may be able to filter incoming packets based on the actual original source IP, while in other cases, the "source IP" that the NetworkPolicy acts on may be the IP of a LoadBalancer or of the Pod's node, etc.

For egress, this means that connections from pods to Service IPs that get rewritten to cluster-external IPs may or may not be subject to ipBlock-based policies.

However, what's not ambiguous is that a pod which is fully isolated for ingress should not accept E/W or N/S traffic. Regardless of how the ingress/LB/cloud handles the traffic, it should end up getting blocked. (Well, unless it gets masqueraded to the pod's node IP, because then it hits that exception to the rules.)

Unfortunately we can't say the same for egress; I think we assume that a pod-to-service-IP connection will be allowed to reach kube-proxy even if the pod is fully isolated-for-egress, but we explicitly don't require that ipBlock policies get applied after service proxying.

Anyway, we should be able to add a test to test/e2e/network/netpol/network_policy.go that confirms that cluster-ingress traffic to a fully isolated-for-ingress pod is not allowed. In particular, if we create a LoadBalancer Service with externalTrafficPolicy: Local, and a NetworkPolicy blocking all ingress to that Service's Pods, then we should not be able to connect to the service via either the LoadBalancer IP/name or via its NodePort (on one of the correct nodes).

/sig network
/area network-policy
/priority backlog
/help
/good-first-issue

@k8s-ci-robot
Copy link
Contributor

@danwinship:
This request has been marked as suitable for new contributors.

Guidelines

Please ensure that the issue body includes answers to the following questions:

  • Why are we solving this issue?
  • To address this issue, are there any code changes? If there are code changes, what needs to be done in the code and what places can the assignee treat as reference points?
  • Does this issue have zero to low barrier of entry?
  • How can the assignee reach out to you for help?

For more details on the requirements of such an issue, please see here and ensure that they are met.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-good-first-issue command.

In response to this:

The NP docs point out that NP's semantics for north/south traffic are not very clearly defined:

Cluster ingress and egress mechanisms often require rewriting the source or destination IP of packets. In cases where this happens, it is not defined whether this happens before or after NetworkPolicy processing, and the behavior may be different for different combinations of network plugin, cloud provider, Service implementation, etc.

In the case of ingress, this means that in some cases you may be able to filter incoming packets based on the actual original source IP, while in other cases, the "source IP" that the NetworkPolicy acts on may be the IP of a LoadBalancer or of the Pod's node, etc.

For egress, this means that connections from pods to Service IPs that get rewritten to cluster-external IPs may or may not be subject to ipBlock-based policies.

However, what's not ambiguous is that a pod which is fully isolated for ingress should not accept E/W or N/S traffic. Regardless of how the ingress/LB/cloud handles the traffic, it should end up getting blocked. (Well, unless it gets masqueraded to the pod's node IP, because then it hits that exception to the rules.)

Unfortunately we can't say the same for egress; I think we assume that a pod-to-service-IP connection will be allowed to reach kube-proxy even if the pod is fully isolated-for-egress, but we explicitly don't require that ipBlock policies get applied after service proxying.

Anyway, we should be able to add a test to test/e2e/network/netpol/network_policy.go that confirms that cluster-ingress traffic to a fully isolated-for-ingress pod is not allowed. In particular, if we create a LoadBalancer Service with externalTrafficPolicy: Local, and a NetworkPolicy blocking all ingress to that Service's Pods, then we should not be able to connect to the service via either the LoadBalancer IP/name or via its NodePort (on one of the correct nodes).

/sig network
/area network-policy
/priority backlog
/help
/good-first-issue

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added sig/network Categorizes an issue or PR as relevant to SIG Network. good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. area/network-policy Issues or PRs related to Network Policy subproject priority/backlog Higher priority than priority/awaiting-more-evidence. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Dec 8, 2022
@k8s-ci-robot
Copy link
Contributor

@danwinship: This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@thockin thockin added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Dec 8, 2022
@MohnishMishra
Copy link

Hello, I am Mohnish Mishra. I am a 2nd year student in BTech in Computer Science and Engineering.
I am new to open-source coding and trying understand the same.
I am interested in contributing to this project.
If you could please guide me on what is to be done and what are the pre-requisites for the same, it will be of great help.

@sftim
Copy link
Contributor

sftim commented Dec 29, 2022

@danwinship this feels to me like an issue that (right now) justifies the help label, but maybe not good-first-issue. WDYT?

@astoycos
Copy link
Contributor

astoycos commented Jan 2, 2023

This has been on my TODO list, and I'm hoping to either get to it early in the new year or have a member of the network policy api subgroup tackle this. @MohnishMishra Feel free to come to that meeting to learn more :) [Note we are changing the time this year see this pr and slack conversation for more information]

But yeah @sftim I'd say this is slightly above good first issue :)

@khushi-ftw
Copy link

Hey. If this is not being actively worked on, I would like to pick this up.

@MohnishMishra
Copy link

Thank you @astoycos.
I have joined the mailing group and hope to join the meetings and learn.
Thank you for the opportunity.

@aayush1205
Copy link

If there is no active development here, I'd like to take it up!

@nag-geek
Copy link

Hy, I'm Nag, im new to this, i'd like to contribute to this open source i don't know from where to start. could you help me guys.

@astoycos
Copy link
Contributor

astoycos commented Jan 17, 2023

/assign @daman1807

who has been very active in the KPNG community and is going to pick this up :) thanks!

@k8s-ci-robot
Copy link
Contributor

@astoycos: GitHub didn't allow me to assign the following users: daman1807.

Note that only kubernetes members with read permissions, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time.
For more information please see the contributor guide

In response to this:

/assign @daman1807

who has been very active in the KPNG community and it going to pick this up :) thanks!

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@aroradaman
Copy link
Member

/assign

@DaviMatheus
Copy link

/assign

@jayunit100
Copy link
Member

@Daman if you use metallb you can directly implement 9 services, for the 9 pods in the reachability matrix, and then poll them all using dans idea

@jayunit100
Copy link
Member

jayunit100 commented Jan 18, 2023

looking at this test w/ daman, it seems like an interesting test, but it seems extremely dependent on the infrastructure provider ... i wonder if maybe we should define it in some way as a "networking integration" test, and run it on specific setups - i.e. maybe where the e2e thing is running outside of the cluster, and so on.... ? (https://github.com/kubernetes/test-infra). .... or maybe im overthinking it....

@astoycos astoycos removed their assignment Feb 2, 2023
@aroradaman aroradaman removed their assignment Feb 11, 2023
@danwinship
Copy link
Contributor Author

looking at this test w/ daman, it seems like an interesting test, but it seems extremely dependent on the infrastructure provider

How so? If both LoadBalancers and NetworkPolicy are available, then it must be the case that LoadBalancer traffic is blocked when there is a NetworkPolicy blocking all ingress. (Unless the LoadBalancer ingress path ends up SNATting traffic to the pod's node IP, hence the use of externalTrafficPolicy: Local to ensure there is no SNAT.)

@jayunit100
Copy link
Member

jayunit100 commented Feb 15, 2023

how so

If u try to access an external service of type LB from a node... the kube proxy will iirc actually not go through the LB and cheat going straight to the pod...

In our e2es since usually we run the test from a pod ... the kube proxy impl therefore I think maybe is the thing being tested just as much as the netpol impl

But I guess we could add these tests. In general I guess I'd wonder what the "right" way to run them is: would users wanna run em outside the cluster to test the realistic data path that flows from service LB into the pool?

@danwinship
Copy link
Contributor Author

The LoadBalancer e2e tests, in general, connect from the host running e2e.test, not from a host inside the cluster, since, as you note, otherwise we would not actually be testing the load balancers.

@ashutosh887
Copy link

let me work on this issue now please
/assign

@jayunit100
Copy link
Member

Ok....this feels like a loadbalancer e2e more then a netpol one but either way works .... let's just make sure we log that it must be run from outside a pod to be meaningful

@k8s-triage-robot
Copy link

This issue has not been updated in over 1 year, and should be re-triaged.

You can:

  • Confirm that this issue is still relevant with /triage accepted (org members only)
  • Close this issue with /close

For more details on the triage process, see https://www.kubernetes.dev/docs/guide/issue-triage/

/remove-triage accepted

@k8s-ci-robot k8s-ci-robot added needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. and removed triage/accepted Indicates an issue or PR is ready to be actively worked on. labels Mar 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/network-policy Issues or PRs related to Network Policy subproject good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. priority/backlog Higher priority than priority/awaiting-more-evidence. sig/network Categorizes an issue or PR as relevant to SIG Network.
Projects
None yet
Development

Successfully merging a pull request may close this issue.