diff --git a/cmd/local-user-authenticator/main_test.go b/cmd/local-user-authenticator/main_test.go index 632fbde0..5a6fb0bd 100644 --- a/cmd/local-user-authenticator/main_test.go +++ b/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) diff --git a/cmd/pinniped/cmd/flag_types_test.go b/cmd/pinniped/cmd/flag_types_test.go index 6d967969..101191d5 100644 --- a/cmd/pinniped/cmd/flag_types_test.go +++ b/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") diff --git a/cmd/pinniped/cmd/kubeconfig_test.go b/cmd/pinniped/cmd/kubeconfig_test.go index f3b27d5d..879e2d71 100644 --- a/cmd/pinniped/cmd/kubeconfig_test.go +++ b/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)) diff --git a/cmd/pinniped/cmd/login_oidc_test.go b/cmd/pinniped/cmd/login_oidc_test.go index 976e3adf..2b1e8469 100644 --- a/cmd/pinniped/cmd/login_oidc_test.go +++ b/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") diff --git a/cmd/pinniped/cmd/login_static_test.go b/cmd/pinniped/cmd/login_static_test.go index 1d097123..caf41df5 100644 --- a/cmd/pinniped/cmd/login_static_test.go +++ b/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") diff --git a/internal/certauthority/certauthority.go b/internal/certauthority/certauthority.go index ad81e38a..b64bf0fa 100644 --- a/internal/certauthority/certauthority.go +++ b/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. -func New(subject pkix.Name, ttl time.Duration) (*CA, error) { - return newInternal(subject, ttl, secureEnv()) +// New generates a fresh certificate authority with the given Common Name and TTL. +func New(commonName string, ttl time.Duration) (*CA, error) { + 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. -func (c *CA) Issue(subject pkix.Name, dnsNames []string, ips []net.IP, ttl time.Duration) (*tls.Certificate, error) { +// IssueClientCert issues a new client certificate with username and groups included in the Kube-style +// 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, - Subject: subject, - NotBefore: notBefore, - NotAfter: notAfter, - KeyUsage: x509.KeyUsageDigitalSignature, - // TODO split this function into two funcs that handle client and serving certs differently - ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageClientAuth}, + SerialNumber: serialNumber, + Subject: subject, + NotBefore: notBefore, + NotAfter: notAfter, + ExtKeyUsage: []x509.ExtKeyUsage{extKeyUsage}, 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 } diff --git a/internal/certauthority/certauthority_test.go b/internal/certauthority/certauthority_test.go index 7f9e3571..80948a77 100644 --- a/internal/certauthority/certauthority_test.go +++ b/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)), - keygenRNG: strings.NewReader(strings.Repeat("x", 64)), + serialRNG: strings.NewReader(strings.Repeat("x", numRandBytes)), + 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)), - keygenRNG: strings.NewReader(strings.Repeat("x", 64)), + serialRNG: strings.NewReader(strings.Repeat("x", numRandBytes)), + 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)), - keygenRNG: strings.NewReader(strings.Repeat("x", 64)), + serialRNG: strings.NewReader(strings.Repeat("x", numRandBytes)), + 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)), - keygenRNG: strings.NewReader(strings.Repeat("x", 64)), + serialRNG: strings.NewReader(strings.Repeat("x", numRandBytes)), + 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) +} diff --git a/internal/certauthority/dynamiccertauthority/dynamiccertauthority.go b/internal/certauthority/dynamiccertauthority/dynamiccertauthority.go index 3c0c4b72..78eb97c6 100644 --- a/internal/certauthority/dynamiccertauthority/dynamiccertauthority.go +++ b/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) } diff --git a/internal/certauthority/dynamiccertauthority/dynamiccertauthority_test.go b/internal/certauthority/dynamiccertauthority/dynamiccertauthority_test.go index 41e0f291..47bc6539 100644 --- a/internal/certauthority/dynamiccertauthority/dynamiccertauthority_test.go +++ b/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.RequireCommonName("some-common-name") - crtAssertions.RequireDNSName("some-dns-name") - crtAssertions.RequireDNSName("some-other-dns-name") + crtAssertions := testutil.ValidateClientCertificate(t, string(caCrtPEM), string(crtPEM)) + crtAssertions.RequireCommonName("some-username") + crtAssertions.RequireOrganizations([]string{"some-group1", "some-group2"}) 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( - pkix.Name{ - CommonName: "some-common-name", - }, - []string{"some-dns-name", "some-other-dns-name"}, - time.Hour*24, - ) + return ca.IssueClientCertPEM("some-username", []string{"some-group1", "some-group2"}, time.Hour*24) } diff --git a/internal/concierge/apiserver/apiserver.go b/internal/concierge/apiserver/apiserver.go index f64e0104..f1bce95b 100644 --- a/internal/concierge/apiserver/apiserver.go +++ b/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 diff --git a/internal/concierge/impersonator/impersonator_test.go b/internal/concierge/impersonator/impersonator_test.go index 9d1e231c..66985781 100644 --- a/internal/concierge/impersonator/impersonator_test.go +++ b/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( - pkix.Name{CommonName: username, Organization: groups}, nil, nil, time.Hour, - ) + certPEM, keyPEM, err := ca.IssueClientCertPEM(username, groups, time.Hour) require.NoError(t, err) return &clientCert{ certPEM: certPEM, diff --git a/internal/concierge/server/server.go b/internal/concierge/server/server.go index 8602d07d..4a4415a5 100644 --- a/internal/concierge/server/server.go +++ b/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, diff --git a/internal/controller/apicerts/certs_manager.go b/internal/controller/apicerts/certs_manager.go index f0c11d0d..c3d8bf36 100644 --- a/internal/controller/apicerts/certs_manager.go +++ b/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( - pkix.Name{CommonName: serviceEndpoint}, - []string{serviceEndpoint}, - nil, - c.certDuration, - ) + tlsCert, err := ca.IssueServerCert([]string{serviceEndpoint}, nil, c.certDuration) if err != nil { return fmt.Errorf("could not issue serving certificate: %w", err) } diff --git a/internal/controller/apicerts/certs_manager_test.go b/internal/controller/apicerts/certs_manager_test.go index b32b9228..092f504b 100644 --- a/internal/controller/apicerts/certs_manager_test.go +++ b/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) }) diff --git a/internal/controller/impersonatorconfig/impersonator_config.go b/internal/controller/impersonatorconfig/impersonator_config.go index 6f81901d..1733d9da 100644 --- a/internal/controller/impersonatorconfig/impersonator_config.go +++ b/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 - defaultHTTPSPort = 443 - oneHundredYears = 100 * 365 * 24 * time.Hour - caCommonName = "Pinniped Impersonation Proxy CA" - caCrtKey = "ca.crt" - caKeyKey = "ca.key" - appLabelKey = "app" + impersonationProxyPort = 8444 + defaultHTTPSPort = 443 + approximatelyOneHundredYears = 100 * 365 * 24 * time.Hour + caCommonName = "Pinniped Impersonation Proxy CA" + caCrtKey = "ca.crt" + caKeyKey = "ca.key" + 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) } diff --git a/internal/controller/impersonatorconfig/impersonator_config_test.go b/internal/controller/impersonatorconfig/impersonator_config_test.go index f926208b..f4996184 100644 --- a/internal/controller/impersonatorconfig/impersonator_config_test.go +++ b/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) }) diff --git a/internal/issuer/issuer.go b/internal/issuer/issuer.go index 94e2f235..88ac1538 100644 --- a/internal/issuer/issuer.go +++ b/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 { - IssuePEM(subject pkix.Name, dnsNames []string, ttl time.Duration) (certPEM, keyPEM []byte, err error) +type ClientCertIssuer interface { + 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 diff --git a/internal/mocks/credentialrequestmocks/credentialrequestmocks.go b/internal/mocks/credentialrequestmocks/credentialrequestmocks.go index 58bd134d..68ff7f2e 100644 --- a/internal/mocks/credentialrequestmocks/credentialrequestmocks.go +++ b/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 diff --git a/internal/mocks/credentialrequestmocks/generate.go b/internal/mocks/credentialrequestmocks/generate.go index c22ec978..ce19cf3f 100644 --- a/internal/mocks/credentialrequestmocks/generate.go +++ b/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 diff --git a/internal/mocks/issuermocks/generate.go b/internal/mocks/issuermocks/generate.go new file mode 100644 index 00000000..7d0c9937 --- /dev/null +++ b/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 diff --git a/internal/mocks/issuermocks/issuermocks.go b/internal/mocks/issuermocks/issuermocks.go new file mode 100644 index 00000000..99dce2c5 --- /dev/null +++ b/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) +} diff --git a/internal/registry/credentialrequest/rest.go b/internal/registry/credentialrequest/rest.go index a1846a40..9c9165d5 100644 --- a/internal/registry/credentialrequest/rest.go +++ b/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() == "" { - traceSuccess(t, user, false) + if userInfo == nil || userInfo.GetName() == "" { + traceSuccess(t, userInfo, false) return failureResponse(), nil } - certPEM, keyPEM, err := r.issuer.IssuePEM( - pkix.Name{ - CommonName: user.GetName(), - Organization: user.GetGroups(), - }, - []string{}, - clientCertificateTTL, - ) + certPEM, keyPEM, err := r.issuer.IssueClientCertPEM(userInfo.GetName(), userInfo.GetGroups(), 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{ diff --git a/internal/registry/credentialrequest/rest_test.go b/internal/registry/credentialrequest/rest_test.go index 6b71ec20..9c05c2bb 100644 --- a/internal/registry/credentialrequest/rest_test.go +++ b/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) - issuer.EXPECT().IssuePEM( - pkix.Name{ - CommonName: "test-user", - Organization: []string{"test-group-1", "test-group-2"}}, - []string{}, + clientCertIssuer := issuermocks.NewMockClientCertIssuer(ctrl) + clientCertIssuer.EXPECT().IssueClientCertPEM( + "test-user", + []string{"test-group-1", "test-group-2"}, 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) - issuer.EXPECT(). - IssuePEM(gomock.Any(), gomock.Any(), gomock.Any()). + clientCertIssuer := issuermocks.NewMockClientCertIssuer(ctrl) + clientCertIssuer.EXPECT(). + 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 { - certIssuer := credentialrequestmocks.NewMockCertIssuer(ctrl) - certIssuer.EXPECT(). - IssuePEM(gomock.Any(), gomock.Any(), gomock.Any()). +func successfulIssuer(ctrl *gomock.Controller) issuer.ClientCertIssuer { + clientCertIssuer := issuermocks.NewMockClientCertIssuer(ctrl) + clientCertIssuer.EXPECT(). + IssueClientCertPEM(gomock.Any(), gomock.Any(), gomock.Any()). Return([]byte("test-cert"), []byte("test-key"), nil) - return certIssuer + return clientCertIssuer } func stringPtr(s string) *string { diff --git a/internal/testutil/certs.go b/internal/testutil/certs.go index 79792195..9d6b41d3 100644 --- a/internal/testutil/certs.go +++ b/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. -func ValidateCertificate(t *testing.T, caPEM string, certPEM string) *ValidCert { +// ValidateServerCertificate validates a certificate and provides an object for asserting properties of the certificate. +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. diff --git a/pkg/conciergeclient/conciergeclient_test.go b/pkg/conciergeclient/conciergeclient_test.go index 162ecfee..6787cc92 100644 --- a/pkg/conciergeclient/conciergeclient_test.go +++ b/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 { diff --git a/test/integration/e2e_test.go b/test/integration/e2e_test.go index dfa4e2a0..b382d85f 100644 --- a/test/integration/e2e_test.go +++ b/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( - pkix.Name{CommonName: issuerURL.Hostname()}, - []string{issuerURL.Hostname()}, - nil, - 1*time.Hour, - ) + tlsCert, err := ca.IssueServerCert([]string{issuerURL.Hostname()}, nil, 1*time.Hour) require.NoError(t, err) certPEM, keyPEM, err := certauthority.ToPEM(tlsCert) require.NoError(t, err) diff --git a/test/integration/supervisor_discovery_test.go b/test/integration/supervisor_discovery_test.go index 8712a316..d076f828 100644 --- a/test/integration/supervisor_discovery_test.go +++ b/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. diff --git a/test/integration/supervisor_login_test.go b/test/integration/supervisor_login_test.go index 9d6b6787..fe2060ea 100644 --- a/test/integration/supervisor_login_test.go +++ b/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( - pkix.Name{CommonName: issuerURL.Hostname()}, - []string{issuerURL.Hostname()}, - nil, - 1*time.Hour, - ) + tlsCert, err := ca.IssueServerCert([]string{issuerURL.Hostname()}, nil, 1*time.Hour) require.NoError(t, err) certPEM, keyPEM, err := certauthority.ToPEM(tlsCert) require.NoError(t, err)