Initial network Policies for hub proxy singleuser

[manics]

Adds network policies based on #534 (comment)

For now I've included ingress and egress rules so that there's a record of what was discussed above.

Policies are enabled by setting {hub,proxy,singleuser}.networkPolicy.enabled, default is False.

I've added {hub,proxy,singleuser}.networkPolicy.egress with the egress rules I thought could be optional, the idea being:

• Ingress/Egress rules which are required for jupyterhub pods to minimally function should always be defined
• Rules which are useful but which someone might want to restrict further are defaults which can be overridden

Since the proxy and hub should be trusted relaxing the rules and allowing all egress for them could be OK, the main benefit of egress rules are for restricting the singleuser servers.

Example of overriding singleuser egress to allow only DNS lookups and a access to single network:

singleuser:
  storage:
    type: none
  networkPolicy:
    enabled: true
    egress:
      - ports:
        - port: 53
          protocol: UDP
      - to:
        - ipBlock:
            cidr: 193.60.0.0/14

This works for me on openstack (Kubespray, canal network plugin). I've been trying to get this testable with minikube but it works only intermittently (See https://gist.github.com/manics/fe07200d535d12c59623801678f873d0 for setup if you're interested)

[minrk]

Awesome, thanks!

I'll rephrase my comment over there about where to store the information. Is it better for the policy to include explicit access like it does here, giving the proxy access to the Hub (network policy needs to know which pods should have access):

ingress:
  - from:
    - podSelector:
        matchLabels:
          name: proxy
          component: proxy

or to specify labels to use on pods for requesting access, e.g.

ingress:
  - from:
    - podSelector:
        matchExpressions:
          - key: access-hub-api
            operator: Exists

and then applying those labels to the pods that need the access:

labels:
  access-hub-api: "true" # value doesn't matter with matchExpressions: Exists
  access-singleuser: "true"

Pros of using access labels:

• only edit pod descriptions to give them access,
  rather than copying pod label specs into network policy
• network policy is relatively static
• easy to add additional pods in custom deployments with access to these APIs; just apply the relevant label(s).

Cons of access labels:

• a little weird to have labels whose values don't matter
• maybe harder to grant access to pods whose labels are hard to edit, e.g. in another helm chart (does this happen?)
• network policy is no longer a one-stop-shop to look at who has access to what.

Access labels seems like a cleaner solution to me, but I'm no Kubernetes expert.

[manics]

I don't have much Kubernetes experience either, but from what you've outlined access labels sounds cleaner, especially from the extensibility point of view.

Is there a reason to prefer matchExpressions over matchLabels?

matchLabels:
  access-hub-api: "true"

[yuvipanda]

I think access labels are the better solution too.

One additional thing is we should namespace our labels. I think maybe hub.jupyter.org/network-access-hub-api: "true" and friends perhaps? We're already using hub.jupyter.org as a prefix elsewhere.

This is awesome, @manics :)

[manics]

I've switched the ingress rules to use podSelector: matchLabels: hub.jupyter.org/network-access-*: "true". I chose matchLabels to avoid confusion over matchExpressions not needing a value, also this is in the Kubernetes example

I'm not sure what to do with the egress rules. I've added egress ports 443 and 6443 for hub and proxy (nginx-ingress) since they require access to the kubernetes api-server: https://kubernetes.io/docs/admin/accessing-the-api/ suggests these are the defaults and kubespray seems to fit with this.

[minrk]

Good questions, re: egress. JupyterHub does need egress to kubernetes itself, and will need egress to the outside world for most Hub implementations. If it's not simple to do this, maybe don't set a default egress policy on the Hub.

The proxy needs egress to the hub and single-user server and any possible additional hub services, which means it is probably best to use network-access-proxy-target label for proxy egress, rather than whitelisting destinations with label selectors.

[minrk]

single-user servers do not need egress to the proxy I think, since all connections will be initiated by the proxy.

[manics]

I'm having trouble getting my head around these labels. Here's my understanding so far.

Two types of labels:

1. Pod A should allow access from B:

   • This is a label on B e.g. Hub has label hub.jupyter.org/network-access-proxy-api: "true"
   • This corresponds to an ingress rule on A e.g. Proxy allows ingress to it's API port from Hub and any other pod with the above label
   • This is currently implemented in this PR

2. Pod C should be allowed to talk to D:

   • This would be a label on D, e.g. Hub could have label hub.jupyter.org/network-access-allowed-proxy: "true" ?
   • This corresponds to an egress rule on C, e.g. Proxy is allowed to talk to Hub and any other pod with the above label

Have I understood you correctly?

[manics]

Maybe this is overcomplicating things, and egress rules should default to allow-all for both hub and proxy? Ingress rules should prevent malicious inbound connections. Egress rules would limit malicious outbound connections but this implies exploitation of a bug or config error in the hub and proxy which is hopefully unlikely.

This leaves just the singleuser server where egress rules are useful because users may not be trusted. If I allow all egress by default but also include an explicit rule to allow egress to the hub this makes it easy for an admin to block everything else.

[manics]

I went ahead and removed the complicated egress rules for hub and proxy as described in my last comment.

[minrk]

This is great! I think removing the egress policies for the hub and proxy makes sense. Perhaps the only additional thing I would add is to make the default single-user egress policy slightly more strict by adding the whole cluster-cidr to the allow-0.0.0.0 exception, so that all in-cluster communication is disabled by default from the user containers.

[manics]

I don't think there's a standard cluster-cidr. I could block egress to all private IPs:

    egress:
      - ports:
        - protocol: UDP
          port: 53  # internal kube-dns
      - to:
        - ipBlock:
            cidr: 0.0.0.0/0
            except:
              - 10.0.0.0/8
              - 172.16.0.0/12
              - 192.168.0.0/16
              - 169.254.0.0/16

but that might obstruct private jupyterhub deployments?

[minrk]

Good point. Let's start with this, then.

[manics]

Thanks for merging!

[yuvipanda]

I can confirm that this works! It was fairly easy to add an egress rule to singleuser-server to prevent external network access too!.

Thank you very much for your contribution, @manics!

[manics]

Thanks! Do you want me to add some docs? Security looks like the best place: https://github.com/jupyterhub/zero-to-jupyterhub-k8s/blob/master/doc/source/security.md

[yuvipanda]

+1, security looks like the right place!

…

On Wed, Mar 21, 2018 at 10:19 AM, Simon Li ***@***.***> wrote: Thanks! Do you want me to add some docs- Security looks like the best place: https://github.com/jupyterhub/zero-to-jupyterhub-k8s/blob/ master/doc/source/security.md — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#546 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAB23o4eeQM9IIi4lNU-PH-saimyeYH2ks5tgouSgaJpZM4SXGpB> .

-- Yuvi Panda T http://yuvi.in/blog

[choldgraf]

spot-checking this PR from the fuuuture

what does {{- if and <XXX> }} accomplish? To the untrained eye it seems like this is a typo, or is there a particular use for if and in the context of jinja?

[betatim]

(nb. these aren't jinja templates but go templates)

[yuvipanda]

Pretty sure it's a typo that's Mostly Harmless(tm)