diff --git a/internal/cert/pem.go b/internal/cert/pem.go new file mode 100644 index 000000000..1a6b4d458 --- /dev/null +++ b/internal/cert/pem.go @@ -0,0 +1,13 @@ +// Copyright 2024 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package cert + +import "time" + +type PEM struct { + CertPEM []byte + KeyPEM []byte + NotBefore time.Time + NotAfter time.Time +} diff --git a/internal/certauthority/certauthority.go b/internal/certauthority/certauthority.go index ab9e086d2..76ef5d97f 100644 --- a/internal/certauthority/certauthority.go +++ b/internal/certauthority/certauthority.go @@ -1,4 +1,4 @@ -// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 // Package certauthority implements a simple x509 certificate authority suitable for use in an aggregated API service. @@ -20,6 +20,7 @@ import ( "net" "time" + "go.pinniped.dev/internal/cert" "go.pinniped.dev/internal/constable" ) @@ -38,7 +39,7 @@ type env struct { // clock tells the current time (usually time.Now(), but broken out here for tests). clock func() time.Time - // parse function to parse an ASN.1 byte slice into an x509 struct (normally x509.ParseCertificate) + // parse function to parse an ASN.1 byte slice into a x509 struct (normally x509.ParseCertificate) parseCert func([]byte) (*x509.Certificate, error) } @@ -180,19 +181,19 @@ func (c *CA) IssueServerCert(dnsNames []string, ips []net.IP, ttl time.Duration) } // IssueClientCertPEM is similar to IssueClientCert, but returns 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) { +// for the certificate and private key, along with the notBefore and notAfter values. +func (c *CA) IssueClientCertPEM(username string, groups []string, ttl time.Duration) (*cert.PEM, error) { return toPEM(c.IssueClientCert(username, groups, ttl)) } // IssueServerCertPEM is similar to IssueServerCert, but returns 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) { +// for the certificate and private key, along with the notBefore and notAfter values. +func (c *CA) IssueServerCertPEM(dnsNames []string, ips []net.IP, ttl time.Duration) (*cert.PEM, 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. + // Choose a random 128-bit serial number. serialNumber, err := randomSerial(c.env.serialRNG) if err != nil { return nil, fmt.Errorf("could not generate serial number for certificate: %w", err) @@ -209,7 +210,7 @@ func (c *CA) issueCert(extKeyUsage x509.ExtKeyUsage, subject pkix.Name, dnsNames notBefore := now.Add(-certBackdate) notAfter := now.Add(ttl) - // Parse the DER encoded certificate to get an x509.Certificate. + // Parse the DER encoded certificate to get a x509.Certificate. caCert, err := x509.ParseCertificate(c.caCertBytes) if err != nil { return nil, fmt.Errorf("could not parse CA certificate: %w", err) @@ -246,18 +247,23 @@ func (c *CA) issueCert(extKeyUsage x509.ExtKeyUsage, subject pkix.Name, dnsNames }, nil } -func toPEM(cert *tls.Certificate, err error) ([]byte, []byte, error) { +func toPEM(certificate *tls.Certificate, err error) (*cert.PEM, error) { // If the wrapped IssueServerCert() returned an error, pass it back. if err != nil { - return nil, nil, err + return nil, err } - certPEM, keyPEM, err := ToPEM(cert) + certPEM, keyPEM, err := ToPEM(certificate) if err != nil { - return nil, nil, err + return nil, err } - return certPEM, keyPEM, nil + return &cert.PEM{ + CertPEM: certPEM, + KeyPEM: keyPEM, + NotBefore: certificate.Leaf.NotBefore, + NotAfter: certificate.Leaf.NotAfter, + }, nil } // ToPEM encodes a tls.Certificate into a private key PEM and a cert chain PEM. @@ -279,7 +285,7 @@ func ToPEM(cert *tls.Certificate) ([]byte, []byte, error) { return certPEM, keyPEM, nil } -// randomSerial generates a random 128 bit serial number. +// randomSerial generates a random 128-bit serial number. func randomSerial(rng io.Reader) (*big.Int, error) { return rand.Int(rng, new(big.Int).Lsh(big.NewInt(1), 128)) } diff --git a/internal/certauthority/certauthority_test.go b/internal/certauthority/certauthority_test.go index 0128b1ad0..b59eeb0f5 100644 --- a/internal/certauthority/certauthority_test.go +++ b/internal/certauthority/certauthority_test.go @@ -228,7 +228,7 @@ 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) + now := time.Date(2020, 7, 10, 12, 41, 12, 0, time.UTC) realCA, err := Load(testCert, testKey) require.NoError(t, err) @@ -323,6 +323,8 @@ func TestIssue(t *testing.T) { } else { require.NoError(t, err) require.NotNil(t, got) + require.Equal(t, now.Add(-5*time.Minute), got.Leaf.NotBefore) // always back-dated + require.Equal(t, now.Add(10*time.Minute), got.Leaf.NotAfter) } got, err = tt.ca.IssueClientCert("test-user", []string{"group1", "group2"}, 10*time.Minute) if tt.wantErr != "" { @@ -331,6 +333,8 @@ func TestIssue(t *testing.T) { } else { require.NoError(t, err) require.NotNil(t, got) + require.Equal(t, now.Add(-5*time.Minute), got.Leaf.NotBefore) // always back-dated + require.Equal(t, now.Add(10*time.Minute), got.Leaf.NotAfter) } }) } @@ -341,26 +345,26 @@ func TestToPEM(t *testing.T) { require.NoError(t, err) t.Run("error from input", func(t *testing.T) { - certPEM, keyPEM, err := toPEM(nil, fmt.Errorf("some error")) + pem, err := toPEM(nil, fmt.Errorf("some error")) require.EqualError(t, err, "some error") - require.Nil(t, certPEM) - require.Nil(t, keyPEM) + require.Nil(t, pem) }) t.Run("invalid private key", func(t *testing.T) { cert := realCert cert.PrivateKey = nil - certPEM, keyPEM, err := toPEM(&cert, nil) + pem, err := toPEM(&cert, nil) require.EqualError(t, err, "failed to marshal private key into PKCS8: x509: unknown key type while marshaling PKCS#8: ") - require.Nil(t, certPEM) - require.Nil(t, keyPEM) + require.Nil(t, pem) }) t.Run("success", func(t *testing.T) { - certPEM, keyPEM, err := toPEM(&realCert, nil) + pem, err := toPEM(&realCert, nil) require.NoError(t, err) - require.NotEmpty(t, certPEM) - require.NotEmpty(t, keyPEM) + require.NotEmpty(t, pem.CertPEM) + require.NotEmpty(t, pem.KeyPEM) + require.Equal(t, time.Date(2020, time.July, 25, 21, 4, 18, 0, time.UTC), pem.NotBefore) + require.Equal(t, time.Date(2030, time.July, 23, 21, 4, 18, 0, time.UTC), pem.NotAfter) }) } @@ -381,21 +385,21 @@ func TestIssueMethods(t *testing.T) { require.NoError(t, err) validateClientCert(t, ca.Bundle(), certPEM, keyPEM, user, groups, ttl) - certPEM, keyPEM, err = ca.IssueClientCertPEM(user, groups, ttl) + pem, err := ca.IssueClientCertPEM(user, groups, ttl) require.NoError(t, err) - validateClientCert(t, ca.Bundle(), certPEM, keyPEM, user, groups, ttl) + validateClientCert(t, ca.Bundle(), pem.CertPEM, pem.KeyPEM, user, groups, ttl) - certPEM, keyPEM, err = ca.IssueClientCertPEM(user, nil, ttl) + pem, err = ca.IssueClientCertPEM(user, nil, ttl) require.NoError(t, err) - validateClientCert(t, ca.Bundle(), certPEM, keyPEM, user, nil, ttl) + validateClientCert(t, ca.Bundle(), pem.CertPEM, pem.KeyPEM, user, nil, ttl) - certPEM, keyPEM, err = ca.IssueClientCertPEM(user, []string{}, ttl) + pem, err = ca.IssueClientCertPEM(user, []string{}, ttl) require.NoError(t, err) - validateClientCert(t, ca.Bundle(), certPEM, keyPEM, user, nil, ttl) + validateClientCert(t, ca.Bundle(), pem.CertPEM, pem.KeyPEM, user, nil, ttl) - certPEM, keyPEM, err = ca.IssueClientCertPEM("", []string{}, ttl) + pem, err = ca.IssueClientCertPEM("", []string{}, ttl) require.NoError(t, err) - validateClientCert(t, ca.Bundle(), certPEM, keyPEM, "", nil, ttl) + validateClientCert(t, ca.Bundle(), pem.CertPEM, pem.KeyPEM, "", nil, ttl) }) t.Run("server certs", func(t *testing.T) { @@ -408,25 +412,25 @@ func TestIssueMethods(t *testing.T) { require.NoError(t, err) validateServerCert(t, ca.Bundle(), certPEM, keyPEM, dnsNames, ips, ttl) - certPEM, keyPEM, err = ca.IssueServerCertPEM(dnsNames, ips, ttl) + pem, err := ca.IssueServerCertPEM(dnsNames, ips, ttl) require.NoError(t, err) - validateServerCert(t, ca.Bundle(), certPEM, keyPEM, dnsNames, ips, ttl) + validateServerCert(t, ca.Bundle(), pem.CertPEM, pem.KeyPEM, dnsNames, ips, ttl) - certPEM, keyPEM, err = ca.IssueServerCertPEM(nil, ips, ttl) + pem, err = ca.IssueServerCertPEM(nil, ips, ttl) require.NoError(t, err) - validateServerCert(t, ca.Bundle(), certPEM, keyPEM, nil, ips, ttl) + validateServerCert(t, ca.Bundle(), pem.CertPEM, pem.KeyPEM, nil, ips, ttl) - certPEM, keyPEM, err = ca.IssueServerCertPEM(dnsNames, nil, ttl) + pem, err = ca.IssueServerCertPEM(dnsNames, nil, ttl) require.NoError(t, err) - validateServerCert(t, ca.Bundle(), certPEM, keyPEM, dnsNames, nil, ttl) + validateServerCert(t, ca.Bundle(), pem.CertPEM, pem.KeyPEM, dnsNames, nil, ttl) - certPEM, keyPEM, err = ca.IssueServerCertPEM([]string{}, ips, ttl) + pem, err = ca.IssueServerCertPEM([]string{}, ips, ttl) require.NoError(t, err) - validateServerCert(t, ca.Bundle(), certPEM, keyPEM, nil, ips, ttl) + validateServerCert(t, ca.Bundle(), pem.CertPEM, pem.KeyPEM, nil, ips, ttl) - certPEM, keyPEM, err = ca.IssueServerCertPEM(dnsNames, []net.IP{}, ttl) + pem, err = ca.IssueServerCertPEM(dnsNames, []net.IP{}, ttl) require.NoError(t, err) - validateServerCert(t, ca.Bundle(), certPEM, keyPEM, dnsNames, nil, ttl) + validateServerCert(t, ca.Bundle(), pem.CertPEM, pem.KeyPEM, dnsNames, nil, ttl) }) } diff --git a/internal/certauthority/dynamiccertauthority/dynamiccertauthority.go b/internal/certauthority/dynamiccertauthority/dynamiccertauthority.go index e75c7a257..197112797 100644 --- a/internal/certauthority/dynamiccertauthority/dynamiccertauthority.go +++ b/internal/certauthority/dynamiccertauthority/dynamiccertauthority.go @@ -1,4 +1,4 @@ -// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 // Package dynamiccertauthority implements a x509 certificate authority capable of issuing @@ -10,6 +10,7 @@ import ( "k8s.io/apiserver/pkg/server/dynamiccertificates" + "go.pinniped.dev/internal/cert" "go.pinniped.dev/internal/certauthority" "go.pinniped.dev/internal/clientcertissuer" ) @@ -32,15 +33,15 @@ func (c *ca) Name() string { } // 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) IssueClientCertPEM(username string, groups []string, ttl time.Duration) ([]byte, []byte, error) { +// pair of PEM-formatted byte slices for the certificate and private key, along with the notBefore and notAfter values. +func (c *ca) IssueClientCertPEM(username string, groups []string, ttl time.Duration) (*cert.PEM, error) { caCrtPEM, caKeyPEM := c.provider.CurrentCertKeyContent() // in the future we could split dynamiccert.Private into two interfaces (Private and PrivateRead) // and have this code take PrivateRead as input. We would then add ourselves as a listener to // the PrivateRead. This would allow us to only reload the CA contents when they actually change. ca, err := certauthority.Load(string(caCrtPEM), string(caKeyPEM)) if err != nil { - return nil, nil, err + return nil, err } return ca.IssueClientCertPEM(username, groups, ttl) diff --git a/internal/certauthority/dynamiccertauthority/dynamiccertauthority_test.go b/internal/certauthority/dynamiccertauthority/dynamiccertauthority_test.go index 665f6d191..34935e845 100644 --- a/internal/certauthority/dynamiccertauthority/dynamiccertauthority_test.go +++ b/internal/certauthority/dynamiccertauthority/dynamiccertauthority_test.go @@ -9,6 +9,7 @@ import ( "github.com/stretchr/testify/require" + "go.pinniped.dev/internal/cert" "go.pinniped.dev/internal/clientcertissuer" "go.pinniped.dev/internal/dynamiccert" "go.pinniped.dev/internal/testutil" @@ -93,36 +94,35 @@ func TestCAIssuePEM(t *testing.T) { // Can't run these steps in parallel, because each one depends on the previous steps being // run. - crtPEM, keyPEM, err := issuePEM(provider, ca, step.caCrtPEM, step.caKeyPEM) + pem, err := issuePEM(provider, ca, step.caCrtPEM, step.caKeyPEM) if step.wantError != "" { require.EqualError(t, err, step.wantError) - require.Empty(t, crtPEM) - require.Empty(t, keyPEM) + require.Nil(t, pem) } else { require.NoError(t, err) - require.NotEmpty(t, crtPEM) - require.NotEmpty(t, keyPEM) + require.NotEmpty(t, pem.CertPEM) + require.NotEmpty(t, pem.KeyPEM) caCrtPEM, _ := provider.CurrentCertKeyContent() - crtAssertions := testutil.ValidateClientCertificate(t, string(caCrtPEM), string(crtPEM)) + crtAssertions := testutil.ValidateClientCertificate(t, string(caCrtPEM), string(pem.CertPEM)) 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)) + crtAssertions.RequireMatchesPrivateKey(string(pem.KeyPEM)) } }) } } -func issuePEM(provider dynamiccert.Provider, ca clientcertissuer.ClientCertIssuer, caCrt, caKey []byte) ([]byte, []byte, error) { +func issuePEM(provider dynamiccert.Provider, ca clientcertissuer.ClientCertIssuer, caCrt, caKey []byte) (*cert.PEM, error) { // if setting fails, look at that error if caCrt != nil || caKey != nil { if err := provider.SetCertKeyContent(caCrt, caKey); err != nil { - return nil, nil, err + return nil, err } } - // otherwise check to see if their is an issuing error + // otherwise check to see if there is an issuing error return ca.IssueClientCertPEM("some-username", []string{"some-group1", "some-group2"}, time.Hour*24) } diff --git a/internal/clientcertissuer/issuer.go b/internal/clientcertissuer/issuer.go index f84f7beda..3d7901051 100644 --- a/internal/clientcertissuer/issuer.go +++ b/internal/clientcertissuer/issuer.go @@ -10,6 +10,7 @@ import ( utilerrors "k8s.io/apimachinery/pkg/util/errors" + "go.pinniped.dev/internal/cert" "go.pinniped.dev/internal/constable" ) @@ -17,7 +18,7 @@ const defaultCertIssuerErr = constable.Error("failed to issue cert") type ClientCertIssuer interface { Name() string - IssueClientCertPEM(username string, groups []string, ttl time.Duration) (certPEM, keyPEM []byte, err error) + IssueClientCertPEM(username string, groups []string, ttl time.Duration) (pem *cert.PEM, err error) } var _ ClientCertIssuer = ClientCertIssuers{} @@ -37,20 +38,20 @@ func (c ClientCertIssuers) Name() string { return strings.Join(names, ",") } -func (c ClientCertIssuers) IssueClientCertPEM(username string, groups []string, ttl time.Duration) ([]byte, []byte, error) { +func (c ClientCertIssuers) IssueClientCertPEM(username string, groups []string, ttl time.Duration) (*cert.PEM, error) { errs := make([]error, 0, len(c)) for _, issuer := range c { - certPEM, keyPEM, err := issuer.IssueClientCertPEM(username, groups, ttl) + pem, err := issuer.IssueClientCertPEM(username, groups, ttl) if err == nil { - return certPEM, keyPEM, nil + return pem, nil } errs = append(errs, fmt.Errorf("%s failed to issue client cert: %w", issuer.Name(), err)) } if err := utilerrors.NewAggregate(errs); err != nil { - return nil, nil, err + return nil, err } - return nil, nil, defaultCertIssuerErr + return nil, defaultCertIssuerErr } diff --git a/internal/clientcertissuer/issuer_test.go b/internal/clientcertissuer/issuer_test.go index 81a615971..a14e08607 100644 --- a/internal/clientcertissuer/issuer_test.go +++ b/internal/clientcertissuer/issuer_test.go @@ -11,6 +11,7 @@ import ( "github.com/stretchr/testify/require" "go.uber.org/mock/gomock" + "go.pinniped.dev/internal/cert" "go.pinniped.dev/internal/mocks/mockissuer" ) @@ -85,7 +86,7 @@ func TestIssueClientCertPEM(t *testing.T) { errClientCertIssuer.EXPECT().Name().Return("error cert issuer") errClientCertIssuer.EXPECT(). IssueClientCertPEM("username", []string{"group1", "group2"}, 32*time.Second). - Return(nil, nil, errors.New("error from wrapped cert issuer")) + Return(nil, errors.New("error from wrapped cert issuer")) return ClientCertIssuers{errClientCertIssuer} }, wantErrorMessage: "error cert issuer failed to issue client cert: error from wrapped cert issuer", @@ -96,7 +97,7 @@ func TestIssueClientCertPEM(t *testing.T) { validClientCertIssuer := mockissuer.NewMockClientCertIssuer(ctrl) validClientCertIssuer.EXPECT(). IssueClientCertPEM("username", []string{"group1", "group2"}, 32*time.Second). - Return([]byte("cert"), []byte("key"), nil) + Return(&cert.PEM{CertPEM: []byte("cert"), KeyPEM: []byte("key")}, nil) return ClientCertIssuers{validClientCertIssuer} }, wantCert: []byte("cert"), @@ -109,12 +110,12 @@ func TestIssueClientCertPEM(t *testing.T) { errClientCertIssuer.EXPECT().Name().Return("error cert issuer") errClientCertIssuer.EXPECT(). IssueClientCertPEM("username", []string{"group1", "group2"}, 32*time.Second). - Return(nil, nil, errors.New("error from wrapped cert issuer")) + Return(nil, errors.New("error from wrapped cert issuer")) validClientCertIssuer := mockissuer.NewMockClientCertIssuer(ctrl) validClientCertIssuer.EXPECT(). IssueClientCertPEM("username", []string{"group1", "group2"}, 32*time.Second). - Return([]byte("cert"), []byte("key"), nil) + Return(&cert.PEM{CertPEM: []byte("cert"), KeyPEM: []byte("key")}, nil) return ClientCertIssuers{ errClientCertIssuer, validClientCertIssuer, @@ -130,13 +131,13 @@ func TestIssueClientCertPEM(t *testing.T) { err1ClientCertIssuer.EXPECT().Name().Return("error1 cert issuer") err1ClientCertIssuer.EXPECT(). IssueClientCertPEM("username", []string{"group1", "group2"}, 32*time.Second). - Return(nil, nil, errors.New("error1 from wrapped cert issuer")) + Return(nil, errors.New("error1 from wrapped cert issuer")) err2ClientCertIssuer := mockissuer.NewMockClientCertIssuer(ctrl) err2ClientCertIssuer.EXPECT().Name().Return("error2 cert issuer") err2ClientCertIssuer.EXPECT(). IssueClientCertPEM("username", []string{"group1", "group2"}, 32*time.Second). - Return(nil, nil, errors.New("error2 from wrapped cert issuer")) + Return(nil, errors.New("error2 from wrapped cert issuer")) return ClientCertIssuers{ err1ClientCertIssuer, @@ -152,17 +153,16 @@ func TestIssueClientCertPEM(t *testing.T) { t.Run(testcase.name, func(t *testing.T) { t.Parallel() - certPEM, keyPEM, err := testcase.buildIssuerMocks(). + pem, err := testcase.buildIssuerMocks(). IssueClientCertPEM("username", []string{"group1", "group2"}, 32*time.Second) if testcase.wantErrorMessage != "" { require.ErrorContains(t, err, testcase.wantErrorMessage) - require.Empty(t, certPEM) - require.Empty(t, keyPEM) + require.Nil(t, pem) } else { require.NoError(t, err) - require.Equal(t, testcase.wantCert, certPEM) - require.Equal(t, testcase.wantKey, keyPEM) + require.Equal(t, testcase.wantCert, pem.CertPEM) + require.Equal(t, testcase.wantKey, pem.KeyPEM) } }) } diff --git a/internal/concierge/apiserver/apiserver.go b/internal/concierge/apiserver/apiserver.go index b58a4b2c2..88d38f922 100644 --- a/internal/concierge/apiserver/apiserver.go +++ b/internal/concierge/apiserver/apiserver.go @@ -15,7 +15,6 @@ import ( "k8s.io/apiserver/pkg/registry/rest" genericapiserver "k8s.io/apiserver/pkg/server" utilversion "k8s.io/apiserver/pkg/util/version" - "k8s.io/utils/clock" "go.pinniped.dev/internal/clientcertissuer" "go.pinniped.dev/internal/controllerinit" @@ -89,7 +88,6 @@ func (c completedConfig) New() (*PinnipedServer, error) { c.ExtraConfig.Issuer, tokenCredReqGVR.GroupResource(), c.ExtraConfig.AuditLogger, - clock.RealClock{}, ) return tokenCredReqGVR, tokenCredStorage }, diff --git a/internal/concierge/impersonator/impersonator_test.go b/internal/concierge/impersonator/impersonator_test.go index ede5ba385..aa49eaed2 100644 --- a/internal/concierge/impersonator/impersonator_test.go +++ b/internal/concierge/impersonator/impersonator_test.go @@ -70,10 +70,10 @@ func TestImpersonator(t *testing.T) { err = caContent.SetCertKeyContent(ca.Bundle(), caKey) require.NoError(t, err) - cert, key, err := ca.IssueServerCertPEM(nil, []net.IP{net.ParseIP("127.0.0.1")}, time.Hour) + pem, err := ca.IssueServerCertPEM(nil, []net.IP{net.ParseIP("127.0.0.1")}, time.Hour) require.NoError(t, err) certKeyContent := dynamiccert.NewServingCert("cert-key") - err = certKeyContent.SetCertKeyContent(cert, key) + err = certKeyContent.SetCertKeyContent(pem.CertPEM, pem.KeyPEM) require.NoError(t, err) unrelatedCA, err := certauthority.New("ca", time.Hour) @@ -1997,11 +1997,11 @@ type clientCert struct { func newClientCert(t *testing.T, ca *certauthority.CA, username string, groups []string) *clientCert { t.Helper() - certPEM, keyPEM, err := ca.IssueClientCertPEM(username, groups, time.Hour) + pem, err := ca.IssueClientCertPEM(username, groups, time.Hour) require.NoError(t, err) return &clientCert{ - certPEM: certPEM, - keyPEM: keyPEM, + certPEM: pem.CertPEM, + keyPEM: pem.KeyPEM, } } diff --git a/internal/controller/apicerts/certs_observer_test.go b/internal/controller/apicerts/certs_observer_test.go index 245fba1f0..ccb11929e 100644 --- a/internal/controller/apicerts/certs_observer_test.go +++ b/internal/controller/apicerts/certs_observer_test.go @@ -1,4 +1,4 @@ -// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package apicerts @@ -173,10 +173,10 @@ func TestObserverControllerSync(t *testing.T) { ca, err := certauthority.Load(string(caCrt), string(caKey)) require.NoError(t, err) - crt, key, err := ca.IssueServerCertPEM(nil, nil, time.Hour) + pem, err := ca.IssueServerCertPEM(nil, nil, time.Hour) require.NoError(t, err) - err = dynamicCertProvider.SetCertKeyContent(crt, key) + err = dynamicCertProvider.SetCertKeyContent(pem.CertPEM, pem.KeyPEM) r.NoError(err) }) @@ -202,7 +202,7 @@ func TestObserverControllerSync(t *testing.T) { ca, err := certauthority.Load(string(caCrt), string(caKey)) require.NoError(t, err) - crt, key, err := ca.IssueServerCertPEM(nil, nil, time.Hour) + pem, err := ca.IssueServerCertPEM(nil, nil, time.Hour) require.NoError(t, err) apiServingCertSecret := &corev1.Secret{ @@ -212,8 +212,8 @@ func TestObserverControllerSync(t *testing.T) { }, Data: map[string][]byte{ "caCertificate": []byte("fake cert"), - "tlsPrivateKey": key, - "tlsCertificateChain": crt, + "tlsPrivateKey": pem.KeyPEM, + "tlsCertificateChain": pem.CertPEM, }, } err = kubeInformerClient.Tracker().Add(apiServingCertSecret) diff --git a/internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go b/internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go index b1654be82..89ce4cdaa 100644 --- a/internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go +++ b/internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go @@ -79,7 +79,7 @@ func TestController(t *testing.T) { require.NoError(t, err) someUnknownHostNames := []string{"some-dns-name", "some-other-dns-name"} someLocalIPAddress := []net.IP{net.ParseIP("10.2.3.4")} - pemServerCertForUnknownServer, _, err := caForUnknownServer.IssueServerCertPEM( + pemServerCertForUnknownServer, err := caForUnknownServer.IssueServerCertPEM( someUnknownHostNames, someLocalIPAddress, time.Hour, @@ -216,7 +216,7 @@ func TestController(t *testing.T) { badWebhookAuthenticatorSpecGoodEndpointButUnknownCA := authenticationv1alpha1.WebhookAuthenticatorSpec{ Endpoint: goodWebhookDefaultServingCertEndpoint, TLS: &authenticationv1alpha1.TLSSpec{ - CertificateAuthorityData: base64.StdEncoding.EncodeToString(pemServerCertForUnknownServer), + CertificateAuthorityData: base64.StdEncoding.EncodeToString(pemServerCertForUnknownServer.CertPEM), }, } diff --git a/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher_test.go b/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher_test.go index eb4325c9c..c65b300fb 100644 --- a/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher_test.go +++ b/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher_test.go @@ -127,7 +127,7 @@ func TestController(t *testing.T) { caForUnknownServer, err := certauthority.New("Some Unknown CA", time.Hour) require.NoError(t, err) - unknownServerCABytes, _, err := caForUnknownServer.IssueServerCertPEM( + unknownServerPEM, err := caForUnknownServer.IssueServerCertPEM( []string{"some-dns-name", "some-other-dns-name"}, []net.IP{net.ParseIP("10.2.3.4")}, time.Hour, @@ -1849,7 +1849,7 @@ func TestController(t *testing.T) { func() runtime.Object { badIDP := validFilledOutIDP.DeepCopy() badIDP.Spec.GitHubAPI.TLS = &idpv1alpha1.TLSSpec{ - CertificateAuthorityData: base64.StdEncoding.EncodeToString(unknownServerCABytes), + CertificateAuthorityData: base64.StdEncoding.EncodeToString(unknownServerPEM.CertPEM), } return badIDP }(), @@ -1861,7 +1861,7 @@ func TestController(t *testing.T) { Spec: func() idpv1alpha1.GitHubIdentityProviderSpec { badSpec := validFilledOutIDP.Spec.DeepCopy() badSpec.GitHubAPI.TLS = &idpv1alpha1.TLSSpec{ - CertificateAuthorityData: base64.StdEncoding.EncodeToString(unknownServerCABytes), + CertificateAuthorityData: base64.StdEncoding.EncodeToString(unknownServerPEM.CertPEM), } return *badSpec }(), diff --git a/internal/dynamiccert/provider_test.go b/internal/dynamiccert/provider_test.go index 385adc364..ca55f5d8e 100644 --- a/internal/dynamiccert/provider_test.go +++ b/internal/dynamiccert/provider_test.go @@ -114,13 +114,13 @@ func TestProviderWithDynamicServingCertificateController(t *testing.T) { newCA, err := certauthority.New(names.SimpleNameGenerator.GenerateName("new-ca"), time.Hour) require.NoError(t, err) - certPEM, keyPEM, err := newCA.IssueServerCertPEM(nil, []net.IP{net.ParseIP("127.0.0.2")}, time.Hour) + pem, err := newCA.IssueServerCertPEM(nil, []net.IP{net.ParseIP("127.0.0.2")}, time.Hour) require.NoError(t, err) - err = certKey.SetCertKeyContent(certPEM, keyPEM) + err = certKey.SetCertKeyContent(pem.CertPEM, pem.KeyPEM) require.NoError(t, err) - cert, err := tls.X509KeyPair(certPEM, keyPEM) + cert, err := tls.X509KeyPair(pem.CertPEM, pem.KeyPEM) require.NoError(t, err) return []tls.Certificate{cert} @@ -144,13 +144,13 @@ func TestProviderWithDynamicServingCertificateController(t *testing.T) { newCA, err := certauthority.New(names.SimpleNameGenerator.GenerateName("new-ca"), time.Hour) require.NoError(t, err) - certPEM, keyPEM, err := newCA.IssueServerCertPEM(nil, []net.IP{net.ParseIP("127.0.0.3")}, time.Hour) + pem, err := newCA.IssueServerCertPEM(nil, []net.IP{net.ParseIP("127.0.0.3")}, time.Hour) require.NoError(t, err) - err = certKey.SetCertKeyContent(certPEM, keyPEM) + err = certKey.SetCertKeyContent(pem.CertPEM, pem.KeyPEM) require.NoError(t, err) - cert, err := tls.X509KeyPair(certPEM, keyPEM) + cert, err := tls.X509KeyPair(pem.CertPEM, pem.KeyPEM) require.NoError(t, err) return []tls.Certificate{cert} @@ -170,10 +170,10 @@ func TestProviderWithDynamicServingCertificateController(t *testing.T) { err = caContent.SetCertKeyContent(ca.Bundle(), caKey) require.NoError(t, err) - cert, key, err := ca.IssueServerCertPEM(nil, []net.IP{net.ParseIP("127.0.0.1")}, time.Hour) + pem, err := ca.IssueServerCertPEM(nil, []net.IP{net.ParseIP("127.0.0.1")}, time.Hour) require.NoError(t, err) certKeyContent := NewServingCert("cert-key") - err = certKeyContent.SetCertKeyContent(cert, key) + err = certKeyContent.SetCertKeyContent(pem.CertPEM, pem.KeyPEM) require.NoError(t, err) tlsConfig := ptls.Default(nil) diff --git a/internal/mocks/mockissuer/mockissuer.go b/internal/mocks/mockissuer/mockissuer.go index 1950e3dd8..4a22d01de 100644 --- a/internal/mocks/mockissuer/mockissuer.go +++ b/internal/mocks/mockissuer/mockissuer.go @@ -17,6 +17,7 @@ import ( reflect "reflect" time "time" + cert "go.pinniped.dev/internal/cert" gomock "go.uber.org/mock/gomock" ) @@ -45,13 +46,12 @@ func (m *MockClientCertIssuer) EXPECT() *MockClientCertIssuerMockRecorder { } // IssueClientCertPEM mocks base method. -func (m *MockClientCertIssuer) IssueClientCertPEM(username string, groups []string, ttl time.Duration) ([]byte, []byte, error) { +func (m *MockClientCertIssuer) IssueClientCertPEM(username string, groups []string, ttl time.Duration) (*cert.PEM, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "IssueClientCertPEM", username, groups, ttl) - ret0, _ := ret[0].([]byte) - ret1, _ := ret[1].([]byte) - ret2, _ := ret[2].(error) - return ret0, ret1, ret2 + ret0, _ := ret[0].(*cert.PEM) + ret1, _ := ret[1].(error) + return ret0, ret1 } // IssueClientCertPEM indicates an expected call of IssueClientCertPEM. diff --git a/internal/registry/credentialrequest/rest.go b/internal/registry/credentialrequest/rest.go index a8ef1c318..6edd83340 100644 --- a/internal/registry/credentialrequest/rest.go +++ b/internal/registry/credentialrequest/rest.go @@ -20,7 +20,6 @@ import ( "k8s.io/apiserver/pkg/authentication/user" genericapirequest "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/registry/rest" - "k8s.io/utils/clock" loginapi "go.pinniped.dev/generated/latest/apis/concierge/login" "go.pinniped.dev/internal/auditevent" @@ -40,14 +39,12 @@ func NewREST( issuer clientcertissuer.ClientCertIssuer, resource schema.GroupResource, auditLogger plog.AuditLogger, - clock clock.Clock, ) *REST { return &REST{ authenticator: authenticator, issuer: issuer, tableConvertor: rest.NewDefaultTableConvertor(resource), auditLogger: auditLogger, - clock: clock, } } @@ -56,7 +53,6 @@ type REST struct { issuer clientcertissuer.ClientCertIssuer tableConvertor rest.TableConvertor auditLogger plog.AuditLogger - clock clock.Clock } // Assert that our *REST implements all the optional interfaces that we expect it to implement. @@ -162,9 +158,7 @@ func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation return authenticationFailedResponse(), nil } - // this timestamp should be returned from IssueClientCertPEM but this is a safe approximation - expires := metav1.NewTime(r.clock.Now().UTC().Add(clientCertificateTTL)) - certPEM, keyPEM, err := r.issuer.IssueClientCertPEM(userInfo.GetName(), userInfo.GetGroups(), clientCertificateTTL) + pem, err := r.issuer.IssueClientCertPEM(userInfo.GetName(), userInfo.GetGroups(), clientCertificateTTL) if err != nil { r.auditLogger.Audit(auditevent.TokenCredentialRequestUnexpectedError, &plog.AuditParams{ ReqCtx: ctx, @@ -177,6 +171,9 @@ func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation return authenticationFailedResponse(), nil } + notBefore := metav1.NewTime(pem.NotBefore) + notAfter := metav1.NewTime(pem.NotAfter) + r.auditLogger.Audit(auditevent.TokenCredentialRequestAuthenticatedUser, &plog.AuditParams{ ReqCtx: ctx, PIIKeysAndValues: []any{ @@ -184,7 +181,10 @@ func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation "groups", userInfo.GetGroups(), }, KeysAndValues: []any{ - "issuedClientCertExpires", expires.Format(time.RFC3339), + "issuedClientCert", map[string]string{ + "notBefore": notBefore.Format(time.RFC3339), + "notAfter": notAfter.Format(time.RFC3339), + }, "authenticator", credentialRequest.Spec.Authenticator, }, }) @@ -192,9 +192,9 @@ func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation return &loginapi.TokenCredentialRequest{ Status: loginapi.TokenCredentialRequestStatus{ Credential: &loginapi.ClusterCredential{ - ExpirationTimestamp: expires, - ClientCertificateData: string(certPEM), - ClientKeyData: string(keyPEM), + ExpirationTimestamp: notAfter, + ClientCertificateData: string(pem.CertPEM), + ClientKeyData: string(pem.KeyPEM), }, }, }, nil diff --git a/internal/registry/credentialrequest/rest_test.go b/internal/registry/credentialrequest/rest_test.go index 854746738..2bbf38a9b 100644 --- a/internal/registry/credentialrequest/rest_test.go +++ b/internal/registry/credentialrequest/rest_test.go @@ -24,11 +24,10 @@ import ( "k8s.io/apiserver/pkg/authentication/user" genericapirequest "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/registry/rest" - "k8s.io/utils/clock" - clocktesting "k8s.io/utils/clock/testing" "k8s.io/utils/ptr" loginapi "go.pinniped.dev/generated/latest/apis/concierge/login" + "go.pinniped.dev/internal/cert" "go.pinniped.dev/internal/clientcertissuer" "go.pinniped.dev/internal/mocks/mockcredentialrequest" "go.pinniped.dev/internal/mocks/mockissuer" @@ -37,7 +36,7 @@ import ( ) func TestNew(t *testing.T) { - r := NewREST(nil, nil, schema.GroupResource{Group: "bears", Resource: "panda"}, nil, clock.RealClock{}) + r := NewREST(nil, nil, schema.GroupResource{Group: "bears", Resource: "panda"}, nil) require.NotNil(t, r) require.False(t, r.NamespaceScoped()) require.Equal(t, []string{"pinniped"}, r.Categories()) @@ -78,16 +77,14 @@ func TestCreate(t *testing.T) { var ctrl *gomock.Controller var auditLogger plog.AuditLogger var actualAuditLog *bytes.Buffer - var frozenNow time.Time - var frozenClock *clocktesting.FakeClock + var fakeNow time.Time var wantAuditLog []testutil.WantedAuditLog it.Before(func() { r = require.New(t) ctrl = gomock.NewController(t) auditLogger, actualAuditLog = plog.TestAuditLogger(t) - frozenNow = time.Date(2024, time.September, 12, 4, 25, 56, 778899, time.UTC) - frozenClock = clocktesting.NewFakeClock(frozenNow) + fakeNow = time.Date(2024, time.September, 12, 4, 25, 56, 778899, time.UTC) }) it.After(func() { @@ -110,9 +107,14 @@ func TestCreate(t *testing.T) { "test-user", []string{"test-group-1", "test-group-2"}, 5*time.Minute, - ).Return([]byte("test-cert"), []byte("test-key"), nil) + ).Return(&cert.PEM{ + CertPEM: []byte("test-cert"), + KeyPEM: []byte("test-key"), + NotBefore: fakeNow.Add(-5 * time.Minute), + NotAfter: fakeNow.Add(5 * time.Minute), + }, nil) - storage := NewREST(requestAuthenticator, clientCertIssuer, schema.GroupResource{}, auditLogger, frozenClock) + storage := NewREST(requestAuthenticator, clientCertIssuer, schema.GroupResource{}, auditLogger) response, err := callCreate(storage, req) @@ -122,7 +124,7 @@ func TestCreate(t *testing.T) { r.Equal(response, &loginapi.TokenCredentialRequest{ Status: loginapi.TokenCredentialRequestStatus{ Credential: &loginapi.ClusterCredential{ - ExpirationTimestamp: metav1.NewTime(frozenNow.Add(5 * time.Minute).UTC()), + ExpirationTimestamp: metav1.NewTime(fakeNow.Add(5 * time.Minute).UTC()), ClientCertificateData: "test-cert", ClientKeyData: "test-key", }, @@ -141,7 +143,10 @@ func TestCreate(t *testing.T) { "kind": "FakeAuthenticatorKind", "name": "fake-authenticator-name", }, - "issuedClientCertExpires": "2024-09-12T04:30:56Z", // this is frozenNow + 5 minutes in UTC + "issuedClientCert": map[string]any{ + "notBefore": "2024-09-12T04:20:56Z", // this is fakeNow - 5 minutes in UTC + "notAfter": "2024-09-12T04:30:56Z", // this is fakeNow + 5 minutes in UTC + }, "personalInfo": map[string]any{ "username": "test-user", "groups": []any{"test-group-1", "test-group-2"}, @@ -163,9 +168,9 @@ func TestCreate(t *testing.T) { clientCertIssuer := mockissuer.NewMockClientCertIssuer(ctrl) clientCertIssuer.EXPECT(). IssueClientCertPEM(gomock.Any(), gomock.Any(), gomock.Any()). - Return(nil, nil, fmt.Errorf("some certificate authority error")) + Return(nil, fmt.Errorf("some certificate authority error")) - storage := NewREST(requestAuthenticator, clientCertIssuer, schema.GroupResource{}, auditLogger, frozenClock) + storage := NewREST(requestAuthenticator, clientCertIssuer, schema.GroupResource{}, auditLogger) response, err := callCreate(storage, req) requireSuccessfulResponseWithAuthenticationFailureMessage(t, err, response) @@ -194,7 +199,7 @@ func TestCreate(t *testing.T) { requestAuthenticator := mockcredentialrequest.NewMockTokenCredentialRequestAuthenticator(ctrl) requestAuthenticator.EXPECT().AuthenticateTokenCredentialRequest(gomock.Any(), req).Return(nil, nil) - storage := NewREST(requestAuthenticator, nil, schema.GroupResource{}, auditLogger, frozenClock) + storage := NewREST(requestAuthenticator, nil, schema.GroupResource{}, auditLogger) response, err := callCreate(storage, req) @@ -224,7 +229,7 @@ func TestCreate(t *testing.T) { requestAuthenticator.EXPECT().AuthenticateTokenCredentialRequest(gomock.Any(), req). Return(nil, errors.New("some webhook error")) - storage := NewREST(requestAuthenticator, nil, schema.GroupResource{}, auditLogger, frozenClock) + storage := NewREST(requestAuthenticator, nil, schema.GroupResource{}, auditLogger) response, err := callCreate(storage, req) @@ -255,7 +260,7 @@ func TestCreate(t *testing.T) { requestAuthenticator.EXPECT().AuthenticateTokenCredentialRequest(gomock.Any(), req). Return(&user.DefaultInfo{Name: "", UID: "test-uid"}, nil) - storage := NewREST(requestAuthenticator, nil, schema.GroupResource{}, auditLogger, frozenClock) + storage := NewREST(requestAuthenticator, nil, schema.GroupResource{}, auditLogger) response, err := callCreate(storage, req) @@ -295,7 +300,7 @@ func TestCreate(t *testing.T) { Groups: []string{"test-group-1", "test-group-2"}, }, nil) - storage := NewREST(requestAuthenticator, nil, schema.GroupResource{}, auditLogger, frozenClock) + storage := NewREST(requestAuthenticator, nil, schema.GroupResource{}, auditLogger) response, err := callCreate(storage, req) @@ -335,7 +340,7 @@ func TestCreate(t *testing.T) { Extra: map[string][]string{"test-key": {"test-val-1", "test-val-2"}}, }, nil) - storage := NewREST(requestAuthenticator, nil, schema.GroupResource{}, auditLogger, frozenClock) + storage := NewREST(requestAuthenticator, nil, schema.GroupResource{}, auditLogger) response, err := callCreate(storage, req) @@ -366,7 +371,7 @@ func TestCreate(t *testing.T) { it("CreateFailsWhenGivenTheWrongInputType", func() { notACredentialRequest := runtime.Unknown{} - response, err := NewREST(nil, nil, schema.GroupResource{}, auditLogger, frozenClock).Create( + response, err := NewREST(nil, nil, schema.GroupResource{}, auditLogger).Create( genericapirequest.NewContext(), ¬ACredentialRequest, rest.ValidateAllObjectFunc, @@ -376,7 +381,7 @@ func TestCreate(t *testing.T) { }) it("CreateFailsWhenTokenValueIsEmptyInRequest", func() { - storage := NewREST(nil, nil, schema.GroupResource{}, auditLogger, frozenClock) + storage := NewREST(nil, nil, schema.GroupResource{}, auditLogger) response, err := callCreate(storage, credentialRequest(loginapi.TokenCredentialRequestSpec{ Token: "", })) @@ -386,7 +391,7 @@ func TestCreate(t *testing.T) { }) it("CreateFailsWhenValidationFails", func() { - storage := NewREST(nil, nil, schema.GroupResource{}, auditLogger, frozenClock) + storage := NewREST(nil, nil, schema.GroupResource{}, auditLogger) response, err := storage.Create( context.Background(), validCredentialRequest(), @@ -408,7 +413,7 @@ func TestCreate(t *testing.T) { fakeReqContext := audit.WithAuditContext(context.Background()) audit.WithAuditID(fakeReqContext, "fake-audit-id") - storage := NewREST(requestAuthenticator, successfulIssuer(ctrl), schema.GroupResource{}, auditLogger, frozenClock) + storage := NewREST(requestAuthenticator, successfulIssuer(ctrl, fakeNow), schema.GroupResource{}, auditLogger) response, err := storage.Create( fakeReqContext, req, @@ -433,7 +438,10 @@ func TestCreate(t *testing.T) { "kind": "FakeAuthenticatorKind", "name": "fake-authenticator-name", }, - "issuedClientCertExpires": "2024-09-12T04:30:56Z", // this is frozenNow + 5 minutes in UTC + "issuedClientCert": map[string]any{ + "notBefore": "2024-09-12T04:20:56Z", // this is fakeNow - 5 minutes in UTC + "notAfter": "2024-09-12T04:30:56Z", // this is fakeNow + 5 minutes in UTC + }, "personalInfo": map[string]any{ "username": "test-user", "groups": []any{}, @@ -449,7 +457,7 @@ func TestCreate(t *testing.T) { requestAuthenticator.EXPECT().AuthenticateTokenCredentialRequest(gomock.Any(), req.DeepCopy()). Return(&user.DefaultInfo{Name: "test-user"}, nil) - storage := NewREST(requestAuthenticator, successfulIssuer(ctrl), schema.GroupResource{}, auditLogger, frozenClock) + storage := NewREST(requestAuthenticator, successfulIssuer(ctrl, fakeNow), schema.GroupResource{}, auditLogger) fakeReqContext := audit.WithAuditContext(context.Background()) audit.WithAuditID(fakeReqContext, "fake-audit-id") @@ -484,7 +492,10 @@ func TestCreate(t *testing.T) { "kind": "FakeAuthenticatorKind", "name": "fake-authenticator-name", }, - "issuedClientCertExpires": "2024-09-12T04:30:56Z", // this is frozenNow + 5 minutes in UTC + "issuedClientCert": map[string]any{ + "notBefore": "2024-09-12T04:20:56Z", // this is fakeNow - 5 minutes in UTC + "notAfter": "2024-09-12T04:30:56Z", // this is fakeNow + 5 minutes in UTC + }, "personalInfo": map[string]any{ "username": "test-user", "groups": []any{}, @@ -494,7 +505,7 @@ func TestCreate(t *testing.T) { }) it("CreateFailsWhenRequestOptionsDryRunIsNotEmpty", func() { - response, err := NewREST(nil, nil, schema.GroupResource{}, auditLogger, frozenClock).Create( + response, err := NewREST(nil, nil, schema.GroupResource{}, auditLogger).Create( genericapirequest.NewContext(), validCredentialRequest(), rest.ValidateAllObjectFunc, @@ -507,7 +518,7 @@ func TestCreate(t *testing.T) { }) it("CreateFailsWhenNamespaceIsNotEmpty", func() { - response, err := NewREST(nil, nil, schema.GroupResource{}, auditLogger, frozenClock).Create( + response, err := NewREST(nil, nil, schema.GroupResource{}, auditLogger).Create( genericapirequest.WithNamespace(genericapirequest.NewContext(), "some-ns"), validCredentialRequest(), rest.ValidateAllObjectFunc, @@ -576,10 +587,15 @@ func requireSuccessfulResponseWithAuthenticationFailureMessage(t *testing.T, err }) } -func successfulIssuer(ctrl *gomock.Controller) clientcertissuer.ClientCertIssuer { +func successfulIssuer(ctrl *gomock.Controller, fakeNow time.Time) clientcertissuer.ClientCertIssuer { clientCertIssuer := mockissuer.NewMockClientCertIssuer(ctrl) clientCertIssuer.EXPECT(). IssueClientCertPEM(gomock.Any(), gomock.Any(), gomock.Any()). - Return([]byte("test-cert"), []byte("test-key"), nil) + Return(&cert.PEM{ + CertPEM: []byte("test-cert"), + KeyPEM: []byte("test-key"), + NotBefore: fakeNow.Add(-5 * time.Minute), + NotAfter: fakeNow.Add(5 * time.Minute), + }, nil) return clientCertIssuer } diff --git a/site/content/docs/reference/audit-logging.md b/site/content/docs/reference/audit-logging.md index 93763d5d4..c3ab8c2ed 100644 --- a/site/content/docs/reference/audit-logging.md +++ b/site/content/docs/reference/audit-logging.md @@ -499,7 +499,10 @@ the Kubernetes audit logs for the same request, allowing them to be correlated. "username": "pinny@example.com", "groups": ["developers", "auditors"] }, - "issuedClientCertExpires": "2024-11-21T17:54:11Z", + "issuedClientCert": { + "notAfter": "2024-11-21T17:54:11Z", + "notBefore": "2024-11-21T17:44:11Z" + }, "authenticator": { "apiGroup": "authentication.concierge.pinniped.dev", "kind": "JWTAuthenticator", diff --git a/test/integration/audit_test.go b/test/integration/audit_test.go index 976142371..335d09fd2 100644 --- a/test/integration/audit_test.go +++ b/test/integration/audit_test.go @@ -190,10 +190,10 @@ func TestAuditLogsDuringLogin_Disruptive(t *testing.T) { timeBeforeLogin, ) removeSomeKeysFromEachAuditLogEvent(allConciergeTCRLogs) - // Also remove issuedClientCertExpires, which is a timestamp that we can't easily predict for the assertions below. + // Also remove issuedClientCert, which contains timestamps that we can't easily predict for the assertions below. for _, log := range allConciergeTCRLogs { - require.NotEmpty(t, log["issuedClientCertExpires"]) - delete(log, "issuedClientCertExpires") + require.NotEmpty(t, log["issuedClientCert"]) + delete(log, "issuedClientCert") } // All values in the personalInfo map should be redacted by default. @@ -338,10 +338,10 @@ func TestAuditLogsDuringLogin_Disruptive(t *testing.T) { timeBeforeLogin, ) removeSomeKeysFromEachAuditLogEvent(allConciergeTCRLogs) - // Also remove issuedClientCertExpires, which is a timestamp that we can't easily predict for the assertions below. + // Also remove issuedClientCert, which contains timestamps that we can't easily predict for the assertions below. for _, log := range allConciergeTCRLogs { - require.NotEmpty(t, log["issuedClientCertExpires"]) - delete(log, "issuedClientCertExpires") + require.NotEmpty(t, log["issuedClientCert"]) + delete(log, "issuedClientCert") } // All values in the personalInfo map should not be redacted anymore. diff --git a/test/integration/concierge_impersonation_proxy_test.go b/test/integration/concierge_impersonation_proxy_test.go index 96df0343c..4ea8525fc 100644 --- a/test/integration/concierge_impersonation_proxy_test.go +++ b/test/integration/concierge_impersonation_proxy_test.go @@ -1783,8 +1783,7 @@ func TestImpersonationProxy(t *testing.T) { //nolint:gocyclo // yeah, it's compl externallyProvidedCA, err = certauthority.New("Impersonation Proxy Integration Test CA", 1*time.Hour) require.NoError(t, err) - var externallyProvidedTLSServingCertPEM, externallyProvidedTLSServingKeyPEM []byte - externallyProvidedTLSServingCertPEM, externallyProvidedTLSServingKeyPEM, err = externallyProvidedCA.IssueServerCertPEM([]string{proxyServiceEndpoint}, nil, 1*time.Hour) + externallyProvidedTLSServingCertPEM, err := externallyProvidedCA.IssueServerCertPEM([]string{proxyServiceEndpoint}, nil, 1*time.Hour) require.NoError(t, err) // Specifically use corev1.Secret.StringData @@ -1796,8 +1795,8 @@ func TestImpersonationProxy(t *testing.T) { //nolint:gocyclo // yeah, it's compl corev1.SecretTypeTLS, map[string]string{ "ca.crt": string(externallyProvidedCA.Bundle()), - corev1.TLSCertKey: string(externallyProvidedTLSServingCertPEM), - corev1.TLSPrivateKeyKey: string(externallyProvidedTLSServingKeyPEM), + corev1.TLSCertKey: string(externallyProvidedTLSServingCertPEM.CertPEM), + corev1.TLSPrivateKeyKey: string(externallyProvidedTLSServingCertPEM.KeyPEM), }) _, originalInternallyGeneratedCAPEM := performImpersonatorDiscoveryURL(ctx, t, env, adminConciergeClient) @@ -1855,8 +1854,7 @@ func TestImpersonationProxy(t *testing.T) { //nolint:gocyclo // yeah, it's compl externallyProvidedCA, err = certauthority.New("Impersonation Proxy Integration Test CA", 1*time.Hour) require.NoError(t, err) - var externallyProvidedTLSServingCertPEM, externallyProvidedTLSServingKeyPEM []byte - externallyProvidedTLSServingCertPEM, externallyProvidedTLSServingKeyPEM, err = externallyProvidedCA.IssueServerCertPEM([]string{proxyServiceEndpoint}, nil, 1*time.Hour) + externallyProvidedTLSServingCertPEM, err := externallyProvidedCA.IssueServerCertPEM([]string{proxyServiceEndpoint}, nil, 1*time.Hour) require.NoError(t, err) // Specifically use corev1.Secret.Data @@ -1868,8 +1866,8 @@ func TestImpersonationProxy(t *testing.T) { //nolint:gocyclo // yeah, it's compl corev1.SecretTypeTLS, map[string][]byte{ "ca.crt": externallyProvidedCA.Bundle(), - corev1.TLSCertKey: externallyProvidedTLSServingCertPEM, - corev1.TLSPrivateKeyKey: externallyProvidedTLSServingKeyPEM, + corev1.TLSCertKey: externallyProvidedTLSServingCertPEM.CertPEM, + corev1.TLSPrivateKeyKey: externallyProvidedTLSServingCertPEM.KeyPEM, }) _, originalInternallyGeneratedCAPEM := performImpersonatorDiscoveryURL(ctx, t, env, adminConciergeClient)