CA: Set IssuerID when integrating orphaned certs (#5083)

There are two code paths which end up calling the SA's AddPrecertificate
RPC: one "normal" path from inside the CA's IssuePrecertificate
method, and one "exceptional" path from inside the orphan-integration
path which only gets executed if the initial attempt to store the cert
failed for some reason.

The first of these paths always sets the IssuerID in the RPC's
AddCertificateRequest. The latter of these paths never does.
Historically, this has been fine, as the SA has stored NULL in the
certificateStatus table when given a nil IssuerID, which matches the
behavior prior to the StoreIssuerInfo flag (when the IssuerID was
always nil, and cert status was tracked by full cert DER instead).

However, with the switch to proto3, the SA now interprets a nil
IssuerID as a 0, and stores that value in the table instead. The
ocsp-updater code which consumes rows of the certificateStatus table
is not able to properly handle IssuerIDs of 0, leading to fun times.

This change ensures that the orphan handling code also sets the
IssuerID when asking the SA to create a new certificateStatus row,
so we should stop producing new rows with a 0 IssuerID.
This commit is contained in:
Aaron Gable 2020-09-09 09:51:31 -07:00 committed by GitHub
parent 74c3139680
commit 86c49278ac
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 15 additions and 8 deletions

View File

@ -617,6 +617,7 @@ func (ca *CertificateAuthorityImpl) IssuePrecertificate(ctx context.Context, iss
RegID: regID,
OCSPResp: ocspResp.Response,
Precert: true,
IssuerID: idForIssuer(issuer.cert),
})
}
return nil, err
@ -710,7 +711,7 @@ func (ca *CertificateAuthorityImpl) IssueCertificateForPrecertificate(ctx contex
ca.log.AuditInfof("Signing success: serial=[%s] names=[%s] csr=[%s] certificate=[%s]",
serialHex, strings.Join(precert.DNSNames, ", "), hex.EncodeToString(req.DER),
hex.EncodeToString(certDER))
err = ca.storeCertificate(ctx, req.RegistrationID, req.OrderID, precert.SerialNumber, certDER)
err = ca.storeCertificate(ctx, req.RegistrationID, req.OrderID, precert.SerialNumber, certDER, idForIssuer(issuer.cert))
if err != nil {
return nil, err
}
@ -895,7 +896,8 @@ func (ca *CertificateAuthorityImpl) storeCertificate(
regID int64,
orderID int64,
serialBigInt *big.Int,
certDER []byte) error {
certDER []byte,
issuerID int64) error {
var err error
now := ca.clk.Now()
_, err = ca.sa.AddCertificate(ctx, certDER, regID, nil, &now)
@ -908,8 +910,9 @@ func (ca *CertificateAuthorityImpl) storeCertificate(
core.SerialToString(serialBigInt), hex.EncodeToString(certDER), err, regID, orderID)
if ca.orphanQueue != nil {
ca.queueOrphan(&orphanedCert{
DER: certDER,
RegID: regID,
DER: certDER,
RegID: regID,
IssuerID: issuerID,
})
}
return err
@ -922,6 +925,7 @@ type orphanedCert struct {
OCSPResp []byte
RegID int64
Precert bool
IssuerID int64
}
func (ca *CertificateAuthorityImpl) queueOrphan(o *orphanedCert) {
@ -974,10 +978,11 @@ func (ca *CertificateAuthorityImpl) integrateOrphan() error {
if orphan.Precert {
issuedNanos := issued.UnixNano()
_, err = ca.sa.AddPrecertificate(context.Background(), &sapb.AddCertificateRequest{
Der: orphan.DER,
RegID: orphan.RegID,
Ocsp: orphan.OCSPResp,
Issued: issuedNanos,
Der: orphan.DER,
RegID: orphan.RegID,
Ocsp: orphan.OCSPResp,
Issued: issuedNanos,
IssuerID: orphan.IssuerID,
})
if err != nil && !berrors.Is(err, berrors.Duplicate) {
return fmt.Errorf("failed to store orphaned precertificate: %s", err)

View File

@ -1131,6 +1131,7 @@ func TestOrphanQueue(t *testing.T) {
1,
tmpl.SerialNumber,
certDER,
1,
)
test.AssertError(t, err, "storeCertificate didn't fail when AddCertificate failed")
@ -1172,6 +1173,7 @@ func TestOrphanQueue(t *testing.T) {
1,
tmpl.SerialNumber,
certDER,
1,
)
test.AssertError(t, err, "storeCertificate didn't fail when AddCertificate failed")
err = orphanQueue.Close()