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

Enable rotation of service account tokens #2839

Merged
merged 5 commits into from
Jan 17, 2020
Merged

Conversation

linki
Copy link
Member

@linki linki commented Jan 7, 2020

Adds timestamps to ServiceAccountTokens so that they can be rejected by the API server when they have been expired (via iat, exp and nbf fields). Tokens will also be rejected once the corresponding Pod terminates.

exp: expires at
iat: issued at
nbf: not valid before (equals issued at)

# JWT header
# ...
# JWT payload
{
  "aud": [
    "kubernetes/serviceaccount"
  ],
  "exp": 1578415862,
  "iat": 1578412262,
  "iss": "kubernetes/serviceaccount",
  "kubernetes.io": {
    "namespace": "kapp",
    "pod": {
      "name": "chaoskube-56967cd969-bcmht",
      "uid": "feaf2cc8-ff62-4681-a92a-f25f96a6beff"
    },
    "serviceaccount": {
      "name": "chaoskube",
      "uid": "d1a29604-36f2-4c90-94e8-d7886353e2f9"
    }
  },
  "nbf": 1578412262,
  "sub": "system:serviceaccount:kapp:chaoskube"
}
# JWT signature
# ...

Like before Kubernetes injects ServiceAccountToken mounts into the Pod but they look different now.

  # PodSpec
  # ...
  # following is injected into each pod
  volumes:
  - name: kube-api-access-7nxqz
    projected:
      defaultMode: 420
      sources:
      - serviceAccountToken:
          expirationSeconds: 3600
          path: token
      - configMap:
          items:
          - key: ca.crt
            path: ca.crt
          name: kube-root-ca.crt
      - downwardAPI:
          items:
          - fieldRef:
              apiVersion: v1
              fieldPath: metadata.namespace
            path: namespace

The feature uses projected volumes under the hood. With this it's possible to retain the original filesystem structure which is three files that are now gathered from different sources.

ls /var/run/secrets/kubernetes.io/serviceaccount/
ca.crt     namespace  token

Instead of a ServiceAccount secret with three entries the following is used:

namespace: via downward API
ca.crt: a shared configmap containing only the root cert of the cluster
token: new volume type serviceAccountToken that provides an expiring token

Currently, the "old" secret still exists and contains a non-expiring ServiceAccountToken which still gets accepted by the API server.

The only different for clients is the requirement to reload refreshed tokens from the filesystem before they expire.

Part of: https://github.bus.zalan.do/teapot/issues/issues/1993

Todo:

  • prevent issue of "old" service account secrets (--controllers flag on controller manager)
  • test how clients behave when the token gets refreshed (recent client-go works fine)
  • cleanup
  • probably: find a way to opt-out individual pods to ease migration efforts
  • optional: open-up external access for service account tokens again

In order for non-root applications to read the credentials you might need to add the following to your pod spec. This requirement might go away in future versions. Also see here.

      securityContext:
        fsGroup: 65534

Original feature issue: kubernetes/kubernetes#70679

@linki linki force-pushed the bound-service-account-tokens branch 2 times, most recently from af7d263 to d646ebf Compare January 8, 2020 14:23
@linki linki force-pushed the bound-service-account-tokens branch from 0654dc0 to 1fa08bc Compare January 8, 2020 16:10
@linki
Copy link
Member Author

linki commented Jan 10, 2020

Ultimately, this has to be split into two PRs to make the rollout safe:

  • PR 1: apply manifest changes for securityContext and projected volume
  • PR 2: roll the nodes and switch to rotated service account tokens

@linki linki force-pushed the bound-service-account-tokens branch 2 times, most recently from a1288a8 to 929878e Compare January 10, 2020 15:31
@linki
Copy link
Member Author

linki commented Jan 10, 2020

Merging this doesn't enable it yet. It needs to be enabled with a config item.

@linki
Copy link
Member Author

linki commented Jan 13, 2020

@mikkeloscar @szuecs I would like to get your opinion on adding securityContext.fsGroup to several deployments in this PR (e.g. skipper, external-dns).

This is necessary because the new tokens are mounted with slightly different file permissions (more secure).

Previously, inside the container it looked like this (note the file permissions, owner and group):

$ ls -la /var/run/secrets/kubernetes.io/serviceaccount/..data/
-rw-r--r--    1 root     root           872 Jan 13 10:17 token

So, the token was readable by everybody, including non-root users in the container (e.g. 1000 for skipper would be able to read it)

With the new token system token mounts looks like the following (note the slightly different file permission):

$ ls -la /var/run/secrets/kubernetes.io/serviceaccount/..data/
-rw-r-----    1 root     root         908 Jan 13 09:58 token

This means non-root users cannot read the token anymore (e.g. skipper).

One way to fix it is to use securityContext.fsGroup: 1000 which changes the file permissions of any mounted volume (note the changed group file permission to the running process' user):

$ ls -la /meta/aws-iam/..data/
-rw-r--r--    1 root     1000          69 Jan 13 10:34 role-arn

This should not matter since the files were world-readable before.

However, the service account token mounts now look like this:

$ ls -la /var/run/secrets/kubernetes.io/serviceaccount/..data/
-rw-r-----    1 root     1000         908 Jan 13 09:58 token

which means they can now be read by the main process (e.g. skipper, external-dns, etc.)

Do you see any issues with this?

@mikkeloscar
Copy link
Contributor

@linki I don't see any issues with changing the file permissions.

@linki linki force-pushed the bound-service-account-tokens branch from 9f40c1d to 929878e Compare January 13, 2020 11:03
@szuecs
Copy link
Member

szuecs commented Jan 13, 2020

If you change all securityContext.fsgroups it seems to be fine for me.
It's progress in security, so let's do it!

@linki linki force-pushed the bound-service-account-tokens branch from 6344bed to 0940b38 Compare January 15, 2020 14:16
@linki
Copy link
Member Author

linki commented Jan 17, 2020

👍

1 similar comment
@szuecs
Copy link
Member

szuecs commented Jan 17, 2020

👍

@szuecs szuecs merged commit 1717c00 into dev Jan 17, 2020
@szuecs szuecs deleted the bound-service-account-tokens branch January 17, 2020 12:30
This was referenced Jan 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants