diff --git a/design/20210209.certificates.k8s.io-adoption.md b/design/20210209.certificates.k8s.io-adoption.md index 47e52d3d9..fd5901b4e 100644 --- a/design/20210209.certificates.k8s.io-adoption.md +++ b/design/20210209.certificates.k8s.io-adoption.md @@ -9,7 +9,7 @@ approvers: - @jetstack/team-cert-manager editor: "@joshvanl" creation-date: 2021-02-09 -last-updated: 2021-02-09 +last-updated: 2021-02-17 status: provisional --- @@ -20,14 +20,20 @@ status: provisional - [Summary](#summary) - [Motivation](#motivation) - - [Goals](#goals) - - [Non-Goals](#non-goals) -- [Reasons For Approach](#reasons-for-approach) + * [Goals](#goals) + * [Non-Goals](#non-goals) +- [Reasons for Approach](#reasons-for-approach) + * [Read all access to `CertificateSigningRequests`](#read-all-access-to-certificatesigningrequests) + * [Namespaced issuers referenced by `CertificateSigningRequest`](#namespaced-issuers-referenced-by-certificatesigningrequest) + * [Duration Field](#duration-field) + * [CA Field](#ca-field) + * [External Issuers](#external-issuers) - [Proposal](#proposal) - - [Signers](#signers) - - [API Changes](#api-changes) - - [Upgrading](#upgrading) - - [Risks and Mitigations](#risks-and-mitigations) + * [Signers](#signers) + * [Conditions](#conditions) + * [API Changes](#api-changes) + * [Upgrading](#upgrading) + * [Graduation](#graduation) ## Summary @@ -48,83 +54,156 @@ the core Kubernetes resource type. This gives cert-manager the ability to integrate with components that it otherwise wouldn't have, without changes to the third-party project. +Having the "request" resource managed inside the API server has better a +security profile (i.e. `UserInfo` fields are managed by the API server rather +than a configurable webhook, +[see](https://github.com/jetstack/cert-manager/pull/3630)). + +Consumers of this API do not require cert-manager as a hard dependency. +cert-manager can cause issues with order of operation scenarios during +installation or upgrades. For example, the `CertificateRequest` resource needs +to be installed and the webhook ready for consumers/clients of this API to make +requests. + +Support for the `CertificateSigningRequest` resource in cert-manager is a +stepping stone to migrating to this resource in favour of the +`CertificateRequest` resource, if the project does indeed want to migrate. ### Goals -- Add `CertificateSigningRequest` signer controllers to cert-manager for all +* Add `CertificateSigningRequest` signer controllers to cert-manager for all `Issuer` types +* Pave a way for complete adoption for the `CertificateSigningRequest` + resource in future if wanted ### Non-Goals -- Remove the `CertificateRequest` resource in favour of - `CertificateSigningRequest` -- Change the `Certificate` controllers behaviour -- Support `CertificateSigningRequest` pre v1 -- Make changes to upstream Kubernetes to implement controllers in cert-manager +* Remove the `CertificateRequest` resource +--- ## Reasons for Approach -Below are a summary of properties of the `CertificateSigningRequest`, and -explanations as to why other approaches to integrate the resource with -cert-manager aren't possible (i.e. completely switching the `CertificateRequest` -resource to `CertificateSigningRequest`: +Bellow are a summary of properties of the `CertificateSigningRequest` which +cause issues or caveats which need to be addressed when adding support for the +resource. -1. `CertificateSigningRequests` are cluster scoped resources. This means that - the concept of namespaced `Issuers` doesn't fit with this resource, and any - attempt to integrate it would open up the ability of privilege escalation - (i.e. referencing Secrets in another tenants Namespace). Switching to - `CertificateSigningRequests` would mean the removal of the `Issuer` resource. +### Read all access to `CertificateSigningRequests` -1. As `CertificateSigningRequests` are cluster scoped, they do not provide the - same kind of isolation that `CertificateRequests` provide- preventing tenants - reading others requests, namespace specific issuer authentication (e.g. Vault - Kubernetes auth), namespacing cert-manager. +`CertificateSigningRequests` are cluster scoped resources, whereas +`CertificateRequests` are not. This means that in order for users to have the +same debugging and usage capabilities of the `CertificateRequest`, users will +have the ability to list all requests in the cluster, rather than being scoped to +specific namespaces. -1. `CertificateSigningRequests` do not include a duration field. This means that - any attempt to "proxy", or copy `CertificateRequests`, would result in a - regression of this feature. A feature which many users of cert-manager rely - on. +A solution to this problem could be given to users by providing examples or +guides to external tooling (e.g. [Open Policy Agent +(OPA)](https://github.com/open-policy-agent/opa), whereby cluster administrators +can limit users access to certain `CertificateSigningRequests` based on label +selectors. -1. cert-manager has a large number of [external - issuers](https://cert-manager.io/docs/configuration/external/) created by - the community that operate over the `CertificateRequest` resource. No longer - supporting these resources would require these authors to completely re-write - these issuers to continue to be supported by cert-manager. -Supporting both resource types, but making `CertificateRequests` the "priority" -resource (i.e. the resource created by the `Certificates` controller), means -that we don't have any regression in the features cert-manager offers, whilst -maintaining the same security properties. +### Namespaced issuers referenced by `CertificateSigningRequest` + +`CertificateSigningRequests` are cluster scoped resources, whereas +`CertificateRequests` are not. cert-manager has the concept of of namespaced +issuers that may only be referenced by `CertificateRequests` that reside in +the same namespace. Since `CertificateSigningRequests` can reference any +namespaced `Issuer`, there must be some mechanism to prevent privilege +escalation. + +cert-manager will enforce an +[RBAC](https://kubernetes.io/docs/reference/access-authn-authz/rbac/) noun and +verb whereby the requester must have this role bound to them, in order for the +`CertificateSigningRequest` referencing a namespaced `Issuer`, be approved by the +cert-manager controller. See [here](#conditions). + +This will be done via a +[`SubjectAccessReview`](https://github.com/kubernetes/api/blob/4a626d306b987a4096cf0784ec01af1be2f6d67f/authorization/v1/types.go#L52) +against the `UserInfo` fields on the `CertificateSigningRequest`. The required +role must be of `certificates.k8s.io/CertificateSigningRequest` with the verb +`create` within the target namespace. + +Since `Certificates` are also namespaced resources, cert-manager validates that +created `Certificate` resources may only reference `Issuers` in the same +namespace, or `ClusterIssuers`. Any resulting `CertificateSigningRequests` will +reference `Issuers` in the same namespace. + +### Duration Field + +`CertificateSigningRequests` do not include a `duration` field. To have parity +with the `CertificateRequest` resource, the duration field will be moved to an +annotation as `cert-manager.io/duration` which includes a [Go time duration +string](https://golang.org/pkg/time/#Duration.String). This annotation may not +be changed after creation. + +### CA Field + +`CertificateSigningRequests` do not include a `ca` field. To have parity with +the `CertificateRequest` resource, the `ca` field will be moved to an annotation +as `cert-manager.io/ca`. This annotation value contains the base 64 PEM encoded +CA certificate (if one was returned by the signer). This annotation may not be +changed after creation. + +### External Issuers + +All current [external +issuers](https://cert-manager.io/docs/configuration/external/) are built for the +`CertificateRequest` resource. The project should continue to support this +resource indefinitely, and provide an example project for creating external +signers if and when the migration to `CertificateSigningRequests` occurs. + + +--- + ## Proposal +cert-manager will support both `CertificateRequest` and +`CertificateSigningRequest` types until and if the project completely migrates to +the `CertificateSigningRequest` resource. The `Certificates` controller will +continue to create `CertificateRequest` resources, unless a flag is changed on +the cert-manager controller. This default can be switched in future. + +The cert-manager controller will not enable the `CertificateSigningRequest` +controllers unless the resource is available in the running cluster. This can be +checked via the [discovery endpoint of the API +server](https://github.com/kubernetes/client-go/blob/7279fc64d8478bd5f9d38122143b5e6294f06e75/discovery/discovery_client.go#L55). +If the resource is not available, the `CertificateSigningRequest` controllers +should gracefully never start. If the Kubernetes API server were to be upgraded +to a version that does support this resource, cert-manager will need to be +restarted to make use of these controllers. This is acceptable- worker nodes are +typically always restarted during a cluster upgrade. + + Instead of the concept of an `IssuerRef` for `CertificateRequests`, `CertificateSigningRequests` have the concept of a `SignerName`. Since -`CertificateSigningRequests` are cluster scoped resources, the signer name can -be directly mapped to the `ClusterIssuer` resource. `ClusterIssuers` will be -referenced in the following format: +`CertificateSigningRequests` are cluster scoped resources, the signer name must +include the namespace, if the referenced `Issuer` is namespaced. The signer name +for cert-manager signers will be prefixed with `cert-manager.io` to prevent +conflicts with other external signer projects. ```yaml - signerName: cert-manager.io/${ClusterIssuer} + # Namespaced issuer reference + signerName: cert-manager.io/. + + # Cluster scoped issuer reference + signerName: cert-manager.io/ ``` -Each `CertificateSigningRequest` controller will behave in the same way as the -existing `CertificateRequest` resource, by getting the referenced -`ClusterIssuer`, and attempting to sign. If the `ClusterIssuer` type is not -managed by this controller, do nothing, else sign. +Using the same approach of referencing by _just_ name, rather than issuer type +(e.g. CA, Vault etc.), keeps the behaviour of this resource in line with +`CertificateRequests` for end users. -Each `CertificateSigningRequest` controller will automatically set the -`CertificateSigningRequest` `RequestCondition` to `Approved` if the request is -for their managed `ClusterIssuer` type. If the controller manages that -`ClusterIssuer` type, but the `ClusterIssuer` doesn't exist or is not ready, the -controller will set the condition to `Pending`. If signing fails during -processing, it will set the `CertificateSigningRequest` condition to `Failed`. +Each `CertificateSigningRequest` controller will behave in the same way as the +existing `CertificateRequest` resource, by getting the referenced signer, and +attempting to sign. If the issuer type is not managed by this controller, do +nothing, else sign. ### Signers -Some special cases for some `ClusterIssuers` that need to be addressed: +Some special cases for some `[Cluster]Issuers` that need to be addressed: - SelfSigned: Makes use of annotations, and so these annotations should also be present on `CertificateSigningRequests`. @@ -132,26 +211,82 @@ Some special cases for some `ClusterIssuers` that need to be addressed: - Venafi: Makes use of annotations, and so these annotations should also be present on `CertificateSigningRequests`. +- ACME: Makes use of annotations, and so these annotations should also be + present on `CertificateSigningRequests`. + - ACME: The ACME controller creates sub-resources (`Orders`). Since - `CertificateSigningRequests` are cluster scoped resources, we should create - `Orders` in the `Cluster Resource Namespace` (default `cert-manager`). + `CertificateSigningRequests` are cluster scoped resources, when referencing + a `ClusterIssuer` we should create `Orders` in the `Cluster Resource + Namespace` (default `cert-manager`), else the namespace of the referenced + `Issuer`. + + +### Conditions + +The `CertificateSigningRequest` has [well-known condition +types](https://github.com/kubernetes/api/blob/4a626d306b987a4096cf0784ec01af1be2f6d67f/certificates/v1/types.go#L222) +of `Approved`, `Denied`, and `Failed`. All `CertificateSigningRequest` signer +controllers should not begin computation of the request until the resource has +the `Approved` condition set to `True`. + +In the absence of further policy, the `Approved` condition is always set with +`True`, _unless_, the `CertificateSigningRequest` references a namespaced +`Issuer` and the requester does not have create permissions for +`certificates.k8s.io/CertificateSigningRequests` in that namespace. In the case +of this namespace validation failing, the `CertificateSigningRequest` `Denied` +condition should be set to `True`, and no signer controllers should process the +request. + +If a signer fails when processing a `CertificateSigningRequest`, the well-known +`Failed` condition should be set to `True`. No signer controllers should process +the request again. + +If the referenced issuer resource doesn't exist or is not in a ready state, the +corresponding `CertificateSigningRequest` controller should set the `Pending` +condition to `True`. + +When the request has been successfully signed, the [cert-manager `Ready` +condition](https://github.com/jetstack/cert-manager/blob/969b678f330c68a6429b7a71b271761c59651a85/pkg/apis/certmanager/v1/types_certificaterequest.go#L139) +should be set to `True` on the resource, to match the same behaviour as +`CertificateRequests`. ### API Changes -No API changes are required to support this resource. +* The `cert-manager.io/duration` annotation on `CertificateSigningRequests` + denotes the requested duration as a Go duration string. + +* The `cert-manager.io/ca` annotation on `CertificateSigningRequests` contains + the returned base 64 PEM encoded CA certificate from the signer. + +* The requester of a `CertificateSigningRequest` must have `create` permissions + for `certificates.k8s.io/CertificateSigningRequests` in the same namespace + the referenced issuer resides. + +* The [Approval + Condition](https://github.com/kubernetes/api/blob/4a626d306b987a4096cf0784ec01af1be2f6d67f/certificates/v1/types.go#L222) + will be replicated on the `CertificateRequest` resource. This condition will + _always_ be set to true, in the absence of policy. All `CertificateRequest` + controllers must wait until the `Approved` condition is set to `True` before + beginning computation. ### Upgrading -No effect to upgrades as only additional controllers added. No API changes. +No effect to upgrades as only additional controllers added. No CRD API changes. +### Graduation -### Risks and Mitigations +Below is each level of graduation for support of `CertificateSigningRequest` in +cert-manager. + +1. Create signer implementations for each cert-manager.io issuers. These + controllers are active in a default cert-manager installation, though + `CertificateRequests` are the resource created from `Certificate` resources. + +1. [TBD] Swap `CertificateRequests` for `CertificateSigningRequests` as the resource + created from `Certificates`. + +1. [TBD] Remove the `CertificateRequest` resource from the cert-manager project, + in favour of the `CertificateSigningRequest` resource. -The controllers need to be aware if the `certificates.k8s.io/v1` -`CertificateSigningRequest` resource exists (e.g. pre v1.19 cluster). If they -don't exist, they should gracefully never start. If the Kubernetes API server -were to be upgraded to a version that does support this resource, cert-manager -will need to be restarted to make use of these controllers. This is acceptable- -worker nodes are typically always restarted during a cluster upgrade.