Promote networkpolicies out of /contrib

[juliusvonkohout]

Follow up of #2311 (comment)

@kimwnasptd @annajung @TobiasGoerke

Yes, we should promote it even for 1.7. It is a second security barrier and best practice. It prevented some of the latest security flaws that we had in Kubeflow with missing sidecars or before that the unprotected KFAM.

@kimwnasptd common/networkpolicies is the right place and we should include it in the example installation file https://github.com/kubeflow/manifests/blob/master/example/kustomization.yaml

It would also make sense to add me to the root owner file then, since istio-cni is the next topic.in /common

[kimwnasptd]

Thanks for creating the issue!

Before deciding on promoting them to be added as an extra layer in Kubeflow I'd like to understand if they offer something that we currently can't achieve with Istio. In both the above cases for the missing sidecar and the unprotected KFAM the issue was with an incorrect misconfiguration in Istio's auth mechanism (AuthorizationPolicies and sidecars).

Personally for Notebooks WG I'd like to avoid adding another layer of complexity in the KF stack, which will add a small burden on debugging and maintaining this logic, if this can already be achieved with Istio.

In my mind securing who can talk to who is the responsibility of the service mesh, that's why I keep on bringing Istio. Is there a scenario that we currently can't secure with Istio and for which we need explicitly NetworkPolicies?

let's also bring this to the attention of the rest of the WG leads, since such a change would affect other WGs as well
cc @andreyvelich @johnugeorge @zijianjoy

[juliusvonkohout]

@kimwnasptd i do not think that all components support istio yet and it is also useful for isolating user namespaces where sometimes not everything uses istio. If we get to a perfect world in 2 years and really do not need it anymore we can always think about deprecation. Also for it administrators that have to specify network flows and satisfy enterprise guidelines it comes in quite handy. Furthermore i do not see debugging problems and it is used by some other companies as well. I can maintain it for the next years, although there is not much to do.

[juliusvonkohout]

And again something that would have been prevented by networkpolicies kubeflow/kubeflow#7032. In the long term i also want to protect the usernamespaces with networkpolicies as done in another PR.

[rawc0der]

Hello! 👋
Do you think @kimwnasptd that migrating the networkpolicies into /common might be in scope for #2456?

[kimwnasptd]

@rawc0der I think there are still missing details we need to define before merging #2456

There are 2 questions that I think are not clear regarding this:

1. Why do we need an extra component when we can achieve the same functionality with Istio?
2. Moving this in /common means that then it's up to the Manifests WG to maintain an extra layer that should be added on top of all WGs. Shouldn't this change be pushed in WGs instead?

[juliusvonkohout]

For 1. we could list the exploits so far that have been or would have been prevented by networkpolicies in the kubeflow and or user namespaces.
Furthermore there are some workloads which do not support istio yet. It also provides a proper network layer description in one place. So far i know of 2 critical and 2 high CVEs

For 2. I think the security related stuff has to be mostly in one place. This is also valid for podsecuritypolicies / standards. It would take years to get this into all WGs anyway. But if this is your long term goal we should have it in manifests and migrate it step by step in the WGs.

[rawc0der]

Hi @kimwnasptd!

1. I want to point out that Istio although a dependency to Kubeflow is not Kubernetes native and adding some native guardrails is not actually a bad thing.
   Imagine a structure of security-in-depth components that all add up to the entire security posture. You could consider the following analogy (Kernel & OS, where Istio AuthPol secure only the os layer and the networkpolicies secure the kernel).

2. I tend to echo with @juliusvonkohout here. What happens to all security related resources could be better managed under one place in /common in order to have a better overview of all the layers and relationships between components.
   Things change quickly PSP get deprecated, PSS become stable, then what about adding some OPA polcies for further enforcing runtime conditions.. how would you keep track of it all with the complexity and uncertainties in individual WGs?

[kimwnasptd]

Hi @rawc0der. Regarding the first point I see the following two problems:

• Having 2 different components for achieving the same functionality seems like additional maintenance (or are corner cases that Istio doesn't handle? Can we list them as I asked above?)
• I'm not sure I understand what you mean by Kubernetes native. What I'd say though is that for me the decision is either we (Kubeflow project) decide that we depend on Istio or not. If we do then let's use what it provides for securing the mesh, which Istio is there to configure in the first place

[kimwnasptd]

Regarding the second point, that @juliusvonkohout is also talking about:

Right now the Manifests repo is just a place for gathering the components maintained by the KF WGs and providing some third-party ones for folks to deploy a whole solution. Adding NetworkPolicies to /common means that Manifests WG will have overlay logic that affects all the components. Who is then the entity responsible for this functionality? Is it the Manifests WG? Is it the WGs that are affected themselves? Is it the Security Team/WG?

We currently don't have such an agreed process right now (but, by all means let's continue the discussion to establish one). And for me this is how it should look like, for efforts need to affect all components:

1. WGs need to approve the effort
2. WGs will need to own and maintain the part that corresponds to them, in their repos
   • In this case, for NetworkPolicies, for Manifests WG we'll need to maintain NPs for Istio, Cert Manager, Dex etc
3. For efforts that for implementation reasons all files need to be in one place there still needs to be a clear distinction of which entity owns what
   • Which to me again it should be each WG owning their part, not Manifests owning the corresponding part for each WG
   • What is the expectations also from the Security Team/WG for such efforts

Lastly, to @juliusvonkohout's point

It would take years to get this into all WGs anyway. But if this is your long term goal we should have it in manifests and migrate it step by step in the WGs.

I wouldn't want the manifests repo to be a place where functionality gets added because the WGs don't accept a change or discussions take too long. The bug in this case is how features are agreed and discussed in WGs. But the solution to this bug is not to have a "backdoor" for pushing efforts in a central place in order to avoid interactions with WGs and impose decisions on them.

[rawc0der]

Hi @kimwnasptd,

Thanks for raising up these points and for trying to describe the decision process from a WG Lead perspective. Decision making is tough and comes with some risk, but the community needs to figure out how to move forward and try to come to some conclusions.
Hope I can shed some light on some of the proposals, so here's my two cents:

I'm not sure I understand what you mean by Kubernetes native.

Istio authn/z enforcement is different from the Kubernetes networkpolicies in the sense that netpols are part of kuberntes in-tree api services ("native") and one should expect Stable behaviour as they are part of --> networking.k8s.io/v1 apigroup, while Istio Authpols are enabled by extension and the behaviour can change since they are part of beta apigroups --> security.istio.io/v1beta1 .
Istio security enforcement is acting on Layer 7 (HTTP requests) in envoy proxies securing traffic from a client-server perspective, while Kubernetes Netpols act as true network firewall on Layer 3/4 (IP and ports).

(or are corner cases that Istio doesn't handle? Can we list them as I asked above?)

This is something I can't answer, but because of different conditions whre security is enforced, you can have edge cases yes. This is what keeps some people up at night :D.

Adding NetworkPolicies to /common means that Manifests WG will have overlay logic that affects all the components. Who is then the entity responsible for this functionality? Is it the Manifests WG? Is it the WGs that are affected themselves? Is it the Security Team/WG?

Well, I believe since the functionality added by NP is security related the responsibility and ownership should fall under Security WG Team (once process, official channels and core members are established). Until then seems that Manifest WG responsibilities are a bit all-over the place, stretching from /contrib to /common and all corners of this repo.
Moving ownership of security related assets (NetPols, Istio Authpols, PSS, Admission Controls etc) to the Security WG Team and having a consensus of who is responsible for what should improve collaboration between WGs and also take some pressure off the Manifests WG concerns.

For efforts that for implementation reasons all files need to be in one place there still needs to be a clear distinction of which entity owns what
Which to me again it should be each WG owning their part, not Manifests owning the corresponding part for each WG
What is the expectations also from the Security Team/WG for such efforts

I strongly believe that multiple WG should commit to maintain clean in structure the same repository, having all files (kustomize manifests) in one place makes sense and corresponding WG should maintain the parts they are concerned with.

Obviously there needs to be some synergy between Security/Manifests WG - so let's keep the discussion alive to bring these ideas to life.

[juliusvonkohout]

@andreyvelich @johnugeorge @jbottum this is probably a decision for the KSC or TOC