Repository: smallstep/crypto
Pull request #168 (2023-02-03 23:18:41)

Add support for extended SANs on certificate requests
### Description

This commit supports extended SANs like the permanent identifier used in device attestation flows with CSRs. Step already has support for this on certificates but not on certificate requests.

With this, it will be possible to run:
```shell
step certificate create --csr --template att.tpl \
--kms "yubikey:?pin-value=123456" --key "yubikey:slot-id=82" \
123456789 att.csr
```

Where att.tpl can be something like:
```tpl
{
  "subject": {{ toJson .Subject }},
  "sans": [{
    "type": "permanentIdentifier",
    "value": {{ toJson .Subject.CommonName }}
  }]
}
```



Patch:
diff --git a/x509util/certificate.go b/x509util/certificate.go
index e4938b1..317c100 100644
--- a/x509util/certificate.go
+++ b/x509util/certificate.go
@@ -71,7 +71,7 @@ func NewCertificate(cr *x509.CertificateRequest, opts ...Option) (*Certificate,
 	// Generate the subjectAltName extension if the certificate contains SANs
 	// that are not supported in the Go standard library.
 	if cert.hasExtendedSANs() && !cert.hasExtension(oidExtensionSubjectAltName) {
-		ext, err := createSubjectAltNameExtension(&cert, cert.Subject.IsEmpty())
+		ext, err := createCertificateSubjectAltNameExtension(cert, cert.Subject.IsEmpty())
 		if err != nil {
 			return nil, err
 		}
diff --git a/x509util/certificate_request.go b/x509util/certificate_request.go
index 161ff3e..563b6cc 100644
--- a/x509util/certificate_request.go
+++ b/x509util/certificate_request.go
@@ -57,6 +57,19 @@ func NewCertificateRequest(signer crypto.Signer, opts ...Option) (*CertificateRe
 	}
 	cr.PublicKey = pub
 	cr.Signer = signer
+
+	// Generate the subjectAltName extension if the certificate contains SANs
+	// that are not supported in the Go standard library.
+	if cr.hasExtendedSANs() && !cr.hasExtension(oidExtensionSubjectAltName) {
+		ext, err := createCertificateRequestSubjectAltNameExtension(cr, cr.Subject.IsEmpty())
+		if err != nil {
+			return nil, err
+		}
+		// Prepend extension to achieve a certificate as similar as possible to
+		// the one generated by the Go standard library.
+		cr.Extensions = append([]Extension{ext}, cr.Extensions...)
+	}
+
 	return &cr, nil
 }
 
@@ -149,6 +162,30 @@ func (c *CertificateRequest) GetLeafCertificate() *Certificate {
 	return cert
 }
 
+// hasExtendedSANs returns true if the certificate contains any SAN types that
+// are not supported by the golang x509 library (i.e. RegisteredID, OtherName,
+// DirectoryName, X400Address, or EDIPartyName)
+//
+// See also https://datatracker.ietf.org/doc/html/rfc5280.html#section-4.2.1.6
+func (c *CertificateRequest) hasExtendedSANs() bool {
+	for _, san := range c.SANs {
+		if !(san.Type == DNSType || san.Type == EmailType || san.Type == IPType || san.Type == URIType || san.Type == AutoType || san.Type == "") {
+			return true
+		}
+	}
+	return false
+}
+
+// hasExtension returns true if the given extension oid is in the certificate.
+func (c *CertificateRequest) hasExtension(oid ObjectIdentifier) bool {
+	for _, e := range c.Extensions {
+		if e.ID.Equal(oid) {
+			return true
+		}
+	}
+	return false
+}
+
 // CreateCertificateRequest creates a simple X.509 certificate request with the
 // given common name and sans.
 func CreateCertificateRequest(commonName string, sans []string, signer crypto.Signer) (*x509.CertificateRequest, error) {
diff --git a/x509util/extensions.go b/x509util/extensions.go
index 3bbdd4b..c94289b 100644
--- a/x509util/extensions.go
+++ b/x509util/extensions.go
@@ -922,6 +922,14 @@ func (s *SerialNumber) UnmarshalJSON(data []byte) error {
 	return nil
 }
 
+func createCertificateSubjectAltNameExtension(c Certificate, subjectIsEmpty bool) (Extension, error) {
+	return createSubjectAltNameExtension(c.DNSNames, c.EmailAddresses, c.IPAddresses, c.URIs, c.SANs, subjectIsEmpty)
+}
+
+func createCertificateRequestSubjectAltNameExtension(c CertificateRequest, subjectIsEmpty bool) (Extension, error) {
+	return createSubjectAltNameExtension(c.DNSNames, c.EmailAddresses, c.IPAddresses, c.URIs, c.SANs, subjectIsEmpty)
+}
+
 // createSubjectAltNameExtension will construct an Extension containing all
 // SubjectAlternativeNames held in a Certificate. It implements more types than
 // the golang x509 library, so it is used whenever OtherName or RegisteredID
@@ -931,11 +939,11 @@ func (s *SerialNumber) UnmarshalJSON(data []byte) error {
 //
 // TODO(mariano,unreality): X400Address, DirectoryName, and EDIPartyName types
 // are defined in RFC5280 but are currently unimplemented
-func createSubjectAltNameExtension(c *Certificate, subjectIsEmpty bool) (Extension, error) {
+func createSubjectAltNameExtension(dnsNames, emailAddresses MultiString, ipAddresses MultiIP, uris MultiURL, sans []SubjectAlternativeName, subjectIsEmpty bool) (Extension, error) {
 	var zero Extension
 
 	var rawValues []asn1.RawValue
-	for _, dnsName := range c.DNSNames {
+	for _, dnsName := range dnsNames {
 		rawValue, err := SubjectAlternativeName{
 			Type: DNSType, Value: dnsName,
 		}.RawValue()
@@ -946,7 +954,7 @@ func createSubjectAltNameExtension(c *Certificate, subjectIsEmpty bool) (Extensi
 		rawValues = append(rawValues, rawValue)
 	}
 
-	for _, emailAddress := range c.EmailAddresses {
+	for _, emailAddress := range emailAddresses {
 		rawValue, err := SubjectAlternativeName{
 			Type: EmailType, Value: emailAddress,
 		}.RawValue()
@@ -957,9 +965,9 @@ func createSubjectAltNameExtension(c *Certificate, subjectIsEmpty bool) (Extensi
 		rawValues = append(rawValues, rawValue)
 	}
 
-	for _, uri := range c.URIs {
+	for _, ip := range ipAddresses {
 		rawValue, err := SubjectAlternativeName{
-			Type: URIType, Value: uri.String(),
+			Type: IPType, Value: ip.String(),
 		}.RawValue()
 		if err != nil {
 			return zero, err
@@ -968,9 +976,9 @@ func createSubjectAltNameExtension(c *Certificate, subjectIsEmpty bool) (Extensi
 		rawValues = append(rawValues, rawValue)
 	}
 
-	for _, ip := range c.IPAddresses {
+	for _, uri := range uris {
 		rawValue, err := SubjectAlternativeName{
-			Type: IPType, Value: ip.String(),
+			Type: URIType, Value: uri.String(),
 		}.RawValue()
 		if err != nil {
 			return zero, err
@@ -979,7 +987,7 @@ func createSubjectAltNameExtension(c *Certificate, subjectIsEmpty bool) (Extensi
 		rawValues = append(rawValues, rawValue)
 	}
 
-	for _, san := range c.SANs {
+	for _, san := range sans {
 		rawValue, err := san.RawValue()
 		if err != nil {
 			return zero, err


Test patch:
diff --git a/x509util/certificate_request_test.go b/x509util/certificate_request_test.go
index 6e200e6..bf70793 100644
--- a/x509util/certificate_request_test.go
+++ b/x509util/certificate_request_test.go
@@ -8,6 +8,8 @@ import (
 	"crypto/x509"
 	"crypto/x509/pkix"
 	"encoding/asn1"
+	"encoding/base64"
+	"fmt"
 	"net"
 	"net/url"
 	"reflect"
@@ -20,6 +22,49 @@ func TestNewCertificateRequest(t *testing.T) {
 		t.Fatal(err)
 	}
 
+	// ok extended sans
+	sans := []SubjectAlternativeName{
+		{Type: DNSType, Value: "foo.com"},
+		{Type: EmailType, Value: "root@foo.com"},
+		{Type: IPType, Value: "3.14.15.92"},
+		{Type: URIType, Value: "mailto:root@foo.com"},
+		{Type: PermanentIdentifierType, Value: "123456789"},
+	}
+	extendedSANs := CreateTemplateData("123456789", nil)
+	extendedSANs.SetSubjectAlternativeNames(sans...)
+	extendedSANsExtension, err := createSubjectAltNameExtension(nil, nil, nil, nil, sans, false)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// ok extended sans and extension
+	extendedSANsAndExtensionsTemplate := fmt.Sprintf(`{
+		"subject": {{ toJson .Subject }},
+		"sans": {{ toJson .SANs }},
+		"extensions": [
+			{"id":"2.5.29.17", "value":"%s"}
+		]
+	}`, base64.StdEncoding.EncodeToString(extendedSANsExtension.Value))
+
+	// ok permanent identifier template
+	permanentIdentifierTemplate := `{ 
+		"subject": {{ toJson .Subject }},
+		"sans": [{
+			"type": "permanentIdentifier", 
+			"value": {{ toJson .Subject.CommonName }}
+		}]
+	}`
+	permanentIdentifierTemplateExtension, err := createSubjectAltNameExtension(nil, nil, nil, nil, []SubjectAlternativeName{
+		{Type: PermanentIdentifierType, Value: "123456789"},
+	}, false)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// fail extended sans
+	failExtendedSANs := CreateTemplateData("123456789", nil)
+	failExtendedSANs.SetSubjectAlternativeNames(SubjectAlternativeName{Type: "badType", Value: "foo.com"})
+
 	type args struct {
 		signer crypto.Signer
 		opts   []Option
@@ -47,8 +92,50 @@ func TestNewCertificateRequest(t *testing.T) {
 			PublicKey: signer.Public(),
 			Signer:    signer,
 		}, false},
+		{"ok extended sans", args{signer, []Option{
+			WithTemplate(DefaultCertificateRequestTemplate, extendedSANs),
+		}}, &CertificateRequest{
+			Subject: Subject{CommonName: "123456789"},
+			SANs: []SubjectAlternativeName{
+				{Type: "dns", Value: "foo.com"},
+				{Type: "email", Value: "root@foo.com"},
+				{Type: "ip", Value: "3.14.15.92"},
+				{Type: "uri", Value: "mailto:root@foo.com"},
+				{Type: "permanentIdentifier", Value: "123456789"},
+			},
+			Extensions: []Extension{extendedSANsExtension},
+			PublicKey:  signer.Public(),
+			Signer:     signer,
+		}, false},
+		{"ok extended sans and extension", args{signer, []Option{
+			WithTemplate(extendedSANsAndExtensionsTemplate, extendedSANs),
+		}}, &CertificateRequest{
+			Subject: Subject{CommonName: "123456789"},
+			SANs: []SubjectAlternativeName{
+				{Type: "dns", Value: "foo.com"},
+				{Type: "email", Value: "root@foo.com"},
+				{Type: "ip", Value: "3.14.15.92"},
+				{Type: "uri", Value: "mailto:root@foo.com"},
+				{Type: "permanentIdentifier", Value: "123456789"},
+			},
+			Extensions: []Extension{extendedSANsExtension},
+			PublicKey:  signer.Public(),
+			Signer:     signer,
+		}, false},
+		{"ok permanent identifier template", args{signer, []Option{
+			WithTemplate(permanentIdentifierTemplate, CreateTemplateData("123456789", []string{})),
+		}}, &CertificateRequest{
+			Subject: Subject{CommonName: "123456789"},
+			SANs: []SubjectAlternativeName{
+				{Type: "permanentIdentifier", Value: "123456789"},
+			},
+			Extensions: []Extension{permanentIdentifierTemplateExtension},
+			PublicKey:  signer.Public(),
+			Signer:     signer,
+		}, false},
 		{"fail apply", args{signer, []Option{WithTemplateFile("testdata/missing.tpl", NewTemplateData())}}, nil, true},
 		{"fail unmarshal", args{signer, []Option{WithTemplate("{badjson", NewTemplateData())}}, nil, true},
+		{"fail extended sans", args{signer, []Option{WithTemplate(DefaultCertificateRequestTemplate, failExtendedSANs)}}, nil, true},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -58,7 +145,7 @@ func TestNewCertificateRequest(t *testing.T) {
 				return
 			}
 			if !reflect.DeepEqual(got, tt.want) {
-				t.Errorf("NewCertificateRequest() = %v, want %v", got, tt.want)
+				t.Errorf("NewCertificateRequest() = %+v, want %v", got, tt.want)
 			}
 		})
 	}
diff --git a/x509util/extensions_test.go b/x509util/extensions_test.go
index ab6e6ed..2a03de1 100644
--- a/x509util/extensions_test.go
+++ b/x509util/extensions_test.go
@@ -127,6 +127,29 @@ func TestExtension_Set(t *testing.T) {
 	}
 }
 
+func TestObjectIdentifier_Equal(t *testing.T) {
+	type args struct {
+		v ObjectIdentifier
+	}
+	tests := []struct {
+		name string
+		o    ObjectIdentifier
+		args args
+		want bool
+	}{
+		{"ok", ObjectIdentifier{1, 2, 3, 4}, args{ObjectIdentifier{1, 2, 3, 4}}, true},
+		{"false length", ObjectIdentifier{1, 2, 3}, args{ObjectIdentifier{1, 2, 3, 4}}, false},
+		{"false content", ObjectIdentifier{1, 2, 3, 5}, args{ObjectIdentifier{1, 2, 3, 4}}, false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := tt.o.Equal(tt.args.v); got != tt.want {
+				t.Errorf("ObjectIdentifier.Equal() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
 func TestObjectIdentifier_MarshalJSON(t *testing.T) {
 	tests := []struct {
 		name    string
@@ -1203,7 +1226,7 @@ func TestSerialNumber_UnmarshalJSON(t *testing.T) {
 
 func Test_createSubjectAltNameExtension(t *testing.T) {
 	type args struct {
-		c              *Certificate
+		c              Certificate
 		subjectIsEmpty bool
 	}
 	tests := []struct {
@@ -1212,42 +1235,42 @@ func Test_createSubjectAltNameExtension(t *testing.T) {
 		want    Extension
 		wantErr bool
 	}{
-		{"ok dns", args{&Certificate{
+		{"ok dns", args{Certificate{
 			DNSNames: []string{"foo.com"},
 		}, false}, Extension{
 			ID:       oidExtensionSubjectAltName,
 			Critical: false,
 			Value:    append([]byte{0x30, 9, 0x80 | nameTypeDNS, 7}, []byte("foo.com")...),
 		}, false},
-		{"ok dns critical", args{&Certificate{
+		{"ok dns critical", args{Certificate{
 			DNSNames: []string{"foo.com"},
 		}, true}, Extension{
 			ID:       oidExtensionSubjectAltName,
 			Critical: true,
 			Value:    append([]byte{0x30, 9, 0x80 | nameTypeDNS, 7}, []byte("foo.com")...),
 		}, false},
-		{"ok email", args{&Certificate{
+		{"ok email", args{Certificate{
 			EmailAddresses: []string{"bar@foo.com"},
 		}, false}, Extension{
 			ID:       oidExtensionSubjectAltName,
 			Critical: false,
 			Value:    append([]byte{0x30, 13, 0x80 | nameTypeEmail, 11}, []byte("bar@foo.com")...),
 		}, false},
-		{"ok uri", args{&Certificate{
+		{"ok uri", args{Certificate{
 			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{
+		{"ok ip", args{Certificate{
 			IPAddresses: []net.IP{net.ParseIP("1.2.3.4")},
 		}, false}, Extension{
 			ID:       oidExtensionSubjectAltName,
 			Critical: false,
 			Value:    []byte{0x30, 6, 0x80 | nameTypeIP, 4, 1, 2, 3, 4},
 		}, false},
-		{"ok sans", args{&Certificate{
+		{"ok sans", args{Certificate{
 			SANs: []SubjectAlternativeName{
 				{Type: "dns", Value: "foo.com"},
 				{Type: "email", Value: "bar@foo.com"},
@@ -1265,7 +1288,7 @@ func Test_createSubjectAltNameExtension(t *testing.T) {
 				{0x80 | nameTypeIP, 4, 1, 2, 3, 4},
 			}, nil),
 		}, false},
-		{"ok otherName", args{&Certificate{
+		{"ok otherName", args{Certificate{
 			SANs: []SubjectAlternativeName{
 				{Type: "dns", Value: "foo.com"},
 				{Type: "1.2.3.4", Value: "utf8:bar@foo.com"},
@@ -1280,19 +1303,19 @@ func Test_createSubjectAltNameExtension(t *testing.T) {
 				{0xA0, 13, asn1.TagUTF8String, 11}, []byte("bar@foo.com"),
 			}, nil),
 		}, false},
-		{"fail dns", args{&Certificate{
+		{"fail dns", args{Certificate{
 			DNSNames: []string{""},
 		}, false}, Extension{}, true},
-		{"fail email", args{&Certificate{
+		{"fail email", args{Certificate{
 			EmailAddresses: []string{"nöt@ia5.com"},
 		}, false}, Extension{}, true},
-		{"fail uri", args{&Certificate{
+		{"fail uri", args{Certificate{
 			URIs: []*url.URL{{Scheme: "urn", Opaque: "nöt:ia5"}},
 		}, false}, Extension{}, true},
-		{"fail ip", args{&Certificate{
+		{"fail ip", args{Certificate{
 			IPAddresses: []net.IP{{1, 2, 3}},
 		}, false}, Extension{}, true},
-		{"fail otherName", args{&Certificate{
+		{"fail otherName", args{Certificate{
 			SANs: []SubjectAlternativeName{
 				{Type: "1.2.3.4", Value: "int:bar@foo.com"},
 			},
@@ -1300,13 +1323,30 @@ func Test_createSubjectAltNameExtension(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			got, err := createSubjectAltNameExtension(tt.args.c, tt.args.subjectIsEmpty)
+			gotCert, err := createCertificateSubjectAltNameExtension(tt.args.c, tt.args.subjectIsEmpty)
 			if (err != nil) != tt.wantErr {
-				t.Errorf("createSubjectAltNameExtension() error = %v, wantErr %v", err, tt.wantErr)
+				t.Errorf("createCertificateSubjectAltNameExtension() error = %v, wantErr %v", err, tt.wantErr)
 				return
 			}
-			if !reflect.DeepEqual(got, tt.want) {
-				t.Errorf("createSubjectAltNameExtension() = \n%v, want \n%v", got, tt.want)
+			if !reflect.DeepEqual(gotCert, tt.want) {
+				t.Errorf("createCertificateSubjectAltNameExtension() = %v, want %v", gotCert, tt.want)
+			}
+
+			cr := CertificateRequest{
+				DNSNames:       tt.args.c.DNSNames,
+				EmailAddresses: tt.args.c.EmailAddresses,
+				IPAddresses:    tt.args.c.IPAddresses,
+				URIs:           tt.args.c.URIs,
+				SANs:           tt.args.c.SANs,
+			}
+
+			gotCSR, err := createCertificateRequestSubjectAltNameExtension(cr, tt.args.subjectIsEmpty)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("createCertificateRequestSubjectAltNameExtension() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if !reflect.DeepEqual(gotCSR, tt.want) {
+				t.Errorf("createCertificateRequestSubjectAltNameExtension() = %v, want %v", gotCSR, tt.want)
 			}
 		})
 	}