Fix --csr.hosts flag for client and server

The docs currently state the --csr.hosts
takes a space-separated list but it should
actually be a comma-separate list.

Updated the help for the command flags and
regenerated the docs as well.

Also adding logic to trim spaces and
modified the function which checks SANs
to use built-in fields of
x509.Certificate.

FABC-831 #done

Change-Id: I41ebeeb98997fe5ad1672e62076214ad6a9d7c4a
Signed-off-by: Gari Singh <[email protected]>

cmd/fabric-ca-client/command/clientcmd.go

@@ -208,7 +208,7 @@ func (c *ClientCmd) registerFlags() {
 	tags := map[string]string{
 		"help.csr.cn":                "The common name field of the certificate signing request",
 		"help.csr.serialnumber":      "The serial number in a certificate signing request",
-		"help.csr.hosts":             "A list of space-separated host names in a certificate signing request",
+		"help.csr.hosts":             "A list of comma-separated host names in a certificate signing request",
 		"skip.csp.pluginopts.config": "true", // Skipping because this a map
 	}
 	err = util.RegisterFlags(c.myViper, pflags, c.clientCfg, tags)

cmd/fabric-ca-client/command/main_test.go

@@ -1425,12 +1425,16 @@ func testReenroll(t *testing.T) {
 	t.Log("Testing Reenroll command")
 	defYaml = util.GetDefaultConfigFile("fabric-ca-client")
 
-	err := RunMain([]string{cmdName, "reenroll", "-u", serverURL, "--csr.hosts", "host1"})
+	err := RunMain([]string{cmdName, "reenroll", "-u", serverURL, "--csr.hosts", "host1,host2"})
 	if err != nil {
 		t.Errorf("client reenroll --url -f failed: %s", err)
 	}
 
-	err = util.CheckHostsInCert(filepath.Join(filepath.Dir(defYaml), "msp", "signcerts", "cert.pem"), "host1")
+	err = util.CheckHostsInCert(
+		filepath.Join(filepath.Dir(defYaml), "msp", "signcerts", "cert.pem"),
+		"host1",
+		"host2",
+	)
 	if err != nil {
 		t.Error(err)
 	}

cmd/fabric-ca-server/servercmd.go

@@ -160,7 +160,7 @@ func (s *ServerCmd) registerFlags() {
 	tags := map[string]string{
 		"help.csr.cn":           "The common name field of the certificate signing request to a parent fabric-ca-server",
 		"help.csr.serialnumber": "The serial number in a certificate signing request to a parent fabric-ca-server",
-		"help.csr.hosts":        "A list of space-separated host names in a certificate signing request to a parent fabric-ca-server",
+		"help.csr.hosts":        "A list of comma-separated host names in a certificate signing request to a parent fabric-ca-server",
 	}
 	err := util.RegisterFlags(s.myViper, pflags, s.cfg, nil)
 	if err != nil {

docs/source/clientcli.rst

@@ -25,7 +25,7 @@ Fabric-CA Client's CLI
     Flags:
           --caname string                  Name of CA
           --csr.cn string                  The common name field of the certificate signing request
-          --csr.hosts stringSlice          A list of space-separated host names in a certificate signing request
+          --csr.hosts stringSlice          A list of comma-separated host names in a certificate signing request
           --csr.keyrequest.algo string     Specify key algorithm
           --csr.keyrequest.size int        Specify key size
           --csr.names stringSlice          A list of comma-separated CSR names of the form <name>=<value> (e.g. C=CA,O=Org1)

docs/source/servercli.rst

@@ -29,7 +29,7 @@ Fabric-CA Server's CLI
           --crl.expiry duration                       Expiration for the CRL generated by the gencrl request (default 24h0m0s)
           --crlsizelimit int                          Size limit of an acceptable CRL in bytes (default 512000)
           --csr.cn string                             The common name field of the certificate signing request to a parent fabric-ca-server
-          --csr.hosts stringSlice                     A list of space-separated host names in a certificate signing request to a parent fabric-ca-server
+          --csr.hosts stringSlice                     A list of comma-separated host names in a certificate signing request to a parent fabric-ca-server
           --csr.keyrequest.algo string                Specify key algorithm
           --csr.keyrequest.size int                   Specify key size
           --csr.serialnumber string                   The serial number in a certificate signing request to a parent fabric-ca-server

util/util.go

@@ -21,7 +21,6 @@ import (
 	"crypto/ecdsa"
 	"crypto/rsa"
 	"crypto/x509"
-	"encoding/asn1"
 	"encoding/base64"
 	"encoding/json"
 	"encoding/pem"
@@ -687,21 +686,17 @@ func GetMaskedURL(url string) string {
 func NormalizeStringSlice(slice []string) []string {
 	var normalizedSlice []string
 
-	if len(slice) > 0 {
-		for _, item := range slice {
-			// Remove surrounding brackets "[]" if specified
-			if strings.HasPrefix(item, "[") && strings.HasSuffix(item, "]") {
-				item = item[1 : len(item)-1]
-			}
-			// Split elements based on comma and add to normalized slice
-			if strings.Contains(item, ",") {
-				normalizedSlice = append(normalizedSlice, strings.Split(item, ",")...)
-			} else {
-				normalizedSlice = append(normalizedSlice, item)
-			}
+	for _, item := range slice {
+		// Remove surrounding brackets "[]" if specified
+		if strings.HasPrefix(item, "[") && strings.HasSuffix(item, "]") {
+			item = item[1 : len(item)-1]
+		}
+		// Split elements based on comma and add to normalized slice
+		elems := strings.Split(item, ",")
+		for _, elem := range elems {
+			normalizedSlice = append(normalizedSlice, strings.TrimSpace(elem))
 		}
 	}
-
 	return normalizedSlice
 }
 
@@ -722,8 +717,7 @@ func NormalizeFileList(files []string, homeDir string) ([]string, error) {
 }
 
 // CheckHostsInCert checks to see if host correctly inserted into certificate
-func CheckHostsInCert(certFile string, host string) error {
-	containsHost := false
+func CheckHostsInCert(certFile string, hosts ...string) error {
 	certBytes, err := ioutil.ReadFile(certFile)
 	if err != nil {
 		return errors.Wrapf(err, "Failed to read certificate file at '%s'", certFile)
@@ -733,22 +727,27 @@ func CheckHostsInCert(certFile string, host string) error {
 	if err != nil {
 		return errors.Wrap(err, "Failed to get certificate")
 	}
-	// Run through the extensions for the certificates
-	for _, ext := range cert.Extensions {
-		// asn1 identifier for 'Subject Alternative Name'
-		if ext.Id.Equal(asn1.ObjectIdentifier{2, 5, 29, 17}) {
-			if !strings.Contains(string(ext.Value), host) {
-				return errors.Errorf("Host '%s' was not found in the certificate in file '%s'", host, certFile)
-			}
-			containsHost = true
+
+	// combine DNSNames and IPAddresses from cert
+	sans := cert.DNSNames
+	for _, ip := range cert.IPAddresses {
+		sans = append(sans, ip.String())
+	}
+	for _, host := range hosts {
+		if !containsString(sans, host) {
+			return errors.Errorf("Host '%s' was not found in the certificate in file '%s'", host, certFile)
 		}
 	}
+	return nil
+}
 
-	if !containsHost {
-		return errors.New("Certificate contains no hosts")
+func containsString(list []string, item string) bool {
+	for _, elem := range list {
+		if elem == item {
+			return true
+		}
 	}
-
-	return nil
+	return false
 }
 
 // Read reads from Reader into a byte array
@@ -826,6 +825,25 @@ func ValidateAndReturnAbsConf(configFilePath, homeDir, cmdName string) (string,
 	return configFile, homeDir, nil
 }
 
+// GetSliceFromList will return a slice from a list
+func GetSliceFromList(split string, delim string) []string {
+	return strings.Split(strings.Replace(split, " ", "", -1), delim)
+}
+
+// ListContains looks through a comma separated list to see if a string exists
+func ListContains(list, find string) bool {
+	items := strings.Split(list, ",")
+	for _, item := range items {
+		item = strings.TrimPrefix(item, " ")
+		if item == find {
+			return true
+		}
+	}
+	return false
+}
+
+//TODO:  move these out of production code
+
 // FatalError will check to see if an error occured if so it will cause the test cases exit
 func FatalError(t *testing.T, err error, msg string, args ...interface{}) {
 	if len(args) > 0 {
@@ -846,20 +864,3 @@ func ErrorContains(t *testing.T, err error, contains, msg string, args ...interf
 		assert.Contains(t, caerrors.Print(err), contains)
 	}
 }
-
-// GetSliceFromList will return a slice from a list
-func GetSliceFromList(split string, delim string) []string {
-	return strings.Split(strings.Replace(split, " ", "", -1), delim)
-}
-
-// ListContains looks through a comma separated list to see if a string exists
-func ListContains(list, find string) bool {
-	items := strings.Split(list, ",")
-	for _, item := range items {
-		item = strings.TrimPrefix(item, " ")
-		if item == find {
-			return true
-		}
-	}
-	return false
-}

util/util_test.go

@@ -594,6 +594,43 @@ func TestStructToString(t *testing.T) {
 	assert.NotContains(t, idStr, "barpwd", "Identity password is not masked in the output")
 }
 
+func TestNormalizeStringSlice(t *testing.T) {
+	var tests = []struct {
+		slice    []string
+		expected []string
+	}{
+		{
+			slice:    []string{"string1"},
+			expected: []string{"string1"},
+		},
+		{
+			slice:    []string{" string1"},
+			expected: []string{"string1"},
+		},
+		{
+			slice:    []string{" string1   "},
+			expected: []string{"string1"},
+		},
+		{
+			slice:    []string{" string1   "},
+			expected: []string{"string1"},
+		},
+		{
+			slice:    []string{"string1", "string2"},
+			expected: []string{"string1", "string2"},
+		},
+		{
+			slice:    []string{"string1", "   string2"},
+			expected: []string{"string1", "string2"},
+		},
+	}
+
+	for _, test := range tests {
+		actual := NormalizeStringSlice(test.slice)
+		assert.Equal(t, test.expected, actual)
+	}
+}
+
 // Test file list with multiple and single entries both with and without brackets
 func TestNormalizeFileList(t *testing.T) {
 	slice := []string{"[file0,file1]", "file2,file3", "file4", "[file5]"}
@@ -717,7 +754,7 @@ func TestCheckHostsInCert(t *testing.T) {
 	err = CheckHostsInCert("../testdata/tls_server-cert.pem", "localhost")
 	assert.NoError(t, err, fmt.Sprintf("Failed to find 'localhost' for host in certificate: %s", err))
 
-	err = CheckHostsInCert("../testdata/tls_server-cert.pem", "fakehost")
+	err = CheckHostsInCert("../testdata/tls_server-cert.pem", "localhost", "fakehost")
 	assert.Error(t, err, "Certificate does not contain 'fakehost', should have failed")
 
 	err = CheckHostsInCert("../testdata/root.pem", "x")