diff --git a/ca/ca.go b/ca/ca.go index 44d56cc94..a76872254 100644 --- a/ca/ca.go +++ b/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 diff --git a/ca/ca_test.go b/ca/ca_test.go index f7cbda9b3..1fe904f81 100644 --- a/ca/ca_test.go +++ b/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) +} diff --git a/test/config-next/ca-a.json b/test/config-next/ca-a.json index f857162b4..abdae00e5 100644 --- a/test/config-next/ca-a.json +++ b/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": { diff --git a/test/config-next/ca-b.json b/test/config-next/ca-b.json index fc6de5e7c..bfa7b026c 100644 --- a/test/config-next/ca-b.json +++ b/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": {