Proof of concept for using Kube CSR API in TokenCredentialRequest

deploy/concierge/rbac.yaml

@@ -18,6 +18,13 @@ rules:
   - apiGroups: [ apiregistration.k8s.io ]
     resources: [ apiservices ]
     verbs: [ get, list, patch, update, watch ]
+  - apiGroups: [ certificates.k8s.io ]
+    resources: [ certificatesigningrequests, certificatesigningrequests/approval ]
+    verbs: [ create, get, list, patch, update, watch ]
+  - apiGroups: [ certificates.k8s.io ]
+    resources: [ signers ]
+    resourceNames: [ kubernetes.io/kube-apiserver-client ]
+    verbs: [ approve ]
   - apiGroups: [ admissionregistration.k8s.io ]
     resources: [ validatingwebhookconfigurations, mutatingwebhookconfigurations ]
     verbs: [ get, list, watch ]

internal/concierge/apiserver/apiserver.go

@@ -14,6 +14,7 @@ import (
 	"k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/apiserver/pkg/registry/rest"
 	genericapiserver "k8s.io/apiserver/pkg/server"
+	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/pkg/version"
 
 	"go.pinniped.dev/internal/controllerinit"
@@ -29,13 +30,14 @@ type Config struct {
 }
 
 type ExtraConfig struct {
-	Authenticator                 credentialrequest.TokenCredentialRequestAuthenticator
+	Authenticator                   credentialrequest.TokenCredentialRequestAuthenticator
-	Issuer                        issuer.ClientCertIssuer
+	Issuer                          issuer.ClientCertIssuer
-	BuildControllersPostStartHook controllerinit.RunnerBuilder
+	BuildControllersPostStartHook   controllerinit.RunnerBuilder
-	Scheme                        *runtime.Scheme
+	Scheme                          *runtime.Scheme
-	NegotiatedSerializer          runtime.NegotiatedSerializer
+	NegotiatedSerializer            runtime.NegotiatedSerializer
-	LoginConciergeGroupVersion    schema.GroupVersion
+	LoginConciergeGroupVersion      schema.GroupVersion
-	IdentityConciergeGroupVersion schema.GroupVersion
+	IdentityConciergeGroupVersion   schema.GroupVersion
+	KubeClientWithoutLeaderElection kubernetes.Interface
 }
 
 type PinnipedServer struct {
@@ -80,7 +82,7 @@ func (c completedConfig) New() (*PinnipedServer, error) {
 	for _, f := range []func() (schema.GroupVersionResource, rest.Storage){
 		func() (schema.GroupVersionResource, rest.Storage) {
 			tokenCredReqGVR := c.ExtraConfig.LoginConciergeGroupVersion.WithResource("tokencredentialrequests")
-			tokenCredStorage := credentialrequest.NewREST(c.ExtraConfig.Authenticator, c.ExtraConfig.Issuer, tokenCredReqGVR.GroupResource())
+			tokenCredStorage := credentialrequest.NewREST(c.ExtraConfig.Authenticator, c.ExtraConfig.Issuer, c.ExtraConfig.KubeClientWithoutLeaderElection, tokenCredReqGVR.GroupResource())
 			return tokenCredReqGVR, tokenCredStorage
 		},
 		func() (schema.GroupVersionResource, rest.Storage) {

internal/concierge/server/server.go

@@ -17,6 +17,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/serializer"
 	genericapiserver "k8s.io/apiserver/pkg/server"
 	genericoptions "k8s.io/apiserver/pkg/server/options"
+	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/pkg/version"
 	"k8s.io/client-go/rest"
 	"k8s.io/component-base/logs"
@@ -162,8 +163,16 @@ func (a *App) runServer(ctx context.Context) error {
 		dynamiccertauthority.New(impersonationProxySigningCertProvider), // fallback to our internal CA if we need to
 	}
 
+	// Make a kube client without leader election to allow the rest endpoints to use it selectively
+	// only where it makes sense. This could use the deploymentRef middleware, but we don't need it yet.
+	clientWithoutLeaderElection, err := kubeclient.New()
+	if err != nil {
+		return fmt.Errorf("could not create clientWithoutLeaderElection for the rest handlers: %w", err)
+	}
+
 	// Get the aggregated API server config.
 	aggregatedAPIServerConfig, err := getAggregatedAPIServerConfig(
+		clientWithoutLeaderElection.Kubernetes,
 		dynamicServingCertProvider,
 		authenticators,
 		certIssuer,
@@ -190,6 +199,7 @@ func (a *App) runServer(ctx context.Context) error {
 
 // Create a configuration for the aggregated API server.
 func getAggregatedAPIServerConfig(
+	kubeClientWithoutLeaderElection kubernetes.Interface,
 	dynamicCertProvider dynamiccert.Private,
 	authenticator credentialrequest.TokenCredentialRequestAuthenticator,
 	issuer issuer.ClientCertIssuer,
@@ -238,13 +248,14 @@ func getAggregatedAPIServerConfig(
 	apiServerConfig := &apiserver.Config{
 		GenericConfig: serverConfig,
 		ExtraConfig: apiserver.ExtraConfig{
-			Authenticator:                 authenticator,
+			Authenticator:                   authenticator,
-			Issuer:                        issuer,
+			Issuer:                          issuer,
-			BuildControllersPostStartHook: buildControllers,
+			BuildControllersPostStartHook:   buildControllers,
-			Scheme:                        scheme,
+			Scheme:                          scheme,
-			NegotiatedSerializer:          codecs,
+			NegotiatedSerializer:            codecs,
-			LoginConciergeGroupVersion:    loginConciergeGroupVersion,
+			LoginConciergeGroupVersion:      loginConciergeGroupVersion,
-			IdentityConciergeGroupVersion: identityConciergeGroupVersion,
+			IdentityConciergeGroupVersion:   identityConciergeGroupVersion,
+			KubeClientWithoutLeaderElection: kubeClientWithoutLeaderElection,
 		},
 	}
 	return apiServerConfig, nil

internal/registry/credentialrequest/rest.go

@@ -6,9 +6,17 @@ package credentialrequest
 
 import (
 	"context"
+	"crypto/ecdsa"
+	"crypto/elliptic"
+	"crypto/rand"
+	"crypto/x509"
+	"crypto/x509/pkix"
+	"encoding/pem"
 	"fmt"
 	"time"
 
+	certificatesv1 "k8s.io/api/certificates/v1"
+	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metainternalversion "k8s.io/apimachinery/pkg/apis/meta/internalversion"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -18,6 +26,10 @@ import (
 	"k8s.io/apiserver/pkg/authentication/user"
 	genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
 	"k8s.io/apiserver/pkg/registry/rest"
+	"k8s.io/client-go/kubernetes"
+	"k8s.io/client-go/util/cert"
+	"k8s.io/client-go/util/certificate/csr"
+	"k8s.io/client-go/util/keyutil"
 	"k8s.io/utils/trace"
 
 	loginapi "go.pinniped.dev/generated/latest/apis/concierge/login"
@@ -31,18 +43,20 @@ type TokenCredentialRequestAuthenticator interface {
 	AuthenticateTokenCredentialRequest(ctx context.Context, req *loginapi.TokenCredentialRequest) (user.Info, error)
 }
 
-func NewREST(authenticator TokenCredentialRequestAuthenticator, issuer issuer.ClientCertIssuer, resource schema.GroupResource) *REST {
+func NewREST(authenticator TokenCredentialRequestAuthenticator, issuer issuer.ClientCertIssuer, kubeClientWithoutLeaderElection kubernetes.Interface, resource schema.GroupResource) *REST {
 	return &REST{
-		authenticator:  authenticator,
+		authenticator:                   authenticator,
-		issuer:         issuer,
+		issuer:                          issuer,
-		tableConvertor: rest.NewDefaultTableConvertor(resource),
+		tableConvertor:                  rest.NewDefaultTableConvertor(resource),
+		kubeClientWithoutLeaderElection: kubeClientWithoutLeaderElection,
 	}
 }
 
 type REST struct {
-	authenticator  TokenCredentialRequestAuthenticator
+	authenticator                   TokenCredentialRequestAuthenticator
-	issuer         issuer.ClientCertIssuer
+	issuer                          issuer.ClientCertIssuer
-	tableConvertor rest.TableConvertor
+	tableConvertor                  rest.TableConvertor
+	kubeClientWithoutLeaderElection kubernetes.Interface
 }
 
 // Assert that our *REST implements all the optional interfaces that we expect it to implement.
@@ -106,12 +120,19 @@ func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation
 		return failureResponse(), nil
 	}
 
-	// this timestamp should be returned from IssueClientCertPEM but this is a safe approximation
+	// By commenting out this code for the spike, we prevent the usual kube cert agent and impersonation proxy
-	expires := metav1.NewTime(time.Now().UTC().Add(clientCertificateTTL))
+	// strategies from getting involved in creating client certs. Instead, we will use the Kube CSR APIs below.
-	certPEM, keyPEM, err := r.issuer.IssueClientCertPEM(userInfo.GetName(), userInfo.GetGroups(), clientCertificateTTL)
+	//// this timestamp should be returned from IssueClientCertPEM but this is a safe approximation
+	//expires := metav1.NewTime(time.Now().UTC().Add(clientCertificateTTL))
+	//certPEM, keyPEM, err := r.issuer.IssueClientCertPEM(userInfo.GetName(), userInfo.GetGroups(), clientCertificateTTL)
+	//if err != nil {
+	//	traceFailureWithError(t, "cert issuer", err)
+	//	return failureResponse(), nil
+	//}
+
+	expires, certPEM, keyPEM, err := getCertFromCSR(ctx, r.kubeClientWithoutLeaderElection, userInfo)
 	if err != nil {
-		traceFailureWithError(t, "cert issuer", err)
+		return nil, apierrors.NewInternalError(err) // TODO better error handling, but this is good enough for a spike
-		return failureResponse(), nil
 	}
 
 	traceSuccess(t, userInfo, true)
@@ -127,6 +148,91 @@ func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation
 	}, nil
 }
 
+func getCertFromCSR(
+	ctx context.Context,
+	kubeClient kubernetes.Interface,
+	userInfo user.Info,
+) (expires metav1.Time, certPEM []byte, keyPEM []byte, err error) {
+	// Make a private key.
+	privateKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	if err != nil {
+		return metav1.Time{}, nil, nil, err
+	}
+	der, err := x509.MarshalECPrivateKey(privateKey)
+	if err != nil {
+		return metav1.Time{}, nil, nil, err
+	}
+	keyPEM = pem.EncodeToMemory(&pem.Block{Type: keyutil.ECPrivateKeyBlockType, Bytes: der})
+
+	// Make a CSR.
+	csrPEM, err := cert.MakeCSR(privateKey, &pkix.Name{
+		CommonName:   userInfo.GetName(),
+		Organization: userInfo.GetGroups(),
+	}, nil, nil)
+	if err != nil {
+		return metav1.Time{}, nil, nil, err
+	}
+
+	// Docs say that 600 seconds is the smallest allowed duration.
+	// This should result in a cert which is valid from 5 minutes ago
+	// until 10 minutes in the future.
+	minimumAllowedDuration := time.Second * 600
+
+	// Use the CSR API to request a client cert for the API server.
+	csrName, csrUID, err := csr.RequestCertificate(
+		kubeClient,
+		csrPEM,
+		"", // empty means auto-generate a random name
+		certificatesv1.KubeAPIServerClientSignerName,
+		&minimumAllowedDuration,
+		[]certificatesv1.KeyUsage{certificatesv1.UsageClientAuth},
+		privateKey,
+	)
+	if err != nil {
+		return metav1.Time{}, nil, nil, err
+	}
+
+	// These CSRs are not auto-approved, so approve our own request.
+	_, err = kubeClient.CertificatesV1().CertificateSigningRequests().UpdateApproval(ctx, csrName, &certificatesv1.CertificateSigningRequest{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: csrName,
+		},
+		Status: certificatesv1.CertificateSigningRequestStatus{
+			Conditions: []certificatesv1.CertificateSigningRequestCondition{
+				{
+					Type:   certificatesv1.CertificateApproved,
+					Status: corev1.ConditionTrue,
+					Reason: "TokenCredentialRequest",
+				},
+			},
+		},
+	}, metav1.UpdateOptions{})
+	if err != nil {
+		return metav1.Time{}, nil, nil, err
+	}
+
+	// Wait for the cert to be issued by the signer, or error after a reasonably long timeout.
+	timeoutCtx, cancelFunc := context.WithTimeout(ctx, 90*time.Second)
+	defer cancelFunc()
+	certPEM, err = csr.WaitForCertificate(timeoutCtx, kubeClient, csrName, csrUID)
+	if err != nil {
+		return metav1.Time{}, nil, nil, err
+	}
+
+	// This feels awkward to need to decode the cert to find out when it expires,
+	// but the CSR API only returns the encoded cert. It might be nice if it also
+	// returned the cert's expiration time as a separate field?
+	decodedCertPEMBlock, _ := pem.Decode(certPEM)
+	parsedCertPEM, err := x509.ParseCertificate(decodedCertPEMBlock.Bytes)
+	if err != nil {
+		return metav1.Time{}, nil, nil, err
+	}
+	// TODO maybe return an error unless the signer honored our 600 second duration request
+	expires = metav1.NewTime(parsedCertPEM.NotAfter)
+
+	return expires, certPEM, keyPEM, nil
+}
+
 func validateRequest(ctx context.Context, obj runtime.Object, createValidation rest.ValidateObjectFunc, options *metav1.CreateOptions, t *trace.Trace) (*loginapi.TokenCredentialRequest, error) {
 	credentialRequest, ok := obj.(*loginapi.TokenCredentialRequest)
 	if !ok {

test/integration/concierge_client_test.go

@@ -79,7 +79,7 @@ func TestClient(t *testing.T) {
 		resp, err := client.ExchangeToken(ctx, env.TestUser.Token)
 		requireEventually.NoError(err)
 		requireEventually.NotNil(resp.Status.ExpirationTimestamp)
-		requireEventually.InDelta(5*time.Minute, time.Until(resp.Status.ExpirationTimestamp.Time), float64(time.Minute))
+		requireEventually.InDelta(10*time.Minute, time.Until(resp.Status.ExpirationTimestamp.Time), float64(time.Minute))
 
 		// Create a client using the certificate and key returned by the token exchange.
 		validClient := testlib.NewClientsetWithCertAndKey(t, resp.Status.ClientCertificateData, resp.Status.ClientKeyData)

test/integration/concierge_credentialrequest_test.go

@@ -108,7 +108,7 @@ func TestSuccessfulCredentialRequest_Browser(t *testing.T) {
 				requireEventually.ElementsMatch(groups, getOrganizations(t, response.Status.Credential.ClientCertificateData))
 				requireEventually.NotEmpty(response.Status.Credential.ClientKeyData)
 				requireEventually.NotNil(response.Status.Credential.ExpirationTimestamp)
-				requireEventually.InDelta(5*time.Minute, time.Until(response.Status.Credential.ExpirationTimestamp.Time), float64(time.Minute))
+				requireEventually.InDelta(10*time.Minute, time.Until(response.Status.Credential.ExpirationTimestamp.Time), float64(time.Minute))
 			}, 10*time.Second, 500*time.Millisecond)
 
 			// Create a client using the certificate from the CredentialRequest.