diff --git a/integration/raft/config_test.go b/integration/raft/config_test.go index 4b5d5f1a0d0..c06d35cf5f2 100644 --- a/integration/raft/config_test.go +++ b/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\)`)) }) }) @@ -371,51 +373,24 @@ var _ = Describe("EndToEnd reconfiguration and onboarding", func() { By("Launching orderer3") launch(orderer3) - By("Expanding the TLS root CA certificates") - nwo.UpdateOrdererMSP(network, peer, orderer, "systemchannel", "OrdererOrg", func(config msp.FabricMSPConfig) msp.FabricMSPConfig { + By("Expanding the TLS root CA certificates and adding orderer3 to the channel") + updateOrdererMSPAndConsensusMetadata(network, peer, orderer, "systemchannel", "OrdererOrg", func(config msp.FabricMSPConfig) msp.FabricMSPConfig { config.TlsRootCerts = append(config.TlsRootCerts, caCert) 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)), - }) + }, + func(metadata *etcdraft.ConfigMetadata) { + metadata.Consenters = append(metadata.Consenters, &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") 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() @@ -423,6 +398,10 @@ var _ = Describe("EndToEnd reconfiguration and onboarding", func() { 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 +416,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))) }) }) @@ -1826,3 +1805,51 @@ func updateEtcdRaftMetadata(network *nwo.Network, peer *nwo.Peer, orderer *nwo.O return newMetadata }) } + +func mutateConsensusMetadata(originalMetadata []byte, f func(md *etcdraft.ConfigMetadata)) []byte { + metadata := &etcdraft.ConfigMetadata{} + err := proto.Unmarshal(originalMetadata, metadata) + Expect(err).NotTo(HaveOccurred()) + + f(metadata) + + newMetadata, err := proto.Marshal(metadata) + Expect(err).NotTo(HaveOccurred()) + return newMetadata +} + +func updateOrdererMSPAndConsensusMetadata(network *nwo.Network, peer *nwo.Peer, orderer *nwo.Orderer, channel, orgID string, mutateMSP nwo.MSPMutator, f func(md *etcdraft.ConfigMetadata)) { + config := nwo.GetConfig(network, peer, orderer, channel) + updatedConfig := proto.Clone(config).(*common.Config) + + // Unpack the MSP config + rawMSPConfig := updatedConfig.ChannelGroup.Groups["Orderer"].Groups[orgID].Values["MSP"] + mspConfig := &msp.MSPConfig{} + err := proto.Unmarshal(rawMSPConfig.Value, mspConfig) + Expect(err).NotTo(HaveOccurred()) + + fabricConfig := &msp.FabricMSPConfig{} + err = proto.Unmarshal(mspConfig.Config, fabricConfig) + Expect(err).NotTo(HaveOccurred()) + + // Mutate it as we are asked + *fabricConfig = mutateMSP(*fabricConfig) + + // Wrap it back into the config + mspConfig.Config = protoutil.MarshalOrPanic(fabricConfig) + rawMSPConfig.Value = protoutil.MarshalOrPanic(mspConfig) + + consensusTypeConfigValue := updatedConfig.ChannelGroup.Groups["Orderer"].Values["ConsensusType"] + consensusTypeValue := &protosorderer.ConsensusType{} + err = proto.Unmarshal(consensusTypeConfigValue.Value, consensusTypeValue) + Expect(err).NotTo(HaveOccurred()) + + consensusTypeValue.Metadata = mutateConsensusMetadata(consensusTypeValue.Metadata, f) + + updatedConfig.ChannelGroup.Groups["Orderer"].Values["ConsensusType"] = &common.ConfigValue{ + ModPolicy: "Admins", + Value: protoutil.MarshalOrPanic(consensusTypeValue), + } + + nwo.UpdateOrdererConfig(network, orderer, channel, config, updatedConfig, peer, orderer) +} diff --git a/orderer/common/msgprocessor/mocks/metadata_validator.go b/orderer/common/msgprocessor/mocks/metadata_validator.go index 9e3d7b0c3a4..e2736ba01b9 100644 --- a/orderer/common/msgprocessor/mocks/metadata_validator.go +++ b/orderer/common/msgprocessor/mocks/metadata_validator.go @@ -3,14 +3,16 @@ package mocks import ( "sync" + + "github.com/hyperledger/fabric/common/channelconfig" ) type MetadataValidator struct { - ValidateConsensusMetadataStub func([]byte, []byte, bool) error + ValidateConsensusMetadataStub func(channelconfig.Orderer, channelconfig.Orderer, bool) error validateConsensusMetadataMutex sync.RWMutex validateConsensusMetadataArgsForCall []struct { - arg1 []byte - arg2 []byte + arg1 channelconfig.Orderer + arg2 channelconfig.Orderer arg3 bool } validateConsensusMetadataReturns struct { @@ -23,25 +25,15 @@ type MetadataValidator struct { invocationsMutex sync.RWMutex } -func (fake *MetadataValidator) ValidateConsensusMetadata(arg1 []byte, arg2 []byte, arg3 bool) error { - var arg1Copy []byte - if arg1 != nil { - arg1Copy = make([]byte, len(arg1)) - copy(arg1Copy, arg1) - } - var arg2Copy []byte - if arg2 != nil { - arg2Copy = make([]byte, len(arg2)) - copy(arg2Copy, arg2) - } +func (fake *MetadataValidator) ValidateConsensusMetadata(arg1 channelconfig.Orderer, arg2 channelconfig.Orderer, arg3 bool) error { fake.validateConsensusMetadataMutex.Lock() ret, specificReturn := fake.validateConsensusMetadataReturnsOnCall[len(fake.validateConsensusMetadataArgsForCall)] fake.validateConsensusMetadataArgsForCall = append(fake.validateConsensusMetadataArgsForCall, struct { - arg1 []byte - arg2 []byte + arg1 channelconfig.Orderer + arg2 channelconfig.Orderer arg3 bool - }{arg1Copy, arg2Copy, arg3}) - fake.recordInvocation("ValidateConsensusMetadata", []interface{}{arg1Copy, arg2Copy, arg3}) + }{arg1, arg2, arg3}) + fake.recordInvocation("ValidateConsensusMetadata", []interface{}{arg1, arg2, arg3}) fake.validateConsensusMetadataMutex.Unlock() if fake.ValidateConsensusMetadataStub != nil { return fake.ValidateConsensusMetadataStub(arg1, arg2, arg3) @@ -59,13 +51,13 @@ func (fake *MetadataValidator) ValidateConsensusMetadataCallCount() int { return len(fake.validateConsensusMetadataArgsForCall) } -func (fake *MetadataValidator) ValidateConsensusMetadataCalls(stub func([]byte, []byte, bool) error) { +func (fake *MetadataValidator) ValidateConsensusMetadataCalls(stub func(channelconfig.Orderer, channelconfig.Orderer, bool) error) { fake.validateConsensusMetadataMutex.Lock() defer fake.validateConsensusMetadataMutex.Unlock() fake.ValidateConsensusMetadataStub = stub } -func (fake *MetadataValidator) ValidateConsensusMetadataArgsForCall(i int) ([]byte, []byte, bool) { +func (fake *MetadataValidator) ValidateConsensusMetadataArgsForCall(i int) (channelconfig.Orderer, channelconfig.Orderer, bool) { fake.validateConsensusMetadataMutex.RLock() defer fake.validateConsensusMetadataMutex.RUnlock() argsForCall := fake.validateConsensusMetadataArgsForCall[i] diff --git a/orderer/common/msgprocessor/systemchannel.go b/orderer/common/msgprocessor/systemchannel.go index e66520ea48a..4150db95342 100644 --- a/orderer/common/msgprocessor/systemchannel.go +++ b/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. diff --git a/orderer/common/msgprocessor/systemchannelfilter.go b/orderer/common/msgprocessor/systemchannelfilter.go index 2ae40f829dc..17f025f8e03 100644 --- a/orderer/common/msgprocessor/systemchannelfilter.go +++ b/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") } diff --git a/orderer/common/msgprocessor/systemchannelfilter_test.go b/orderer/common/msgprocessor/systemchannelfilter_test.go index a53b9e860e4..c795d1af498 100644 --- a/orderer/common/msgprocessor/systemchannelfilter_test.go +++ b/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()) } diff --git a/orderer/common/multichannel/chainsupport.go b/orderer/common/multichannel/chainsupport.go index d441a62192b..9c351c80e9d 100644 --- a/orderer/common/multichannel/chainsupport.go +++ b/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 diff --git a/orderer/common/multichannel/chainsupport_test.go b/orderer/common/multichannel/chainsupport_test.go index b11a7d8d68b..cbfce8a64a3 100644 --- a/orderer/common/multichannel/chainsupport_test.go +++ b/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")) diff --git a/orderer/consensus/consensus.go b/orderer/consensus/consensus.go index a39a4a22883..4651329d921 100644 --- a/orderer/consensus/consensus.go +++ b/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 } diff --git a/orderer/consensus/etcdraft/chain.go b/orderer/consensus/etcdraft/chain.go index df9b6dc7dc6..41364a55d28 100644 --- a/orderer/consensus/etcdraft/chain.go +++ b/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)) diff --git a/orderer/consensus/etcdraft/chain_test.go b/orderer/consensus/etcdraft/chain_test.go index 198d8ca5f94..4d84ed8c77c 100644 --- a/orderer/consensus/etcdraft/chain_test.go +++ b/orderer/consensus/etcdraft/chain_test.go @@ -60,7 +60,11 @@ func init() { factory.InitFactories(nil) } -func mockOrderer(batchTimeout time.Duration, metadata []byte) *mocks.OrdererConfig { +func mockOrderer(metadata []byte) *mocks.OrdererConfig { + return mockOrdererWithBatchTimeout(time.Second, metadata) +} + +func mockOrdererWithBatchTimeout(batchTimeout time.Duration, metadata []byte) *mocks.OrdererConfig { mockOrderer := &mocks.OrdererConfig{} mockOrderer.BatchTimeoutReturns(batchTimeout) mockOrderer.ConsensusMetadataReturns(metadata) @@ -68,7 +72,7 @@ func mockOrderer(batchTimeout time.Duration, metadata []byte) *mocks.OrdererConf } func mockOrdererWithTLSRootCert(batchTimeout time.Duration, metadata []byte, tlsCA tlsgen.CA) *mocks.OrdererConfig { - mockOrderer := mockOrderer(batchTimeout, metadata) + mockOrderer := mockOrdererWithBatchTimeout(batchTimeout, metadata) mockOrg := &mocks.OrdererOrg{} mockMSP := &mocks.MSP{} mockMSP.GetTLSRootCertsReturns([][]byte{tlsCA.CertBytes()}) @@ -345,7 +349,7 @@ var _ = Describe("Chain", func() { By("respecting batch timeout") cutter.CutNext = false timeout := time.Second - support.SharedConfigReturns(mockOrderer(timeout, nil)) + support.SharedConfigReturns(mockOrdererWithBatchTimeout(timeout, nil)) err = chain.Order(env, 0) Expect(err).NotTo(HaveOccurred()) Expect(fakeFields.fakeNormalProposalsReceived.AddCallCount()).To(Equal(2)) @@ -363,7 +367,7 @@ var _ = Describe("Chain", func() { close(cutter.Block) timeout := time.Second - support.SharedConfigReturns(mockOrderer(timeout, nil)) + support.SharedConfigReturns(mockOrdererWithBatchTimeout(timeout, nil)) err := chain.Order(env, 0) Expect(err).NotTo(HaveOccurred()) @@ -384,7 +388,7 @@ var _ = Describe("Chain", func() { It("does not write a block if halted before timeout", func() { close(cutter.Block) timeout := time.Second - support.SharedConfigReturns(mockOrderer(timeout, nil)) + support.SharedConfigReturns(mockOrdererWithBatchTimeout(timeout, nil)) err := chain.Order(env, 0) Expect(err).NotTo(HaveOccurred()) @@ -401,7 +405,7 @@ var _ = Describe("Chain", func() { close(cutter.Block) timeout := time.Second - support.SharedConfigReturns(mockOrderer(timeout, nil)) + support.SharedConfigReturns(mockOrdererWithBatchTimeout(timeout, nil)) err := chain.Order(env, 0) Expect(err).NotTo(HaveOccurred()) @@ -439,7 +443,7 @@ var _ = Describe("Chain", func() { close(cutter.Block) timeout := time.Second - support.SharedConfigReturns(mockOrderer(timeout, nil)) + support.SharedConfigReturns(mockOrdererWithBatchTimeout(timeout, nil)) err := chain.Order(env, 0) Expect(err).NotTo(HaveOccurred()) @@ -457,7 +461,7 @@ var _ = Describe("Chain", func() { close(cutter.Block) timeout := time.Hour - support.SharedConfigReturns(mockOrderer(timeout, nil)) + support.SharedConfigReturns(mockOrdererWithBatchTimeout(timeout, nil)) support.SequenceReturns(1) }) @@ -3300,7 +3304,7 @@ func newChain( if support == nil { support = &consensusmocks.FakeConsenterSupport{} support.ChannelIDReturns(channel) - support.SharedConfigReturns(mockOrderer(timeout, nil)) + support.SharedConfigReturns(mockOrdererWithBatchTimeout(timeout, nil)) } cutter := mockblockcutter.NewReceiver() close(cutter.Block) @@ -3601,7 +3605,7 @@ func createNetwork( m := proto.Clone(raftMetadata).(*raftprotos.BlockMetadata) support := &consensusmocks.FakeConsenterSupport{} support.ChannelIDReturns(channel) - support.SharedConfigReturns(mockOrderer(timeout, nil)) + support.SharedConfigReturns(mockOrdererWithBatchTimeout(timeout, nil)) mockOrdererConfig := mockOrdererWithTLSRootCert(timeout, nil, tlsCA) support.SharedConfigReturns(mockOrdererConfig) n.addChain(newChain(timeout, channel, dir, nodeID, m, consenters, cryptoProvider, support)) diff --git a/orderer/consensus/etcdraft/consenter_test.go b/orderer/consensus/etcdraft/consenter_test.go index 001c8ddb0e0..5d316e48487 100644 --- a/orderer/consensus/etcdraft/consenter_test.go +++ b/orderer/consensus/etcdraft/consenter_test.go @@ -8,6 +8,7 @@ package etcdraft_test import ( "encoding/pem" + "fmt" "io/ioutil" "os" "path" @@ -21,6 +22,7 @@ import ( "github.com/hyperledger/fabric/common/channelconfig" "github.com/hyperledger/fabric/common/crypto/tlsgen" "github.com/hyperledger/fabric/common/flogging" + "github.com/hyperledger/fabric/internal/configtxgen/genesisconfig" "github.com/hyperledger/fabric/internal/pkg/comm" "github.com/hyperledger/fabric/orderer/common/cluster" clustermocks "github.com/hyperledger/fabric/orderer/common/cluster/mocks" @@ -37,6 +39,19 @@ import ( "go.uber.org/zap/zapcore" ) +//These fixtures contain certificates for testing consenters. +//In both folders certificates generated using tlsgen pkg, each certificate is singed by ca.pem inside corresponding folder. +//Each cert has 10years expiration time (tlsgenCa.NewServerCertKeyPair("localhost")). + +//NOTE ONLY FOR GO 1.15+: prior to go1.15 tlsgen produced CA root cert without SubjectKeyId, which is not allowed by MSP validator. +//In this test left tags @ONLY-GO1.15+ in places where fixtures can be replaced with tlsgen runtime generated certs once fabric moved to 1.15 + +const ( + consentersTestDataDir = "testdata/consenters_certs/" + ca1Dir = consentersTestDataDir + "ca1" + ca2Dir = consentersTestDataDir + "ca2" +) + var ( certAsPEM []byte ) @@ -60,12 +75,15 @@ var _ = Describe("Consenter", func() { dataDir string snapDir string walDir string + mspDir string + tlsCA tlsgen.CA ) BeforeEach(func() { - ca, err := tlsgen.NewCA() + var err error + tlsCA, err = tlsgen.NewCA() Expect(err).NotTo(HaveOccurred()) - kp, err := ca.NewClientCertKeyPair() + kp, err := tlsCA.NewClientCertKeyPair() Expect(err).NotTo(HaveOccurred()) if certAsPEM == nil { certAsPEM = kp.Cert @@ -100,21 +118,22 @@ var _ = Describe("Consenter", func() { AfterEach(func() { os.RemoveAll(dataDir) + os.RemoveAll(mspDir) }) When("the consenter is extracting the channel", func() { It("extracts successfully from step requests", func() { - consenter := newConsenter(chainGetter) + consenter := newConsenter(chainGetter, tlsCA.CertBytes(), certAsPEM) ch := consenter.TargetChannel(&orderer.ConsensusRequest{Channel: "mychannel"}) Expect(ch).To(BeIdenticalTo("mychannel")) }) It("extracts successfully from submit requests", func() { - consenter := newConsenter(chainGetter) + consenter := newConsenter(chainGetter, tlsCA.CertBytes(), certAsPEM) ch := consenter.TargetChannel(&orderer.SubmitRequest{Channel: "mychannel"}) Expect(ch).To(BeIdenticalTo("mychannel")) }) It("returns an empty string for the rest of the messages", func() { - consenter := newConsenter(chainGetter) + consenter := newConsenter(chainGetter, tlsCA.CertBytes(), certAsPEM) ch := consenter.TargetChannel(&common.Block{}) Expect(ch).To(BeEmpty()) }) @@ -135,30 +154,30 @@ var _ = Describe("Consenter", func() { Chain: &multichannel.ChainSupport{}, }) }) - It("calls the chain getter and returns the reference when it is found", func() { - consenter := newConsenter(chainGetter) + It("calls the chain manager and returns the reference when it is found", func() { + consenter := newConsenter(chainGetter, tlsCA.CertBytes(), certAsPEM) Expect(consenter).NotTo(BeNil()) chain := consenter.ReceiverByChain("mychannel") Expect(chain).NotTo(BeNil()) Expect(chain).To(BeIdenticalTo(chainInstance)) }) - It("calls the chain getter and returns nil when it's not found", func() { - consenter := newConsenter(chainGetter) + It("calls the chain manager and returns nil when it's not found", func() { + consenter := newConsenter(chainGetter, tlsCA.CertBytes(), certAsPEM) Expect(consenter).NotTo(BeNil()) chain := consenter.ReceiverByChain("notmychannel") Expect(chain).To(BeNil()) }) - It("calls the chain getter and returns nil when it's not a raft chain", func() { - consenter := newConsenter(chainGetter) + It("calls the chain manager and returns nil when it's not a raft chain", func() { + consenter := newConsenter(chainGetter, tlsCA.CertBytes(), certAsPEM) Expect(consenter).NotTo(BeNil()) chain := consenter.ReceiverByChain("notraftchain") Expect(chain).To(BeNil()) }) It("calls the chain getter and panics when the chain has a bad internal state", func() { - consenter := newConsenter(chainGetter) + consenter := newConsenter(chainGetter, tlsCA.CertBytes(), certAsPEM) Expect(consenter).NotTo(BeNil()) Expect(func() { @@ -191,7 +210,7 @@ var _ = Describe("Consenter", func() { ) support.SharedConfigReturns(mockOrderer) - consenter := newConsenter(chainGetter) + consenter := newConsenter(chainGetter, tlsCA.CertBytes(), certAsPEM) consenter.EtcdRaftConfig.WALDir = walDir consenter.EtcdRaftConfig.SnapDir = snapDir // consenter.EtcdRaftConfig.EvictionSuspicion is missing @@ -237,7 +256,7 @@ var _ = Describe("Consenter", func() { ) support.SharedConfigReturns(mockOrderer) - consenter := newConsenter(chainGetter) + consenter := newConsenter(chainGetter, tlsCA.CertBytes(), certAsPEM) consenter.EtcdRaftConfig.WALDir = walDir consenter.EtcdRaftConfig.SnapDir = snapDir //without a system channel, the InactiveChainRegistry is nil @@ -287,7 +306,7 @@ var _ = Describe("Consenter", func() { support.SharedConfigReturns(mockOrderer) support.ChannelIDReturns("foo") - consenter := newConsenter(chainGetter) + consenter := newConsenter(chainGetter, tlsCA.CertBytes(), certAsPEM) chain, err := consenter.HandleChain(support, &common.Metadata{}) Expect(chain).To(Not(BeNil())) @@ -312,7 +331,7 @@ var _ = Describe("Consenter", func() { ) support.SharedConfigReturns(mockOrderer) - consenter := newConsenter(chainGetter) + consenter := newConsenter(chainGetter, tlsCA.CertBytes(), certAsPEM) chain, err := consenter.HandleChain(support, nil) Expect(chain).To(BeNil()) @@ -342,7 +361,7 @@ var _ = Describe("Consenter", func() { mockOrderer.CapabilitiesReturns(&mocks.OrdererCapabilities{}) support.SharedConfigReturns(mockOrderer) - consenter := newConsenter(chainGetter) + consenter := newConsenter(chainGetter, tlsCA.CertBytes(), certAsPEM) chain, err := consenter.HandleChain(support, nil) Expect(chain).To(BeNil()) @@ -373,7 +392,7 @@ var _ = Describe("Consenter", func() { mockOrderer.CapabilitiesReturns(&mocks.OrdererCapabilities{}) support.SharedConfigReturns(mockOrderer) - consenter := newConsenter(chainGetter) + consenter := newConsenter(chainGetter, tlsCA.CertBytes(), certAsPEM) consenter.EtcdRaftConfig.TickIntervalOverride = "seven" _, err := consenter.HandleChain(support, nil) @@ -405,7 +424,7 @@ var _ = Describe("Consenter", func() { support.SharedConfigReturns(mockOrderer) support.ChannelIDReturns("foo") - consenter := newConsenter(chainGetter) + consenter := newConsenter(chainGetter, tlsCA.CertBytes(), certAsPEM) //without a system channel, the InactiveChainRegistry is nil consenter.InactiveChainRegistry = nil consenter.icr = nil @@ -424,10 +443,8 @@ type consenter struct { icr *mocks.InactiveChainRegistry } -func newConsenter(chainGetter *mocks.ChainGetter) *consenter { +func newConsenter(chainGetter *mocks.ChainGetter, caCert, cert []byte) *consenter { communicator := &clustermocks.Communicator{} - ca, err := tlsgen.NewCA() - Expect(err).NotTo(HaveOccurred()) communicator.On("Configure", mock.Anything, mock.Anything) icr := &mocks.InactiveChainRegistry{} icr.On("TrackChain", "foo", mock.Anything, mock.Anything) @@ -438,7 +455,7 @@ func newConsenter(chainGetter *mocks.ChainGetter) *consenter { c := &etcdraft.Consenter{ InactiveChainRegistry: icr, Communication: communicator, - Cert: certAsPEM, + Cert: cert, Logger: flogging.MustGetLogger("test"), Chains: chainGetter, Dispatcher: &etcdraft.Dispatcher{ @@ -448,7 +465,7 @@ func newConsenter(chainGetter *mocks.ChainGetter) *consenter { Dialer: &cluster.PredicateDialer{ Config: comm.ClientConfig{ SecOpts: comm.SecureOptions{ - Certificate: ca.CertBytes(), + Certificate: caCert, }, }, }, @@ -459,3 +476,27 @@ func newConsenter(chainGetter *mocks.ChainGetter) *consenter { icr: icr, } } + +func generateCertificates(confAppRaft *genesisconfig.Profile, tlsCA tlsgen.CA, certDir string) [][]byte { + certificates := [][]byte{} + for i, c := range confAppRaft.Orderer.EtcdRaft.Consenters { + srvC, err := tlsCA.NewServerCertKeyPair(c.Host) + Expect(err).NotTo(HaveOccurred()) + srvP := path.Join(certDir, fmt.Sprintf("server%d.crt", i)) + err = ioutil.WriteFile(srvP, srvC.Cert, 0644) + Expect(err).NotTo(HaveOccurred()) + + clnC, err := tlsCA.NewClientCertKeyPair() + Expect(err).NotTo(HaveOccurred()) + clnP := path.Join(certDir, fmt.Sprintf("client%d.crt", i)) + err = ioutil.WriteFile(clnP, clnC.Cert, 0644) + Expect(err).NotTo(HaveOccurred()) + + c.ServerTlsCert = []byte(srvP) + c.ClientTlsCert = []byte(clnP) + + certificates = append(certificates, srvC.Cert) + } + + return certificates +} diff --git a/orderer/consensus/etcdraft/membership.go b/orderer/consensus/etcdraft/membership.go index 41806295798..bc799625190 100644 --- a/orderer/consensus/etcdraft/membership.go +++ b/orderer/consensus/etcdraft/membership.go @@ -7,13 +7,10 @@ SPDX-License-Identifier: Apache-2.0 package etcdraft import ( - "crypto/x509" - "encoding/pem" "fmt" "github.com/golang/protobuf/proto" "github.com/hyperledger/fabric-protos-go/orderer/etcdraft" - "github.com/hyperledger/fabric/common/channelconfig" "github.com/pkg/errors" "go.etcd.io/etcd/raft" "go.etcd.io/etcd/raft/raftpb" @@ -42,7 +39,7 @@ type MembershipChanges struct { // ComputeMembershipChanges computes membership update based on information about new consenters, returns // two slices: a slice of added consenters and a slice of consenters to be removed -func ComputeMembershipChanges(oldMetadata *etcdraft.BlockMetadata, oldConsenters map[uint64]*etcdraft.Consenter, newConsenters []*etcdraft.Consenter, ordererConfig channelconfig.Orderer) (mc *MembershipChanges, err error) { +func ComputeMembershipChanges(oldMetadata *etcdraft.BlockMetadata, oldConsenters map[uint64]*etcdraft.Consenter, newConsenters []*etcdraft.Consenter) (mc *MembershipChanges, err error) { result := &MembershipChanges{ NewConsenters: map[uint64]*etcdraft.Consenter{}, NewBlockMetadata: proto.Clone(oldMetadata).(*etcdraft.BlockMetadata), @@ -60,10 +57,6 @@ func ComputeMembershipChanges(oldMetadata *etcdraft.BlockMetadata, oldConsenters result.NewConsenters[nodeID] = c continue } - err := validateConsenterTLSCerts(c, ordererConfig) - if err != nil { - return nil, err - } addedNodeIndex = i result.AddedNodes = append(result.AddedNodes, c) } @@ -71,7 +64,7 @@ func ComputeMembershipChanges(oldMetadata *etcdraft.BlockMetadata, oldConsenters var deletedNodeID uint64 newConsentersSet := ConsentersToMap(newConsenters) for nodeID, c := range oldConsenters { - if _, exists := newConsentersSet[string(c.ClientTlsCert)]; !exists { + if !newConsentersSet.Exists(c) { result.RemovedNodes = append(result.RemovedNodes, c) deletedNodeID = nodeID } @@ -112,94 +105,6 @@ func ComputeMembershipChanges(oldMetadata *etcdraft.BlockMetadata, oldConsenters return result, nil } -func validateConsenterTLSCerts(c *etcdraft.Consenter, ordererConfig channelconfig.Orderer) error { - clientCert, err := parseCertificateFromBytes(c.ClientTlsCert) - if err != nil { - return errors.Wrap(err, "parsing tls client cert") - } - serverCert, err := parseCertificateFromBytes(c.ServerTlsCert) - if err != nil { - return errors.Wrap(err, "parsing tls server cert") - } - - opts, err := createX509VerifyOptions(ordererConfig.Organizations()) - if err != nil { - return errors.WithMessage(err, "creating x509 verify options") - } - _, err = clientCert.Verify(opts) - if err != nil { - return fmt.Errorf("verifying tls client cert with serial number %d: %v", clientCert.SerialNumber, err) - } - - _, err = serverCert.Verify(opts) - if err != nil { - return fmt.Errorf("verifying tls server cert with serial number %d: %v", serverCert.SerialNumber, err) - } - - return nil -} - -func createX509VerifyOptions(orgs map[string]channelconfig.OrdererOrg) (x509.VerifyOptions, error) { - tlsRoots := x509.NewCertPool() - tlsIntermediates := x509.NewCertPool() - - for _, org := range orgs { - rootCerts, err := parseCertificateListFromBytes(org.MSP().GetTLSRootCerts()) - if err != nil { - return x509.VerifyOptions{}, errors.Wrap(err, "parsing tls root certs") - } - intermediateCerts, err := parseCertificateListFromBytes(org.MSP().GetTLSIntermediateCerts()) - if err != nil { - return x509.VerifyOptions{}, errors.Wrap(err, "parsing tls intermediate certs") - } - - for _, cert := range rootCerts { - tlsRoots.AddCert(cert) - } - for _, cert := range intermediateCerts { - tlsIntermediates.AddCert(cert) - } - } - - return x509.VerifyOptions{ - Roots: tlsRoots, - Intermediates: tlsIntermediates, - KeyUsages: []x509.ExtKeyUsage{ - x509.ExtKeyUsageClientAuth, - x509.ExtKeyUsageServerAuth, - }, - }, nil -} - -func parseCertificateListFromBytes(certs [][]byte) ([]*x509.Certificate, error) { - certificateList := []*x509.Certificate{} - - for _, cert := range certs { - certificate, err := parseCertificateFromBytes(cert) - if err != nil { - return certificateList, err - } - - certificateList = append(certificateList, certificate) - } - - return certificateList, nil -} - -func parseCertificateFromBytes(cert []byte) (*x509.Certificate, error) { - pemBlock, _ := pem.Decode(cert) - if pemBlock == nil { - return &x509.Certificate{}, fmt.Errorf("no PEM data found in cert[% x]", cert) - } - - certificate, err := x509.ParseCertificate(pemBlock.Bytes) - if err != nil { - return &x509.Certificate{}, err - } - - return certificate, nil -} - // Stringer implements fmt.Stringer interface func (mc *MembershipChanges) String() string { return fmt.Sprintf("add %d node(s), remove %d node(s)", len(mc.AddedNodes), len(mc.RemovedNodes)) diff --git a/orderer/consensus/etcdraft/membership_test.go b/orderer/consensus/etcdraft/membership_test.go index dc7d55881dc..38457f3ec74 100644 --- a/orderer/consensus/etcdraft/membership_test.go +++ b/orderer/consensus/etcdraft/membership_test.go @@ -7,7 +7,6 @@ SPDX-License-Identifier: Apache-2.0 package etcdraft_test import ( - "fmt" "testing" etcdraftproto "github.com/hyperledger/fabric-protos-go/orderer/etcdraft" @@ -155,10 +154,6 @@ func TestMembershipChanges(t *testing.T) { client4, err := tlsIntermediateCA2.NewClientCertKeyPair() require.NoError(t, err) - // cert for fake-org3 - tlsCA3, err := tlsgen.NewCA() - require.NoError(t, err) - client5, err := tlsCA3.NewClientCertKeyPair() require.NoError(t, err) c := []*etcdraftproto.Consenter{ @@ -231,54 +226,6 @@ func TestMembershipChanges(t *testing.T) { Rotated: false, ExpectedErr: "", }, - { - Name: "Add a node with an invalid client cert bytes", - OldConsenters: map[uint64]*etcdraftproto.Consenter{ - 1: c[0], - }, - NewConsenters: []*etcdraftproto.Consenter{ - c[0], - {ClientTlsCert: []byte("woops")}, - }, - Changes: nil, - ExpectedErr: fmt.Sprintf("parsing tls client cert: no PEM data found in cert[% x]", []byte("woops")), - }, - { - Name: "Add a node with an invalid server cert bytes", - OldConsenters: map[uint64]*etcdraftproto.Consenter{ - 1: c[0], - }, - NewConsenters: []*etcdraftproto.Consenter{ - c[0], - {ClientTlsCert: client3.Cert, ServerTlsCert: []byte("doh!")}, - }, - Changes: nil, - ExpectedErr: fmt.Sprintf("parsing tls server cert: no PEM data found in cert[% x]", []byte("doh!")), - }, - { - Name: "Add a node with an invalid tls client cert", - OldConsenters: map[uint64]*etcdraftproto.Consenter{ - 1: c[0], - }, - NewConsenters: []*etcdraftproto.Consenter{ - c[0], - {ClientTlsCert: client5.Cert, ServerTlsCert: client3.Cert}, - }, - Changes: nil, - ExpectedErr: fmt.Sprintf("verifying tls client cert with serial number %d: x509: certificate signed by unknown authority", client5.TLSCert.SerialNumber), - }, - { - Name: "Add a node with an invalid tls server cert", - OldConsenters: map[uint64]*etcdraftproto.Consenter{ - 1: c[0], - }, - NewConsenters: []*etcdraftproto.Consenter{ - c[0], - {ClientTlsCert: client3.Cert, ServerTlsCert: client5.Cert}, - }, - Changes: nil, - ExpectedErr: fmt.Sprintf("verifying tls server cert with serial number %d: x509: certificate signed by unknown authority", client5.TLSCert.SerialNumber), - }, { Name: "Remove a node", OldConsenters: map[uint64]*etcdraftproto.Consenter{ @@ -383,7 +330,7 @@ func TestMembershipChanges(t *testing.T) { for _, test := range tests { t.Run(test.Name, func(t *testing.T) { - changes, err := etcdraft.ComputeMembershipChanges(blockMetadata, test.OldConsenters, test.NewConsenters, mockOrdererConfig) + changes, err := etcdraft.ComputeMembershipChanges(blockMetadata, test.OldConsenters, test.NewConsenters) if test.ExpectedErr != "" { require.EqualError(t, err, test.ExpectedErr) diff --git a/orderer/consensus/etcdraft/testdata/consenters_certs/ca1/ca.pem b/orderer/consensus/etcdraft/testdata/consenters_certs/ca1/ca.pem new file mode 100644 index 00000000000..b19793d7cca --- /dev/null +++ b/orderer/consensus/etcdraft/testdata/consenters_certs/ca1/ca.pem @@ -0,0 +1,12 @@ +-----BEGIN CERTIFICATE----- +MIIBxTCCAWqgAwIBAgIRAJfE/nlTLmP8FXwo1iQppRIwCgYIKoZIzj0EAwIwMjEw +MC4GA1UEBRMnMjAxNzM2Mjc4ODkyMTg1NDUzMzQxMjIyNzU2MzkxNDgzNzEyNzg2 +MB4XDTIwMTAwMTEzNDcwOFoXDTMwMDkzMDEzNDcwOFowMjEwMC4GA1UEBRMnMjAx +NzM2Mjc4ODkyMTg1NDUzMzQxMjIyNzU2MzkxNDgzNzEyNzg2MFkwEwYHKoZIzj0C +AQYIKoZIzj0DAQcDQgAEVJiaycMxW/VTrHMv0cme6CLSItWCyX0dra0qqV6qkcfb +6/ZTNuGHU04KUuPEFObHpFhJhzwP4MPBFkWLARj05aNhMF8wDgYDVR0PAQH/BAQD +AgGmMB0GA1UdJQQWMBQGCCsGAQUFBwMCBggrBgEFBQcDATAPBgNVHRMBAf8EBTAD +AQH/MB0GA1UdDgQWBBRCqpOXs1eIfE4DF7Wuq2f6dfyx6DAKBggqhkjOPQQDAgNJ +ADBGAiEAl4CZfgRBElX2gTHOaRUQEcNROyqjmLfgnzGZwgwT2jkCIQDkXc0zh2tF +Oe8uRH7h/89avK8HPIX9baWqJYGMoqJwBg== +-----END CERTIFICATE----- diff --git a/orderer/consensus/etcdraft/testdata/consenters_certs/ca1/expired-client.pem b/orderer/consensus/etcdraft/testdata/consenters_certs/ca1/expired-client.pem new file mode 100644 index 00000000000..e8eaaf29e44 --- /dev/null +++ b/orderer/consensus/etcdraft/testdata/consenters_certs/ca1/expired-client.pem @@ -0,0 +1,11 @@ +-----BEGIN CERTIFICATE----- +MIIBqjCCAU+gAwIBAgIQK3VSM8NZjTTUe16wYCcpczAKBggqhkjOPQQDAjAyMTAw +LgYDVQQFEycyMDE3MzYyNzg4OTIxODU0NTMzNDEyMjI3NTYzOTE0ODM3MTI3ODYw +HhcNMjAxMDAxMTM0NzA4WhcNMjAxMDAxMTQ0NzA4WjAxMS8wLQYDVQQFEyY1Nzc2 +NTk2OTgwOTg4MTU4MzE3MzE4MTQ1NDkyODQ3MDAyNjYxMTBZMBMGByqGSM49AgEG +CCqGSM49AwEHA0IABL3+dJ2+Vgp/RFpVaL/pIHfHEidNwCA4ILLmzeRty1nF8cki +xb37KZ2qsnzhmRMWbzZsslPJIGN9waJiLHyhtRejSDBGMA4GA1UdDwEB/wQEAwIF +oDATBgNVHSUEDDAKBggrBgEFBQcDAjAfBgNVHSMEGDAWgBRCqpOXs1eIfE4DF7Wu +q2f6dfyx6DAKBggqhkjOPQQDAgNJADBGAiEAmKOQBDL7OPskAzvdDEn66SG39fZu +0/882H7hEahhq4UCIQCkpRYzXgWDiWWmlsgYR1xrq5NOF/qykeBFGk6j9VCh9w== +-----END CERTIFICATE----- diff --git a/orderer/consensus/etcdraft/util.go b/orderer/consensus/etcdraft/util.go index 51e49a6da78..9b0d4f216c1 100644 --- a/orderer/consensus/etcdraft/util.go +++ b/orderer/consensus/etcdraft/util.go @@ -37,8 +37,15 @@ func RaftPeers(consenterIDs []uint64) []raft.Peer { return peers } +type ConsentersMap map[string]struct{} + +func (c ConsentersMap) Exists(consenter *etcdraft.Consenter) bool { + _, exists := c[string(consenter.ClientTlsCert)] + return exists +} + // ConsentersToMap maps consenters into set where key is client TLS certificate -func ConsentersToMap(consenters []*etcdraft.Consenter) map[string]struct{} { +func ConsentersToMap(consenters []*etcdraft.Consenter) ConsentersMap { set := map[string]struct{}{} for _, c := range consenters { set[string(c.ClientTlsCert)] = struct{}{} @@ -197,8 +204,9 @@ func ConsensusMetadataFromConfigBlock(block *common.Block) (*etcdraft.ConfigMeta return MetadataFromConfigUpdate(configUpdate) } -// CheckConfigMetadata validates Raft config metadata -func CheckConfigMetadata(metadata *etcdraft.ConfigMetadata) error { +// VerifyConfigMetadata validates Raft config metadata. +// Note: ignores certificates expiration. +func VerifyConfigMetadata(metadata *etcdraft.ConfigMetadata, verifyOpts x509.VerifyOptions) error { if metadata == nil { // defensive check. this should not happen as CheckConfigMetadata // should always be called with non-nil config metadata @@ -233,15 +241,12 @@ func CheckConfigMetadata(metadata *etcdraft.ConfigMetadata) error { return errors.Errorf("empty consenter set") } - // sanity check of certificates + //verifying certificates for being signed by CA, expiration is ignored for _, consenter := range metadata.Consenters { if consenter == nil { return errors.Errorf("metadata has nil consenter") } - if err := validateCert(consenter.ServerTlsCert, "server"); err != nil { - return err - } - if err := validateCert(consenter.ClientTlsCert, "client"); err != nil { + if err := validateConsenterTLSCerts(consenter, verifyOpts, true); err != nil { return err } } @@ -253,16 +258,96 @@ func CheckConfigMetadata(metadata *etcdraft.ConfigMetadata) error { return nil } -func validateCert(pemData []byte, certRole string) error { - bl, _ := pem.Decode(pemData) +func parseCertificateFromBytes(cert []byte) (*x509.Certificate, error) { + pemBlock, _ := pem.Decode(cert) + if pemBlock == nil { + return &x509.Certificate{}, errors.Errorf("no PEM data found in cert[% x]", cert) + } - if bl == nil { - return errors.Errorf("%s TLS certificate is not PEM encoded: %s", certRole, string(pemData)) + certificate, err := x509.ParseCertificate(pemBlock.Bytes) + if err != nil { + return nil, errors.Errorf("%s TLS certificate has invalid ASN1 structure %s", err, string(pemBlock.Bytes)) } - if _, err := x509.ParseCertificate(bl.Bytes); err != nil { - return errors.Errorf("%s TLS certificate has invalid ASN1 structure, %v: %s", certRole, err, string(pemData)) + return certificate, nil +} + +func parseCertificateListFromBytes(certs [][]byte) ([]*x509.Certificate, error) { + var certificateList []*x509.Certificate + + for _, cert := range certs { + certificate, err := parseCertificateFromBytes(cert) + if err != nil { + return certificateList, err + } + + certificateList = append(certificateList, certificate) } + + return certificateList, nil +} + +func createX509VerifyOptions(ordererConfig channelconfig.Orderer) (x509.VerifyOptions, error) { + tlsRoots := x509.NewCertPool() + tlsIntermediates := x509.NewCertPool() + + for _, org := range ordererConfig.Organizations() { + rootCerts, err := parseCertificateListFromBytes(org.MSP().GetTLSRootCerts()) + if err != nil { + return x509.VerifyOptions{}, errors.Wrap(err, "parsing tls root certs") + } + intermediateCerts, err := parseCertificateListFromBytes(org.MSP().GetTLSIntermediateCerts()) + if err != nil { + return x509.VerifyOptions{}, errors.Wrap(err, "parsing tls intermediate certs") + } + + for _, cert := range rootCerts { + tlsRoots.AddCert(cert) + } + + for _, cert := range intermediateCerts { + tlsIntermediates.AddCert(cert) + } + } + + return x509.VerifyOptions{ + Roots: tlsRoots, + Intermediates: tlsIntermediates, + KeyUsages: []x509.ExtKeyUsage{ + x509.ExtKeyUsageClientAuth, + x509.ExtKeyUsageServerAuth, + }, + }, nil +} + +//validateConsenterTLSCerts decodes PEM cert, parses and validates it. +func validateConsenterTLSCerts(c *etcdraft.Consenter, opts x509.VerifyOptions, ignoreExpiration bool) error { + clientCert, err := parseCertificateFromBytes(c.ClientTlsCert) + if err != nil { + return errors.Wrapf(err, "parsing tls client cert of %s:%d", c.Host, c.Port) + } + + serverCert, err := parseCertificateFromBytes(c.ServerTlsCert) + if err != nil { + return errors.Wrapf(err, "parsing tls server cert of %s:%d", c.Host, c.Port) + } + + verify := func(certType string, cert *x509.Certificate, opts x509.VerifyOptions) error { + if _, err := cert.Verify(opts); err != nil { + if validationRes, ok := err.(x509.CertificateInvalidError); !ok || (!ignoreExpiration || validationRes.Reason != x509.Expired) { + return errors.Wrapf(err, "verifying tls %s cert with serial number %d", certType, cert.SerialNumber) + } + } + return nil + } + + if err := verify("client", clientCert, opts); err != nil { + return err + } + if err := verify("server", serverCert, opts); err != nil { + return err + } + return nil } diff --git a/orderer/consensus/etcdraft/util_test.go b/orderer/consensus/etcdraft/util_test.go index afcaefcdfb3..0d954650bb4 100644 --- a/orderer/consensus/etcdraft/util_test.go +++ b/orderer/consensus/etcdraft/util_test.go @@ -7,7 +7,9 @@ SPDX-License-Identifier: Apache-2.0 package etcdraft import ( + "crypto/x509" "encoding/base64" + "fmt" "io/ioutil" "path/filepath" "testing" @@ -24,6 +26,12 @@ import ( "github.com/stretchr/testify/require" ) +const ( + consentersTestDataDir = "testdata/consenters_certs/" + ca1Dir = consentersTestDataDir + "ca1" + ca2Dir = consentersTestDataDir + "ca2" +) + func TestIsConsenterOfChannel(t *testing.T) { certInsideConfigBlock, err := base64.StdEncoding.DecodeString("LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSUNmekNDQWlhZ0F3SUJBZ0l" + "SQUo4bjFLYTVzS1ZaTXRMTHJ1dldERDB3Q2dZSUtvWkl6ajBFQXdJd2JERUwKTUFrR0ExVUVCaE1DVlZNeEV6QVJCZ05WQkFnVENrTmhiR" + @@ -118,21 +126,52 @@ func TestIsConsenterOfChannel(t *testing.T) { } } -func TestCheckConfigMetadata(t *testing.T) { +func TestVerifyConfigMetadata(t *testing.T) { tlsCA, err := tlsgen.NewCA() if err != nil { panic(err) } + + caRootCert, err := parseCertificateFromBytes(tlsCA.CertBytes()) + if err != nil { + panic(err) + } + serverPair, err := tlsCA.NewServerCertKeyPair("localhost") - serverCert := serverPair.Cert if err != nil { panic(err) } + clientPair, err := tlsCA.NewClientCertKeyPair() - clientCert := clientPair.Cert if err != nil { panic(err) } + + unknownTlsCA, err := tlsgen.NewCA() + if err != nil { + panic(err) + } + + unknownServerPair, err := unknownTlsCA.NewServerCertKeyPair("unknownhost") + if err != nil { + panic(err) + } + + unknownServerCert, err := parseCertificateFromBytes(unknownServerPair.Cert) + if err != nil { + panic(err) + } + + unknownClientPair, err := unknownTlsCA.NewClientCertKeyPair() + if err != nil { + panic(err) + } + + unknownClientCert, err := parseCertificateFromBytes(unknownClientPair.Cert) + if err != nil { + panic(err) + } + validOptions := &etcdraftproto.Options{ TickInterval: "500ms", ElectionTick: 10, @@ -143,8 +182,18 @@ func TestCheckConfigMetadata(t *testing.T) { singleConsenter := &etcdraftproto.Consenter{ Host: "host1", Port: 10001, - ClientTlsCert: clientCert, - ServerTlsCert: serverCert, + ClientTlsCert: clientPair.Cert, + ServerTlsCert: serverPair.Cert, + } + + rootCertPool := x509.NewCertPool() + rootCertPool.AddCert(caRootCert) + goodVerifyingOpts := x509.VerifyOptions{ + Roots: rootCertPool, + KeyUsages: []x509.ExtKeyUsage{ + x509.ExtKeyUsageClientAuth, + x509.ExtKeyUsageServerAuth, + }, } // valid metadata should give nil error @@ -154,22 +203,25 @@ func TestCheckConfigMetadata(t *testing.T) { singleConsenter, }, } - assert.Nil(t, CheckConfigMetadata(goodMetadata)) + assert.Nil(t, VerifyConfigMetadata(goodMetadata, goodVerifyingOpts)) // test variety of bad metadata for _, testCase := range []struct { description string metadata *etcdraftproto.ConfigMetadata + verifyOpts x509.VerifyOptions errRegex string }{ { description: "nil metadata", metadata: nil, errRegex: "nil Raft config metadata", + verifyOpts: goodVerifyingOpts, }, { description: "nil options", metadata: &etcdraftproto.ConfigMetadata{}, + verifyOpts: goodVerifyingOpts, errRegex: "nil Raft config metadata options", }, { @@ -179,7 +231,8 @@ func TestCheckConfigMetadata(t *testing.T) { HeartbeatTick: 0, }, }, - errRegex: "none of HeartbeatTick .* can be zero", + verifyOpts: goodVerifyingOpts, + errRegex: "none of HeartbeatTick .* can be zero", }, { description: "ElectionTick is 0", @@ -189,7 +242,8 @@ func TestCheckConfigMetadata(t *testing.T) { ElectionTick: 0, }, }, - errRegex: "none of .* ElectionTick .* can be zero", + verifyOpts: goodVerifyingOpts, + errRegex: "none of .* ElectionTick .* can be zero", }, { description: "MaxInflightBlocks is 0", @@ -200,7 +254,8 @@ func TestCheckConfigMetadata(t *testing.T) { MaxInflightBlocks: 0, }, }, - errRegex: "none of .* MaxInflightBlocks .* can be zero", + verifyOpts: goodVerifyingOpts, + errRegex: "none of .* MaxInflightBlocks .* can be zero", }, { description: "ElectionTick is less than HeartbeatTick", @@ -211,7 +266,8 @@ func TestCheckConfigMetadata(t *testing.T) { MaxInflightBlocks: validOptions.MaxInflightBlocks, }, }, - errRegex: "ElectionTick .* must be greater than HeartbeatTick", + verifyOpts: goodVerifyingOpts, + errRegex: "ElectionTick .* must be greater than HeartbeatTick", }, { description: "TickInterval is not parsable", @@ -223,7 +279,8 @@ func TestCheckConfigMetadata(t *testing.T) { TickInterval: "abcd", }, }, - errRegex: "failed to parse TickInterval .* to time duration", + verifyOpts: goodVerifyingOpts, + errRegex: "failed to parse TickInterval .* to time duration", }, { description: "TickInterval is 0", @@ -235,7 +292,8 @@ func TestCheckConfigMetadata(t *testing.T) { TickInterval: "0s", }, }, - errRegex: "TickInterval cannot be zero", + verifyOpts: goodVerifyingOpts, + errRegex: "TickInterval cannot be zero", }, { description: "consenter set is empty", @@ -243,7 +301,8 @@ func TestCheckConfigMetadata(t *testing.T) { Options: validOptions, Consenters: []*etcdraftproto.Consenter{}, }, - errRegex: "empty consenter set", + verifyOpts: goodVerifyingOpts, + errRegex: "empty consenter set", }, { description: "metadata has nil consenter", @@ -253,7 +312,8 @@ func TestCheckConfigMetadata(t *testing.T) { nil, }, }, - errRegex: "metadata has nil consenter", + verifyOpts: goodVerifyingOpts, + errRegex: "metadata has nil consenter", }, { description: "consenter has invalid server cert", @@ -262,11 +322,12 @@ func TestCheckConfigMetadata(t *testing.T) { Consenters: []*etcdraftproto.Consenter{ { ServerTlsCert: []byte("invalid"), - ClientTlsCert: clientCert, + ClientTlsCert: clientPair.Cert, }, }, }, - errRegex: "server TLS certificate is not PEM encoded", + verifyOpts: goodVerifyingOpts, + errRegex: "no PEM data found in cert", }, { description: "consenter has invalid client cert", @@ -274,12 +335,13 @@ func TestCheckConfigMetadata(t *testing.T) { Options: validOptions, Consenters: []*etcdraftproto.Consenter{ { - ServerTlsCert: serverCert, + ServerTlsCert: serverPair.Cert, ClientTlsCert: []byte("invalid"), }, }, }, - errRegex: "client TLS certificate is not PEM encoded", + verifyOpts: goodVerifyingOpts, + errRegex: "no PEM data found in cert", }, { description: "metadata has duplicate consenters", @@ -290,11 +352,69 @@ func TestCheckConfigMetadata(t *testing.T) { singleConsenter, }, }, - errRegex: "duplicate consenter", + verifyOpts: goodVerifyingOpts, + errRegex: "duplicate consenter", + }, + { + description: "consenter has client cert signed by unknown authority", + metadata: &etcdraftproto.ConfigMetadata{ + Options: validOptions, + Consenters: []*etcdraftproto.Consenter{ + { + ClientTlsCert: unknownClientPair.Cert, + ServerTlsCert: serverPair.Cert, + }, + }, + }, + verifyOpts: goodVerifyingOpts, + errRegex: fmt.Sprintf("verifying tls client cert with serial number %d: x509: certificate signed by unknown authority", unknownClientCert.SerialNumber), + }, + { + description: "consenter has server cert signed by unknown authority", + metadata: &etcdraftproto.ConfigMetadata{ + Options: validOptions, + Consenters: []*etcdraftproto.Consenter{ + { + ServerTlsCert: unknownServerPair.Cert, + ClientTlsCert: clientPair.Cert, + }, + }, + }, + verifyOpts: goodVerifyingOpts, + errRegex: fmt.Sprintf("verifying tls server cert with serial number %d: x509: certificate signed by unknown authority", unknownServerCert.SerialNumber), }, } { - err := CheckConfigMetadata(testCase.metadata) - assert.NotNil(t, err, testCase.description) - assert.Regexp(t, testCase.errRegex, err) + t.Run(testCase.description, func(t *testing.T) { + err := VerifyConfigMetadata(testCase.metadata, testCase.verifyOpts) + assert.NotNil(t, err) + assert.Regexp(t, testCase.errRegex, err) + }) } + + //test use case when consenter has expired certificates + tlsCaCertBytes, err := ioutil.ReadFile(filepath.Join(ca1Dir, "ca.pem")) + assert.Nil(t, err) + tlsCaCert, err := parseCertificateFromBytes(tlsCaCertBytes) + assert.Nil(t, err) + + tlsClientCert, err := ioutil.ReadFile(filepath.Join(ca1Dir, "expired-client.pem")) + assert.Nil(t, err) + + expiredCertVerifyOpts := goodVerifyingOpts + expiredCertVerifyOpts.Roots.AddCert(tlsCaCert) + consenterWithExpiredCerts := &etcdraftproto.Consenter{ + Host: "host1", + Port: 10001, + ClientTlsCert: tlsClientCert, + ServerTlsCert: tlsClientCert, + } + + metadataWithExpiredConsenter := &etcdraftproto.ConfigMetadata{ + Options: validOptions, + Consenters: []*etcdraftproto.Consenter{ + consenterWithExpiredCerts, + }, + } + + assert.Nil(t, VerifyConfigMetadata(metadataWithExpiredConsenter, expiredCertVerifyOpts)) } diff --git a/orderer/consensus/etcdraft/validator_test.go b/orderer/consensus/etcdraft/validator_test.go index 2228b45a109..01d97a0815a 100644 --- a/orderer/consensus/etcdraft/validator_test.go +++ b/orderer/consensus/etcdraft/validator_test.go @@ -7,6 +7,8 @@ package etcdraft_test import ( + "github.com/hyperledger/fabric/common/channelconfig" + "github.com/hyperledger/fabric/orderer/consensus/etcdraft/mocks" "io/ioutil" "time" @@ -22,6 +24,14 @@ import ( . "github.com/onsi/gomega" ) +func makeOrdererOrg(caCert []byte) *mocks.OrdererOrg { + ordererOrg := &mocks.OrdererOrg{} + mockMSP := &mocks.MSP{} + mockMSP.GetTLSRootCertsReturns([][]byte{caCert}) + ordererOrg.MSPReturns(mockMSP) + return ordererOrg +} + var _ = Describe("Metadata Validation", func() { var ( chain *etcdraft.Chain @@ -78,38 +88,50 @@ var _ = Describe("Metadata Validation", func() { }) When("determining parameter well-formedness", func() { - It("succeeds when new consensus metadata is nil", func() { - Expect(chain.ValidateConsensusMetadata(nil, nil, false)).To(Succeed()) + It("succeeds when new orderer config is nil", func() { + Expect(chain.ValidateConsensusMetadata(mockOrderer(nil), mockOrderer(nil), false)).To(Succeed()) }) - It("fails when new consensus metadata is not nil while old consensus metadata is nil", func() { + It("fails when new orderer config is not nil while old orderer config is nil", func() { + newOrdererConf := mockOrderer([]byte("test")) Expect(func() { - chain.ValidateConsensusMetadata(nil, []byte("test"), false) + chain.ValidateConsensusMetadata(nil, newOrdererConf, false) + }).To(Panic()) + }) + + It("fails when new orderer config is not nil while old config metadata is nil", func() { + newOrdererConf := mockOrderer([]byte("test")) + Expect(func() { + chain.ValidateConsensusMetadata(mockOrderer(nil), newOrdererConf, false) }).To(Panic()) }) It("fails when old consensus metadata is not well-formed", func() { + oldOrdererConf := mockOrderer([]byte("test")) + newOrdererConf := mockOrderer([]byte("test")) Expect(func() { - chain.ValidateConsensusMetadata([]byte("test"), []byte("test"), false) + chain.ValidateConsensusMetadata(oldOrdererConf, newOrdererConf, false) }).To(Panic()) }) It("fails when new consensus metadata is not well-formed", func() { oldBytes, _ := proto.Marshal(&etcdraftproto.ConfigMetadata{}) - Expect(chain.ValidateConsensusMetadata(oldBytes, []byte("test"), false)).NotTo(Succeed()) + oldOrdererConf := mockOrderer(oldBytes) + newOrdererConf := mockOrderer([]byte("test")) + Expect(chain.ValidateConsensusMetadata(oldOrdererConf, newOrdererConf, false)).NotTo(Succeed()) }) }) Context("valid old consensus metadata", func() { var ( - oldBytes []byte - oldMetadata *etcdraftproto.ConfigMetadata - newMetadata *etcdraftproto.ConfigMetadata - newChannel bool + metadata etcdraftproto.ConfigMetadata + oldOrdererConfig *mocks.OrdererConfig + newOrdererConfig *mocks.OrdererConfig + newChannel bool ) BeforeEach(func() { - oldMetadata = &etcdraftproto.ConfigMetadata{ + metadata = etcdraftproto.ConfigMetadata{ Options: &etcdraftproto.Options{ TickInterval: "500ms", ElectionTick: 10, @@ -138,16 +160,31 @@ var _ = Describe("Metadata Validation", func() { }, }, } - newMetadata = oldMetadata - oldBytes, _ = proto.Marshal(oldMetadata) + + oldBytes, err := proto.Marshal(&metadata) + Expect(err).NotTo(HaveOccurred()) + oldOrdererConfig = mockOrderer(oldBytes) + org1 := makeOrdererOrg(tlsCA.CertBytes()) + oldOrdererConfig.OrganizationsReturns(map[string]channelconfig.OrdererOrg{ + "org1": org1, + }) + + newOrdererConfig = mockOrderer(oldBytes) + newOrdererConfig.OrganizationsReturns(map[string]channelconfig.OrdererOrg{ + "org1": org1, + }) + newChannel = false }) It("fails when new consensus metadata has invalid options", func() { // NOTE: we are not checking all failures here since tests for CheckConfigMetadata does that + newMetadata := metadata newMetadata.Options.TickInterval = "" - newBytes, _ := proto.Marshal(newMetadata) - Expect(chain.ValidateConsensusMetadata(oldBytes, newBytes, newChannel)).NotTo(Succeed()) + newBytes, err := proto.Marshal(&newMetadata) + Expect(err).NotTo(HaveOccurred()) + newOrdererConfig.ConsensusMetadataReturns(newBytes) + Expect(chain.ValidateConsensusMetadata(oldOrdererConfig, newOrdererConfig, newChannel)).NotTo(Succeed()) }) Context("new channel creation", func() { @@ -156,26 +193,53 @@ var _ = Describe("Metadata Validation", func() { }) It("fails when the new consenters are an empty set", func() { + newMetadata := metadata newMetadata.Consenters = []*etcdraftproto.Consenter{} - newBytes, _ := proto.Marshal(newMetadata) - Expect(chain.ValidateConsensusMetadata(oldBytes, newBytes, newChannel)).NotTo(Succeed()) + newBytes, err := proto.Marshal(&newMetadata) + Expect(err).NotTo(HaveOccurred()) + newOrdererConfig.ConsensusMetadataReturns(newBytes) + Expect(chain.ValidateConsensusMetadata(oldOrdererConfig, newOrdererConfig, newChannel)).NotTo(Succeed()) }) It("succeeds when the new consenters are the same as the existing consenters", func() { - newBytes, _ := proto.Marshal(newMetadata) - Expect(chain.ValidateConsensusMetadata(oldBytes, newBytes, newChannel)).To(Succeed()) + newMetadata := metadata + newBytes, err := proto.Marshal(&newMetadata) + Expect(err).NotTo(HaveOccurred()) + newOrdererConfig.ConsensusMetadataReturns(newBytes) + Expect(chain.ValidateConsensusMetadata(oldOrdererConfig, newOrdererConfig, newChannel)).To(Succeed()) }) It("succeeds when the new consenters are a subset of the existing consenters", func() { + newMetadata := metadata newMetadata.Consenters = newMetadata.Consenters[:2] - newBytes, _ := proto.Marshal(newMetadata) - Expect(chain.ValidateConsensusMetadata(oldBytes, newBytes, newChannel)).To(Succeed()) + newBytes, err := proto.Marshal(&newMetadata) + Expect(err).NotTo(HaveOccurred()) + newOrdererConfig.ConsensusMetadataReturns(newBytes) + Expect(chain.ValidateConsensusMetadata(oldOrdererConfig, newOrdererConfig, newChannel)).To(Succeed()) }) It("fails when the new consenters are not a subset of the existing consenters", func() { + newMetadata := metadata newMetadata.Consenters[2].ClientTlsCert = clientTLSCert(tlsCA) - newBytes, _ := proto.Marshal(newMetadata) - Expect(chain.ValidateConsensusMetadata(oldBytes, newBytes, newChannel)).NotTo(Succeed()) + newBytes, err := proto.Marshal(&newMetadata) + Expect(err).NotTo(HaveOccurred()) + newOrdererConfig.ConsensusMetadataReturns(newBytes) + Expect(chain.ValidateConsensusMetadata(oldOrdererConfig, newOrdererConfig, newChannel)).NotTo(Succeed()) + }) + + It("fails when the new consenter has certificate which not signed by any CA of an orderer org", func() { + anotherCa, err := tlsgen.NewCA() + Expect(err).NotTo(HaveOccurred()) + newMetadata := metadata + newMetadata.Consenters[2].ClientTlsCert = clientTLSCert(anotherCa) + newBytes, err := proto.Marshal(&newMetadata) + Expect(err).NotTo(HaveOccurred()) + newOrdererConfig.ConsensusMetadataReturns(newBytes) + Expect(chain.ValidateConsensusMetadata(oldOrdererConfig, newOrdererConfig, newChannel)).NotTo(Succeed()) + }) + + It("succeeds when the new consenters are a subset of the system consenters and certificates signed by MSP participant on a channel", func() { + Expect(chain.ValidateConsensusMetadata(oldOrdererConfig, newOrdererConfig, newChannel)).To(Succeed()) }) }) @@ -186,29 +250,39 @@ var _ = Describe("Metadata Validation", func() { }) It("fails when the new consenters are an empty set", func() { + newMetadata := metadata // NOTE: This also takes care of the case when we remove node from a singleton consenter set newMetadata.Consenters = []*etcdraftproto.Consenter{} - newBytes, _ := proto.Marshal(newMetadata) - Expect(chain.ValidateConsensusMetadata(oldBytes, newBytes, newChannel)).NotTo(Succeed()) + newBytes, err := proto.Marshal(&newMetadata) + Expect(err).NotTo(HaveOccurred()) + newOrdererConfig.ConsensusMetadataReturns(newBytes) + Expect(chain.ValidateConsensusMetadata(oldOrdererConfig, newOrdererConfig, newChannel)).NotTo(Succeed()) }) It("succeeds when the new consenters are the same as the existing consenters", func() { - newBytes, _ := proto.Marshal(newMetadata) - Expect(chain.ValidateConsensusMetadata(oldBytes, newBytes, newChannel)).To(Succeed()) + newMetadata := metadata + newBytes, err := proto.Marshal(&newMetadata) + Expect(err).NotTo(HaveOccurred()) + newOrdererConfig.ConsensusMetadataReturns(newBytes) + Expect(chain.ValidateConsensusMetadata(oldOrdererConfig, newOrdererConfig, newChannel)).To(Succeed()) }) It("succeeds on addition of a single consenter", func() { + newMetadata := metadata newMetadata.Consenters = append(newMetadata.Consenters, &etcdraftproto.Consenter{ Host: "host4", Port: 10004, ClientTlsCert: clientTLSCert(tlsCA), ServerTlsCert: serverTLSCert(tlsCA), }) - newBytes, _ := proto.Marshal(newMetadata) - Expect(chain.ValidateConsensusMetadata(oldBytes, newBytes, newChannel)).To(Succeed()) + newBytes, err := proto.Marshal(&newMetadata) + Expect(err).NotTo(HaveOccurred()) + newOrdererConfig.ConsensusMetadataReturns(newBytes) + Expect(chain.ValidateConsensusMetadata(oldOrdererConfig, newOrdererConfig, newChannel)).To(Succeed()) }) It("fails on addition of more than one consenter", func() { + newMetadata := metadata newMetadata.Consenters = append(newMetadata.Consenters, &etcdraftproto.Consenter{ Host: "host4", @@ -223,45 +297,62 @@ var _ = Describe("Metadata Validation", func() { ServerTlsCert: serverTLSCert(tlsCA), }, ) - newBytes, _ := proto.Marshal(newMetadata) - Expect(chain.ValidateConsensusMetadata(oldBytes, newBytes, newChannel)).NotTo(Succeed()) + newBytes, err := proto.Marshal(&newMetadata) + Expect(err).NotTo(HaveOccurred()) + newOrdererConfig.ConsensusMetadataReturns(newBytes) + Expect(chain.ValidateConsensusMetadata(oldOrdererConfig, newOrdererConfig, newChannel)).NotTo(Succeed()) }) It("succeeds on removal of a single consenter", func() { + newMetadata := metadata newMetadata.Consenters = newMetadata.Consenters[:2] - newBytes, _ := proto.Marshal(newMetadata) - Expect(chain.ValidateConsensusMetadata(oldBytes, newBytes, newChannel)).To(Succeed()) + newBytes, err := proto.Marshal(&newMetadata) + Expect(err).NotTo(HaveOccurred()) + newOrdererConfig.ConsensusMetadataReturns(newBytes) + Expect(chain.ValidateConsensusMetadata(oldOrdererConfig, newOrdererConfig, newChannel)).To(Succeed()) }) It("fails on removal of more than one consenter", func() { + newMetadata := metadata newMetadata.Consenters = newMetadata.Consenters[:1] - newBytes, _ := proto.Marshal(newMetadata) - Expect(chain.ValidateConsensusMetadata(oldBytes, newBytes, newChannel)).NotTo(Succeed()) + newBytes, err := proto.Marshal(&newMetadata) + Expect(err).NotTo(HaveOccurred()) + newOrdererConfig.ConsensusMetadataReturns(newBytes) + Expect(chain.ValidateConsensusMetadata(oldOrdererConfig, newOrdererConfig, newChannel)).NotTo(Succeed()) }) It("succeeds on rotating certs in case of both addition and removal of a node each to reuse the raft NodeId", func() { + newMetadata := metadata newMetadata.Consenters = append(newMetadata.Consenters[:2], &etcdraftproto.Consenter{ Host: "host4", Port: 10004, ClientTlsCert: clientTLSCert(tlsCA), ServerTlsCert: serverTLSCert(tlsCA), }) - newBytes, _ := proto.Marshal(newMetadata) - Expect(chain.ValidateConsensusMetadata(oldBytes, newBytes, newChannel)).To(Succeed()) + newBytes, err := proto.Marshal(&newMetadata) + Expect(err).NotTo(HaveOccurred()) + newOrdererConfig.ConsensusMetadataReturns(newBytes) + Expect(chain.ValidateConsensusMetadata(oldOrdererConfig, newOrdererConfig, newChannel)).To(Succeed()) }) It("succeeds on removal of inactive node in 2/3 cluster", func() { chain.ActiveNodes.Store([]uint64{1, 2}) + newMetadata := metadata newMetadata.Consenters = newMetadata.Consenters[:2] - newBytes, _ := proto.Marshal(newMetadata) - Expect(chain.ValidateConsensusMetadata(oldBytes, newBytes, newChannel)).To(Succeed()) + newBytes, err := proto.Marshal(&newMetadata) + Expect(err).NotTo(HaveOccurred()) + newOrdererConfig.ConsensusMetadataReturns(newBytes) + Expect(chain.ValidateConsensusMetadata(oldOrdererConfig, newOrdererConfig, newChannel)).To(Succeed()) }) It("fails on removal of active node in 2/3 cluster", func() { chain.ActiveNodes.Store([]uint64{1, 2}) + newMetadata := metadata newMetadata.Consenters = newMetadata.Consenters[1:] - newBytes, _ := proto.Marshal(newMetadata) - Expect(chain.ValidateConsensusMetadata(oldBytes, newBytes, newChannel)).To( + newBytes, err := proto.Marshal(&newMetadata) + Expect(err).NotTo(HaveOccurred()) + newOrdererConfig.ConsensusMetadataReturns(newBytes) + Expect(chain.ValidateConsensusMetadata(oldOrdererConfig, newOrdererConfig, newChannel)).To( MatchError("2 out of 3 nodes are alive, configuration will result in quorum loss")) }) @@ -294,9 +385,12 @@ var _ = Describe("Metadata Validation", func() { It("succeeds on removal of inactive node in 2/3 cluster", func() { chain.ActiveNodes.Store([]uint64{2, 3}) // 4 is inactive + newMetadata := metadata newMetadata.Consenters = newMetadata.Consenters[:2] - newBytes, _ := proto.Marshal(newMetadata) - Expect(chain.ValidateConsensusMetadata(oldBytes, newBytes, newChannel)).To(Succeed()) + newBytes, err := proto.Marshal(&newMetadata) + Expect(err).NotTo(HaveOccurred()) + newOrdererConfig.ConsensusMetadataReturns(newBytes) + Expect(chain.ValidateConsensusMetadata(oldOrdererConfig, newOrdererConfig, newChannel)).To(Succeed()) }) }) }) diff --git a/release_notes/v2.2.2.md b/release_notes/v2.2.2.md index 1d66e39bf0a..8cef38eb6e3 100644 --- a/release_notes/v2.2.2.md +++ b/release_notes/v2.2.2.md @@ -8,10 +8,17 @@ What's New in Hyperledger Fabric v2.2.2 Fixes ----- -**FAB-XXXX: component - fix title** - -Fix description - +**FAB-18192: orderer certificate update - consenter cert validation fails when +MSP is not part of existing configuration** + +v2.2.0 introduced validation of the consenter certificates against the orderer +MSP configuration when adding a node to the consenters set. However, it +did not allow for updating the consenters set using new MSP configuration +included in a single config update and required two separate updates - one for +the MSP configuration and then one for the consenter that referenced the new +MSP configuration. This fix resolves this by validating new consenter certs +against the simulated config to ensure it factors in any simultaneous +config updates. Deprecations (existing) -----------------------