add multiple signers to CSRs

[deads2k]

Adds the ability for CSRs to have different signers while keep backward compatibility with existing consumers of v1beta1. This is on the path to a GA API.

@kubernetes/sig-auth-api-reviews

/priority important-soon
/assign @liggitt

[enj]

I like the overall idea of signerName.

I think including the trust information in status would make it easier to consume this API.

[enj]

cc @munnerz

[deads2k]

I like the overall idea of signerName.

I think including the trust information in status would make it easier to consume this API.

I don't think that providing trust information (currently not done) is required for properly subdividing the signers we have. This doesn't preclude adding such a feature, but trust distribution is distinct from signing overall.

[mikedanese]

Can we spec interaction between approvers and signers in this change? Should we think about scoping authorization for an approver to a specific signer? Should approvers be 1:1 with signers? Should signers be 1:1 with CAs? Why don't we have a single signer for kubernetes.io/kubelet vs breaking it up?

[enj]

I like the overall idea of signerName.
I think including the trust information in status would make it easier to consume this API.

I don't think that providing trust information (currently not done) is required for properly subdividing the signers we have. This doesn't preclude adding such a feature, but trust distribution is distinct from signing overall.

Echoing Jordan's comment - as long we tackle trust distribution before v1 I am happy keeping the signer and trust bits as separate efforts.

[fejta-bot]

This PR may require API review.

If so, when the changes are ready, complete the pre-review checklist and request an API review.

Status of requested reviews is tracked in the API Review project.

[deads2k]

Why don't we have a single signer for kubernetes.io/kubelet vs breaking it up?

@mikedanese

They are trusted by the kube-apiserver for different purposes. A cluster-admin could use the same CA for each, but it should also be possible to selectively narrow your signers to their respective domains. Trusting a CA bundle to terminate an outbound connection is distinctly different than trusting a CA bundle to assert identity on an inbound connection.

[deads2k]

Can we spec interaction between approvers and signers in this change? Should we think about scoping authorization for an approver to a specific signer? Should approvers be 1:1 with signers? Should signers be 1:1 with CAs?

I don't see this as changing the role of approvers or changing the existing relationship between approvers and signers.

I would expect that generally signers would be signing with a single key at a time.

[answer1991]

@deads2k @liggitt

What's the relationship between signers' CA and the CA of kube-apiserver's flag --client-ca-file?

We had divide the CA of kube-apiserver-client and kubelet-client, but --client-ca-file is StringVar, and we had to merge two CA's content into --client-ca-file single file. Should we change it to be StringSliceVar?

https://github.com/kubernetes/kubernetes/blob/f3ebd95f3aac60db977a0f057032297ea26596f5/staging/src/k8s.io/apiserver/pkg/server/options/authentication.go#L153-L156

[munnerz]

For reference, in cert-manager we have added the following fields to our equivalent of the CSR type: https://cert-manager.io/docs/reference/api-docs/#cert-manager.io/v1alpha2.CertificateRequestSpec

(duration, issuerRef, isCA and usages).

We have additionally added an optional annotation, cert-manager.io/private-key-secret-name in order to allow our 'self signing' issuer to function: https://github.com/jetstack/cert-manager/blob/master/design/20190708.certificate-request-crd.md#certificaterequest-annotations

There has also been a request to extend the 'status reasons' on our CertificateRequest resource to include some form of "FailedNoRetry" parameter, to indicate a particular request for a certificate failed because the parameters on the request itself are not acceptable for the given signer: cert-manager/cert-manager#2289 - this signals to other controllers that may create CertificateRequest resources that they should inform the user to update their request rather than blindly retry over and over.

[liggitt]

What's the relationship between signers' CA and the CA of kube-apiserver's flag --client-ca-file?

Are you asking what CA would sign kubernetes.io/kube-apiserver-client and kubernetes.io/kubelet-client certificates? Both of those would issue client certificates recognized by the kube-apiserver. Likely, they would both sign with the same CA. The distinction between them is that kubernetes.io/kube-apiserver-client could request arbitrary subjects and kubernetes.io/kubelet-client would be limited to subjects matching the node client structure (O=system:nodes,CN=system:node:<name>)

We had divide the CA of kube-apiserver-client and kubelet-client, but --client-ca-file is StringVar, and we had to merge two CA's content into --client-ca-file single file. Should we change it to be StringSliceVar?

No, --client-ca-file can contain multiple CA certificates if needed (this is already supported for CA rotation, etc)

[answer1991]

@liggitt

Are you asking what CA would sign kubernetes.io/kube-apiserver-client and kubernetes.io/kubelet-client certificates? Both of those would issue client certificates recognized by the kube-apiserver. Likely, they would both sign with the same CA.

I think they should be signed with different CA/Key pair.

In my company, we use different CA/Key pair to sign cert(kubeconfig) for developers based on which team the developer working for, which means every team has a CA/Key pair to sign cert(kubeconfig), and the Kubernetes admin should merge different CA to a single --client-ca-file file. When one team's CA/Key leak or loss, we just remove this team's CA from --client-ca-file file and add a new one, then re-sign new certs for developers, which can reduce the loss when we leak or loss CA/Key pair.

That's why I suggest --client-ca-file should be StringSliceVar :-P

[liggitt]

That's why I suggest --client-ca-file should be StringSliceVar :-P

It is simpler to manage a process that aggregates all your client cert signers into a single file and replace the file passed to --client-ca-file than to modify the kube-apiserver invocation and restart the process. I also think that we support dynamically reloading the --client-ca-file without a kube-apiserver restart as of 1.17 (@deads2k can confirm), which is a benefit of passing a single file containing all the CAs instead of N files each containing a single CA

[munnerz]

I've opened kubernetes/kubernetes#86476 to add the API fields, validation and defaulting behaviour described in this KEP. The signer and approver changes can be added later, unless you want to include them all in one PR 😄

[mikedanese]

cc @awly @sambdavidson

[liggitt]

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, liggitt

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• keps/sig-auth/OWNERS [deads2k,liggitt]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[munnerz]

nit: I think this should be kube-apiserver-client-kubelet now?

[liggitt]

fixed in https://github.com/kubernetes/enhancements/pull/1485/commits

[mikedanese]

Why is this implemented in admission control vs approver or signer policy?

[munnerz]

@deads2k have you got any input here? We currently have this implemented as an admission plugin in kubernetes/kubernetes#86933, but is it better we move this to the approval flow?