Repository: smallstep/crypto
Pull request #248 (2023-05-25 21:20:57)

Add minimal `NewCertificateFromX509` implementation
Minimal implementation of `NewCertificateFromX509` to create an `x509.Certificate` without signing a CSR.

This PR replaces #239 and #247. We decided on keeping the logic simple, and to not add generic processing of `x509.Certificate` and `x509.CertificateRequest` for now.

The changes from the other PRs might still be of interest in the (near) future when more options are added for manipulating a `x509.Certificate` or `x509.CertificateRequest`, so we may reconsider the decision by then.


Patch:
diff --git a/x509util/certificate.go b/x509util/certificate.go
index 317c100..918e28d 100644
--- a/x509util/certificate.go
+++ b/x509util/certificate.go
@@ -40,8 +40,8 @@ type Certificate struct {
 	PublicKey             interface{}              `json:"-"`
 }
 
-// NewCertificate creates a new Certificate from an x509.Certificate request and
-// some template options.
+// NewCertificate creates a new Certificate from an x509.CertificateRequest and
+// will apply some template options.
 func NewCertificate(cr *x509.CertificateRequest, opts ...Option) (*Certificate, error) {
 	if err := cr.CheckSignature(); err != nil {
 		return nil, errors.Wrap(err, "error validating certificate request")
@@ -52,10 +52,44 @@ func NewCertificate(cr *x509.CertificateRequest, opts ...Option) (*Certificate,
 		return nil, err
 	}
 
-	// If no template use only the certificate request with the default leaf key
-	// usages.
+	return newCertificateWithOptions(cr, o)
+}
+
+// NewCertificateFromX509 creates a new Certificate from an x509.Certificate and
+// will apply template options. A new (unsigned) x509.CertificateRequest is created,
+// with data from the x509.Certificate template. This function is primarily useful
+// when signing a certificate for a key that can't sign a CSR or when the private
+// key is not available.
+func NewCertificateFromX509(template *x509.Certificate, opts ...Option) (*Certificate, error) {
+	// Copy data from the template to a new, unsigned CSR.
+	csr := &x509.CertificateRequest{
+		PublicKey:          template.PublicKey,
+		PublicKeyAlgorithm: template.PublicKeyAlgorithm,
+		Subject:            template.Subject,
+		DNSNames:           template.DNSNames,
+		EmailAddresses:     template.EmailAddresses,
+		IPAddresses:        template.IPAddresses,
+		URIs:               template.URIs,
+		Extensions:         template.ExtraExtensions,
+	}
+
+	o, err := new(Options).apply(csr, opts)
+	if err != nil {
+		return nil, err
+	}
+
+	return newCertificateWithOptions(csr, o)
+}
+
+// newCertificateWithOptions creates a new Certificate from an x509.CertificateRequest
+// with options applied. If no template was applied, the data from the x509.CertificateRequest
+// will simply be copied over and returned with the default leaf key usages. Otherwise, the
+// data from the template will be filled in.
+func newCertificateWithOptions(csr *x509.CertificateRequest, o *Options) (*Certificate, error) {
+	// If no template is set, use only the certificate request with the
+	// default leaf key usages.
 	if o.CertBuffer == nil {
-		return NewCertificateRequestFromX509(cr).GetLeafCertificate(), nil
+		return NewCertificateRequestFromX509(csr).GetLeafCertificate(), nil
 	}
 
 	// With templates
@@ -64,9 +98,9 @@ func NewCertificate(cr *x509.CertificateRequest, opts ...Option) (*Certificate,
 		return nil, errors.Wrap(err, "error unmarshaling certificate")
 	}
 
-	// Complete with certificate request
-	cert.PublicKey = cr.PublicKey
-	cert.PublicKeyAlgorithm = cr.PublicKeyAlgorithm
+	// Enforce the public key
+	cert.PublicKey = csr.PublicKey
+	cert.PublicKeyAlgorithm = csr.PublicKeyAlgorithm
 
 	// Generate the subjectAltName extension if the certificate contains SANs
 	// that are not supported in the Go standard library.


Test patch:
diff --git a/x509util/certificate_test.go b/x509util/certificate_test.go
index 9bae360..2aae98d 100644
--- a/x509util/certificate_test.go
+++ b/x509util/certificate_test.go
@@ -3,7 +3,9 @@ package x509util
 import (
 	"bytes"
 	"crypto"
+	"crypto/ecdsa"
 	"crypto/ed25519"
+	"crypto/elliptic"
 	"crypto/rand"
 	"crypto/x509"
 	"crypto/x509/pkix"
@@ -17,6 +19,9 @@ import (
 	"reflect"
 	"testing"
 	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func createCertificateRequest(t *testing.T, commonName string, sans []string) (*x509.CertificateRequest, crypto.Signer) {
@@ -287,6 +292,163 @@ func TestNewCertificate(t *testing.T) {
 	}
 }
 
+func TestNewCertificateFromX509(t *testing.T) {
+	priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	require.NoError(t, err)
+	template := &x509.Certificate{ // similar template as the certificate request for TestNewCertificate
+		PublicKey:          priv.Public(),
+		PublicKeyAlgorithm: x509.ECDSA,
+		Subject:            pkix.Name{CommonName: "commonName"},
+		DNSNames:           []string{"foo.com"},
+		EmailAddresses:     []string{"root@foo.com"},
+	}
+	customSANsData := CreateTemplateData("commonName", nil)
+	customSANsData.Set(SANsKey, []SubjectAlternativeName{
+		{Type: PermanentIdentifierType, Value: "123456"},
+		{Type: "1.2.3.4", Value: "utf8:otherName"},
+	})
+	badCustomSANsData := CreateTemplateData("commonName", nil)
+	badCustomSANsData.Set(SANsKey, []SubjectAlternativeName{
+		{Type: "1.2.3.4", Value: "int:not-an-int"},
+	})
+	ipNet := func(s string) *net.IPNet {
+		_, ipNet, err := net.ParseCIDR(s)
+		require.NoError(t, err)
+		return ipNet
+	}
+	type args struct {
+		template *x509.Certificate
+		opts     []Option
+	}
+	tests := []struct {
+		name    string
+		args    args
+		want    *Certificate
+		wantErr bool
+	}{
+		{"okSimple", args{template, nil}, &Certificate{
+			Subject:        Subject{CommonName: "commonName"},
+			DNSNames:       []string{"foo.com"},
+			EmailAddresses: []string{"root@foo.com"},
+			KeyUsage:       KeyUsage(x509.KeyUsageDigitalSignature),
+			ExtKeyUsage: ExtKeyUsage([]x509.ExtKeyUsage{
+				x509.ExtKeyUsageServerAuth,
+				x509.ExtKeyUsageClientAuth,
+			}),
+			Extensions:         newExtensions(template.Extensions),
+			PublicKey:          priv.Public(),
+			PublicKeyAlgorithm: x509.ECDSA,
+			SignatureAlgorithm: SignatureAlgorithm(x509.UnknownSignatureAlgorithm),
+		}, false},
+		{"okDefaultTemplate", args{template, []Option{WithTemplate(DefaultLeafTemplate, CreateTemplateData("commonName", []string{"foo.com"}))}}, &Certificate{
+			Subject:  Subject{CommonName: "commonName"},
+			SANs:     []SubjectAlternativeName{{Type: DNSType, Value: "foo.com"}},
+			KeyUsage: KeyUsage(x509.KeyUsageDigitalSignature),
+			ExtKeyUsage: ExtKeyUsage([]x509.ExtKeyUsage{
+				x509.ExtKeyUsageServerAuth,
+				x509.ExtKeyUsageClientAuth,
+			}),
+			PublicKey:          priv.Public(),
+			PublicKeyAlgorithm: x509.ECDSA,
+		}, false},
+		{"okCustomSANs", args{template, []Option{WithTemplate(DefaultLeafTemplate, customSANsData)}}, &Certificate{
+			Subject: Subject{CommonName: "commonName"},
+			SANs: []SubjectAlternativeName{
+				{Type: PermanentIdentifierType, Value: "123456"},
+				{Type: "1.2.3.4", Value: "utf8:otherName"},
+			},
+			Extensions: []Extension{{
+				ID:       ObjectIdentifier{2, 5, 29, 17},
+				Critical: false,
+				Value:    []byte{48, 44, 160, 22, 6, 8, 43, 6, 1, 5, 5, 7, 8, 3, 160, 10, 48, 8, 12, 6, 49, 50, 51, 52, 53, 54, 160, 18, 6, 3, 42, 3, 4, 160, 11, 12, 9, 111, 116, 104, 101, 114, 78, 97, 109, 101},
+			}},
+			KeyUsage: KeyUsage(x509.KeyUsageDigitalSignature),
+			ExtKeyUsage: ExtKeyUsage([]x509.ExtKeyUsage{
+				x509.ExtKeyUsageServerAuth,
+				x509.ExtKeyUsageClientAuth,
+			}),
+			PublicKey:          priv.Public(),
+			PublicKeyAlgorithm: x509.ECDSA,
+		}, false},
+		{"okExample", args{template, []Option{WithTemplateFile("./testdata/example.tpl", TemplateData{
+			SANsKey: []SubjectAlternativeName{
+				{Type: "dns", Value: "foo.com"},
+			},
+			TokenKey: map[string]interface{}{
+				"iss": "https://iss",
+				"sub": "sub",
+			},
+		})}}, &Certificate{
+			Subject:        Subject{CommonName: "commonName"},
+			SANs:           []SubjectAlternativeName{{Type: DNSType, Value: "foo.com"}},
+			EmailAddresses: []string{"root@foo.com"},
+			URIs:           []*url.URL{{Scheme: "https", Host: "iss", Fragment: "sub"}},
+			KeyUsage:       KeyUsage(x509.KeyUsageDigitalSignature),
+			ExtKeyUsage: ExtKeyUsage([]x509.ExtKeyUsage{
+				x509.ExtKeyUsageServerAuth,
+				x509.ExtKeyUsageClientAuth,
+			}),
+			PublicKey:          priv.Public(),
+			PublicKeyAlgorithm: x509.ECDSA,
+		}, false},
+		{"okFullSimple", args{template, []Option{WithTemplateFile("./testdata/fullsimple.tpl", TemplateData{})}}, &Certificate{
+			Version:               3,
+			Subject:               Subject{CommonName: "subjectCommonName"},
+			SerialNumber:          SerialNumber{big.NewInt(78187493520)},
+			Issuer:                Issuer{CommonName: "issuerCommonName"},
+			DNSNames:              []string{"doe.com"},
+			IPAddresses:           []net.IP{net.ParseIP("127.0.0.1")},
+			EmailAddresses:        []string{"jane@doe.com"},
+			URIs:                  []*url.URL{{Scheme: "https", Host: "doe.com"}},
+			SANs:                  []SubjectAlternativeName{{Type: DNSType, Value: "www.doe.com"}},
+			Extensions:            []Extension{{ID: []int{1, 2, 3, 4}, Critical: true, Value: []byte("extension")}},
+			KeyUsage:              KeyUsage(x509.KeyUsageDigitalSignature),
+			ExtKeyUsage:           ExtKeyUsage([]x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}),
+			UnknownExtKeyUsage:    []asn1.ObjectIdentifier{[]int{1, 3, 6, 1, 4, 1, 44924, 1, 6}, []int{1, 3, 6, 1, 4, 1, 44924, 1, 7}},
+			SubjectKeyID:          []byte("subjectKeyId"),
+			AuthorityKeyID:        []byte("authorityKeyId"),
+			OCSPServer:            []string{"https://ocsp.server"},
+			IssuingCertificateURL: []string{"https://ca.com"},
+			CRLDistributionPoints: []string{"https://ca.com/ca.crl"},
+			PolicyIdentifiers:     PolicyIdentifiers{[]int{1, 2, 3, 4, 5, 6}},
+			BasicConstraints: &BasicConstraints{
+				IsCA:       false,
+				MaxPathLen: 0,
+			},
+			NameConstraints: &NameConstraints{
+				Critical:                true,
+				PermittedDNSDomains:     []string{"jane.doe.com"},
+				ExcludedDNSDomains:      []string{"john.doe.com"},
+				PermittedIPRanges:       []*net.IPNet{ipNet("127.0.0.1/32")},
+				ExcludedIPRanges:        []*net.IPNet{ipNet("0.0.0.0/0")},
+				PermittedEmailAddresses: []string{"jane@doe.com"},
+				ExcludedEmailAddresses:  []string{"john@doe.com"},
+				PermittedURIDomains:     []string{"https://jane.doe.com"},
+				ExcludedURIDomains:      []string{"https://john.doe.com"},
+			},
+			SignatureAlgorithm: SignatureAlgorithm(x509.PureEd25519),
+			PublicKey:          priv.Public(),
+			PublicKeyAlgorithm: x509.ECDSA,
+		}, false},
+		{"failTemplate", args{template, []Option{WithTemplate(`{{ fail "fatal error }}`, CreateTemplateData("commonName", []string{"foo.com"}))}}, nil, true},
+		{"missingTemplate", args{template, []Option{WithTemplateFile("./testdata/missing.tpl", CreateTemplateData("commonName", []string{"foo.com"}))}}, nil, true},
+		{"badJson", args{template, []Option{WithTemplate(`"this is not a json object"`, CreateTemplateData("commonName", []string{"foo.com"}))}}, nil, true},
+		{"failCustomSANs", args{template, []Option{WithTemplate(DefaultLeafTemplate, badCustomSANsData)}}, nil, true},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := NewCertificateFromX509(tt.args.template, tt.args.opts...)
+			if tt.wantErr {
+				assert.Error(t, err)
+				return
+			}
+
+			assert.NoError(t, err)
+			assert.Equal(t, tt.want, got)
+		})
+	}
+}
+
 func TestCertificate_GetCertificate(t *testing.T) {
 	type fields struct {
 		Version               int