Skip to content

Commit b8ee84d

Browse files
rolandshoemakerrolandshoemaker
authored
Switch GenerateOCSP to directly use protos instead of wrapper (letsencrypt#4549)
1 parent ef18f4c commit b8ee84d

File tree

9 files changed

+98
-127
lines changed

9 files changed

+98
-127
lines changed

‎ca/ca.go‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -392,8 +392,8 @@ var ocspStatusToCode = map[string]int{
392392
}
393393

394394
// GenerateOCSP produces a new OCSP response and returns it
395-
func (ca *CertificateAuthorityImpl) GenerateOCSP(ctx context.Context, xferObj core.OCSPSigningRequest) ([]byte, error) {
396-
cert, err := x509.ParseCertificate(xferObj.CertDER)
395+
func (ca *CertificateAuthorityImpl) GenerateOCSP(ctx context.Context, req *caPB.GenerateOCSPRequest) (*caPB.OCSPResponse, error) {
396+
cert, err := x509.ParseCertificate(req.CertDER)
397397
if err != nil {
398398
ca.log.AuditErr(err.Error())
399399
return nil, err
@@ -414,22 +414,22 @@ func (ca *CertificateAuthorityImpl) GenerateOCSP(ctx context.Context, xferObj co
414414

415415
now := ca.clk.Now().Truncate(time.Hour)
416416
tbsResponse := ocsp.Response{
417-
Status: ocspStatusToCode[xferObj.Status],
417+
Status: ocspStatusToCode[*req.Status],
418418
SerialNumber: cert.SerialNumber,
419419
ThisUpdate: now,
420420
NextUpdate: now.Add(ca.ocspLifetime),
421421
}
422422
if tbsResponse.Status == ocsp.Revoked {
423-
tbsResponse.RevokedAt = xferObj.RevokedAt
424-
tbsResponse.RevocationReason = int(xferObj.Reason)
423+
tbsResponse.RevokedAt = time.Unix(0, *req.RevokedAt)
424+
tbsResponse.RevocationReason = int(*req.Reason)
425425
}
426426

427427
ocspResponse, err := ocsp.CreateResponse(issuer.cert, issuer.cert, tbsResponse, issuer.ocspSigner)
428428
ca.noteSignError(err)
429429
if err == nil {
430430
ca.signatureCount.With(prometheus.Labels{"purpose": "ocsp"}).Inc()
431431
}
432-
return ocspResponse, err
432+
return &caPB.OCSPResponse{Response: ocspResponse}, err
433433
}
434434

435435
func (ca *CertificateAuthorityImpl) IssuePrecertificate(ctx context.Context, issueReq *caPB.IssueCertificateRequest) (*caPB.IssuePrecertificateResponse, error) {
@@ -458,9 +458,10 @@ func (ca *CertificateAuthorityImpl) IssuePrecertificate(ctx context.Context, iss
458458
return nil, err
459459
}
460460

461-
ocspResp, err := ca.GenerateOCSP(ctx, core.OCSPSigningRequest{
461+
status := string(core.OCSPStatusGood)
462+
ocspResp, err := ca.GenerateOCSP(ctx, &caPB.GenerateOCSPRequest{
462463
CertDER: precertDER,
463-
Status: string(core.OCSPStatusGood),
464+
Status: &status,
464465
})
465466
if err != nil {
466467
err = berrors.InternalServerError(err.Error())
@@ -471,7 +472,7 @@ func (ca *CertificateAuthorityImpl) IssuePrecertificate(ctx context.Context, iss
471472
_, err = ca.sa.AddPrecertificate(ctx, &sapb.AddCertificateRequest{
472473
Der: precertDER,
473474
RegID: &regID,
474-
Ocsp: ocspResp,
475+
Ocsp: ocspResp.Response,
475476
Issued: &nowNanos,
476477
})
477478
if err != nil {
@@ -482,7 +483,7 @@ func (ca *CertificateAuthorityImpl) IssuePrecertificate(ctx context.Context, iss
482483
ca.queueOrphan(&orphanedCert{
483484
DER: precertDER,
484485
RegID: regID,
485-
OCSPResp: ocspResp,
486+
OCSPResp: ocspResp.Response,
486487
Precert: true,
487488
})
488489
}

‎ca/ca_test.go‎

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -24,14 +24,14 @@ import (
2424
"github.com/cloudflare/cfssl/helpers"
2525
"github.com/cloudflare/cfssl/signer"
2626
"github.com/cloudflare/cfssl/signer/local"
27-
"github.com/google/certificate-transparency-go"
27+
ct "github.com/google/certificate-transparency-go"
2828
cttls "github.com/google/certificate-transparency-go/tls"
2929
"github.com/jmhodges/clock"
3030
"github.com/prometheus/client_golang/prometheus"
3131
"github.com/zmap/zlint/lints"
3232
"golang.org/x/crypto/ocsp"
3333

34-
"github.com/letsencrypt/boulder/ca/config"
34+
ca_config "github.com/letsencrypt/boulder/ca/config"
3535
caPB "github.com/letsencrypt/boulder/ca/proto"
3636
"github.com/letsencrypt/boulder/cmd"
3737
"github.com/letsencrypt/boulder/core"
@@ -505,21 +505,22 @@ func TestOCSP(t *testing.T) {
505505
test.AssertNotError(t, err, "Failed to issue")
506506
parsedCert, err := x509.ParseCertificate(cert.DER)
507507
test.AssertNotError(t, err, "Failed to parse cert")
508-
ocspResp, err := ca.GenerateOCSP(ctx, core.OCSPSigningRequest{
508+
status := string(core.OCSPStatusGood)
509+
ocspResp, err := ca.GenerateOCSP(ctx, &caPB.GenerateOCSPRequest{
509510
CertDER: cert.DER,
510-
Status: string(core.OCSPStatusGood),
511+
Status: &status,
511512
})
512513
test.AssertNotError(t, err, "Failed to generate OCSP")
513-
parsed, err := ocsp.ParseResponse(ocspResp, caCert)
514+
parsed, err := ocsp.ParseResponse(ocspResp.Response, caCert)
514515
test.AssertNotError(t, err, "Failed to parse validate OCSP")
515516
test.AssertEquals(t, parsed.Status, 0)
516517
test.AssertEquals(t, parsed.RevocationReason, 0)
517518
test.AssertEquals(t, parsed.SerialNumber.Cmp(parsedCert.SerialNumber), 0)
518519

519520
// Test that signatures are checked.
520-
_, err = ca.GenerateOCSP(ctx, core.OCSPSigningRequest{
521+
_, err = ca.GenerateOCSP(ctx, &caPB.GenerateOCSPRequest{
521522
CertDER: append(cert.DER, byte(0)),
522-
Status: string(core.OCSPStatusGood),
523+
Status: &status,
523524
})
524525
test.AssertError(t, err, "Generated OCSP for cert with bad signature")
525526

@@ -560,22 +561,22 @@ func TestOCSP(t *testing.T) {
560561

561562
// ocspResp2 is a second OCSP response for `cert` (issued by caCert), and
562563
// should be signed by caCert.
563-
ocspResp2, err := ca.GenerateOCSP(ctx, core.OCSPSigningRequest{
564+
ocspResp2, err := ca.GenerateOCSP(ctx, &caPB.GenerateOCSPRequest{
564565
CertDER: append(cert.DER),
565-
Status: string(core.OCSPStatusGood),
566+
Status: &status,
566567
})
567568
test.AssertNotError(t, err, "Failed to sign second OCSP response")
568-
_, err = ocsp.ParseResponse(ocspResp2, caCert)
569+
_, err = ocsp.ParseResponse(ocspResp2.Response, caCert)
569570
test.AssertNotError(t, err, "Failed to parse / validate second OCSP response")
570571

571572
// newCertOcspResp is an OCSP response for `newCert` (issued by newIssuer),
572573
// and should be signed by newIssuer.
573-
newCertOcspResp, err := ca.GenerateOCSP(ctx, core.OCSPSigningRequest{
574+
newCertOcspResp, err := ca.GenerateOCSP(ctx, &caPB.GenerateOCSPRequest{
574575
CertDER: newCert.DER,
575-
Status: string(core.OCSPStatusGood),
576+
Status: &status,
576577
})
577578
test.AssertNotError(t, err, "Failed to generate OCSP")
578-
parsedNewCertOcspResp, err := ocsp.ParseResponse(newCertOcspResp, newIssuerCert)
579+
parsedNewCertOcspResp, err := ocsp.ParseResponse(newCertOcspResp.Response, newIssuerCert)
579580
test.AssertNotError(t, err, "Failed to parse / validate OCSP for newCert")
580581
test.AssertEquals(t, parsedNewCertOcspResp.Status, 0)
581582
test.AssertEquals(t, parsedNewCertOcspResp.RevocationReason, 0)

‎cmd/boulder-ra/main.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ func main() {
149149

150150
caConn, err := bgrpc.ClientSetup(c.RA.CAService, tlsConfig, clientMetrics, clk)
151151
cmd.FailOnError(err, "Unable to create CA client")
152-
cac := bgrpc.NewCertificateAuthorityClient(caPB.NewCertificateAuthorityClient(caConn), nil)
152+
cac := bgrpc.NewCertificateAuthorityClient(caPB.NewCertificateAuthorityClient(caConn))
153153

154154
var ctp *ctpolicy.CTPolicy
155155
conn, err := bgrpc.ClientSetup(c.RA.PublisherService, tlsConfig, clientMetrics, clk)

‎cmd/ocsp-updater/main.go‎

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ type OCSPUpdater struct {
4242

4343
dbMap ocspDB
4444

45-
cac core.CertificateAuthority
45+
ogc capb.OCSPGeneratorClient
4646
sac core.StorageAuthority
4747

4848
// Used to calculate how far back stale OCSP responses should be looked for
@@ -64,7 +64,7 @@ func newUpdater(
6464
stats metrics.Scope,
6565
clk clock.Clock,
6666
dbMap ocspDB,
67-
ca core.CertificateAuthority,
67+
ogc capb.OCSPGeneratorClient,
6868
sac core.StorageAuthority,
6969
apc akamaipb.AkamaiPurgerClient,
7070
config OCSPUpdaterConfig,
@@ -90,7 +90,7 @@ func newUpdater(
9090
stats: stats,
9191
clk: clk,
9292
dbMap: dbMap,
93-
cac: ca,
93+
ogc: ogc,
9494
log: log,
9595
sac: sac,
9696
ocspMinTimeToExpiry: config.OCSPMinTimeToExpiry.Duration,
@@ -177,20 +177,21 @@ func (updater *OCSPUpdater) generateResponse(ctx context.Context, status core.Ce
177177
}
178178
}
179179

180-
signRequest := core.OCSPSigningRequest{
180+
reason := int32(status.RevokedReason)
181+
statusStr := string(status.Status)
182+
revokedAt := status.RevokedDate.UnixNano()
183+
ocspResponse, err := updater.ogc.GenerateOCSP(ctx, &capb.GenerateOCSPRequest{
181184
CertDER: cert.DER,
182-
Reason: status.RevokedReason,
183-
Status: string(status.Status),
184-
RevokedAt: status.RevokedDate,
185-
}
186-
187-
ocspResponse, err := updater.cac.GenerateOCSP(ctx, signRequest)
185+
Reason: &reason,
186+
Status: &statusStr,
187+
RevokedAt: &revokedAt,
188+
})
188189
if err != nil {
189190
return nil, err
190191
}
191192

192193
status.OCSPLastUpdated = updater.clk.Now()
193-
status.OCSPResponse = ocspResponse
194+
status.OCSPResponse = ocspResponse.Response
194195

195196
return &status, nil
196197
}
@@ -388,7 +389,7 @@ type OCSPUpdaterConfig struct {
388389
}
389390

390391
func setupClients(c OCSPUpdaterConfig, stats metrics.Scope, clk clock.Clock) (
391-
core.CertificateAuthority,
392+
capb.OCSPGeneratorClient,
392393
core.StorageAuthority,
393394
akamaipb.AkamaiPurgerClient,
394395
) {
@@ -404,7 +405,7 @@ func setupClients(c OCSPUpdaterConfig, stats metrics.Scope, clk clock.Clock) (
404405
// Make a CA client that is only capable of signing OCSP.
405406
// TODO(jsha): Once we've fully moved to gRPC, replace this
406407
// with a plain caPB.NewOCSPGeneratorClient.
407-
cac := bgrpc.NewCertificateAuthorityClient(nil, capb.NewOCSPGeneratorClient(caConn))
408+
ogc := capb.NewOCSPGeneratorClient(caConn)
408409

409410
saConn, err := bgrpc.ClientSetup(c.SAService, tls, clientMetrics, clk)
410411
cmd.FailOnError(err, "Failed to load credentials and create gRPC connection to SA")
@@ -417,7 +418,7 @@ func setupClients(c OCSPUpdaterConfig, stats metrics.Scope, clk clock.Clock) (
417418
apc = akamaipb.NewAkamaiPurgerClient(apcConn)
418419
}
419420

420-
return cac, sac, apc
421+
return ogc, sac, apc
421422
}
422423

423424
func main() {
@@ -450,13 +451,13 @@ func main() {
450451
sa.InitDBMetrics(dbMap, scope)
451452

452453
clk := cmd.Clock()
453-
cac, sac, apc := setupClients(conf, scope, clk)
454+
ogc, sac, apc := setupClients(conf, scope, clk)
454455

455456
updater, err := newUpdater(
456457
scope,
457458
clk,
458459
dbMap,
459-
cac,
460+
ogc,
460461
sac,
461462
apc,
462463
// Necessary evil for now

‎cmd/ocsp-updater/main_test.go‎

Lines changed: 7 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -23,31 +23,19 @@ import (
2323
"github.com/letsencrypt/boulder/sa/satest"
2424
"github.com/letsencrypt/boulder/test"
2525
"github.com/letsencrypt/boulder/test/vars"
26+
"google.golang.org/grpc"
2627
"gopkg.in/go-gorp/gorp.v2"
2728
)
2829

2930
var ctx = context.Background()
3031

31-
type mockCA struct {
32+
type mockOCSP struct {
3233
sleepTime time.Duration
3334
}
3435

35-
func (ca *mockCA) IssueCertificate(_ context.Context, _ *caPB.IssueCertificateRequest) (core.Certificate, error) {
36-
return core.Certificate{}, nil
37-
}
38-
39-
func (ca *mockCA) IssuePrecertificate(_ context.Context, _ *caPB.IssueCertificateRequest) (*caPB.IssuePrecertificateResponse, error) {
40-
return nil, errors.New("IssuePrecertificate is not implemented by mockCA")
41-
}
42-
43-
func (ca *mockCA) IssueCertificateForPrecertificate(_ context.Context, _ *caPB.IssueCertificateForPrecertificateRequest) (core.Certificate, error) {
44-
return core.Certificate{}, errors.New("IssueCertificateForPrecertificate is not implemented by mockCA")
45-
}
46-
47-
func (ca *mockCA) GenerateOCSP(_ context.Context, xferObj core.OCSPSigningRequest) (ocsp []byte, err error) {
48-
ocsp = []byte{1, 2, 3}
36+
func (ca *mockOCSP) GenerateOCSP(_ context.Context, req *caPB.GenerateOCSPRequest, _ ...grpc.CallOption) (*caPB.OCSPResponse, error) {
4937
time.Sleep(ca.sleepTime)
50-
return
38+
return &caPB.OCSPResponse{Response: []byte{1, 2, 3}}, nil
5139
}
5240

5341
var log = blog.UseMock()
@@ -80,7 +68,7 @@ func setup(t *testing.T) (*OCSPUpdater, core.StorageAuthority, *gorp.DbMap, cloc
8068
metrics.NewNoopScope(),
8169
fc,
8270
dbMap,
83-
&mockCA{},
71+
&mockOCSP{},
8472
sa,
8573
nil,
8674
OCSPUpdaterConfig{
@@ -155,7 +143,7 @@ func TestGenerateOCSPResponses(t *testing.T) {
155143
// Note that this test also tests the basic functionality of
156144
// generateOCSPResponses.
157145
start := time.Now()
158-
updater.cac = &mockCA{time.Second}
146+
updater.ogc = &mockOCSP{time.Second}
159147
updater.parallelGenerateOCSPRequests = 10
160148
err = updater.generateOCSPResponses(ctx, certs, metrics.NewNoopScope())
161149
test.AssertNotError(t, err, "Couldn't generate OCSP responses")
@@ -458,7 +446,7 @@ func TestGenerateOCSPResponsePrecert(t *testing.T) {
458446
// Directly call generateResponse with the result, when the PrecertificateOCSP
459447
// feature flag is disabled we expect this to error because no matching
460448
// certificates row will be found.
461-
updater.cac = &mockCA{time.Second}
449+
updater.ogc = &mockOCSP{time.Second}
462450
_, err = updater.generateResponse(ctx, certs[0])
463451
test.AssertError(t, err, "generateResponse for precert without PrecertificateOCSP did not error")
464452

‎core/interfaces.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ type CertificateAuthority interface {
9999
// [RegistrationAuthority]
100100
IssueCertificateForPrecertificate(ctx context.Context, req *caPB.IssueCertificateForPrecertificateRequest) (Certificate, error)
101101

102-
GenerateOCSP(ctx context.Context, ocspReq OCSPSigningRequest) ([]byte, error)
102+
GenerateOCSP(ctx context.Context, ocspReq *caPB.GenerateOCSPRequest) (*caPB.OCSPResponse, error)
103103
}
104104

105105
// PolicyAuthority defines the public interface for the Boulder PA

0 commit comments

Comments
 (0)