Validate TLS certs during raft consenter addition (#1223)

* raft membership_test.go cleanup

- For negative tests, test that expected error message is returned.
- For positive tests, switch the order so expected/actual match the
expected usage of "testify/require" package.

Signed-off-by: Will Lahti <wtlahti@us.ibm.com>

* Validate TLS certs during raft consenter addition

FAB-17733 #done

Signed-off-by: Will Lahti <wtlahti@us.ibm.com>

common/crypto/tlsgen/ca.go

@@ -29,6 +29,8 @@ type CA interface {
 	// CertBytes returns the certificate of the CA in PEM encoding
 	CertBytes() []byte
 
+	NewIntermediateCA() (CA, error)
+
 	// newCertKeyPair returns a certificate and private key pair and nil,
 	// or nil, error in case of failure
 	// The certificate is signed by the CA and is used for TLS client authentication
@@ -55,6 +57,16 @@ func NewCA() (CA, error) {
 	return c, nil
 }
 
+func (c *ca) NewIntermediateCA() (CA, error) {
+	intermediateCA := &ca{}
+	var err error
+	intermediateCA.caCert, err = newCertKeyPair(true, false, "", c.caCert.Signer, c.caCert.TLSCert)
+	if err != nil {
+		return nil, err
+	}
+	return intermediateCA, nil
+}
+
 // CertBytes returns the certificate of the CA in PEM encoding
 func (c *ca) CertBytes() []byte {
 	return c.caCert.Cert

integration/raft/config_test.go

@@ -26,6 +26,7 @@ import (
 	"github.com/hyperledger/fabric-protos-go/msp"
 	protosorderer "github.com/hyperledger/fabric-protos-go/orderer"
 	"github.com/hyperledger/fabric-protos-go/orderer/etcdraft"
+	"github.com/hyperledger/fabric/common/crypto/tlsgen"
 	"github.com/hyperledger/fabric/integration/nwo"
 	"github.com/hyperledger/fabric/integration/nwo/commands"
 	"github.com/hyperledger/fabric/internal/configtxgen/encoder"
@@ -56,7 +57,7 @@ var _ = Describe("EndToEnd reconfiguration and onboarding", func() {
 		ordererProcesses = nil
 
 		var err error
-		testDir, err = ioutil.TempDir("", "e2e-etcfraft_reconfig")
+		testDir, err = ioutil.TempDir("", "e2e-etcdraft_reconfig")
 		Expect(err).NotTo(HaveOccurred())
 
 		client, err = docker.NewClientFromEnv()
@@ -303,9 +304,10 @@ var _ = Describe("EndToEnd reconfiguration and onboarding", func() {
 			By("Waiting for a leader to be re-elected")
 			findLeader(ordererRunners)
 
-			// In the next part of the test we're going to bring up a third node with a different TLS root CA
-			// and we're going to add the TLS root CA *after* we add it to the channel, to ensure
-			// that we can dynamically update TLS root CAs in Raft while membership stays the same.
+			// In the next part of the test we're going to bring up a third node
+			// with a different TLS root CA. We're then going to remove the TLS
+			// root CA and restart the orderer, to ensure that we can dynamically
+			// update TLS root CAs in Raft while membership stays the same.
 
 			By("Creating configuration for a third orderer with a different TLS root CA")
 			orderer3 := &nwo.Orderer{
@@ -369,20 +371,12 @@ var _ = Describe("EndToEnd reconfiguration and onboarding", func() {
 				Expect(err).NotTo(HaveOccurred())
 			}
 
-			By("Adding the third orderer to the channel")
-			addConsenter(network, peer, orderer, "systemchannel", etcdraft.Consenter{
-				ServerTlsCert: thirdOrdererCertificate,
-				ClientTlsCert: thirdOrdererCertificate,
-				Host:          "127.0.0.1",
-				Port:          uint32(network.OrdererPort(orderer3, nwo.ClusterPort)),
-			})
-
 			By("Obtaining the last config block from the orderer once more to update the bootstrap file")
 			configBlock = nwo.GetConfigBlock(network, peer, orderer, "systemchannel")
 			err = ioutil.WriteFile(filepath.Join(testDir, "systemchannel_block.pb"), protoutil.MarshalOrPanic(configBlock), 0644)
 			Expect(err).NotTo(HaveOccurred())
 
-			By("Launching the third orderer")
+			By("Launching orderer3")
 			launch(orderer3)
 
 			By("Expanding the TLS root CA certificates")
@@ -391,20 +385,67 @@ var _ = Describe("EndToEnd reconfiguration and onboarding", func() {
 				return config
 			})
 
+			By("Adding orderer3 to the channel")
+			addConsenter(network, peer, orderer, "systemchannel", etcdraft.Consenter{
+				ServerTlsCert: thirdOrdererCertificate,
+				ClientTlsCert: thirdOrdererCertificate,
+				Host:          "127.0.0.1",
+				Port:          uint32(network.OrdererPort(orderer3, nwo.ClusterPort)),
+			})
+
 			By("Waiting for orderer3 to see the leader")
-			leader := findLeader([]*ginkgomon.Runner{ordererRunners[2]})
-			leaderIndex := leader - 1
+			findLeader([]*ginkgomon.Runner{ordererRunners[2]})
 
-			fmt.Fprint(GinkgoWriter, "Killing the leader", leader)
-			ordererProcesses[leaderIndex].Signal(syscall.SIGTERM)
-			Eventually(ordererProcesses[leaderIndex].Wait(), network.EventuallyTimeout).Should(Receive())
+			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("Ensuring orderer3 detects leader loss")
-			leaderLoss := fmt.Sprintf("Raft leader changed: %d -> 0", leader)
-			Eventually(ordererRunners[2].Err(), network.EventuallyTimeout, time.Second).Should(gbytes.Say(leaderLoss))
+			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: client didn't provide a 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("Waiting for the leader to be re-elected")
-			findLeader([]*ginkgomon.Runner{ordererRunners[2]})
+			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())
+
+			current, updated := consenterAdder(
+				network,
+				peer,
+				orderer,
+				"systemchannel",
+				etcdraft.Consenter{
+					ServerTlsCert: client.Cert,
+					ClientTlsCert: client.Cert,
+					Host:          "127.0.0.1",
+					Port:          uint32(network.OrdererPort(orderer3, nwo.ListenPort)),
+				},
+			)
+			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)))
 		})
 	})
 

orderer/consensus/etcdraft/chain.go

@@ -942,7 +942,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)
+	changes, err := ComputeMembershipChanges(c.opts.BlockMetadata, c.opts.Consenters, configMetadata.Consenters, c.support.SharedConfig())
 	if err != nil {
 		c.logger.Panicf("illegal configuration change detected: %s", err)
 	}
@@ -1306,7 +1306,7 @@ func (c *Chain) ValidateConsensusMetadata(oldMetadataBytes, newMetadataBytes []b
 	// create the dummy parameters for ComputeMembershipChanges
 	dummyOldBlockMetadata, _ := ReadBlockMetadata(nil, oldMetadata)
 	dummyOldConsentersMap := CreateConsentersMap(dummyOldBlockMetadata, oldMetadata)
-	changes, err := ComputeMembershipChanges(dummyOldBlockMetadata, dummyOldConsentersMap, newMetadata.Consenters)
+	changes, err := ComputeMembershipChanges(dummyOldBlockMetadata, dummyOldConsentersMap, newMetadata.Consenters, c.support.SharedConfig())
 	if err != nil {
 		return err
 	}