CA: implement CFSSL/zlint pre-issuance linting. (#4378)

The `test/config-next` CA configs are both updated to use `zlint` to lint TBS
pre-certificates with a throw-away key and treat any lint findings >=
`lints.Pass` as an error, blocking the CA from signing the TBS pre-cert with its
private key.

The CA `issuePrecertificateInner` function is updated to specifically catch
linting related errors from CFSSL to marshal the linting findings to the audit
log. A small unit test for this change is included.

The CA `IssueCertificateForPrecertificate` function remains unchanged: the CFSSL
interface that defines `SignFromPrecert` doesn't facilitate linting. We still
lint final certificates post-issuance with `cert-checker` and accept the
possibility there may be some compliance issues that could occur between the
precertificate passing linting and the final certificate being signed.

Resolves https://github.com/letsencrypt/boulder/issues/4255

ca/ca.go

@@ -139,11 +139,18 @@ type Issuer struct {
 	Cert   *x509.Certificate
 }
 
+// localSigner is an interface describing the functions of a cfssl.local.Signer
+// that the Boulder CA uses. It allows mocking the local.Signer in unit tests.
+type localSigner interface {
+	Sign(signer.SignRequest) ([]byte, error)
+	SignFromPrecert(*x509.Certificate, []ct.SignedCertificateTimestamp) ([]byte, error)
+}
+
 // internalIssuer represents the fully initialized internal state for a single
 // issuer, including the cfssl signer and OCSP signer objects.
 type internalIssuer struct {
 	cert       *x509.Certificate
-	eeSigner   *local.Signer
+	eeSigner   localSigner
 	ocspSigner ocsp.Signer
 }
 
@@ -574,6 +581,18 @@ func (ca *CertificateAuthorityImpl) issuePrecertificateInner(ctx context.Context
 	certPEM, err := issuer.eeSigner.Sign(req)
 	ca.noteSignError(err)
 	if err != nil {
+		// If the Signing error was a pre-issuance lint error then marshal the
+		// linting errors to include in the audit err msg.
+		if lErr, ok := err.(*local.LintError); ok {
+			// NOTE(@cpu): We throw away the JSON marshal error here. If marshaling
+			// fails for some reason it's acceptable to log an empty string for the
+			// JSON component.
+			lintErrsJSON, _ := json.Marshal(lErr.ErrorResults)
+			ca.log.AuditErrf("Signing failed: serial=[%s] err=[%v] lintErrors=%s",
+				serialHex, err, string(lintErrsJSON))
+			return nil, berrors.InternalServerError("failed to sign certificate: %s", err)
+		}
+
 		err = berrors.InternalServerError("failed to sign certificate: %s", err)
 		ca.log.AuditErrf("Signing failed: serial=[%s] err=[%v]", serialHex, err)
 		return nil, err

ca/ca_test.go

@@ -22,10 +22,12 @@ import (
 	cfsslConfig "github.com/cloudflare/cfssl/config"
 	"github.com/cloudflare/cfssl/helpers"
 	"github.com/cloudflare/cfssl/signer"
+	"github.com/cloudflare/cfssl/signer/local"
 	"github.com/google/certificate-transparency-go"
 	cttls "github.com/google/certificate-transparency-go/tls"
 	"github.com/jmhodges/clock"
 	"github.com/prometheus/client_golang/prometheus"
+	"github.com/zmap/zlint/lints"
 	"golang.org/x/crypto/ocsp"
 
 	"github.com/letsencrypt/boulder/ca/config"
@@ -161,7 +163,7 @@ type testCtx struct {
 	keyPolicy goodkey.KeyPolicy
 	fc        clock.FakeClock
 	stats     metrics.Scope
-	logger    blog.Logger
+	logger    *blog.Mock
 }
 
 type mockSA struct {
@@ -996,3 +998,69 @@ func TestOrphanQueue(t *testing.T) {
 		t.Fatalf("Unexpected error, wanted %q, got %q", goque.ErrEmpty, err)
 	}
 }
+
+type linttrapSigner struct {
+	lintErr error
+}
+
+func (s *linttrapSigner) Sign(signer.SignRequest) ([]byte, error) {
+	return nil, s.lintErr
+}
+
+func (s *linttrapSigner) SignFromPrecert(*x509.Certificate, []ct.SignedCertificateTimestamp) ([]byte, error) {
+	return nil, errors.New("SignFromPrecert not implemented for linttrapSigner")
+}
+
+func TestIssuePrecertificateLinting(t *testing.T) {
+	testCtx := setup(t)
+	sa := &mockSA{}
+	ca, err := NewCertificateAuthorityImpl(
+		testCtx.caConfig,
+		sa,
+		testCtx.pa,
+		testCtx.fc,
+		testCtx.stats,
+		testCtx.issuers,
+		testCtx.keyPolicy,
+		testCtx.logger,
+		nil)
+	test.AssertNotError(t, err, "Failed to create CA")
+
+	// Reconfigure the CA's eeSigner to be a linttrapSigner that always returns
+	// two LintResults.
+	ca.defaultIssuer.eeSigner = &linttrapSigner{
+		lintErr: &local.LintError{
+			ErrorResults: map[string]lints.LintResult{
+				"foobar": lints.LintResult{
+					Status:  lints.Error,
+					Details: "foobar is error",
+				},
+				"foobar2": lints.LintResult{
+					Status:  lints.Warn,
+					Details: "foobar2 is warning",
+				},
+			},
+		},
+	}
+
+	// Clear the mock logger
+	testCtx.logger.Clear()
+
+	// Attempt to issue a pre-certificate
+	_, err = ca.IssuePrecertificate(ctx, &caPB.IssueCertificateRequest{
+		Csr:            CNandSANCSR,
+		RegistrationID: &arbitraryRegID,
+	})
+	// It should error
+	test.AssertError(t, err, "expected err from IssuePrecertificate with linttrapSigner")
+	// The local.LintError should have been converted to an internal server error
+	// berror with the correct message.
+	test.Assert(t, berrors.Is(err, berrors.InternalServer), "Incorrect error type returned")
+	test.AssertEquals(t, err.Error(), "failed to sign certificate: pre-issuance linting found 2 error results")
+
+	// We also expect that an AUDIT level error is logged that includes the expect
+	// serialized JSON lintErrors
+	regex := `ERR: \[AUDIT\] Signing failed: serial=\[.*\] err=\[pre-issuance linting found 2 error results\] lintErrors=\{"foobar":\{"result":"error","details":"foobar is error"\},"foobar2":\{"result":"warn","details":"foobar2 is warning"\}\}`
+	matches := testCtx.logger.GetAllMatching(regex)
+	test.AssertEquals(t, len(matches), 1)
+}

test/config-next/ca-a.json

@@ -76,7 +76,12 @@
               "SignatureAlgorithm": true
             },
             "ClientProvidesSerialNumbers": true,
-            "allowed_extensions": [ "1.3.6.1.5.5.7.1.24" ]
+            "allowed_extensions": [ "1.3.6.1.5.5.7.1.24" ],
+            "lint_error_level": "pass",
+            "ignored_lints": [
+              "ct_sct_policy_count_unsatisfied",
+              "n_subject_common_name_included"
+            ]
           },
           "ecdsaEE": {
             "usages": [
@@ -113,7 +118,12 @@
               "SignatureAlgorithm": true
             },
             "ClientProvidesSerialNumbers": true,
-            "allowed_extensions": [ "1.3.6.1.5.5.7.1.24" ]
+            "allowed_extensions": [ "1.3.6.1.5.5.7.1.24" ],
+            "lint_error_level": "pass",
+            "ignored_lints": [
+              "ct_sct_policy_count_unsatisfied",
+              "n_subject_common_name_included"
+            ]
           }
         },
         "default": {

test/config-next/ca-b.json

@@ -77,7 +77,12 @@
               "SignatureAlgorithm": true
             },
             "ClientProvidesSerialNumbers": true,
-            "allowed_extensions": [ "1.3.6.1.5.5.7.1.24" ]
+            "allowed_extensions": [ "1.3.6.1.5.5.7.1.24" ],
+            "lint_error_level": "pass",
+            "ignored_lints": [
+              "ct_sct_policy_count_unsatisfied",
+              "n_subject_common_name_included"
+            ]
           },
           "ecdsaEE": {
             "usages": [
@@ -114,7 +119,12 @@
               "SignatureAlgorithm": true
             },
             "ClientProvidesSerialNumbers": true,
-            "allowed_extensions": [ "1.3.6.1.5.5.7.1.24" ]
+            "allowed_extensions": [ "1.3.6.1.5.5.7.1.24" ],
+            "lint_error_level": "pass",
+            "ignored_lints": [
+              "ct_sct_policy_count_unsatisfied",
+              "n_subject_common_name_included"
+            ]
           }
         },
         "default": {