certauthority.go: Refactor issuing client versus server certs

We were previously issuing both client certs and server certs with
both extended key usages included. Split the Issue*() methods into
separate methods for issuing server certs versus client certs so
they can have different extended key usages tailored for each use
case.

Also took the opportunity to clean up the parameters of the Issue*()
methods and New() methods to more closely match how we prefer to call
them. We were always only passing the common name part of the
pkix.Name to New(), so now the New() method just takes the common name
as a string. When making a server cert, we don't need to set the
deprecated common name field, so remove that param. When making a client
cert, we're always making it in the format expected by the Kube API
server, so just accept the username and group as parameters directly.

cmd/local-user-authenticator/main_test.go

@@ -8,7 +8,6 @@ import (
 	"context"
 	"crypto/tls"
 	"crypto/x509"
-	"crypto/x509/pkix"
 	"encoding/json"
 	"fmt"
 	"io"
@@ -464,10 +463,10 @@ func newCertProvider(t *testing.T) (dynamiccert.Provider, []byte, string) {
 
 	serverName := "local-user-authenticator"
 
-	ca, err := certauthority.New(pkix.Name{CommonName: serverName + " CA"}, time.Hour*24)
+	ca, err := certauthority.New(serverName+" CA", time.Hour*24)
 	require.NoError(t, err)
 
-	cert, err := ca.Issue(pkix.Name{CommonName: serverName}, []string{serverName}, nil, time.Hour*24)
+	cert, err := ca.IssueServerCert([]string{serverName}, nil, time.Hour*24)
 	require.NoError(t, err)
 
 	certPEM, keyPEM, err := certauthority.ToPEM(cert)

cmd/pinniped/cmd/flag_types_test.go

@@ -5,7 +5,6 @@ package cmd
 
 import (
 	"bytes"
-	"crypto/x509/pkix"
 	"fmt"
 	"io/ioutil"
 	"path/filepath"
@@ -51,7 +50,7 @@ func TestConciergeModeFlag(t *testing.T) {
 }
 
 func TestCABundleFlag(t *testing.T) {
-	testCA, err := certauthority.New(pkix.Name{CommonName: "Test CA"}, 1*time.Hour)
+	testCA, err := certauthority.New("Test CA", 1*time.Hour)
 	require.NoError(t, err)
 	tmpdir := testutil.TempDir(t)
 	emptyFilePath := filepath.Join(tmpdir, "empty")

cmd/pinniped/cmd/kubeconfig_test.go

@@ -5,7 +5,6 @@ package cmd
 
 import (
 	"bytes"
-	"crypto/x509/pkix"
 	"encoding/base64"
 	"fmt"
 	"io/ioutil"
@@ -30,13 +29,13 @@ import (
 )
 
 func TestGetKubeconfig(t *testing.T) {
-	testOIDCCA, err := certauthority.New(pkix.Name{CommonName: "Test CA"}, 1*time.Hour)
+	testOIDCCA, err := certauthority.New("Test CA", 1*time.Hour)
 	require.NoError(t, err)
 	tmpdir := testutil.TempDir(t)
 	testOIDCCABundlePath := filepath.Join(tmpdir, "testca.pem")
 	require.NoError(t, ioutil.WriteFile(testOIDCCABundlePath, testOIDCCA.Bundle(), 0600))
 
-	testConciergeCA, err := certauthority.New(pkix.Name{CommonName: "Test Concierge CA"}, 1*time.Hour)
+	testConciergeCA, err := certauthority.New("Test Concierge CA", 1*time.Hour)
 	require.NoError(t, err)
 	testConciergeCABundlePath := filepath.Join(tmpdir, "testconciergeca.pem")
 	require.NoError(t, ioutil.WriteFile(testConciergeCABundlePath, testConciergeCA.Bundle(), 0600))

cmd/pinniped/cmd/login_oidc_test.go

@@ -6,7 +6,6 @@ package cmd
 import (
 	"bytes"
 	"context"
-	"crypto/x509/pkix"
 	"encoding/base64"
 	"fmt"
 	"io/ioutil"
@@ -29,7 +28,7 @@ import (
 func TestLoginOIDCCommand(t *testing.T) {
 	cfgDir := mustGetConfigDir()
 
-	testCA, err := certauthority.New(pkix.Name{CommonName: "Test CA"}, 1*time.Hour)
+	testCA, err := certauthority.New("Test CA", 1*time.Hour)
 	require.NoError(t, err)
 	tmpdir := testutil.TempDir(t)
 	testCABundlePath := filepath.Join(tmpdir, "testca.pem")

cmd/pinniped/cmd/login_static_test.go

@@ -6,7 +6,6 @@ package cmd
 import (
 	"bytes"
 	"context"
-	"crypto/x509/pkix"
 	"fmt"
 	"io/ioutil"
 	"path/filepath"
@@ -24,7 +23,7 @@ import (
 )
 
 func TestLoginStaticCommand(t *testing.T) {
-	testCA, err := certauthority.New(pkix.Name{CommonName: "Test CA"}, 1*time.Hour)
+	testCA, err := certauthority.New("Test CA", 1*time.Hour)
 	require.NoError(t, err)
 	tmpdir := testutil.TempDir(t)
 	testCABundlePath := filepath.Join(tmpdir, "testca.pem")

internal/certauthority/certauthority.go

@@ -89,13 +89,13 @@ func Load(certPEM string, keyPEM string) (*CA, error) {
 	}, nil
 }
 
-// New generates a fresh certificate authority with the given subject and ttl.
+// New generates a fresh certificate authority with the given Common Name and TTL.
-func New(subject pkix.Name, ttl time.Duration) (*CA, error) {
+func New(commonName string, ttl time.Duration) (*CA, error) {
-	return newInternal(subject, ttl, secureEnv())
+	return newInternal(commonName, ttl, secureEnv())
 }
 
 // newInternal is the internal guts of New, broken out for easier testing.
-func newInternal(subject pkix.Name, ttl time.Duration, env env) (*CA, error) {
+func newInternal(commonName string, ttl time.Duration, env env) (*CA, error) {
 	ca := CA{env: env}
 	// Generate a random serial for the CA
 	serialNumber, err := randomSerial(env.serialRNG)
@@ -118,7 +118,7 @@ func newInternal(subject pkix.Name, ttl time.Duration, env env) (*CA, error) {
 	// Create CA cert template
 	caTemplate := x509.Certificate{
 		SerialNumber:          serialNumber,
-		Subject:               subject,
+		Subject:               pkix.Name{CommonName: commonName},
 		NotBefore:             notBefore,
 		NotAfter:              notAfter,
 		IsCA:                  true,
@@ -160,8 +160,31 @@ func (c *CA) Pool() *x509.CertPool {
 	return pool
 }
 
-// Issue a new server certificate for the given identity and duration.
+// IssueClientCert issues a new client certificate with username and groups included in the Kube-style
-func (c *CA) Issue(subject pkix.Name, dnsNames []string, ips []net.IP, ttl time.Duration) (*tls.Certificate, error) {
+// certificate subject for the given identity and duration.
+func (c *CA) IssueClientCert(username string, groups []string, ttl time.Duration) (*tls.Certificate, error) {
+	return c.issueCert(x509.ExtKeyUsageClientAuth, pkix.Name{CommonName: username, Organization: groups}, nil, nil, ttl)
+}
+
+// IssueServerCert issues a new server certificate for the given identity and duration.
+// The dnsNames and ips are each optional, but at least one of them should be specified.
+func (c *CA) IssueServerCert(dnsNames []string, ips []net.IP, ttl time.Duration) (*tls.Certificate, error) {
+	return c.issueCert(x509.ExtKeyUsageServerAuth, pkix.Name{}, dnsNames, ips, ttl)
+}
+
+// Similar to IssueClientCert, but returning the new cert as a pair of PEM-formatted byte slices
+// for the certificate and private key.
+func (c *CA) IssueClientCertPEM(username string, groups []string, ttl time.Duration) ([]byte, []byte, error) {
+	return toPEM(c.IssueClientCert(username, groups, ttl))
+}
+
+// Similar to IssueServerCert, but returning the new cert as a pair of PEM-formatted byte slices
+// for the certificate and private key.
+func (c *CA) IssueServerCertPEM(dnsNames []string, ips []net.IP, ttl time.Duration) ([]byte, []byte, error) {
+	return toPEM(c.IssueServerCert(dnsNames, ips, ttl))
+}
+
+func (c *CA) issueCert(extKeyUsage x509.ExtKeyUsage, subject pkix.Name, dnsNames []string, ips []net.IP, ttl time.Duration) (*tls.Certificate, error) {
 	// Choose a random 128 bit serial number.
 	serialNumber, err := randomSerial(c.env.serialRNG)
 	if err != nil {
@@ -187,13 +210,11 @@ func (c *CA) Issue(subject pkix.Name, dnsNames []string, ips []net.IP, ttl time.
 
 	// Sign a cert, getting back the DER-encoded certificate bytes.
 	template := x509.Certificate{
-		SerialNumber: serialNumber,
+		SerialNumber:          serialNumber,
-		Subject:      subject,
+		Subject:               subject,
-		NotBefore:    notBefore,
+		NotBefore:             notBefore,
-		NotAfter:     notAfter,
+		NotAfter:              notAfter,
-		KeyUsage:     x509.KeyUsageDigitalSignature,
+		ExtKeyUsage:           []x509.ExtKeyUsage{extKeyUsage},
-		// TODO split this function into two funcs that handle client and serving certs differently
-		ExtKeyUsage:           []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageClientAuth},
 		BasicConstraintsValid: true,
 		IsCA:                  false,
 		DNSNames:              dnsNames,
@@ -218,14 +239,8 @@ func (c *CA) Issue(subject pkix.Name, dnsNames []string, ips []net.IP, ttl time.
 	}, 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, ips []net.IP, ttl time.Duration) ([]byte, []byte, error) {
-	return toPEM(c.Issue(subject, dnsNames, ips, ttl))
-}
-
 func toPEM(cert *tls.Certificate, err error) ([]byte, []byte, error) {
-	// If the wrapped Issue() returned an error, pass it back.
+	// If the wrapped IssueServerCert() returned an error, pass it back.
 	if err != nil {
 		return nil, nil, err
 	}

internal/certauthority/certauthority_test.go

@@ -7,7 +7,6 @@ import (
 	"crypto"
 	"crypto/tls"
 	"crypto/x509"
-	"crypto/x509/pkix"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -17,6 +16,8 @@ import (
 	"time"
 
 	"github.com/stretchr/testify/require"
+
+	"go.pinniped.dev/internal/testutil"
 )
 
 func loadFromFiles(t *testing.T, certPath string, keyPath string) (*CA, error) {
@@ -87,7 +88,7 @@ func TestLoad(t *testing.T) {
 
 func TestNew(t *testing.T) {
 	now := time.Now()
-	ca, err := New(pkix.Name{CommonName: "Test CA"}, time.Minute)
+	ca, err := New("Test CA", time.Minute)
 	require.NoError(t, err)
 	require.NotNil(t, ca)
 
@@ -158,7 +159,7 @@ func TestNewInternal(t *testing.T) {
 	for _, tt := range tests {
 		tt := tt
 		t.Run(tt.name, func(t *testing.T) {
-			got, err := newInternal(pkix.Name{CommonName: "Test CA"}, tt.ttl, tt.env)
+			got, err := newInternal("Test CA", tt.ttl, tt.env)
 			if tt.wantErr != "" {
 				require.EqualError(t, err, tt.wantErr)
 				require.Nil(t, got)
@@ -184,7 +185,7 @@ func TestBundle(t *testing.T) {
 }
 
 func TestPrivateKeyToPEM(t *testing.T) {
-	ca, err := New(pkix.Name{CommonName: "Test CA"}, time.Hour)
+	ca, err := New("Test CA", time.Hour)
 	require.NoError(t, err)
 	keyPEM, err := ca.PrivateKeyToPEM()
 	require.NoError(t, err)
@@ -201,7 +202,7 @@ func TestPrivateKeyToPEM(t *testing.T) {
 }
 
 func TestPool(t *testing.T) {
-	ca, err := New(pkix.Name{CommonName: "test"}, 1*time.Hour)
+	ca, err := New("test", 1*time.Hour)
 	require.NoError(t, err)
 
 	pool := ca.Pool()
@@ -220,6 +221,8 @@ func (e *errSigner) Sign(_ io.Reader, _ []byte, _ crypto.SignerOpts) ([]byte, er
 }
 
 func TestIssue(t *testing.T) {
+	const numRandBytes = 64 * 2 // each call to issue a cert will consume 64 bytes from the reader
+
 	now := time.Date(2020, 7, 10, 12, 41, 12, 1234, time.UTC)
 
 	realCA, err := loadFromFiles(t, "./testdata/test.crt", "./testdata/test.key")
@@ -243,7 +246,7 @@ func TestIssue(t *testing.T) {
 			name: "failed to generate keypair",
 			ca: CA{
 				env: env{
-					serialRNG: strings.NewReader(strings.Repeat("x", 64)),
+					serialRNG: strings.NewReader(strings.Repeat("x", numRandBytes)),
 					keygenRNG: strings.NewReader(""),
 				},
 			},
@@ -253,8 +256,8 @@ func TestIssue(t *testing.T) {
 			name: "invalid CA certificate",
 			ca: CA{
 				env: env{
-					serialRNG: strings.NewReader(strings.Repeat("x", 64)),
+					serialRNG: strings.NewReader(strings.Repeat("x", numRandBytes)),
-					keygenRNG: strings.NewReader(strings.Repeat("x", 64)),
+					keygenRNG: strings.NewReader(strings.Repeat("x", numRandBytes)),
 					clock:     func() time.Time { return now },
 				},
 			},
@@ -264,8 +267,8 @@ func TestIssue(t *testing.T) {
 			name: "signing error",
 			ca: CA{
 				env: env{
-					serialRNG: strings.NewReader(strings.Repeat("x", 64)),
+					serialRNG: strings.NewReader(strings.Repeat("x", numRandBytes)),
-					keygenRNG: strings.NewReader(strings.Repeat("x", 64)),
+					keygenRNG: strings.NewReader(strings.Repeat("x", numRandBytes)),
 					clock:     func() time.Time { return now },
 				},
 				caCertBytes: realCA.caCertBytes,
@@ -277,11 +280,11 @@ func TestIssue(t *testing.T) {
 			wantErr: "could not sign certificate: some signer error",
 		},
 		{
-			name: "success",
+			name: "parse certificate error",
 			ca: CA{
 				env: env{
-					serialRNG: strings.NewReader(strings.Repeat("x", 64)),
+					serialRNG: strings.NewReader(strings.Repeat("x", numRandBytes)),
-					keygenRNG: strings.NewReader(strings.Repeat("x", 64)),
+					keygenRNG: strings.NewReader(strings.Repeat("x", numRandBytes)),
 					clock:     func() time.Time { return now },
 					parseCert: func(_ []byte) (*x509.Certificate, error) {
 						return nil, fmt.Errorf("some parse certificate error")
@@ -296,8 +299,8 @@ func TestIssue(t *testing.T) {
 			name: "success",
 			ca: CA{
 				env: env{
-					serialRNG: strings.NewReader(strings.Repeat("x", 64)),
+					serialRNG: strings.NewReader(strings.Repeat("x", numRandBytes)),
-					keygenRNG: strings.NewReader(strings.Repeat("x", 64)),
+					keygenRNG: strings.NewReader(strings.Repeat("x", numRandBytes)),
 					clock:     func() time.Time { return now },
 					parseCert: x509.ParseCertificate,
 				},
@@ -309,28 +312,26 @@ func TestIssue(t *testing.T) {
 	for _, tt := range tests {
 		tt := tt
 		t.Run(tt.name, func(t *testing.T) {
-			got, err := tt.ca.Issue(pkix.Name{CommonName: "Test Server"}, []string{"example.com"}, []net.IP{net.IPv4(1, 2, 3, 4)}, 10*time.Minute)
+			got, err := tt.ca.IssueServerCert([]string{"example.com"}, []net.IP{net.IPv4(1, 2, 3, 4)}, 10*time.Minute)
 			if tt.wantErr != "" {
 				require.EqualError(t, err, tt.wantErr)
 				require.Nil(t, got)
-				return
+			} else {
+				require.NoError(t, err)
+				require.NotNil(t, got)
+			}
+			got, err = tt.ca.IssueClientCert("test-user", []string{"group1", "group2"}, 10*time.Minute)
+			if tt.wantErr != "" {
+				require.EqualError(t, err, tt.wantErr)
+				require.Nil(t, got)
+			} else {
+				require.NoError(t, err)
+				require.NotNil(t, got)
 			}
-			require.NoError(t, err)
-			require.NotNil(t, got)
 		})
 	}
 }
 
-func TestIssuePEM(t *testing.T) {
-	realCA, err := loadFromFiles(t, "./testdata/test.crt", "./testdata/test.key")
-	require.NoError(t, err)
-
-	certPEM, keyPEM, err := realCA.IssuePEM(pkix.Name{CommonName: "Test Server"}, []string{"example.com"}, nil, 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)
@@ -358,3 +359,90 @@ func TestToPEM(t *testing.T) {
 		require.NotEmpty(t, keyPEM)
 	})
 }
+
+func TestIssueMethods(t *testing.T) {
+	// One CA can be used to issue both kinds of certs.
+	ca, err := New("Test CA", time.Hour)
+	require.NoError(t, err)
+
+	ttl := 121 * time.Hour
+
+	t.Run("client certs", func(t *testing.T) {
+		user := "test-username"
+		groups := []string{"group1", "group2"}
+
+		clientCert, err := ca.IssueClientCert(user, groups, ttl)
+		require.NoError(t, err)
+		certPEM, keyPEM, err := ToPEM(clientCert)
+		require.NoError(t, err)
+		validateClientCert(t, ca.Bundle(), certPEM, keyPEM, user, groups, ttl)
+
+		certPEM, keyPEM, err = ca.IssueClientCertPEM(user, groups, ttl)
+		require.NoError(t, err)
+		validateClientCert(t, ca.Bundle(), certPEM, keyPEM, user, groups, ttl)
+
+		certPEM, keyPEM, err = ca.IssueClientCertPEM(user, nil, ttl)
+		require.NoError(t, err)
+		validateClientCert(t, ca.Bundle(), certPEM, keyPEM, user, nil, ttl)
+
+		certPEM, keyPEM, err = ca.IssueClientCertPEM(user, []string{}, ttl)
+		require.NoError(t, err)
+		validateClientCert(t, ca.Bundle(), certPEM, keyPEM, user, nil, ttl)
+
+		certPEM, keyPEM, err = ca.IssueClientCertPEM("", []string{}, ttl)
+		require.NoError(t, err)
+		validateClientCert(t, ca.Bundle(), certPEM, keyPEM, "", nil, ttl)
+	})
+
+	t.Run("server certs", func(t *testing.T) {
+		dnsNames := []string{"example.com", "pinniped.dev"}
+		ips := []net.IP{net.ParseIP("127.0.0.1"), net.ParseIP("1.2.3.4")}
+
+		serverCert, err := ca.IssueServerCert(dnsNames, ips, ttl)
+		require.NoError(t, err)
+		certPEM, keyPEM, err := ToPEM(serverCert)
+		require.NoError(t, err)
+		validateServerCert(t, ca.Bundle(), certPEM, keyPEM, dnsNames, ips, ttl)
+
+		certPEM, keyPEM, err = ca.IssueServerCertPEM(dnsNames, ips, ttl)
+		require.NoError(t, err)
+		validateServerCert(t, ca.Bundle(), certPEM, keyPEM, dnsNames, ips, ttl)
+
+		certPEM, keyPEM, err = ca.IssueServerCertPEM(nil, ips, ttl)
+		require.NoError(t, err)
+		validateServerCert(t, ca.Bundle(), certPEM, keyPEM, nil, ips, ttl)
+
+		certPEM, keyPEM, err = ca.IssueServerCertPEM(dnsNames, nil, ttl)
+		require.NoError(t, err)
+		validateServerCert(t, ca.Bundle(), certPEM, keyPEM, dnsNames, nil, ttl)
+
+		certPEM, keyPEM, err = ca.IssueServerCertPEM([]string{}, ips, ttl)
+		require.NoError(t, err)
+		validateServerCert(t, ca.Bundle(), certPEM, keyPEM, nil, ips, ttl)
+
+		certPEM, keyPEM, err = ca.IssueServerCertPEM(dnsNames, []net.IP{}, ttl)
+		require.NoError(t, err)
+		validateServerCert(t, ca.Bundle(), certPEM, keyPEM, dnsNames, nil, ttl)
+	})
+}
+
+func validateClientCert(t *testing.T, caBundle []byte, certPEM []byte, keyPEM []byte, expectedUser string, expectedGroups []string, expectedTTL time.Duration) {
+	const fudgeFactor = 10 * time.Second
+	v := testutil.ValidateClientCertificate(t, string(caBundle), string(certPEM))
+	v.RequireLifetime(time.Now(), time.Now().Add(expectedTTL), certBackdate+fudgeFactor)
+	v.RequireMatchesPrivateKey(string(keyPEM))
+	v.RequireCommonName(expectedUser)
+	v.RequireOrganizations(expectedGroups)
+	v.RequireEmptyDNSNames()
+	v.RequireEmptyIPs()
+}
+
+func validateServerCert(t *testing.T, caBundle []byte, certPEM []byte, keyPEM []byte, expectedDNSNames []string, expectedIPs []net.IP, expectedTTL time.Duration) {
+	const fudgeFactor = 10 * time.Second
+	v := testutil.ValidateServerCertificate(t, string(caBundle), string(certPEM))
+	v.RequireLifetime(time.Now(), time.Now().Add(expectedTTL), certBackdate+fudgeFactor)
+	v.RequireMatchesPrivateKey(string(keyPEM))
+	v.RequireCommonName("")
+	v.RequireDNSNames(expectedDNSNames)
+	v.RequireIPs(expectedIPs)
+}

internal/certauthority/dynamiccertauthority/dynamiccertauthority.go

@@ -6,7 +6,6 @@
 package dynamiccertauthority
 
 import (
-	"crypto/x509/pkix"
 	"time"
 
 	"k8s.io/apiserver/pkg/server/dynamiccertificates"
@@ -27,14 +26,14 @@ func New(provider dynamiccertificates.CertKeyContentProvider) *CA {
 	}
 }
 
-// IssuePEM issues a new server certificate for the given identity and duration, returning it as a
+// IssueClientCertPEM issues a new client 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) {
+func (c *CA) IssueClientCertPEM(username string, groups []string, ttl time.Duration) ([]byte, []byte, error) {
 	caCrtPEM, caKeyPEM := c.provider.CurrentCertKeyContent()
 	ca, err := certauthority.Load(string(caCrtPEM), string(caKeyPEM))
 	if err != nil {
 		return nil, nil, err
 	}
 
-	return ca.IssuePEM(subject, dnsNames, nil, ttl)
+	return ca.IssueClientCertPEM(username, groups, ttl)
 }

internal/certauthority/dynamiccertauthority/dynamiccertauthority_test.go

@@ -1,10 +1,9 @@
-// Copyright 2020 the Pinniped contributors. All Rights Reserved.
+// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
 package dynamiccertauthority
 
 import (
-	"crypto/x509/pkix"
 	"testing"
 	"time"
 
@@ -106,10 +105,9 @@ func TestCAIssuePEM(t *testing.T) {
 				require.NotEmpty(t, keyPEM)
 
 				caCrtPEM, _ := provider.CurrentCertKeyContent()
-				crtAssertions := testutil.ValidateCertificate(t, string(caCrtPEM), string(crtPEM))
+				crtAssertions := testutil.ValidateClientCertificate(t, string(caCrtPEM), string(crtPEM))
-				crtAssertions.RequireCommonName("some-common-name")
+				crtAssertions.RequireCommonName("some-username")
-				crtAssertions.RequireDNSName("some-dns-name")
+				crtAssertions.RequireOrganizations([]string{"some-group1", "some-group2"})
-				crtAssertions.RequireDNSName("some-other-dns-name")
 				crtAssertions.RequireLifetime(time.Now(), time.Now().Add(time.Hour*24), time.Minute*10)
 				crtAssertions.RequireMatchesPrivateKey(string(keyPEM))
 			}
@@ -126,11 +124,5 @@ func issuePEM(provider dynamiccert.Provider, ca *CA, caCrt, caKey []byte) ([]byt
 	}
 
 	// otherwise check to see if their is an issuing error
-	return ca.IssuePEM(
+	return ca.IssueClientCertPEM("some-username", []string{"some-group1", "some-group2"}, time.Hour*24)
-		pkix.Name{
-			CommonName: "some-common-name",
-		},
-		[]string{"some-dns-name", "some-other-dns-name"},
-		time.Hour*24,
-	)
 }

internal/concierge/apiserver/apiserver.go

@@ -28,7 +28,7 @@ type Config struct {
 
 type ExtraConfig struct {
 	Authenticator                 credentialrequest.TokenCredentialRequestAuthenticator
-	Issuer                        issuer.CertIssuer
+	Issuer                        issuer.ClientCertIssuer
 	StartControllersPostStartHook func(ctx context.Context)
 	Scheme                        *runtime.Scheme
 	NegotiatedSerializer          runtime.NegotiatedSerializer

internal/concierge/impersonator/impersonator_test.go

@@ -5,7 +5,6 @@ package impersonator
 
 import (
 	"context"
-	"crypto/x509/pkix"
 	"net"
 	"net/http"
 	"net/http/httptest"
@@ -36,7 +35,7 @@ import (
 func TestImpersonator(t *testing.T) {
 	const port = 9444
 
-	ca, err := certauthority.New(pkix.Name{CommonName: "ca"}, time.Hour)
+	ca, err := certauthority.New("ca", time.Hour)
 	require.NoError(t, err)
 	caKey, err := ca.PrivateKeyToPEM()
 	require.NoError(t, err)
@@ -44,13 +43,13 @@ func TestImpersonator(t *testing.T) {
 	err = caContent.SetCertKeyContent(ca.Bundle(), caKey)
 	require.NoError(t, err)
 
-	cert, key, err := ca.IssuePEM(pkix.Name{}, nil, []net.IP{net.ParseIP("127.0.0.1")}, time.Hour)
+	cert, key, err := ca.IssueServerCertPEM(nil, []net.IP{net.ParseIP("127.0.0.1")}, time.Hour)
 	require.NoError(t, err)
 	certKeyContent := dynamiccert.New("cert-key")
 	err = certKeyContent.SetCertKeyContent(cert, key)
 	require.NoError(t, err)
 
-	unrelatedCA, err := certauthority.New(pkix.Name{CommonName: "ca"}, time.Hour)
+	unrelatedCA, err := certauthority.New("ca", time.Hour)
 	require.NoError(t, err)
 
 	// Punch out just enough stuff to make New actually run without error.
@@ -486,9 +485,7 @@ type clientCert struct {
 }
 
 func newClientCert(t *testing.T, ca *certauthority.CA, username string, groups []string) *clientCert {
-	certPEM, keyPEM, err := ca.IssuePEM(
+	certPEM, keyPEM, err := ca.IssueClientCertPEM(username, groups, time.Hour)
-		pkix.Name{CommonName: username, Organization: groups}, nil, nil, time.Hour,
-	)
 	require.NoError(t, err)
 	return &clientCert{
 		certPEM: certPEM,

internal/concierge/server/server.go

@@ -150,7 +150,7 @@ func (a *App) runServer(ctx context.Context) error {
 		return fmt.Errorf("could not prepare controllers: %w", err)
 	}
 
-	certIssuer := issuer.CertIssuers{
+	certIssuer := issuer.ClientCertIssuers{
 		dynamiccertauthority.New(dynamicSigningCertProvider),            // attempt to use the real Kube CA if possible
 		dynamiccertauthority.New(impersonationProxySigningCertProvider), // fallback to our internal CA if we need to
 	}
@@ -184,7 +184,7 @@ func (a *App) runServer(ctx context.Context) error {
 func getAggregatedAPIServerConfig(
 	dynamicCertProvider dynamiccert.Provider,
 	authenticator credentialrequest.TokenCredentialRequestAuthenticator,
-	issuer issuer.CertIssuer,
+	issuer issuer.ClientCertIssuer,
 	startControllersPostStartHook func(context.Context),
 	apiGroupSuffix string,
 	scheme *runtime.Scheme,

internal/controller/apicerts/certs_manager.go

@@ -4,7 +4,6 @@
 package apicerts
 
 import (
-	"crypto/x509/pkix"
 	"fmt"
 	"time"
 
@@ -94,7 +93,7 @@ func (c *certsManagerController) Sync(ctx controllerlib.Context) error {
 	}
 
 	// Create a CA.
-	ca, err := certauthority.New(pkix.Name{CommonName: c.generatedCACommonName}, c.certDuration)
+	ca, err := certauthority.New(c.generatedCACommonName, c.certDuration)
 	if err != nil {
 		return fmt.Errorf("could not initialize CA: %w", err)
 	}
@@ -120,12 +119,7 @@ func (c *certsManagerController) Sync(ctx controllerlib.Context) error {
 	// Using the CA from above, create a TLS server cert if we have service name.
 	if len(c.serviceNameForGeneratedCertCommonName) != 0 {
 		serviceEndpoint := c.serviceNameForGeneratedCertCommonName + "." + c.namespace + ".svc"
-		tlsCert, err := ca.Issue(
+		tlsCert, err := ca.IssueServerCert([]string{serviceEndpoint}, nil, c.certDuration)
-			pkix.Name{CommonName: serviceEndpoint},
-			[]string{serviceEndpoint},
-			nil,
-			c.certDuration,
-		)
 		if err != nil {
 			return fmt.Errorf("could not issue serving certificate: %w", err)
 		}

internal/controller/apicerts/certs_manager_test.go

@@ -218,12 +218,12 @@ func TestManagerControllerSync(t *testing.T) {
 				r.NotEmpty(actualCertChain)
 				r.Len(actualSecret.StringData, 4)
 
-				validCACert := testutil.ValidateCertificate(t, actualCACert, actualCACert)
+				validCACert := testutil.ValidateServerCertificate(t, actualCACert, actualCACert)
 				validCACert.RequireMatchesPrivateKey(actualCAPrivateKey)
 				validCACert.RequireLifetime(time.Now(), time.Now().Add(certDuration), 6*time.Minute)
 
 				// Validate the created cert using the CA, and also validate the cert's hostname
-				validCert := testutil.ValidateCertificate(t, actualCACert, actualCertChain)
+				validCert := testutil.ValidateServerCertificate(t, actualCACert, actualCertChain)
 				validCert.RequireDNSName("pinniped-api." + installedInNamespace + ".svc")
 				validCert.RequireLifetime(time.Now(), time.Now().Add(certDuration), 6*time.Minute)
 				validCert.RequireMatchesPrivateKey(actualPrivateKey)
@@ -252,7 +252,7 @@ func TestManagerControllerSync(t *testing.T) {
 				r.NotEmpty(actualCAPrivateKey)
 				r.Len(actualSecret.StringData, 2)
 
-				validCACert := testutil.ValidateCertificate(t, actualCACert, actualCACert)
+				validCACert := testutil.ValidateServerCertificate(t, actualCACert, actualCACert)
 				validCACert.RequireMatchesPrivateKey(actualCAPrivateKey)
 				validCACert.RequireLifetime(time.Now(), time.Now().Add(certDuration), 6*time.Minute)
 			})

internal/controller/impersonatorconfig/impersonator_config.go

@@ -7,7 +7,6 @@ import (
 	"context"
 	"crypto/tls"
 	"crypto/x509"
-	"crypto/x509/pkix"
 	"encoding/base64"
 	"encoding/pem"
 	"fmt"
@@ -40,13 +39,13 @@ import (
 )
 
 const (
-	impersonationProxyPort = 8444
+	impersonationProxyPort       = 8444
-	defaultHTTPSPort       = 443
+	defaultHTTPSPort             = 443
-	oneHundredYears        = 100 * 365 * 24 * time.Hour
+	approximatelyOneHundredYears = 100 * 365 * 24 * time.Hour
-	caCommonName           = "Pinniped Impersonation Proxy CA"
+	caCommonName                 = "Pinniped Impersonation Proxy CA"
-	caCrtKey               = "ca.crt"
+	caCrtKey                     = "ca.crt"
-	caKeyKey               = "ca.key"
+	caKeyKey                     = "ca.key"
-	appLabelKey            = "app"
+	appLabelKey                  = "app"
 )
 
 type impersonatorConfigController struct {
@@ -622,7 +621,7 @@ func (c *impersonatorConfigController) ensureCASecretIsCreated(ctx context.Conte
 }
 
 func (c *impersonatorConfigController) createCASecret(ctx context.Context) (*certauthority.CA, error) {
-	impersonationCA, err := certauthority.New(pkix.Name{CommonName: caCommonName}, oneHundredYears)
+	impersonationCA, err := certauthority.New(caCommonName, approximatelyOneHundredYears)
 	if err != nil {
 		return nil, fmt.Errorf("could not create impersonation CA: %w", err)
 	}
@@ -716,7 +715,7 @@ func (c *impersonatorConfigController) createNewTLSSecret(ctx context.Context, c
 		ips = []net.IP{ip}
 	}
 
-	impersonationCert, err := ca.Issue(pkix.Name{}, hostnames, ips, oneHundredYears)
+	impersonationCert, err := ca.IssueServerCert(hostnames, ips, approximatelyOneHundredYears)
 	if err != nil {
 		return nil, fmt.Errorf("could not create impersonation cert: %w", err)
 	}

internal/controller/impersonatorconfig/impersonator_config_test.go

@@ -7,7 +7,6 @@ import (
 	"context"
 	"crypto/tls"
 	"crypto/x509"
-	"crypto/x509/pkix"
 	"encoding/base64"
 	"encoding/pem"
 	"errors"
@@ -352,7 +351,10 @@ func TestImpersonatorConfigControllerSync(t *testing.T) {
 					require.NoError(t, err)
 					roots := x509.NewCertPool()
 					require.True(t, roots.AppendCertsFromPEM(currentClientCertCA))
-					opts := x509.VerifyOptions{Roots: roots}
+					opts := x509.VerifyOptions{
+						Roots:     roots,
+						KeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth},
+					}
 					_, err = parsed.Verify(opts)
 					require.NoError(t, err)
 					return nil
@@ -594,7 +596,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) {
 		}
 
 		var newCA = func() *certauthority.CA {
-			ca, err := certauthority.New(pkix.Name{CommonName: "test CA"}, 24*time.Hour)
+			ca, err := certauthority.New("test CA", 24*time.Hour)
 			r.NoError(err)
 			return ca
 		}
@@ -609,7 +611,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) {
 		}
 
 		var newTLSCertSecretData = func(ca *certauthority.CA, dnsNames []string, ip string) map[string][]byte {
-			impersonationCert, err := ca.Issue(pkix.Name{}, dnsNames, []net.IP{net.ParseIP(ip)}, 24*time.Hour)
+			impersonationCert, err := ca.IssueServerCert(dnsNames, []net.IP{net.ParseIP(ip)}, 24*time.Hour)
 			r.NoError(err)
 			certPEM, keyPEM, err := certauthority.ToPEM(impersonationCert)
 			r.NoError(err)
@@ -939,7 +941,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) {
 			createdKeyPEM := createdSecret.Data[corev1.TLSPrivateKeyKey]
 			r.NotNil(createdKeyPEM)
 			r.NotNil(createdCertPEM)
-			validCert := testutil.ValidateCertificate(t, string(caCert), string(createdCertPEM))
+			validCert := testutil.ValidateServerCertificate(t, string(caCert), string(createdCertPEM))
 			validCert.RequireMatchesPrivateKey(string(createdKeyPEM))
 			validCert.RequireLifetime(time.Now().Add(-10*time.Second), time.Now().Add(100*time.Hour*24*365), 10*time.Second)
 		}
@@ -980,7 +982,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) {
 			signingCAKeyPEM, err = ca.PrivateKeyToPEM()
 			r.NoError(err)
 			signingCASecret = newSigningKeySecret(caSignerName, signingCACertPEM, signingCAKeyPEM)
-			validClientCert, err = ca.Issue(pkix.Name{}, nil, nil, time.Hour)
+			validClientCert, err = ca.IssueClientCert("username", nil, time.Hour)
 			r.NoError(err)
 		})
 

internal/issuer/issuer.go

@@ -4,7 +4,6 @@
 package issuer
 
 import (
-	"crypto/x509/pkix"
 	"time"
 
 	"k8s.io/apimachinery/pkg/util/errors"
@@ -14,19 +13,19 @@ import (
 
 const defaultCertIssuerErr = constable.Error("failed to issue cert")
 
-type CertIssuer interface {
+type ClientCertIssuer interface {
-	IssuePEM(subject pkix.Name, dnsNames []string, ttl time.Duration) (certPEM, keyPEM []byte, err error)
+	IssueClientCertPEM(username string, groups []string, ttl time.Duration) (certPEM, keyPEM []byte, err error)
 }
 
-var _ CertIssuer = CertIssuers{}
+var _ ClientCertIssuer = ClientCertIssuers{}
 
-type CertIssuers []CertIssuer
+type ClientCertIssuers []ClientCertIssuer
 
-func (c CertIssuers) IssuePEM(subject pkix.Name, dnsNames []string, ttl time.Duration) ([]byte, []byte, error) {
+func (c ClientCertIssuers) IssueClientCertPEM(username string, groups []string, ttl time.Duration) ([]byte, []byte, error) {
 	var errs []error
 
 	for _, issuer := range c {
-		certPEM, keyPEM, err := issuer.IssuePEM(subject, dnsNames, ttl)
+		certPEM, keyPEM, err := issuer.IssueClientCertPEM(username, groups, ttl)
 		if err != nil {
 			errs = append(errs, err)
 			continue

internal/mocks/credentialrequestmocks/credentialrequestmocks.go

@@ -3,61 +3,20 @@
 //
 
 // Code generated by MockGen. DO NOT EDIT.
-// Source: go.pinniped.dev/internal/registry/credentialrequest (interfaces: CertIssuer,TokenCredentialRequestAuthenticator)
+// Source: go.pinniped.dev/internal/registry/credentialrequest (interfaces: TokenCredentialRequestAuthenticator)
 
 // Package credentialrequestmocks is a generated GoMock package.
 package credentialrequestmocks
 
 import (
 	context "context"
-	pkix "crypto/x509/pkix"
 	reflect "reflect"
-	time "time"
 
 	gomock "github.com/golang/mock/gomock"
 	login "go.pinniped.dev/generated/latest/apis/concierge/login"
 	user "k8s.io/apiserver/pkg/authentication/user"
 )
 
-// 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)
-}
-
 // MockTokenCredentialRequestAuthenticator is a mock of TokenCredentialRequestAuthenticator interface.
 type MockTokenCredentialRequestAuthenticator struct {
 	ctrl     *gomock.Controller

internal/mocks/credentialrequestmocks/generate.go

@@ -1,6 +1,6 @@
-// Copyright 2020 the Pinniped contributors. All Rights Reserved.
+// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
 package credentialrequestmocks
 
-//go:generate go run -v github.com/golang/mock/mockgen  -destination=credentialrequestmocks.go -package=credentialrequestmocks -copyright_file=../../../hack/header.txt go.pinniped.dev/internal/registry/credentialrequest CertIssuer,TokenCredentialRequestAuthenticator
+//go:generate go run -v github.com/golang/mock/mockgen  -destination=credentialrequestmocks.go -package=credentialrequestmocks -copyright_file=../../../hack/header.txt go.pinniped.dev/internal/registry/credentialrequest TokenCredentialRequestAuthenticator

internal/mocks/issuermocks/generate.go

@@ -0,0 +1,6 @@
+// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package issuermocks
+
+//go:generate go run -v github.com/golang/mock/mockgen  -destination=issuermocks.go -package=issuermocks -copyright_file=../../../hack/header.txt go.pinniped.dev/internal/issuer ClientCertIssuer

internal/mocks/issuermocks/issuermocks.go

@@ -0,0 +1,55 @@
+// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+//
+
+// Code generated by MockGen. DO NOT EDIT.
+// Source: go.pinniped.dev/internal/issuer (interfaces: ClientCertIssuer)
+
+// Package issuermocks is a generated GoMock package.
+package issuermocks
+
+import (
+	reflect "reflect"
+	time "time"
+
+	gomock "github.com/golang/mock/gomock"
+)
+
+// MockClientCertIssuer is a mock of ClientCertIssuer interface.
+type MockClientCertIssuer struct {
+	ctrl     *gomock.Controller
+	recorder *MockClientCertIssuerMockRecorder
+}
+
+// MockClientCertIssuerMockRecorder is the mock recorder for MockClientCertIssuer.
+type MockClientCertIssuerMockRecorder struct {
+	mock *MockClientCertIssuer
+}
+
+// NewMockClientCertIssuer creates a new mock instance.
+func NewMockClientCertIssuer(ctrl *gomock.Controller) *MockClientCertIssuer {
+	mock := &MockClientCertIssuer{ctrl: ctrl}
+	mock.recorder = &MockClientCertIssuerMockRecorder{mock}
+	return mock
+}
+
+// EXPECT returns an object that allows the caller to indicate expected use.
+func (m *MockClientCertIssuer) EXPECT() *MockClientCertIssuerMockRecorder {
+	return m.recorder
+}
+
+// IssueClientCertPEM mocks base method.
+func (m *MockClientCertIssuer) IssueClientCertPEM(arg0 string, arg1 []string, arg2 time.Duration) ([]byte, []byte, error) {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "IssueClientCertPEM", arg0, arg1, arg2)
+	ret0, _ := ret[0].([]byte)
+	ret1, _ := ret[1].([]byte)
+	ret2, _ := ret[2].(error)
+	return ret0, ret1, ret2
+}
+
+// IssueClientCertPEM indicates an expected call of IssueClientCertPEM.
+func (mr *MockClientCertIssuerMockRecorder) IssueClientCertPEM(arg0, arg1, arg2 interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IssueClientCertPEM", reflect.TypeOf((*MockClientCertIssuer)(nil).IssueClientCertPEM), arg0, arg1, arg2)
+}

internal/registry/credentialrequest/rest.go

@@ -6,7 +6,6 @@ package credentialrequest
 
 import (
 	"context"
-	"crypto/x509/pkix"
 	"fmt"
 	"time"
 
@@ -32,7 +31,7 @@ type TokenCredentialRequestAuthenticator interface {
 	AuthenticateTokenCredentialRequest(ctx context.Context, req *loginapi.TokenCredentialRequest) (user.Info, error)
 }
 
-func NewREST(authenticator TokenCredentialRequestAuthenticator, issuer issuer.CertIssuer, resource schema.GroupResource) *REST {
+func NewREST(authenticator TokenCredentialRequestAuthenticator, issuer issuer.ClientCertIssuer, resource schema.GroupResource) *REST {
 	return &REST{
 		authenticator:  authenticator,
 		issuer:         issuer,
@@ -42,7 +41,7 @@ func NewREST(authenticator TokenCredentialRequestAuthenticator, issuer issuer.Ce
 
 type REST struct {
 	authenticator  TokenCredentialRequestAuthenticator
-	issuer         issuer.CertIssuer
+	issuer         issuer.ClientCertIssuer
 	tableConvertor rest.TableConvertor
 }
 
@@ -97,30 +96,23 @@ func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation
 		return nil, err
 	}
 
-	user, err := r.authenticator.AuthenticateTokenCredentialRequest(ctx, credentialRequest)
+	userInfo, err := r.authenticator.AuthenticateTokenCredentialRequest(ctx, credentialRequest)
 	if err != nil {
 		traceFailureWithError(t, "token authentication", err)
 		return failureResponse(), nil
 	}
-	if user == nil || user.GetName() == "" {
+	if userInfo == nil || userInfo.GetName() == "" {
-		traceSuccess(t, user, false)
+		traceSuccess(t, userInfo, false)
 		return failureResponse(), nil
 	}
 
-	certPEM, keyPEM, err := r.issuer.IssuePEM(
+	certPEM, keyPEM, err := r.issuer.IssueClientCertPEM(userInfo.GetName(), userInfo.GetGroups(), clientCertificateTTL)
-		pkix.Name{
-			CommonName:   user.GetName(),
-			Organization: user.GetGroups(),
-		},
-		[]string{},
-		clientCertificateTTL,
-	)
 	if err != nil {
 		traceFailureWithError(t, "cert issuer", err)
 		return failureResponse(), nil
 	}
 
-	traceSuccess(t, user, true)
+	traceSuccess(t, userInfo, true)
 
 	return &loginapi.TokenCredentialRequest{
 		Status: loginapi.TokenCredentialRequestStatus{

internal/registry/credentialrequest/rest_test.go

@@ -5,7 +5,6 @@ package credentialrequest
 
 import (
 	"context"
-	"crypto/x509/pkix"
 	"errors"
 	"fmt"
 	"testing"
@@ -26,6 +25,7 @@ import (
 	loginapi "go.pinniped.dev/generated/latest/apis/concierge/login"
 	"go.pinniped.dev/internal/issuer"
 	"go.pinniped.dev/internal/mocks/credentialrequestmocks"
+	"go.pinniped.dev/internal/mocks/issuermocks"
 	"go.pinniped.dev/internal/testutil"
 )
 
@@ -89,16 +89,14 @@ func TestCreate(t *testing.T) {
 					Groups: []string{"test-group-1", "test-group-2"},
 				}, nil)
 
-			issuer := credentialrequestmocks.NewMockCertIssuer(ctrl)
+			clientCertIssuer := issuermocks.NewMockClientCertIssuer(ctrl)
-			issuer.EXPECT().IssuePEM(
+			clientCertIssuer.EXPECT().IssueClientCertPEM(
-				pkix.Name{
+				"test-user",
-					CommonName:   "test-user",
+				[]string{"test-group-1", "test-group-2"},
-					Organization: []string{"test-group-1", "test-group-2"}},
-				[]string{},
 				5*time.Minute,
 			).Return([]byte("test-cert"), []byte("test-key"), nil)
 
-			storage := NewREST(requestAuthenticator, issuer, schema.GroupResource{})
+			storage := NewREST(requestAuthenticator, clientCertIssuer, schema.GroupResource{})
 
 			response, err := callCreate(context.Background(), storage, req)
 
@@ -132,12 +130,12 @@ func TestCreate(t *testing.T) {
 					Groups: []string{"test-group-1", "test-group-2"},
 				}, nil)
 
-			issuer := credentialrequestmocks.NewMockCertIssuer(ctrl)
+			clientCertIssuer := issuermocks.NewMockClientCertIssuer(ctrl)
-			issuer.EXPECT().
+			clientCertIssuer.EXPECT().
-				IssuePEM(gomock.Any(), gomock.Any(), gomock.Any()).
+				IssueClientCertPEM(gomock.Any(), gomock.Any(), gomock.Any()).
 				Return(nil, nil, fmt.Errorf("some certificate authority error"))
 
-			storage := NewREST(requestAuthenticator, issuer, schema.GroupResource{})
+			storage := NewREST(requestAuthenticator, clientCertIssuer, schema.GroupResource{})
 
 			response, err := callCreate(context.Background(), storage, req)
 			requireSuccessfulResponseWithAuthenticationFailureMessage(t, err, response)
@@ -354,12 +352,12 @@ func requireSuccessfulResponseWithAuthenticationFailureMessage(t *testing.T, err
 	})
 }
 
-func successfulIssuer(ctrl *gomock.Controller) issuer.CertIssuer {
+func successfulIssuer(ctrl *gomock.Controller) issuer.ClientCertIssuer {
-	certIssuer := credentialrequestmocks.NewMockCertIssuer(ctrl)
+	clientCertIssuer := issuermocks.NewMockClientCertIssuer(ctrl)
-	certIssuer.EXPECT().
+	clientCertIssuer.EXPECT().
-		IssuePEM(gomock.Any(), gomock.Any(), gomock.Any()).
+		IssueClientCertPEM(gomock.Any(), gomock.Any(), gomock.Any()).
 		Return([]byte("test-cert"), []byte("test-key"), nil)
-	return certIssuer
+	return clientCertIssuer
 }
 
 func stringPtr(s string) *string {

internal/testutil/certs.go

@@ -1,4 +1,4 @@
-// Copyright 2020 the Pinniped contributors. All Rights Reserved.
+// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
 package testutil
@@ -12,6 +12,7 @@ import (
 	"crypto/x509/pkix"
 	"encoding/pem"
 	"math/big"
+	"net"
 	"testing"
 	"time"
 
@@ -25,10 +26,18 @@ type ValidCert struct {
 	parsed  *x509.Certificate
 }
 
-// ValidateCertificate validates a certificate and provides an object for asserting properties of the certificate.
+// ValidateServerCertificate validates a certificate and provides an object for asserting properties of the certificate.
-func ValidateCertificate(t *testing.T, caPEM string, certPEM string) *ValidCert {
+func ValidateServerCertificate(t *testing.T, caPEM string, certPEM string) *ValidCert {
 	t.Helper()
+	return validateCertificate(t, x509.ExtKeyUsageServerAuth, caPEM, certPEM)
+}
 
+func ValidateClientCertificate(t *testing.T, caPEM string, certPEM string) *ValidCert {
+	t.Helper()
+	return validateCertificate(t, x509.ExtKeyUsageClientAuth, caPEM, certPEM)
+}
+
+func validateCertificate(t *testing.T, extKeyUsage x509.ExtKeyUsage, caPEM string, certPEM string) *ValidCert {
 	block, _ := pem.Decode([]byte(certPEM))
 	require.NotNil(t, block)
 	parsed, err := x509.ParseCertificate(block.Bytes)
@@ -37,7 +46,10 @@ func ValidateCertificate(t *testing.T, caPEM string, certPEM string) *ValidCert
 	// Validate the created cert using the CA.
 	roots := x509.NewCertPool()
 	require.True(t, roots.AppendCertsFromPEM([]byte(caPEM)))
-	opts := x509.VerifyOptions{Roots: roots}
+	opts := x509.VerifyOptions{
+		Roots:     roots,
+		KeyUsages: []x509.ExtKeyUsage{extKeyUsage},
+	}
 	_, err = parsed.Verify(opts)
 	require.NoError(t, err)
 
@@ -61,6 +73,35 @@ func (v *ValidCert) RequireDNSName(expectDNSName string) {
 	require.Contains(v.t, v.parsed.DNSNames, expectDNSName, "expected an explicit DNS SAN, not just Common Name")
 }
 
+func (v *ValidCert) RequireDNSNames(names []string) {
+	v.t.Helper()
+	require.Equal(v.t, names, v.parsed.DNSNames)
+}
+
+func (v *ValidCert) RequireEmptyDNSNames() {
+	v.t.Helper()
+	require.Empty(v.t, v.parsed.DNSNames)
+}
+
+func (v *ValidCert) RequireIPs(ips []net.IP) {
+	v.t.Helper()
+	actualIPs := v.parsed.IPAddresses
+	actualIPsStrings := make([]string, len(actualIPs))
+	for i := range actualIPs {
+		actualIPsStrings[i] = actualIPs[i].String()
+	}
+	expectedIPsStrings := make([]string, len(ips))
+	for i := range ips {
+		expectedIPsStrings[i] = ips[i].String()
+	}
+	require.Equal(v.t, expectedIPsStrings, actualIPsStrings)
+}
+
+func (v *ValidCert) RequireEmptyIPs() {
+	v.t.Helper()
+	require.Empty(v.t, v.parsed.IPAddresses)
+}
+
 // RequireLifetime asserts that the lifetime of the certificate matches the expected timestamps.
 func (v *ValidCert) RequireLifetime(expectNotBefore time.Time, expectNotAfter time.Time, delta time.Duration) {
 	v.t.Helper()
@@ -81,6 +122,11 @@ func (v *ValidCert) RequireCommonName(commonName string) {
 	require.Equal(v.t, commonName, v.parsed.Subject.CommonName)
 }
 
+func (v *ValidCert) RequireOrganizations(orgs []string) {
+	v.t.Helper()
+	require.Equal(v.t, orgs, v.parsed.Subject.Organization)
+}
+
 // CreateCertificate creates a certificate with the provided time bounds, and returns the PEM
 // representation of the certificate and its private key. The returned certificate is capable of
 // signing child certificates.

pkg/conciergeclient/conciergeclient_test.go

@@ -5,7 +5,6 @@ package conciergeclient
 
 import (
 	"context"
-	"crypto/x509/pkix"
 	"encoding/base64"
 	"encoding/json"
 	"fmt"
@@ -26,7 +25,7 @@ import (
 
 func TestNew(t *testing.T) {
 	t.Parallel()
-	testCA, err := certauthority.New(pkix.Name{}, 1*time.Hour)
+	testCA, err := certauthority.New("Test CA", 1*time.Hour)
 	require.NoError(t, err)
 
 	tests := []struct {

test/integration/e2e_test.go

@@ -6,7 +6,6 @@ import (
 	"bufio"
 	"bytes"
 	"context"
-	"crypto/x509/pkix"
 	"encoding/base64"
 	"errors"
 	"fmt"
@@ -69,7 +68,7 @@ func TestE2EFullIntegration(t *testing.T) {
 
 	// Generate a CA bundle with which to serve this provider.
 	t.Logf("generating test CA")
-	ca, err := certauthority.New(pkix.Name{CommonName: "Downstream Test CA"}, 1*time.Hour)
+	ca, err := certauthority.New("Downstream Test CA", 1*time.Hour)
 	require.NoError(t, err)
 
 	// Save that bundle plus the one that signs the upstream issuer, for test purposes.
@@ -80,12 +79,7 @@ func TestE2EFullIntegration(t *testing.T) {
 
 	// Use the CA to issue a TLS server cert.
 	t.Logf("issuing test certificate")
-	tlsCert, err := ca.Issue(
+	tlsCert, err := ca.IssueServerCert([]string{issuerURL.Hostname()}, nil, 1*time.Hour)
-		pkix.Name{CommonName: issuerURL.Hostname()},
-		[]string{issuerURL.Hostname()},
-		nil,
-		1*time.Hour,
-	)
 	require.NoError(t, err)
 	certPEM, keyPEM, err := certauthority.ToPEM(tlsCert)
 	require.NoError(t, err)

test/integration/supervisor_discovery_test.go

@@ -7,7 +7,6 @@ import (
 	"context"
 	"crypto/tls"
 	"crypto/x509"
-	"crypto/x509/pkix"
 	"encoding/json"
 	"fmt"
 	"io/ioutil"
@@ -288,11 +287,11 @@ func defaultTLSCertSecretName(env *library.TestEnv) string {
 
 func createTLSCertificateSecret(ctx context.Context, t *testing.T, ns string, hostname string, ips []net.IP, secretName string, kubeClient kubernetes.Interface) *certauthority.CA {
 	// Create a CA.
-	ca, err := certauthority.New(pkix.Name{CommonName: "Acme Corp"}, 1000*time.Hour)
+	ca, err := certauthority.New("Acme Corp", 1000*time.Hour)
 	require.NoError(t, err)
 
 	// Using the CA, create a TLS server cert.
-	tlsCert, err := ca.Issue(pkix.Name{CommonName: hostname}, []string{hostname}, ips, 1000*time.Hour)
+	tlsCert, err := ca.IssueServerCert([]string{hostname}, ips, 1000*time.Hour)
 	require.NoError(t, err)
 
 	// Write the serving cert to the SNI secret.

test/integration/supervisor_login_test.go

@@ -6,7 +6,6 @@ package integration
 import (
 	"context"
 	"crypto/tls"
-	"crypto/x509/pkix"
 	"encoding/base64"
 	"encoding/json"
 	"fmt"
@@ -58,7 +57,7 @@ func TestSupervisorLogin(t *testing.T) {
 
 	// Generate a CA bundle with which to serve this provider.
 	t.Logf("generating test CA")
-	ca, err := certauthority.New(pkix.Name{CommonName: "Downstream Test CA"}, 1*time.Hour)
+	ca, err := certauthority.New("Downstream Test CA", 1*time.Hour)
 	require.NoError(t, err)
 
 	// Create an HTTP client that can reach the downstream discovery endpoint using the CA certs.
@@ -85,12 +84,7 @@ func TestSupervisorLogin(t *testing.T) {
 
 	// Use the CA to issue a TLS server cert.
 	t.Logf("issuing test certificate")
-	tlsCert, err := ca.Issue(
+	tlsCert, err := ca.IssueServerCert([]string{issuerURL.Hostname()}, nil, 1*time.Hour)
-		pkix.Name{CommonName: issuerURL.Hostname()},
-		[]string{issuerURL.Hostname()},
-		nil,
-		1*time.Hour,
-	)
 	require.NoError(t, err)
 	certPEM, keyPEM, err := certauthority.ToPEM(tlsCert)
 	require.NoError(t, err)