Make default for --anonymous-auth be false. #38708

erictune · 2016-12-13T15:38:53Z

Changes the default value of the "anonymous-auth" flag to a safer default value of false. This affects kube apiserver and federation apiserver. See https://groups.google.com/forum/#!topic/kubernetes-announce/iclRj-6Nfsg for more details.

k8s-reviewable · 2016-12-13T15:39:00Z

This change is

erictune · 2016-12-13T15:39:31Z

I'm not 100% sure this is the fix we want, but I am sending it so that we can talk about it and test it.

k8s-github-robot · 2016-12-13T17:22:14Z

This PR is not for the master branch but does not have the cherrypick-approved label. Adding the do-not-merge label.

erictune · 2016-12-13T18:00:00Z

@saad-ali don't kick off after merging this. Still checking if federation change practical

k8s-github-robot · 2016-12-13T18:08:56Z

This PR is not for the master branch but does not have the cherrypick-approved label. Adding the do-not-merge label.

luxas · 2016-12-13T18:10:46Z

pkg/genericapiserver/options/server_run_options.go

@@ -293,7 +293,7 @@ func (s *ServerRunOptions) AddUniversalFlags(fs *pflag.FlagSet) {
 	fs.BoolVar(&s.AnonymousAuth, "anonymous-auth", s.AnonymousAuth, ""+
 		"Enables anonymous requests to the secure port of the API server. "+
 		"Requests that are not rejected by another authentication method are treated as anonymous requests. "+
-		"Anonymous requests have a username of system:anonymous, and a group name of system:unauthenticated.")
+		"Anonymous requests have a username of system:anonymous, and a group name of system:unauthenticated. ")


seems like a typo or something

@erictune it would be nice to fix the typo, but if @saad-ali is in a big hurry, I wouldn't block on it.

Nope, absolutely not needed for the v1.5.1 cut

deads2k · 2016-12-13T19:27:22Z

It's a shame, but I'm ok with it for 1.5.

At some point we need to rip off the bandaid, but it doesn't have to be now. lgtm

saad-ali · 2016-12-13T19:33:34Z

@erictune Can you add a release note please in the original post

saad-ali · 2016-12-13T19:56:40Z

@saad-ali don't kick off after merging this. Still checking if federation change practical

Ack, @erictune. Will wait for your go ahead before proceeding with merge.

erictune · 2016-12-13T20:23:59Z

Ok to merge. @mahudustancs showed me that this covers federation too

saad-ali · 2016-12-13T20:26:33Z

Awesome merging

saad-ali · 2016-12-13T20:33:31Z

All checks are green:

@k8s-ci-robot  Jenkins CRI GCE Node e2e — Build succeeded.
Details@k8s-ci-robot  Jenkins GCE Node e2e — Build succeeded.
Details@k8s-ci-robot  Jenkins GCE e2e — Build succeeded.
Details@k8s-ci-robot  Jenkins GCE etcd3 e2e — Build succeeded.
Details@k8s-ci-robot  Jenkins GCI GCE e2e — Build succeeded.
Details@k8s-ci-robot  Jenkins GCI GKE smoke e2e — Build succeeded.
Details@k8s-ci-robot  Jenkins GKE smoke e2e — Build succeeded.
Details@k8s-ci-robot  Jenkins Kubemark GCE e2e — Build succeeded.
Details@k8s-ci-robot  Jenkins unit/integration — Build succeeded.
Details@k8s-ci-robot  Jenkins verification — Build succeeded.
Details@thelinuxfoundation  cla/linuxfoundation — erictune authorized

Manually merging for v1.5.1

saad-ali · 2016-12-13T22:16:08Z

Let's make sure this gets merged back to master at some point

saad-ali · 2016-12-16T22:23:49Z

Let's make sure this gets merged back to master at some point

@deads2k, @erictune is out, could you make sure this PR #38717 and #38898 are cherr-picked back to the master branch?

deads2k · 2016-12-19T12:45:40Z

@deads2k, @erictune is out, could you make sure this PR #38717 and #38898 are cherr-picked back to the master branch?

@saad-ali @liggitt I think we want to try to switch the default again in 1.6. If we update the abac user star evaluation, eliminate the rbac user star (no need for it and its alpha), and use my default stomping pull I think we have a reasonable shot.

liggitt · 2016-12-19T18:47:23Z

#38968 makes ABAC safe with anonymous auth enabled

liggitt · 2016-12-19T18:56:39Z

#38981 removes special handling of * for RBAC bindings

saad-ali · 2016-12-19T19:08:03Z

@saad-ali @liggitt I think we want to try to switch the default again in 1.6. If we update the abac user star evaluation, eliminate the rbac user star (no need for it and its alpha), and use my default stomping pull I think we have a reasonable shot.

Ack. Feel free to correct me, as I'm completely out of my depth here, but wasn't the concern with this that the default authorization was AllowAll which when combined with the new default authentication scheme permitting anonymous access was dangerous. If that's the case, modifying the ABAC and RBAC wouldn't fix AllowAll, right? Just want to make sure we don't have to repeat the same song and dance for 1.6. Thank you both for being on this.

liggitt · 2016-12-19T19:21:47Z

#38706 is open to address the anonymous+AlwaysAllow combination

there were secondary concerns with legacy RBAC/ABAC policies matching anonymous users unintentionally

saad-ali · 2016-12-19T19:30:48Z

Perfect, thanks @liggitt

Make default for --anonymous-auth be false.

c7c2c73

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Dec 13, 2016

erictune added do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. release-note-none Denotes a PR that doesn't merit a release note. labels Dec 13, 2016

k8s-github-robot assigned nikhiljindal Dec 13, 2016

k8s-github-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Dec 13, 2016

erictune removed the do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. label Dec 13, 2016

k8s-github-robot added the do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. label Dec 13, 2016

erictune removed the do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. label Dec 13, 2016

erictune assigned saad-ali Dec 13, 2016

k8s-github-robot added the do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. label Dec 13, 2016

luxas reviewed Dec 13, 2016

View reviewed changes

saad-ali requested review from liggitt and deads2k December 13, 2016 19:23

saad-ali assigned liggitt and deads2k Dec 13, 2016

saad-ali added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Dec 13, 2016

saad-ali added this to the v1.5 milestone Dec 13, 2016

saad-ali added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 13, 2016

saad-ali removed the do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. label Dec 13, 2016

saad-ali merged commit db74b2d into kubernetes:release-1.5 Dec 13, 2016

janetkuo mentioned this pull request Dec 16, 2016

hack/local-up-cluster.sh broken on latest 1.5 branch #38847

Closed

saad-ali mentioned this pull request Dec 16, 2016

make local-up-cluster.sh match "normal" launch without anonymous access #38898

Merged

erictune deleted the one-five-one branch August 8, 2017 18:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make default for --anonymous-auth be false. #38708

Make default for --anonymous-auth be false. #38708

erictune commented Dec 13, 2016 •

edited by saad-ali

k8s-reviewable commented Dec 13, 2016

erictune commented Dec 13, 2016

k8s-github-robot commented Dec 13, 2016

erictune commented Dec 13, 2016

k8s-github-robot commented Dec 13, 2016

luxas Dec 13, 2016

deads2k Dec 13, 2016

luxas Dec 13, 2016

deads2k commented Dec 13, 2016

saad-ali commented Dec 13, 2016

saad-ali commented Dec 13, 2016 •

edited

erictune commented Dec 13, 2016

saad-ali commented Dec 13, 2016

saad-ali commented Dec 13, 2016 •

edited

saad-ali commented Dec 13, 2016

saad-ali commented Dec 16, 2016

deads2k commented Dec 19, 2016

liggitt commented Dec 19, 2016

liggitt commented Dec 19, 2016

saad-ali commented Dec 19, 2016

liggitt commented Dec 19, 2016

saad-ali commented Dec 19, 2016

Make default for --anonymous-auth be false. #38708

Make default for --anonymous-auth be false. #38708

Conversation

erictune commented Dec 13, 2016 • edited by saad-ali

k8s-reviewable commented Dec 13, 2016

erictune commented Dec 13, 2016

k8s-github-robot commented Dec 13, 2016

erictune commented Dec 13, 2016

k8s-github-robot commented Dec 13, 2016

luxas Dec 13, 2016

Choose a reason for hiding this comment

deads2k Dec 13, 2016

Choose a reason for hiding this comment

luxas Dec 13, 2016

Choose a reason for hiding this comment

deads2k commented Dec 13, 2016

saad-ali commented Dec 13, 2016

saad-ali commented Dec 13, 2016 • edited

erictune commented Dec 13, 2016

saad-ali commented Dec 13, 2016

saad-ali commented Dec 13, 2016 • edited

saad-ali commented Dec 13, 2016

saad-ali commented Dec 16, 2016

deads2k commented Dec 19, 2016

liggitt commented Dec 19, 2016

liggitt commented Dec 19, 2016

saad-ali commented Dec 19, 2016

liggitt commented Dec 19, 2016

saad-ali commented Dec 19, 2016

erictune commented Dec 13, 2016 •

edited by saad-ali

saad-ali commented Dec 13, 2016 •

edited

saad-ali commented Dec 13, 2016 •

edited