diff --git a/common/configtx/manager.go b/common/configtx/manager.go index 4c268f67d9d..8ad58a9265a 100644 --- a/common/configtx/manager.go +++ b/common/configtx/manager.go @@ -19,17 +19,29 @@ package configtx import ( "fmt" "reflect" + "regexp" "github.com/hyperledger/fabric/common/policies" cb "github.com/hyperledger/fabric/protos/common" "errors" + "github.com/golang/protobuf/proto" logging "github.com/op/go-logging" ) var logger = logging.MustGetLogger("common/configtx") +// Constraints for valid chain IDs +var ( + allowedChars = "[a-zA-Z0-9.-]+" + maxLength = 249 + illegalNames = map[string]struct{}{ + ".": struct{}{}, + "..": struct{}{}, + } +) + // Handler provides a hook which allows other pieces of code to participate in config proposals type Handler interface { // BeginConfig called when a config proposal is begun @@ -91,8 +103,7 @@ func computeChainIDAndSequence(configtx *cb.ConfigurationEnvelope) (string, uint for _, signedItem := range configtx.Items { item := &cb.ConfigurationItem{} - err := proto.Unmarshal(signedItem.ConfigurationItem, item) - if err != nil { + if err := proto.Unmarshal(signedItem.ConfigurationItem, item); err != nil { return "", 0, fmt.Errorf("Error unmarshaling signedItem.ConfigurationItem: %s", err) } @@ -115,11 +126,45 @@ func computeChainIDAndSequence(configtx *cb.ConfigurationEnvelope) (string, uint return "", 0, fmt.Errorf("Mismatched chainIDs in envelope %s != %s", chainID, item.Header.ChainID) } } + + if err := validateChainID(chainID); err != nil { + return "", 0, err + } } return chainID, m, nil } +// validateChainID makes sure that proposed chain IDs (i.e. channel names) +// comply with the following restrictions: +// 1. Contain only ASCII alphanumerics, dots '.', dashes '-' +// 2. Are shorter than 250 characters. +// 3. Are not the strings "." or "..". +// +// Our hand here is forced by: +// https://github.com/apache/kafka/blob/trunk/core/src/main/scala/kafka/common/Topic.scala#L29 +func validateChainID(chainID string) error { + re, _ := regexp.Compile(allowedChars) + // Length + if len(chainID) <= 0 { + return fmt.Errorf("chain ID illegal, cannot be empty") + } + if len(chainID) > maxLength { + return fmt.Errorf("chain ID illegal, cannot be longer than %d", maxLength) + } + // Illegal name + if _, ok := illegalNames[chainID]; ok { + return fmt.Errorf("name '%s' for chain ID is not allowed", chainID) + } + // Illegal characters + matched := re.FindString(chainID) + if len(matched) != len(chainID) { + return fmt.Errorf("Chain ID '%s' contains illegal characters", chainID) + } + + return nil +} + // NewManagerImpl creates a new Manager unless an error is encountered, each element of the callOnUpdate slice // is invoked when a new configuration is committed func NewManagerImpl(configtx *cb.ConfigurationEnvelope, initializer Initializer, callOnUpdate []func(Manager)) (Manager, error) { @@ -218,7 +263,7 @@ func (cm *configurationManager) processConfig(configtx *cb.ConfigurationEnvelope } // Ensure the config sequence numbers are correct to prevent replay attacks - isModified := false + var isModified bool if val, ok := cm.configuration[config.Type][config.Key]; ok { // Config was modified if any of the contents changed diff --git a/common/configtx/manager_test.go b/common/configtx/manager_test.go index 561cae4dcf5..6e694631870 100644 --- a/common/configtx/manager_test.go +++ b/common/configtx/manager_test.go @@ -26,6 +26,7 @@ import ( cb "github.com/hyperledger/fabric/protos/common" "errors" + "github.com/golang/protobuf/proto" ) @@ -118,8 +119,8 @@ func TestCallback(t *testing.T) { } } -// TestWrongChainID tests that a configuration update for a different chain ID fails -func TestWrongChainID(t *testing.T) { +// TestDifferentChainID tests that a configuration update for a different chain ID fails +func TestDifferentChainID(t *testing.T) { cm, err := NewManagerImpl(&cb.ConfigurationEnvelope{ Items: []*cb.SignedConfigurationItem{makeSignedConfigurationItem("foo", "foo", 0, []byte("foo"), defaultChain)}, }, &mockconfigtx.Initializer{PolicyManagerVal: &mockPolicyManager{&mockPolicy{}}, HandlersVal: defaultHandlers()}, nil) diff --git a/common/configtx/template.go b/common/configtx/template.go index 1b9076b0c97..04ea33fe94d 100644 --- a/common/configtx/template.go +++ b/common/configtx/template.go @@ -163,6 +163,9 @@ func MakeChainCreationTransaction(creationPolicy string, chainID string, signer } manager, err := NewManagerImpl(&cb.ConfigurationEnvelope{Items: items}, NewInitializer(), nil) + if err != nil { + return nil, err + } newChainTemplate := NewChainCreationTemplate(creationPolicy, manager.ChainConfig().HashingAlgorithm(), composite) signedConfigItems, err := newChainTemplate.Items(chainID) diff --git a/common/configtx/util_test.go b/common/configtx/util_test.go new file mode 100644 index 00000000000..a8a86b5d7a3 --- /dev/null +++ b/common/configtx/util_test.go @@ -0,0 +1,71 @@ +/* +Copyright IBM Corp. 2017 All Rights Reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package configtx + +import ( + "math/rand" + "testing" +) + +// TestValidchainID checks that the constraints on chain IDs are enforced properly +func TestValidChainID(t *testing.T) { + acceptMsg := "Should have accepted valid chain ID" + rejectMsg := "Should have rejected invalid chain ID" + + t.Run("ZeroLength", func(t *testing.T) { + if err := validateChainID(""); err == nil { + t.Fatal(rejectMsg) + } + }) + + t.Run("LongerThanMaxAllowed", func(t *testing.T) { + if err := validateChainID(randomAlphaString(maxLength + 1)); err == nil { + t.Fatal(rejectMsg) + } + }) + + t.Run("HasIllegalName", func(t *testing.T) { + for illegalName := range illegalNames { + if err := validateChainID(illegalName); err == nil { + t.Fatal(rejectMsg) + } + } + }) + + t.Run("ContainsIllegalCharacter", func(t *testing.T) { + if err := validateChainID("foo_bar"); err == nil { + t.Fatal(rejectMsg) + } + }) + + t.Run("ValidName", func(t *testing.T) { + if err := validateChainID("foo.bar"); err != nil { + t.Fatal(acceptMsg) + } + }) +} + +// Helper functions + +func randomAlphaString(size int) string { + letters := []rune("abcdefghijklmnopqrstuvwxyz") + output := make([]rune, size) + for i := range output { + output[i] = letters[rand.Intn(len(letters))] + } + return string(output) +}