From 82d3e0da45c2f5735352d6d3c5b6857c58f0b4a6 Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Thu, 17 Mar 2022 13:35:15 -0700 Subject: [PATCH] Proof of concept for using Kube CSR API in TokenCredentialRequest --- deploy/concierge/rbac.yaml | 7 + internal/concierge/apiserver/apiserver.go | 18 +-- internal/concierge/server/server.go | 25 +++- internal/registry/credentialrequest/rest.go | 130 ++++++++++++++++-- test/integration/concierge_client_test.go | 2 +- .../concierge_credentialrequest_test.go | 2 +- 6 files changed, 155 insertions(+), 29 deletions(-) diff --git a/deploy/concierge/rbac.yaml b/deploy/concierge/rbac.yaml index 61ffa57b..45d1f423 100644 --- a/deploy/concierge/rbac.yaml +++ b/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 ] diff --git a/internal/concierge/apiserver/apiserver.go b/internal/concierge/apiserver/apiserver.go index bc08ad68..41379322 100644 --- a/internal/concierge/apiserver/apiserver.go +++ b/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 - Issuer issuer.ClientCertIssuer - BuildControllersPostStartHook controllerinit.RunnerBuilder - Scheme *runtime.Scheme - NegotiatedSerializer runtime.NegotiatedSerializer - LoginConciergeGroupVersion schema.GroupVersion - IdentityConciergeGroupVersion schema.GroupVersion + Authenticator credentialrequest.TokenCredentialRequestAuthenticator + Issuer issuer.ClientCertIssuer + BuildControllersPostStartHook controllerinit.RunnerBuilder + Scheme *runtime.Scheme + NegotiatedSerializer runtime.NegotiatedSerializer + LoginConciergeGroupVersion 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) { diff --git a/internal/concierge/server/server.go b/internal/concierge/server/server.go index 1b724700..0476049b 100644 --- a/internal/concierge/server/server.go +++ b/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, - Issuer: issuer, - BuildControllersPostStartHook: buildControllers, - Scheme: scheme, - NegotiatedSerializer: codecs, - LoginConciergeGroupVersion: loginConciergeGroupVersion, - IdentityConciergeGroupVersion: identityConciergeGroupVersion, + Authenticator: authenticator, + Issuer: issuer, + BuildControllersPostStartHook: buildControllers, + Scheme: scheme, + NegotiatedSerializer: codecs, + LoginConciergeGroupVersion: loginConciergeGroupVersion, + IdentityConciergeGroupVersion: identityConciergeGroupVersion, + KubeClientWithoutLeaderElection: kubeClientWithoutLeaderElection, }, } return apiServerConfig, nil diff --git a/internal/registry/credentialrequest/rest.go b/internal/registry/credentialrequest/rest.go index 4a5d1e0b..5f6a894b 100644 --- a/internal/registry/credentialrequest/rest.go +++ b/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, - issuer: issuer, - tableConvertor: rest.NewDefaultTableConvertor(resource), + authenticator: authenticator, + issuer: issuer, + tableConvertor: rest.NewDefaultTableConvertor(resource), + kubeClientWithoutLeaderElection: kubeClientWithoutLeaderElection, } } type REST struct { - authenticator TokenCredentialRequestAuthenticator - issuer issuer.ClientCertIssuer - tableConvertor rest.TableConvertor + authenticator TokenCredentialRequestAuthenticator + issuer issuer.ClientCertIssuer + 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 - expires := metav1.NewTime(time.Now().UTC().Add(clientCertificateTTL)) - certPEM, keyPEM, err := r.issuer.IssueClientCertPEM(userInfo.GetName(), userInfo.GetGroups(), clientCertificateTTL) + // By commenting out this code for the spike, we prevent the usual kube cert agent and impersonation proxy + // strategies from getting involved in creating client certs. Instead, we will use the Kube CSR APIs below. + //// 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 failureResponse(), nil + return nil, apierrors.NewInternalError(err) // TODO better error handling, but this is good enough for a spike } 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 { diff --git a/test/integration/concierge_client_test.go b/test/integration/concierge_client_test.go index f5613d77..0e8865ce 100644 --- a/test/integration/concierge_client_test.go +++ b/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) diff --git a/test/integration/concierge_credentialrequest_test.go b/test/integration/concierge_credentialrequest_test.go index 1d9bd6ee..450fc919 100644 --- a/test/integration/concierge_credentialrequest_test.go +++ b/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.