Ensure CN check is made when exclude_cn_from_sans is used

Fixes #2363

builtin/logical/pki/cert_util.go

@@ -217,7 +217,7 @@ func fetchCertBySerial(req *logical.Request, prefix, serial string) (*logical.St
 // Given a set of requested names for a certificate, verifies that all of them
 // match the various toggles set in the role for controlling issuance.
 // If one does not pass, it is returned in the string argument.
-func validateNames(req *logical.Request, names []string, role *roleEntry) (string, error) {
+func validateNames(req *logical.Request, names []string, role *roleEntry) string {
 	for _, name := range names {
 		sanitizedName := name
 		emailDomain := name
@@ -233,7 +233,7 @@ func validateNames(req *logical.Request, names []string, role *roleEntry) (strin
 		if strings.Contains(name, "@") {
 			splitEmail := strings.Split(name, "@")
 			if len(splitEmail) != 2 {
-				return name, nil
+				return name
 			}
 			sanitizedName = splitEmail[1]
 			emailDomain = splitEmail[1]
@@ -250,7 +250,7 @@ func validateNames(req *logical.Request, names []string, role *roleEntry) (strin
 
 		// Email addresses using wildcard domain names do not make sense
 		if isEmail && isWildcard {
-			return name, nil
+			return name
 		}
 
 		// AllowAnyName is checked after this because EnforceHostnames still
@@ -259,7 +259,7 @@ func validateNames(req *logical.Request, names []string, role *roleEntry) (strin
 		// wildcard prefix.
 		if role.EnforceHostnames {
 			if !hostnameRegex.MatchString(sanitizedName) {
-				return name, nil
+				return name
 			}
 		}
 
@@ -368,10 +368,10 @@ func validateNames(req *logical.Request, names []string, role *roleEntry) (strin
 		}
 
 		//panic(fmt.Sprintf("\nName is %s\nRole is\n%#v\n", name, role))
-		return name, nil
+		return name
 	}
 
-	return "", nil
+	return ""
 }
 
 func generateCert(b *backend,
@@ -560,8 +560,10 @@ func generateCreationBundle(b *backend,
 	var err error
 	var ok bool
 
-	// Get the common name
+	// Read in names -- CN, DNS and email addresses
 	var cn string
+	dnsNames := []string{}
+	emailAddresses := []string{}
 	{
 		if csr != nil && role.UseCSRCommonName {
 			cn = csr.Subject.CommonName
@@ -572,28 +574,7 @@ func generateCreationBundle(b *backend,
 				return nil, errutil.UserError{Err: `the common_name field is required, or must be provided in a CSR with "use_csr_common_name" set to true`}
 			}
 		}
-	}
-
-	// Set OU (organizationalUnit) values if specified in the role
-	ou := []string{}
-	{
-		if role.OU != "" {
-			ou = strutil.ParseDedupAndSortStrings(role.OU, ",")
-		}
-	}
-
-	// Set O (organization) values if specified in the role
-	organization := []string{}
-	{
-		if role.Organization != "" {
-			organization = strutil.ParseDedupAndSortStrings(role.Organization, ",")
-		}
-	}
 
-	// Read in alternate names -- DNS and email addresses
-	dnsNames := []string{}
-	emailAddresses := []string{}
-	{
 		if csr != nil && role.UseCSRSANs {
 			dnsNames = csr.DNSNames
 			emailAddresses = csr.EmailAddresses
@@ -613,38 +594,38 @@ func generateCreationBundle(b *backend,
 		}
 
 		if csr == nil || !role.UseCSRSANs {
-			cnAltInt, ok := data.GetOk("alt_names")
+			cnAltRaw, ok := data.GetOk("alt_names")
 			if ok {
-				cnAlt := cnAltInt.(string)
-				if len(cnAlt) != 0 {
-					for _, v := range strings.Split(cnAlt, ",") {
-						if strings.Contains(v, "@") {
-							emailAddresses = append(emailAddresses, v)
-						} else {
-							dnsNames = append(dnsNames, v)
-						}
+				cnAlt := strutil.ParseDedupAndSortStrings(cnAltRaw.(string), ",")
+				for _, v := range cnAlt {
+					if strings.Contains(v, "@") {
+						emailAddresses = append(emailAddresses, v)
+					} else {
+						dnsNames = append(dnsNames, v)
 					}
 				}
 			}
 		}
 
+		// Check the CN. This ensures that the CN is checked even if it's
+		// excluded from SANs.
+		badName := validateNames(req, []string{cn}, role)
+		if len(badName) != 0 {
+			return nil, errutil.UserError{Err: fmt.Sprintf(
+				"common name %s not allowed by this role", badName)}
+		}
+
 		// Check for bad email and/or DNS names
-		badName, err := validateNames(req, dnsNames, role)
+		badName = validateNames(req, dnsNames, role)
 		if len(badName) != 0 {
 			return nil, errutil.UserError{Err: fmt.Sprintf(
-				"name %s not allowed by this role", badName)}
-		} else if err != nil {
-			return nil, errutil.InternalError{Err: fmt.Sprintf(
-				"error validating name %s: %s", badName, err)}
+				"subject alternate name %s not allowed by this role", badName)}
 		}
 
-		badName, err = validateNames(req, emailAddresses, role)
+		badName = validateNames(req, emailAddresses, role)
 		if len(badName) != 0 {
 			return nil, errutil.UserError{Err: fmt.Sprintf(
-				"email %s not allowed by this role", badName)}
-		} else if err != nil {
-			return nil, errutil.InternalError{Err: fmt.Sprintf(
-				"error validating name %s: %s", badName, err)}
+				"email address %s not allowed by this role", badName)}
 		}
 	}
 
@@ -680,6 +661,22 @@ func generateCreationBundle(b *backend,
 		}
 	}
 
+	// Set OU (organizationalUnit) values if specified in the role
+	ou := []string{}
+	{
+		if role.OU != "" {
+			ou = strutil.ParseDedupAndSortStrings(role.OU, ",")
+		}
+	}
+
+	// Set O (organization) values if specified in the role
+	organization := []string{}
+	{
+		if role.Organization != "" {
+			organization = strutil.ParseDedupAndSortStrings(role.Organization, ",")
+		}
+	}
+
 	// Get the TTL and very it against the max allowed
 	var ttlField string
 	var ttl time.Duration