From 6cc8a2f8ddb3a594b88ea68e35732f80f5637646 Mon Sep 17 00:00:00 2001 From: Andrew Keesler Date: Fri, 24 Jul 2020 11:40:08 -0400 Subject: [PATCH 01/10] WIP: initial integration test for cert issuing --- test/integration/loginrequest_test.go | 32 +++++++++++++++++++++++---- test/library/client.go | 30 ++++++++++++++++++++++--- 2 files changed, 55 insertions(+), 7 deletions(-) diff --git a/test/integration/loginrequest_test.go b/test/integration/loginrequest_test.go index e3b7f86b..132da669 100644 --- a/test/integration/loginrequest_test.go +++ b/test/integration/loginrequest_test.go @@ -8,6 +8,7 @@ package integration import ( "context" "encoding/json" + "net/http" "os" "testing" "time" @@ -49,14 +50,34 @@ func TestSuccessfulLoginRequest(t *testing.T) { require.Empty(t, response.Spec) require.NotNil(t, response.Status.Credential) - require.NotEmpty(t, response.Status.Credential.Token) - require.Empty(t, response.Status.Credential.ClientCertificateData) - require.Empty(t, response.Status.Credential.ClientKeyData) + require.Empty(t, response.Status.Credential.Token) + require.NotEmpty(t, response.Status.Credential.ClientCertificateData) + require.NotEmpty(t, response.Status.Credential.ClientKeyData) require.Nil(t, response.Status.Credential.ExpirationTimestamp) require.NotNil(t, response.Status.User) require.NotEmpty(t, response.Status.User.Name) require.Contains(t, response.Status.User.Groups, "tmc:member") + + clientWithCert := library.NewClientsetWithConfig( + t, + library.NewClientConfigWithCertAndKey( + t, + response.Status.Credential.ClientCertificateData, + response.Status.Credential.ClientKeyData, + ), + ) + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + _, err = clientWithCert.CoreV1().Namespaces().List(ctx, metav1.ListOptions{}) + + // Response status should be 403 Forbidden because we assume this actor does + // not have any permissions on this cluster. + require.Error(t, err) + statusError, isStatus := err.(*errors.StatusError) + require.True(t, isStatus) + require.Equal(t, http.StatusForbidden, statusError.Status().Code) } func TestFailedLoginRequestWhenTheRequestIsValidButTheTokenDoesNotAuthenticateTheUser(t *testing.T) { @@ -74,7 +95,7 @@ func TestFailedLoginRequestWhenTheRequestIsValidButTheTokenDoesNotAuthenticateTh } func TestLoginRequest_ShouldFailWhenRequestDoesNotIncludeToken(t *testing.T) { - _, err := makeRequest(t, v1alpha1.LoginRequestSpec{ + response, err := makeRequest(t, v1alpha1.LoginRequestSpec{ Type: v1alpha1.TokenLoginCredentialType, Token: nil, }) @@ -88,6 +109,9 @@ func TestLoginRequest_ShouldFailWhenRequestDoesNotIncludeToken(t *testing.T) { require.Equal(t, metav1.CauseType("FieldValueRequired"), cause.Type) require.Equal(t, "Required value: token must be supplied", cause.Message) require.Equal(t, "spec.token.value", cause.Field) + + require.Empty(t, response.Spec) + require.Nil(t, response.Status.Credential) } func TestGetDiscovery(t *testing.T) { diff --git a/test/library/client.go b/test/library/client.go index 1df00147..308fbc5a 100644 --- a/test/library/client.go +++ b/test/library/client.go @@ -6,12 +6,14 @@ SPDX-License-Identifier: Apache-2.0 package library import ( + "encoding/base64" "testing" "github.com/stretchr/testify/require" "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" "k8s.io/client-go/tools/clientcmd" + clientcmdapi "k8s.io/client-go/tools/clientcmd/api" placeholdernameclientset "github.com/suzerain-io/placeholder-name-client-go/pkg/generated/clientset/versioned" ) @@ -19,18 +21,40 @@ import ( func NewClientConfig(t *testing.T) *rest.Config { t.Helper() + return newClientConfigWithOverrides(t, &clientcmd.ConfigOverrides{}) +} + +func NewClientConfigWithCertAndKey(t *testing.T, cert, key string) *rest.Config { + t.Helper() + + return newClientConfigWithOverrides(t, &clientcmd.ConfigOverrides{ + AuthInfo: clientcmdapi.AuthInfo{ + ClientCertificateData: []byte(base64.StdEncoding.EncodeToString([]byte(cert))), + ClientKeyData: []byte(base64.StdEncoding.EncodeToString([]byte(key))), + }, + }) +} + +func newClientConfigWithOverrides(t *testing.T, overrides *clientcmd.ConfigOverrides) *rest.Config { + t.Helper() + loader := clientcmd.NewDefaultClientConfigLoadingRules() - clientConfig := clientcmd.NewNonInteractiveDeferredLoadingClientConfig(loader, &clientcmd.ConfigOverrides{}) + clientConfig := clientcmd.NewNonInteractiveDeferredLoadingClientConfig(loader, overrides) config, err := clientConfig.ClientConfig() require.NoError(t, err) - return config } func NewClientset(t *testing.T) kubernetes.Interface { t.Helper() - return kubernetes.NewForConfigOrDie(NewClientConfig(t)) + return NewClientsetWithConfig(t, NewClientConfig(t)) +} + +func NewClientsetWithConfig(t *testing.T, config *rest.Config) kubernetes.Interface { + t.Helper() + + return kubernetes.NewForConfigOrDie(config) } func NewPlaceholderNameClientset(t *testing.T) placeholdernameclientset.Interface { From e5902533eb7ab0586fe2ef83720873a8cb021805 Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Fri, 24 Jul 2020 15:41:51 -0500 Subject: [PATCH 02/10] Add "--cluster-signing-*-file" flags pointing at a host volume mount. This is a somewhat more basic way to get access to the certificate and private key we need to issue short lived certificates. The host path, tolerations, and node selector here should work on any kubeadm-derived cluster including TKG-S and Kind. Signed-off-by: Matt Moyer --- cmd/placeholder-name/app/app.go | 33 +++++++++++++++++++++++++++++++-- deploy/deployment.yaml | 18 ++++++++++++++++++ 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/cmd/placeholder-name/app/app.go b/cmd/placeholder-name/app/app.go index 82a420c9..7b9d726e 100644 --- a/cmd/placeholder-name/app/app.go +++ b/cmd/placeholder-name/app/app.go @@ -14,6 +14,7 @@ import ( "encoding/pem" "fmt" "io" + "io/ioutil" "log" "time" @@ -49,8 +50,10 @@ type App struct { cmd *cobra.Command // CLI flags - configPath string - downwardAPIPath string + configPath string + downwardAPIPath string + clusterSigningCertFilePath string + clusterSigningKeyFilePath string recommendedOptions *genericoptions.RecommendedOptions } @@ -76,6 +79,18 @@ func New(ctx context.Context, args []string, stdout, stderr io.Writer) *App { credential from somewhere to an internal credential to be used for authenticating to the Kubernetes API.`, RunE: func(cmd *cobra.Command, args []string) error { + clusterSigningCertificatePEM, err := ioutil.ReadFile(a.clusterSigningCertFilePath) + if err != nil { + return fmt.Errorf("could not read cluster signing certificate: %w", err) + } + clusterSigningPrivateKeyPEM, err := ioutil.ReadFile(a.clusterSigningKeyFilePath) + if err != nil { + return fmt.Errorf("could not read cluster signing private key: %w", err) + } + // TODO: use these value for something useful + _ = clusterSigningCertificatePEM + _ = clusterSigningPrivateKeyPEM + // Load the Kubernetes client configuration (kubeconfig), kubeConfig, err := restclient.InClusterConfig() if err != nil { @@ -121,6 +136,20 @@ authenticating to the Kubernetes API.`, "path to Downward API volume mount", ) + cmd.Flags().StringVar( + &a.clusterSigningCertFilePath, + "cluster-signing-cert-file", + "", + "path to cluster signing certificate", + ) + + cmd.Flags().StringVar( + &a.clusterSigningKeyFilePath, + "cluster-signing-key-file", + "", + "path to cluster signing private key", + ) + a.cmd = cmd return a diff --git a/deploy/deployment.yaml b/deploy/deployment.yaml index 764ec5f8..9cfb474b 100644 --- a/deploy/deployment.yaml +++ b/deploy/deployment.yaml @@ -47,6 +47,8 @@ spec: metadata: labels: app: #@ data.values.app_name + annotations: + scheduler.alpha.kubernetes.io/critical-pod: "" spec: serviceAccountName: #@ data.values.app_name + "-service-account" containers: @@ -62,11 +64,15 @@ spec: args: - --config=/etc/config/placeholder-name.yaml - --downward-api-path=/etc/podinfo + - --cluster-signing-cert-file=/etc/kubernetes/pki/ca.crt + - --cluster-signing-key-file=/etc/kubernetes/pki/ca.key volumeMounts: - name: config-volume mountPath: /etc/config - name: podinfo mountPath: /etc/podinfo + - name: k8s-certs + mountPath: /etc/kubernetes/pki volumes: - name: config-volume configMap: @@ -80,3 +86,15 @@ spec: - path: "namespace" fieldRef: fieldPath: metadata.namespace + - name: k8s-certs + hostPath: + path: /etc/kubernetes/pki + type: DirectoryOrCreate + priorityClassName: system-cluster-critical + nodeSelector: + node-role.kubernetes.io/master: "" + tolerations: + - key: CriticalAddonsOnly + operator: Exists + - effect: NoSchedule + key: node-role.kubernetes.io/master From 6001f1f456c2651c7ffe7cc2a42cd3b2e01275d3 Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Fri, 24 Jul 2020 14:32:07 -0700 Subject: [PATCH 03/10] Fix a failing unit test and import mistake from previous commits --- cmd/placeholder-name/app/app_test.go | 10 ++++++---- test/integration/loginrequest_test.go | 1 - 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/cmd/placeholder-name/app/app_test.go b/cmd/placeholder-name/app/app_test.go index 2cacf6d3..849925da 100644 --- a/cmd/placeholder-name/app/app_test.go +++ b/cmd/placeholder-name/app/app_test.go @@ -25,10 +25,12 @@ Usage: placeholder-name [flags] Flags: - -c, --config string path to configuration file (default "placeholder-name.yaml") - --downward-api-path string path to Downward API volume mount (default "/etc/podinfo") - -h, --help help for placeholder-name - --log-flush-frequency duration Maximum number of seconds between log flushes (default 5s) + --cluster-signing-cert-file string path to cluster signing certificate + --cluster-signing-key-file string path to cluster signing private key + -c, --config string path to configuration file (default "placeholder-name.yaml") + --downward-api-path string path to Downward API volume mount (default "/etc/podinfo") + -h, --help help for placeholder-name + --log-flush-frequency duration Maximum number of seconds between log flushes (default 5s) ` func TestCommand(t *testing.T) { diff --git a/test/integration/loginrequest_test.go b/test/integration/loginrequest_test.go index 8c56d539..9a119dd5 100644 --- a/test/integration/loginrequest_test.go +++ b/test/integration/loginrequest_test.go @@ -7,7 +7,6 @@ package integration import ( "context" - "encoding/json" "net/http" "os" "testing" From 899f736b8c66e1f8c59b1e5aab8f17d2de2c8f4c Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Fri, 24 Jul 2020 15:57:29 -0700 Subject: [PATCH 04/10] Int test for LoginRequest grants permissions to test user - Dynamically grant RBAC permission to the test user to allow them to make read requests via the API - Then use the credential returned from the LoginRequest to make a request back to the API server which should be successful --- test/integration/loginrequest_test.go | 66 +++++++++++++++++++++------ 1 file changed, 52 insertions(+), 14 deletions(-) diff --git a/test/integration/loginrequest_test.go b/test/integration/loginrequest_test.go index 9a119dd5..ca13b642 100644 --- a/test/integration/loginrequest_test.go +++ b/test/integration/loginrequest_test.go @@ -13,6 +13,7 @@ import ( "time" "github.com/stretchr/testify/require" + rbacv1 "k8s.io/api/rbac/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -45,6 +46,8 @@ func TestSuccessfulLoginRequest(t *testing.T) { }) require.NoError(t, err) + + // Note: If this assertion fails then your TMC token might have expired. Get a fresh one and try again. require.Empty(t, response.Status.Message) require.Empty(t, response.Spec) @@ -53,11 +56,52 @@ func TestSuccessfulLoginRequest(t *testing.T) { require.NotEmpty(t, response.Status.Credential.ClientCertificateData) require.NotEmpty(t, response.Status.Credential.ClientKeyData) require.Nil(t, response.Status.Credential.ExpirationTimestamp) - require.NotNil(t, response.Status.User) require.NotEmpty(t, response.Status.User.Name) require.Contains(t, response.Status.User.Groups, "tmc:member") + ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) + defer cancel() + + const readonlyBindingName = "integration-test-user-readonly-role-binding" + + adminClient := library.NewClientset(t) + _, err = adminClient.RbacV1().ClusterRoleBindings().Get(ctx, readonlyBindingName, metav1.GetOptions{}) + if err != nil { + // "404 not found" errors are acceptable, but others would be unexpected + statusError, isStatus := err.(*errors.StatusError) + require.True(t, isStatus) + require.Equal(t, http.StatusNotFound, int(statusError.Status().Code)) + + // Create a ClusterRoleBinding for this user only if one is not already found (so you can run tests more than once) + bindUserToReadonly := rbacv1.ClusterRoleBinding{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Name: readonlyBindingName, + }, + Subjects: []rbacv1.Subject{{ + Kind: rbacv1.UserKind, + APIGroup: rbacv1.GroupName, + Name: response.Status.User.Name, + }}, + RoleRef: rbacv1.RoleRef{ + Kind: "ClusterRole", + APIGroup: rbacv1.GroupName, + Name: "view", + }, + } + _, err = adminClient.RbacV1().ClusterRoleBindings().Create(ctx, &bindUserToReadonly, metav1.CreateOptions{}) + require.NoError(t, err) + } + + defer func() { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + err = adminClient.RbacV1().ClusterRoleBindings().Delete(ctx, readonlyBindingName, metav1.DeleteOptions{}) + require.NoError(t, err, "Test failed to clean up after itself") + }() + + // Create a client using the certificate from the LoginRequest clientWithCert := library.NewClientsetWithConfig( t, library.NewClientConfigWithCertAndKey( @@ -66,17 +110,11 @@ func TestSuccessfulLoginRequest(t *testing.T) { response.Status.Credential.ClientKeyData, ), ) - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) - defer cancel() - _, err = clientWithCert.CoreV1().Namespaces().List(ctx, metav1.ListOptions{}) - - // Response status should be 403 Forbidden because we assume this actor does - // not have any permissions on this cluster. - require.Error(t, err) - statusError, isStatus := err.(*errors.StatusError) - require.True(t, isStatus) - require.Equal(t, http.StatusForbidden, statusError.Status().Code) + // Use the client which is authenticated as the TMC user to list namespaces + listNamespaceResponse, err := clientWithCert.CoreV1().Namespaces().List(ctx, metav1.ListOptions{}) + require.NoError(t, err) + require.NotEmpty(t, listNamespaceResponse.Items) } func TestFailedLoginRequestWhenTheRequestIsValidButTheTokenDoesNotAuthenticateTheUser(t *testing.T) { @@ -126,7 +164,7 @@ func TestGetAPIResourceList(t *testing.T) { expectedGroup := &metav1.APIGroup{ Name: "placeholder.suzerain-io.github.io", Versions: []metav1.GroupVersionForDiscovery{ - metav1.GroupVersionForDiscovery{ + { GroupVersion: "placeholder.suzerain-io.github.io/v1alpha1", Version: "v1alpha1", }, @@ -149,7 +187,7 @@ func TestGetAPIResourceList(t *testing.T) { }, GroupVersion: "placeholder.suzerain-io.github.io/v1alpha1", APIResources: []metav1.APIResource{ - metav1.APIResource{ + { Name: "loginrequests", Kind: "LoginRequest", SingularName: "", // TODO(akeesler): what should this be? @@ -167,7 +205,7 @@ func TestGetAPIVersion(t *testing.T) { version, err := client.Discovery().ServerVersion() require.NoError(t, err) - require.NotNil(t, version) // TODO(akeesler: what can we assert here? + require.NotNil(t, version) // TODO(akeesler): what can we assert here? } func findGroup(name string, groups []*metav1.APIGroup) *metav1.APIGroup { From 757d987204135c154e0179b50f90f235f00f24d0 Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Mon, 27 Jul 2020 07:50:59 -0500 Subject: [PATCH 05/10] Extend certauthority to support loading an existing CA. I think we may still split this apart into multiple packages, but for now it works pretty well in both use cases. Signed-off-by: Matt Moyer --- internal/certauthority/certauthority.go | 113 ++++--- internal/certauthority/certauthority_test.go | 300 +++++++++++++------ internal/certauthority/testdata/empty | 0 internal/certauthority/testdata/invalid | 1 + internal/certauthority/testdata/multiple.crt | 34 +++ internal/certauthority/testdata/test.crt | 17 ++ internal/certauthority/testdata/test.key | 27 ++ internal/certauthority/testdata/test2.key | 27 ++ 8 files changed, 394 insertions(+), 125 deletions(-) create mode 100644 internal/certauthority/testdata/empty create mode 100644 internal/certauthority/testdata/invalid create mode 100644 internal/certauthority/testdata/multiple.crt create mode 100644 internal/certauthority/testdata/test.crt create mode 100644 internal/certauthority/testdata/test.key create mode 100644 internal/certauthority/testdata/test2.key diff --git a/internal/certauthority/certauthority.go b/internal/certauthority/certauthority.go index 8026d8e6..8c4d6e54 100644 --- a/internal/certauthority/certauthority.go +++ b/internal/certauthority/certauthority.go @@ -22,8 +22,7 @@ import ( "time" ) -// CA holds the state for a simple x509 certificate authority suitable for use in an aggregated API service. -type CA struct { +type env struct { // secure random number generators for various steps (usually crypto/rand.Reader, but broken out here for tests). serialRNG io.Reader keygenRNG io.Reader @@ -32,45 +31,70 @@ type CA struct { // clock tells the current time (usually time.Now(), but broken out here for tests). clock func() time.Time + // parse function to parse an ASN.1 byte slice into an x509 struct (normally x509.ParseCertificate) + parseCert func([]byte) (*x509.Certificate, error) +} + +// CA holds the state for a simple x509 certificate authority suitable for use in an aggregated API service. +type CA struct { + // caCert is the DER-encoded certificate for the current CA. + caCertBytes []byte + // signer is the private key for the current CA. signer crypto.Signer - // caCert is the DER-encoded certificate for the current CA. - caCertBytes []byte + // env is our reference to the outside world (clocks and random number generation). + env env } -// Option to pass when calling New. -type Option func(*CA) error - -func New(subject pkix.Name, opts ...Option) (*CA, error) { - // Initialize the result by starting with some defaults and applying any provided options. - ca := CA{ +// secureEnv is the "real" environment using secure RNGs and the real system clock. +func secureEnv() env { + return env{ serialRNG: rand.Reader, keygenRNG: rand.Reader, signingRNG: rand.Reader, clock: time.Now, + parseCert: x509.ParseCertificate, } - for _, opt := range opts { - if err := opt(&ca); err != nil { - return nil, err - } - } +} +// Load a certificate authority from an existing certificate and private key (in PEM format). +func Load(certPath string, keyPath string) (*CA, error) { + cert, err := tls.LoadX509KeyPair(certPath, keyPath) + if err != nil { + return nil, fmt.Errorf("could not load CA: %w", err) + } + if certCount := len(cert.Certificate); certCount != 1 { + return nil, fmt.Errorf("expected CA to be a single certificate, found %d certificates", certCount) + } + return &CA{ + caCertBytes: cert.Certificate[0], + signer: cert.PrivateKey.(crypto.Signer), + env: secureEnv(), + }, nil +} + +// New generates a fresh certificate authority with the given subject. +func New(subject pkix.Name) (*CA, error) { return newInternal(subject, secureEnv()) } + +// newInternal is the internal guts of New, broken out for easier testing. +func newInternal(subject pkix.Name, env env) (*CA, error) { + ca := CA{env: env} // Generate a random serial for the CA - serialNumber, err := randomSerial(ca.serialRNG) + serialNumber, err := randomSerial(env.serialRNG) if err != nil { return nil, fmt.Errorf("could not generate CA serial: %w", err) } // Generate a new P256 keypair. - privateKey, err := ecdsa.GenerateKey(elliptic.P256(), ca.keygenRNG) + privateKey, err := ecdsa.GenerateKey(elliptic.P256(), env.keygenRNG) if err != nil { return nil, fmt.Errorf("could not generate CA private key: %w", err) } ca.signer = privateKey // Make a CA certificate valid for 100 years and backdated by one minute. - now := ca.clock() + now := env.clock() notBefore := now.Add(-1 * time.Minute) notAfter := now.Add(24 * time.Hour * 365 * 100) @@ -87,7 +111,7 @@ func New(subject pkix.Name, opts ...Option) (*CA, error) { } // Self-sign the CA to get the DER certificate. - caCertBytes, err := x509.CreateCertificate(ca.signingRNG, &caTemplate, &caTemplate, &privateKey.PublicKey, privateKey) + caCertBytes, err := x509.CreateCertificate(env.signingRNG, &caTemplate, &caTemplate, &privateKey.PublicKey, privateKey) if err != nil { return nil, fmt.Errorf("could not issue CA certificate: %w", err) } @@ -95,37 +119,27 @@ func New(subject pkix.Name, opts ...Option) (*CA, error) { return &ca, nil } -// WriteBundle writes the current CA signing bundle in concatenated PEM format. -func (c *CA) WriteBundle(out io.Writer) error { - if err := pem.Encode(out, &pem.Block{Type: "CERTIFICATE", Bytes: c.caCertBytes}); err != nil { - return fmt.Errorf("could not encode CA certificate to PEM: %w", err) - } - return nil -} - // Bundle returns the current CA signing bundle in concatenated PEM format. -func (c *CA) Bundle() ([]byte, error) { - var out bytes.Buffer - err := c.WriteBundle(&out) - return out.Bytes(), err +func (c *CA) Bundle() []byte { + return pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: c.caCertBytes}) } // Issue a new server certificate for the given identity and duration. func (c *CA) Issue(subject pkix.Name, dnsNames []string, ttl time.Duration) (*tls.Certificate, error) { // Choose a random 128 bit serial number. - serialNumber, err := randomSerial(c.serialRNG) + serialNumber, err := randomSerial(c.env.serialRNG) if err != nil { return nil, fmt.Errorf("could not generate serial number for certificate: %w", err) } // Generate a new P256 keypair. - privateKey, err := ecdsa.GenerateKey(elliptic.P256(), c.keygenRNG) + privateKey, err := ecdsa.GenerateKey(elliptic.P256(), c.env.keygenRNG) if err != nil { return nil, fmt.Errorf("could not generate private key: %w", err) } // Make a CA caCert valid for the requested TTL and backdated by one minute. - now := c.clock() + now := c.env.clock() notBefore := now.Add(-1 * time.Minute) notAfter := now.Add(ttl) @@ -153,7 +167,7 @@ func (c *CA) Issue(subject pkix.Name, dnsNames []string, ttl time.Duration) (*tl } // Parse the DER encoded certificate back out into an *x509.Certificate. - newCert, err := x509.ParseCertificate(certBytes) + newCert, err := c.env.parseCert(certBytes) if err != nil { return nil, fmt.Errorf("could not parse certificate: %w", err) } @@ -166,6 +180,35 @@ func (c *CA) Issue(subject pkix.Name, dnsNames []string, ttl time.Duration) (*tl }, nil } +// IssuePEM issues a new server certificate for the given identity and duration, returning it as a pair of +// PEM-formatted byte slices for the certificate and private key. +func (c *CA) IssuePEM(subject pkix.Name, dnsNames []string, ttl time.Duration) ([]byte, []byte, error) { + return toPEM(c.Issue(subject, dnsNames, ttl)) +} + +func toPEM(cert *tls.Certificate, err error) ([]byte, []byte, error) { + // If the wrapped Issue() returned an error, pass it back. + if err != nil { + return nil, nil, err + } + + // Encode the certificate(s) to PEM. + certPEMBlocks := make([][]byte, 0, len(cert.Certificate)) + for _, c := range cert.Certificate { + certPEMBlocks = append(certPEMBlocks, pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: c})) + } + certPEM := bytes.Join(certPEMBlocks, nil) + + // Encode the private key to PEM, which means we first need to convert to PKCS8 (DER). + privateKeyPKCS8, err := x509.MarshalPKCS8PrivateKey(cert.PrivateKey) + if err != nil { + return nil, nil, fmt.Errorf("failed to marshal private key into PKCS8: %w", err) + } + keyPEM := pem.EncodeToMemory(&pem.Block{Type: "PRIVATE KEY", Bytes: privateKeyPKCS8}) + + return certPEM, keyPEM, nil +} + // randomSerial generates a random 128 bit serial number. func randomSerial(rng io.Reader) (*big.Int, error) { return rand.Int(rng, new(big.Int).Lsh(big.NewInt(1), 128)) diff --git a/internal/certauthority/certauthority_test.go b/internal/certauthority/certauthority_test.go index 44d99303..f0ae2dd8 100644 --- a/internal/certauthority/certauthority_test.go +++ b/internal/certauthority/certauthority_test.go @@ -6,8 +6,8 @@ SPDX-License-Identifier: Apache-2.0 package certauthority import ( - "bytes" "crypto" + "crypto/tls" "crypto/x509" "crypto/x509/pkix" "fmt" @@ -19,65 +19,149 @@ import ( "github.com/stretchr/testify/require" ) -func TestNew(t *testing.T) { - now := time.Date(2020, 7, 10, 12, 41, 12, 1234, time.UTC) - +func TestLoad(t *testing.T) { tests := []struct { - name string - opts []Option - wantErr string + name string + certPath string + keyPath string + wantErr string }{ { - name: "error option", - opts: []Option{func(ca *CA) error { - return fmt.Errorf("some error") - }}, - wantErr: "some error", + name: "missing cert", + certPath: "./testdata/cert-does-not-exist", + keyPath: "./testdata/test.key", + wantErr: "could not load CA: open ./testdata/cert-does-not-exist: no such file or directory", }, { - name: "failed to generate CA serial", - opts: []Option{func(ca *CA) error { - ca.serialRNG = strings.NewReader("") - ca.keygenRNG = strings.NewReader("") - ca.signingRNG = strings.NewReader("") - return nil - }}, - wantErr: "could not generate CA serial: EOF", + name: "empty cert", + certPath: "./testdata/empty", + keyPath: "./testdata/test.key", + wantErr: "could not load CA: tls: failed to find any PEM data in certificate input", }, { - name: "failed to generate CA key", - opts: []Option{func(ca *CA) error { - ca.serialRNG = strings.NewReader(strings.Repeat("x", 64)) - ca.keygenRNG = strings.NewReader("") - return nil - }}, - wantErr: "could not generate CA private key: EOF", + name: "invalid cert", + certPath: "./testdata/invalid", + keyPath: "./testdata/test.key", + wantErr: "could not load CA: tls: failed to find any PEM data in certificate input", }, { - name: "failed to self-sign", - opts: []Option{func(ca *CA) error { - ca.serialRNG = strings.NewReader(strings.Repeat("x", 64)) - ca.keygenRNG = strings.NewReader(strings.Repeat("y", 64)) - ca.signingRNG = strings.NewReader("") - return nil - }}, - wantErr: "could not issue CA certificate: EOF", + name: "missing key", + certPath: "./testdata/test.crt", + keyPath: "./testdata/key-does-not-exist", + wantErr: "could not load CA: open ./testdata/key-does-not-exist: no such file or directory", }, { - name: "success", - opts: []Option{func(ca *CA) error { - ca.serialRNG = strings.NewReader(strings.Repeat("x", 64)) - ca.keygenRNG = strings.NewReader(strings.Repeat("y", 64)) - ca.signingRNG = strings.NewReader(strings.Repeat("z", 64)) - ca.clock = func() time.Time { return now } - return nil - }}, + name: "empty key", + certPath: "./testdata/test.crt", + keyPath: "./testdata/empty", + wantErr: "could not load CA: tls: failed to find any PEM data in key input", + }, + { + name: "invalid key", + certPath: "./testdata/test.crt", + keyPath: "./testdata/invalid", + wantErr: "could not load CA: tls: failed to find any PEM data in key input", + }, + { + name: "mismatched cert and key", + certPath: "./testdata/test.crt", + keyPath: "./testdata/test2.key", + wantErr: "could not load CA: tls: private key does not match public key", + }, + { + name: "multiple certs", + certPath: "./testdata/multiple.crt", + keyPath: "./testdata/test.key", + wantErr: "expected CA to be a single certificate, found 2 certificates", + }, + { + name: "success", + certPath: "./testdata/test.crt", + keyPath: "./testdata/test.key", }, } for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { - got, err := New(pkix.Name{CommonName: "Test CA"}, tt.opts...) + ca, err := Load(tt.certPath, tt.keyPath) + if tt.wantErr != "" { + require.EqualError(t, err, tt.wantErr) + return + } + require.NoError(t, err) + require.NotEmpty(t, ca.caCertBytes) + require.NotNil(t, ca.signer) + }) + } +} + +func TestNew(t *testing.T) { + got, err := New(pkix.Name{CommonName: "Test CA"}) + require.NoError(t, err) + require.NotNil(t, got) + + // Make sure the CA certificate looks roughly like what we expect. + caCert, err := x509.ParseCertificate(got.caCertBytes) + require.NoError(t, err) + require.Equal(t, "Test CA", caCert.Subject.CommonName) +} + +func TestNewInternal(t *testing.T) { + now := time.Date(2020, 7, 10, 12, 41, 12, 1234, time.UTC) + + tests := []struct { + name string + env env + wantErr string + wantCommonName string + wantNotBefore time.Time + wantNotAfter time.Time + }{ + { + name: "failed to generate CA serial", + env: env{ + serialRNG: strings.NewReader(""), + keygenRNG: strings.NewReader(""), + signingRNG: strings.NewReader(""), + }, + wantErr: "could not generate CA serial: EOF", + }, + { + name: "failed to generate CA key", + env: env{ + serialRNG: strings.NewReader(strings.Repeat("x", 64)), + keygenRNG: strings.NewReader(""), + signingRNG: strings.NewReader(""), + }, + wantErr: "could not generate CA private key: EOF", + }, + { + name: "failed to self-sign", + env: env{ + serialRNG: strings.NewReader(strings.Repeat("x", 64)), + keygenRNG: strings.NewReader(strings.Repeat("y", 64)), + signingRNG: strings.NewReader(""), + clock: func() time.Time { return now }, + }, + wantErr: "could not issue CA certificate: EOF", + }, + { + name: "success", + env: env{ + serialRNG: strings.NewReader(strings.Repeat("x", 64)), + keygenRNG: strings.NewReader(strings.Repeat("y", 64)), + signingRNG: strings.NewReader(strings.Repeat("z", 64)), + clock: func() time.Time { return now }, + }, + wantCommonName: "Test CA", + wantNotAfter: now.Add(100 * 365 * 24 * time.Hour), + wantNotBefore: now.Add(-1 * time.Minute), + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + got, err := newInternal(pkix.Name{CommonName: "Test CA"}, tt.env) if tt.wantErr != "" { require.EqualError(t, err, tt.wantErr) require.Nil(t, got) @@ -89,46 +173,17 @@ func TestNew(t *testing.T) { // Make sure the CA certificate looks roughly like what we expect. caCert, err := x509.ParseCertificate(got.caCertBytes) require.NoError(t, err) - require.Equal(t, "Test CA", caCert.Subject.CommonName) - require.Equal(t, now.Add(100*365*24*time.Hour).Unix(), caCert.NotAfter.Unix()) - require.Equal(t, now.Add(-1*time.Minute).Unix(), caCert.NotBefore.Unix()) + require.Equal(t, tt.wantCommonName, caCert.Subject.CommonName) + require.Equal(t, tt.wantNotAfter.Unix(), caCert.NotAfter.Unix()) + require.Equal(t, tt.wantNotBefore.Unix(), caCert.NotBefore.Unix()) }) } } -type errWriter struct { - err error -} - -func (e *errWriter) Write(p []byte) (n int, err error) { return 0, e.err } - -func TestWriteBundle(t *testing.T) { - t.Run("error", func(t *testing.T) { - ca := CA{} - out := errWriter{fmt.Errorf("some error")} - require.EqualError(t, ca.WriteBundle(&out), "could not encode CA certificate to PEM: some error") - }) - - t.Run("empty", func(t *testing.T) { - ca := CA{} - var out bytes.Buffer - require.NoError(t, ca.WriteBundle(&out)) - require.Equal(t, "-----BEGIN CERTIFICATE-----\n-----END CERTIFICATE-----\n", out.String()) - }) - - t.Run("success", func(t *testing.T) { - ca := CA{caCertBytes: []byte{1, 2, 3, 4, 5, 6, 7, 8}} - var out bytes.Buffer - require.NoError(t, ca.WriteBundle(&out)) - require.Equal(t, "-----BEGIN CERTIFICATE-----\nAQIDBAUGBwg=\n-----END CERTIFICATE-----\n", out.String()) - }) -} - func TestBundle(t *testing.T) { t.Run("success", func(t *testing.T) { ca := CA{caCertBytes: []byte{1, 2, 3, 4, 5, 6, 7, 8}} - got, err := ca.Bundle() - require.NoError(t, err) + got := ca.Bundle() require.Equal(t, "-----BEGIN CERTIFICATE-----\nAQIDBAUGBwg=\n-----END CERTIFICATE-----\n", string(got)) }) } @@ -147,7 +202,7 @@ func (e *errSigner) Sign(_ io.Reader, _ []byte, _ crypto.SignerOpts) ([]byte, er func TestIssue(t *testing.T) { now := time.Date(2020, 7, 10, 12, 41, 12, 1234, time.UTC) - realCA, err := New(pkix.Name{CommonName: "Test CA"}) + realCA, err := Load("./testdata/test.crt", "./testdata/test.key") require.NoError(t, err) tests := []struct { @@ -158,33 +213,41 @@ func TestIssue(t *testing.T) { { name: "failed to generate serial", ca: CA{ - serialRNG: strings.NewReader(""), + env: env{ + serialRNG: strings.NewReader(""), + }, }, wantErr: "could not generate serial number for certificate: EOF", }, { name: "failed to generate keypair", ca: CA{ - serialRNG: strings.NewReader(strings.Repeat("x", 64)), - keygenRNG: strings.NewReader(""), + env: env{ + serialRNG: strings.NewReader(strings.Repeat("x", 64)), + keygenRNG: strings.NewReader(""), + }, }, wantErr: "could not generate private key: EOF", }, { name: "invalid CA certificate", ca: CA{ - serialRNG: strings.NewReader(strings.Repeat("x", 64)), - keygenRNG: strings.NewReader(strings.Repeat("x", 64)), - clock: func() time.Time { return now }, + env: env{ + serialRNG: strings.NewReader(strings.Repeat("x", 64)), + keygenRNG: strings.NewReader(strings.Repeat("x", 64)), + clock: func() time.Time { return now }, + }, }, wantErr: "could not parse CA certificate: asn1: syntax error: sequence truncated", }, { name: "signing error", ca: CA{ - serialRNG: strings.NewReader(strings.Repeat("x", 64)), - keygenRNG: strings.NewReader(strings.Repeat("x", 64)), - clock: func() time.Time { return now }, + env: env{ + serialRNG: strings.NewReader(strings.Repeat("x", 64)), + keygenRNG: strings.NewReader(strings.Repeat("x", 64)), + clock: func() time.Time { return now }, + }, caCertBytes: realCA.caCertBytes, signer: &errSigner{ pubkey: realCA.signer.Public(), @@ -196,9 +259,28 @@ func TestIssue(t *testing.T) { { name: "success", ca: CA{ - serialRNG: strings.NewReader(strings.Repeat("x", 64)), - keygenRNG: strings.NewReader(strings.Repeat("x", 64)), - clock: func() time.Time { return now }, + env: env{ + serialRNG: strings.NewReader(strings.Repeat("x", 64)), + keygenRNG: strings.NewReader(strings.Repeat("x", 64)), + clock: func() time.Time { return now }, + parseCert: func(_ []byte) (*x509.Certificate, error) { + return nil, fmt.Errorf("some parse certificate error") + }, + }, + caCertBytes: realCA.caCertBytes, + signer: realCA.signer, + }, + wantErr: "could not parse certificate: some parse certificate error", + }, + { + name: "success", + ca: CA{ + env: env{ + serialRNG: strings.NewReader(strings.Repeat("x", 64)), + keygenRNG: strings.NewReader(strings.Repeat("x", 64)), + clock: func() time.Time { return now }, + parseCert: x509.ParseCertificate, + }, caCertBytes: realCA.caCertBytes, signer: realCA.signer, }, @@ -218,3 +300,41 @@ func TestIssue(t *testing.T) { }) } } + +func TestIssuePEM(t *testing.T) { + realCA, err := Load("./testdata/test.crt", "./testdata/test.key") + require.NoError(t, err) + + certPEM, keyPEM, err := realCA.IssuePEM(pkix.Name{CommonName: "Test Server"}, []string{"example.com"}, 10*time.Minute) + require.NoError(t, err) + require.NotEmpty(t, certPEM) + require.NotEmpty(t, keyPEM) +} + +func TestToPEM(t *testing.T) { + realCert, err := tls.LoadX509KeyPair("./testdata/test.crt", "./testdata/test.key") + require.NoError(t, err) + + t.Run("error from input", func(t *testing.T) { + certPEM, keyPEM, err := toPEM(nil, fmt.Errorf("some error")) + require.EqualError(t, err, "some error") + require.Nil(t, certPEM) + require.Nil(t, keyPEM) + }) + + t.Run("invalid private key", func(t *testing.T) { + cert := realCert + cert.PrivateKey = nil + certPEM, keyPEM, err := toPEM(&cert, nil) + require.EqualError(t, err, "failed to marshal private key into PKCS8: x509: unknown key type while marshaling PKCS#8: ") + require.Nil(t, certPEM) + require.Nil(t, keyPEM) + }) + + t.Run("success", func(t *testing.T) { + certPEM, keyPEM, err := toPEM(&realCert, nil) + require.NoError(t, err) + require.NotEmpty(t, certPEM) + require.NotEmpty(t, keyPEM) + }) +} diff --git a/internal/certauthority/testdata/empty b/internal/certauthority/testdata/empty new file mode 100644 index 00000000..e69de29b diff --git a/internal/certauthority/testdata/invalid b/internal/certauthority/testdata/invalid new file mode 100644 index 00000000..f31839d5 --- /dev/null +++ b/internal/certauthority/testdata/invalid @@ -0,0 +1 @@ +Some invalid file contents diff --git a/internal/certauthority/testdata/multiple.crt b/internal/certauthority/testdata/multiple.crt new file mode 100644 index 00000000..b69be2ad --- /dev/null +++ b/internal/certauthority/testdata/multiple.crt @@ -0,0 +1,34 @@ +-----BEGIN CERTIFICATE----- +MIICyDCCAbCgAwIBAgIBADANBgkqhkiG9w0BAQsFADAVMRMwEQYDVQQDEwprdWJl +cm5ldGVzMB4XDTIwMDcyNTIxMDQxOFoXDTMwMDcyMzIxMDQxOFowFTETMBEGA1UE +AxMKa3ViZXJuZXRlczCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAL3K +hYv2gIQ1Dwzh2cWMid+ofAnvLIfV2Xv61vTLGprUI+XUqB4/gtf6X6UNn0Lett2n +d8p4wy7hw73hU/ggdvmWJvqBrSjc3JGfy+kj66fKXX+PTlbL7QbwiRvcSqIXIWlV +lHHxECWrED8jCulw/NVqfook/h5iNUCT9yswSJr/0fImiVnoTlIoEYG2eCNejZ5c +g39uD3ZTqd9ZxWwSLLnI+2kpJnZBPcd1ZQ8AQqzDgZtYRCqacn5gckQUKZWKQlxo +Eft6g1XHJouAWAZw7hEtk0v8rG0/eKF7wamxFi6BFVlbjWBsB4T9rApbdBWTKeCJ +Hv8fv5RMFSzpT3uzTO8CAwEAAaMjMCEwDgYDVR0PAQH/BAQDAgKkMA8GA1UdEwEB +/wQFMAMBAf8wDQYJKoZIhvcNAQELBQADggEBACh5RhbxqJe+Z/gc17cZhKNmdiwu +I2pLp3QBfwvN+Wbmajzw/7rYhY0d8JYVTJzXSCPWi6UAKxAtXOLF8WIIf9i39n6R +uKOBGW14FzzGyRJiD3qaG/JTvEW+SLhwl68Ndr5LHSnbugAqq31abcQy6Zl9v5A8 +JKC97Lj/Sn8rj7opKy4W3oq7NCQsAb0zh4IllRF6UvSnJySfsg7xdXHHpxYDHtOS +XcOu5ySUIZTgFe9RfeUZlGZ5xn0ckMlQ7qW2Wx1q0OVWw5us4NtkGqKrHG4Tn1X7 +uwo/Yytn5sDxrDv1/oii6AZOCsTPre4oD3wz4nmVzCVJcgrqH4Q24hT8WNg= +-----END CERTIFICATE----- +-----BEGIN CERTIFICATE----- +MIICyzCCAbOgAwIBAgIBADANBgkqhkiG9w0BAQsFADAVMRMwEQYDVQQDEwprdWJl +cm5ldGVzMB4XDTIwMDcyMTIxMDcwMloXDTMwMDcxOTIxMTIwMlowFTETMBEGA1UE +AxMKa3ViZXJuZXRlczCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBALRz +y8TRjCTkqxTGpMoGa6nmZAZU76GqchVJh6x/T/utsfE822IgyLo5oNKKtLsNiXOT +pO1DdaPtwKJBlifmAIfPwPIOikcqYicLYMS6PM1f6pPGUM9zlYW198sQfTybVVQG +KBLEjyYbcFPtM98omXaILM+V8KXokMiSWgho36qH/abcdxKKTPJs44U+mby5gkew +Iee+8yXOiqCmCPc6a+UsU28eYt72XTiAMLKQsGu71Q99IMXYq6/kZbxxFk8zh2r+ +ZuaBbCbasclznutLnWB4hpfswLnTY3Ct9kv07mjzmIsLguOOQGzoVJ72xsLwxfoG +KS5jnTImXFVp28NFP6kCAwEAAaMmMCQwDgYDVR0PAQH/BAQDAgKkMBIGA1UdEwEB +/wQIMAYBAf8CAQAwDQYJKoZIhvcNAQELBQADggEBAF4E3IA8Wst9GOeFjjv/IPAp +LT89JHehMrTRMngw+Lvv4WG9rymtHFpvEiVgh8MhcP+MuNc95tIPLaonpTdKJ/dR +PgDlpOFL0dvaskuOVWQYZnD/ehf8zF+vlt9Mby3rJleruGETahdvQVd7DB61zSC6 +7g2rE2VqZ62Vj48RdEF9n+tYXWa4wRw3I1i8DUA/fNAS8P4/KXN6VvdVHe8y+nim +ASs8VX0QEWaGRIQZankFXv5p1/tXOFo/Wmo8vrVa9OsCXJm7iN5riwYiQn237iBJ +XmEs5jp04H7V7aDqZOX/N13uzQbGSPChAgWr/LxO1tIa6PmK+vfo60lRsBWeYrE= +-----END CERTIFICATE----- diff --git a/internal/certauthority/testdata/test.crt b/internal/certauthority/testdata/test.crt new file mode 100644 index 00000000..796a7690 --- /dev/null +++ b/internal/certauthority/testdata/test.crt @@ -0,0 +1,17 @@ +-----BEGIN CERTIFICATE----- +MIICyDCCAbCgAwIBAgIBADANBgkqhkiG9w0BAQsFADAVMRMwEQYDVQQDEwprdWJl +cm5ldGVzMB4XDTIwMDcyNTIxMDQxOFoXDTMwMDcyMzIxMDQxOFowFTETMBEGA1UE +AxMKa3ViZXJuZXRlczCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAL3K +hYv2gIQ1Dwzh2cWMid+ofAnvLIfV2Xv61vTLGprUI+XUqB4/gtf6X6UNn0Lett2n +d8p4wy7hw73hU/ggdvmWJvqBrSjc3JGfy+kj66fKXX+PTlbL7QbwiRvcSqIXIWlV +lHHxECWrED8jCulw/NVqfook/h5iNUCT9yswSJr/0fImiVnoTlIoEYG2eCNejZ5c +g39uD3ZTqd9ZxWwSLLnI+2kpJnZBPcd1ZQ8AQqzDgZtYRCqacn5gckQUKZWKQlxo +Eft6g1XHJouAWAZw7hEtk0v8rG0/eKF7wamxFi6BFVlbjWBsB4T9rApbdBWTKeCJ +Hv8fv5RMFSzpT3uzTO8CAwEAAaMjMCEwDgYDVR0PAQH/BAQDAgKkMA8GA1UdEwEB +/wQFMAMBAf8wDQYJKoZIhvcNAQELBQADggEBACh5RhbxqJe+Z/gc17cZhKNmdiwu +I2pLp3QBfwvN+Wbmajzw/7rYhY0d8JYVTJzXSCPWi6UAKxAtXOLF8WIIf9i39n6R +uKOBGW14FzzGyRJiD3qaG/JTvEW+SLhwl68Ndr5LHSnbugAqq31abcQy6Zl9v5A8 +JKC97Lj/Sn8rj7opKy4W3oq7NCQsAb0zh4IllRF6UvSnJySfsg7xdXHHpxYDHtOS +XcOu5ySUIZTgFe9RfeUZlGZ5xn0ckMlQ7qW2Wx1q0OVWw5us4NtkGqKrHG4Tn1X7 +uwo/Yytn5sDxrDv1/oii6AZOCsTPre4oD3wz4nmVzCVJcgrqH4Q24hT8WNg= +-----END CERTIFICATE----- diff --git a/internal/certauthority/testdata/test.key b/internal/certauthority/testdata/test.key new file mode 100644 index 00000000..7ad653ae --- /dev/null +++ b/internal/certauthority/testdata/test.key @@ -0,0 +1,27 @@ +-----BEGIN RSA PRIVATE KEY----- +MIIEogIBAAKCAQEAvcqFi/aAhDUPDOHZxYyJ36h8Ce8sh9XZe/rW9MsamtQj5dSo +Hj+C1/pfpQ2fQt623ad3ynjDLuHDveFT+CB2+ZYm+oGtKNzckZ/L6SPrp8pdf49O +VsvtBvCJG9xKohchaVWUcfEQJasQPyMK6XD81Wp+iiT+HmI1QJP3KzBImv/R8iaJ +WehOUigRgbZ4I16NnlyDf24PdlOp31nFbBIsucj7aSkmdkE9x3VlDwBCrMOBm1hE +KppyfmByRBQplYpCXGgR+3qDVccmi4BYBnDuES2TS/ysbT94oXvBqbEWLoEVWVuN +YGwHhP2sClt0FZMp4Ike/x+/lEwVLOlPe7NM7wIDAQABAoIBAFC1tUEmHNUcM0BJ +M3D9KQzB+63F1mwVlx1QOOV1EeVR3co5Ox1R6PSr9sycFGQ9jgqI0zp5TJe9Tp6L +GkhklfPh1MWnK9o6wlnzWKXWrrp2Jni+mpPyuOPAmq4Maniv2XeP+0bROwqpyojv +AA7yC7M+TH226ZJGNVs3EV9+cwHml0yuzBfIJn/rv/w2g+WRKM/MC0S7k2d8bRlA +NycKVGAGBhKTltjoVYOeh6aHEpSjK8zfaePjo5dYJvoVIli60YCgcJOU/8jXT+Np +1Fm7tRvAtj3pUp0Sqdaf2RUzh9jfJp2VFCHuSJ6TPqArOyQojtMcTHF0TiW7xrHP +xOCRIAECgYEAwGBPU7vdthMJBg+ORUoGQQaItTeJvQwIqJvbKD2osp4jhS1dGZBw +W30GKEc/gd8JNtOq9BBnMicPF7hktuy+bSPv41XPud67rSSO7Tsw20C10gFRq06B +zIJWFAUqK3IkvVc3VDmtSLSDox4QZ/BdqaMlQ5y5JCsC5kThmkZFlO8CgYEA/I9X +YHi6RioMJE1fqOHJL4DDjlezmcuRrD7fE5InKbtJZ2JhGYOX/C0KXnHTOWTCDxxN +FBvpvD6Xv5o3PhB9Z6k2fqvJ4GS8urkG/KU4xcC+bak+9ava8oaiSqG16zD9NH2P +jJ60NrbLl1J0pU9fiwuFVUKJ4hDZOfN9RqYdyAECgYAVwo8WhJiGgM6zfcz073OX +pVqPTPHqjVLpZ3+5pIfRdGvGI6R1QM5EuvaYVb7MPOM47WZX5wcVOC/P2g6iVlMP +21HGIC2384a9BfaYxOo40q/+SiHnw6CQ9mkwKIllkqqvNA9RGpkMMUb2i28For2l +c4vCgxa6DZdtXns6TRqPxwKBgCfY5cxOv/T6BVhk7MbUeM2J31DB/ZAyUhV/Bess +kAlBh19MYk2IOZ6L7KriApV3lDaWHIMjtEkDByYvyq98Io0MYZCywfMpca10K+oI +l2B7/I+IuGpCZxUEsO5dfTpSTGDPvqpND9niFVUWqVi7oTNq6ep9yQtl5SADjqxq +4SABAoGAIm0hUg1wtcS46cGLy6PIkPM5tocTSghtz4vFsuk/i4QA9GBoBO2gH6ty ++kJHmeaXt2dmgySp0QAWit5UlceEumB0NXnAdJZQxeGSFSyYkDWhwXd8wDceKo/1 +LfCU6Dk8IN/SsppVUWXQ2rlORvxlrHeCio8o0kS9Yiu55WMYg4g= +-----END RSA PRIVATE KEY----- diff --git a/internal/certauthority/testdata/test2.key b/internal/certauthority/testdata/test2.key new file mode 100644 index 00000000..19b7096c --- /dev/null +++ b/internal/certauthority/testdata/test2.key @@ -0,0 +1,27 @@ +-----BEGIN RSA PRIVATE KEY----- +MIIEowIBAAKCAQEAqeddVCjzZNKp/hVprYRY4EKos0PkrpkA0cbQTIhKrJIU2JXW +G0/RgffNnfMEXGR/6xhGPGM9CU+jD5aZJtl/+7w0zhlZib1lQLeSIfOLGO4rqbsr +k4KXYosUaUD8qd6El1FXUB6OfpZFjmvE5qzFlpjOEbedDrgbzVeD6BsdeCrgT3G4 +Jly/79MWsOjGk8Oa+OS/4OLf6W0UnKCWbyENNDVrsihLByffv1YZZ+s4bOMFb1cA +Oc078TF/1M5ekTcicGdKKxr7CQT9RtR6iuPcVucuBhd9eDCsK4tH9PEWtyY/hISz +Pnrigt6LEW++IvHLJQ7mX+COzFMNramyK/F8aQIDAQABAoIBAFUd02OWIFkiMIdZ +std6tgujWWB1YtsVS5PMRg4ROVe61zap2dlU42B5BElctZKTxoHAZ29ZR/qiKs5k +Y9VSoQs7/jhB+tlGSLNjQ5I+sDCNINKnMe10PuLfShpwtCNlloc3+MXqiPhhz/bJ +hpsJcvM/Gf1GPyhgk40Lisl8zAamouLZ0qQSspmHmPo4ztpA2w81bG05GfTJTqVp +6pceOotVU4NNkiqq0SOF13ZajUaR3ifCnEbjkRnh8L8x0rqGUv7aAZMIc6x3kHRT +ZG2HfLvQW8QWsOWB6WIdZqAPMtrTJw0ozMfLJYfkfosGjLC99pjefMY7ENPnGwvN +FdOgwAECgYEA3WtNyi9bE5Licrv0Ys/A3oI11in2eKL8cmeJM/Hxvxz3EzQH8Kl8 +nYtfCP18kHSg7fddg4xbqW6NLPIkOtopDy0evTzPpZpxBc6mu1buqFm4EBeUAsIu +rK1Ol0dKxZ7EoFAQGHN0JpTd62diRHnxCiIvfQD55ClloyrR/Cd8SuECgYEAxHBk +YsDSFs9DFbhvFEWigfPPYjViHg9FsNBYodtuiyWXbkH8QXmjTpEaiLXk5Zc0VOYA +8OBYbxly1l6nAW9/v2LewPoxVQTyTnTnsLlarLo2ax8GT1NNu9gCPGzbEORvlHky +h/NCw0RBcmGnfTijoOVEC8RETbN1SK3x5as0qokCgYAM3myyAJiZhaL1qijlCVAb +XpQEc4HotwhXGd9mjnxPcD6H9jEz8pXUjkIiwqDXwH+N9R+RQrodGdjIsPYcGYvj +Xur3cq5a4KQLA1y7bK0ISdah0M0AcArIbHYx4qnc3IJvEtgso6EvkN1pDiQu+Kti +vGPoLwNXGHTYy+dScXUO4QKBgQCmgFFGNwOb28+T0IEuYJuOpJZKOs9QhUdfyCjo +ADMhdBp3lSx4Xt6h0HH6IJrEU7ZCo7V2deHfQWXJ9+58VAKmuOnwDeDUnF25THO5 +olIOB8PqZiCWChjgOAYlK2s/VTCSW2wOOY2ELw1+IvGxPNnMnadghdoTNiIaGX3o +WoZIaQKBgEDojPr84GMR+M5d2YrvLJmLSmDLwWR0j6soANUXsaLzZVZBifOSKIG8 +ySO4QOe4O2Nb1nWAfmEP964VwoQ0rHlkt0rJopwJTj4uYjhDRGIN+x2aJZ00bm4u +/1c7cowqxj/TkjTP4JASxBwBPUYpJxiajATTZiddxt4xRxaNVKlK +-----END RSA PRIVATE KEY----- From 07a71236aad97eed414754c55776e8669cfaec3c Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Mon, 27 Jul 2020 07:52:36 -0500 Subject: [PATCH 06/10] Fix a bad assumption in library.NewClientConfigWithCertAndKey. It turns out these fields are not meant to be base64 encoded, even though that's how they are in the kubeconfig. Signed-off-by: Matt Moyer --- test/library/client.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/test/library/client.go b/test/library/client.go index 308fbc5a..fbdd9e42 100644 --- a/test/library/client.go +++ b/test/library/client.go @@ -6,7 +6,6 @@ SPDX-License-Identifier: Apache-2.0 package library import ( - "encoding/base64" "testing" "github.com/stretchr/testify/require" @@ -29,8 +28,8 @@ func NewClientConfigWithCertAndKey(t *testing.T, cert, key string) *rest.Config return newClientConfigWithOverrides(t, &clientcmd.ConfigOverrides{ AuthInfo: clientcmdapi.AuthInfo{ - ClientCertificateData: []byte(base64.StdEncoding.EncodeToString([]byte(cert))), - ClientKeyData: []byte(base64.StdEncoding.EncodeToString([]byte(key))), + ClientCertificateData: []byte(cert), + ClientKeyData: []byte(key), }, }) } @@ -54,7 +53,9 @@ func NewClientset(t *testing.T) kubernetes.Interface { func NewClientsetWithConfig(t *testing.T, config *rest.Config) kubernetes.Interface { t.Helper() - return kubernetes.NewForConfigOrDie(config) + result, err := kubernetes.NewForConfig(config) + require.NoError(t, err, "unexpected failure from kubernetes.NewForConfig()") + return result } func NewPlaceholderNameClientset(t *testing.T) placeholdernameclientset.Interface { From d8c7a25487fb3b17111c1da60ff03c27e3c45fe7 Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Mon, 27 Jul 2020 07:55:33 -0500 Subject: [PATCH 07/10] Extend the REST service to keep a CertIssuer. Signed-off-by: Matt Moyer --- cmd/placeholder-name/app/app.go | 56 +++++++++++-------------------- pkg/apiserver/apiserver.go | 3 +- pkg/registry/loginrequest/rest.go | 10 +++++- 3 files changed, 31 insertions(+), 38 deletions(-) diff --git a/cmd/placeholder-name/app/app.go b/cmd/placeholder-name/app/app.go index 7b9d726e..99e977da 100644 --- a/cmd/placeholder-name/app/app.go +++ b/cmd/placeholder-name/app/app.go @@ -14,33 +14,27 @@ import ( "encoding/pem" "fmt" "io" - "io/ioutil" - "log" "time" - "k8s.io/apiserver/pkg/server/dynamiccertificates" - + "github.com/spf13/cobra" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/util/intstr" - apiregistrationv1 "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1" - - "github.com/suzerain-io/placeholder-name/internal/autoregistration" - "github.com/suzerain-io/placeholder-name/internal/certauthority" - "github.com/suzerain-io/placeholder-name/internal/downward" - "k8s.io/apimachinery/pkg/runtime" - - "github.com/spf13/cobra" + "k8s.io/apimachinery/pkg/util/intstr" genericapiserver "k8s.io/apiserver/pkg/server" + "k8s.io/apiserver/pkg/server/dynamiccertificates" genericoptions "k8s.io/apiserver/pkg/server/options" "k8s.io/apiserver/plugin/pkg/authenticator/token/webhook" "k8s.io/client-go/kubernetes" corev1client "k8s.io/client-go/kubernetes/typed/core/v1" restclient "k8s.io/client-go/rest" + apiregistrationv1 "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1" aggregationv1client "k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset" placeholderv1alpha1 "github.com/suzerain-io/placeholder-name-api/pkg/apis/placeholder/v1alpha1" + "github.com/suzerain-io/placeholder-name/internal/autoregistration" + "github.com/suzerain-io/placeholder-name/internal/certauthority" + "github.com/suzerain-io/placeholder-name/internal/downward" "github.com/suzerain-io/placeholder-name/pkg/apiserver" "github.com/suzerain-io/placeholder-name/pkg/config" ) @@ -79,18 +73,6 @@ func New(ctx context.Context, args []string, stdout, stderr io.Writer) *App { credential from somewhere to an internal credential to be used for authenticating to the Kubernetes API.`, RunE: func(cmd *cobra.Command, args []string) error { - clusterSigningCertificatePEM, err := ioutil.ReadFile(a.clusterSigningCertFilePath) - if err != nil { - return fmt.Errorf("could not read cluster signing certificate: %w", err) - } - clusterSigningPrivateKeyPEM, err := ioutil.ReadFile(a.clusterSigningKeyFilePath) - if err != nil { - return fmt.Errorf("could not read cluster signing private key: %w", err) - } - // TODO: use these value for something useful - _ = clusterSigningCertificatePEM - _ = clusterSigningPrivateKeyPEM - // Load the Kubernetes client configuration (kubeconfig), kubeConfig, err := restclient.InClusterConfig() if err != nil { @@ -169,9 +151,15 @@ func (a *App) run( return fmt.Errorf("could not load config: %w", err) } + // Load the Kubernetes cluster signing CA. + clientCA, err := certauthority.Load(a.clusterSigningCertFilePath, a.clusterSigningKeyFilePath) + if err != nil { + return fmt.Errorf("could not load cluster signing CA: %w", err) + } + webhookTokenAuthenticator, err := config.NewWebhook(cfg.WebhookConfig) if err != nil { - return fmt.Errorf("could create webhook client: %w", err) + return fmt.Errorf("could not create webhook client: %w", err) } podinfo, err := downward.Load(a.downwardAPIPath) @@ -181,19 +169,14 @@ func (a *App) run( // TODO use the postStart hook to generate certs? - ca, err := certauthority.New(pkix.Name{CommonName: "Placeholder CA"}) + apiCA, err := certauthority.New(pkix.Name{CommonName: "Placeholder CA"}) if err != nil { return fmt.Errorf("could not initialize CA: %w", err) } - caBundle, err := ca.Bundle() - if err != nil { - return fmt.Errorf("could not read CA bundle: %w", err) - } - log.Printf("initialized CA bundle:\n%s", string(caBundle)) const serviceName = "placeholder-name-api" - cert, err := ca.Issue( + cert, err := apiCA.Issue( pkix.Name{CommonName: serviceName + "." + podinfo.Namespace + ".svc"}, []string{}, 24*365*time.Hour, @@ -224,7 +207,7 @@ func (a *App) run( Spec: apiregistrationv1.APIServiceSpec{ Group: placeholderv1alpha1.GroupName, Version: placeholderv1alpha1.SchemeGroupVersion.Version, - CABundle: caBundle, + CABundle: apiCA.Bundle(), GroupPriorityMinimum: 2500, // TODO what is the right value? https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.18/#apiservicespec-v1beta1-apiregistration-k8s-io VersionPriority: 10, // TODO what is the right value? https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.18/#apiservicespec-v1beta1-apiregistration-k8s-io }, @@ -239,7 +222,7 @@ func (a *App) run( return fmt.Errorf("could not register API service: %w", err) } - apiServerConfig, err := a.ConfigServer(cert, webhookTokenAuthenticator) + apiServerConfig, err := a.ConfigServer(cert, webhookTokenAuthenticator, clientCA) if err != nil { return err } @@ -252,7 +235,7 @@ func (a *App) run( return server.GenericAPIServer.PrepareRun().Run(ctx.Done()) } -func (a *App) ConfigServer(cert *tls.Certificate, webhookTokenAuthenticator *webhook.WebhookTokenAuthenticator) (*apiserver.Config, error) { +func (a *App) ConfigServer(cert *tls.Certificate, webhookTokenAuthenticator *webhook.WebhookTokenAuthenticator, ca *certauthority.CA) (*apiserver.Config, error) { provider, err := createStaticCertKeyProvider(cert) if err != nil { return nil, fmt.Errorf("could not create static cert key provider: %w", err) @@ -268,6 +251,7 @@ func (a *App) ConfigServer(cert *tls.Certificate, webhookTokenAuthenticator *web GenericConfig: serverConfig, ExtraConfig: apiserver.ExtraConfig{ Webhook: webhookTokenAuthenticator, + Issuer: ca, }, } return apiServerConfig, nil diff --git a/pkg/apiserver/apiserver.go b/pkg/apiserver/apiserver.go index 315389ff..1feb1212 100644 --- a/pkg/apiserver/apiserver.go +++ b/pkg/apiserver/apiserver.go @@ -57,6 +57,7 @@ type Config struct { type ExtraConfig struct { Webhook authenticator.Token + Issuer loginrequest.CertIssuer } type PlaceHolderServer struct { @@ -108,7 +109,7 @@ func (c completedConfig) New() (*PlaceHolderServer, error) { NegotiatedSerializer: Codecs, } - loginRequestStorage := loginrequest.NewREST(c.ExtraConfig.Webhook) + loginRequestStorage := loginrequest.NewREST(c.ExtraConfig.Webhook, c.ExtraConfig.Issuer) v1alpha1Storage, ok := apiGroupInfo.VersionedResourcesStorageMap[gvr.Version] if !ok { diff --git a/pkg/registry/loginrequest/rest.go b/pkg/registry/loginrequest/rest.go index 692b8da1..bc03dd37 100644 --- a/pkg/registry/loginrequest/rest.go +++ b/pkg/registry/loginrequest/rest.go @@ -8,7 +8,9 @@ package loginrequest import ( "context" + "crypto/x509/pkix" "fmt" + "time" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -28,14 +30,20 @@ var ( _ rest.Storage = &REST{} ) -func NewREST(webhook authenticator.Token) *REST { +type CertIssuer interface { + IssuePEM(subject pkix.Name, dnsNames []string, ttl time.Duration) ([]byte, []byte, error) +} + +func NewREST(webhook authenticator.Token, issuer CertIssuer) *REST { return &REST{ webhook: webhook, + issuer: issuer, } } type REST struct { webhook authenticator.Token + issuer CertIssuer } func (r *REST) New() runtime.Object { From 613f324a47c1f71d563dcfec4a0bce36f8d4cdb9 Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Mon, 27 Jul 2020 08:07:34 -0500 Subject: [PATCH 08/10] Add generated mock for loginrequest.CertIssuer interface. Signed-off-by: Matt Moyer --- go.mod | 1 + go.sum | 8 +++ hack/header.txt | 2 + internal/mocks/mockcertissuer/generate.go | 8 +++ .../mocks/mockcertissuer/mockcertissuer.go | 54 +++++++++++++++++++ tools/tools.go | 1 + 6 files changed, 74 insertions(+) create mode 100644 hack/header.txt create mode 100644 internal/mocks/mockcertissuer/generate.go create mode 100644 internal/mocks/mockcertissuer/mockcertissuer.go diff --git a/go.mod b/go.mod index f65ac314..0091acad 100644 --- a/go.mod +++ b/go.mod @@ -4,6 +4,7 @@ go 1.14 require ( github.com/davecgh/go-spew v1.1.1 + github.com/golang/mock v1.4.3 github.com/golangci/golangci-lint v1.28.1 github.com/google/go-cmp v0.4.0 github.com/spf13/cobra v1.0.0 diff --git a/go.sum b/go.sum index 98126ac4..38f799c9 100644 --- a/go.sum +++ b/go.sum @@ -188,7 +188,10 @@ github.com/golang/groupcache v0.0.0-20191227052852-215e87163ea7 h1:5ZkaAPbicIKTF github.com/golang/groupcache v0.0.0-20191227052852-215e87163ea7/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc= github.com/golang/mock v1.1.1/go.mod h1:oTYuIxOrZwtPieC+H1uAHpcLFnEyAGVDL/k47Jfbm0A= github.com/golang/mock v1.2.0/go.mod h1:oTYuIxOrZwtPieC+H1uAHpcLFnEyAGVDL/k47Jfbm0A= +github.com/golang/mock v1.3.1 h1:qGJ6qTW+x6xX/my+8YUVl4WNpX9B7+/l2tRsHGZ7f2s= github.com/golang/mock v1.3.1/go.mod h1:sBzyDLLjw3U8JLTeZvSv8jJB+tU5PVekmnlKIyFUx0Y= +github.com/golang/mock v1.4.3 h1:GV+pQPG/EUUbkh47niozDcADz6go/dUwhVzdUQHIVRw= +github.com/golang/mock v1.4.3/go.mod h1:UOMv5ysSaYNkG+OFQykRIcU/QvvxJf3p21QfJ2Bt3cw= github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/protobuf v1.3.1/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/protobuf v1.3.2/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= @@ -674,6 +677,7 @@ golang.org/x/sys v0.0.0-20200323222414-85ca7c5b95cd/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20200420163511-1957bb5e6d1f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200622214017-ed371f2e16b4 h1:5/PjkGUjvEU5Gl6BxmvKRPpqo2uNMv4rcHBMwzk/st8= golang.org/x/sys v0.0.0-20200622214017-ed371f2e16b4/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.1-0.20180807135948-17ff2d5776d2/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk= @@ -853,6 +857,10 @@ mvdan.cc/lint v0.0.0-20170908181259-adc824a0674b/go.mod h1:2odslEg/xrtNQqCYg2/jC mvdan.cc/unparam v0.0.0-20190720180237-d51796306d8f h1:Cq7MalBHYACRd6EesksG1Q8EoIAKOsiZviGKbOLIej4= mvdan.cc/unparam v0.0.0-20190720180237-d51796306d8f/go.mod h1:4G1h5nDURzA3bwVMZIVpwbkw+04kSxk3rAtzlimaUJw= rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8= +rsc.io/quote/v3 v3.1.0 h1:9JKUTTIUgS6kzR9mK1YuGKv6Nl+DijDNIc0ghT58FaY= +rsc.io/quote/v3 v3.1.0/go.mod h1:yEA65RcK8LyAZtP9Kv3t0HmxON59tX3rD+tICJqUlj0= +rsc.io/sampler v1.3.0 h1:7uVkIFmeBqHfdjD+gZwtXXI+RODJ2Wc4O7MPEh/QiW4= +rsc.io/sampler v1.3.0/go.mod h1:T1hPZKmBbMNahiBKFy5HrXp6adAjACjK9JXDnKaTXpA= sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.0.9 h1:rusRLrDhjBp6aYtl9sGEvQJr6faoHoDLd0YcUBTZguI= sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.0.9/go.mod h1:dzAXnQbTRyDlZPJX2SUPEqvnB+j7AJjtlox7PEwigU0= sigs.k8s.io/structured-merge-diff/v3 v3.0.0-20200116222232-67a7b8c61874/go.mod h1:PlARxl6Hbt/+BC80dRLi1qAmnMqwqDg62YvvVkZjemw= diff --git a/hack/header.txt b/hack/header.txt new file mode 100644 index 00000000..9c120c6d --- /dev/null +++ b/hack/header.txt @@ -0,0 +1,2 @@ +Copyright 2020 VMware, Inc. +SPDX-License-Identifier: Apache-2.0 \ No newline at end of file diff --git a/internal/mocks/mockcertissuer/generate.go b/internal/mocks/mockcertissuer/generate.go new file mode 100644 index 00000000..f0cc9b57 --- /dev/null +++ b/internal/mocks/mockcertissuer/generate.go @@ -0,0 +1,8 @@ +/* +Copyright 2020 VMware, Inc. +SPDX-License-Identifier: Apache-2.0 +*/ + +package mockcertissuer + +//go:generate go run -v github.com/golang/mock/mockgen -destination=mockcertissuer.go -package=mockcertissuer -copyright_file=../../../hack/header.txt github.com/suzerain-io/placeholder-name/pkg/registry/loginrequest CertIssuer diff --git a/internal/mocks/mockcertissuer/mockcertissuer.go b/internal/mocks/mockcertissuer/mockcertissuer.go new file mode 100644 index 00000000..ebb64d5f --- /dev/null +++ b/internal/mocks/mockcertissuer/mockcertissuer.go @@ -0,0 +1,54 @@ +// Copyright 2020 VMware, Inc. +// SPDX-License-Identifier: Apache-2.0 + +// Code generated by MockGen. DO NOT EDIT. +// Source: github.com/suzerain-io/placeholder-name/pkg/registry/loginrequest (interfaces: CertIssuer) + +// Package mockcertissuer is a generated GoMock package. +package mockcertissuer + +import ( + pkix "crypto/x509/pkix" + gomock "github.com/golang/mock/gomock" + reflect "reflect" + time "time" +) + +// MockCertIssuer is a mock of CertIssuer interface +type MockCertIssuer struct { + ctrl *gomock.Controller + recorder *MockCertIssuerMockRecorder +} + +// MockCertIssuerMockRecorder is the mock recorder for MockCertIssuer +type MockCertIssuerMockRecorder struct { + mock *MockCertIssuer +} + +// NewMockCertIssuer creates a new mock instance +func NewMockCertIssuer(ctrl *gomock.Controller) *MockCertIssuer { + mock := &MockCertIssuer{ctrl: ctrl} + mock.recorder = &MockCertIssuerMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use +func (m *MockCertIssuer) EXPECT() *MockCertIssuerMockRecorder { + return m.recorder +} + +// IssuePEM mocks base method +func (m *MockCertIssuer) IssuePEM(arg0 pkix.Name, arg1 []string, arg2 time.Duration) ([]byte, []byte, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "IssuePEM", arg0, arg1, arg2) + ret0, _ := ret[0].([]byte) + ret1, _ := ret[1].([]byte) + ret2, _ := ret[2].(error) + return ret0, ret1, ret2 +} + +// IssuePEM indicates an expected call of IssuePEM +func (mr *MockCertIssuerMockRecorder) IssuePEM(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IssuePEM", reflect.TypeOf((*MockCertIssuer)(nil).IssuePEM), arg0, arg1, arg2) +} diff --git a/tools/tools.go b/tools/tools.go index f57e7842..7dd1bb2f 100644 --- a/tools/tools.go +++ b/tools/tools.go @@ -9,5 +9,6 @@ SPDX-License-Identifier: Apache-2.0 package tools import ( + _ "github.com/golang/mock/mockgen" _ "github.com/golangci/golangci-lint/cmd/golangci-lint" ) From 8606cc96624f6ea29db7e9e3bfc6679af262e024 Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Mon, 27 Jul 2020 08:08:39 -0500 Subject: [PATCH 09/10] Update loginrequest/REST.Create to issue client certificates. Signed-off-by: Matt Moyer --- pkg/registry/loginrequest/rest.go | 30 ++++--- pkg/registry/loginrequest/rest_test.go | 106 ++++++++++++++++++++----- 2 files changed, 103 insertions(+), 33 deletions(-) diff --git a/pkg/registry/loginrequest/rest.go b/pkg/registry/loginrequest/rest.go index bc03dd37..bd0b0e1e 100644 --- a/pkg/registry/loginrequest/rest.go +++ b/pkg/registry/loginrequest/rest.go @@ -118,32 +118,36 @@ func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation klog.Warningf("webhook authentication failure: %v", err) return failureResponse(), nil } - - var out *placeholderapi.LoginRequest - if authenticated && authResponse.User != nil && authResponse.User.GetName() != "" { - out = successfulResponse(authResponse) - } else { - out = failureResponse() + if !authenticated || authResponse.User == nil || authResponse.User.GetName() == "" { + return failureResponse(), nil } - return out, nil -} + certPEM, keyPEM, err := r.issuer.IssuePEM( + pkix.Name{ + CommonName: authResponse.User.GetName(), + OrganizationalUnit: authResponse.User.GetGroups(), + }, + []string{}, + 5*time.Minute, + ) + if err != nil { + klog.Warningf("failed to issue short lived client certificate: %v", err) + return failureResponse(), nil + } -func successfulResponse(authResponse *authenticator.Response) *placeholderapi.LoginRequest { return &placeholderapi.LoginRequest{ Status: placeholderapi.LoginRequestStatus{ Credential: &placeholderapi.LoginRequestCredential{ ExpirationTimestamp: nil, - Token: "snorlax", - ClientCertificateData: "", - ClientKeyData: "", + ClientCertificateData: string(certPEM), + ClientKeyData: string(keyPEM), }, User: &placeholderapi.User{ Name: authResponse.User.GetName(), Groups: authResponse.User.GetGroups(), }, }, - } + }, nil } func failureResponse() *placeholderapi.LoginRequest { diff --git a/pkg/registry/loginrequest/rest_test.go b/pkg/registry/loginrequest/rest_test.go index 43424276..4368de19 100644 --- a/pkg/registry/loginrequest/rest_test.go +++ b/pkg/registry/loginrequest/rest_test.go @@ -7,11 +7,13 @@ package loginrequest import ( "context" + "crypto/x509/pkix" "errors" "fmt" "testing" "time" + "github.com/golang/mock/gomock" "github.com/stretchr/testify/require" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -22,6 +24,7 @@ import ( "k8s.io/apiserver/pkg/registry/rest" placeholderapi "github.com/suzerain-io/placeholder-name-api/pkg/apis/placeholder" + "github.com/suzerain-io/placeholder-name/internal/mocks/mockcertissuer" ) type contextKey struct{} @@ -113,7 +116,18 @@ func requireSuccessfulResponseWithAuthenticationFailureMessage(t *testing.T, err }) } +func successfulIssuer(ctrl *gomock.Controller) CertIssuer { + issuer := mockcertissuer.NewMockCertIssuer(ctrl) + issuer.EXPECT(). + IssuePEM(gomock.Any(), gomock.Any(), gomock.Any()). + Return([]byte("test-cert"), []byte("test-key"), nil) + return issuer +} + func TestCreateSucceedsWhenGivenATokenAndTheWebhookAuthenticatesTheToken(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + webhook := FakeToken{ returnResponse: &authenticator.Response{ User: &user.DefaultInfo{ @@ -123,7 +137,17 @@ func TestCreateSucceedsWhenGivenATokenAndTheWebhookAuthenticatesTheToken(t *test }, returnUnauthenticated: false, } - storage := NewREST(&webhook) + + issuer := mockcertissuer.NewMockCertIssuer(ctrl) + issuer.EXPECT().IssuePEM( + pkix.Name{ + CommonName: "test-user", + OrganizationalUnit: []string{"test-group-1", "test-group-2"}}, + []string{}, + 5*time.Minute, + ).Return([]byte("test-cert"), []byte("test-key"), nil) + + storage := NewREST(&webhook, issuer) requestToken := "a token" response, err := callCreate(context.Background(), storage, validLoginRequestWithToken(requestToken)) @@ -137,9 +161,8 @@ func TestCreateSucceedsWhenGivenATokenAndTheWebhookAuthenticatesTheToken(t *test }, Credential: &placeholderapi.LoginRequestCredential{ ExpirationTimestamp: nil, - Token: "snorlax", - ClientCertificateData: "", - ClientKeyData: "", + ClientCertificateData: "test-cert", + ClientKeyData: "test-key", }, Message: "", }, @@ -147,11 +170,38 @@ func TestCreateSucceedsWhenGivenATokenAndTheWebhookAuthenticatesTheToken(t *test require.Equal(t, requestToken, webhook.calledWithToken) } +func TestCreateFailsWithValidTokenWhenCertIssuerFails(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + webhook := FakeToken{ + returnResponse: &authenticator.Response{ + User: &user.DefaultInfo{ + Name: "test-user", + Groups: []string{"test-group-1", "test-group-2"}, + }, + }, + returnUnauthenticated: false, + } + + issuer := mockcertissuer.NewMockCertIssuer(ctrl) + issuer.EXPECT(). + IssuePEM(gomock.Any(), gomock.Any(), gomock.Any()). + Return(nil, nil, fmt.Errorf("some certificate authority error")) + + storage := NewREST(&webhook, issuer) + requestToken := "a token" + + response, err := callCreate(context.Background(), storage, validLoginRequestWithToken(requestToken)) + requireSuccessfulResponseWithAuthenticationFailureMessage(t, err, response) + require.Equal(t, requestToken, webhook.calledWithToken) +} + func TestCreateSucceedsWithAnUnauthenticatedStatusWhenGivenATokenAndTheWebhookDoesNotAuthenticateTheToken(t *testing.T) { webhook := FakeToken{ returnUnauthenticated: true, } - storage := NewREST(&webhook) + storage := NewREST(&webhook, nil) requestToken := "a token" response, err := callCreate(context.Background(), storage, validLoginRequestWithToken(requestToken)) @@ -164,7 +214,7 @@ func TestCreateSucceedsWithAnUnauthenticatedStatusWhenWebhookFails(t *testing.T) webhook := FakeToken{ returnErr: errors.New("some webhook error"), } - storage := NewREST(&webhook) + storage := NewREST(&webhook, nil) response, err := callCreate(context.Background(), storage, validLoginRequest()) @@ -175,7 +225,7 @@ func TestCreateSucceedsWithAnUnauthenticatedStatusWhenWebhookDoesNotReturnAnyUse webhook := FakeToken{ returnResponse: &authenticator.Response{}, } - storage := NewREST(&webhook) + storage := NewREST(&webhook, nil) response, err := callCreate(context.Background(), storage, validLoginRequest()) @@ -190,7 +240,7 @@ func TestCreateSucceedsWithAnUnauthenticatedStatusWhenWebhookReturnsAnEmptyUsern }, }, } - storage := NewREST(&webhook) + storage := NewREST(&webhook, nil) response, err := callCreate(context.Background(), storage, validLoginRequest()) @@ -198,10 +248,13 @@ func TestCreateSucceedsWithAnUnauthenticatedStatusWhenWebhookReturnsAnEmptyUsern } func TestCreateDoesNotPassAdditionalContextInfoToTheWebhook(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + webhook := FakeToken{ returnResponse: webhookSuccessResponse(), } - storage := NewREST(&webhook) + storage := NewREST(&webhook, successfulIssuer(ctrl)) ctx := context.WithValue(context.Background(), contextKey{}, "context-value") _, err := callCreate(ctx, storage, validLoginRequest()) @@ -212,7 +265,7 @@ func TestCreateDoesNotPassAdditionalContextInfoToTheWebhook(t *testing.T) { func TestCreateFailsWhenGivenTheWrongInputType(t *testing.T) { notALoginRequest := runtime.Unknown{} - response, err := NewREST(&FakeToken{}).Create( + response, err := NewREST(&FakeToken{}, nil).Create( genericapirequest.NewContext(), ¬ALoginRequest, rest.ValidateAllObjectFunc, @@ -222,7 +275,7 @@ func TestCreateFailsWhenGivenTheWrongInputType(t *testing.T) { } func TestCreateFailsWhenTokenIsNilInRequest(t *testing.T) { - storage := NewREST(&FakeToken{}) + storage := NewREST(&FakeToken{}, nil) response, err := callCreate(context.Background(), storage, loginRequest(placeholderapi.LoginRequestSpec{ Type: placeholderapi.TokenLoginCredentialType, Token: nil, @@ -233,7 +286,7 @@ func TestCreateFailsWhenTokenIsNilInRequest(t *testing.T) { } func TestCreateFailsWhenTypeInRequestIsMissing(t *testing.T) { - storage := NewREST(&FakeToken{}) + storage := NewREST(&FakeToken{}, nil) response, err := callCreate(context.Background(), storage, loginRequest(placeholderapi.LoginRequestSpec{ Type: "", Token: &placeholderapi.LoginRequestTokenCredential{Value: "a token"}, @@ -244,7 +297,7 @@ func TestCreateFailsWhenTypeInRequestIsMissing(t *testing.T) { } func TestCreateFailsWhenTypeInRequestIsNotLegal(t *testing.T) { - storage := NewREST(&FakeToken{}) + storage := NewREST(&FakeToken{}, nil) response, err := callCreate(context.Background(), storage, loginRequest(placeholderapi.LoginRequestSpec{ Type: "this in an invalid type", Token: &placeholderapi.LoginRequestTokenCredential{Value: "a token"}, @@ -255,7 +308,7 @@ func TestCreateFailsWhenTypeInRequestIsNotLegal(t *testing.T) { } func TestCreateFailsWhenTokenValueIsEmptyInRequest(t *testing.T) { - storage := NewREST(&FakeToken{}) + storage := NewREST(&FakeToken{}, nil) response, err := callCreate(context.Background(), storage, loginRequest(placeholderapi.LoginRequestSpec{ Type: placeholderapi.TokenLoginCredentialType, Token: &placeholderapi.LoginRequestTokenCredential{Value: ""}, @@ -266,7 +319,7 @@ func TestCreateFailsWhenTokenValueIsEmptyInRequest(t *testing.T) { } func TestCreateFailsWhenValidationFails(t *testing.T) { - storage := NewREST(&FakeToken{}) + storage := NewREST(&FakeToken{}, nil) response, err := storage.Create( context.Background(), validLoginRequest(), @@ -279,10 +332,13 @@ func TestCreateFailsWhenValidationFails(t *testing.T) { } func TestCreateDoesNotAllowValidationFunctionToMutateRequest(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + webhook := FakeToken{ returnResponse: webhookSuccessResponse(), } - storage := NewREST(&webhook) + storage := NewREST(&webhook, successfulIssuer(ctrl)) requestToken := "a token" response, err := storage.Create( context.Background(), @@ -299,10 +355,14 @@ func TestCreateDoesNotAllowValidationFunctionToMutateRequest(t *testing.T) { } func TestCreateDoesNotAllowValidationFunctionToSeeTheActualRequestToken(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + webhook := FakeToken{ returnResponse: webhookSuccessResponse(), } - storage := NewREST(&webhook) + + storage := NewREST(&webhook, successfulIssuer(ctrl)) validationFunctionWasCalled := false var validationFunctionSawTokenValue string response, err := storage.Create( @@ -322,7 +382,7 @@ func TestCreateDoesNotAllowValidationFunctionToSeeTheActualRequestToken(t *testi } func TestCreateFailsWhenRequestOptionsDryRunIsNotEmpty(t *testing.T) { - response, err := NewREST(&FakeToken{}).Create( + response, err := NewREST(&FakeToken{}, nil).Create( genericapirequest.NewContext(), validLoginRequest(), rest.ValidateAllObjectFunc, @@ -335,13 +395,16 @@ func TestCreateFailsWhenRequestOptionsDryRunIsNotEmpty(t *testing.T) { } func TestCreateCancelsTheWebhookInvocationWhenTheCallToCreateIsCancelledItself(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + webhookStartedRunningNotificationChan := make(chan bool) webhook := FakeToken{ timeout: time.Second * 2, webhookStartedRunningNotificationChan: webhookStartedRunningNotificationChan, returnResponse: webhookSuccessResponse(), } - storage := NewREST(&webhook) + storage := NewREST(&webhook, successfulIssuer(ctrl)) ctx, cancel := context.WithCancel(context.Background()) c := make(chan bool) @@ -362,13 +425,16 @@ func TestCreateCancelsTheWebhookInvocationWhenTheCallToCreateIsCancelledItself(t } func TestCreateAllowsTheWebhookInvocationToFinishWhenTheCallToCreateIsNotCancelledItself(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + webhookStartedRunningNotificationChan := make(chan bool) webhook := FakeToken{ timeout: 0, webhookStartedRunningNotificationChan: webhookStartedRunningNotificationChan, returnResponse: webhookSuccessResponse(), } - storage := NewREST(&webhook) + storage := NewREST(&webhook, successfulIssuer(ctrl)) ctx, cancel := context.WithCancel(context.Background()) defer cancel() From a1593c4b7b77421cc5ff0efc9c24e00c01961ea0 Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Mon, 27 Jul 2020 08:18:37 -0500 Subject: [PATCH 10/10] Fix linter error in certauthority. The error was: ``` internal/certauthority/certauthority.go:68:15: err113: do not define dynamic errors, use wrapped static errors instead: "fmt.Errorf(\"expected CA to be a single certificate, found %d certificates\", certCount)" (goerr113) return nil, fmt.Errorf("expected CA to be a single certificate, found %d certificates", certCount) ^ exit status 1 ``` I'm not sure if I love this err113 linter. --- internal/certauthority/certauthority.go | 5 ++++- internal/certauthority/certauthority_test.go | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/internal/certauthority/certauthority.go b/internal/certauthority/certauthority.go index 8c4d6e54..a95c67b2 100644 --- a/internal/certauthority/certauthority.go +++ b/internal/certauthority/certauthority.go @@ -58,6 +58,9 @@ func secureEnv() env { } } +// ErrInvalidCACertificate is returned when the contents of the loaded CA certificate do not meet our assumptions. +var ErrInvalidCACertificate = fmt.Errorf("invalid CA certificate") + // Load a certificate authority from an existing certificate and private key (in PEM format). func Load(certPath string, keyPath string) (*CA, error) { cert, err := tls.LoadX509KeyPair(certPath, keyPath) @@ -65,7 +68,7 @@ func Load(certPath string, keyPath string) (*CA, error) { return nil, fmt.Errorf("could not load CA: %w", err) } if certCount := len(cert.Certificate); certCount != 1 { - return nil, fmt.Errorf("expected CA to be a single certificate, found %d certificates", certCount) + return nil, fmt.Errorf("%w: expected a single certificate, found %d certificates", ErrInvalidCACertificate, certCount) } return &CA{ caCertBytes: cert.Certificate[0], diff --git a/internal/certauthority/certauthority_test.go b/internal/certauthority/certauthority_test.go index f0ae2dd8..0b89869f 100644 --- a/internal/certauthority/certauthority_test.go +++ b/internal/certauthority/certauthority_test.go @@ -72,7 +72,7 @@ func TestLoad(t *testing.T) { name: "multiple certs", certPath: "./testdata/multiple.crt", keyPath: "./testdata/test.key", - wantErr: "expected CA to be a single certificate, found 2 certificates", + wantErr: "invalid CA certificate: expected a single certificate, found 2 certificates", }, { name: "success",