feat(network): enable internal ingress network policy #208

andrewazores · 2024-11-13T17:12:46Z

Related to #205

To test on OpenShift:

helm install --set core.route.enabled=true --set authentication.openshift.enabled=true cryostat ./charts/cryostat
Wait for the deployment to become ready.
oc run runner --image=registry.access.redhat.com/ubi8/ubi --rm -it /bin/bash. This runs an additional Pod in the same installation namespace, but one which is not part of Cryostat and does not have the same selector labels. Once this comes up and you have a shell, curl -v http://cryostat:8181 or curl -v http://cryostat-storage:8333. These should time out.
Open Route (echo https://$(oc get route -n cryostat cryostat -o jsonpath="{.status.ingress[0].host}")) and ensure Cryostat UI behaves as usual. Create a localhost:0 custom target, start and archive a recording, etc.
helm uninstall cryostat
helm install --set core.route.enabled=true --set authentication.openshift.enabled=true --set networkPolicy.ingress.enabled=false cryostat ./charts/cryostat
Wait for the deployment to become ready.
oc run runner --image=registry.access.redhat.com/ubi8/ubi --rm -it /bin/bash. Once this comes up and you have a shell, curl -v http://cryostat:8181 or curl -v http://cryostat-storage:8333. These should succeed and return HTTP responses quickly.
Repeat the test from step 4 to ensure Cryostat is working as expected from the user's POV.

Similar testing on other types of cluster (kind, minikube) should also work, with the usual adjustments (don't use core.route.enabled or authentication.openshift.enabled, etc.).

I also spent some time trying to define Egress policies. I thought this would be interesting along the same lines as cryostatio/cryostat-agent#242 and cryostatio/cryostat#323 - we could add network-level restrictions (firewall rules) to prevent Cryostat from being made to open network connections to unexpected destinations, ie. namespaces outside of the admin's chosen list of target namespaces. However, I ran into issues where this interrupted Cryostat's ability to connect to its own database deployment, and I was also not completely sure the right approach to allow Cryostat and the openshift-oauth-proxy to have traffic egress to the k8s API server (for doing RBAC checks, or Endpoints discovery). I'm holding off on that idea for now, but it may be worth following up with again later.

ebaron

When testing on OpenShift, I seem to be getting 503 errors when trying to access Cryostat through its Route. When the Network Policy is disabled, the route works fine. Not sure why

tthvo · 2024-11-22T03:40:47Z

charts/cryostat/templates/networkpolicy_ingress.yaml

+kind: NetworkPolicy
+metadata:
+  name: {{ .Release.Name }}-internal-ingress
+  namespace: {{ .Release.Namespace }}


I think we can remove this since helm will just use the release namespace if not set?

tthvo · 2024-11-22T06:02:32Z

charts/cryostat/templates/networkpolicy_ingress.yaml

+  ingress:
+    - from:
+      - podSelector:
+          matchLabels:
+            {{- include "cryostat.selectorLabels" $ | nindent 12 }}
+        namespaceSelector:
+          matchLabels:
+            kubernetes.io/metadata.name: {{ .Release.Namespace }}


When testing on OpenShift, I seem to be getting 503 errors when trying to access Cryostat through its Route. When the Network Policy is disabled, the route works fine. Not sure why

I wonder if router configurations in different OpenShift clusters play a role here. I tried locally with CRC with OpenShift 4.13 (OpenShift SDN CNI) and OpenShift v4.15 (OVN-Kubernetes CNI) and was able to access Cryostat via route URL.

If we could add an Ingress rule like below, would it help?

- from: - namespaceSelector: matchLabels: policy-group.network.openshift.io/ingress: ""

Reference: https://docs.openshift.com/container-platform/4.16/networking/network_security/network_policy/about-network-policy.html#nw-networkpolicy-allow-from-router_about-network-policy

Thanks for finding that @tthvo! That could be what we need. @andrewazores fwiw, this was using a ROSA cluster.

Thanks @tthvo , that looks like it did the trick. I was testing in crc and that was working, and after adding this additional rule it's also working on a ROSA cluster.

Oh nice! Glad that helped ^^

tthvo · 2024-11-22T09:05:02Z

charts/cryostat/templates/networkpolicy_ingress.yaml

+    - from:
+      - podSelector:
+          matchLabels:
+            {{- include "cryostat.selectorLabels" $ | nindent 12 }}
+        namespaceSelector:
+          matchLabels:
+            kubernetes.io/metadata.name: {{ .Release.Namespace }}


On this same topic as #208 (comment), accessing Cryostat via ingress is causing timeout. It does seem like ingress pods are being rejected :((

I tested with minikube with Calico as CNI and Nginx Ingress. I think the default Kindnet is not actually implementing NetworkPolicies.

I wonder if it would make sense to allow all ingress to Cryostat main pod instead?

I tested with minikube with Calico as CNI and Nginx Ingress. I think the default Kindnet is not actually implementing NetworkPolicies.

Yea, Kindnet doesn't implement them. That's supposed to mean they just get ignored and all traffic restrictions are lifted (or rather, are just not imposed)...

I wonder if it would make sense to allow all ingress to Cryostat main pod instead?

At that point, the policies may as well just be disabled then. Maybe these should be turned off by default along with the egress policies, since they are just meant to be an additional level of protection.

Would it make sense to use the NetworkPolicies to restrict traffic only for certain ports? Like for Cryostat talking to the storage pod? If the port is meant to be accessible from outside the cluster, then it's probably okay to be accessed from inside the cluster too.

I'm not sure that I understand. Creating an Ingress NetworkPolicy basically blocks all incoming traffic, and then the various rules create an allowlist for traffic that is allowed through based on the origin of that traffic. Since there are no specifications on the ports in these rules currently, all ports are allowed so long as the origin Pod matches one of the rules. So I think it's already behaving like this: "If the port is meant to be accessible from outside the cluster, then it's probably okay to be accessed from inside the cluster too."

I was thinking something like having one or more NetworkPolicy that:

Always allows traffic to the OAuth Proxy container, no matter the source

Filters traffic to the storage container/pod to only Cryostat itself

Ah, I see. In that case I think that would mean the policies' selectors would only select the storage Pod, and not Cryostat's own Pod (which is what also contains the OAuth Proxy). That way all traffic to the Cryostat Pod is allowed, but traffic to storage would only be accepted if it originated from Cryostat.

Same thing would also be applied to the database.

Right, they are additive, but the default when none are present is "allow everything". The moment the first one is defined for a Pod then the default policy for that Pod becomes "allow nothing", and the additive rules build the allowlist for what can come through. I think what you're describing would be a policy for the storage, a policy for the database, and no policy on Cryostat itself.

That would work. Not sure if there are any ports in the Cryostat pod we'd want to restrict.

On another note, one use case I was thinking of for the operator would be for the new nginx container. We could have a NetworkPolicy to only allow traffic to it from pods in the target namespaces.

I suppose we could still put a policy on the Cryostat Pod just for completeness, to make sure only the authproxy's HTTP(S) port(s) are externally visible.

I guess splitting netpol would also be helpful for fine-grained egress rules in #209 :D

mergify bot added the safe-to-test label Nov 13, 2024

andrewazores added the feat New feature or request label Nov 13, 2024

andrewazores marked this pull request as ready for review November 13, 2024 17:23

andrewazores requested review from Josh-Matsuoka and ebaron November 13, 2024 17:23

andrewazores mentioned this pull request Nov 13, 2024

feat(network): enable optional egress network policy #209

Draft

andrewazores force-pushed the network-policies branch from 4fc35da to 7ddb61a Compare November 21, 2024 16:21

ebaron reviewed Nov 21, 2024

View reviewed changes

tthvo reviewed Nov 22, 2024

View reviewed changes

andrewazores added 9 commits November 22, 2024 13:56

feat(network): enable internal ingress network policy

018a397

typo

314ae3c

nested ingress enablement property

1886e1c

rephrase

52235ca

remove tmp file

4d6e056

unit test fixup

9af9f81

add ingress rule to allow external traffic

e5ea6b8

fixup! add ingress rule to allow external traffic

15263a0

remove explicit namespace setting

735dd80

andrewazores force-pushed the network-policies branch from 12c39b4 to 735dd80 Compare November 22, 2024 18:56

split policies for each service

987181f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(network): enable internal ingress network policy #208

feat(network): enable internal ingress network policy #208

andrewazores commented Nov 13, 2024 •

edited

Loading

ebaron left a comment

tthvo Nov 22, 2024

tthvo Nov 22, 2024

ebaron Nov 22, 2024

andrewazores Nov 22, 2024

tthvo Nov 22, 2024

tthvo Nov 22, 2024 •

edited

Loading

andrewazores Nov 22, 2024

ebaron Nov 22, 2024

andrewazores Nov 22, 2024

ebaron Nov 22, 2024

andrewazores Nov 22, 2024

andrewazores Nov 22, 2024

ebaron Nov 22, 2024 •

edited

Loading

andrewazores Nov 22, 2024

tthvo Nov 22, 2024

feat(network): enable internal ingress network policy #208

Are you sure you want to change the base?

feat(network): enable internal ingress network policy #208

Conversation

andrewazores commented Nov 13, 2024 • edited Loading

ebaron left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tthvo Nov 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ebaron Nov 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewazores commented Nov 13, 2024 •

edited

Loading

tthvo Nov 22, 2024 •

edited

Loading

ebaron Nov 22, 2024 •

edited

Loading