[FAB-18171] Disregard certificate validity period in intra-orderer co…

…mmunication (#1825)

This change set makes the orderer cluster authentication infrastructure
disregard validity periods when comparing certificates, and only regard public keys.

With this change, one can replace the TLS certificate of Raft,
by a certificate that has the same public key without issuing channel config updates.

This is to ensure that if a certificate of a consenter expired,
one can quickly renew it and start the orderer without performing
a config update per channel.

Change-Id: Id1b29e220d37aa33617b2143b702075bf5b01f6f
Signed-off-by: yacovm <yacovm@il.ibm.com>

common/crypto/expiration.go

@@ -7,8 +7,10 @@ SPDX-License-Identifier: Apache-2.0
 package crypto
 
 import (
+	"bytes"
 	"crypto/x509"
 	"encoding/pem"
+	"errors"
 	"time"
 
 	"github.com/golang/protobuf/proto"
@@ -98,3 +100,46 @@ func trackCertExpiration(rawCert []byte, certRole string, info MessageFunc, warn
 	})
 
 }
+
+var (
+	// ErrPubKeyMismatch is used by CertificatesWithSamePublicKey to indicate the two public keys mismatch
+	ErrPubKeyMismatch = errors.New("public keys do not match")
+)
+
+// LogNonPubKeyMismatchErr logs an error which is not an ErrPubKeyMismatch error
+func LogNonPubKeyMismatchErr(log func(template string, args ...interface{}), err error, cert1DER, cert2DER []byte) {
+	cert1PEM := &pem.Block{Type: "CERTIFICATE", Bytes: cert1DER}
+	cert2PEM := &pem.Block{Type: "CERTIFICATE", Bytes: cert2DER}
+	log("Failed determining if public key of %s matches public key of %s: %s",
+		string(pem.EncodeToMemory(cert1PEM)),
+		string(pem.EncodeToMemory(cert2PEM)),
+		err)
+}
+
+// CertificatesWithSamePublicKey returns nil if both byte slices
+// are valid DER encoding of certificates with the same public key.
+func CertificatesWithSamePublicKey(der1, der2 []byte) error {
+	cert1canonized, err := publicKeyFromCertificate(der1)
+	if err != nil {
+		return err
+	}
+
+	cert2canonized, err := publicKeyFromCertificate(der2)
+	if err != nil {
+		return err
+	}
+
+	if bytes.Equal(cert1canonized, cert2canonized) {
+		return nil
+	}
+	return ErrPubKeyMismatch
+}
+
+// publicKeyFromCertificate returns the public key of the given ASN1 DER certificate.
+func publicKeyFromCertificate(der []byte) ([]byte, error) {
+	cert, err := x509.ParseCertificate(der)
+	if err != nil {
+		return nil, err
+	}
+	return x509.MarshalPKIXPublicKey(cert.PublicKey)
+}

common/crypto/expiration_test.go

@@ -7,8 +7,10 @@ SPDX-License-Identifier: Apache-2.0
 package crypto_test
 
 import (
+	"bytes"
 	"crypto/x509"
 	"encoding/pem"
+	"errors"
 	"fmt"
 	"io/ioutil"
 	"path/filepath"
@@ -213,3 +215,118 @@ func TestTrackExpiration(t *testing.T) {
 		})
 	}
 }
+
+func TestLogNonPubKeyMismatchErr(t *testing.T) {
+	ca, err := tlsgen.NewCA()
+	require.NoError(t, err)
+
+	aliceKeyPair, err := ca.NewClientCertKeyPair()
+	require.NoError(t, err)
+
+	bobKeyPair, err := ca.NewClientCertKeyPair()
+	require.NoError(t, err)
+
+	expected := &bytes.Buffer{}
+	expected.WriteString(fmt.Sprintf("Failed determining if public key of %s matches public key of %s: foo",
+		string(aliceKeyPair.Cert),
+		string(bobKeyPair.Cert)))
+
+	b := &bytes.Buffer{}
+	f := func(template string, args ...interface{}) {
+		fmt.Fprintf(b, template, args...)
+	}
+
+	crypto.LogNonPubKeyMismatchErr(f, errors.New("foo"), aliceKeyPair.TLSCert.Raw, bobKeyPair.TLSCert.Raw)
+
+	require.Equal(t, expected.String(), b.String())
+}
+
+func TestCertificatesWithSamePublicKey(t *testing.T) {
+	ca, err := tlsgen.NewCA()
+	require.NoError(t, err)
+
+	bobKeyPair, err := ca.NewClientCertKeyPair()
+	require.NoError(t, err)
+
+	bobCert := bobKeyPair.Cert
+	bob := pem2der(bobCert)
+
+	aliceCert := `-----BEGIN CERTIFICATE-----
+MIIBNjCB3KADAgECAgELMAoGCCqGSM49BAMCMBAxDjAMBgNVBAUTBUFsaWNlMB4X
+DTIwMDgxODIxMzU1NFoXDTIwMDgyMDIxMzU1NFowEDEOMAwGA1UEBRMFQWxpY2Uw
+WTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAAQjZP5VD/RaczoPFbA4gkt1qb54R6SP
+J/V5oxkhDboG9xWi0wpyghaMGwwxC7Q9wegEnyOVp9nXoLrQ8LUJ5BfZoycwJTAO
+BgNVHQ8BAf8EBAMCBaAwEwYDVR0lBAwwCgYIKwYBBQUHAwIwCgYIKoZIzj0EAwID
+SQAwRgIhAK4le5XgH5edyhaQ9Sz7sFz3Zc4bbhPAzt9zQUYnoqK+AiEA5zcyLB/4
+Oqe93lroE6GF9W7UoCZFzD7lXsWku/dgFOU=
+-----END CERTIFICATE-----`
+
+	reIssuedAliceCert := `-----BEGIN CERTIFICATE-----
+MIIBNDCB3KADAgECAgELMAoGCCqGSM49BAMCMBAxDjAMBgNVBAUTBUFsaWNlMB4X
+DTIwMDgxODIxMzY1NFoXDTIwMDgyMDIxMzY1NFowEDEOMAwGA1UEBRMFQWxpY2Uw
+WTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAAQjZP5VD/RaczoPFbA4gkt1qb54R6SP
+J/V5oxkhDboG9xWi0wpyghaMGwwxC7Q9wegEnyOVp9nXoLrQ8LUJ5BfZoycwJTAO
+BgNVHQ8BAf8EBAMCBaAwEwYDVR0lBAwwCgYIKwYBBQUHAwIwCgYIKoZIzj0EAwID
+RwAwRAIgDc8WyXFvsxCk97KS7D/LdYJxMpDKdHNFqpzJT9LddlsCIEr8KcMd/t5p
+cRv6rqxvy5M+t0DhRtiwCen70YCUsksb
+-----END CERTIFICATE-----`
+
+	alice := pem2der([]byte(aliceCert))
+	aliceMakesComeback := pem2der([]byte(reIssuedAliceCert))
+
+	for _, test := range []struct {
+		description string
+		errContains string
+		first       []byte
+		second      []byte
+	}{
+		{
+			description: "Bad first certificate",
+			errContains: "asn1:",
+			first:       []byte{1, 2, 3},
+			second:      bob,
+		},
+
+		{
+			description: "Bad second certificate",
+			errContains: "asn1:",
+			first:       alice,
+			second:      []byte{1, 2, 3},
+		},
+
+		{
+			description: "Different certificate",
+			errContains: crypto.ErrPubKeyMismatch.Error(),
+			first:       alice,
+			second:      bob,
+		},
+
+		{
+			description: "Same certificate",
+			first:       alice,
+			second:      alice,
+		},
+
+		{
+			description: "Same certificate but different validity period",
+			first:       alice,
+			second:      aliceMakesComeback,
+		},
+	} {
+		t.Run(test.description, func(t *testing.T) {
+			err := crypto.CertificatesWithSamePublicKey(test.first, test.second)
+			if test.errContains != "" {
+				require.Error(t, err)
+				require.Contains(t, err.Error(), test.errContains)
+				return
+			}
+
+			require.NoError(t, err)
+		})
+	}
+}
+
+func pem2der(p []byte) []byte {
+	b, _ := pem.Decode(p)
+	return b.Bytes
+}

integration/e2e/cft_test.go

@@ -398,7 +398,7 @@ var _ = Describe("EndToEnd Crash Fault Tolerance", func() {
 
 			By("Expiring orderer TLS certificates")
 			for filePath, certPEM := range serverTLSCerts {
-				expiredCert, earlyMadeCACert := expireCertificate(certPEM, ordererTLSCACert, ordererTLSCAKey)
+				expiredCert, earlyMadeCACert := expireCertificate(certPEM, ordererTLSCACert, ordererTLSCAKey, time.Now())
 				err = ioutil.WriteFile(filePath, expiredCert, 600)
 				Expect(err).NotTo(HaveOccurred())
 
@@ -530,6 +530,82 @@ var _ = Describe("EndToEnd Crash Fault Tolerance", func() {
 			By("Waiting for a leader to be elected")
 			findLeader([]*ginkgomon.Runner{o1Runner, o2Runner, o3Runner})
 		})
+
+		It("disregards certificate renewal if only the validity period changed", func() {
+			config := nwo.MultiNodeEtcdRaft()
+			config.Channels = append(config.Channels, &nwo.Channel{Name: "foo", Profile: "TwoOrgsChannel"})
+			config.Channels = append(config.Channels, &nwo.Channel{Name: "bar", Profile: "TwoOrgsChannel"})
+			network = nwo.New(config, testDir, client, 33000, components)
+
+			network.GenerateConfigTree()
+			network.Bootstrap()
+
+			peer = network.Peer("Org1", "peer0")
+
+			o1 := network.Orderer("orderer1")
+			o2 := network.Orderer("orderer2")
+			o3 := network.Orderer("orderer3")
+
+			orderers := []*nwo.Orderer{o1, o2, o3}
+
+			o1Runner := network.OrdererRunner(o1)
+			o2Runner := network.OrdererRunner(o2)
+			o3Runner := network.OrdererRunner(o3)
+			ordererRunners := []*ginkgomon.Runner{o1Runner, o2Runner, o3Runner}
+
+			o1Proc = ifrit.Invoke(o1Runner)
+			o2Proc = ifrit.Invoke(o2Runner)
+			o3Proc = ifrit.Invoke(o3Runner)
+			ordererProcesses := []ifrit.Process{o1Proc, o2Proc, o3Proc}
+
+			Eventually(o1Proc.Ready(), network.EventuallyTimeout).Should(BeClosed())
+			Eventually(o2Proc.Ready(), network.EventuallyTimeout).Should(BeClosed())
+			Eventually(o3Proc.Ready(), network.EventuallyTimeout).Should(BeClosed())
+
+			By("Waiting for them to elect a leader")
+			findLeader(ordererRunners)
+
+			By("Creating a channel")
+			network.CreateChannel("foo", o1, peer)
+
+			assertBlockReception(map[string]int{
+				"foo":           0,
+				"systemchannel": 1,
+			}, []*nwo.Orderer{o1, o2, o3}, peer, network)
+
+			By("Killing all orderers")
+			for i := range orderers {
+				ordererProcesses[i].Signal(syscall.SIGTERM)
+				Eventually(ordererProcesses[i].Wait(), network.EventuallyTimeout).Should(Receive())
+			}
+
+			By("Renewing the certificates for all orderers")
+			renewOrdererCertificates(network, o1, o2, o3)
+
+			By("Starting the orderers again")
+			for i := range orderers {
+				ordererRunner := network.OrdererRunner(orderers[i])
+				ordererRunners[i] = ordererRunner
+				ordererProcesses[i] = ifrit.Invoke(ordererRunner)
+				Eventually(ordererProcesses[0].Ready(), network.EventuallyTimeout).Should(BeClosed())
+			}
+
+			o1Proc = ordererProcesses[0]
+			o2Proc = ordererProcesses[1]
+			o3Proc = ordererProcesses[2]
+
+			By("Waiting for them to elect a leader once again")
+			findLeader(ordererRunners)
+
+			By("Creating a channel again")
+			network.CreateChannel("bar", o1, peer)
+
+			assertBlockReception(map[string]int{
+				"foo":           0,
+				"bar":           0,
+				"systemchannel": 2,
+			}, []*nwo.Orderer{o1, o2, o3}, peer, network)
+		})
 	})
 
 	When("admin certificate expires", func() {
@@ -560,7 +636,7 @@ var _ = Describe("EndToEnd Crash Fault Tolerance", func() {
 			originalAdminCert, err := ioutil.ReadFile(adminCertPath)
 			Expect(err).NotTo(HaveOccurred())
 
-			expiredAdminCert, earlyCACert := expireCertificate(originalAdminCert, ordererCACert, ordererCAKey)
+			expiredAdminCert, earlyCACert := expireCertificate(originalAdminCert, ordererCACert, ordererCAKey, time.Now())
 			err = ioutil.WriteFile(adminCertPath, expiredAdminCert, 600)
 			Expect(err).NotTo(HaveOccurred())
 
@@ -741,7 +817,34 @@ func findLeader(ordererRunners []*ginkgomon.Runner) int {
 	return firstLeader
 }
 
-func expireCertificate(certPEM, caCertPEM, caKeyPEM []byte) (expiredcertPEM []byte, earlyMadeCACertPEM []byte) {
+func renewOrdererCertificates(network *nwo.Network, o1, o2, o3 *nwo.Orderer) {
+	ordererDomain := network.Organization(o1.Organization).Domain
+	ordererTLSCACertPath := filepath.Join(network.RootDir, "crypto", "ordererOrganizations",
+		ordererDomain, "tlsca", fmt.Sprintf("tlsca.%s-cert.pem", ordererDomain))
+	ordererTLSCACert, err := ioutil.ReadFile(ordererTLSCACertPath)
+	Expect(err).NotTo(HaveOccurred())
+
+	ordererTLSCAKeyPath := filepath.Join(network.RootDir, "crypto", "ordererOrganizations",
+		ordererDomain, "tlsca", privateKeyFileName(ordererTLSCACert))
+
+	ordererTLSCAKey, err := ioutil.ReadFile(ordererTLSCAKeyPath)
+	Expect(err).NotTo(HaveOccurred())
+
+	serverTLSCerts := make(map[string][]byte)
+	for _, orderer := range []*nwo.Orderer{o1, o2, o3} {
+		tlsCertPath := filepath.Join(network.OrdererLocalTLSDir(orderer), "server.crt")
+		serverTLSCerts[tlsCertPath], err = ioutil.ReadFile(tlsCertPath)
+		Expect(err).NotTo(HaveOccurred())
+	}
+
+	for filePath, certPEM := range serverTLSCerts {
+		renewedCert, _ := expireCertificate(certPEM, ordererTLSCACert, ordererTLSCAKey, time.Now().Add(time.Hour))
+		err = ioutil.WriteFile(filePath, renewedCert, 0600)
+		Expect(err).NotTo(HaveOccurred())
+	}
+}
+
+func expireCertificate(certPEM, caCertPEM, caKeyPEM []byte, expirationTime time.Time) (expiredcertPEM []byte, earlyMadeCACertPEM []byte) {
 	keyAsDER, _ := pem.Decode(caKeyPEM)
 	caKeyWithoutType, err := x509.ParsePKCS8PrivateKey(keyAsDER.Bytes)
 	Expect(err).NotTo(HaveOccurred())
@@ -762,7 +865,7 @@ func expireCertificate(certPEM, caCertPEM, caKeyPEM []byte) (expiredcertPEM []by
 	// As well as the CA certificate
 	caCert.NotBefore = time.Now().Add((-1) * time.Hour)
 	// The certificate expires now
-	cert.NotAfter = time.Now()
+	cert.NotAfter = expirationTime
 
 	// The CA signs the certificate
 	certBytes, err := x509.CreateCertificate(rand.Reader, cert, caCert, cert.PublicKey, caKey)

orderer/common/cluster/comm.go

@@ -104,6 +104,7 @@ type Comm struct {
 	Connections                      *ConnectionStore
 	Chan2Members                     MembersByChannel
 	Metrics                          *Metrics
+	CompareCertificate               CertificateComparator
 }
 
 type requestContext struct {
@@ -231,9 +232,9 @@ func (c *Comm) Shutdown() {
 
 	c.shutdown = true
 	for _, members := range c.Chan2Members {
-		for _, member := range members {
-			c.Connections.Disconnect(member.ServerTLSCert)
-		}
+		members.Foreach(func(id uint64, stub *Stub) {
+			c.Connections.Disconnect(stub.ServerTLSCert)
+		})
 	}
 }
 
@@ -272,15 +273,15 @@ func (c *Comm) applyMembershipConfig(channel string, newNodes []RemoteNode) {
 
 	// Remove all stubs without a corresponding node
 	// in the new nodes
-	for id, stub := range mapping {
+	mapping.Foreach(func(id uint64, stub *Stub) {
 		if _, exists := newNodeIDs[id]; exists {
 			c.Logger.Info(id, "exists in both old and new membership for channel", channel, ", skipping its deactivation")
-			continue
+			return
 		}
 		c.Logger.Info("Deactivated node", id, "who's endpoint is", stub.Endpoint, "as it's removed from membership")
-		delete(mapping, id)
+		mapping.Remove(id)
 		stub.Deactivate()
-	}
+	})
 }
 
 // updateStubInMapping updates the given RemoteNode and adds it to the MemberMapping
@@ -374,7 +375,10 @@ func (c *Comm) getOrCreateMapping(channel string) MemberMapping {
 	// Lazily create a mapping if it doesn't already exist
 	mapping, exists := c.Chan2Members[channel]
 	if !exists {
-		mapping = make(MemberMapping)
+		mapping = MemberMapping{
+			id2stub:       make(map[uint64]*Stub),
+			SamePublicKey: c.CompareCertificate,
+		}
 		c.Chan2Members[channel] = mapping
 	}
 	return mapping