X Tutup
Skip to content

Commit fd561ef

Browse files
Roland Bracewell Shoemakerjsha
Roland Bracewell Shoemaker
authored and
jsha
committed
Block issuance on first OCSP response generation (letsencrypt#2633)
Generate first OCSP response in ca.IssueCertificate instead of ocsp-updater.newCertificateTick if features.GenerateOCSPEarly is enabled. Adds a new field to the sa.AddCertiifcate RPC for the OCSP response and only adds it to the certificate status + sets ocspLastUpdated if it is a non-empty slice. ocsp-updater.newCertificateTick stays the same so we can catch certificates that were successfully signed + stored but a OCSP response couldn't be generated (for whatever reason). Fixes letsencrypt#2477.
1 parent f4c11a6 commit fd561ef

File tree

17 files changed

+183
-120
lines changed

17 files changed

+183
-120
lines changed

‎ca/ca.go‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import (
3030
"github.com/letsencrypt/boulder/core"
3131
csrlib "github.com/letsencrypt/boulder/csr"
3232
berrors "github.com/letsencrypt/boulder/errors"
33+
"github.com/letsencrypt/boulder/features"
3334
"github.com/letsencrypt/boulder/goodkey"
3435
blog "github.com/letsencrypt/boulder/log"
3536
"github.com/letsencrypt/boulder/metrics"
@@ -97,7 +98,7 @@ const (
9798
)
9899

99100
type certificateStorage interface {
100-
AddCertificate(context.Context, []byte, int64) (string, error)
101+
AddCertificate(context.Context, []byte, int64, []byte) (string, error)
101102
}
102103

103104
// CertificateAuthorityImpl represents a CA that signs certificates, CRLs, and
@@ -492,8 +493,24 @@ func (ca *CertificateAuthorityImpl) IssueCertificate(ctx context.Context, csr x5
492493
return emptyCert, err
493494
}
494495

496+
var ocspResp []byte
497+
if features.Enabled(features.GenerateOCSPEarly) {
498+
ocspResp, err = ca.GenerateOCSP(ctx, core.OCSPSigningRequest{
499+
CertDER: certDER,
500+
Status: "good",
501+
})
502+
if err != nil {
503+
err = berrors.InternalServerError(err.Error())
504+
ca.log.AuditInfo(fmt.Sprintf("OCSP Signing failure: serial=[%s] pem=[%s] err=[%s]",
505+
serialHex, certPEM, err))
506+
// Ignore errors here to avoid orphaning the certificate. The
507+
// ocsp-updater will look for certs with a zero ocspLastUpdated
508+
// and generate the initial response in this case.
509+
}
510+
}
511+
495512
// Store the cert with the certificate authority, if provided
496-
_, err = ca.SA.AddCertificate(ctx, certDER, regID)
513+
_, err = ca.SA.AddCertificate(ctx, certDER, regID, ocspResp)
497514
if err != nil {
498515
err = berrors.InternalServerError(err.Error())
499516
// Note: This log line is parsed by cmd/orphan-finder. If you make any

‎ca/ca_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ type mockSA struct {
145145
certificate core.Certificate
146146
}
147147

148-
func (m *mockSA) AddCertificate(ctx context.Context, der []byte, _ int64) (string, error) {
148+
func (m *mockSA) AddCertificate(ctx context.Context, der []byte, _ int64, _ []byte) (string, error) {
149149
m.certificate.DER = der
150150
return "", nil
151151
}

‎cmd/cert-checker/main_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ func TestGetAndProcessCerts(t *testing.T) {
206206
rawCert.SerialNumber = big.NewInt(mrand.Int63())
207207
certDER, err := x509.CreateCertificate(rand.Reader, &rawCert, &rawCert, &testKey.PublicKey, testKey)
208208
test.AssertNotError(t, err, "Couldn't create certificate")
209-
_, err = sa.AddCertificate(context.Background(), certDER, reg.ID)
209+
_, err = sa.AddCertificate(context.Background(), certDER, reg.ID, nil)
210210
test.AssertNotError(t, err, "Couldn't add certificate")
211211
}
212212

‎cmd/ocsp-updater/main_test.go‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ func TestGenerateAndStoreOCSPResponse(t *testing.T) {
173173
reg := satest.CreateWorkingRegistration(t, sa)
174174
parsedCert, err := core.LoadCert("test-cert.pem")
175175
test.AssertNotError(t, err, "Couldn't read test certificate")
176-
_, err = sa.AddCertificate(ctx, parsedCert.Raw, reg.ID)
176+
_, err = sa.AddCertificate(ctx, parsedCert.Raw, reg.ID, nil)
177177
test.AssertNotError(t, err, "Couldn't add test-cert.pem")
178178

179179
status, err := sa.GetCertificateStatus(ctx, core.SerialToString(parsedCert.SerialNumber))
@@ -201,11 +201,11 @@ func TestGenerateOCSPResponses(t *testing.T) {
201201
reg := satest.CreateWorkingRegistration(t, sa)
202202
parsedCertA, err := core.LoadCert("test-cert.pem")
203203
test.AssertNotError(t, err, "Couldn't read test certificate")
204-
_, err = sa.AddCertificate(ctx, parsedCertA.Raw, reg.ID)
204+
_, err = sa.AddCertificate(ctx, parsedCertA.Raw, reg.ID, nil)
205205
test.AssertNotError(t, err, "Couldn't add test-cert.pem")
206206
parsedCertB, err := core.LoadCert("test-cert-b.pem")
207207
test.AssertNotError(t, err, "Couldn't read test certificate")
208-
_, err = sa.AddCertificate(ctx, parsedCertB.Raw, reg.ID)
208+
_, err = sa.AddCertificate(ctx, parsedCertB.Raw, reg.ID, nil)
209209
test.AssertNotError(t, err, "Couldn't add test-cert-b.pem")
210210

211211
// We need to set a fake "ocspLastUpdated" value for the two certs we created
@@ -250,7 +250,7 @@ func TestFindStaleOCSPResponses(t *testing.T) {
250250
reg := satest.CreateWorkingRegistration(t, sa)
251251
parsedCert, err := core.LoadCert("test-cert.pem")
252252
test.AssertNotError(t, err, "Couldn't read test certificate")
253-
_, err = sa.AddCertificate(ctx, parsedCert.Raw, reg.ID)
253+
_, err = sa.AddCertificate(ctx, parsedCert.Raw, reg.ID, nil)
254254
test.AssertNotError(t, err, "Couldn't add test-cert.pem")
255255

256256
// We need to set a fake "ocspLastUpdated" value for the cert we created
@@ -287,11 +287,11 @@ func TestFindStaleOCSPResponsesStaleMaxAge(t *testing.T) {
287287
reg := satest.CreateWorkingRegistration(t, sa)
288288
parsedCertA, err := core.LoadCert("test-cert.pem")
289289
test.AssertNotError(t, err, "Couldn't read test certificate")
290-
_, err = sa.AddCertificate(ctx, parsedCertA.Raw, reg.ID)
290+
_, err = sa.AddCertificate(ctx, parsedCertA.Raw, reg.ID, nil)
291291
test.AssertNotError(t, err, "Couldn't add test-cert.pem")
292292
parsedCertB, err := core.LoadCert("test-cert-b.pem")
293293
test.AssertNotError(t, err, "Couldn't read test certificate")
294-
_, err = sa.AddCertificate(ctx, parsedCertB.Raw, reg.ID)
294+
_, err = sa.AddCertificate(ctx, parsedCertB.Raw, reg.ID, nil)
295295
test.AssertNotError(t, err, "Couldn't add test-cert-b.pem")
296296

297297
// Set a "ocspLastUpdated" value of 3 days ago for parsedCertA
@@ -327,7 +327,7 @@ func TestGetCertificatesWithMissingResponses(t *testing.T) {
327327
reg := satest.CreateWorkingRegistration(t, sa)
328328
cert, err := core.LoadCert("test-cert.pem")
329329
test.AssertNotError(t, err, "Couldn't read test certificate")
330-
_, err = sa.AddCertificate(ctx, cert.Raw, reg.ID)
330+
_, err = sa.AddCertificate(ctx, cert.Raw, reg.ID, nil)
331331
test.AssertNotError(t, err, "Couldn't add test-cert.pem")
332332

333333
statuses, err := updater.getCertificatesWithMissingResponses(10)
@@ -342,7 +342,7 @@ func TestFindRevokedCertificatesToUpdate(t *testing.T) {
342342
reg := satest.CreateWorkingRegistration(t, sa)
343343
cert, err := core.LoadCert("test-cert.pem")
344344
test.AssertNotError(t, err, "Couldn't read test certificate")
345-
_, err = sa.AddCertificate(ctx, cert.Raw, reg.ID)
345+
_, err = sa.AddCertificate(ctx, cert.Raw, reg.ID, nil)
346346
test.AssertNotError(t, err, "Couldn't add test-cert.pem")
347347

348348
statuses, err := updater.findRevokedCertificatesToUpdate(10)
@@ -364,7 +364,7 @@ func TestNewCertificateTick(t *testing.T) {
364364
reg := satest.CreateWorkingRegistration(t, sa)
365365
parsedCert, err := core.LoadCert("test-cert.pem")
366366
test.AssertNotError(t, err, "Couldn't read test certificate")
367-
_, err = sa.AddCertificate(ctx, parsedCert.Raw, reg.ID)
367+
_, err = sa.AddCertificate(ctx, parsedCert.Raw, reg.ID, nil)
368368
test.AssertNotError(t, err, "Couldn't add test-cert.pem")
369369

370370
prev := fc.Now().Add(-time.Hour)
@@ -383,7 +383,7 @@ func TestOldOCSPResponsesTick(t *testing.T) {
383383
reg := satest.CreateWorkingRegistration(t, sa)
384384
parsedCert, err := core.LoadCert("test-cert.pem")
385385
test.AssertNotError(t, err, "Couldn't read test certificate")
386-
_, err = sa.AddCertificate(ctx, parsedCert.Raw, reg.ID)
386+
_, err = sa.AddCertificate(ctx, parsedCert.Raw, reg.ID, nil)
387387
test.AssertNotError(t, err, "Couldn't add test-cert.pem")
388388

389389
updater.ocspMinTimeToExpiry = 1 * time.Hour
@@ -415,7 +415,7 @@ func TestOldOCSPResponsesTickIsExpired(t *testing.T) {
415415
serial := core.SerialToString(parsedCert.SerialNumber)
416416

417417
// Add a new test certificate
418-
_, err = sa.AddCertificate(ctx, parsedCert.Raw, reg.ID)
418+
_, err = sa.AddCertificate(ctx, parsedCert.Raw, reg.ID, nil)
419419
test.AssertNotError(t, err, "Couldn't add test-cert.pem")
420420

421421
// We need to set a fake "ocspLastUpdated" value for the cert we created
@@ -458,7 +458,7 @@ func TestMissingReceiptsTick(t *testing.T) {
458458
parsedCert, err := core.LoadCert("test-cert.pem")
459459
test.AssertNotError(t, err, "Couldn't read test certificate")
460460
fc.Set(parsedCert.NotBefore.Add(time.Minute))
461-
_, err = sa.AddCertificate(ctx, parsedCert.Raw, reg.ID)
461+
_, err = sa.AddCertificate(ctx, parsedCert.Raw, reg.ID, nil)
462462
test.AssertNotError(t, err, "Couldn't add test-cert.pem")
463463

464464
updater.oldestIssuedSCT = 2 * time.Hour
@@ -502,7 +502,7 @@ func TestMissingOnlyReceiptsTick(t *testing.T) {
502502
parsedCert, err := core.LoadCert("test-cert.pem")
503503
test.AssertNotError(t, err, "Couldn't read test certificate")
504504
fc.Set(parsedCert.NotBefore.Add(time.Minute))
505-
_, err = sa.AddCertificate(ctx, parsedCert.Raw, reg.ID)
505+
_, err = sa.AddCertificate(ctx, parsedCert.Raw, reg.ID, nil)
506506
test.AssertNotError(t, err, "Couldn't add test-cert.pem")
507507

508508
updater.oldestIssuedSCT = 2 * time.Hour
@@ -599,7 +599,7 @@ func TestRevokedCertificatesTick(t *testing.T) {
599599
reg := satest.CreateWorkingRegistration(t, sa)
600600
parsedCert, err := core.LoadCert("test-cert.pem")
601601
test.AssertNotError(t, err, "Couldn't read test certificate")
602-
_, err = sa.AddCertificate(ctx, parsedCert.Raw, reg.ID)
602+
_, err = sa.AddCertificate(ctx, parsedCert.Raw, reg.ID, nil)
603603
test.AssertNotError(t, err, "Couldn't add test-cert.pem")
604604

605605
err = sa.MarkCertificateRevoked(ctx, core.SerialToString(parsedCert.SerialNumber), revocation.KeyCompromise)
@@ -625,7 +625,7 @@ func TestStoreResponseGuard(t *testing.T) {
625625
reg := satest.CreateWorkingRegistration(t, sa)
626626
parsedCert, err := core.LoadCert("test-cert.pem")
627627
test.AssertNotError(t, err, "Couldn't read test certificate")
628-
_, err = sa.AddCertificate(ctx, parsedCert.Raw, reg.ID)
628+
_, err = sa.AddCertificate(ctx, parsedCert.Raw, reg.ID, nil)
629629
test.AssertNotError(t, err, "Couldn't add test-cert.pem")
630630

631631
status, err := sa.GetCertificateStatus(ctx, core.SerialToString(parsedCert.SerialNumber))
@@ -704,7 +704,7 @@ func TestGetSubmittedReceipts(t *testing.T) {
704704
parsedCert, err := core.LoadCert("test-cert.pem")
705705
test.AssertNotError(t, err, "Couldn't read test certificate")
706706
fc.Set(parsedCert.NotBefore.Add(time.Minute))
707-
_, err = sa.AddCertificate(ctx, parsedCert.Raw, reg.ID)
707+
_, err = sa.AddCertificate(ctx, parsedCert.Raw, reg.ID, nil)
708708
test.AssertNotError(t, err, "Couldn't add test-cert.pem")
709709

710710
// Before adding any SCTs, there should be no receipts or errors for serial 00

‎cmd/orphan-finder/main.go‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ type config struct {
4747
}
4848

4949
type certificateStorage interface {
50-
AddCertificate(context.Context, []byte, int64) (string, error)
50+
AddCertificate(context.Context, []byte, int64, []byte) (string, error)
5151
GetCertificate(ctx context.Context, serial string) (core.Certificate, error)
5252
}
5353

@@ -110,7 +110,9 @@ func parseLogLine(sa certificateStorage, logger blog.Logger, line string) (found
110110
logger.AuditErr(fmt.Sprintf("Couldn't parse regID: %s, [%s]", err, line))
111111
return true, false
112112
}
113-
_, err = sa.AddCertificate(ctx, der, int64(regID))
113+
// OCSP-Updater will do the first response generation for this cert so pass an
114+
// empty OCSP response
115+
_, err = sa.AddCertificate(ctx, der, int64(regID), nil)
114116
if err != nil {
115117
logger.AuditErr(fmt.Sprintf("Failed to store certificate: %s, [%s]", err, line))
116118
return true, false
@@ -202,7 +204,7 @@ func main() {
202204
cmd.FailOnError(err, "Failed to read DER file")
203205
err = checkDER(sa, der)
204206
cmd.FailOnError(err, "Pre-AddCertificate checks failed")
205-
_, err = sa.AddCertificate(ctx, der, int64(*regID))
207+
_, err = sa.AddCertificate(ctx, der, int64(*regID), nil)
206208
cmd.FailOnError(err, "Failed to add certificate to database")
207209

208210
default:

‎cmd/orphan-finder/main_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ type mockSA struct {
2020
certificate core.Certificate
2121
}
2222

23-
func (m *mockSA) AddCertificate(ctx context.Context, der []byte, _ int64) (string, error) {
23+
func (m *mockSA) AddCertificate(ctx context.Context, der []byte, _ int64, _ []byte) (string, error) {
2424
m.certificate.DER = der
2525
return "", nil
2626
}

‎core/interfaces.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ type StorageAdder interface {
117117
UpdatePendingAuthorization(ctx context.Context, authz Authorization) error
118118
FinalizeAuthorization(ctx context.Context, authz Authorization) error
119119
MarkCertificateRevoked(ctx context.Context, serial string, reasonCode revocation.Reason) error
120-
AddCertificate(ctx context.Context, der []byte, regID int64) (digest string, err error)
120+
AddCertificate(ctx context.Context, der []byte, regID int64, ocsp []byte) (digest string, err error)
121121
AddSCTReceipt(ctx context.Context, sct SignedCertificateTimestamp) error
122122
RevokeAuthorizationsByDomain(ctx context.Context, domain AcmeIdentifier) (finalized, pending int64, err error)
123123
DeactivateRegistration(ctx context.Context, id int64) error

‎features/featureflag_string.go‎

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

‎features/features.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ const (
1919
GoogleSafeBrowsingV4
2020
UseAIAIssuerURL
2121
AllowTLS02Challenges
22+
GenerateOCSPEarly
2223
)
2324

2425
// List of features and their default value, protected by fMu
@@ -31,6 +32,7 @@ var features = map[FeatureFlag]bool{
3132
GoogleSafeBrowsingV4: false,
3233
UseAIAIssuerURL: false,
3334
AllowTLS02Challenges: false,
35+
GenerateOCSPEarly: false,
3436
}
3537

3638
var fMu = new(sync.RWMutex)

‎grpc/sa-wrappers.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -364,10 +364,11 @@ func (sac StorageAuthorityClientWrapper) MarkCertificateRevoked(ctx context.Cont
364364
return nil
365365
}
366366

367-
func (sac StorageAuthorityClientWrapper) AddCertificate(ctx context.Context, der []byte, regID int64) (string, error) {
367+
func (sac StorageAuthorityClientWrapper) AddCertificate(ctx context.Context, der []byte, regID int64, ocspResponse []byte) (string, error) {
368368
response, err := sac.inner.AddCertificate(ctx, &sapb.AddCertificateRequest{
369369
Der: der,
370370
RegID: &regID,
371+
Ocsp: ocspResponse,
371372
})
372373
if err != nil {
373374
return "", err
@@ -758,7 +759,7 @@ func (sas StorageAuthorityServerWrapper) AddCertificate(ctx context.Context, req
758759
return nil, errIncompleteRequest
759760
}
760761

761-
digest, err := sas.inner.AddCertificate(ctx, request.Der, *request.RegID)
762+
digest, err := sas.inner.AddCertificate(ctx, request.Der, *request.RegID, request.Ocsp)
762763
if err != nil {
763764
return nil, err
764765
}

0 commit comments

Comments
 (0)
X Tutup