ca/linter: Port safety checks from ceremony tool

ca/ca.go

@@ -24,6 +24,8 @@ import (
 	"github.com/miekg/pkcs11"
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/zmap/zlint/v3/lint"
+	"golang.org/x/crypto/cryptobyte"
+	cryptobyte_asn1 "golang.org/x/crypto/cryptobyte/asn1"
 	"golang.org/x/crypto/ocsp"
 	"google.golang.org/protobuf/types/known/timestamppb"
 
@@ -419,7 +421,7 @@ func (ca *certificateAuthorityImpl) IssueCertificateForPrecertificate(ctx contex
 	ca.log.AuditInfof("Signing cert: issuer=[%s] serial=[%s] regID=[%d] names=[%s] certProfileName=[%s] certProfileHash=[%x] precert=[%s]",
 		issuer.Name(), serialHex, req.RegistrationID, names, certProfile.name, certProfile.hash, hex.EncodeToString(precert.Raw))
 
-	_, issuanceToken, err := issuer.Prepare(certProfile.profile, issuanceReq)
+	lintCertBytes, issuanceToken, err := issuer.Prepare(certProfile.profile, issuanceReq)
 	if err != nil {
 		ca.log.AuditErrf("Preparing cert failed: issuer=[%s] serial=[%s] regID=[%d] names=[%s] certProfileName=[%s] certProfileHash=[%x] err=[%v]",
 			issuer.Name(), serialHex, req.RegistrationID, names, certProfile.name, certProfile.hash, err)
@@ -434,6 +436,11 @@ func (ca *certificateAuthorityImpl) IssueCertificateForPrecertificate(ctx contex
 		return nil, berrors.InternalServerError("failed to sign certificate: %s", err)
 	}
 
+	err = tbsCertIsDeterministic(lintCertBytes, certDER)
+	if err != nil {
+		return nil, err
+	}
+
 	ca.signatureCount.With(prometheus.Labels{"purpose": string(certType), "issuer": issuer.Name()}).Inc()
 	ca.log.AuditInfof("Signing cert success: issuer=[%s] serial=[%s] regID=[%d] names=[%s] certificate=[%s] certProfileName=[%s] certProfileHash=[%x]",
 		issuer.Name(), serialHex, req.RegistrationID, names, hex.EncodeToString(certDER), certProfile.name, certProfile.hash)
@@ -609,9 +616,77 @@ func (ca *certificateAuthorityImpl) issuePrecertificateInner(ctx context.Context
 		return nil, nil, berrors.InternalServerError("failed to sign precertificate: %s", err)
 	}
 
+	err = tbsCertIsDeterministic(lintCertBytes, certDER)
+	if err != nil {
+		return nil, nil, err
+	}
+
 	ca.signatureCount.With(prometheus.Labels{"purpose": string(precertType), "issuer": issuer.Name()}).Inc()
 	ca.log.AuditInfof("Signing precert success: issuer=[%s] serial=[%s] regID=[%d] names=[%s] precertificate=[%s] certProfileName=[%s] certProfileHash=[%x]",
 		issuer.Name(), serialHex, issueReq.RegistrationID, strings.Join(csr.DNSNames, ", "), hex.EncodeToString(certDER), certProfile.name, certProfile.hash)
 
 	return certDER, &certProfileWithID{certProfile.name, certProfile.hash, nil}, nil
 }
+
+// verifyTBSCertIsDeterministic verifies that x509.CreateCertificate signing
+// operation is deterministic and produced identical DER bytes between the given
+// lint certificate and leaf certificate. If the DER byte equality check fails
+// it's mississuance, but it's better to know about the problem sooner than
+// later. The caller is responsible for passing the appropriate valid
+// certificate bytes in the correct position.
+func tbsCertIsDeterministic(lintCertBytes []byte, leafCertBytes []byte) error {
+	if core.IsAnyNilOrZero(lintCertBytes, leafCertBytes) {
+		return fmt.Errorf("lintCertBytes of leafCertBytes were nil")
+	}
+
+	// extractTBSCertBytes is a partial copy of //crypto/x509/parser.go to
+	// extract the RawTBSCertificate field from given DER bytes. It the
+	// RawTBSCertificate field bytes or an error if the given bytes cannot be
+	// parsed. This is far more performant than parsing the entire *Certificate
+	// structure with x509.ParseCertificate().
+	//
+	// RFC 5280, Section 4.1
+	//    Certificate  ::=  SEQUENCE  {
+	//      tbsCertificate       TBSCertificate,
+	//      signatureAlgorithm   AlgorithmIdentifier,
+	//      signatureValue       BIT STRING  }
+	//
+	//    TBSCertificate  ::=  SEQUENCE  {
+	//      ..
+	extractTBSCertBytes := func(inputDERBytes *[]byte) ([]byte, error) {
+		input := cryptobyte.String(*inputDERBytes)
+
+		// Extract the Certificate bytes
+		if !input.ReadASN1(&input, cryptobyte_asn1.SEQUENCE) {
+			return nil, errors.New("malformed certificate")
+		}
+
+		var tbs cryptobyte.String
+		// Extract the TBSCertificate bytes from the Certificate bytes
+		if !input.ReadASN1(&tbs, cryptobyte_asn1.SEQUENCE) {
+			return nil, errors.New("malformed tbs certificate")
+		}
+
+		if tbs.Empty() {
+			return nil, errors.New("parsed RawTBSCertificate field was empty")
+		}
+
+		return tbs, nil
+	}
+
+	lintRawTBSCert, err := extractTBSCertBytes(&lintCertBytes)
+	if err != nil {
+		return fmt.Errorf("while extracting lint TBS cert: %w", err)
+	}
+
+	leafRawTBSCert, err := extractTBSCertBytes(&leafCertBytes)
+	if err != nil {
+		return fmt.Errorf("while extracting leaf TBS cert: %w", err)
+	}
+
+	if !bytes.Equal(lintRawTBSCert, leafRawTBSCert) {
+		return fmt.Errorf("mismatch between lintCert and leafCert RawTBSCertificate DER bytes: \"%x\" != \"%x\"", lintRawTBSCert, leafRawTBSCert)
+	}
+
+	return nil
+}

ca/ca_test.go

@@ -10,6 +10,7 @@ import (
 	"encoding/asn1"
 	"errors"
 	"fmt"
+	"math/big"
 	"os"
 	"strings"
 	"testing"
@@ -1300,3 +1301,102 @@ func TestGenerateSKID(t *testing.T) {
 	test.AssertEquals(t, cap(sha256skid), 20)
 	features.Reset()
 }
+
+func TestVerifyTBSCertIsDeterministic(t *testing.T) {
+	t.Parallel()
+
+	// Create first keypair and cert
+	testKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	test.AssertNotError(t, err, "unable to generate ECDSA private key")
+	template := &x509.Certificate{
+		NotAfter:     time.Now().Add(1 * time.Hour),
+		DNSNames:     []string{"example.com"},
+		SerialNumber: big.NewInt(1),
+	}
+	certDer1, err := x509.CreateCertificate(rand.Reader, template, template, &testKey.PublicKey, testKey)
+	test.AssertNotError(t, err, "unable to create certificate")
+
+	// Create second keypair and cert
+	testKey2, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	test.AssertNotError(t, err, "unable to generate ECDSA private key")
+	template2 := &x509.Certificate{
+		NotAfter:     time.Now().Add(2 * time.Hour),
+		DNSNames:     []string{"example.net"},
+		SerialNumber: big.NewInt(2),
+	}
+	certDer2, err := x509.CreateCertificate(rand.Reader, template2, template2, &testKey2.PublicKey, testKey2)
+	test.AssertNotError(t, err, "unable to create certificate")
+
+	testCases := []struct {
+		name          string
+		lintCertBytes []byte
+		leafCertBytes []byte
+		errorSubstr   string
+	}{
+		{
+			name:          "Both nil",
+			lintCertBytes: nil,
+			leafCertBytes: nil,
+			errorSubstr:   "were nil",
+		},
+		{
+			name:          "Missing a value, invalid input",
+			lintCertBytes: nil,
+			leafCertBytes: []byte{0x6, 0x6, 0x6},
+			errorSubstr:   "were nil",
+		},
+		{
+			name:          "Missing a value, valid input",
+			lintCertBytes: nil,
+			leafCertBytes: certDer1,
+			errorSubstr:   "were nil",
+		},
+		{
+			name:          "Mismatched bytes, invalid input",
+			lintCertBytes: []byte{0x6, 0x6, 0x6},
+			leafCertBytes: []byte{0x1, 0x2, 0x3},
+			errorSubstr:   "malformed certificate",
+		},
+		{
+			name:          "Mismatched bytes, invalider input",
+			lintCertBytes: certDer1,
+			leafCertBytes: []byte{0x1, 0x2, 0x3},
+			errorSubstr:   "malformed certificate",
+		},
+		{
+			// This case is an example of when a linting cert's DER bytes are
+			// mismatched compared to then precert or final cert created from
+			// that linting cert's DER bytes.
+			name:          "Mismatched bytes, valid input",
+			lintCertBytes: certDer1,
+			leafCertBytes: certDer2,
+			errorSubstr:   "mismatch between",
+		},
+		{
+			// Take this with a grain of salt since this test is not actually
+			// creating a linting certificate and performing two
+			// x509.CreateCertificate() calls like
+			// ca.IssueCertificateForPrecertificate and
+			// ca.issuePrecertificateInner do. However, we're still going to
+			// verify the equality.
+			name:          "Valid",
+			lintCertBytes: certDer1,
+			leafCertBytes: certDer1,
+		},
+	}
+
+	for _, testCase := range testCases {
+		// TODO(#7454) Remove this rebinding
+		testCase := testCase
+		t.Run(testCase.name, func(t *testing.T) {
+			t.Parallel()
+			err := tbsCertIsDeterministic(testCase.lintCertBytes, testCase.leafCertBytes)
+			if testCase.errorSubstr != "" {
+				test.AssertError(t, err, "your lack of errors is disturbing")
+				test.AssertContains(t, err.Error(), testCase.errorSubstr)
+			} else {
+				test.AssertNotError(t, err, "unexpected error")
+			}
+		})
+	}
+}

cmd/ceremony/main.go

@@ -645,10 +645,6 @@ func rootCeremony(configBytes []byte) error {
 	if err != nil {
 		return err
 	}
-	// Verify that the lintCert is self-signed.
-	if !bytes.Equal(lintCert.RawSubject, lintCert.RawIssuer) {
-		return fmt.Errorf("mismatch between self-signed lintCert RawSubject and RawIssuer DER bytes: \"%x\" != \"%x\"", lintCert.RawSubject, lintCert.RawIssuer)
-	}
 	finalCert, err := signAndWriteCert(template, template, lintCert, keyInfo.key, signer, config.Outputs.CertificatePath)
 	if err != nil {
 		return err
@@ -697,10 +693,6 @@ func intermediateCeremony(configBytes []byte, ct certType) error {
 	if err != nil {
 		return err
 	}
-	// Verify that the lintCert (and therefore the eventual finalCert) corresponds to the specified issuer certificate.
-	if !bytes.Equal(issuer.RawSubject, lintCert.RawIssuer) {
-		return fmt.Errorf("mismatch between issuer RawSubject and lintCert RawIssuer DER bytes: \"%x\" != \"%x\"", issuer.RawSubject, lintCert.RawIssuer)
-	}
 	finalCert, err := signAndWriteCert(template, issuer, lintCert, pub, signer, config.Outputs.CertificatePath)
 	if err != nil {
 		return err
@@ -780,9 +772,6 @@ func crossCertCeremony(configBytes []byte, ct certType) error {
 	if lintCert.NotBefore.Before(toBeCrossSigned.NotBefore) {
 		return fmt.Errorf("cross-signed subordinate CA's NotBefore predates the existing CA's NotBefore")
 	}
-	if !bytes.Equal(issuer.RawSubject, lintCert.RawIssuer) {
-		return fmt.Errorf("mismatch between issuer RawSubject and lintCert RawIssuer DER bytes: \"%x\" != \"%x\"", issuer.RawSubject, lintCert.RawIssuer)
-	}
 	// BR 7.1.2.2.3 Cross-Certified Subordinate CA Extensions
 	if !slices.Equal(lintCert.ExtKeyUsage, toBeCrossSigned.ExtKeyUsage) {
 		return fmt.Errorf("lint cert and toBeCrossSigned cert EKUs differ")

linter/linter.go

@@ -1,6 +1,7 @@
 package linter
 
 import (
+	"bytes"
 	"crypto"
 	"crypto/ecdsa"
 	"crypto/rand"
@@ -239,6 +240,16 @@ func makeLintCert(tbs *x509.Certificate, subjectPubKey crypto.PublicKey, issuer
 	if err != nil {
 		return nil, nil, fmt.Errorf("failed to parse lint certificate: %w", err)
 	}
+	// RFC 5280, Sections 4.1.2.6 and 8
+	//
+	// When the subject of the certificate is a CA, the subject
+	// field MUST be encoded in the same way as it is encoded in the
+	// issuer field (Section 4.1.2.4) in all certificates issued by
+	// the subject CA.
+	if !bytes.Equal(issuer.RawSubject, lintCert.RawIssuer) {
+		return nil, nil, fmt.Errorf("mismatch between lint issuer RawSubject and lintCert.RawIssuer DER bytes: \"%x\" != \"%x\"", issuer.RawSubject, lintCert.RawIssuer)
+	}
+
 	return lintCertBytes, lintCert, nil
 }