dynamiccertauthority: fix cert expiration test failure

Signed-off-by: Andrew Keesler <akeesler@vmware.com>

internal/certauthority/dynamiccertauthority/dynamiccertauthority.go

@@ -27,7 +27,7 @@ func New(provider dynamiccert.Provider) *CA {
 }
 
 // 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.
+// 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) {
 	caCrtPEM, caKeyPEM := c.provider.CurrentCertKeyContent()
 	ca, err := certauthority.Load(string(caCrtPEM), string(caKeyPEM))

internal/certauthority/dynamiccertauthority/dynamiccertauthority_test.go

@@ -5,7 +5,6 @@ package dynamiccertauthority
 
 import (
 	"crypto/x509/pkix"
-	"io/ioutil"
 	"testing"
 	"time"
 
@@ -21,10 +20,22 @@ func TestCAIssuePEM(t *testing.T) {
 	provider := dynamiccert.New()
 	ca := New(provider)
 
+	goodCACrtPEM0, goodCAKeyPEM0, err := testutil.CreateCertificate(
+		time.Now().Add(-time.Hour),
+		time.Now().Add(time.Hour),
+	)
+	require.NoError(t, err)
+
+	goodCACrtPEM1, goodCAKeyPEM1, err := testutil.CreateCertificate(
+		time.Now().Add(-time.Hour),
+		time.Now().Add(time.Hour),
+	)
+	require.NoError(t, err)
+
 	steps := []struct {
-		name                 string
-		caCrtPath, caKeyPath string
-		wantError            string
+		name               string
+		caCrtPEM, caKeyPEM []byte
+		wantError          string
 	}{
 		{
 			name:      "no cert+key",
@@ -32,70 +43,59 @@ func TestCAIssuePEM(t *testing.T) {
 		},
 		{
 			name:      "only cert",
-			caCrtPath: "testdata/ca-0.crt",
+			caCrtPEM:  goodCACrtPEM0,
 			wantError: "could not load CA: tls: failed to find any PEM data in key input",
 		},
 		{
 			name:      "only key",
-			caKeyPath: "testdata/ca-0.key",
+			caKeyPEM:  goodCAKeyPEM0,
 			wantError: "could not load CA: tls: failed to find any PEM data in certificate input",
 		},
 		{
-			name:      "new cert+key",
-			caCrtPath: "testdata/ca-0.crt",
-			caKeyPath: "testdata/ca-0.key",
+			name:     "new cert+key",
+			caCrtPEM: goodCACrtPEM0,
+			caKeyPEM: goodCAKeyPEM0,
 		},
 		{
 			name: "same cert+key",
 		},
 		{
-			name:      "another new cert+key",
-			caCrtPath: "testdata/ca-1.crt",
-			caKeyPath: "testdata/ca-1.key",
+			name:     "another new cert+key",
+			caCrtPEM: goodCACrtPEM1,
+			caKeyPEM: goodCAKeyPEM1,
 		},
 		{
 			name:      "bad cert",
-			caCrtPath: "testdata/ca-bad.crt",
-			caKeyPath: "testdata/ca-0.key",
+			caCrtPEM:  []byte("this is not a cert"),
+			caKeyPEM:  goodCAKeyPEM0,
 			wantError: "could not load CA: tls: failed to find any PEM data in certificate input",
 		},
 		{
 			name:      "bad key",
-			caCrtPath: "testdata/ca-0.crt",
-			caKeyPath: "testdata/ca-bad.key",
+			caCrtPEM:  goodCACrtPEM0,
+			caKeyPEM:  []byte("this is not a key"),
 			wantError: "could not load CA: tls: failed to find any PEM data in key input",
 		},
 		{
 			name:      "mismatch cert+key",
-			caCrtPath: "testdata/ca-0.crt",
-			caKeyPath: "testdata/ca-1.key",
+			caCrtPEM:  goodCACrtPEM0,
+			caKeyPEM:  goodCAKeyPEM1,
 			wantError: "could not load CA: tls: private key does not match public key",
 		},
 		{
-			name:      "good cert+key again",
-			caCrtPath: "testdata/ca-1.crt",
-			caKeyPath: "testdata/ca-1.key",
+			name:     "good cert+key again",
+			caCrtPEM: goodCACrtPEM0,
+			caKeyPEM: goodCAKeyPEM0,
 		},
 	}
 	for _, step := range steps {
 		step := step
 		t.Run(step.name, func(t *testing.T) {
-			var caCrtPEM, caKeyPEM []byte
-			var err error
-			if step.caCrtPath != "" {
-				caCrtPEM, err = ioutil.ReadFile(step.caCrtPath)
-				require.NoError(t, err)
-			}
+			// Can't run these steps in parallel, because each one depends on the previous steps being
+			// run.
 
-			if step.caKeyPath != "" {
-				caKeyPEM, err = ioutil.ReadFile(step.caKeyPath)
-				require.NoError(t, err)
-			}
-
-			if step.caCrtPath != "" || step.caKeyPath != "" {
-				provider.Set(caCrtPEM, caKeyPEM)
-			} else {
-				caCrtPEM, _ = provider.CurrentCertKeyContent()
+			if step.caCrtPEM != nil || step.caKeyPEM != nil {
+				provider.Set(step.caCrtPEM, step.caKeyPEM)
 			}
 
 			crtPEM, keyPEM, err := ca.IssuePEM(
@@ -115,6 +115,7 @@ func TestCAIssuePEM(t *testing.T) {
 				require.NotEmpty(t, crtPEM)
 				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")

internal/certauthority/dynamiccertauthority/testdata/ca-0.crt

@@ -1,17 +0,0 @@
------BEGIN CERTIFICATE-----
-MIICxjCCAa4CCQCawPQa0TyOmDANBgkqhkiG9w0BAQsFADAlMQ0wCwYDVQQDDAR0
-dW5hMRQwEgYDVQQKDAtmaXNoLG1hcmxpbjAeFw0yMDA5MjMxMzMwMzZaFw0yMDEw
-MjMxMzMwMzZaMCUxDTALBgNVBAMMBHR1bmExFDASBgNVBAoMC2Zpc2gsbWFybGlu
-MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAxwLD08r2ZWh9FMGqq82O
-CwraRKafmdgFk0qTpWS48di2A/Jz5R0zgx0fHYTs+pCL5FhfpDhTCYGTWMoXI2bW
-7n6prDwrfNE+bsKdekEHM8MugdSFm2cPJONS7upIGODDIvK3zVGCg4Y0H/nE5Zf2
-g+GrrY31a4tWYkS9b2J7ZxQlQ2ixsxICHvDN1JYwg6a90txOKJAJBK9lxtcY1//p
-Dx0v/norGt92chMRq/QZYDELs2h2KoQeXmPYR7/MZYCavjV3Dcc55ueUksTQjusI
-INYiYh/Jm3IKBSUjl0TjQaJz/q4+JOPb1D7aQIggOTuCTPXb858DHW0lgo64H3Zf
-nQIDAQABMA0GCSqGSIb3DQEBCwUAA4IBAQB7qwje3H5Aw3OFssPuc7cIUrW7ohU9
-mcKAq5T5KWRsX3buCkLRJO9BOhBpRIM2jcHSBZupC/JiwFwLMCA5/l0GE++9WLFE
-9ta2eTjTV5Og7wwpWlljD0uqZ/IROEV206Nvs0k3L3XhM1eoz3MESh7Esmr3ZSwK
-pNATzBhvhoYNRMYBM4Yy3poBzweLFlLnNW98jI4Gp+A/fA38cqVdmNXFsfNSpesu
-nesMwBleDec2o8adOs3LtXhfufDLVrmpRJgYqRhgQe1X+PicdCeGRAEN+fH4PF2i
-qza2f08YEWiobqdVsN9oIWupyobPrfEnuLjj1xYNSf4LTKvVUqeK7F8H
------END CERTIFICATE-----

internal/certauthority/dynamiccertauthority/testdata/ca-0.key

@@ -1,28 +0,0 @@
------BEGIN PRIVATE KEY-----
-MIIEvgIBADANBgkqhkiG9w0BAQEFAASCBKgwggSkAgEAAoIBAQDHAsPTyvZlaH0U
-waqrzY4LCtpEpp+Z2AWTSpOlZLjx2LYD8nPlHTODHR8dhOz6kIvkWF+kOFMJgZNY
-yhcjZtbufqmsPCt80T5uwp16QQczwy6B1IWbZw8k41Lu6kgY4MMi8rfNUYKDhjQf
-+cTll/aD4autjfVri1ZiRL1vYntnFCVDaLGzEgIe8M3UljCDpr3S3E4okAkEr2XG
-1xjX/+kPHS/+eisa33ZyExGr9BlgMQuzaHYqhB5eY9hHv8xlgJq+NXcNxznm55SS
-xNCO6wgg1iJiH8mbcgoFJSOXRONBonP+rj4k49vUPtpAiCA5O4JM9dvznwMdbSWC
-jrgfdl+dAgMBAAECggEATe9OQyXLomDuAu/o54kDJF3mplXeUMAhRtJAydeM+eEV
-Pqx3KHVsE9+WrNe+ek2bCxx0r/oCwslEq9PQheOjLhjaV4HkweernHKIezT2HhZE
-o0FX5UjRzG9drgR/oYZ7xLbqtfRCuUEYLqGAsyz0tCbvNXNPdgNGHAuxZ+pYS0AN
-6DPOJKG1gUNSAzjU7U6g1UtOxr/qXVSZvBM+F7nTsbRKlnq/RYvLvfGLfnkQZvom
-QxcyZmiC6fgmrpiCPN9poUKmB6jt3IWQJ9qyxadWI612fYrHWMEcXgNcxxOvCxyI
-S1cGAq9Vw51pvGLlXsGzci3i4qemEDl8TDIp46C0oQKBgQD3ImywgoJn+IvTwtml
-vFNfJ3s/J61SFv32u4iR9lim118mzvOi7FphjCfCj72cppfiuzI/F3Bsv0eR2qv9
-9eXahvQe8egZvWPnW+f2GRYOAbHq/SmYA5kH2hHXOPHdJvfYj+fnEU+y6XXbFtp6
-MLvmbgrPL6le1UHri3XPpcOjNQKBgQDOJmS+yK+ozZTpkNXYq2GNaVr5LaUhOePV
-y+hvNj1IlhU7R97PcBe59izO73sYIUeOCmGM2cX3zGy7+upSyT2K3+Jh5BVpnwME
-OqyrYFXfMJxwf9SuD9GJRiOtxtglBxcjNZEcninV1qynuhC9SaHjyhPnzcyZR/bB
-Ipy4+mKvyQKBgQDktqs4P9BIQLHHbRDYXY4bBgL9086HplerPKuLyh0Ja1DYLbc1
-FOAgXwF9AmJM07DFWNGLqjmVqwClb2a1RhlPQI41BVP0Xl5TA6+NBnJuzArImzsf
-QFUj+yF/uWe9cA74EVZhdpf30DAObvwLEYcUHstKK6Xn6h1zzEFfxt1j8QKBgHjr
-2Gh4e2E+xbyDcoyXaq9yPySue5ATzurXos2pOSVcs7OEItP24lP4bKwtmTy8OKa+
-vB3Ml+0Ugit5sP1CgdD2JmpZSZ4c4b4XVLVp2rUCZKEwumYnbTdiZwdF/f8qO44x
-m573v35pX/k6kRsXF9jv7eEovHyk077SOK/gXwbRAoGBAO5HfelAfPG4CFZdvj5z
-sCCf8rr1Jpyk+1hILrI8hQUriL3bwStRppQHJajfswjZKP9pjyJXi/cWp7DvKBpo
-DFGikRWzbSP/xkRNRW35bvGlcy8oit395dQJZOemrf9KoHQPLGT2pEnLDtqOtJAQ
-YsquGd2q0TmDBVolI62cHjr2
------END PRIVATE KEY-----

internal/certauthority/dynamiccertauthority/testdata/ca-1.crt

@@ -1,17 +0,0 @@
------BEGIN CERTIFICATE-----
-MIICxjCCAa4CCQC6B88ml9yfhzANBgkqhkiG9w0BAQsFADAlMQ0wCwYDVQQDDAR0
-dW5hMRQwEgYDVQQKDAtmaXNoLG1hcmxpbjAeFw0yMDA5MjMxMzMwNDRaFw0yMDEw
-MjMxMzMwNDRaMCUxDTALBgNVBAMMBHR1bmExFDASBgNVBAoMC2Zpc2gsbWFybGlu
-MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA4rabilzx1UHfD52mRquC
-S1t7YyeTrPWgS2VgUgD2NcghL0TL85P17KUh+OcEJ5N5QQMXgs8WMotnP786pg/Z
-yX9U+KGAMIRla3k+YgX5qd45nN4ZMsCspK2gnfQbqbaPKA6oXRxXbqgrVy+kTx0/
-QylSHrwOEZacObPhQ5wM+RGlkSCUJ4hlRr6POOLOSwqlcw7Uxw0I482FTK11jAaY
-0EgItpW0ROsv7j6RhomGGGSuPa58qe9zwzz9nR8urnCJ4m+5Kw/hae3FxTSrOMmc
-OiOXwyzEjihTax7Afvpe9rfjjXLZpz7EXyCmeA0BlFwjvuDx2oZ+/WitC/zUMKvM
-XQIDAQABMA0GCSqGSIb3DQEBCwUAA4IBAQCA0DJsNEJC6sZletC+c6qHRKRjJDMY
-0nsXaV6rFPASk+956rI+1DtBuc54UmhgO+VhoVda6lYeBUiXC7lMSU5T9MtFOgLA
-OhWVSwL9XveLOEc2JrBaI8NKG2V9E0b5d6TJeIvxotv2En0gJIvachQy4JwM3fKr
-mITVqeqvhWczLoFNIXmwYpIwqzhF6t2QmV7ecwXi8g0hoX9d9JVDs6qtvudlplIz
-TjefZ6tNpNpXKBQCSI44ZydyNelz5q/A9j7iyG9tmU8jrCouii2DkUoFblrX/x+Q
-bZ7was9gqUshQ3IHBxfynuNkKvT0VMSfpBFcTipW7QVFatCxZHb8idFD
------END CERTIFICATE-----

internal/certauthority/dynamiccertauthority/testdata/ca-1.key

@@ -1,28 +0,0 @@
------BEGIN PRIVATE KEY-----
-MIIEvgIBADANBgkqhkiG9w0BAQEFAASCBKgwggSkAgEAAoIBAQDitpuKXPHVQd8P
-naZGq4JLW3tjJ5Os9aBLZWBSAPY1yCEvRMvzk/XspSH45wQnk3lBAxeCzxYyi2c/
-vzqmD9nJf1T4oYAwhGVreT5iBfmp3jmc3hkywKykraCd9Bupto8oDqhdHFduqCtX
-L6RPHT9DKVIevA4Rlpw5s+FDnAz5EaWRIJQniGVGvo844s5LCqVzDtTHDQjjzYVM
-rXWMBpjQSAi2lbRE6y/uPpGGiYYYZK49rnyp73PDPP2dHy6ucInib7krD+Fp7cXF
-NKs4yZw6I5fDLMSOKFNrHsB++l72t+ONctmnPsRfIKZ4DQGUXCO+4PHahn79aK0L
-/NQwq8xdAgMBAAECggEADnosuowu4ThgnyWpDZA8NMW1vmIzmvLgdpAVs4beMhcC
-j3ssLvS+2oq8/aD68fAH6S/iW3cP6tBeGoMCosIWXTilW28suWxq8Y7/fdD13XQU
-Z8EDCOum2qk/vXZuIctHnv44oCGEL8vLYLjpHCg49vf3h4DowRTGCyVCeEfJyEnK
-ZUZzKtmohS88IhOfOoNC8qvXGhdJmbNmFngJqW1Z+4qozxpAYzjr4HwqFJF/YeQh
-xKxjKSS5ikxdPqySdRxuUsIqFe72ZPIVo2NxVPkY+w75DZAqKDkemrDETM8TirMb
-3n1lEabpiY1kWTF+UcthKhN4RV4RwYIRNFw/kpS7vQKBgQD77OPOSBsHqEw+Gykd
-kVhTXJxspYI8pNQft1HvnlCNAEUsdT5jSW9O2PGjkUwsKQvSXGuMmYtnHdRv84G7
-/or+yyZw2P3T1V01220Vj+fQh2YCrHqSEcmHlA7XSZblj4qs9rOrj5jRQbM8T78H
-r09bum1wxIjHdRebPDCUATwFNwKBgQDmYVNwtF+kCUQNYYSDolxJSH9jjCOfGkWR
-RHtimsM75jzzg8TX5XGJCJMO0rQ1k2A4YdVEwNCGUEOx/uX9Ey8LSyEaUGp8bXJ9
-1sAjjPY6xmEV0gyQTFWR7EQHc/F90i+QaYaAbOvq479jwoamcKkDoY9wJsrQsxX/
-AZAvX66FCwKBgHSYwyaqALiekAr+jxz8NCEA7/To9UoUD4lOU0HPyAA6a8mTyfgx
-+K5JgizuBo85wBrwY2aDeh2TlMrrrNPRj4s1gukfxPrR+X3/vJEKNYQx5mi+Y0xP
-pzJ0uBm0GX7N3KUI2UdCArx25/m1/vgTj2so8ZYLoDKQYwiZ5vHZUGopAoGBAOAZ
-MFdjvd0M/ludzo9Vcjv+/5vQCB6OKbnDSdqC/QLZVdTzCpmQoT5RTuuOXqn28BQo
-ZpJ4lN7yWMdeUk45SPvNWygDrXHX7RUnvsNWLXbC3lGhf4MmHd7SWuJ9EB36RTZO
-z/1A9sQIQnZCFUT2NhJIKAVKVuNoMS9bT+wlQNg9AoGBALBj/5zbc2swgh1rB0Ob
-7AQgPi9KHnsJcpZrY0jVwzQgQkYwtuaL4PL5eduNM/feaEOl54U3BCm5txa04H8N
-Anp6RGPM3wHKSa2k0e3PtvICp/mL767kbxCuKcb1NhSpgAXAV5GLZNLxsWN5yJnZ
-TVgvHlTpm6RMm7zDYHYyGFiO
------END PRIVATE KEY-----

internal/certauthority/dynamiccertauthority/testdata/ca-bad.crt

@@ -1 +0,0 @@
-this is not a certificate

internal/certauthority/dynamiccertauthority/testdata/ca-bad.key

@@ -1 +0,0 @@
-this is not a key

internal/controller/apicerts/certs_expirer_test.go

@@ -137,7 +137,7 @@ func TestExpirerControllerSync(t *testing.T) {
 			name:        "lifetime below threshold",
 			renewBefore: 7 * time.Hour,
 			fillSecretData: func(t *testing.T, m map[string][]byte) {
-				certPEM, err := testutil.CreateCertificate(
+				certPEM, _, err := testutil.CreateCertificate(
 					time.Now().Add(-5*time.Hour),
 					time.Now().Add(5*time.Hour),
 				)
@@ -152,7 +152,7 @@ func TestExpirerControllerSync(t *testing.T) {
 			name:        "lifetime above threshold",
 			renewBefore: 3 * time.Hour,
 			fillSecretData: func(t *testing.T, m map[string][]byte) {
-				certPEM, err := testutil.CreateCertificate(
+				certPEM, _, err := testutil.CreateCertificate(
 					time.Now().Add(-5*time.Hour),
 					time.Now().Add(5*time.Hour),
 				)
@@ -167,7 +167,7 @@ func TestExpirerControllerSync(t *testing.T) {
 			name:        "cert expired",
 			renewBefore: 3 * time.Hour,
 			fillSecretData: func(t *testing.T, m map[string][]byte) {
-				certPEM, err := testutil.CreateCertificate(
+				certPEM, _, err := testutil.CreateCertificate(
 					time.Now().Add(-2*time.Hour),
 					time.Now().Add(-1*time.Hour),
 				)
@@ -182,7 +182,7 @@ func TestExpirerControllerSync(t *testing.T) {
 			name:        "delete failure",
 			renewBefore: 3 * time.Hour,
 			fillSecretData: func(t *testing.T, m map[string][]byte) {
-				certPEM, err := testutil.CreateCertificate(
+				certPEM, _, err := testutil.CreateCertificate(
 					time.Now().Add(-5*time.Hour),
 					time.Now().Add(5*time.Hour),
 				)

internal/testutil/certs.go

@@ -81,15 +81,13 @@ func (v *ValidCert) RequireCommonName(commonName string) {
 	require.Equal(v.t, commonName, v.parsed.Subject.CommonName)
 }
 
-// CreateCertificate creates a certificate with the provided time bounds, and
-// returns the PEM representation of the certificate.
-//
-// There is nothing very special about the certificate that it creates, just
-// that it is a valid certificate that can be used for testing.
-func CreateCertificate(notBefore, notAfter time.Time) ([]byte, error) {
+// 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.
+func CreateCertificate(notBefore, notAfter time.Time) ([]byte, []byte, error) {
 	privateKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 
 	template := x509.Certificate{
@@ -97,8 +95,11 @@ func CreateCertificate(notBefore, notAfter time.Time) ([]byte, error) {
 		Subject: pkix.Name{
 			CommonName: "some-common-name",
 		},
-		NotBefore: notBefore,
-		NotAfter:  notAfter,
+		NotBefore:             notBefore,
+		NotAfter:              notAfter,
+		IsCA:                  true,
+		KeyUsage:              x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign,
+		BasicConstraintsValid: true,
 	}
 	cert, err := x509.CreateCertificate(
 		rand.Reader,
@@ -108,12 +109,20 @@ func CreateCertificate(notBefore, notAfter time.Time) ([]byte, error) {
 		privateKey,
 	)
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 
 	certPEM := pem.EncodeToMemory(&pem.Block{
 		Type:  "CERTIFICATE",
 		Bytes: cert,
 	})
-	return certPEM, nil
+	privateKeyDER, err := x509.MarshalPKCS8PrivateKey(privateKey)
+	if err != nil {
+		return nil, nil, err
+	}
+	privateKeyPEM := pem.EncodeToMemory(&pem.Block{
+		Type:  "PRIVATE KEY",
+		Bytes: privateKeyDER,
+	})
+	return certPEM, privateKeyPEM, nil
 }

test/integration/concierge_api_serving_certs_test.go

@@ -57,7 +57,7 @@ func TestAPIServingCertificateAutoCreationAndRotation(t *testing.T) {
 					return err
 				}
 
-				secret.Data["tlsCertificateChain"], err = createExpiredCertificate()
+				secret.Data["tlsCertificateChain"], _, err = createExpiredCertificate()
 				if err != nil {
 					return err
 				}
@@ -161,7 +161,7 @@ func TestAPIServingCertificateAutoCreationAndRotation(t *testing.T) {
 	}
 }
 
-func createExpiredCertificate() ([]byte, error) {
+func createExpiredCertificate() ([]byte, []byte, error) {
 	return testutil.CreateCertificate(
 		time.Now().Add(-24*time.Hour), // notBefore
 		time.Now().Add(-time.Hour),    // notAfter