require timestamping leaf certificate to be an end-entity (#1416)

Signed-off-by: Javid Khan <dxbjavid@gmail.com>

Author: dxbjavid

pkg/verification/verify.go

@@ -111,6 +111,14 @@ func verifyLeafCert(leafCert *x509.Certificate, verifiedChains [][]*x509.Certifi
 
 	errMsg := "failed to verify TSA certificate"
 
+	// The TSA certificate must be an end-entity certificate, per RFC 3161 2.3.
+	// crypto/x509 chain verification does not reject a CA certificate used as
+	// the leaf, so a certificate that asserts the CA bit yet carries the
+	// timestamping EKU would otherwise be accepted as a signer.
+	if leafCert.IsCA {
+		return fmt.Errorf("%s: %w", errMsg, errors.New("TSA certificate must be an end-entity certificate, not a CA"))
+	}
+
 	err := verifyLeafCertCriticalEKU(leafCert)
 	if err != nil {
 		return fmt.Errorf("%s: %w", errMsg, err)

pkg/verification/verify_test.go

@@ -284,6 +284,36 @@ func TestVerifyLeafCert(t *testing.T) {
 	}
 }
 
+// A certificate that asserts the CA bit but carries the timestamping EKU must
+// not be accepted as a TSA leaf, per RFC 3161 2.3.
+func TestVerifyLeafCertRejectsCACertificate(t *testing.T) {
+	criticalExtension := pkix.Extension{
+		Id:       EKUOID,
+		Critical: true,
+	}
+
+	caCert := &x509.Certificate{
+		Raw:                   []byte("abc123"),
+		RawIssuer:             []byte("abc123"),
+		SerialNumber:          big.NewInt(int64(123)),
+		Extensions:            []pkix.Extension{criticalExtension},
+		ExtKeyUsage:           []x509.ExtKeyUsage{x509.ExtKeyUsageTimeStamping},
+		BasicConstraintsValid: true,
+		IsCA:                  true,
+		Subject: pkix.Name{
+			CommonName: "TSA-Service",
+		},
+	}
+
+	err := verifyLeafCert(caCert, nil, VerifyOpts{})
+	if err == nil {
+		t.Fatal("expected verification to fail for a CA certificate used as a timestamping leaf")
+	}
+	if !strings.Contains(err.Error(), "end-entity") {
+		t.Fatalf("expected an end-entity error, got %s", err.Error())
+	}
+}
+
 func TestVerifySubjectCommonName(t *testing.T) {
 	type test struct {
 		optsCommonName      string

pkg/x509/x509.go

@@ -77,6 +77,12 @@ func VerifyCertChain(certs []*x509.Certificate, signer crypto.Signer, enforceInt
 		}
 	}
 
+	// Verify the leaf is an end-entity certificate, per RFC 3161 2.3. The chain
+	// verification above does not reject a CA certificate used as the leaf.
+	if leaf.IsCA {
+		return errors.New("leaf certificate must be an end-entity certificate, not a CA")
+	}
+
 	// Verify leaf has only a single EKU for timestamping, per RFC 3161 2.3
 	// This should be enforced by Verify already. Key purposes that crypto/x509
 	// does not recognise land in UnknownExtKeyUsage, so count those too.

pkg/x509/x509_test.go

@@ -64,4 +64,11 @@ func TestVerifyCertChain(t *testing.T) {
 	if err := VerifyCertChain([]*x509.Certificate{extraEKULeaf, subCert, rootCert}, extraEKUKey, true); err == nil || !strings.Contains(err.Error(), "should only contain one EKU") {
 		t.Fatalf("expected failure verifying certificate chain with extra EKU: %v", err)
 	}
+
+	// failure: leaf asserts the CA bit, so it is not an end-entity, per RFC 3161 2.3
+	caLeaf, caLeafKey, _ := testutils.GenerateLeafCert(subCert, subKey)
+	caLeaf.IsCA = true
+	if err := VerifyCertChain([]*x509.Certificate{caLeaf, subCert, rootCert}, caLeafKey, true); err == nil || !strings.Contains(err.Error(), "end-entity") {
+		t.Fatalf("expected failure verifying certificate chain with a CA leaf: %v", err)
+	}
 }