From aefdfc7be7a7c4d5b038de0b3142b47c7dce1b00 Mon Sep 17 00:00:00 2001
From: Mariano Cano <mariano.cano@gmail.com>
Date: Wed, 19 Oct 2022 19:10:50 -0700
Subject: [PATCH] Use RawSubject on renew and rekey

Renew was not replicating exactly the subject because extra names
gets decoded into pkix.Name.Names, the non-default ones should be
added to pkix.Name.ExtraNames. Instead of doing that, this commit
sets the RawSubject that will also keep the order.

Fixes #1106
---
 authority/tls.go      |  2 +-
 authority/tls_test.go | 42 +++++++++++++++++++++++++++++++++---------
 2 files changed, 34 insertions(+), 10 deletions(-)

diff --git a/authority/tls.go b/authority/tls.go
index 90316cc3..6c176f06 100644
--- a/authority/tls.go
+++ b/authority/tls.go
@@ -320,7 +320,7 @@ func (a *Authority) Rekey(oldCert *x509.Certificate, pk crypto.PublicKey) ([]*x5
 	// Create new certificate from previous values.
 	// Issuer, NotBefore, NotAfter and SubjectKeyId will be set by the CAS.
 	newCert := &x509.Certificate{
-		Subject:                     oldCert.Subject,
+		RawSubject:                  oldCert.RawSubject,
 		KeyUsage:                    oldCert.KeyUsage,
 		UnhandledCriticalExtensions: oldCert.UnhandledCriticalExtensions,
 		ExtKeyUsage:                 oldCert.ExtKeyUsage,
diff --git a/authority/tls_test.go b/authority/tls_test.go
index dd52ea74..53d0401a 100644
--- a/authority/tls_test.go
+++ b/authority/tls_test.go
@@ -139,6 +139,13 @@ func generateIntermidiateCertificate(t *testing.T, issuer *x509.Certificate, sig
 	return cert, priv
 }
 
+func withSubject(sub pkix.Name) provisioner.CertificateModifierFunc {
+	return func(crt *x509.Certificate, _ provisioner.SignOptions) error {
+		crt.Subject = sub
+		return nil
+	}
+}
+
 func withProvisionerOID(name, kid string) provisioner.CertificateModifierFunc {
 	return func(crt *x509.Certificate, _ provisioner.SignOptions) error {
 		b, err := asn1.Marshal(stepProvisionerASN1{
@@ -952,6 +959,18 @@ func TestAuthority_Renew(t *testing.T) {
 		withProvisionerOID("Max", a.config.AuthorityConfig.Provisioners[0].(*provisioner.JWK).Key.KeyID),
 		withSigner(issuer, signer))
 
+	certExtraNames := generateCertificate(t, "renew", []string{"test.smallstep.com", "test"},
+		withSubject(pkix.Name{
+			CommonName: "renew",
+			ExtraNames: []pkix.AttributeTypeAndValue{
+				{Type: asn1.ObjectIdentifier{0, 9, 2342, 19200300, 100, 1, 25}, Value: "dc"},
+			},
+		}),
+		withNotBeforeNotAfter(so.NotBefore.Time(), so.NotAfter.Time()),
+		withDefaultASN1DN(a.config.AuthorityConfig.Template),
+		withProvisionerOID("Max", a.config.AuthorityConfig.Provisioners[0].(*provisioner.JWK).Key.KeyID),
+		withSigner(issuer, signer))
+
 	certNoRenew := generateCertificate(t, "renew", []string{"test.smallstep.com", "test"},
 		withNotBeforeNotAfter(so.NotBefore.Time(), so.NotAfter.Time()),
 		withDefaultASN1DN(a.config.AuthorityConfig.Template),
@@ -1001,6 +1020,12 @@ func TestAuthority_Renew(t *testing.T) {
 				cert: cert,
 			}, nil
 		},
+		"ok/WithExtraNames": func() (*renewTest, error) {
+			return &renewTest{
+				auth: a,
+				cert: certExtraNames,
+			}, nil
+		},
 		"ok/success-new-intermediate": func() (*renewTest, error) {
 			rootCert, rootSigner := generateRootCertificate(t)
 			intCert, intSigner := generateIntermidiateCertificate(t, rootCert, rootSigner)
@@ -1063,15 +1088,14 @@ func TestAuthority_Renew(t *testing.T) {
 					assert.True(t, leaf.NotAfter.Before(expiry.Add(time.Hour)))
 
 					tmplt := a.config.AuthorityConfig.Template
-					assert.Equals(t, leaf.Subject.String(),
-						pkix.Name{
-							Country:       []string{tmplt.Country},
-							Organization:  []string{tmplt.Organization},
-							Locality:      []string{tmplt.Locality},
-							StreetAddress: []string{tmplt.StreetAddress},
-							Province:      []string{tmplt.Province},
-							CommonName:    tmplt.CommonName,
-						}.String())
+					assert.Equals(t, leaf.RawSubject, tc.cert.RawSubject)
+					assert.Equals(t, leaf.Subject.Country, []string{tmplt.Country})
+					assert.Equals(t, leaf.Subject.Organization, []string{tmplt.Organization})
+					assert.Equals(t, leaf.Subject.Locality, []string{tmplt.Locality})
+					assert.Equals(t, leaf.Subject.StreetAddress, []string{tmplt.StreetAddress})
+					assert.Equals(t, leaf.Subject.Province, []string{tmplt.Province})
+					assert.Equals(t, leaf.Subject.CommonName, tmplt.CommonName)
+
 					assert.Equals(t, leaf.Issuer, intermediate.Subject)
 
 					assert.Equals(t, leaf.SignatureAlgorithm, x509.ECDSAWithSHA256)