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

IP based netpols can't select in cluster resources with Cilium CNI #871

Closed
jacobbmay opened this issue Oct 7, 2024 · 2 comments · Fixed by #884
Closed

IP based netpols can't select in cluster resources with Cilium CNI #871

jacobbmay opened this issue Oct 7, 2024 · 2 comments · Fixed by #884
Assignees
Labels
bug Something isn't working

Comments

@jacobbmay
Copy link
Contributor

Environment

Device and OS: RHEL 8
App version: 0.28.0-registry1
Kubernetes distro being used: eks-d
Other: Using Cilium for CNI

Steps to reproduce

  1. Deploy uds core to cluster using Cilium for CNI
  2. Attempt to log into Grafana using SSO

Expected result

User is successfully logged into Grafana using keycloak

Actual Result

Grafana throws error trying to get a token from Keycloak

Visual Proof (screenshots, videos, text, etc)

logger=authn.service t=2024-10-07T17:41:18.145089994Z level=error msg="Failed to authenticate request" client=auth.client.generic_oauth error="[auth.oauth.token.exchange] failed to exchange code to token: Post \"http://keycloak-http.keycloak.svc.cluster.local:8080/realms/uds/protocol/openid-connect/token\": context deadline exceeded (Client.Timeout exceeded while awaiting headers)"

Severity/Priority

High

Cause of the issue identified

Cilium can not select in cluster resources using CIDR based selectors in netpols. Currently the allow-keycloak-ingress-keycloak-backchannel-access netpol is using a cidr based policy to match 0.0.0.0/0, which means Grafana (and any other apps that attempt to access keycloak directly via its internal service hostname) will timeout when running in an environment using Cilium for CNI.

This should be updated to include a namespace selector that matches all namespaces instead of or in addition to the 0.0.0.0/0 policy. Have confirmed that adding - namespaceSelector: {} to the from selector resolves the issue.

Additional context

This issue will affect anything relying on network policies that use an IP selector to allow ingress or egress to in cluster resources, not just keycloak backchannel access. So there may be other netpols that should be updated in the same way. The issue was just identified with Grafana SSO.

@jacobbmay jacobbmay added the possible-bug Something may not be working label Oct 7, 2024
@mjnagel
Copy link
Contributor

mjnagel commented Oct 7, 2024

I think our intent with that specific policy was to only apply to in-cluster resources so making the swap (removing Anywhere and switching to namespaceSelector: {}) makes sense to me.

We might also/alternatively want to modify our anywhere endpoint to have both the CIDR and namespaceSelector: {} which would nicely handle all the other cases you mentioned that could be floating out there but are unknown.

@jacobbmay
Copy link
Contributor Author

I think updating the anywhere endpoint to have both makes sense. Would resolve this issue for Cilium users and still allow traffic from anywhere external the same way that it currently does.

That specific policy does seem like it should only allow access in cluster though.

@mjnagel mjnagel self-assigned this Oct 8, 2024
@mjnagel mjnagel added bug Something isn't working and removed possible-bug Something may not be working labels Oct 8, 2024
mjnagel added a commit that referenced this issue Oct 10, 2024
## Description

Reference [this
doc](https://github.com/cilium/cilium/blob/v1.16.2/Documentation/network/kubernetes/policy.rst#networkpolicy)
for the limitations of Cilium with `ipBlock` based netpols.

Two changes included to support this behavior:
- Modifies the keycloak backchannel policy to include all namespaces
instead of using the `Anywhere` generated target. This was intended to
be anywhere in cluster anyways (see the deleted TODO comment in the
diff).
- Modifies `Anywhere` target to include both the `0.0.0.0/0` CIDR and an
empty namespace selector. For any non-Cilium CNIs `0.0.0.0/0` would've
already covered any in-cluster endpoints, so this only changes the
behavior for Cilium.

## Related Issue

Fixes #871

## Type of change

- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Other (security config, docs update, etc)

## Checklist before merging

- [x] Test, docs, adr added or updated as needed
- [x] [Contributor
Guide](https://github.com/defenseunicorns/uds-template-capability/blob/main/CONTRIBUTING.md)
followed
docandrew pushed a commit that referenced this issue Oct 17, 2024
## Description

Reference [this
doc](https://github.com/cilium/cilium/blob/v1.16.2/Documentation/network/kubernetes/policy.rst#networkpolicy)
for the limitations of Cilium with `ipBlock` based netpols.

Two changes included to support this behavior:
- Modifies the keycloak backchannel policy to include all namespaces
instead of using the `Anywhere` generated target. This was intended to
be anywhere in cluster anyways (see the deleted TODO comment in the
diff).
- Modifies `Anywhere` target to include both the `0.0.0.0/0` CIDR and an
empty namespace selector. For any non-Cilium CNIs `0.0.0.0/0` would've
already covered any in-cluster endpoints, so this only changes the
behavior for Cilium.

## Related Issue

Fixes #871

## Type of change

- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Other (security config, docs update, etc)

## Checklist before merging

- [x] Test, docs, adr added or updated as needed
- [x] [Contributor
Guide](https://github.com/defenseunicorns/uds-template-capability/blob/main/CONTRIBUTING.md)
followed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants