letsencrypt
/
boulder
mirror of https://github.com/letsencrypt/boulder.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix reference bug in CA.noteSignError (#7534) 

In the process of writing
https://github.com/letsencrypt/boulder/pull/7533 I discovered that the
method for detecting pkcs11.Error errors is broken: it attempts to
unwrap the returned error into a pointer-to-a-pointer type, which
doesn't work because only `pkcs11.Error` implements the Error interface,
while `*pkcs11.Error` does not.

Add a test which shows that the current noteSignError implementation is
broken. Then fix noteSignError and the two locations which duplicate
that code by removing the extra layer of indirection. And since the same
code exists in three locations, refactor how the caImpl, ocspImpl, and
crlImpl share metrics so that it only has to exist in one place.

A minimal reproduction case of this type of breakage can be seen here:
https://go.dev/play/p/qCLDQ1SFiWu
This commit is contained in:
Aaron Gable 2024-06-07 12:34:02 -07:00 committed by GitHub
parent de8401e345
commit 602f3e4708
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 131 additions and 160 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

77
ca/ca.go
View File

@ -79,6 +79,46 @@ type certProfilesMaps struct {
profileByName map[string]*certProfileWithID
}
// caMetrics holds various metrics which are shared between caImpl, ocspImpl,
// and crlImpl.
type caMetrics struct {
signatureCount *prometheus.CounterVec
signErrorCount *prometheus.CounterVec
lintErrorCount prometheus.Counter
}
func NewCAMetrics(stats prometheus.Registerer) *caMetrics {
signatureCount := prometheus.NewCounterVec(
prometheus.CounterOpts{
Name: "signatures",
Help: "Number of signatures",
},
[]string{"purpose", "issuer"})
stats.MustRegister(signatureCount)
signErrorCount := prometheus.NewCounterVec(prometheus.CounterOpts{
Name: "signature_errors",
Help: "A counter of signature errors labelled by error type",
}, []string{"type"})
stats.MustRegister(signErrorCount)
lintErrorCount := prometheus.NewCounter(
prometheus.CounterOpts{
Name: "lint_errors",
Help: "Number of issuances that were halted by linting errors",
})
stats.MustRegister(lintErrorCount)
return &caMetrics{signatureCount, signErrorCount, lintErrorCount}
}
func (m *caMetrics) noteSignError(err error) {
var pkcs11Error pkcs11.Error
if errors.As(err, &pkcs11Error) {
m.signErrorCount.WithLabelValues("HSM").Inc()
}
}
// certificateAuthorityImpl represents a CA that signs certificates.
// It can sign OCSP responses as well, but only via delegation to an ocspImpl.
type certificateAuthorityImpl struct {
@ -98,9 +138,7 @@ type certificateAuthorityImpl struct {
keyPolicy goodkey.KeyPolicy
clk clock.Clock
log blog.Logger
signatureCount *prometheus.CounterVec
signErrorCount *prometheus.CounterVec
lintErrorCount prometheus.Counter
metrics *caMetrics
}
var _ capb.CertificateAuthorityServer = (*certificateAuthorityImpl)(nil)
@ -219,9 +257,7 @@ func NewCertificateAuthorityImpl(
maxNames int,
keyPolicy goodkey.KeyPolicy,
logger blog.Logger,
stats prometheus.Registerer,
signatureCount *prometheus.CounterVec,
signErrorCount *prometheus.CounterVec,
metrics *caMetrics,
clk clock.Clock,
) (*certificateAuthorityImpl, error) {
var ca *certificateAuthorityImpl
@ -253,13 +289,6 @@ func NewCertificateAuthorityImpl(
return nil, err
}
lintErrorCount := prometheus.NewCounter(
prometheus.CounterOpts{
Name: "lint_errors",
Help: "Number of issuances that were halted by linting errors",
})
stats.MustRegister(lintErrorCount)
ca = &certificateAuthorityImpl{
sa: sa,
pa: pa,
@ -271,9 +300,7 @@ func NewCertificateAuthorityImpl(
maxNames: maxNames,
keyPolicy: keyPolicy,
log: logger,
signatureCount: signatureCount,
signErrorCount: signErrorCount,
lintErrorCount: lintErrorCount,
metrics: metrics,
clk: clk,
ecdsaAllowList: ecdsaAllowList,
}
@ -281,14 +308,6 @@ func NewCertificateAuthorityImpl(
return ca, nil
}
// noteSignError is called after operations that may cause a PKCS11 signing error.
func (ca *certificateAuthorityImpl) noteSignError(err error) {
var pkcs11Error *pkcs11.Error
if errors.As(err, &pkcs11Error) {
ca.signErrorCount.WithLabelValues("HSM").Inc()
}
}
var ocspStatusToCode = map[string]int{
"good": ocsp.Good,
"revoked": ocsp.Revoked,
@ -432,7 +451,7 @@ func (ca *certificateAuthorityImpl) IssueCertificateForPrecertificate(ctx contex
certDER, err := issuer.Issue(issuanceToken)
if err != nil {
ca.noteSignError(err)
ca.metrics.noteSignError(err)
ca.log.AuditErrf("Signing 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)
return nil, berrors.InternalServerError("failed to sign certificate: %s", err)
@ -443,7 +462,7 @@ func (ca *certificateAuthorityImpl) IssueCertificateForPrecertificate(ctx contex
return nil, err
}
ca.signatureCount.With(prometheus.Labels{"purpose": string(certType), "issuer": issuer.Name()}).Inc()
ca.metrics.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)
@ -594,7 +613,7 @@ func (ca *certificateAuthorityImpl) issuePrecertificateInner(ctx context.Context
ca.log.AuditErrf("Preparing precert failed: issuer=[%s] serial=[%s] regID=[%d] names=[%s] certProfileName=[%s] certProfileHash=[%x] err=[%v]",
issuer.Name(), serialHex, issueReq.RegistrationID, strings.Join(csr.DNSNames, ", "), certProfile.name, certProfile.hash, err)
if errors.Is(err, linter.ErrLinting) {
ca.lintErrorCount.Inc()
ca.metrics.lintErrorCount.Inc()
}
return nil, nil, berrors.InternalServerError("failed to prepare precertificate signing: %s", err)
}
@ -612,7 +631,7 @@ func (ca *certificateAuthorityImpl) issuePrecertificateInner(ctx context.Context
certDER, err := issuer.Issue(issuanceToken)
if err != nil {
ca.noteSignError(err)
ca.metrics.noteSignError(err)
ca.log.AuditErrf("Signing precert failed: issuer=[%s] serial=[%s] regID=[%d] names=[%s] certProfileName=[%s] certProfileHash=[%x] err=[%v]",
issuer.Name(), serialHex, issueReq.RegistrationID, strings.Join(csr.DNSNames, ", "), certProfile.name, certProfile.hash, err)
return nil, nil, berrors.InternalServerError("failed to sign precertificate: %s", err)
@ -623,7 +642,7 @@ func (ca *certificateAuthorityImpl) issuePrecertificateInner(ctx context.Context
return nil, nil, err
}
ca.signatureCount.With(prometheus.Labels{"purpose": string(precertType), "issuer": issuer.Name()}).Inc()
ca.metrics.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)

112
ca/ca_test.go
View File

@ -20,6 +20,7 @@ import (
cttls "github.com/google/certificate-transparency-go/tls"
ctx509 "github.com/google/certificate-transparency-go/x509"
"github.com/jmhodges/clock"
"github.com/miekg/pkcs11"
"github.com/prometheus/client_golang/prometheus"
"github.com/zmap/zlint/v3/lint"
"google.golang.org/grpc"
@ -112,9 +113,7 @@ type testCtx struct {
boulderIssuers []*issuance.Issuer
keyPolicy goodkey.KeyPolicy
fc clock.FakeClock
stats prometheus.Registerer
signatureCount *prometheus.CounterVec
signErrorCount *prometheus.CounterVec
metrics *caMetrics
logger *blog.Mock
}
@ -214,6 +213,12 @@ func setup(t *testing.T) *testCtx {
Name: "signature_errors",
Help: "A counter of signature errors labelled by error type",
}, []string{"type"})
lintErrorCount := prometheus.NewCounter(
prometheus.CounterOpts{
Name: "lint_errors",
Help: "Number of issuances that were halted by linting errors",
})
cametrics := &caMetrics{signatureCount, signErrorCount, lintErrorCount}
lints, err := linter.NewRegistry([]string{"w_subject_common_name_included"})
test.AssertNotError(t, err, "Failed to create zlint registry")
@ -225,8 +230,7 @@ func setup(t *testing.T) *testCtx {
time.Second,
blog.NewMock(),
metrics.NoopRegisterer,
signatureCount,
signErrorCount,
cametrics,
fc,
)
test.AssertNotError(t, err, "Failed to create ocsp impl")
@ -239,8 +243,7 @@ func setup(t *testing.T) *testCtx {
},
100,
blog.NewMock(),
signatureCount,
signErrorCount,
cametrics,
)
test.AssertNotError(t, err, "Failed to create crl impl")
@ -258,9 +261,7 @@ func setup(t *testing.T) *testCtx {
boulderIssuers: boulderIssuers,
keyPolicy: keyPolicy,
fc: fc,
stats: metrics.NoopRegisterer,
signatureCount: signatureCount,
signErrorCount: signErrorCount,
metrics: cametrics,
logger: blog.NewMock(),
}
}
@ -283,8 +284,6 @@ func TestSerialPrefix(t *testing.T) {
testCtx.maxNames,
testCtx.keyPolicy,
testCtx.logger,
testCtx.stats,
nil,
nil,
testCtx.fc)
test.AssertError(t, err, "CA should have failed with no SerialPrefix")
@ -303,13 +302,24 @@ func TestSerialPrefix(t *testing.T) {
testCtx.maxNames,
testCtx.keyPolicy,
testCtx.logger,
testCtx.stats,
nil,
nil,
testCtx.fc)
test.AssertError(t, err, "CA should have failed with too-large SerialPrefix")
}
func TestNoteSignError(t *testing.T) {
testCtx := setup(t)
metrics := testCtx.metrics
err := fmt.Errorf("wrapped non-signing error: %w", errors.New("oops"))
metrics.noteSignError(err)
test.AssertMetricWithLabelsEquals(t, metrics.signErrorCount, prometheus.Labels{"type": "HSM"}, 0)
err = fmt.Errorf("wrapped signing error: %w", pkcs11.Error(5))
metrics.noteSignError(err)
test.AssertMetricWithLabelsEquals(t, metrics.signErrorCount, prometheus.Labels{"type": "HSM"}, 1)
}
type TestCertificateIssuance struct {
ca *certificateAuthorityImpl
sa *mockSA
@ -401,9 +411,7 @@ func issueCertificateSubTestSetup(t *testing.T, e *ECDSAAllowList) (*certificate
testCtx.maxNames,
testCtx.keyPolicy,
testCtx.logger,
testCtx.stats,
testCtx.signatureCount,
testCtx.signErrorCount,
testCtx.metrics,
testCtx.fc)
test.AssertNotError(t, err, "Failed to create CA")
@ -451,9 +459,7 @@ func TestNoIssuers(t *testing.T) {
testCtx.maxNames,
testCtx.keyPolicy,
testCtx.logger,
testCtx.stats,
testCtx.signatureCount,
testCtx.signErrorCount,
testCtx.metrics,
testCtx.fc)
test.AssertError(t, err, "No issuers found during CA construction.")
test.AssertEquals(t, err.Error(), "must have at least one issuer")
@ -478,9 +484,7 @@ func TestMultipleIssuers(t *testing.T) {
testCtx.maxNames,
testCtx.keyPolicy,
testCtx.logger,
testCtx.stats,
testCtx.signatureCount,
testCtx.signErrorCount,
testCtx.metrics,
testCtx.fc)
test.AssertNotError(t, err, "Failed to remake CA")
@ -502,7 +506,7 @@ func TestMultipleIssuers(t *testing.T) {
}
}
test.Assert(t, validated, "Certificate failed signature validation")
test.AssertMetricWithLabelsEquals(t, ca.signatureCount, prometheus.Labels{"purpose": "precertificate", "status": "success"}, 1)
test.AssertMetricWithLabelsEquals(t, ca.metrics.signatureCount, prometheus.Labels{"purpose": "precertificate", "status": "success"}, 1)
// Test that an ECDSA CSR gets issuance from an ECDSA issuer.
issuedCert, err = ca.IssuePrecertificate(ctx, &capb.IssueCertificateRequest{Csr: ECDSACSR, RegistrationID: arbitraryRegID, CertProfileName: selectedProfile})
@ -518,7 +522,7 @@ func TestMultipleIssuers(t *testing.T) {
}
}
test.Assert(t, validated, "Certificate failed signature validation")
test.AssertMetricWithLabelsEquals(t, ca.signatureCount, prometheus.Labels{"purpose": "precertificate", "status": "success"}, 2)
test.AssertMetricWithLabelsEquals(t, ca.metrics.signatureCount, prometheus.Labels{"purpose": "precertificate", "status": "success"}, 2)
}
func TestUnpredictableIssuance(t *testing.T) {
@ -558,9 +562,7 @@ func TestUnpredictableIssuance(t *testing.T) {
testCtx.maxNames,
testCtx.keyPolicy,
testCtx.logger,
testCtx.stats,
testCtx.signatureCount,
testCtx.signErrorCount,
testCtx.metrics,
testCtx.fc)
test.AssertNotError(t, err, "Failed to remake CA")
@ -729,9 +731,7 @@ func TestProfiles(t *testing.T) {
testCtx.maxNames,
testCtx.keyPolicy,
testCtx.logger,
testCtx.stats,
testCtx.signatureCount,
testCtx.signErrorCount,
testCtx.metrics,
testCtx.fc,
)
@ -873,9 +873,7 @@ func TestInvalidCSRs(t *testing.T) {
testCtx.maxNames,
testCtx.keyPolicy,
testCtx.logger,
testCtx.stats,
testCtx.signatureCount,
testCtx.signErrorCount,
testCtx.metrics,
testCtx.fc)
test.AssertNotError(t, err, "Failed to create CA")
@ -886,7 +884,7 @@ func TestInvalidCSRs(t *testing.T) {
_, err = ca.IssuePrecertificate(ctx, issueReq)
test.AssertErrorIs(t, err, testCase.errorType)
test.AssertMetricWithLabelsEquals(t, ca.signatureCount, prometheus.Labels{"purpose": "cert"}, 0)
test.AssertMetricWithLabelsEquals(t, ca.metrics.signatureCount, prometheus.Labels{"purpose": "cert"}, 0)
test.AssertError(t, err, testCase.errorMessage)
if testCase.check != nil {
@ -914,9 +912,7 @@ func TestRejectValidityTooLong(t *testing.T) {
testCtx.maxNames,
testCtx.keyPolicy,
testCtx.logger,
testCtx.stats,
nil,
nil,
testCtx.metrics,
testCtx.fc)
test.AssertNotError(t, err, "Failed to create CA")
@ -958,12 +954,12 @@ func countMustStaple(t *testing.T, cert *x509.Certificate) (count int) {
}
func issueCertificateSubTestMustStaple(t *testing.T, i *TestCertificateIssuance) {
test.AssertMetricWithLabelsEquals(t, i.ca.signatureCount, prometheus.Labels{"purpose": "precertificate"}, 1)
test.AssertMetricWithLabelsEquals(t, i.ca.metrics.signatureCount, prometheus.Labels{"purpose": "precertificate"}, 1)
test.AssertEquals(t, countMustStaple(t, i.cert), 1)
}
func issueCertificateSubTestUnknownExtension(t *testing.T, i *TestCertificateIssuance) {
test.AssertMetricWithLabelsEquals(t, i.ca.signatureCount, prometheus.Labels{"purpose": "precertificate"}, 1)
test.AssertMetricWithLabelsEquals(t, i.ca.metrics.signatureCount, prometheus.Labels{"purpose": "precertificate"}, 1)
// NOTE: The hard-coded value here will have to change over time as Boulder
// adds or removes (unrequested/default) extensions in certificates.
@ -972,7 +968,7 @@ func issueCertificateSubTestUnknownExtension(t *testing.T, i *TestCertificateIss
}
func issueCertificateSubTestCTPoisonExtension(t *testing.T, i *TestCertificateIssuance) {
test.AssertMetricWithLabelsEquals(t, i.ca.signatureCount, prometheus.Labels{"purpose": "precertificate"}, 1)
test.AssertMetricWithLabelsEquals(t, i.ca.metrics.signatureCount, prometheus.Labels{"purpose": "precertificate"}, 1)
}
func findExtension(extensions []pkix.Extension, id asn1.ObjectIdentifier) *pkix.Extension {
@ -1017,9 +1013,7 @@ func TestIssueCertificateForPrecertificate(t *testing.T) {
testCtx.maxNames,
testCtx.keyPolicy,
testCtx.logger,
testCtx.stats,
testCtx.signatureCount,
testCtx.signErrorCount,
testCtx.metrics,
testCtx.fc)
test.AssertNotError(t, err, "Failed to create CA")
@ -1031,8 +1025,8 @@ func TestIssueCertificateForPrecertificate(t *testing.T) {
test.AssertNotError(t, err, "Failed to issue precert")
parsedPrecert, err := x509.ParseCertificate(precert.DER)
test.AssertNotError(t, err, "Failed to parse precert")
test.AssertMetricWithLabelsEquals(t, ca.signatureCount, prometheus.Labels{"purpose": "precertificate", "status": "success"}, 1)
test.AssertMetricWithLabelsEquals(t, ca.signatureCount, prometheus.Labels{"purpose": "certificate", "status": "success"}, 0)
test.AssertMetricWithLabelsEquals(t, ca.metrics.signatureCount, prometheus.Labels{"purpose": "precertificate", "status": "success"}, 1)
test.AssertMetricWithLabelsEquals(t, ca.metrics.signatureCount, prometheus.Labels{"purpose": "certificate", "status": "success"}, 0)
// Check for poison extension
poisonExtension := findExtension(parsedPrecert.Extensions, OIDExtensionCTPoison)
@ -1056,7 +1050,7 @@ func TestIssueCertificateForPrecertificate(t *testing.T) {
test.AssertNotError(t, err, "Failed to issue cert from precert")
parsedCert, err := x509.ParseCertificate(cert.Der)
test.AssertNotError(t, err, "Failed to parse cert")
test.AssertMetricWithLabelsEquals(t, ca.signatureCount, prometheus.Labels{"purpose": "certificate", "status": "success"}, 1)
test.AssertMetricWithLabelsEquals(t, ca.metrics.signatureCount, prometheus.Labels{"purpose": "certificate", "status": "success"}, 1)
// Check for SCT list extension
sctListExtension := findExtension(parsedCert.Extensions, OIDExtensionSCTList)
@ -1088,9 +1082,7 @@ func TestIssueCertificateForPrecertificateWithSpecificCertificateProfile(t *test
testCtx.maxNames,
testCtx.keyPolicy,
testCtx.logger,
testCtx.stats,
testCtx.signatureCount,
testCtx.signErrorCount,
testCtx.metrics,
testCtx.fc)
test.AssertNotError(t, err, "Failed to create CA")
@ -1108,8 +1100,8 @@ func TestIssueCertificateForPrecertificateWithSpecificCertificateProfile(t *test
test.AssertNotError(t, err, "Failed to issue precert")
parsedPrecert, err := x509.ParseCertificate(precert.DER)
test.AssertNotError(t, err, "Failed to parse precert")
test.AssertMetricWithLabelsEquals(t, ca.signatureCount, prometheus.Labels{"purpose": "precertificate", "status": "success"}, 1)
test.AssertMetricWithLabelsEquals(t, ca.signatureCount, prometheus.Labels{"purpose": "certificate", "status": "success"}, 0)
test.AssertMetricWithLabelsEquals(t, ca.metrics.signatureCount, prometheus.Labels{"purpose": "precertificate", "status": "success"}, 1)
test.AssertMetricWithLabelsEquals(t, ca.metrics.signatureCount, prometheus.Labels{"purpose": "certificate", "status": "success"}, 0)
// Check for poison extension
poisonExtension := findExtension(parsedPrecert.Extensions, OIDExtensionCTPoison)
@ -1133,7 +1125,7 @@ func TestIssueCertificateForPrecertificateWithSpecificCertificateProfile(t *test
test.AssertNotError(t, err, "Failed to issue cert from precert")
parsedCert, err := x509.ParseCertificate(cert.Der)
test.AssertNotError(t, err, "Failed to parse cert")
test.AssertMetricWithLabelsEquals(t, ca.signatureCount, prometheus.Labels{"purpose": "certificate", "status": "success"}, 1)
test.AssertMetricWithLabelsEquals(t, ca.metrics.signatureCount, prometheus.Labels{"purpose": "certificate", "status": "success"}, 1)
// Check for SCT list extension
sctListExtension := findExtension(parsedCert.Extensions, OIDExtensionSCTList)
@ -1210,9 +1202,7 @@ func TestIssueCertificateForPrecertificateDuplicateSerial(t *testing.T) {
testCtx.maxNames,
testCtx.keyPolicy,
testCtx.logger,
testCtx.stats,
testCtx.signatureCount,
testCtx.signErrorCount,
testCtx.metrics,
testCtx.fc)
test.AssertNotError(t, err, "Failed to create CA")
@ -1228,7 +1218,7 @@ func TestIssueCertificateForPrecertificateDuplicateSerial(t *testing.T) {
issueReq := capb.IssueCertificateRequest{Csr: CNandSANCSR, RegistrationID: arbitraryRegID, OrderID: 0}
precert, err := ca.IssuePrecertificate(ctx, &issueReq)
test.AssertNotError(t, err, "Failed to issue precert")
test.AssertMetricWithLabelsEquals(t, ca.signatureCount, prometheus.Labels{"purpose": "precertificate", "status": "success"}, 1)
test.AssertMetricWithLabelsEquals(t, ca.metrics.signatureCount, prometheus.Labels{"purpose": "precertificate", "status": "success"}, 1)
_, err = ca.IssueCertificateForPrecertificate(ctx, &capb.IssueCertificateForPrecertificateRequest{
DER: precert.DER,
SCTs: sctBytes,
@ -1244,7 +1234,7 @@ func TestIssueCertificateForPrecertificateDuplicateSerial(t *testing.T) {
}
// The success metric doesn't increase when a duplicate certificate issuance
// is attempted.
test.AssertMetricWithLabelsEquals(t, ca.signatureCount, prometheus.Labels{"purpose": "certificate", "status": "success"}, 0)
test.AssertMetricWithLabelsEquals(t, ca.metrics.signatureCount, prometheus.Labels{"purpose": "certificate", "status": "success"}, 0)
// Now check what happens if there is an error (e.g. timeout) while checking
// for the duplicate.
@ -1263,9 +1253,7 @@ func TestIssueCertificateForPrecertificateDuplicateSerial(t *testing.T) {
testCtx.maxNames,
testCtx.keyPolicy,
testCtx.logger,
testCtx.stats,
testCtx.signatureCount,
testCtx.signErrorCount,
testCtx.metrics,
testCtx.fc)
test.AssertNotError(t, err, "Failed to create CA")
@ -1284,7 +1272,7 @@ func TestIssueCertificateForPrecertificateDuplicateSerial(t *testing.T) {
}
// The success metric doesn't increase when a duplicate certificate issuance
// is attempted.
test.AssertMetricWithLabelsEquals(t, ca.signatureCount, prometheus.Labels{"purpose": "certificate", "status": "success"}, 0)
test.AssertMetricWithLabelsEquals(t, ca.metrics.signatureCount, prometheus.Labels{"purpose": "certificate", "status": "success"}, 0)
}
func TestGenerateSKID(t *testing.T) {

33
ca/crl.go
View File

@ -10,7 +10,6 @@ import (
"google.golang.org/grpc"
"github.com/miekg/pkcs11"
"github.com/prometheus/client_golang/prometheus"
capb "github.com/letsencrypt/boulder/ca/proto"
@ -23,12 +22,11 @@ import (
type crlImpl struct {
capb.UnsafeCRLGeneratorServer
issuers map[issuance.NameID]*issuance.Issuer
profile *issuance.CRLProfile
maxLogLen int
log blog.Logger
signatureCount *prometheus.CounterVec
signErrorCount *prometheus.CounterVec
issuers map[issuance.NameID]*issuance.Issuer
profile *issuance.CRLProfile
maxLogLen int
log blog.Logger
metrics *caMetrics
}
var _ capb.CRLGeneratorServer = (*crlImpl)(nil)
@ -42,8 +40,7 @@ func NewCRLImpl(
profileConfig issuance.CRLProfileConfig,
maxLogLen int,
logger blog.Logger,
signatureCount *prometheus.CounterVec,
signErrorCount *prometheus.CounterVec,
metrics *caMetrics,
) (*crlImpl, error) {
issuersByNameID := make(map[issuance.NameID]*issuance.Issuer, len(issuers))
for _, issuer := range issuers {
@ -56,12 +53,11 @@ func NewCRLImpl(
}
return &crlImpl{
issuers: issuersByNameID,
profile: profile,
maxLogLen: maxLogLen,
log: logger,
signatureCount: signatureCount,
signErrorCount: signErrorCount,
issuers: issuersByNameID,
profile: profile,
maxLogLen: maxLogLen,
log: logger,
metrics: metrics,
}, nil
}
@ -144,13 +140,10 @@ func (ci *crlImpl) GenerateCRL(stream grpc.BidiStreamingServer[capb.GenerateCRLR
crlBytes, err := issuer.IssueCRL(ci.profile, req)
if err != nil {
var pkcs11Error *pkcs11.Error
if errors.As(err, &pkcs11Error) {
ci.signErrorCount.WithLabelValues("HSM").Inc()
}
ci.metrics.noteSignError(err)
return fmt.Errorf("signing crl: %w", err)
}
ci.signatureCount.With(prometheus.Labels{"purpose": "crl", "issuer": issuer.Name()}).Inc()
ci.metrics.signatureCount.With(prometheus.Labels{"purpose": "crl", "issuer": issuer.Name()}).Inc()
hash := sha256.Sum256(crlBytes)
ci.log.AuditInfof(

38
ca/ocsp.go
View File

@ -2,14 +2,12 @@ package ca
import (
"context"
"errors"
"fmt"
"strings"
"sync"
"time"
"github.com/jmhodges/clock"
"github.com/miekg/pkcs11"
"github.com/prometheus/client_golang/prometheus"
"golang.org/x/crypto/ocsp"
@ -23,13 +21,12 @@ import (
// ocspImpl provides a backing implementation for the OCSP gRPC service.
type ocspImpl struct {
capb.UnsafeOCSPGeneratorServer
issuers map[issuance.NameID]*issuance.Issuer
ocspLifetime time.Duration
ocspLogQueue *ocspLogQueue
log blog.Logger
signatureCount *prometheus.CounterVec
signErrorCount *prometheus.CounterVec
clk clock.Clock
issuers map[issuance.NameID]*issuance.Issuer
ocspLifetime time.Duration
ocspLogQueue *ocspLogQueue
log blog.Logger
metrics *caMetrics
clk clock.Clock
}
var _ capb.OCSPGeneratorServer = (*ocspImpl)(nil)
@ -41,8 +38,7 @@ func NewOCSPImpl(
ocspLogPeriod time.Duration,
logger blog.Logger,
stats prometheus.Registerer,
signatureCount *prometheus.CounterVec,
signErrorCount *prometheus.CounterVec,
metrics *caMetrics,
clk clock.Clock,
) (*ocspImpl, error) {
issuersByNameID := make(map[issuance.NameID]*issuance.Issuer, len(issuers))
@ -60,13 +56,12 @@ func NewOCSPImpl(
}
oi := &ocspImpl{
issuers: issuersByNameID,
ocspLifetime: ocspLifetime,
ocspLogQueue: ocspLogQueue,
log: logger,
signatureCount: signatureCount,
signErrorCount: signErrorCount,
clk: clk,
issuers: issuersByNameID,
ocspLifetime: ocspLifetime,
ocspLogQueue: ocspLogQueue,
log: logger,
metrics: metrics,
clk: clk,
}
return oi, nil
}
@ -125,12 +120,9 @@ func (oi *ocspImpl) GenerateOCSP(ctx context.Context, req *capb.GenerateOCSPRequ
ocspResponse, err := ocsp.CreateResponse(issuer.Cert.Certificate, issuer.Cert.Certificate, tbsResponse, issuer.Signer)
if err == nil {
oi.signatureCount.With(prometheus.Labels{"purpose": "ocsp", "issuer": issuer.Name()}).Inc()
oi.metrics.signatureCount.With(prometheus.Labels{"purpose": "ocsp", "issuer": issuer.Name()}).Inc()
} else {
var pkcs11Error *pkcs11.Error
if errors.As(err, &pkcs11Error) {
oi.signErrorCount.WithLabelValues("HSM").Inc()
}
oi.metrics.noteSignError(err)
}
return &capb.OCSPResponse{Response: ocspResponse}, err
}

4
ca/ocsp_test.go
View File

@ -43,9 +43,7 @@ func TestOCSP(t *testing.T) {
testCtx.maxNames,
testCtx.keyPolicy,
testCtx.logger,
testCtx.stats,
testCtx.signatureCount,
testCtx.signErrorCount,
testCtx.metrics,
testCtx.fc)
test.AssertNotError(t, err, "Failed to create CA")
ocspi := testCtx.ocsp

27
cmd/boulder-ca/main.go
View File

@ -7,7 +7,6 @@ import (
"reflect"
"time"
"github.com/prometheus/client_golang/prometheus"
"github.com/zmap/zlint/v3/lint"
"github.com/letsencrypt/boulder/ca"
@ -173,21 +172,7 @@ func main() {
defer oTelShutdown(context.Background())
logger.Info(cmd.VersionString())
// These two metrics are created and registered here so they can be shared
// between NewCertificateAuthorityImpl and NewOCSPImpl.
signatureCount := prometheus.NewCounterVec(
prometheus.CounterOpts{
Name: "signatures",
Help: "Number of signatures",
},
[]string{"purpose", "issuer"})
scope.MustRegister(signatureCount)
signErrorCount := prometheus.NewCounterVec(prometheus.CounterOpts{
Name: "signature_errors",
Help: "A counter of signature errors labelled by error type",
}, []string{"type"})
scope.MustRegister(signErrorCount)
metrics := ca.NewCAMetrics(scope)
cmd.FailOnError(c.PA.CheckChallenges(), "Invalid PA configuration")
@ -270,8 +255,7 @@ func main() {
c.CA.OCSPLogPeriod.Duration,
logger,
scope,
signatureCount,
signErrorCount,
metrics,
clk,
)
cmd.FailOnError(err, "Failed to create OCSP impl")
@ -287,8 +271,7 @@ func main() {
c.CA.Issuance.CRLProfile,
c.CA.OCSPLogMaxLength,
logger,
signatureCount,
signErrorCount,
metrics,
)
cmd.FailOnError(err, "Failed to create CRL impl")
@ -310,9 +293,7 @@ func main() {
c.CA.MaxNames,
kp,
logger,
scope,
signatureCount,
signErrorCount,
metrics,
clk)
cmd.FailOnError(err, "Failed to create CA impl")