Skip to content

Commit 7bf854f

Browse files
aarongableaarongable
authored
Move OCSP gRPC service to separate file and struct (letsencrypt#5402)
Create a new `ocspImpl` struct which satisfies the interface required by the `OCSPGenerator` gRPC service. Move the `GenerateOCSP` method from the `certificateAuthorityImpl` to this new type. To support existing gRPC clients, keep a reference to the new OCSP service in the CA impl, and maintain a pass-through `GenerateOCSP` method. Simplify some of the CA setup code, and make the CA implementation non-exported because it doesn't need to be. In order to maintain our existing signature and sign error metrics, they now need to be initialized outside the CA and OCSP constructors. This complicates the tests slightly, but seems like a worthwhile tradeoff. Fixes letsencrypt#5226 Fixes letsencrypt#5086
1 parent 5457680 commit 7bf854f

File tree

9 files changed

+382
-296
lines changed

9 files changed

+382
-296
lines changed

‎ca/ca.go‎

Lines changed: 54 additions & 154 deletions
Large diffs are not rendered by default.

‎ca/ca_test.go‎

Lines changed: 63 additions & 119 deletions
Large diffs are not rendered by default.

‎ca/ocsp.go‎

Lines changed: 122 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,140 @@
11
package ca
22

3-
// TODO(##5226): Move the GenerateOCSP service into this file too.
4-
53
import (
4+
"context"
5+
"errors"
66
"fmt"
77
"strings"
88
"sync"
99
"time"
1010

1111
"github.com/jmhodges/clock"
12+
"github.com/miekg/pkcs11"
1213
"github.com/prometheus/client_golang/prometheus"
1314
"golang.org/x/crypto/ocsp"
1415

16+
capb "github.com/letsencrypt/boulder/ca/proto"
17+
"github.com/letsencrypt/boulder/core"
18+
berrors "github.com/letsencrypt/boulder/errors"
19+
"github.com/letsencrypt/boulder/issuance"
1520
blog "github.com/letsencrypt/boulder/log"
1621
)
1722

23+
// ocspImpl provides a backing implementation for the OCSP gRPC service.
24+
type ocspImpl struct {
25+
capb.UnimplementedOCSPGeneratorServer
26+
sa certificateStorage
27+
// TODO(#5152): Replace IssuerID with IssuerNameID.
28+
issuers map[issuance.IssuerID]*issuance.Issuer
29+
ocspLifetime time.Duration
30+
ocspLogQueue *ocspLogQueue
31+
log blog.Logger
32+
signatureCount *prometheus.CounterVec
33+
signErrorCount *prometheus.CounterVec
34+
clk clock.Clock
35+
}
36+
37+
func NewOCSPImpl(
38+
sa certificateStorage,
39+
issuers []*issuance.Issuer,
40+
ocspLifetime time.Duration,
41+
ocspLogMaxLength int,
42+
ocspLogPeriod time.Duration,
43+
logger blog.Logger,
44+
stats prometheus.Registerer,
45+
signatureCount *prometheus.CounterVec,
46+
signErrorCount *prometheus.CounterVec,
47+
clk clock.Clock,
48+
) (*ocspImpl, error) {
49+
issuersByID := make(map[issuance.IssuerID]*issuance.Issuer, len(issuers))
50+
for _, issuer := range issuers {
51+
issuersByID[issuer.ID()] = issuer
52+
}
53+
54+
var ocspLogQueue *ocspLogQueue
55+
if ocspLogMaxLength > 0 {
56+
ocspLogQueue = newOCSPLogQueue(ocspLogMaxLength, ocspLogPeriod, stats, logger)
57+
}
58+
59+
oi := &ocspImpl{
60+
sa: sa,
61+
issuers: issuersByID,
62+
ocspLifetime: ocspLifetime,
63+
ocspLogQueue: ocspLogQueue,
64+
log: logger,
65+
signatureCount: signatureCount,
66+
signErrorCount: signErrorCount,
67+
clk: clk,
68+
}
69+
return oi, nil
70+
}
71+
72+
// LogOCSPLoop collects OCSP generation log events into bundles, and logs
73+
// them periodically.
74+
func (oi *ocspImpl) LogOCSPLoop() {
75+
if oi.ocspLogQueue != nil {
76+
oi.ocspLogQueue.loop()
77+
}
78+
}
79+
80+
// Stop asks this ocspImpl to shut down. It must be called after the
81+
// corresponding RPC service is shut down and there are no longer any inflight
82+
// RPCs. It will attempt to drain any logging queues (which may block), and will
83+
// return only when done.
84+
func (oi *ocspImpl) Stop() {
85+
if oi.ocspLogQueue != nil {
86+
oi.ocspLogQueue.stop()
87+
}
88+
}
89+
90+
// GenerateOCSP produces a new OCSP response and returns it
91+
func (oi *ocspImpl) GenerateOCSP(ctx context.Context, req *capb.GenerateOCSPRequest) (*capb.OCSPResponse, error) {
92+
// req.Status, req.Reason, and req.RevokedAt are often 0, for non-revoked certs.
93+
if core.IsAnyNilOrZero(req, req.Serial, req.IssuerID) {
94+
return nil, berrors.InternalServerError("Incomplete generate OCSP request")
95+
}
96+
97+
serialInt, err := core.StringToSerial(req.Serial)
98+
if err != nil {
99+
return nil, err
100+
}
101+
serial := serialInt
102+
103+
// TODO(#5152): Replace IssuerID with IssuerNameID.
104+
var ok bool
105+
issuer, ok := oi.issuers[issuance.IssuerID(req.IssuerID)]
106+
if !ok {
107+
return nil, fmt.Errorf("This CA doesn't have an issuer cert with ID %d", req.IssuerID)
108+
}
109+
110+
now := oi.clk.Now().Truncate(time.Hour)
111+
tbsResponse := ocsp.Response{
112+
Status: ocspStatusToCode[req.Status],
113+
SerialNumber: serial,
114+
ThisUpdate: now,
115+
NextUpdate: now.Add(oi.ocspLifetime),
116+
}
117+
if tbsResponse.Status == ocsp.Revoked {
118+
tbsResponse.RevokedAt = time.Unix(0, req.RevokedAt)
119+
tbsResponse.RevocationReason = int(req.Reason)
120+
}
121+
122+
if oi.ocspLogQueue != nil {
123+
oi.ocspLogQueue.enqueue(serial.Bytes(), now, ocsp.ResponseStatus(tbsResponse.Status))
124+
}
125+
126+
ocspResponse, err := ocsp.CreateResponse(issuer.Cert.Certificate, issuer.Cert.Certificate, tbsResponse, issuer.Signer)
127+
if err == nil {
128+
oi.signatureCount.With(prometheus.Labels{"purpose": "ocsp", "issuer": issuer.Name()}).Inc()
129+
} else {
130+
var pkcs11Error *pkcs11.Error
131+
if errors.As(err, &pkcs11Error) {
132+
oi.signErrorCount.WithLabelValues("HSM").Inc()
133+
}
134+
}
135+
return &capb.OCSPResponse{Response: ocspResponse}, err
136+
}
137+
18138
// ocspLogQueue accumulates OCSP logging events and writes several of them
19139
// in a single log line. This reduces the number of log lines and bytes,
20140
// which would otherwise be quite high. As of Jan 2021 we do approximately

‎ca/ocsp_test.go‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,14 @@
11
package ca
22

33
import (
4+
"context"
5+
"crypto/x509"
46
"encoding/hex"
57
"testing"
68
"time"
79

10+
capb "github.com/letsencrypt/boulder/ca/proto"
11+
"github.com/letsencrypt/boulder/core"
812
blog "github.com/letsencrypt/boulder/log"
913
"github.com/letsencrypt/boulder/metrics"
1014
"github.com/letsencrypt/boulder/test"
@@ -20,6 +24,89 @@ func serial(t *testing.T) []byte {
2024

2125
}
2226

27+
func TestOCSP(t *testing.T) {
28+
testCtx := setup(t)
29+
ca, err := NewCertificateAuthorityImpl(
30+
&mockSA{},
31+
testCtx.pa,
32+
testCtx.ocsp,
33+
testCtx.boulderIssuers,
34+
nil,
35+
testCtx.certExpiry,
36+
testCtx.certBackdate,
37+
testCtx.serialPrefix,
38+
testCtx.maxNames,
39+
testCtx.keyPolicy,
40+
nil,
41+
testCtx.logger,
42+
testCtx.stats,
43+
testCtx.signatureCount,
44+
testCtx.signErrorCount,
45+
testCtx.fc)
46+
test.AssertNotError(t, err, "Failed to create CA")
47+
ocspi := testCtx.ocsp
48+
49+
// Issue a certificate from the RSA issuer caCert, then check OCSP comes from the same issuer.
50+
rsaIssuerID := ca.issuers.byAlg[x509.RSA].ID()
51+
rsaCertPB, err := ca.IssuePrecertificate(ctx, &capb.IssueCertificateRequest{Csr: CNandSANCSR, RegistrationID: arbitraryRegID})
52+
test.AssertNotError(t, err, "Failed to issue certificate")
53+
rsaCert, err := x509.ParseCertificate(rsaCertPB.DER)
54+
test.AssertNotError(t, err, "Failed to parse rsaCert")
55+
rsaOCSPPB, err := ocspi.GenerateOCSP(ctx, &capb.GenerateOCSPRequest{
56+
Serial: core.SerialToString(rsaCert.SerialNumber),
57+
IssuerID: int64(rsaIssuerID),
58+
Status: string(core.OCSPStatusGood),
59+
})
60+
test.AssertNotError(t, err, "Failed to generate OCSP")
61+
rsaOCSP, err := ocsp.ParseResponse(rsaOCSPPB.Response, caCert.Certificate)
62+
test.AssertNotError(t, err, "Failed to parse / validate OCSP for rsaCert")
63+
test.AssertEquals(t, rsaOCSP.Status, 0)
64+
test.AssertEquals(t, rsaOCSP.RevocationReason, 0)
65+
test.AssertEquals(t, rsaOCSP.SerialNumber.Cmp(rsaCert.SerialNumber), 0)
66+
67+
// Issue a certificate from the ECDSA issuer caCert2, then check OCSP comes from the same issuer.
68+
ecdsaIssuerID := ca.issuers.byAlg[x509.ECDSA].ID()
69+
ecdsaCertPB, err := ca.IssuePrecertificate(ctx, &capb.IssueCertificateRequest{Csr: ECDSACSR, RegistrationID: arbitraryRegID})
70+
test.AssertNotError(t, err, "Failed to issue certificate")
71+
ecdsaCert, err := x509.ParseCertificate(ecdsaCertPB.DER)
72+
test.AssertNotError(t, err, "Failed to parse ecdsaCert")
73+
ecdsaOCSPPB, err := ocspi.GenerateOCSP(ctx, &capb.GenerateOCSPRequest{
74+
Serial: core.SerialToString(ecdsaCert.SerialNumber),
75+
IssuerID: int64(ecdsaIssuerID),
76+
Status: string(core.OCSPStatusGood),
77+
})
78+
test.AssertNotError(t, err, "Failed to generate OCSP")
79+
ecdsaOCSP, err := ocsp.ParseResponse(ecdsaOCSPPB.Response, caCert2.Certificate)
80+
test.AssertNotError(t, err, "Failed to parse / validate OCSP for ecdsaCert")
81+
test.AssertEquals(t, ecdsaOCSP.Status, 0)
82+
test.AssertEquals(t, ecdsaOCSP.RevocationReason, 0)
83+
test.AssertEquals(t, ecdsaOCSP.SerialNumber.Cmp(ecdsaCert.SerialNumber), 0)
84+
85+
// GenerateOCSP with a bad IssuerID should fail.
86+
_, err = ocspi.GenerateOCSP(context.Background(), &capb.GenerateOCSPRequest{
87+
Serial: core.SerialToString(rsaCert.SerialNumber),
88+
IssuerID: int64(666),
89+
Status: string(core.OCSPStatusGood),
90+
})
91+
test.AssertError(t, err, "GenerateOCSP didn't fail with invalid IssuerID")
92+
93+
// GenerateOCSP with a bad Serial should fail.
94+
_, err = ocspi.GenerateOCSP(context.Background(), &capb.GenerateOCSPRequest{
95+
Serial: "BADDECAF",
96+
IssuerID: int64(rsaIssuerID),
97+
Status: string(core.OCSPStatusGood),
98+
})
99+
test.AssertError(t, err, "GenerateOCSP didn't fail with invalid Serial")
100+
101+
// GenerateOCSP with a valid-but-nonexistent Serial should *not* fail.
102+
_, err = ocspi.GenerateOCSP(context.Background(), &capb.GenerateOCSPRequest{
103+
Serial: "03DEADBEEFBADDECAFFADEFACECAFE30",
104+
IssuerID: int64(rsaIssuerID),
105+
Status: string(core.OCSPStatusGood),
106+
})
107+
test.AssertNotError(t, err, "GenerateOCSP failed with fake-but-valid Serial")
108+
}
109+
23110
// Set up an ocspLogQueue with a very long period and a large maxLen,
24111
// to ensure any buffered entries get flushed on `.stop()`.
25112
func TestOcspLogFlushOnExit(t *testing.T) {

‎ca/proto/ca.pb.go‎

Lines changed: 2 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎ca/proto/ca.proto‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ message GenerateOCSPRequest {
4444
int32 reason = 3;
4545
int64 revokedAt = 4;
4646
string serial = 5;
47+
// TODO(#5152): Replace issuerID with issuerNameID.
4748
int64 issuerID = 6;
4849
}
4950

‎cmd/boulder-ca/main.go‎

Lines changed: 51 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"sync"
88

99
"github.com/beeker1121/goque"
10+
"github.com/prometheus/client_golang/prometheus"
1011
"google.golang.org/grpc/health"
1112
healthpb "google.golang.org/grpc/health/grpc_health_v1"
1213

@@ -165,6 +166,22 @@ func main() {
165166
defer logger.AuditPanic()
166167
logger.Info(cmd.VersionString())
167168

169+
// These two metrics are created and registered here so they can be shared
170+
// between NewCertificateAuthorityImpl and NewOCSPImpl.
171+
signatureCount := prometheus.NewCounterVec(
172+
prometheus.CounterOpts{
173+
Name: "signatures",
174+
Help: "Number of signatures",
175+
},
176+
[]string{"purpose", "issuer"})
177+
scope.MustRegister(signatureCount)
178+
179+
signErrorCount := prometheus.NewCounterVec(prometheus.CounterOpts{
180+
Name: "signature_errors",
181+
Help: "A counter of signature errors labelled by error type",
182+
}, []string{"type"})
183+
scope.MustRegister(signErrorCount)
184+
168185
cmd.FailOnError(c.PA.CheckChallenges(), "Invalid PA configuration")
169186

170187
pa, err := policy.New(c.PA.Challenges)
@@ -200,32 +217,58 @@ func main() {
200217
defer func() { _ = orphanQueue.Close() }()
201218
}
202219

220+
serverMetrics := bgrpc.NewServerMetrics(scope)
221+
var wg sync.WaitGroup
222+
223+
ocspi, err := ca.NewOCSPImpl(
224+
sa,
225+
boulderIssuers,
226+
c.CA.LifespanOCSP.Duration,
227+
c.CA.OCSPLogMaxLength,
228+
c.CA.OCSPLogPeriod.Duration,
229+
logger,
230+
scope,
231+
signatureCount,
232+
signErrorCount,
233+
clk,
234+
)
235+
cmd.FailOnError(err, "Failed to create OCSP impl")
236+
go ocspi.LogOCSPLoop()
237+
238+
ocspSrv, ocspListener, err := bgrpc.NewServer(c.CA.GRPCOCSPGenerator, tlsConfig, serverMetrics, clk)
239+
cmd.FailOnError(err, "Unable to setup CA gRPC server")
240+
capb.RegisterOCSPGeneratorServer(ocspSrv, ocspi)
241+
ocspHealth := health.NewServer()
242+
healthpb.RegisterHealthServer(ocspSrv, ocspHealth)
243+
wg.Add(1)
244+
go func() {
245+
cmd.FailOnError(cmd.FilterShutdownErrors(ocspSrv.Serve(ocspListener)),
246+
"OCSPGenerator gRPC service failed")
247+
wg.Done()
248+
}()
249+
203250
cai, err := ca.NewCertificateAuthorityImpl(
204251
sa,
205252
pa,
253+
ocspi,
206254
boulderIssuers,
207255
c.CA.ECDSAAllowedAccounts,
208256
c.CA.Expiry.Duration,
209257
c.CA.Backdate.Duration,
210258
c.CA.SerialPrefix,
211259
c.CA.MaxNames,
212-
c.CA.LifespanOCSP.Duration,
213260
kp,
214261
orphanQueue,
215-
c.CA.OCSPLogMaxLength,
216-
c.CA.OCSPLogPeriod.Duration,
217262
logger,
218263
scope,
264+
signatureCount,
265+
signErrorCount,
219266
clk)
220267
cmd.FailOnError(err, "Failed to create CA impl")
221268

222269
if orphanQueue != nil {
223270
go cai.OrphanIntegrationLoop()
224271
}
225-
go cai.LogOCSPLoop()
226-
227-
serverMetrics := bgrpc.NewServerMetrics(scope)
228-
var wg sync.WaitGroup
229272

230273
caSrv, caListener, err := bgrpc.NewServer(c.CA.GRPCCA, tlsConfig, serverMetrics, clk)
231274
cmd.FailOnError(err, "Unable to setup CA gRPC server")
@@ -238,25 +281,13 @@ func main() {
238281
wg.Done()
239282
}()
240283

241-
ocspSrv, ocspListener, err := bgrpc.NewServer(c.CA.GRPCOCSPGenerator, tlsConfig, serverMetrics, clk)
242-
cmd.FailOnError(err, "Unable to setup CA gRPC server")
243-
capb.RegisterOCSPGeneratorServer(ocspSrv, cai)
244-
ocspHealth := health.NewServer()
245-
healthpb.RegisterHealthServer(ocspSrv, ocspHealth)
246-
wg.Add(1)
247-
go func() {
248-
cmd.FailOnError(cmd.FilterShutdownErrors(ocspSrv.Serve(ocspListener)),
249-
"OCSPGenerator gRPC service failed")
250-
wg.Done()
251-
}()
252-
253284
go cmd.CatchSignals(logger, func() {
254285
caHealth.Shutdown()
255286
ocspHealth.Shutdown()
256287
caSrv.GracefulStop()
257288
ocspSrv.GracefulStop()
258289
wg.Wait()
259-
cai.Stop()
290+
ocspi.Stop()
260291
})
261292

262293
select {}

‎cmd/ocsp-updater/main.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,7 @@ func (updater *OCSPUpdater) findStaleOCSPResponses(oldestLastUpdatedTime time.Ti
152152
}
153153

154154
func (updater *OCSPUpdater) generateResponse(ctx context.Context, status core.CertificateStatus) (*core.CertificateStatus, error) {
155+
// TODO(#5152): Replace IssuerID with IssuerNameID.
155156
if status.IssuerID == nil || *status.IssuerID == 0 {
156157
return nil, errors.New("cert status has nil or 0 IssuerID")
157158
}

0 commit comments

Comments
 (0)