Commit f9975970 authored by Jay Guo's avatar Jay Guo
Browse files

FAB-14840 check Raft config for HeaderType_CONFIG



Raft config metadata checks, e.g. non-zero TickInterval, should also
be done against HeaderType_CONFIG transactions.

Also, since we check against nil consenter set, this also prevents
user from removing node from single node cluster, which results in
dead channel.

Change-Id: Idf0967ed19ffed964f16c83a9862c1c59e132686
Signed-off-by: default avatarJay Guo <guojiannan1101@gmail.com>
parent 7d06d382
......@@ -423,8 +423,12 @@ func (c *Chain) checkConfigUpdateValidity(ctx *common.Envelope) error {
return err
}
switch chdr.Type {
case int32(common.HeaderType_ORDERER_TRANSACTION):
if chdr.Type != int32(common.HeaderType_ORDERER_TRANSACTION) &&
chdr.Type != int32(common.HeaderType_CONFIG) {
return errors.Errorf("config transaction has unknown header type: %s", common.HeaderType(chdr.Type))
}
if chdr.Type == int32(common.HeaderType_ORDERER_TRANSACTION) {
newChannelConfig, err := utils.UnmarshalEnvelope(payload.Data)
if err != nil {
return err
......@@ -434,46 +438,53 @@ func (c *Chain) checkConfigUpdateValidity(ctx *common.Envelope) error {
if err != nil {
return err
}
}
configUpdate, err := configtx.UnmarshalConfigUpdateFromPayload(payload)
if err != nil {
return err
}
configUpdate, err := configtx.UnmarshalConfigUpdateFromPayload(payload)
if err != nil {
return err
}
metadata, err := MetadataFromConfigUpdate(configUpdate)
if err != nil {
return err
}
metadata, err := MetadataFromConfigUpdate(configUpdate)
if err != nil {
return err
}
if metadata == nil {
return nil // ConsensusType is not updated
}
if metadata == nil {
return nil // ConsensusType is not updated
}
if err = CheckConfigMetadata(metadata); err != nil {
return err
}
switch chdr.Type {
case int32(common.HeaderType_ORDERER_TRANSACTION):
c.raftMetadataLock.RLock()
set := MembershipByCert(c.opts.Consenters)
c.raftMetadataLock.RUnlock()
return CheckConfigMetadata(metadata, set)
case int32(common.HeaderType_CONFIG):
configUpdate, err := configtx.UnmarshalConfigUpdateFromPayload(payload)
if err != nil {
return err
}
metadata, err := MetadataFromConfigUpdate(configUpdate)
if err != nil {
return err
for _, c := range metadata.Consenters {
if _, exits := set[string(c.ClientTlsCert)]; !exits {
return errors.Errorf("new channel has consenter that is not part of system consenter set")
}
}
if metadata != nil {
return c.checkConsentersSet(metadata)
}
return nil
case int32(common.HeaderType_CONFIG):
c.raftMetadataLock.RLock()
_, err = ComputeMembershipChanges(c.opts.BlockMetadata, c.opts.Consenters, metadata.Consenters)
c.raftMetadataLock.RUnlock()
return err
default:
return errors.Errorf("config transaction has unknown header type")
// panic here because we have just check header type and return early
c.logger.Panicf("Programming error, unknown header type")
}
return nil
}
// WaitReady blocks when the chain:
......@@ -1215,31 +1226,6 @@ func (c *Chain) pemToDER(pemBytes []byte, id uint64, certType string) ([]byte, e
return bl.Bytes, nil
}
// checkConsentersSet validates correctness of the consenters set provided within configuration value
func (c *Chain) checkConsentersSet(updatedMetadata *etcdraft.ConfigMetadata) error {
// sanity check of certificates
for _, consenter := range updatedMetadata.Consenters {
if bl, _ := pem.Decode(consenter.ServerTlsCert); bl == nil {
return errors.Errorf("invalid server TLS cert: %s", string(consenter.ServerTlsCert))
}
if bl, _ := pem.Decode(consenter.ClientTlsCert); bl == nil {
return errors.Errorf("invalid client TLS cert: %s", string(consenter.ClientTlsCert))
}
}
var err error
if err = MetadataHasDuplication(updatedMetadata); err != nil {
return err
}
c.raftMetadataLock.RLock()
_, err = ComputeMembershipChanges(c.opts.BlockMetadata, c.opts.Consenters, updatedMetadata.Consenters)
c.raftMetadataLock.RUnlock()
return err
}
// writeConfigBlock writes configuration blocks into the ledger in
// addition extracts updates about raft replica set and if there
// are changes updates cluster membership as well
......
......@@ -472,7 +472,7 @@ var _ = Describe("Chain", func() {
It("should throw an error", func() {
err := chain.Configure(configEnv, configSeq)
Expect(err).To(MatchError("config transaction has unknown header type"))
Expect(err).To(MatchError("config transaction has unknown header type: CONFIG_UPDATE"))
Expect(fakeFields.fakeConfigProposalsReceived.AddCallCount()).To(Equal(1))
Expect(fakeFields.fakeConfigProposalsReceived.AddArgsForCall(0)).To(Equal(float64(1)))
Expect(fakeFields.fakeProposalFailures.AddCallCount()).To(Equal(1))
......@@ -700,7 +700,7 @@ var _ = Describe("Chain", func() {
Expect(err).NotTo(HaveOccurred())
})
It("should be able to process config update removing single node", func() {
It("should not be able to remove node from single node cluster", func() {
metadata := proto.Clone(consenterMetadata).(*raftprotos.ConfigMetadata)
// Remove one of the consenters
metadata.Consenters = metadata.Consenters[1:]
......@@ -717,8 +717,7 @@ var _ = Describe("Chain", func() {
newConfigUpdateEnv(channelID, nil, values))
configSeq = 0
err := chain.Configure(configEnv, configSeq)
Expect(err).NotTo(HaveOccurred())
Expect(chain.Configure(configEnv, configSeq)).To(MatchError("empty consenter set"))
})
})
})
......@@ -1376,7 +1375,16 @@ var _ = Describe("Chain", func() {
},
}
metadata := &raftprotos.ConfigMetadata{Consenters: []*raftprotos.Consenter{consenters[2]}}
metadata := &raftprotos.ConfigMetadata{
Options: &raftprotos.Options{
TickInterval: "500ms",
ElectionTick: 10,
HeartbeatTick: 1,
MaxInflightBlocks: 5,
SnapshotIntervalSize: 200,
},
Consenters: []*raftprotos.Consenter{consenters[2]},
}
value := map[string]*common.ConfigValue{
"ConsensusType": {
Version: 1,
......@@ -1700,7 +1708,7 @@ var _ = Describe("Chain", func() {
}
Expect(c1.Configure(createChannelEnv(metadata), 0)).To(MatchError(
"none of the fields in Raft config option can be zero"))
"none of HeartbeatTick (1), ElectionTick (0) and MaxInflightBlocks (5) can be zero"))
})
It("fails with zero max inflgith blocks", func() {
......@@ -1711,7 +1719,7 @@ var _ = Describe("Chain", func() {
}
Expect(c1.Configure(createChannelEnv(metadata), 0)).To(MatchError(
"none of the fields in Raft config option can be zero"))
"none of HeartbeatTick (1), ElectionTick (10) and MaxInflightBlocks (0) can be zero"))
})
It("fails with invalid tick interval", func() {
......@@ -1740,7 +1748,7 @@ var _ = Describe("Chain", func() {
metadata := &raftprotos.ConfigMetadata{Options: options}
Expect(c1.Configure(createChannelEnv(metadata), 0)).To(MatchError(
"cannot create channel with empty consenter set"))
"empty consenter set"))
})
It("fails with invalid certificate", func() {
......@@ -1775,7 +1783,7 @@ var _ = Describe("Chain", func() {
Context("reconfiguration", func() {
It("cannot change consenter set by more than 1 node", func() {
metadata := &raftprotos.ConfigMetadata{}
metadata := &raftprotos.ConfigMetadata{Options: options}
for id, consenter := range consenters {
if id == 2 || id == 3 {
// remove second & third consenter
......@@ -1803,7 +1811,7 @@ var _ = Describe("Chain", func() {
})
It("rejects invalid certificates", func() {
configMetadata := &raftprotos.ConfigMetadata{}
configMetadata := &raftprotos.ConfigMetadata{Options: options}
for _, consenter := range consenters {
configMetadata.Consenters = append(configMetadata.Consenters, consenter)
}
......@@ -1826,7 +1834,7 @@ var _ = Describe("Chain", func() {
})
It("can rotate certificate by adding and removing 1 node in one config update", func() {
metadata := &raftprotos.ConfigMetadata{}
metadata := &raftprotos.ConfigMetadata{Options: options}
for id, consenter := range consenters {
if id == 2 {
// remove second consenter
......@@ -1866,7 +1874,7 @@ var _ = Describe("Chain", func() {
})
It("rotates leader certificate and triggers leadership transfer", func() {
metadata := &raftprotos.ConfigMetadata{}
metadata := &raftprotos.ConfigMetadata{Options: options}
for id, consenter := range consenters {
if id == 1 {
// remove second consenter
......@@ -1908,7 +1916,7 @@ var _ = Describe("Chain", func() {
When("Leader is disconnected after cert rotation", func() {
It("still configures communication after failed leader transfer attempt", func() {
metadata := &raftprotos.ConfigMetadata{}
metadata := &raftprotos.ConfigMetadata{Options: options}
for id, consenter := range consenters {
if id == 1 {
// remove second consenter
......@@ -1964,7 +1972,7 @@ var _ = Describe("Chain", func() {
When("Follower is disconnected while leader cert is being rotated", func() {
It("still configures communication and transfer leader", func() {
metadata := &raftprotos.ConfigMetadata{}
metadata := &raftprotos.ConfigMetadata{Options: options}
for id, consenter := range consenters {
if id == 1 {
// remove second consenter
......@@ -2119,7 +2127,7 @@ var _ = Describe("Chain", func() {
c1.cutter.CutNext = true
consensusType := &orderer.ConsensusType{}
proto.Unmarshal(addConsenterUpdate["ConsensusType"].Value, consensusType)
duplicatedMetadata := &raftprotos.ConfigMetadata{}
duplicatedMetadata := &raftprotos.ConfigMetadata{Options: options}
proto.Unmarshal(consensusType.Metadata, duplicatedMetadata)
duplicatedMetadata.Consenters = append(duplicatedMetadata.Consenters, duplicatedMetadata.Consenters[1])
configEnv = newConfigEnv(channelID, common.HeaderType_CONFIG, newConfigUpdateEnv(channelID, nil, map[string]*common.ConfigValue{
......@@ -3356,7 +3364,12 @@ func nodeConfigFromMetadata(consenterMetadata *raftprotos.ConfigMetadata) []clus
}
func createMetadata(nodeCount int, tlsCA tlsgen.CA) *raftprotos.ConfigMetadata {
md := &raftprotos.ConfigMetadata{}
md := &raftprotos.ConfigMetadata{Options: &raftprotos.Options{
TickInterval: time.Duration(interval).String(),
ElectionTick: ELECTION_TICK,
HeartbeatTick: HEARTBEAT_TICK,
MaxInflightBlocks: 5,
}}
for i := 0; i < nodeCount; i++ {
md.Consenters = append(md.Consenters, &raftprotos.Consenter{
Host: "localhost",
......
......@@ -383,7 +383,7 @@ func ConsensusMetadataFromConfigBlock(block *common.Block) (*etcdraft.ConfigMeta
}
// CheckConfigMetadata validates Raft config metadata
func CheckConfigMetadata(metadata *etcdraft.ConfigMetadata, consenters map[string]uint64) error {
func CheckConfigMetadata(metadata *etcdraft.ConfigMetadata) error {
if metadata == nil {
// defensive check. this should not happen as CheckConfigMetadata
// should always be called with non-nil config metadata
......@@ -393,7 +393,9 @@ func CheckConfigMetadata(metadata *etcdraft.ConfigMetadata, consenters map[strin
if metadata.Options.HeartbeatTick == 0 ||
metadata.Options.ElectionTick == 0 ||
metadata.Options.MaxInflightBlocks == 0 {
return errors.Errorf("none of the fields in Raft config option can be zero")
// if SnapshotIntervalSize is zero, DefaultSnapshotIntervalSize is used
return errors.Errorf("none of HeartbeatTick (%d), ElectionTick (%d) and MaxInflightBlocks (%d) can be zero",
metadata.Options.HeartbeatTick, metadata.Options.ElectionTick, metadata.Options.MaxInflightBlocks)
}
// check Raft options
......@@ -409,7 +411,7 @@ func CheckConfigMetadata(metadata *etcdraft.ConfigMetadata, consenters map[strin
}
if len(metadata.Consenters) == 0 {
return errors.Errorf("cannot create channel with empty consenter set")
return errors.Errorf("empty consenter set")
}
// sanity check of certificates
......@@ -427,12 +429,6 @@ func CheckConfigMetadata(metadata *etcdraft.ConfigMetadata, consenters map[strin
return err
}
for _, c := range metadata.Consenters {
if _, exits := consenters[string(c.ClientTlsCert)]; !exits {
return errors.Errorf("new channel has consenter that is not part of system consenter set")
}
}
return nil
}
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment