WIP: first draft

proposals/2027_opt_out_of_self_signed_certificate_authorities/README.md

@@ -0,0 +1,174 @@
+---
+title: "Opt-Out of self-signed CAs (and their certs)"
+authors: [ "@joshuatcasey" ]
+status: "proposed"
+sponsor: [ "@cfryanr", "@ashish-amarnath" ]
+approval_date: "TBD"
+---
+
+*Disclaimer*: Proposals are point-in-time designs and decisions.
+Once approved and implemented, they become historical documents.
+If you are reading an old proposal, please be aware that the
+features described herein might have continued to evolve since.
+
+# Opt-Out of self-signed CAs (and their certs)
+
+## Problem Statement
+
+Pinniped's Concierge, Supervisor, and Local-User-Authenticator today require the use of self-signed certificate
+authorities (CAs). These self-signed CAs are used for a variety of purposes, but generally are used to sign leaf
+certificates used to serve TLS or sign leaf certificates for client authentication (mTLS).
+
+Using self-signed certificates can mean that custom CA bundles need to be installed on client machines so that clients
+can perform TLS verification with a Pinniped endpoint. As of Pinniped `v0.32.0`, any endpoints that are meant to be
+visited by a client machine can use external certificates. In particular, each Supervisor `FederationDomain` serves
+OIDC discovery and web endpoints that are meant to be visited by client machines, and each Concierge `ImpersonationProxy`
+is an endpoint meant to be visited by client machines. This proposal will not change configuration for those endpoints.
+
+However, Pinniped does expose various endpoints for Kubernetes-internal use, such as to serve a `/healthz` endpoint
+or an endpoint that backs an `APIService` (only the Kubernetes API Service will call an `APIService`), as well as
+create certificates for client authentication (when the impersonation proxy is enabled). Today, Pinniped will generate
+its own CA certificate and any leaf certificates that it needs.
+
+Some customers are unable to use self-signed CAs or certificates due to regulatory factors or some other security
+specification that governs their environments. Other customers may want to have visibility into what CAs or leaf
+certificates are used in their cluster, and relying on Pinniped to generate its own CAs and leaf certificates obscures
+that visibility. Still other customers may want to have complete control over when certificates are rotated, even if
+those certificates do not need to be trusted by client machines.
+
+## Terminology / Concepts
+
+* Self-signed certificate: A certificate not signed by a publicly-trusted CA authority. This is usually a CA certificate,
+but could be a leaf certificate.
+* Certificate Authority (CA): A certificate (with `isCA: true`) used to issue intermediate certificates or leaf
+certificates, such as certificates for serving TLS or certificates for client authentication (mTLS).
+* Generated certificate: A certificate generated by Pinniped, from its own CAs. By definition these generated certificates
+are not themselves self-signed, since they are always signed by a CA (even if that CA is self-signed).
+* External certificate: A certificate provided by something outside of Pinniped, such as `cert-manager`. Usually this
+is a leaf certificate (to serve TLS, for example), but could be a CA certificate.
+* Secret type `kubernetes.io/tls`: Secrets that must have keys `tls.crt` and `tls.key` that can contain either a CA,
+intermediate, or leaf certificate. See https://kubernetes.io/docs/concepts/configuration/secret/#tls-secrets.
+* `cert-manager`: A [tool](https://cert-manager.io/) to manage certificates used within a cluster. Generally outputs
+secrets of type `kubernetes.io/tls`, and may populate an additional data field `ca.crt` with a higher-level certificate
+if available.
+* CA Bundle: A collection of certificates used to verify TLS or some other certificate signature. Usually should only
+contain CA or intermediate certificates (not leaf certificates that should be rotated frequently), and should never
+contain private keys.
+
+## Proposal
+
+Pinniped should accept valid, properly-formatted CAs and certificates in secrets of type `kubernetes.io/tls` in those
+places where it currently generates its own CAs or certificates. Pinniped will perform validations to ensure that the
+CAs and certificates are valid and unexpired, but will not perform lifecycle management (such as rotation) on those
+CAs and certificates. Pinniped will also verify that the certificates it is presented are useable for purpose (such as
+a CA certificate vs a leaf certificate) and have the appropriate key usage (such as `server signing`).
+
+Here is a list of all known locations where Pinniped will generate its own CA or certificate:
+
+### Supervisor
+* Bootstrap CA certificate and leaf certificate for serving TLS. At the moment this appears to be held only in memory,
+but Supervisor ignore this CA and leaf certificate if a secret (default name `pinniped-supervisor`,
+type `kubernetes.io/tls`) exists. We propose that Supervisor no longer generates its own CA and leaf certificate if the
+secret is found. TODO: verify the claims above, also see https://pinniped.dev/docs/howto/supervisor/configure-supervisor/#configuring-tls-for-the-supervisor-oidc-endpoints
+
+_Note that this could require significant effort since it changes an extremely fundamental aspect of how the Supervisor works.
+In implementation, we may decide that this is not worth it, since customers can provide a secret for the default TLS serving cert._
+### Concierge
+
+Go see https://github.com/vmware-tanzu/pinniped/issues/1238#issuecomment-2261503118.
+
+### Local-User-Authenticator
+_Note that local-user-authenticator is not meant for production use. It is included in this proposal simply because
+its code overlaps with Supervisor and Concierge, and excluding it would require significant effort._
+
+* CA certificate and leaf certificate for serving TLS, currently held in secret `local-user-authenticator-tls-serving-certificate`
+in its installed namespace, with type `Opaque`. This secret holds both a CA and a TLS serving cert (and their private keys).
+This secret would be treated in the same way as the proposed solution from https://github.com/vmware-tanzu/pinniped/issues/1238#issuecomment-2261503118.
+
+### Goals and Non-goals
+
+Goal:
+* Pinniped should provide enough configuration options for Pinniped administrators to completely avoid using self-signed CAs
+* Pinniped should generally expect external certificates to be leaf certificates (to serve TLS, for example). Pinniped
+will need a CA certificate when the impersonation proxy is enabled, in order to issue client certificates for mTLS.
+* Pinniped should rely on Kubernetes standards such as secrets with type `kubernetes.io/tls`, instead of coupling itself
+to any specific tool such as `cert-manager`. However, Pinniped will defer to a `ca.crt` field for CA bundles if that
+field is available.
+
+Non-goals:
+* It is not a goal of this proposal to remove Pinniped's self-signed CAs (and require the user to configure all CAs and
+certificates). Pinniped should continue to generate any necessary CAs and certificates that are not externally provided.
+* It is not a goal of this proposal to change how the Supervisor signs the ID tokens that it issues. Those tokens are
+signed by ECDSA private keys specific to each `FederationDomain`. Clients that need to verify tokens issued by the
+Pinniped Supervisor can trivially obtain the public keys through OIDC discovery, not through PKI distribution.
+* It is not a goal of this proposal to change configuration for `FederationDomain` or `ImpersonationProxy` resources
+that already accept external certificates as of `v0.32.0`.
+
+#### API Changes
+
+None. The `FederationDomain` and `ImpersonationProxy` custom resources will not be modified as a result of this proposal.
+
+#### Configuration Changes
+
+We may need to introduce a configuration variable for each of Supervisor, Concierge, and Local-User-Authenticator to
+indicate to each of those services that certificates will be managed externally. This is to prevent any potential
+race condition at startup if a Pinniped service boots up and detects that it needs to create its own CAs and certificates
+before external tooling has provided them. This also implies that Pinniped would not need to manage the lifecycle of
+the listed secrets (such as creation and rotation of the certificates).
+
+#### Upgrades
+
+The upgrade process should be seamless for users that will continue to use Pinniped's self-signed CAs.
+Some downtime may be required for users that choose to opt in to externally-provided certificates, since old versions
+of Pinniped do not anticipate external management of the secrets and may attempt to manage their lifecycle.
+
+#### Tests
+
+Unit and integration tests will be written as necessary to achieve 100% test coverage.
+Integration tests will inspect the certificates presented by Pinniped to ensure that Pinniped is using generated or
+externally provided certificates as configured.
+
+#### New Dependencies
+
+No new dependencies within Pinniped. May require using external tools or tooling to manage certificates.
+
+#### Performance Considerations
+
+None.
+
+#### Observability Considerations
+
+None.
+
+#### Security Considerations
+
+None.
+
+#### Usability Considerations
+
+None, these changes are not user-facing.
+
+#### Documentation Considerations
+
+Pinniped configuration options are only lightly documented outside their own comments.
+A blog post to accompany the release is probably the best external documentation for this feature.
+
+### Other Approaches Considered
+
+Leave as-is.
+
+## Open Questions
+
+TBD
+
+## Answered Questions
+
+TBD
+
+## Implementation Plan
+
+TBD
+
+## Implementation PRs
+
+* TBD

proposals/2027_opt_out_of_self_signed_certificate_authorities/fail_test.go

@@ -0,0 +1,16 @@
+// Copyright 2024 the Pinniped contributors. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package fail_test
+
+// TODO: remove this file before merging.
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func Test(t *testing.T) {
+	require.Fail(t, "fail this test so that unit tests fail and integration tests do not run in CI")
+}