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.

Sign up for GitHub

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

Jump to bottom

Enable service account signing key rotation #34029

Merged
merged 1 commit into from
Oct 11, 2016
Merged

Enable service account signing key rotation #34029

merged 1 commit into from
Oct 11, 2016
+82 −39

Conversation

liggitt
Copy link
Member

@liggitt liggitt commented Oct 4, 2016 •
edited
Loading

fixes #21007

The kube-apiserver --service-account-key-file option can be specified multiple times, or can point to a file containing multiple keys, to enable rotation of signing keys.

This PR enables the apiserver authenticator to verify service account tokens signed by different private keys. This can be done two different ways:

  • including multiple keys in the specified keyfile (e.g. --service-account-key-file=keys.pem)
  • specifying multiple key files (e.g. --service-account-key-file current-key.pem --service-account-key-file=old-key.pem)

This is part of enabling signing key rotation:

  1. update apiserver(s) to verify tokens signed with a new public key while still allowing tokens signed with the current public key (which is what this PR enables)
  2. give controllermanager the new private key to sign new tokens with
  3. remove old service account tokens (determined by verifying signature or by checking creationTimestamp) once they are no longer in use (determined using garbage collection or magic) or some other algorithm (24 hours after rotation, etc). For the deletion to immediately revoke the token, --service-account-lookup must be enabled on the apiserver.
  4. once all old tokens are gone, update apiservers again, removing the old public key.

This change is Reviewable

@liggitt liggitt added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Oct 4, 2016
@liggitt liggitt added the sig/auth Categorizes an issue or PR as relevant to SIG Auth. label Oct 4, 2016
@liggitt
Copy link
Member Author

liggitt commented Oct 4, 2016

@erictune PTAL
cc @kubernetes/sig-auth

@k8s-github-robot k8s-github-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Oct 4, 2016
@liggitt liggitt force-pushed the service-account-rotation branch from 41f7639 to ffa2593 Compare October 4, 2016 17:40
ericchiang
ericchiang reviewed Oct 4, 2016
View reviewed changes
cmd/kube-apiserver/app/options/options.go
@@ -70,9 +70,10 @@ func (s *APIServer) AddFlags(fs *pflag.FlagSet) {
fs.DurationVar(&s.EventTTL, "event-ttl", s.EventTTL,
"Amount of time to retain events. Default is 1h.")

fs.StringVar(&s.ServiceAccountKeyFile, "service-account-key-file", s.ServiceAccountKeyFile, ""+
fs.StringArrayVar(&s.ServiceAccountKeyFiles, "service-account-key-file", s.ServiceAccountKeyFiles, ""+
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this translate into which keys actually sign the token? I've been trying to hunt down that code path but haven't been able to find it.

Copy link
Member Author

@liggitt liggitt Oct 4, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only a single key signs the token, and that private key is given to the controller manager:

kube-controller-manager --help 2>&1 | grep service-account

--service-account-private-key-file string
Filename containing a PEM-encoded private RSA or ECDSA key used to sign service account tokens.

@liggitt liggitt force-pushed the service-account-rotation branch 2 times, most recently from 01eb918 to 6d382f1 Compare October 4, 2016 18:07
@k8s-github-robot k8s-github-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 4, 2016
@liggitt liggitt force-pushed the service-account-rotation branch from 6d382f1 to e6bcba6 Compare October 4, 2016 18:11
@liggitt liggitt force-pushed the service-account-rotation branch from e6bcba6 to 3c92eb7 Compare October 4, 2016 18:16
@k8s-ci-robot
Copy link
Contributor

Jenkins GCI GCE e2e failed for commit 3c92eb7. Full PR test history.

The magic incantation to run this job again is @k8s-bot gci gce e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@liggitt
Copy link
Member Author

liggitt commented Oct 4, 2016

@k8s-bot gci gce e2e test this

@erictune
Copy link
Member

erictune commented Oct 5, 2016

@alex-mohr @roberthbailey
Can you take a look and see if this solves any key rotation issues for you?

@erictune
Copy link
Member

erictune commented Oct 5, 2016

@liggitt can you write a few sentences reminding what the overall steps are for rotation?

@zhouhaibing089
Copy link
Contributor

I would like to understand the steps as well, and what in my mind is:

  1. the jwt would try all of the public keys, and failed only when all of them are not correct.
  2. the rotation only cares about public keys.

am I right?

@liggitt
Copy link
Member Author

liggitt commented Oct 7, 2016 •
edited
Loading

  1. the jwt would try all of the public keys, and failed only when all of them are not correct.
  2. the rotation only cares about public keys.

correct, updated the PR description

@erictune
Copy link
Member

LGTM

@erictune erictune mentioned this pull request Oct 10, 2016
Closed
@erictune
Copy link
Member

Filed kubernetes/website#1421 to get this procedure documented.

@liggitt liggitt added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 10, 2016
@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit 1837914 into kubernetes:master Oct 11, 2016
@liggitt liggitt deleted the service-account-rotation branch October 12, 2016 03:59
@mumoshu mumoshu mentioned this pull request Nov 20, 2017
Closed
2 tasks
@ericchiang ericchiang mentioned this pull request Apr 23, 2018
Open
4 tasks
@Matty9191 Matty9191 mentioned this pull request May 13, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ericchiang ericchiang ericchiang left review comments

Assignees

@erictune erictune

Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/auth Categorizes an issue or PR as relevant to SIG Auth. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Easier service account key rotations: Allow for multiple service account token verification certs
7 participants
@liggitt @k8s-ci-robot @erictune @zhouhaibing089 @k8s-github-robot @ericchiang @googlebot