Merge pull request #867 from smallstep/mariano/empty-sans

Skip empty SANs

Author: maraino

x509util/certificate_test.go

@@ -146,6 +146,8 @@ func TestNewCertificate(t *testing.T) {
 	customSANsData.Set(SANsKey, []SubjectAlternativeName{
 		{Type: PermanentIdentifierType, Value: "123456"},
 		{Type: "1.2.3.4", Value: "utf8:otherName"},
+		// An empty SAN won't be encoded
+		{Type: "auto", Value: ""},
 	})
 	badCustomSANsData := CreateTemplateData("commonName", nil)
 	badCustomSANsData.Set(SANsKey, []SubjectAlternativeName{
@@ -202,6 +204,7 @@ func TestNewCertificate(t *testing.T) {
 			SANs: []SubjectAlternativeName{
 				{Type: PermanentIdentifierType, Value: "123456"},
 				{Type: "1.2.3.4", Value: "utf8:otherName"},
+				{Type: "auto", Value: ""},
 			},
 			Extensions: []Extension{{
 				ID:       ObjectIdentifier{2, 5, 29, 17},
@@ -378,6 +381,7 @@ func TestNewCertificateTemplate(t *testing.T) {
 		(dict "type" "hardwareModuleName" "value" .Insecure.User.hmn)
 		(dict "type" "userPrincipalName" "value" .Token.upn)
 		(dict "type" "1.2.3.4" "value" (printf "int:%s" .Insecure.User.id))
+		(dict "type" "auto" "value" "")
 	) | toJson }},
 	"notBefore": "{{ .Token.nbf | formatTime }}",
 	"notAfter": {{ now | dateModify "24h" | toJson }},

x509util/extensions.go

@@ -339,8 +339,13 @@ type SubjectAlternativeName struct {
 	ASN1Value json.RawMessage `json:"asn1Value,omitempty"`
 }
 
-// Set sets the subject alternative name in the given x509.Certificate.
+// Set sets the subject alternative name in the given x509.Certificate. This
+// method will not set any SAN if the value is empty.
 func (s SubjectAlternativeName) Set(c *x509.Certificate) {
+	if s.Value == "" {
+		return
+	}
+
 	switch strings.ToLower(s.Type) {
 	case DNSType:
 		c.DNSNames = append(c.DNSNames, s.Value)
@@ -1215,6 +1220,10 @@ func createSubjectAltNameExtension(dnsNames, emailAddresses MultiString, ipAddre
 
 	var rawValues []asn1.RawValue
 	for _, dnsName := range dnsNames {
+		if dnsName == "" {
+			continue
+		}
+
 		rawValue, err := SubjectAlternativeName{
 			Type: DNSType, Value: dnsName,
 		}.RawValue()
@@ -1226,6 +1235,10 @@ func createSubjectAltNameExtension(dnsNames, emailAddresses MultiString, ipAddre
 	}
 
 	for _, emailAddress := range emailAddresses {
+		if emailAddress == "" {
+			continue
+		}
+
 		rawValue, err := SubjectAlternativeName{
 			Type: EmailType, Value: emailAddress,
 		}.RawValue()
@@ -1237,6 +1250,10 @@ func createSubjectAltNameExtension(dnsNames, emailAddresses MultiString, ipAddre
 	}
 
 	for _, ip := range ipAddresses {
+		if len(ip) == 0 {
+			continue
+		}
+
 		rawValue, err := SubjectAlternativeName{
 			Type: IPType, Value: ip.String(),
 		}.RawValue()
@@ -1248,8 +1265,13 @@ func createSubjectAltNameExtension(dnsNames, emailAddresses MultiString, ipAddre
 	}
 
 	for _, uri := range uris {
+		v := uri.String()
+		if v == "" {
+			continue
+		}
+
 		rawValue, err := SubjectAlternativeName{
-			Type: URIType, Value: uri.String(),
+			Type: URIType, Value: v,
 		}.RawValue()
 		if err != nil {
 			return zero, err
@@ -1259,6 +1281,10 @@ func createSubjectAltNameExtension(dnsNames, emailAddresses MultiString, ipAddre
 	}
 
 	for _, san := range sans {
+		if san.Value == "" && len(san.ASN1Value) == 0 {
+			continue
+		}
+
 		rawValue, err := san.RawValue()
 		if err != nil {
 			return zero, err

x509util/extensions_test.go

@@ -243,6 +243,7 @@ func TestSubjectAlternativeName_Set(t *testing.T) {
 		{"AutoIPAdd", fields{"", "::1"}, args{&x509.Certificate{IPAddresses: []net.IP{net.ParseIP("127.0.0.1")}}}, &x509.Certificate{IPAddresses: []net.IP{net.ParseIP("127.0.0.1"), net.ParseIP("::1")}}},
 		{"AutoURI", fields{"Auto", "https://foo.com"}, args{&x509.Certificate{}}, &x509.Certificate{URIs: []*url.URL{{Scheme: "https", Host: "foo.com"}}}},
 		{"AutoURIAdd", fields{"", "uri:foo:bar"}, args{&x509.Certificate{URIs: []*url.URL{{Scheme: "https", Host: "foo.com"}}}}, &x509.Certificate{URIs: []*url.URL{{Scheme: "https", Host: "foo.com"}, {Scheme: "uri", Opaque: "foo:bar"}}}},
+		{"skipEmpty", fields{"auto", ""}, args{&x509.Certificate{}}, &x509.Certificate{}},
 		{"panic", fields{"panic", "foo.com"}, args{&x509.Certificate{}}, &x509.Certificate{DNSNames: []string{"foo.com"}}},
 		{"panicAdd", fields{"panic", "bar.com"}, args{&x509.Certificate{DNSNames: []string{"foo.com"}}}, &x509.Certificate{DNSNames: []string{"foo.com"}}},
 	}
@@ -1249,7 +1250,7 @@ func Test_createSubjectAltNameExtension(t *testing.T) {
 		wantErr bool
 	}{
 		{"ok dns", args{Certificate{
-			DNSNames: []string{"foo.com"},
+			DNSNames: []string{"foo.com", ""},
 		}, false}, Extension{
 			ID:       oidExtensionSubjectAltName,
 			Critical: false,
@@ -1263,21 +1264,21 @@ func Test_createSubjectAltNameExtension(t *testing.T) {
 			Value:    append([]byte{0x30, 9, 0x80 | nameTypeDNS, 7}, []byte("foo.com")...),
 		}, false},
 		{"ok email", args{Certificate{
-			EmailAddresses: []string{"[email protected]"},
+			EmailAddresses: []string{"[email protected]", ""},
 		}, false}, Extension{
 			ID:       oidExtensionSubjectAltName,
 			Critical: false,
 			Value:    append([]byte{0x30, 13, 0x80 | nameTypeEmail, 11}, []byte("[email protected]")...),
 		}, false},
 		{"ok uri", args{Certificate{
-			URIs: []*url.URL{{Scheme: "urn", Opaque: "foo:bar"}},
+			URIs: []*url.URL{{Scheme: "urn", Opaque: "foo:bar"}, {}},
 		}, false}, Extension{
 			ID:       oidExtensionSubjectAltName,
 			Critical: false,
 			Value:    append([]byte{0x30, 13, 0x80 | nameTypeURI, 11}, []byte("urn:foo:bar")...),
 		}, false},
 		{"ok ip", args{Certificate{
-			IPAddresses: []net.IP{net.ParseIP("1.2.3.4")},
+			IPAddresses: []net.IP{net.ParseIP("1.2.3.4"), {}},
 		}, false}, Extension{
 			ID:       oidExtensionSubjectAltName,
 			Critical: false,
@@ -1289,6 +1290,7 @@ func Test_createSubjectAltNameExtension(t *testing.T) {
 				{Type: "email", Value: "[email protected]"},
 				{Type: "uri", Value: "urn:foo:bar"},
 				{Type: "ip", Value: "1.2.3.4"},
+				{Type: "auto", Value: ""},
 			},
 		}, false}, Extension{
 			ID:       oidExtensionSubjectAltName,
@@ -1322,7 +1324,7 @@ func Test_createSubjectAltNameExtension(t *testing.T) {
 			}, nil),
 		}, false},
 		{"fail dns", args{Certificate{
-			DNSNames: []string{""},
+			DNSNames: []string{"xn--bücher.example.com"},
 		}, false}, Extension{}, true},
 		{"fail email", args{Certificate{
 			EmailAddresses: []string{"nö[email protected]"},

x509util/utils.go

@@ -55,6 +55,9 @@ func SplitSANs(sans []string) (dnsNames []string, ips []net.IP, emails []string,
 	emails = []string{}
 	uris = []*url.URL{}
 	for _, san := range sans {
+		if san == "" {
+			continue
+		}
 		ip := net.ParseIP(san)
 		u, err := url.Parse(san)
 		switch {

x509util/utils_test.go

@@ -61,7 +61,7 @@ func TestSplitSANs(t *testing.T) {
 			{Scheme: "mailto", Opaque: "[email protected]"},
 			{Scheme: "urn", Opaque: "uuid:ddfe62ba-7e99-4bc1-83b3-8f57fe3e9959"},
 		}},
-		{"mixed", args{[]string{
+		{"mixed", args{[]string{"",
 			"foo.internal", "https://ca.smallstep.com", "[email protected]",
 			"urn:uuid:ddfe62ba-7e99-4bc1-83b3-8f57fe3e9959", "[email protected]",
 			"1.1.1.1", "bar.internal", "https://google.com/index.html",