Various PKI updates (#3953)

builtin/logical/pki/backend_test.go

@@ -33,6 +33,7 @@ import (
 	logicaltest "github.com/hashicorp/vault/logical/testing"
 	"github.com/hashicorp/vault/vault"
 	"github.com/mitchellh/mapstructure"
+	"golang.org/x/net/idna"
 )
 
 var (
@@ -153,8 +154,6 @@ func TestBackend_RSAKey(t *testing.T) {
 		Steps:   []logicaltest.TestStep{},
 	}
 
-	stepCount = len(testCase.Steps)
-
 	intdata := map[string]interface{}{}
 	reqdata := map[string]interface{}{}
 	testCase.Steps = append(testCase.Steps, generateCATestingSteps(t, rsaCACert, rsaCAKey, ecCACert, intdata, reqdata)...)
@@ -183,8 +182,6 @@ func TestBackend_ECKey(t *testing.T) {
 		Steps:   []logicaltest.TestStep{},
 	}
 
-	stepCount = len(testCase.Steps)
-
 	intdata := map[string]interface{}{}
 	reqdata := map[string]interface{}{}
 	testCase.Steps = append(testCase.Steps, generateCATestingSteps(t, ecCACert, ecCAKey, rsaCACert, intdata, reqdata)...)
@@ -211,8 +208,6 @@ func TestBackend_CSRValues(t *testing.T) {
 		Steps:   []logicaltest.TestStep{},
 	}
 
-	stepCount = len(testCase.Steps)
-
 	intdata := map[string]interface{}{}
 	reqdata := map[string]interface{}{}
 	testCase.Steps = append(testCase.Steps, generateCSRSteps(t, ecCACert, ecCAKey, intdata, reqdata)...)
@@ -239,8 +234,6 @@ func TestBackend_URLsCRUD(t *testing.T) {
 		Steps:   []logicaltest.TestStep{},
 	}
 
-	stepCount = len(testCase.Steps)
-
 	intdata := map[string]interface{}{}
 	reqdata := map[string]interface{}{}
 	testCase.Steps = append(testCase.Steps, generateURLSteps(t, ecCACert, ecCAKey, intdata, reqdata)...)
@@ -278,12 +271,10 @@ func TestBackend_RSARoles(t *testing.T) {
 		},
 	}
 
-	stepCount = len(testCase.Steps)
-
 	testCase.Steps = append(testCase.Steps, generateRoleSteps(t, false)...)
 	if len(os.Getenv("VAULT_VERBOSE_PKITESTS")) > 0 {
 		for i, v := range testCase.Steps {
-			fmt.Printf("Step %d:\n%+v\n\n", i+stepCount, v)
+			fmt.Printf("Step %d:\n%+v\n\n", i+1, v)
 		}
 	}
 
@@ -320,12 +311,10 @@ func TestBackend_RSARoles_CSR(t *testing.T) {
 		},
 	}
 
-	stepCount = len(testCase.Steps)
-
 	testCase.Steps = append(testCase.Steps, generateRoleSteps(t, true)...)
 	if len(os.Getenv("VAULT_VERBOSE_PKITESTS")) > 0 {
 		for i, v := range testCase.Steps {
-			fmt.Printf("Step %d:\n%+v\n\n", i+stepCount, v)
+			fmt.Printf("Step %d:\n%+v\n\n", i+1, v)
 		}
 	}
 
@@ -362,12 +351,10 @@ func TestBackend_ECRoles(t *testing.T) {
 		},
 	}
 
-	stepCount = len(testCase.Steps)
-
 	testCase.Steps = append(testCase.Steps, generateRoleSteps(t, false)...)
 	if len(os.Getenv("VAULT_VERBOSE_PKITESTS")) > 0 {
 		for i, v := range testCase.Steps {
-			fmt.Printf("Step %d:\n%+v\n\n", i+stepCount, v)
+			fmt.Printf("Step %d:\n%+v\n\n", i+1, v)
 		}
 	}
 
@@ -404,12 +391,10 @@ func TestBackend_ECRoles_CSR(t *testing.T) {
 		},
 	}
 
-	stepCount = len(testCase.Steps)
-
 	testCase.Steps = append(testCase.Steps, generateRoleSteps(t, true)...)
 	if len(os.Getenv("VAULT_VERBOSE_PKITESTS")) > 0 {
 		for i, v := range testCase.Steps {
-			fmt.Printf("Step %d:\n%+v\n\n", i+stepCount, v)
+			fmt.Printf("Step %d:\n%+v\n\n", i+1, v)
 		}
 	}
 
@@ -588,7 +573,7 @@ func generateURLSteps(t *testing.T, caCert, caKey string, intdata, reqdata map[s
 			Operation: logical.UpdateOperation,
 			Path:      "root/sign-intermediate",
 			Data: map[string]interface{}{
-				"common_name": "Intermediate Cert",
+				"common_name": "intermediate.cert.com",
 				"csr":         string(csrPem1024),
 				"format":      "der",
 			},
@@ -609,7 +594,7 @@ func generateURLSteps(t *testing.T, caCert, caKey string, intdata, reqdata map[s
 			Operation: logical.UpdateOperation,
 			Path:      "root/sign-intermediate",
 			Data: map[string]interface{}{
-				"common_name": "Intermediate Cert",
+				"common_name": "intermediate.cert.com",
 				"csr":         string(csrPem2048),
 				"format":      "der",
 			},
@@ -638,8 +623,8 @@ func generateURLSteps(t *testing.T, caCert, caKey string, intdata, reqdata map[s
 					return fmt.Errorf("expected\n%#v\ngot\n%#v\n", expected.CRLDistributionPoints, cert.CRLDistributionPoints)
 				case !reflect.DeepEqual(expected.OCSPServers, cert.OCSPServer):
 					return fmt.Errorf("expected\n%#v\ngot\n%#v\n", expected.OCSPServers, cert.OCSPServer)
-				case !reflect.DeepEqual([]string{"Intermediate Cert"}, cert.DNSNames):
+				case !reflect.DeepEqual([]string{"intermediate.cert.com"}, cert.DNSNames):
-					return fmt.Errorf("expected\n%#v\ngot\n%#v\n", []string{"Intermediate Cert"}, cert.DNSNames)
+					return fmt.Errorf("expected\n%#v\ngot\n%#v\n", []string{"intermediate.cert.com"}, cert.DNSNames)
 				}
 
 				return nil
@@ -651,7 +636,7 @@ func generateURLSteps(t *testing.T, caCert, caKey string, intdata, reqdata map[s
 			Operation: logical.UpdateOperation,
 			Path:      "root/sign-intermediate",
 			Data: map[string]interface{}{
-				"common_name":          "Intermediate Cert",
+				"common_name":          "intermediate.cert.com",
 				"csr":                  string(csrPem2048),
 				"format":               "der",
 				"exclude_cn_from_sans": true,
@@ -991,7 +976,7 @@ func generateCATestingSteps(t *testing.T, caCert, caKey, otherCaCert string, int
 			Operation: logical.UpdateOperation,
 			Path:      "intermediate/generate/exported",
 			Data: map[string]interface{}{
-				"common_name": "Intermediate Cert",
+				"common_name": "intermediate.cert.com",
 			},
 			Check: func(resp *logical.Response) error {
 				intdata["intermediatecsr"] = resp.Data["csr"].(string)
@@ -1009,7 +994,7 @@ func generateCATestingSteps(t *testing.T, caCert, caKey, otherCaCert string, int
 				delete(reqdata, "pem_bundle")
 				delete(reqdata, "ttl")
 				reqdata["csr"] = intdata["intermediatecsr"].(string)
-				reqdata["common_name"] = "Intermediate Cert"
+				reqdata["common_name"] = "intermediate.cert.com"
 				reqdata["ttl"] = "10s"
 				return nil
 			},
@@ -1144,7 +1129,7 @@ func generateCATestingSteps(t *testing.T, caCert, caKey, otherCaCert string, int
 				"format":      "der",
 				"key_type":    "ec",
 				"key_bits":    384,
-				"common_name": "Intermediate Cert",
+				"common_name": "intermediate.cert.com",
 			},
 			Check: func(resp *logical.Response) error {
 				csrBytes, _ := base64.StdEncoding.DecodeString(resp.Data["csr"].(string))
@@ -1171,7 +1156,7 @@ func generateCATestingSteps(t *testing.T, caCert, caKey, otherCaCert string, int
 				delete(reqdata, "pem_bundle")
 				delete(reqdata, "ttl")
 				reqdata["csr"] = intdata["intermediatecsr"].(string)
-				reqdata["common_name"] = "Intermediate Cert"
+				reqdata["common_name"] = "intermediate.cert.com"
 				reqdata["ttl"] = "10s"
 				return nil
 			},
@@ -1646,7 +1631,18 @@ func generateRoleSteps(t *testing.T, useCSRs bool) []logicaltest.TestStep {
 				retName = cert.EmailAddresses[0]
 			}
 			if retName != name {
-				return fmt.Errorf("Error: returned certificate has a DNS SAN of %s but %s was requested", retName, name)
+				// Check IDNA
+				p := idna.New(
+					idna.StrictDomainName(true),
+					idna.VerifyDNSLength(true),
+				)
+				converted, err := p.ToUnicode(retName)
+				if err != nil {
+					t.Fatal(err)
+				}
+				if converted != name {
+					return fmt.Errorf("Error: returned certificate has a DNS SAN of %s (from idna: %s) but %s was requested", retName, converted, name)
+				}
 			}
 			return nil
 		}
@@ -1662,7 +1658,7 @@ func generateRoleSteps(t *testing.T, useCSRs bool) []logicaltest.TestStep {
 		SubSubdomain         bool `structs:"foo.bar.example.com"`
 		SubSubdomainWildcard bool `structs:"*.bar.example.com"`
 		GlobDomain           bool `structs:"fooexample.com"`
-		NonHostname          bool `structs:"daɪˈɛrɨsɨs"`
+		IDN                  bool `structs:"daɪˈɛrɨsɨs"`
 		AnyHost              bool `structs:"porkslap.beer"`
 	}
 
@@ -1876,10 +1872,10 @@ func generateRoleSteps(t *testing.T, useCSRs bool) []logicaltest.TestStep {
 		roleVals.AllowAnyName = true
 		roleVals.EnforceHostnames = true
 		commonNames.AnyHost = true
+		commonNames.IDN = true
 		addCnTests()
 
 		roleVals.EnforceHostnames = false
-		commonNames.NonHostname = true
 		addCnTests()
 
 		// Ensure that we end up with acceptable key sizes since they won't be

builtin/logical/pki/cert_util.go

@@ -27,6 +27,7 @@ import (
 	"github.com/hashicorp/vault/logical"
 	"github.com/hashicorp/vault/logical/framework"
 	"github.com/ryanuber/go-glob"
+	"golang.org/x/net/idna"
 )
 
 type certExtKeyUsage int
@@ -91,7 +92,11 @@ func (b *caInfoBundle) GetCAChain() []*certutil.CertBlock {
 }
 
 var (
-	hostnameRegex                = regexp.MustCompile(`^(([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\-]*[a-zA-Z0-9])\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])$`)
+	// A note on hostnameRegex: although we set the StrictDomainName option
+	// when doing the idna conversion, this appears to only affect output, not
+	// input, so it will allow e.g. host^123.example.com straight through. So
+	// we still need to use this to check the output.
+	hostnameRegex                = regexp.MustCompile(`^(\*\.)?(([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\-]*[a-zA-Z0-9])\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])$`)
 	oidExtensionBasicConstraints = []int{2, 5, 29, 19}
 )
 
@@ -297,7 +302,15 @@ func validateNames(req *logical.Request, names []string, role *roleEntry) string
 		// ensure that we are not either checking a full email address or a
 		// wildcard prefix.
 		if role.EnforceHostnames {
-			if !hostnameRegex.MatchString(sanitizedName) {
+			p := idna.New(
+				idna.StrictDomainName(true),
+				idna.VerifyDNSLength(true),
+			)
+			converted, err := p.ToASCII(sanitizedName)
+			if err != nil {
+				return name
+			}
+			if !hostnameRegex.MatchString(converted) {
 				return name
 			}
 		}
@@ -413,7 +426,6 @@ func validateNames(req *logical.Request, names []string, role *roleEntry) string
 			}
 		}
 
-		//panic(fmt.Sprintf("\nName is %s\nRole is\n%#v\n", name, role))
 		return name
 	}
 
@@ -642,9 +654,18 @@ func generateCreationBundle(b *backend,
 				// used for the purpose for which they are presented
 				emailAddresses = append(emailAddresses, cn)
 			} else {
-				// Only add to dnsNames if it's actually a DNS name
+				// Only add to dnsNames if it's actually a DNS name but convert
-				if hostnameRegex.MatchString(cn) {
+				// idn first
-					dnsNames = append(dnsNames, cn)
+				p := idna.New(
+					idna.StrictDomainName(true),
+					idna.VerifyDNSLength(true),
+				)
+				converted, err := p.ToASCII(cn)
+				if err != nil {
+					return nil, errutil.UserError{Err: err.Error()}
+				}
+				if hostnameRegex.MatchString(converted) {
+					dnsNames = append(dnsNames, converted)
 				}
 			}
 		}
@@ -657,8 +678,18 @@ func generateCreationBundle(b *backend,
 					if strings.Contains(v, "@") {
 						emailAddresses = append(emailAddresses, v)
 					} else {
-						if hostnameRegex.MatchString(v) {
+						// Only add to dnsNames if it's actually a DNS name but
-							dnsNames = append(dnsNames, v)
+						// convert idn first
+						p := idna.New(
+							idna.StrictDomainName(true),
+							idna.VerifyDNSLength(true),
+						)
+						converted, err := p.ToASCII(v)
+						if err != nil {
+							return nil, errutil.UserError{Err: err.Error()}
+						}
+						if hostnameRegex.MatchString(converted) {
+							dnsNames = append(dnsNames, converted)
 						}
 					}
 				}

vendor/vendor.json

@@ -1647,8 +1647,8 @@
 		{
 			"checksumSHA1": "RcrB7tgYS/GMW4QrwVdMOTNqIU8=",
 			"path": "golang.org/x/net/idna",
-			"revision": "0ed95abb35c445290478a5348a7b38bb154135fd",
+			"revision": "f5dfe339be1d06f81b22525fe34671ee7d2c8904",
-			"revisionTime": "2018-01-24T06:08:02Z"
+			"revisionTime": "2018-02-04T03:50:36Z"
 		},
 		{
 			"checksumSHA1": "5JWn/wMC+EWNDKI/AYE4JifQF54=",