[FAB-2487] Restrict channelIDs to CouchDB/Kafka

The current channel ID checks are to ensure that the IDs comply with the
Kafka topic naming restrictions.

However, it is possible that two different channel IDs such as TestChain
and testchain would both map to the same couchdb name, causing an
intersection and failure.

This CR changes the logic to test for the intersection of the
restrictions on CouchDB and Kafka.  In particular, the logic now
requires that Channel IDs:

1. Contain only lower case ASCII alphanumerics, dots '.' and dashes '-'
2. Are shorter than 250 characters.
3. Start with a letter

Note that this is not a true intersection, because the ledger must still
map '.' to '_' for CouchDB, but this mapping is bijective so is safe and
free from collisions.

Because the configuration key name checking was leveraging this same
code path, the existing function was duplicated, to leave the same logic
checks in place for the config.

Although the code diff is relatively high, this is largely due to the
copying and renaming.

Change-Id: Ie957cf9b8a075233bfcc5d3748e1a91e795ff067
Signed-off-by: Jason Yellick <[email protected]>

Author: Jason Yellick

bddtests/features/bootstrap.feature

@@ -52,7 +52,7 @@ Feature: Bootstrap
         | Organization     |
         |  ordererOrg0     |
 
-    And the ordererBootstrapAdmin generates a GUUID to identify the orderer system chain and refer to it by name as "OrdererSystemChainId"
+    And the ordererBootstrapAdmin generates a GUUID to identify the orderer system chain and refer to it by name as "orderer-system-chain-id"
 
     # We now have an orderer network with NO peers.  Now need to configure and start the peer network
     # This can be currently automated through folder creation of the proper form and placing PEMs.
@@ -72,22 +72,22 @@ Feature: Bootstrap
 
     # Order info includes orderer admin/orderer information and address (host:port) from previous steps
     # Only the peer organizations can vary.
-    And the ordererBootstrapAdmin using cert alias "bootstrapCertAlias" creates the genesis block "ordererGenesisBlock" for chain "OrdererSystemChainId" for composition "<ComposeFile>" and consensus "<ConsensusType>" with consortiums modification policy "/Channel/Orderer/Admins" using consortiums:
+    And the ordererBootstrapAdmin using cert alias "bootstrapCertAlias" creates the genesis block "ordererGenesisBlock" for chain "orderer-system-chain-id" for composition "<ComposeFile>" and consensus "<ConsensusType>" with consortiums modification policy "/Channel/Orderer/Admins" using consortiums:
       | Consortium  |
 #      | consortium1 |
 
 
-    And the orderer admins inspect and approve the genesis block for chain "OrdererSystemChainId"
+    And the orderer admins inspect and approve the genesis block for chain "orderer-system-chain-id"
 
     # to be used for setting the orderer genesis block path parameter in composition
-    And the orderer admins use the genesis block for chain "OrdererSystemChainId" to configure orderers
+    And the orderer admins use the genesis block for chain "orderer-system-chain-id" to configure orderers
 
     And we compose "<ComposeFile>"
 
     # Sleep as to allow system up time
     And I wait "<SystemUpWaitTime>" seconds
 
-    Given user "ordererBootstrapAdmin" gives "OrdererSystemChainId" to user "configAdminOrdererOrg0"
+    Given user "ordererBootstrapAdmin" gives "orderer-system-chain-id" to user "configAdminOrdererOrg0"
     And user "ordererBootstrapAdmin" gives "ordererGenesisBlock" to user "configAdminOrdererOrg0"
 
     And the orderer config admin "configAdminOrdererOrg0" creates a consortium "consortium1" with modification policy "/Channel/Orderer/Admins" for peer orgs who wish to form a network:
@@ -98,9 +98,9 @@ Feature: Bootstrap
 
     And user "configAdminOrdererOrg0" using cert alias "config-admin-cert" connects to deliver function on orderer "<orderer0>"
 
-    And user "configAdminOrdererOrg0" retrieves the latest configuration "latestOrdererConfig" from orderer "<orderer0>" for channel "OrdererSystemChainId"
+    And user "configAdminOrdererOrg0" retrieves the latest configuration "latestOrdererConfig" from orderer "<orderer0>" for channel "orderer-system-chain-id"
 
-    And the orderer config admin "configAdminOrdererOrg0" creates a consortiums config update "consortiumsConfigUpdate1" using config "latestOrdererConfig" using orderer system channel ID "OrdererSystemChainId" to add consortiums:
+    And the orderer config admin "configAdminOrdererOrg0" creates a consortiums config update "consortiumsConfigUpdate1" using config "latestOrdererConfig" using orderer system channel ID "orderer-system-chain-id" to add consortiums:
       | Consortium  |
       | consortium1 |
 
@@ -113,7 +113,7 @@ Feature: Bootstrap
 
     And the user "configAdminOrdererOrg0" creates a ConfigUpdate Tx "consortiumsConfigUpdateTx1" using cert alias "config-admin-cert" using signed ConfigUpdateEnvelope "consortiumsConfigUpdate1Envelope"
 
-    And the user "configAdminOrdererOrg0" using cert alias "config-admin-cert" broadcasts ConfigUpdate Tx "consortiumsConfigUpdateTx1" to orderer "<orderer0>" to create channel "com.acme.blockchain.jdoe.Channel1"
+    And the user "configAdminOrdererOrg0" using cert alias "config-admin-cert" broadcasts ConfigUpdate Tx "consortiumsConfigUpdateTx1" to orderer "<orderer0>" to create channel "com.acme.blockchain.jdoe.channel1"
 
 
     Given the following application developers are defined for peer organizations and each saves their cert as alias
@@ -142,7 +142,7 @@ Feature: Bootstrap
     # Entry point for creating a channel
     And the user "dev0Org0" creates a new channel ConfigUpdate "createChannelConfigUpdate1" using consortium "consortium1"
       | ChannelID                         | PeerOrgSet  | PeerAnchorSet |
-      | com.acme.blockchain.jdoe.Channel1 | peerOrgSet1 | anchors1      |
+      | com.acme.blockchain.jdoe.channel1 | peerOrgSet1 | anchors1      |
 
     And the user "dev0Org0" creates a configUpdateEnvelope "createChannelConfigUpdate1Envelope" using configUpdate "createChannelConfigUpdate1"
 
@@ -154,7 +154,7 @@ Feature: Bootstrap
 
     And the user "dev0Org0" creates a ConfigUpdate Tx "configUpdateTx1" using cert alias "consortium1-cert" using signed ConfigUpdateEnvelope "createChannelConfigUpdate1Envelope"
 
-    And the user "dev0Org0" using cert alias "consortium1-cert" broadcasts ConfigUpdate Tx "configUpdateTx1" to orderer "<orderer0>" to create channel "com.acme.blockchain.jdoe.Channel1"
+    And the user "dev0Org0" using cert alias "consortium1-cert" broadcasts ConfigUpdate Tx "configUpdateTx1" to orderer "<orderer0>" to create channel "com.acme.blockchain.jdoe.channel1"
 
     # Sleep as the local orderer needs to bring up the resources that correspond to the new channel
     # For the Kafka orderer, this includes setting up a producer and consumer for the channel's partition
@@ -164,7 +164,7 @@ Feature: Bootstrap
     When user "dev0Org0" using cert alias "consortium1-cert" connects to deliver function on orderer "<orderer0>"
     And user "dev0Org0" sends deliver a seek request on orderer "<orderer0>" with properties:
       | ChainId                               | Start |  End    |
-      | com.acme.blockchain.jdoe.Channel1     |   0   |  0      |
+      | com.acme.blockchain.jdoe.channel1     |   0   |  0      |
 
     Then user "dev0Org0" should get a delivery "genesisBlockForMyNewChannel" from "<orderer0>" of "1" blocks with "1" messages within "1" seconds
 
@@ -216,7 +216,7 @@ Feature: Bootstrap
       |   init   |  a   |  100  |  b   |  200 |
 
     # Under the covers, create a deployment spec, etc.
-    And user "peer0Admin" using cert alias "peer-admin-cert" creates a install proposal "installProposal1" for channel "com.acme.blockchain.jdoe.Channel1" using chaincode spec "cc_spec"
+    And user "peer0Admin" using cert alias "peer-admin-cert" creates a install proposal "installProposal1" for channel "com.acme.blockchain.jdoe.channel1" using chaincode spec "cc_spec"
 
     And user "peer0Admin" using cert alias "peer-admin-cert" sends proposal "installProposal1" to endorsers with timeout of "90" seconds with proposal responses "installProposalResponses":
         | Endorser |
@@ -229,7 +229,7 @@ Feature: Bootstrap
     Given user "peer0Admin" gives "cc_spec" to user "peer2Admin"
 
     # Under the covers, create a deployment spec, etc.
-    When user "peer2Admin" using cert alias "peer-admin-cert" creates a install proposal "installProposal1" for channel "com.acme.blockchain.jdoe.Channel1" using chaincode spec "cc_spec"
+    When user "peer2Admin" using cert alias "peer-admin-cert" creates a install proposal "installProposal1" for channel "com.acme.blockchain.jdoe.channel1" using chaincode spec "cc_spec"
 
     And user "peer2Admin" using cert alias "peer-admin-cert" sends proposal "installProposal1" to endorsers with timeout of "90" seconds with proposal responses "installProposalResponses":
       | Endorser |
@@ -244,7 +244,7 @@ Feature: Bootstrap
       And user "peer0Admin" gives "cc_spec" to user "configAdminPeerOrg0"
 
 
-    When user "configAdminPeerOrg0" using cert alias "config-admin-cert" creates a instantiate proposal "instantiateProposal1" for channel "com.acme.blockchain.jdoe.Channel1" using chaincode spec "cc_spec"
+    When user "configAdminPeerOrg0" using cert alias "config-admin-cert" creates a instantiate proposal "instantiateProposal1" for channel "com.acme.blockchain.jdoe.channel1" using chaincode spec "cc_spec"
 
     And user "configAdminPeerOrg0" using cert alias "config-admin-cert" sends proposal "instantiateProposal1" to endorsers with timeout of "90" seconds with proposal responses "instantiateProposalResponses":
       | Endorser |
@@ -262,9 +262,9 @@ Feature: Bootstrap
       | peer0    |
       | peer2    |
 
-    When the user "configAdminPeerOrg0" creates transaction "instantiateTx1" from proposal "instantiateProposal1" and proposal responses "instantiateProposalResponses" for channel "com.acme.blockchain.jdoe.Channel1"
+    When the user "configAdminPeerOrg0" creates transaction "instantiateTx1" from proposal "instantiateProposal1" and proposal responses "instantiateProposalResponses" for channel "com.acme.blockchain.jdoe.channel1"
 
-    And the user "configAdminPeerOrg0" broadcasts transaction "instantiateTx1" to orderer "<orderer1>" on channel "com.acme.blockchain.jdoe.Channel1"
+    And the user "configAdminPeerOrg0" broadcasts transaction "instantiateTx1" to orderer "<orderer1>" on channel "com.acme.blockchain.jdoe.channel1"
 
     # Sleep as the local orderer ledger needs to create the block that corresponds to the start number of the seek request
     And I wait "<BroadcastWaitTime>" seconds
@@ -273,7 +273,7 @@ Feature: Bootstrap
 
     And user "configAdminPeerOrg0" sends deliver a seek request on orderer "<orderer0>" with properties:
         | ChainId                               |   Start    |  End    |
-        | com.acme.blockchain.jdoe.Channel1     |   1   |  1      |
+        | com.acme.blockchain.jdoe.channel1     |   1   |  1      |
 
     Then user "configAdminPeerOrg0" should get a delivery "deliveredInstantiateTx1Block" from "<orderer0>" of "1" blocks with "1" messages within "1" seconds
 
@@ -286,7 +286,7 @@ Feature: Bootstrap
         |   query   |  a   |
 
     # Under the covers, create a deployment spec, etc.
-    And user "dev0Org0" using cert alias "consortium1-cert" creates a proposal "queryProposal1" for channel "com.acme.blockchain.jdoe.Channel1" using chaincode spec "querySpec1"
+    And user "dev0Org0" using cert alias "consortium1-cert" creates a proposal "queryProposal1" for channel "com.acme.blockchain.jdoe.channel1" using chaincode spec "querySpec1"
 
     And user "dev0Org0" using cert alias "consortium1-cert" sends proposal "queryProposal1" to endorsers with timeout of "30" seconds with proposal responses "queryProposal1Responses":
         | Endorser |
@@ -310,7 +310,7 @@ Feature: Bootstrap
         |   invoke   |  a   |  b   |   10   |
 
     # Under the covers, create a deployment spec, etc.
-      And user "dev0Org0" using cert alias "consortium1-cert" creates a proposal "invokeProposal1" for channel "com.acme.blockchain.jdoe.Channel1" using chaincode spec "invocationSpec1"
+      And user "dev0Org0" using cert alias "consortium1-cert" creates a proposal "invokeProposal1" for channel "com.acme.blockchain.jdoe.channel1" using chaincode spec "invocationSpec1"
 
       And user "dev0Org0" using cert alias "consortium1-cert" sends proposal "invokeProposal1" to endorsers with timeout of "30" seconds with proposal responses "invokeProposal1Responses":
         | Endorser |
@@ -327,16 +327,16 @@ Feature: Bootstrap
         | peer0    |
         | peer2    |
 
-      When the user "dev0Org0" creates transaction "invokeTx1" from proposal "invokeProposal1" and proposal responses "invokeProposal1Responses" for channel "com.acme.blockchain.jdoe.Channel1"
+      When the user "dev0Org0" creates transaction "invokeTx1" from proposal "invokeProposal1" and proposal responses "invokeProposal1Responses" for channel "com.acme.blockchain.jdoe.channel1"
 
-      And the user "dev0Org0" broadcasts transaction "invokeTx1" to orderer "<orderer2>" on channel "com.acme.blockchain.jdoe.Channel1"
+      And the user "dev0Org0" broadcasts transaction "invokeTx1" to orderer "<orderer2>" on channel "com.acme.blockchain.jdoe.channel1"
 
       # Sleep as the local orderer ledger needs to create the block that corresponds to the start number of the seek request
       And I wait "<BroadcastWaitTime>" seconds
 
       And user "dev0Org0" sends deliver a seek request on orderer "<orderer0>" with properties:
         | ChainId                               |   Start    |  End    |
-        | com.acme.blockchain.jdoe.Channel1     |   2        |  2      |
+        | com.acme.blockchain.jdoe.channel1     |   2        |  2      |
 
       Then user "dev0Org0" should get a delivery "deliveredInvokeTx1Block" from "<orderer0>" of "1" blocks with "1" messages within "1" seconds
 

common/configtx/configmap.go

@@ -59,8 +59,7 @@ func addToMap(cg comparable, result map[string]comparable) error {
 		fqPath = PolicyPrefix
 	}
 
-	// TODO rename validateChainID to validateConfigID
-	if err := validateChainID(cg.key); err != nil {
+	if err := validateConfigID(cg.key); err != nil {
 		return fmt.Errorf("Illegal characters in key: %s", fqPath)
 	}
 

common/configtx/manager.go

@@ -29,11 +29,12 @@ import (
 
 var logger = flogging.MustGetLogger("common/configtx")
 
-// Constraints for valid chain IDs
+// Constraints for valid channel and config IDs
 var (
-	allowedChars = "[a-zA-Z0-9.-]+"
-	maxLength    = 249
-	illegalNames = map[string]struct{}{
+	channelAllowedChars = "[a-z][a-z0-9.-]*"
+	configAllowedChars  = "[a-zA-Z0-9.-]+"
+	maxLength           = 249
+	illegalNames        = map[string]struct{}{
 		".":  struct{}{},
 		"..": struct{}{},
 	}
@@ -53,31 +54,57 @@ type configManager struct {
 	current      *configSet
 }
 
-// validateChainID makes sure that proposed chain IDs (i.e. channel names)
-// comply with the following restrictions:
+// validateConfigID makes sure that the config element names (ie map key of
+// ConfigGroup) 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)
+func validateConfigID(configID string) error {
+	re, _ := regexp.Compile(configAllowedChars)
 	// Length
-	if len(chainID) <= 0 {
-		return fmt.Errorf("chain ID illegal, cannot be empty")
+	if len(configID) <= 0 {
+		return fmt.Errorf("config ID illegal, cannot be empty")
 	}
-	if len(chainID) > maxLength {
-		return fmt.Errorf("chain ID illegal, cannot be longer than %d", maxLength)
+	if len(configID) > maxLength {
+		return fmt.Errorf("config 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)
+	if _, ok := illegalNames[configID]; ok {
+		return fmt.Errorf("name '%s' for config ID is not allowed", configID)
+	}
+	// Illegal characters
+	matched := re.FindString(configID)
+	if len(matched) != len(configID) {
+		return fmt.Errorf("config ID '%s' contains illegal characters", configID)
 	}
+
+	return nil
+}
+
+// validateChannelID makes sure that proposed channel IDs comply with the
+// following restrictions:
+//      1. Contain only lower case ASCII alphanumerics, dots '.', and dashes '-'
+//      2. Are shorter than 250 characters.
+//      3. Start with a letter
+//
+// This is the intersection of the Kafka restrictions and CouchDB restrictions
+// with the following exception: '.' is converted to '_' in the CouchDB naming
+// This is to accomodate existing channel names with '.', especially in the
+// behave tests which rely on the dot notation for their sluggification.
+func validateChannelID(channelID string) error {
+	re, _ := regexp.Compile(channelAllowedChars)
+	// Length
+	if len(channelID) <= 0 {
+		return fmt.Errorf("channel ID illegal, cannot be empty")
+	}
+	if len(channelID) > maxLength {
+		return fmt.Errorf("channel ID illegal, cannot be longer than %d", maxLength)
+	}
+
 	// Illegal characters
-	matched := re.FindString(chainID)
-	if len(matched) != len(chainID) {
-		return fmt.Errorf("Chain ID '%s' contains illegal characters", chainID)
+	matched := re.FindString(channelID)
+	if len(matched) != len(channelID) {
+		return fmt.Errorf("channel ID '%s' contains illegal characters", channelID)
 	}
 
 	return nil
@@ -102,7 +129,7 @@ func NewManagerImpl(envConfig *cb.Envelope, initializer api.Initializer, callOnU
 		return nil, fmt.Errorf("nil channel group")
 	}
 
-	if err := validateChainID(header.ChannelId); err != nil {
+	if err := validateChannelID(header.ChannelId); err != nil {
 		return nil, fmt.Errorf("Bad channel id: %s", err)
 	}
 

common/configtx/manager_test.go

@@ -30,7 +30,7 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-var defaultChain = "DefaultChainID"
+var defaultChain = "default.chain.id"
 
 func defaultInitializer() *mockconfigtx.Initializer {
 	return &mockconfigtx.Initializer{