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

Policy with k8sService on port 443 allows traffic on port 443 to world #20067

Closed
2 tasks done
arjantop-cai opened this issue Jun 2, 2022 · 7 comments
Closed
2 tasks done
Labels
kind/question Frequently asked questions & answers. This issue will be linked from the documentation's FAQ. sig/agent Cilium agent related. sig/policy Impacts whether traffic is allowed or denied based on user-defined policies.
Milestone

Comments

@arjantop-cai
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

What happened?

Policy defined:

apiVersion: cilium.io/v2
kind: CiliumNetworkPolicy
metadata:
  name: my-policy
spec:
  endpointSelector: {}
  ingress:
    - {}
  egress:
    - toServices:
        - k8sService:
            namespace: ns
            serviceName: svc
      toPorts:
        - ports:
            - port: "443"
              protocol: TCP

Expected: Only traffic to service svc.ns on port 443 is allowed
Result: All traffic to "world" on port 443 is allowed

Observed in local k3d install and EKS

Cilium Version

1.10.x and 1.11.x

Kernel Version

Linux hostname 5.4.188-104.359.amzn2.x86_64 #1 SMP Thu Apr 14 20:53:13 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux

Kubernetes Version

v1.21.5-eks-9017834

Sysdump

No response

Relevant log output

No response

Anything else?

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct
@arjantop-cai arjantop-cai added kind/bug This is a bug in the Cilium logic. needs/triage This issue requires triaging to establish severity and next steps. labels Jun 2, 2022
@arjantop-cai
Copy link
Author

Reproduction steps using sw-app example:

kubectl create -ntest -f https://github.com/cilium/cilium/blob/master/examples/minikube/http-sw-app.yaml

Requests to world:80 and world:443 work:

kubectl -ntest exec -it tiefighter -- curl "http://google.com"
kubectl -ntest exec -it tiefighter -- curl "https://google.com"

Apply the policy:

apiVersion: "cilium.io/v2"
kind: CiliumNetworkPolicy
metadata:
  name: "rule1"
spec:
  endpointSelector:
    matchLabels:
      org: empire
      class: tiefighter
  ingress:
    - {}
  egress:
    - toServices:
        - k8sService:
            namespace: test
            serviceName: deathstar
      toPorts:
        - ports:
            - port: "80"
              protocol: TCP
    - toEndpoints:
        - matchLabels:
            io.kubernetes.pod.namespace: kube-system
            k8s-app: kube-dns
      toPorts:
        - ports:
            - port: "53"
              protocol: UDP
          rules:
            dns:
              - matchPattern: "*"

Request to world:80 works (the one specified in the policy toPorts) and traffic to world:443 is dropped (not specified in the policy)

kubectl -ntest exec -it tiefighter -- curl "http://google.com"
kubectl -ntest exec -it tiefighter -- curl "https://google.com"

@atykhyy
Copy link
Contributor

atykhyy commented Jun 6, 2022

I have observed the same behavior with a toServices pointing to kube-dns service. A toServices rule allowing egress to kube-dns service in fact allows requests on port 53 to any remote server. Specifying toEndpoints with a selector of k8s-app=kube-dns instead of toServices works as expected: only requests to kube-dns are allowed.

@aanm aanm added sig/policy Impacts whether traffic is allowed or denied based on user-defined policies. sig/agent Cilium agent related. and removed needs/triage This issue requires triaging to establish severity and next steps. labels Jun 9, 2022
@github-actions
Copy link

github-actions bot commented Aug 9, 2022

This issue has been automatically marked as stale because it has not
had recent activity. It will be closed if no further activity occurs.

@github-actions github-actions bot added the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label Aug 9, 2022
@aanm
Copy link
Member

aanm commented Aug 10, 2022

Maybe it's something that we should document as a limitation of k8sService

@aanm aanm added this to the 1.13 milestone Aug 10, 2022
@github-actions github-actions bot removed the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label Mar 9, 2023
@github-actions
Copy link

github-actions bot commented May 9, 2023

This issue has been automatically marked as stale because it has not
had recent activity. It will be closed if no further activity occurs.

@github-actions github-actions bot added the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label May 9, 2023
@github-actions
Copy link

This issue has not seen any activity since it was marked stale.
Closing.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale May 24, 2023
atykhyy added a commit to Apptane/cilium that referenced this issue Jun 23, 2023
PR cilium#21052 updated Cilium documentation to say that, in network policy
rules, `toServices` statements cannot be combined with `toPorts`
statements. I believe it would be more informative for Cilium users
to say (following RFC 2119) that `toServices` _must not_ be combined
with `toPorts`, as technically Cilium accepts such a network policy
as valid but handles it in the unexpected and potentially dangerous
(e.g. if a setup relies on Cilium network policy to implement egress
filtering) manner described in cilium#20067.

Signed-off-by: Anton Tykhyy <atykhyy@gmail.com>
atykhyy added a commit to Apptane/cilium that referenced this issue Jun 23, 2023
Currently, an egress rule combining toServices and toPorts has
the unexpected and potentially dangerous side effect of allowing egress
traffic to any remote endpoint on the port(s) specified (see cilium#20067).
This change makes such rules fail validation.

Signed-off-by: Anton Tykhyy <atykhyy@gmail.com>
ldelossa pushed a commit that referenced this issue Jun 26, 2023
PR #21052 updated Cilium documentation to say that, in network policy
rules, `toServices` statements cannot be combined with `toPorts`
statements. I believe it would be more informative for Cilium users
to say (following RFC 2119) that `toServices` _must not_ be combined
with `toPorts`, as technically Cilium accepts such a network policy
as valid but handles it in the unexpected and potentially dangerous
(e.g. if a setup relies on Cilium network policy to implement egress
filtering) manner described in #20067.

Signed-off-by: Anton Tykhyy <atykhyy@gmail.com>
ldelossa pushed a commit that referenced this issue Jun 26, 2023
Currently, an egress rule combining toServices and toPorts has
the unexpected and potentially dangerous side effect of allowing egress
traffic to any remote endpoint on the port(s) specified (see #20067).
This change makes such rules fail validation.

Signed-off-by: Anton Tykhyy <atykhyy@gmail.com>
AndiDog added a commit to giantswarm/cilium-app that referenced this issue Jan 22, 2024
…ause of breakage in Cilium v1.14

See cilium/cilium#20067 - the combination is not supported anymore and Cilium prints warnings about the non-effective policy.
AndiDog added a commit to giantswarm/cilium-app that referenced this issue Jan 22, 2024
…ause of breakage in Cilium v1.14

See cilium/cilium#20067 - the combination is not supported anymore and Cilium prints warnings about the non-effective policy.
AndiDog added a commit to giantswarm/cilium-app that referenced this issue Jan 22, 2024
…ause of breakage in Cilium v1.14 (#128)

See cilium/cilium#20067 - the combination is not supported anymore and Cilium prints warnings about the non-effective policy.
@gandro gandro reopened this Feb 26, 2024
@gandro gandro added kind/question Frequently asked questions & answers. This issue will be linked from the documentation's FAQ. and removed kind/bug This is a bug in the Cilium logic. stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. labels Feb 26, 2024
@gandro gandro closed this as completed Feb 26, 2024
@gandro gandro closed this as not planned Won't fix, can't repro, duplicate, stale Feb 26, 2024
@gandro
Copy link
Member

gandro commented Feb 26, 2024

Apologies for the noise, I discovered this issue as it is referenced in the code and the limitation still applies. To reflect that, I've relabeled it as a "question" since this is a known and (now) documented limitation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/question Frequently asked questions & answers. This issue will be linked from the documentation's FAQ. sig/agent Cilium agent related. sig/policy Impacts whether traffic is allowed or denied based on user-defined policies.
Projects
None yet
Development

No branches or pull requests

5 participants