From 128b7848052ce416316bc4a51c2ae13edec1ffaa Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Wed, 21 Oct 2015 17:00:06 -0400 Subject: [PATCH] Add stats to CA --- ca/certificate-authority.go | 17 ++++++++++++++++- ca/certificate-authority_test.go | 30 +++++++++++++++++++----------- cmd/boulder-ca/main.go | 2 +- mocks/mocks.go | 20 ++++++++++++++++++++ 4 files changed, 56 insertions(+), 13 deletions(-) diff --git a/ca/certificate-authority.go b/ca/certificate-authority.go index ed185f217..f6510484a 100644 --- a/ca/certificate-authority.go +++ b/ca/certificate-authority.go @@ -20,6 +20,7 @@ import ( "sync" "time" + "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/cactus/go-statsd-client/statsd" "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/jmhodges/clock" "github.com/letsencrypt/boulder/cmd" "github.com/letsencrypt/boulder/core" @@ -49,6 +50,15 @@ var badSignatureAlgorithms = map[x509.SignatureAlgorithm]bool{ x509.ECDSAWithSHA1: true, } +// Metrics for CA statistics +const ( + // Increments when CA observes an HSM fault + metricHSMFaultObserved = "CA.OCSP.HSMFault.Observed" + + // Increments when CA rejects a request due to an HSM fault + metricHSMFaultRejected = "CA.OCSP.HSMFault.Rejected" +) + // HSM faults tend to be persistent. If we observe one, we will refuse any // further requests for signing (certificates or OCSP) for this period. const hsmFaultTimeout = 5 * 60 * time.Second @@ -64,6 +74,7 @@ type CertificateAuthorityImpl struct { Publisher core.Publisher Clk clock.Clock // TODO(jmhodges): should be private, like log log *blog.AuditLogger + stats statsd.Statter Prefix int // Prepended to the serial number ValidityPeriod time.Duration NotAfter time.Time @@ -79,7 +90,7 @@ type CertificateAuthorityImpl struct { // using CFSSL's authenticated signature scheme. A CA created in this way // issues for a single profile on the remote signer, which is indicated // by name in this constructor. -func NewCertificateAuthorityImpl(config cmd.CAConfig, clk clock.Clock, issuerCert string) (*CertificateAuthorityImpl, error) { +func NewCertificateAuthorityImpl(config cmd.CAConfig, clk clock.Clock, stats statsd.Statter, issuerCert string) (*CertificateAuthorityImpl, error) { var ca *CertificateAuthorityImpl var err error logger := blog.GetAuditLogger() @@ -139,6 +150,7 @@ func NewCertificateAuthorityImpl(config cmd.CAConfig, clk clock.Clock, issuerCer Prefix: config.SerialPrefix, Clk: clk, log: logger, + stats: stats, NotAfter: issuer.NotAfter, } @@ -199,6 +211,7 @@ func (ca *CertificateAuthorityImpl) noteHSMFault(err error) { defer ca.hsmFaultLock.Unlock() if err != nil { + ca.stats.Inc(metricHSMFaultObserved, 1, 1.0) ca.hsmFaultLastObserved = ca.Clk.Now() } return @@ -209,6 +222,7 @@ func (ca *CertificateAuthorityImpl) GenerateOCSP(xferObj core.OCSPSigningRequest if ca.checkHSMFault() { err := core.ServiceUnavailableError("GenerateOCSP call rejected; HSM is unavailable") ca.log.WarningErr(err) + ca.stats.Inc(metricHSMFaultRejected, 1, 1.0) return nil, err } @@ -247,6 +261,7 @@ func (ca *CertificateAuthorityImpl) IssueCertificate(csr x509.CertificateRequest if ca.checkHSMFault() { err := core.ServiceUnavailableError("IssueCertificate call rejected; HSM is unavailable") ca.log.WarningErr(err) + ca.stats.Inc(metricHSMFaultRejected, 1, 1.0) return emptyCert, err } diff --git a/ca/certificate-authority_test.go b/ca/certificate-authority_test.go index 2c1950ee7..8099180b9 100644 --- a/ca/certificate-authority_test.go +++ b/ca/certificate-authority_test.go @@ -116,6 +116,7 @@ type testCtx struct { reg core.Registration pa core.PolicyAuthority fc clock.FakeClock + stats *mocks.Statter cleanUp func() } @@ -192,7 +193,10 @@ func setup(t *testing.T) *testCtx { }, }, } - return &testCtx{ssa, caConfig, reg, pa, fc, cleanUp} + + stats := mocks.NewStatter() + + return &testCtx{ssa, caConfig, reg, pa, fc, &stats, cleanUp} } func TestFailNoSerial(t *testing.T) { @@ -200,14 +204,14 @@ func TestFailNoSerial(t *testing.T) { defer ctx.cleanUp() ctx.caConfig.SerialPrefix = 0 - _, err := NewCertificateAuthorityImpl(ctx.caConfig, ctx.fc, caCertFile) + _, err := NewCertificateAuthorityImpl(ctx.caConfig, ctx.fc, ctx.stats, caCertFile) test.AssertError(t, err, "CA should have failed with no SerialPrefix") } func TestIssueCertificate(t *testing.T) { ctx := setup(t) defer ctx.cleanUp() - ca, err := NewCertificateAuthorityImpl(ctx.caConfig, ctx.fc, caCertFile) + ca, err := NewCertificateAuthorityImpl(ctx.caConfig, ctx.fc, ctx.stats, caCertFile) test.AssertNotError(t, err, "Failed to create CA") ca.Publisher = &mocks.Publisher{} ca.PA = ctx.pa @@ -284,7 +288,7 @@ func TestIssueCertificate(t *testing.T) { func TestRejectNoName(t *testing.T) { ctx := setup(t) defer ctx.cleanUp() - ca, err := NewCertificateAuthorityImpl(ctx.caConfig, ctx.fc, caCertFile) + ca, err := NewCertificateAuthorityImpl(ctx.caConfig, ctx.fc, ctx.stats, caCertFile) test.AssertNotError(t, err, "Failed to create CA") ca.Publisher = &mocks.Publisher{} ca.PA = ctx.pa @@ -301,7 +305,7 @@ func TestRejectNoName(t *testing.T) { func TestRejectTooManyNames(t *testing.T) { ctx := setup(t) defer ctx.cleanUp() - ca, err := NewCertificateAuthorityImpl(ctx.caConfig, ctx.fc, caCertFile) + ca, err := NewCertificateAuthorityImpl(ctx.caConfig, ctx.fc, ctx.stats, caCertFile) test.AssertNotError(t, err, "Failed to create CA") ca.Publisher = &mocks.Publisher{} ca.PA = ctx.pa @@ -318,7 +322,7 @@ func TestRejectTooManyNames(t *testing.T) { func TestDeduplication(t *testing.T) { ctx := setup(t) defer ctx.cleanUp() - ca, err := NewCertificateAuthorityImpl(ctx.caConfig, ctx.fc, caCertFile) + ca, err := NewCertificateAuthorityImpl(ctx.caConfig, ctx.fc, ctx.stats, caCertFile) test.AssertNotError(t, err, "Failed to create CA") ca.Publisher = &mocks.Publisher{} ca.PA = ctx.pa @@ -342,7 +346,7 @@ func TestDeduplication(t *testing.T) { func TestRejectValidityTooLong(t *testing.T) { ctx := setup(t) defer ctx.cleanUp() - ca, err := NewCertificateAuthorityImpl(ctx.caConfig, ctx.fc, caCertFile) + ca, err := NewCertificateAuthorityImpl(ctx.caConfig, ctx.fc, ctx.stats, caCertFile) test.AssertNotError(t, err, "Failed to create CA") ca.Publisher = &mocks.Publisher{} ca.PA = ctx.pa @@ -360,7 +364,7 @@ func TestRejectValidityTooLong(t *testing.T) { func TestShortKey(t *testing.T) { ctx := setup(t) defer ctx.cleanUp() - ca, err := NewCertificateAuthorityImpl(ctx.caConfig, ctx.fc, caCertFile) + ca, err := NewCertificateAuthorityImpl(ctx.caConfig, ctx.fc, ctx.stats, caCertFile) ca.Publisher = &mocks.Publisher{} ca.PA = ctx.pa ca.SA = ctx.sa @@ -376,7 +380,7 @@ func TestShortKey(t *testing.T) { func TestRejectBadAlgorithm(t *testing.T) { ctx := setup(t) defer ctx.cleanUp() - ca, err := NewCertificateAuthorityImpl(ctx.caConfig, ctx.fc, caCertFile) + ca, err := NewCertificateAuthorityImpl(ctx.caConfig, ctx.fc, ctx.stats, caCertFile) ca.Publisher = &mocks.Publisher{} ca.PA = ctx.pa ca.SA = ctx.sa @@ -393,7 +397,7 @@ func TestCapitalizedLetters(t *testing.T) { ctx := setup(t) defer ctx.cleanUp() ctx.caConfig.MaxNames = 3 - ca, err := NewCertificateAuthorityImpl(ctx.caConfig, ctx.fc, caCertFile) + ca, err := NewCertificateAuthorityImpl(ctx.caConfig, ctx.fc, ctx.stats, caCertFile) ca.Publisher = &mocks.Publisher{} ca.PA = ctx.pa ca.SA = ctx.sa @@ -414,7 +418,7 @@ func TestHSMFaultTimeout(t *testing.T) { ctx := setup(t) defer ctx.cleanUp() - ca, err := NewCertificateAuthorityImpl(ctx.caConfig, ctx.fc, caCertFile) + ca, err := NewCertificateAuthorityImpl(ctx.caConfig, ctx.fc, ctx.stats, caCertFile) ca.Publisher = &mocks.Publisher{} ca.PA = ctx.pa ca.SA = ctx.sa @@ -471,4 +475,8 @@ func TestHSMFaultTimeout(t *testing.T) { _, err = ca.GenerateOCSP(ocspRequest) test.AssertError(t, err, "CA failed to persist HSM fault") test.AssertEquals(t, err.Error(), "GenerateOCSP call rejected; HSM is unavailable") + + // Verify that the appropriate stats got recorded for all this + test.AssertEquals(t, ctx.stats.Counters[metricHSMFaultObserved], int64(2)) + test.AssertEquals(t, ctx.stats.Counters[metricHSMFaultRejected], int64(4)) } diff --git a/cmd/boulder-ca/main.go b/cmd/boulder-ca/main.go index 585be6736..7217fdf4a 100644 --- a/cmd/boulder-ca/main.go +++ b/cmd/boulder-ca/main.go @@ -39,7 +39,7 @@ func main() { pa, err := policy.NewPolicyAuthorityImpl(paDbMap, c.PA.EnforcePolicyWhitelist) cmd.FailOnError(err, "Couldn't create PA") - cai, err := ca.NewCertificateAuthorityImpl(c.CA, clock.Default(), c.Common.IssuerCert) + cai, err := ca.NewCertificateAuthorityImpl(c.CA, clock.Default(), stats, c.Common.IssuerCert) cmd.FailOnError(err, "Failed to create CA impl") cai.PA = pa diff --git a/mocks/mocks.go b/mocks/mocks.go index e2c7aac63..a63341d74 100644 --- a/mocks/mocks.go +++ b/mocks/mocks.go @@ -15,6 +15,7 @@ import ( "strings" "time" + "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/cactus/go-statsd-client/statsd" "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/cloudflare/cfssl/config" "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/cloudflare/cfssl/info" "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/cloudflare/cfssl/ocsp" @@ -414,3 +415,22 @@ type BadHSMOCSPSigner string func (bhos BadHSMOCSPSigner) Sign(ocsp.SignRequest) ([]byte, error) { return nil, fmt.Errorf(string(bhos)) } + +// Statter is a stat counter that is a no-op except for locally handling Inc +// calls (which are most of what we use). +type Statter struct { + statsd.NoopClient + Counters map[string]int64 +} + +// Inc increments the indicated metric by the indicated value, in the Counters +// map maintained by the statter +func (s *Statter) Inc(metric string, value int64, rate float32) error { + s.Counters[metric] += value + return nil +} + +// NewStatter returns an empty statter with all counters zero +func NewStatter() Statter { + return Statter{statsd.NoopClient{}, map[string]int64{}} +}