Bug 1810036: Ensure service CA certs are created with unique serial numbers

go.mod

@@ -7,19 +7,19 @@ require (
 	github.com/golang/groupcache v0.0.0-20190129154638-5b532d6fd5ef // indirect
 	github.com/google/uuid v1.1.1
 	github.com/jteeuwen/go-bindata v3.0.8-0.20151023091102-a0ff2567cfb7+incompatible
-	github.com/openshift/api v0.0.0-20200121180347-3b42f2648b08
+	github.com/openshift/api v0.0.0-20200302180901-b4f75e525601
 	github.com/openshift/build-machinery-go v0.0.0-20200211121458-5e3d6e570160
-	github.com/openshift/client-go v0.0.0-20191216194936-57f413491e9e
-	github.com/openshift/library-go v0.0.0-20200113183004-f2ca9aafdf5a
+	github.com/openshift/client-go v0.0.0-20200116152001-92a2713fa240
+	github.com/openshift/library-go v0.0.0-20200306195801-d9c73bbbdd51
 	github.com/prometheus/client_golang v1.1.0
 	github.com/prometheus/common v0.6.0
 	github.com/spf13/cobra v0.0.5
 	github.com/spf13/pflag v1.0.5
 	github.com/tmc/grpc-websocket-proxy v0.0.0-20190109142713-0ad062ec5ee5 // indirect
 	go.uber.org/atomic v1.4.0 // indirect
-	k8s.io/api v0.17.1
-	k8s.io/apiextensions-apiserver v0.17.0
-	k8s.io/apimachinery v0.17.1
+	k8s.io/api v0.17.2
+	k8s.io/apiextensions-apiserver v0.17.1
+	k8s.io/apimachinery v0.17.2
 	k8s.io/client-go v0.17.1
 	k8s.io/component-base v0.17.1
 	k8s.io/klog v1.0.0

pkg/operator/rotate.go

@@ -4,6 +4,7 @@ import (
 	"crypto/rsa"
 	"crypto/x509"
 	"fmt"
+	"math/big"
 	"time"
 
 	corev1 "k8s.io/api/core/v1"
@@ -192,6 +193,19 @@ func createIntermediateCACert(targetCACert, signingCACert *x509.Certificate, sig
 	// Enable key identity chaining
 	template.AuthorityKeyId = signingCACert.SubjectKeyId
 
+	// Set a new serial number so that the intermediate CA cert is
+	// differentiated from the target CA cert. This ensures that a serving
+	// cert bundle that includes the issuing CA cert and an intermediate CA
+	// cert generated by this function - with the issuing CA cert as the
+	// target and the previous CA as the signer - will not result in
+	// SEC_ERROR_REUSED_ISSUER_AND_SERIAL when read by applications like curl.
+	serialGenerator := crypto.RandomSerialGenerator{}
+	serial, err := serialGenerator.Next(template)
+	if err != nil {
+		return nil, fmt.Errorf("failed to find next serial number: %v", err)
+	}
+	template.SerialNumber = big.NewInt(serial)
+
 	// Update the expiry if necessary
 	if expiry != nil {
 		template.NotAfter = *expiry

pkg/operator/rotate_test.go

@@ -247,3 +247,35 @@ func TestRotateSigningCA(t *testing.T) {
 	dnsName := oldServingCert.Certs[0].Subject.CommonName
 	util.CheckRotation(t, dnsName, oldCertPEM, oldKeyPEM, oldBundlePEM, newCertPEM, newKeyPEM, newBundlePEM)
 }
+
+// TestCreateIntermediateCACert checks that the intermediate CA cert
+// created by signing a target CA cert supports identity key chaining
+// and uses a serial number distinct from that of the target CA cert.
+func TestCreateIntermediateCACert(t *testing.T) {
+	// Create the signing CA
+	signingCAConfig, err := crypto.MakeSelfSignedCAConfig("foo", SigningCertificateLifetimeInDays)
+	if err != nil {
+		t.Fatalf("error generating a new ca: %v", err)
+	}
+	signingCACert := signingCAConfig.Certs[0]
+
+	// Create the CA targeted for signing
+	targetCAConfig, err := crypto.MakeSelfSignedCAConfig("foo", SigningCertificateLifetimeInDays)
+	if err != nil {
+		t.Fatalf("error generating a new ca: %v", err)
+	}
+	targetCACert := targetCAConfig.Certs[0]
+
+	intermediateCACert, err := createIntermediateCACert(targetCACert, signingCACert, signingCAConfig.Key.(*rsa.PrivateKey), nil)
+	if err != nil {
+		t.Fatalf("Failed to create intermediate CA cert: %v", err)
+	}
+
+	if bytes.Compare(intermediateCACert.AuthorityKeyId, signingCACert.SubjectKeyId) != 0 {
+		t.Fatalf("Expected intermediate CA cert AuthorityKeyId to match signing CA cert SubjectKeyId")
+	}
+
+	if intermediateCACert.SerialNumber.Cmp(targetCACert.SerialNumber) == 0 {
+		t.Fatalf("Expected intermediate CA cert serial number to differ from serial number of target CA cert")
+	}
+}