FAB-18192 Fixed TLS certs validation for consenters. (hyperledger#1888)

* FAB-18192 Fixed TLS certs validation for consenters.
Verification of TLS cert against simulated config, not the last one. To achieve that, metadata validator interface was changed, now it requires orderer config instead of just consensus metadata. Also, TLS verification was moved to VerifyMetadata function, it shouldn't have been part of ComputeMembershipChanges. Fixed tests.

Signed-off-by: Vladyslav Kopaihorodskyi <vlad.kopaygorodsky@gmail.com>

integration/raft/config_test.go

@@ -8,6 +8,8 @@ package raft
 
 import (
 	"bytes"
+	"crypto/x509"
+	"encoding/pem"
 	"fmt"
 	"io/ioutil"
 	"os"
@@ -206,7 +208,7 @@ var _ = Describe("EndToEnd reconfiguration and onboarding", func() {
 			exitCode := network.CreateChannelExitCode(channel, orderer, org1Peer0, org1Peer0, org2Peer0, orderer)
 			Expect(exitCode).NotTo(Equal(0))
 			Consistently(process.Wait).ShouldNot(Receive()) // malformed tx should not crash orderer
-			Expect(runner.Err()).To(gbytes.Say(`invalid new config metdadata: ElectionTick \(10\) must be greater than HeartbeatTick \(10\)`))
+			Expect(runner.Err()).To(gbytes.Say(`invalid new config metadata: ElectionTick \(10\) must be greater than HeartbeatTick \(10\)`))
 
 			By("Submitting channel config update with illegal value")
 			channel = network.SystemChannel.Name
@@ -234,7 +236,7 @@ var _ = Describe("EndToEnd reconfiguration and onboarding", func() {
 
 			sess := nwo.UpdateOrdererConfigSession(network, orderer, channel, config, updatedConfig, org1Peer0, orderer)
 			Eventually(sess, network.EventuallyTimeout).Should(gexec.Exit(1))
-			Expect(sess.Err).To(gbytes.Say(`invalid new config metdadata: ElectionTick \(10\) must be greater than HeartbeatTick \(10\)`))
+			Expect(sess.Err).To(gbytes.Say(`invalid new config metadata: ElectionTick \(10\) must be greater than HeartbeatTick \(10\)`))
 		})
 	})
 
@@ -388,41 +390,17 @@ var _ = Describe("EndToEnd reconfiguration and onboarding", func() {
 			By("Waiting for orderer3 to see the leader")
 			findLeader([]*ginkgomon.Runner{ordererRunners[2]})
 
-			By("Removing orderer3's TLS root CA certificate")
-			nwo.UpdateOrdererMSP(network, peer, orderer, "systemchannel", "OrdererOrg", func(config msp.FabricMSPConfig) msp.FabricMSPConfig {
-				config.TlsRootCerts = config.TlsRootCerts[:len(config.TlsRootCerts)-1]
-				return config
-			})
-
-			By("Killing orderer3")
-			o3Proc := ordererProcesses[2]
-			o3Proc.Signal(syscall.SIGKILL)
-			Eventually(o3Proc.Wait(), network.EventuallyTimeout).Should(Receive(MatchError("exit status 137")))
-
-			By("Restarting orderer3")
-			o3Runner := network.OrdererRunner(orderer3)
-			ordererRunners[2] = o3Runner
-			o3Proc = ifrit.Invoke(o3Runner)
-			Eventually(o3Proc.Ready(), network.EventuallyTimeout).Should(BeClosed())
-			ordererProcesses[2] = o3Proc
-
-			By("Ensuring TLS handshakes fail with the other orderers")
-			for i, oRunner := range ordererRunners {
-				if i < 2 {
-					Eventually(oRunner.Err(), network.EventuallyTimeout).Should(gbytes.Say("TLS handshake failed with error tls: failed to verify client certificate"))
-					continue
-				}
-				Eventually(oRunner.Err(), network.EventuallyTimeout).Should(gbytes.Say("TLS handshake failed with error remote error: tls: bad certificate"))
-				Eventually(oRunner.Err(), network.EventuallyTimeout).Should(gbytes.Say("Suspecting our own eviction from the channel"))
-			}
-
 			By("Attemping to add a consenter with invalid certs")
 			// create new certs that are not in the channel config
 			ca, err := tlsgen.NewCA()
 			Expect(err).NotTo(HaveOccurred())
 			client, err := ca.NewClientCertKeyPair()
 			Expect(err).NotTo(HaveOccurred())
 
+			newConsenterCertPem, _ := pem.Decode(client.Cert)
+			newConsenterCert, err := x509.ParseCertificate(newConsenterCertPem.Bytes)
+			Expect(err).NotTo(HaveOccurred())
+
 			current, updated := consenterAdder(
 				network,
 				peer,
@@ -437,7 +415,7 @@ var _ = Describe("EndToEnd reconfiguration and onboarding", func() {
 			)
 			sess = nwo.UpdateOrdererConfigSession(network, orderer, network.SystemChannel.Name, current, updated, peer, orderer)
 			Eventually(sess, network.EventuallyTimeout).Should(gexec.Exit(1))
-			Expect(sess.Err).To(gbytes.Say(fmt.Sprintf("BAD_REQUEST -- error applying config update to existing channel 'systemchannel': consensus metadata update for channel config update is invalid: verifying tls client cert with serial number %d: x509: certificate signed by unknown authority", client.TLSCert.SerialNumber)))
+			Expect(sess.Err).To(gbytes.Say(fmt.Sprintf("BAD_REQUEST -- error applying config update to existing channel 'systemchannel': consensus metadata update for channel config update is invalid: invalid new config metadata: verifying tls client cert with serial number %d: x509: certificate signed by unknown authority", newConsenterCert.SerialNumber)))
 		})
 	})
 

orderer/common/msgprocessor/systemchannel.go

@@ -29,7 +29,7 @@ type ChannelConfigTemplator interface {
 
 // MetadataValidator can be used to validate updates to the consensus-specific metadata.
 type MetadataValidator interface {
-	ValidateConsensusMetadata(oldMetadata, newMetadata []byte, newChannel bool) error
+	ValidateConsensusMetadata(oldOrdererConfig, newOrdererConfig channelconfig.Orderer, newChannel bool) error
 }
 
 // SystemChannel implements the Processor interface for the system channel.

orderer/common/msgprocessor/systemchannelfilter.go

@@ -151,23 +151,21 @@ func (scf *SystemChainFilter) authorizeAndInspect(configTx *cb.Envelope) error {
 		return errors.Wrap(err, "new bundle invalid")
 	}
 
-	oc, ok := bundle.OrdererConfig()
+	ordererConfig, ok := bundle.OrdererConfig()
 	if !ok {
 		return errors.New("config is missing orderer group")
 	}
-	newMetadata := oc.ConsensusMetadata()
 
 	oldOrdererConfig, ok := scf.support.OrdererConfig()
 	if !ok {
 		logger.Panic("old config is missing orderer group")
 	}
-	oldMetadata := oldOrdererConfig.ConsensusMetadata()
 
-	if err = scf.validator.ValidateConsensusMetadata(oldMetadata, newMetadata, true); err != nil {
+	if err = scf.validator.ValidateConsensusMetadata(oldOrdererConfig, ordererConfig, true); err != nil {
 		return errors.Wrap(err, "consensus metadata update for channel creation is invalid")
 	}
 
-	if err = oc.Capabilities().Supported(); err != nil {
+	if err = ordererConfig.Capabilities().Supported(); err != nil {
 		return errors.Wrap(err, "config update is not compatible")
 	}
 

orderer/common/msgprocessor/systemchannelfilter_test.go

@@ -455,6 +455,6 @@ func TestFailedMetadataValidation(t *testing.T) {
 	assert.Equal(t, 1, mv.ValidateConsensusMetadataCallCount())
 	om, nm, nc := mv.ValidateConsensusMetadataArgsForCall(0)
 	assert.True(t, nc)
-	assert.Equal(t, []byte("old consensus metadata"), om)
-	assert.Equal(t, []byte("new consensus metadata"), nm)
+	assert.Equal(t, []byte("old consensus metadata"), om.ConsensusMetadata())
+	assert.Equal(t, []byte("new consensus metadata"), nm.ConsensusMetadata())
 }

orderer/common/multichannel/chainsupport.go

@@ -224,16 +224,14 @@ func (cs *ChainSupport) ProposeConfigUpdate(configtx *cb.Envelope) (*cb.ConfigEn
 	if !ok {
 		logger.Panic("old config is missing orderer group")
 	}
-	oldMetadata := oldOrdererConfig.ConsensusMetadata()
 
 	// we can remove this check since this is being validated in checkResources earlier
 	newOrdererConfig, ok := bundle.OrdererConfig()
 	if !ok {
 		return nil, errors.New("new config is missing orderer group")
 	}
-	newMetadata := newOrdererConfig.ConsensusMetadata()
 
-	if err = cs.ValidateConsensusMetadata(oldMetadata, newMetadata, false); err != nil {
+	if err = cs.ValidateConsensusMetadata(oldOrdererConfig, newOrdererConfig, false); err != nil {
 		return nil, errors.Wrap(err, "consensus metadata update for channel config update is invalid")
 	}
 	return env, nil

orderer/common/multichannel/chainsupport_test.go

@@ -160,8 +160,8 @@ func TestConsensusMetadataValidation(t *testing.T) {
 	assert.Equal(t, 1, mv.ValidateConsensusMetadataCallCount())
 	om, nm, nc := mv.ValidateConsensusMetadataArgsForCall(0)
 	assert.False(t, nc)
-	assert.Equal(t, oldConsensusMetadata, om)
-	assert.Equal(t, newConsensusMetadata, nm)
+	assert.Equal(t, oldConsensusMetadata, om.ConsensusMetadata())
+	assert.Equal(t, newConsensusMetadata, nm.ConsensusMetadata())
 
 	// case 2: invalid consensus metadata update
 	mv.ValidateConsensusMetadataReturns(errors.New("bananas"))

orderer/consensus/consensus.go

@@ -40,7 +40,7 @@ type MetadataValidator interface {
 	// updates on the channel.
 	// Since the ConsensusMetadata is specific to the consensus implementation (independent of the particular
 	// chain) this validation also needs to be implemented by the specific consensus implementation.
-	ValidateConsensusMetadata(oldMetadata, newMetadata []byte, newChannel bool) error
+	ValidateConsensusMetadata(oldOrdererConfig, newOrdererConfig channelconfig.Orderer, newChannel bool) error
 }
 
 // Chain defines a way to inject messages for ordering.
@@ -139,6 +139,6 @@ type NoOpMetadataValidator struct {
 
 // ValidateConsensusMetadata determines the validity of a ConsensusMetadata update during config updates
 // on the channel, and it always returns nil error for the NoOpMetadataValidator implementation.
-func (n NoOpMetadataValidator) ValidateConsensusMetadata(oldMetadataBytes, newMetadataBytes []byte, newChannel bool) error {
+func (n NoOpMetadataValidator) ValidateConsensusMetadata(oldChannelConfig, newChannelConfig channelconfig.Orderer, newChannel bool) error {
 	return nil
 }

orderer/consensus/etcdraft/chain.go

@@ -10,6 +10,7 @@ import (
 	"context"
 	"encoding/pem"
 	"fmt"
+	"github.com/hyperledger/fabric/common/channelconfig"
 	"sync"
 	"sync/atomic"
 	"time"
@@ -949,7 +950,7 @@ func (c *Chain) detectConfChange(block *common.Block) *MembershipChanges {
 		c.sizeLimit = configMetadata.Options.SnapshotIntervalSize
 	}
 
-	changes, err := ComputeMembershipChanges(c.opts.BlockMetadata, c.opts.Consenters, configMetadata.Consenters, c.support.SharedConfig())
+	changes, err := ComputeMembershipChanges(c.opts.BlockMetadata, c.opts.Consenters, configMetadata.Consenters)
 	if err != nil {
 		c.logger.Panicf("illegal configuration change detected: %s", err)
 	}
@@ -1274,34 +1275,51 @@ func (c *Chain) newConfigMetadata(block *common.Block) *etcdraft.ConfigMetadata
 
 // ValidateConsensusMetadata determines the validity of a
 // ConsensusMetadata update during config updates on the channel.
-func (c *Chain) ValidateConsensusMetadata(oldMetadataBytes, newMetadataBytes []byte, newChannel bool) error {
+func (c *Chain) ValidateConsensusMetadata(oldOrdererConfig, newOrdererConfig channelconfig.Orderer, newChannel bool) error {
+	if newOrdererConfig == nil {
+		c.logger.Panic("Programming Error: ValidateConsensusMetadata called with nil new channel config")
+		return nil
+	}
+
 	// metadata was not updated
-	if newMetadataBytes == nil {
+	if newOrdererConfig.ConsensusMetadata() == nil {
 		return nil
 	}
-	if oldMetadataBytes == nil {
+
+	if oldOrdererConfig == nil {
+		c.logger.Panic("Programming Error: ValidateConsensusMetadata called with nil old channel config")
+		return nil
+	}
+
+	if oldOrdererConfig.ConsensusMetadata() == nil {
 		c.logger.Panic("Programming Error: ValidateConsensusMetadata called with nil old metadata")
+		return nil
 	}
 
 	oldMetadata := &etcdraft.ConfigMetadata{}
-	if err := proto.Unmarshal(oldMetadataBytes, oldMetadata); err != nil {
+	if err := proto.Unmarshal(oldOrdererConfig.ConsensusMetadata(), oldMetadata); err != nil {
 		c.logger.Panicf("Programming Error: Failed to unmarshal old etcdraft consensus metadata: %v", err)
 	}
+
 	newMetadata := &etcdraft.ConfigMetadata{}
-	if err := proto.Unmarshal(newMetadataBytes, newMetadata); err != nil {
+	if err := proto.Unmarshal(newOrdererConfig.ConsensusMetadata(), newMetadata); err != nil {
 		return errors.Wrap(err, "failed to unmarshal new etcdraft metadata configuration")
 	}
 
-	err := CheckConfigMetadata(newMetadata)
+	verifyOpts, err := createX509VerifyOptions(newOrdererConfig)
 	if err != nil {
-		return errors.Wrap(err, "invalid new config metdadata")
+		return errors.Wrapf(err, "failed to create x509 verify options from old and new orderer config")
+	}
+
+	if err := VerifyConfigMetadata(newMetadata, verifyOpts); err != nil {
+		return errors.Wrap(err, "invalid new config metadata")
 	}
 
 	if newChannel {
 		// check if the consenters are a subset of the existing consenters (system channel consenters)
 		set := ConsentersToMap(oldMetadata.Consenters)
 		for _, c := range newMetadata.Consenters {
-			if _, exits := set[string(c.ClientTlsCert)]; !exits {
+			if !set.Exists(c) {
 				return errors.New("new channel has consenter that is not part of system consenter set")
 			}
 		}
@@ -1314,11 +1332,18 @@ func (c *Chain) ValidateConsensusMetadata(oldMetadataBytes, newMetadataBytes []b
 	c.raftMetadataLock.RUnlock()
 
 	dummyOldConsentersMap := CreateConsentersMap(dummyOldBlockMetadata, oldMetadata)
-	changes, err := ComputeMembershipChanges(dummyOldBlockMetadata, dummyOldConsentersMap, newMetadata.Consenters, c.support.SharedConfig())
+	changes, err := ComputeMembershipChanges(dummyOldBlockMetadata, dummyOldConsentersMap, newMetadata.Consenters)
 	if err != nil {
 		return err
 	}
 
+	//new config metadata was verified above. Additionally need to check new consenters for certificates expiration
+	for _, c := range changes.AddedNodes {
+		if err := validateConsenterTLSCerts(c, verifyOpts, false); err != nil {
+			return errors.Wrapf(err, "consenter %s:%d has invalid certificates", c.Host, c.Port)
+		}
+	}
+
 	active := c.ActiveNodes.Load().([]uint64)
 	if changes.UnacceptableQuorumLoss(active) {
 		return errors.Errorf("%d out of %d nodes are alive, configuration will result in quorum loss", len(active), len(dummyOldConsentersMap))