Skip to content

Commit 6454513

Browse files
jshajsha
authored
Remove StoreIssuerInfo flag in CA (letsencrypt#4850)
As part of that, add support for issuer IDs in orphan-finder's and RA's calls to GenerateOCSP. This factors out the idForIssuer logic from ca/ca.go into a new issuercerts package. orphan-finder refactors: Add a list of issuers in config. Create an orphanFinder struct to hold relevant fields, including the newly added issuers field. Factor out a storeDER function to reduce duplication between the parse-der and parse-ca-log cases. Use test certificates generated specifically for orphan-finder tests. This was necessary because the issuers of these test certificates have to be configured for the orphan finder.
1 parent d003ae8 commit 6454513

File tree

17 files changed

+559
-200
lines changed

17 files changed

+559
-200
lines changed

‎ca/ca.go‎

Lines changed: 49 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77
"crypto/ecdsa"
88
"crypto/rand"
99
"crypto/rsa"
10-
"crypto/sha256"
1110
"crypto/x509"
1211
"encoding/asn1"
1312
"encoding/hex"
@@ -37,8 +36,8 @@ import (
3736
corepb "github.com/letsencrypt/boulder/core/proto"
3837
csrlib "github.com/letsencrypt/boulder/csr"
3938
berrors "github.com/letsencrypt/boulder/errors"
40-
"github.com/letsencrypt/boulder/features"
4139
"github.com/letsencrypt/boulder/goodkey"
40+
"github.com/letsencrypt/boulder/issuercerts"
4241
blog "github.com/letsencrypt/boulder/log"
4342
sapb "github.com/letsencrypt/boulder/sa/proto"
4443
)
@@ -116,11 +115,9 @@ const (
116115
type CertificateAuthorityImpl struct {
117116
rsaProfile string
118117
ecdsaProfile string
119-
// A map from issuer cert common name to an internalIssuer struct
120-
issuers map[string]*internalIssuer
121118
// A map from issuer ID to internalIssuer
122-
idToIssuer map[int64]*internalIssuer
123-
// The common name of the default issuer cert
119+
idToIssuer map[issuercerts.ID]*internalIssuer
120+
// The issuer that will be used for issuance (as opposed to OCSP signing)
124121
defaultIssuer *internalIssuer
125122
sa certificateStorage
126123
pa core.PolicyAuthority
@@ -166,11 +163,12 @@ func makeInternalIssuers(
166163
issuers []Issuer,
167164
policy *cfsslConfig.Signing,
168165
lifespanOCSP time.Duration,
169-
) (map[string]*internalIssuer, error) {
166+
) (map[issuercerts.ID]*internalIssuer, error) {
170167
if len(issuers) == 0 {
171168
return nil, errors.New("No issuers specified.")
172169
}
173-
internalIssuers := make(map[string]*internalIssuer)
170+
internalIssuers := make(map[issuercerts.ID]*internalIssuer)
171+
cns := make(map[string]bool)
174172
for _, iss := range issuers {
175173
if iss.Cert == nil || iss.Signer == nil {
176174
return nil, errors.New("Issuer with nil cert or signer specified.")
@@ -181,10 +179,11 @@ func makeInternalIssuers(
181179
}
182180

183181
cn := iss.Cert.Subject.CommonName
184-
if internalIssuers[cn] != nil {
182+
if cns[cn] {
185183
return nil, errors.New("Multiple issuer certs with the same CommonName are not supported")
186184
}
187-
internalIssuers[cn] = &internalIssuer{
185+
id := issuercerts.FromCert(iss.Cert).ID()
186+
internalIssuers[id] = &internalIssuer{
188187
cert: iss.Cert,
189188
eeSigner: eeSigner,
190189
ocspSigner: iss.Signer,
@@ -193,16 +192,8 @@ func makeInternalIssuers(
193192
return internalIssuers, nil
194193
}
195194

196-
// idForIssuer generates a stable ID for an issuer certificate. This
197-
// is used for identifying which issuer issued a certificate in the
198-
// certificateStatus table.
199-
func idForIssuer(cert *x509.Certificate) int64 {
200-
h := sha256.Sum256(cert.Raw)
201-
return big.NewInt(0).SetBytes(h[:4]).Int64()
202-
}
203-
204195
// NewCertificateAuthorityImpl creates a CA instance that can sign certificates
205-
// from a single issuer (the first first in the issuers slice), and can sign OCSP
196+
// from a single issuer (the first in the issuers slice), and can sign OCSP
206197
// for any of the issuer certificates provided.
207198
func NewCertificateAuthorityImpl(
208199
config ca_config.CAConfig,
@@ -251,7 +242,7 @@ func NewCertificateAuthorityImpl(
251242
if err != nil {
252243
return nil, err
253244
}
254-
defaultIssuer := internalIssuers[issuers[0].Cert.Subject.CommonName]
245+
defaultIssuer := internalIssuers[issuercerts.FromCert(issuers[0].Cert).ID()]
255246

256247
rsaProfile := config.RSAProfile
257248
ecdsaProfile := config.ECDSAProfile
@@ -301,7 +292,6 @@ func NewCertificateAuthorityImpl(
301292
ca = &CertificateAuthorityImpl{
302293
sa: sa,
303294
pa: pa,
304-
issuers: internalIssuers,
305295
defaultIssuer: defaultIssuer,
306296
rsaProfile: rsaProfile,
307297
ecdsaProfile: ecdsaProfile,
@@ -319,9 +309,9 @@ func NewCertificateAuthorityImpl(
319309
signErrorCounter: signErrorCounter,
320310
}
321311

322-
ca.idToIssuer = make(map[int64]*internalIssuer)
323-
for _, ii := range ca.issuers {
324-
id := idForIssuer(ii.cert)
312+
ca.idToIssuer = make(map[issuercerts.ID]*internalIssuer)
313+
for _, ii := range internalIssuers {
314+
id := issuercerts.FromCert(ii.cert).ID()
325315
ca.idToIssuer[id] = ii
326316
}
327317

@@ -433,48 +423,33 @@ var ocspStatusToCode = map[string]int{
433423
func (ca *CertificateAuthorityImpl) GenerateOCSP(ctx context.Context, req *caPB.GenerateOCSPRequest) (*caPB.OCSPResponse, error) {
434424
var issuer *internalIssuer
435425
var serial *big.Int
426+
if req.IssuerID == nil {
427+
return nil, fmt.Errorf("no issuerID provided")
428+
}
429+
if req.Serial == nil {
430+
return nil, fmt.Errorf("no serial provided")
431+
}
436432
// Once the feature is enabled we need to support both RPCs that include
437433
// IssuerID and those that don't as we still need to be able to update rows
438434
// that didn't have an IssuerID set when they were created. Once this feature
439435
// has been enabled for a full OCSP lifetime cycle we can remove this
440436
// functionality.
441-
if features.Enabled(features.StoreIssuerInfo) && req.IssuerID != nil {
442-
serialInt, err := core.StringToSerial(*req.Serial)
443-
if err != nil {
444-
return nil, err
445-
}
446-
serial = serialInt
447-
var ok bool
448-
issuer, ok = ca.idToIssuer[*req.IssuerID]
449-
if !ok {
450-
return nil, fmt.Errorf("This CA doesn't have an issuer cert with ID %d", *req.IssuerID)
451-
}
452-
exists, err := ca.sa.SerialExists(ctx, &sapb.Serial{Serial: req.Serial})
453-
if err != nil {
454-
return nil, err
455-
}
456-
if !*exists.Exists {
457-
return nil, fmt.Errorf("GenerateOCSP was asked to sign OCSP for certification with unknown serial %q", *req.Serial)
458-
}
459-
} else {
460-
cert, err := x509.ParseCertificate(req.CertDER)
461-
if err != nil {
462-
ca.log.AuditErr(err.Error())
463-
return nil, err
464-
}
465-
466-
serial = cert.SerialNumber
467-
cn := cert.Issuer.CommonName
468-
issuer = ca.issuers[cn]
469-
if issuer == nil {
470-
return nil, fmt.Errorf("This CA doesn't have an issuer cert with CommonName %q", cn)
471-
}
472-
err = cert.CheckSignatureFrom(issuer.cert)
473-
if err != nil {
474-
return nil, fmt.Errorf("GenerateOCSP was asked to sign OCSP for cert "+
475-
"%s from %q, but the cert's signature was not valid: %s.",
476-
core.SerialToString(cert.SerialNumber), cn, err)
477-
}
437+
serialInt, err := core.StringToSerial(*req.Serial)
438+
if err != nil {
439+
return nil, err
440+
}
441+
serial = serialInt
442+
var ok bool
443+
issuer, ok = ca.idToIssuer[issuercerts.ID(*req.IssuerID)]
444+
if !ok {
445+
return nil, fmt.Errorf("This CA doesn't have an issuer cert with ID %d", *req.IssuerID)
446+
}
447+
exists, err := ca.sa.SerialExists(ctx, &sapb.Serial{Serial: req.Serial})
448+
if err != nil {
449+
return nil, err
450+
}
451+
if !*exists.Exists {
452+
return nil, fmt.Errorf("GenerateOCSP was asked to sign OCSP for certification with unknown serial %q", *req.Serial)
478453
}
479454

480455
now := ca.clk.Now().Truncate(time.Hour)
@@ -523,10 +498,16 @@ func (ca *CertificateAuthorityImpl) IssuePrecertificate(ctx context.Context, iss
523498
return nil, err
524499
}
525500

501+
// we currently only use one issuer, in the future when we support multiple
502+
// the issuer will need to be derived from issueReq
503+
issuerID := int64(issuercerts.FromCert(ca.defaultIssuer.cert).ID())
504+
526505
status := string(core.OCSPStatusGood)
527506
ocspResp, err := ca.GenerateOCSP(ctx, &caPB.GenerateOCSPRequest{
528-
CertDER: precertDER,
529-
Status: &status,
507+
Serial: &serialHex,
508+
CertDER: precertDER,
509+
Status: &status,
510+
IssuerID: &issuerID,
530511
})
531512
if err != nil {
532513
err = berrors.InternalServerError(err.Error())
@@ -535,17 +516,13 @@ func (ca *CertificateAuthorityImpl) IssuePrecertificate(ctx context.Context, iss
535516
}
536517

537518
req := &sapb.AddCertificateRequest{
538-
Der: precertDER,
539-
RegID: &regID,
540-
Ocsp: ocspResp.Response,
541-
Issued: &nowNanos,
519+
Der: precertDER,
520+
RegID: &regID,
521+
Ocsp: ocspResp.Response,
522+
Issued: &nowNanos,
523+
IssuerID: &issuerID,
542524
}
543525

544-
// we currently only use one issuer, in the future when we support multiple
545-
// the issuer will need to be derived from issueReq
546-
issuerID := idForIssuer(ca.defaultIssuer.cert)
547-
req.IssuerID = &issuerID
548-
549526
_, err = ca.sa.AddPrecertificate(ctx, req)
550527
if err != nil {
551528
ca.orphanCount.With(prometheus.Labels{"type": "precert"}).Inc()

‎ca/ca_test.go‎

Lines changed: 24 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ import (
3939
berrors "github.com/letsencrypt/boulder/errors"
4040
"github.com/letsencrypt/boulder/features"
4141
"github.com/letsencrypt/boulder/goodkey"
42+
"github.com/letsencrypt/boulder/issuercerts"
4243
blog "github.com/letsencrypt/boulder/log"
4344
"github.com/letsencrypt/boulder/metrics"
4445
"github.com/letsencrypt/boulder/policy"
@@ -492,10 +493,16 @@ func TestOCSP(t *testing.T) {
492493
test.AssertNotError(t, err, "Failed to issue")
493494
parsedCert, err := x509.ParseCertificate(cert.DER)
494495
test.AssertNotError(t, err, "Failed to parse cert")
496+
497+
caCertIssuerID := int64(issuercerts.FromCert(caCert).ID())
498+
serial := core.SerialToString(parsedCert.SerialNumber)
495499
status := string(core.OCSPStatusGood)
500+
496501
ocspResp, err := ca.GenerateOCSP(ctx, &caPB.GenerateOCSPRequest{
497-
CertDER: cert.DER,
498-
Status: &status,
502+
IssuerID: &caCertIssuerID,
503+
Serial: &serial,
504+
CertDER: cert.DER,
505+
Status: &status,
499506
})
500507
test.AssertNotError(t, err, "Failed to generate OCSP")
501508
parsed, err := ocsp.ParseResponse(ocspResp.Response, caCert)
@@ -504,13 +511,6 @@ func TestOCSP(t *testing.T) {
504511
test.AssertEquals(t, parsed.RevocationReason, 0)
505512
test.AssertEquals(t, parsed.SerialNumber.Cmp(parsedCert.SerialNumber), 0)
506513

507-
// Test that signatures are checked.
508-
_, err = ca.GenerateOCSP(ctx, &caPB.GenerateOCSPRequest{
509-
CertDER: append(cert.DER, byte(0)),
510-
Status: &status,
511-
})
512-
test.AssertError(t, err, "Generated OCSP for cert with bad signature")
513-
514514
// Load multiple issuers, including the old issuer, and ensure OCSP is still
515515
// signed correctly.
516516
newIssuerCert, err := core.LoadCert("../test/test-ca2.pem")
@@ -543,14 +543,16 @@ func TestOCSP(t *testing.T) {
543543
parsedNewCert, err := x509.ParseCertificate(newCert.DER)
544544
test.AssertNotError(t, err, "Failed to parse newCert")
545545

546-
err = parsedNewCert.CheckSignatureFrom(newIssuerCert)
547-
t.Logf("check sig: %s", err)
546+
newIssuerID := int64(issuercerts.FromCert(newIssuers[0].Cert).ID())
547+
newSerial := core.SerialToString(parsedNewCert.SerialNumber)
548548

549549
// ocspResp2 is a second OCSP response for `cert` (issued by caCert), and
550550
// should be signed by caCert.
551551
ocspResp2, err := ca.GenerateOCSP(ctx, &caPB.GenerateOCSPRequest{
552-
CertDER: append([]byte(nil), cert.DER...),
553-
Status: &status,
552+
IssuerID: &newIssuerID,
553+
Serial: &newSerial,
554+
CertDER: append([]byte(nil), cert.DER...),
555+
Status: &status,
554556
})
555557
test.AssertNotError(t, err, "Failed to sign second OCSP response")
556558
_, err = ocsp.ParseResponse(ocspResp2.Response, caCert)
@@ -559,8 +561,10 @@ func TestOCSP(t *testing.T) {
559561
// newCertOcspResp is an OCSP response for `newCert` (issued by newIssuer),
560562
// and should be signed by newIssuer.
561563
newCertOcspResp, err := ca.GenerateOCSP(ctx, &caPB.GenerateOCSPRequest{
562-
CertDER: newCert.DER,
563-
Status: &status,
564+
IssuerID: &newIssuerID,
565+
Serial: &newSerial,
566+
CertDER: newCert.DER,
567+
Status: &status,
564568
})
565569
test.AssertNotError(t, err, "Failed to generate OCSP")
566570
parsedNewCertOcspResp, err := ocsp.ParseResponse(newCertOcspResp.Response, newIssuerCert)
@@ -1253,7 +1257,6 @@ func TestIssuePrecertificateLinting(t *testing.T) {
12531257
func TestGenerateOCSPWithIssuerID(t *testing.T) {
12541258
testCtx := setup(t)
12551259
sa := &mockSA{}
1256-
_ = features.Set(map[string]bool{"StoreIssuerInfo": true})
12571260
ca, err := NewCertificateAuthorityImpl(
12581261
testCtx.caConfig,
12591262
sa,
@@ -1266,7 +1269,7 @@ func TestGenerateOCSPWithIssuerID(t *testing.T) {
12661269
nil)
12671270
test.AssertNotError(t, err, "Failed to create CA")
12681271

1269-
// GenerateOCSP with feature enabled + req contains bad IssuerID
1272+
// req contains bad IssuerID
12701273
issuerID := int64(666)
12711274
serial := "DEADDEADDEADDEADDEADDEADDEADDEADDEAD"
12721275
status := string(core.OCSPStatusGood)
@@ -1277,22 +1280,22 @@ func TestGenerateOCSPWithIssuerID(t *testing.T) {
12771280
})
12781281
test.AssertError(t, err, "GenerateOCSP didn't fail with invalid IssuerID")
12791282

1280-
// GenerateOCSP with feature enabled + req contains good IssuerID
1281-
issuerID = idForIssuer(ca.defaultIssuer.cert)
1283+
// req contains good IssuerID
1284+
issuerID = int64(issuercerts.FromCert(ca.defaultIssuer.cert).ID())
12821285
_, err = ca.GenerateOCSP(context.Background(), &caPB.GenerateOCSPRequest{
12831286
IssuerID: &issuerID,
12841287
Serial: &serial,
12851288
Status: &status,
12861289
})
12871290
test.AssertNotError(t, err, "GenerateOCSP failed")
12881291

1289-
// GenerateOCSP with feature enabled + req doesn't contain IssuerID
1292+
// req doesn't contain IssuerID or Serial
12901293
issueReq := caPB.IssueCertificateRequest{Csr: CNandSANCSR, RegistrationID: &arbitraryRegID}
12911294
cert, err := ca.IssuePrecertificate(ctx, &issueReq)
12921295
test.AssertNotError(t, err, "Failed to issue")
12931296
_, err = ca.GenerateOCSP(context.Background(), &caPB.GenerateOCSPRequest{
12941297
CertDER: cert.DER,
12951298
Status: &status,
12961299
})
1297-
test.AssertNotError(t, err, "GenerateOCSP failed")
1300+
test.AssertError(t, err, "Expected error from GenerateOCSP")
12981301
}

0 commit comments

Comments
 (0)