add client-ca to configmap in kube-public #41814

deads2k · 2017-02-21T16:22:30Z

Client CA information is not secret and it's required for any API server trying to terminate a TLS connection. This pull adds the information to configmaps in kube-public that look like this:

apiVersion: v1
data:
  client-ca.crt: |
    -----BEGIN CERTIFICATE-----
    -----END CERTIFICATE-----
  requestheader-allowed-names: '["system:auth-proxy"]'
  requestheader-client-ca-file: |
    -----BEGIN CERTIFICATE-----
    -----END CERTIFICATE-----
  requestheader-extra-headers-prefix: '["X-Remote-Extra-"]'
  requestheader-group-headers: '["X-Remote-Group"]'
  requestheader-username-headers: '["X-Remote-User"]'
kind: ConfigMap
metadata:
  creationTimestamp: 2017-02-22T17:54:37Z
  name: extension-apiserver-authentication
  namespace: kube-system
  resourceVersion: "6"
  selfLink: /api/v1/namespaces/kube-system/configmaps/extension-apiserver-authentication
  uid: fa1dd328-f927-11e6-8b0e-28d2447dc82b

@kubernetes/sig-auth-api-reviews @liggitt @kubernetes/sig-api-machinery-pr-reviews @lavalamp @sttts

There will need to be a corresponding pull for permissions

k8s-github-robot · 2017-02-21T16:22:40Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

The following people have approved this PR: deads2k

Needs approval from an approver in each of these OWNERS Files:

~~cmd/kube-apiserver/OWNERS~~ [deads2k]
pkg/master/OWNERS

We suggest the following people:
cc @lavalamp
You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

k8s-reviewable · 2017-02-21T16:23:39Z

This change is

liggitt · 2017-02-21T19:01:52Z

pkg/master/client_ca_hook.go

+		}
+		data := map[string]string{
+			"front-proxy-ca.crt":        string(h.FrontProxyCA),
+			"front-proxy-allowed-names": string(serializedNames),


this is half of the information required for front-proxy config... do we really want to stitch together the header names from args to the UAS with information from here?

this also means write permission to this object is escalating

this also means write permission to this object is escalating

So is the client-ca one. If you control the trusted cert and can write it to match a key you own, it's done.

this is half of the information required for front-proxy config... do we really want to stitch together the header names from args to the UAS with information from here?

More crisply, are you asking for all the --request-header-* options?

liggitt · 2017-02-21T19:02:50Z

pkg/master/client_ca_hook.go

+	}
+
+	if len(h.ClientCA) > 0 {
+		if err := writePublicClientCert(client, "client-ca", map[string]string{


there are several components that take client-ca config... this assumes they all use the same one. is that a good assumption?

there are several components that take client-ca config... this assumes they all use the same one. is that a good assumption?

Open to an alternate name.

liggitt · 2017-02-21T19:04:15Z

pkg/master/client_ca_hook.go

+
+// writeClientCAs is here for unit testing with a fake client
+func (h ClientCARegistrationHook) writeClientCAs(client coreclient.CoreInterface) {
+	if _, err := client.Namespaces().Create(&api.Namespace{ObjectMeta: metav1.ObjectMeta{Name: metav1.NamespacePublic}}); err != nil && !apierrors.IsAlreadyExists(err) {


do these need to be in kube-public? delegating servers will already need credentials allowed to run SubjectAccessReview/TokenReview checks... can we limit the exposure of this info to those users?

can we limit the exposure of this info to those users?

Which namespace would you like it in?

do these need to be in kube-public?

They could really go anywhere, but the whole point of a public key is that it can be public. Why would they live somewhere else?

deads2k · 2017-02-22T17:56:03Z

@liggitt made it configmap/extension-apiserver-authentication in kube-system. I'll start working on permissions separately.

deads2k · 2017-02-22T17:56:13Z

/release-note-none

liggitt · 2017-02-22T21:32:20Z

keys and contents of the configmap LGTM

sttts · 2017-02-23T05:55:09Z

cmd/kube-apiserver/app/server.go

@@ -314,9 +315,26 @@ func Run(s *options.ServerRunOptions) error {
 		return err
 	}

+	clientCA, err := readCAIfPresent(s.Authentication.ClientCert.ClientCA)


will this throw some kind of NotFound error? It should.

sttts · 2017-02-23T05:56:34Z

cmd/kube-apiserver/app/server.go

@@ -354,6 +372,13 @@ func Run(s *options.ServerRunOptions) error {
 	return nil
 }

+func readCAIfPresent(file string) ([]byte, error) {


IfPresent = len != 0. Unexpected. Maybe readCAorNil would be better.

deads2k · 2017-02-23T14:28:03Z

@k8s-bot unit test this
@k8s-bot cvm gce e2e test this

deads2k · 2017-02-25T00:13:02Z

@k8s-bot non-cri e2e test this
@k8s-bot kubemark e2e test this

k8s-github-robot · 2017-02-26T17:32:44Z

Automatic merge from submit-queue

@sttts

Automatic merge from submit-queue Add namespaced role to inspect particular configmap for delegated authentication Builds on #41814 and #41922 (those are already lgtm'ed) with the ultimate goal of making an extension API server zero-config for "normal" authentication cases. This part creates a namespace role in `kube-system` that can *only* look the configmap which gives the delegated authentication check. When a cluster-admin grants the SA running the extension API server the power to run delegated authentication checks, he should also bind this role in this namespace. @sttts Should we add a flag to aggregated API servers to indicate they want to look this up so they can crashloop on startup? The alternative is sometimes having it and sometimes not. I guess we could try to key on explicit "disable front-proxy" which may make more sense. @kubernetes/sig-api-machinery-misc @ncdc I spoke to @liggitt about this before he left and he was ok in concept. Can you take a look at the details?

deads2k assigned lavalamp and liggitt Feb 21, 2017

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Feb 21, 2017

k8s-github-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. release-note-label-needed labels Feb 21, 2017

deads2k force-pushed the agg-06-cas branch from f879695 to 579f723 Compare February 21, 2017 16:27

liggitt reviewed Feb 21, 2017

View reviewed changes

deads2k added this to the v1.6 milestone Feb 22, 2017

deads2k force-pushed the agg-06-cas branch from 579f723 to fe49378 Compare February 22, 2017 17:55

k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. and removed release-note-label-needed labels Feb 22, 2017

deads2k force-pushed the agg-06-cas branch from fe49378 to 873b156 Compare February 22, 2017 18:40

k8s-github-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 22, 2017

sttts reviewed Feb 23, 2017

View reviewed changes

deads2k force-pushed the agg-06-cas branch from 873b156 to 268add9 Compare February 23, 2017 13:07

deads2k added approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Feb 23, 2017

k8s-github-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Feb 23, 2017

deads2k mentioned this pull request Feb 23, 2017

Add namespaced role to inspect particular configmap for delegated authentication #41982

Merged

add client-ca to configmap in kube-public

4a06b69

deads2k force-pushed the agg-06-cas branch from 268add9 to 4a06b69 Compare February 24, 2017 19:51

k8s-github-robot merged commit 1519422 into kubernetes:master Feb 26, 2017

deads2k deleted the agg-06-cas branch March 7, 2017 20:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add client-ca to configmap in kube-public #41814

add client-ca to configmap in kube-public #41814

deads2k commented Feb 21, 2017 •

edited

k8s-github-robot commented Feb 21, 2017

k8s-reviewable commented Feb 21, 2017

liggitt Feb 21, 2017

liggitt Feb 21, 2017

deads2k Feb 21, 2017

deads2k Feb 21, 2017

liggitt Feb 21, 2017

deads2k Feb 21, 2017

liggitt Feb 21, 2017

deads2k Feb 21, 2017

deads2k Feb 21, 2017

deads2k commented Feb 22, 2017

deads2k commented Feb 22, 2017

liggitt commented Feb 22, 2017

sttts Feb 23, 2017

sttts Feb 23, 2017

deads2k Feb 23, 2017

deads2k commented Feb 23, 2017

deads2k commented Feb 25, 2017

k8s-github-robot commented Feb 26, 2017

add client-ca to configmap in kube-public #41814

add client-ca to configmap in kube-public #41814

Conversation

deads2k commented Feb 21, 2017 • edited

k8s-github-robot commented Feb 21, 2017

k8s-reviewable commented Feb 21, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deads2k commented Feb 22, 2017

deads2k commented Feb 22, 2017

liggitt commented Feb 22, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deads2k commented Feb 23, 2017

deads2k commented Feb 25, 2017

k8s-github-robot commented Feb 26, 2017

deads2k commented Feb 21, 2017 •

edited