SA: Ensure that IssuerID is set when adding precertificates (#5099)

This change adds `req.IssuerID` to the set of fields that the SA's
`AddPrecertificate` method requires be non-zero.

As a result, this also updates many tests, both unit and integration,
to ensure that they supply a value (usually just 1) for that field. The
most complex part of the test changes is a slight refactoring to the
orphan-finder code, which makes it easier to reason about the
separation between log line parsing and building and sending the
request.

Based on #5096
Fixes #5097

ca/ca.go

@@ -576,12 +576,14 @@ func (ca *CertificateAuthorityImpl) IssuePrecertificate(ctx context.Context, iss
 		return nil, err
 	}
 
+	issuerID := idForCert(issuer.cert)
+
 	req := &sapb.AddCertificateRequest{
 		Der:      precertDER,
 		RegID:    regID,
 		Ocsp:     ocspResp.Response,
 		Issued:   nowNanos,
-		IssuerID: idForCert(issuer.cert),
+		IssuerID: issuerID,
 	}
 
 	_, err = ca.sa.AddPrecertificate(ctx, req)
@@ -590,15 +592,15 @@ func (ca *CertificateAuthorityImpl) IssuePrecertificate(ctx context.Context, iss
 		err = berrors.InternalServerError(err.Error())
 		// Note: This log line is parsed by cmd/orphan-finder. If you make any
 		// changes here, you should make sure they are reflected in orphan-finder.
-		ca.log.AuditErrf("Failed RPC to store at SA, orphaning precertificate: serial=[%s] cert=[%s] err=[%v], regID=[%d], orderID=[%d]",
+		ca.log.AuditErrf("Failed RPC to store at SA, orphaning precertificate: serial=[%s], cert=[%s], issuerID=[%d], regID=[%d], orderID=[%d], err=[%v]",
-			serialHex, hex.EncodeToString(precertDER), err, issueReq.RegistrationID, issueReq.OrderID)
+			serialHex, hex.EncodeToString(precertDER), issuerID, issueReq.RegistrationID, issueReq.OrderID, err)
 		if ca.orphanQueue != nil {
 			ca.queueOrphan(&orphanedCert{
 				DER:      precertDER,
 				RegID:    regID,
 				OCSPResp: ocspResp.Response,
 				Precert:  true,
-				IssuerID: idForCert(issuer.cert),
+				IssuerID: issuerID,
 			})
 		}
 		return nil, err

cmd/admin-revoker/main_test.go

@@ -73,9 +73,10 @@ func TestRevokeBatch(t *testing.T) {
 		der, err := x509.CreateCertificate(rand.Reader, template, template, &k.PublicKey, k)
 		test.AssertNotError(t, err, "failed to generate test cert")
 		_, err = ssa.AddPrecertificate(context.Background(), &sapb.AddCertificateRequest{
-			Der:    der,
+			Der:      der,
-			RegID:  reg.ID,
+			RegID:    reg.ID,
-			Issued: time.Now().UnixNano(),
+			Issued:   time.Now().UnixNano(),
+			IssuerID: 1,
 		})
 		test.AssertNotError(t, err, "failed to add test cert")
 		now := time.Now()

cmd/ocsp-updater/main_test.go

@@ -93,10 +93,11 @@ func TestGenerateAndStoreOCSPResponse(t *testing.T) {
 	parsedCert, err := core.LoadCert("test-cert.pem")
 	test.AssertNotError(t, err, "Couldn't read test certificate")
 	_, err = sa.AddPrecertificate(ctx, &sapb.AddCertificateRequest{
-		Der:    parsedCert.Raw,
+		Der:      parsedCert.Raw,
-		RegID:  reg.ID,
+		RegID:    reg.ID,
-		Ocsp:   nil,
+		Ocsp:     nil,
-		Issued: nowNano(fc),
+		Issued:   nowNano(fc),
+		IssuerID: 1,
 	})
 	test.AssertNotError(t, err, "Couldn't add test-cert.pem")
 
@@ -117,19 +118,21 @@ func TestGenerateOCSPResponses(t *testing.T) {
 	parsedCertA, err := core.LoadCert("test-cert.pem")
 	test.AssertNotError(t, err, "Couldn't read test certificate")
 	_, err = sa.AddPrecertificate(ctx, &sapb.AddCertificateRequest{
-		Der:    parsedCertA.Raw,
+		Der:      parsedCertA.Raw,
-		RegID:  reg.ID,
+		RegID:    reg.ID,
-		Ocsp:   nil,
+		Ocsp:     nil,
-		Issued: nowNano(fc),
+		Issued:   nowNano(fc),
+		IssuerID: 1,
 	})
 	test.AssertNotError(t, err, "Couldn't add test-cert.pem")
 	parsedCertB, err := core.LoadCert("test-cert-b.pem")
 	test.AssertNotError(t, err, "Couldn't read test certificate")
 	_, err = sa.AddPrecertificate(ctx, &sapb.AddCertificateRequest{
-		Der:    parsedCertB.Raw,
+		Der:      parsedCertB.Raw,
-		RegID:  reg.ID,
+		RegID:    reg.ID,
-		Ocsp:   nil,
+		Ocsp:     nil,
-		Issued: nowNano(fc),
+		Issued:   nowNano(fc),
+		IssuerID: 1,
 	})
 	test.AssertNotError(t, err, "Couldn't add test-cert-b.pem")
 
@@ -173,10 +176,11 @@ func TestFindStaleOCSPResponses(t *testing.T) {
 	parsedCert, err := core.LoadCert("test-cert.pem")
 	test.AssertNotError(t, err, "Couldn't read test certificate")
 	_, err = sa.AddPrecertificate(ctx, &sapb.AddCertificateRequest{
-		Der:    parsedCert.Raw,
+		Der:      parsedCert.Raw,
-		RegID:  reg.ID,
+		RegID:    reg.ID,
-		Ocsp:   nil,
+		Ocsp:     nil,
-		Issued: nowNano(fc),
+		Issued:   nowNano(fc),
+		IssuerID: 1,
 	})
 	test.AssertNotError(t, err, "Couldn't add test-cert.pem")
 
@@ -214,10 +218,11 @@ func TestFindStaleOCSPResponsesRevokedReason(t *testing.T) {
 	parsedCert, err := core.LoadCert("test-cert.pem")
 	test.AssertNotError(t, err, "Couldn't read test certificate")
 	_, err = sa.AddPrecertificate(ctx, &sapb.AddCertificateRequest{
-		Der:    parsedCert.Raw,
+		Der:      parsedCert.Raw,
-		RegID:  reg.ID,
+		RegID:    reg.ID,
-		Ocsp:   nil,
+		Ocsp:     nil,
-		Issued: nowNano(fc),
+		Issued:   nowNano(fc),
+		IssuerID: 1,
 	})
 	test.AssertNotError(t, err, "Couldn't add test-cert.pem")
 
@@ -246,10 +251,11 @@ func TestOldOCSPResponsesTick(t *testing.T) {
 	parsedCert, err := core.LoadCert("test-cert.pem")
 	test.AssertNotError(t, err, "Couldn't read test certificate")
 	_, err = sa.AddPrecertificate(ctx, &sapb.AddCertificateRequest{
-		Der:    parsedCert.Raw,
+		Der:      parsedCert.Raw,
-		RegID:  reg.ID,
+		RegID:    reg.ID,
-		Ocsp:   nil,
+		Ocsp:     nil,
-		Issued: nowNano(fc),
+		Issued:   nowNano(fc),
+		IssuerID: 1,
 	})
 	test.AssertNotError(t, err, "Couldn't add test-cert.pem")
 
@@ -277,10 +283,11 @@ func TestOldOCSPResponsesTickIsExpired(t *testing.T) {
 
 	// Add a new test certificate
 	_, err = sa.AddPrecertificate(ctx, &sapb.AddCertificateRequest{
-		Der:    parsedCert.Raw,
+		Der:      parsedCert.Raw,
-		RegID:  reg.ID,
+		RegID:    reg.ID,
-		Ocsp:   nil,
+		Ocsp:     nil,
-		Issued: nowNano(fc),
+		Issued:   nowNano(fc),
+		IssuerID: 1,
 	})
 	test.AssertNotError(t, err, "Couldn't add test-cert.pem")
 
@@ -326,10 +333,11 @@ func TestStoreResponseGuard(t *testing.T) {
 	parsedCert, err := core.LoadCert("test-cert.pem")
 	test.AssertNotError(t, err, "Couldn't read test certificate")
 	_, err = sa.AddPrecertificate(ctx, &sapb.AddCertificateRequest{
-		Der:    parsedCert.Raw,
+		Der:      parsedCert.Raw,
-		RegID:  reg.ID,
+		RegID:    reg.ID,
-		Ocsp:   nil,
+		Ocsp:     nil,
-		Issued: nowNano(fc),
+		Issued:   nowNano(fc),
+		IssuerID: 1,
 	})
 	test.AssertNotError(t, err, "Couldn't add test-cert.pem")
 
@@ -383,10 +391,11 @@ func TestGenerateOCSPResponsePrecert(t *testing.T) {
 	regID := reg.ID
 	issuedTime := fc.Now().UnixNano()
 	_, err := sa.AddPrecertificate(ctx, &sapb.AddCertificateRequest{
-		Der:    testCert.Raw,
+		Der:      testCert.Raw,
-		RegID:  regID,
+		RegID:    regID,
-		Ocsp:   ocspResp,
+		Ocsp:     ocspResp,
-		Issued: issuedTime,
+		Issued:   issuedTime,
+		IssuerID: 1,
 	})
 	test.AssertNotError(t, err, "Couldn't add test-cert2.der")
 
@@ -433,9 +442,6 @@ func TestIssuerInfo(t *testing.T) {
 	}
 	certA, err := x509.CreateCertificate(rand.Reader, template, template, &k.PublicKey, k)
 	test.AssertNotError(t, err, "x509.CreateCertificate failed")
-	template.SerialNumber = big.NewInt(2)
-	certB, err := x509.CreateCertificate(rand.Reader, template, template, &k.PublicKey, k)
-	test.AssertNotError(t, err, "x509.CreateCertificate failed")
 
 	now := fc.Now().UnixNano()
 	id := int64(1234)
@@ -447,27 +453,16 @@ func TestIssuerInfo(t *testing.T) {
 		IssuerID: id,
 	})
 	test.AssertNotError(t, err, "sa.AddPrecertificate failed")
-	_, err = sa.AddPrecertificate(context.Background(), &sapb.AddCertificateRequest{
-		Der:    certB,
-		RegID:  reg.ID,
-		Ocsp:   []byte{1, 2, 3},
-		Issued: now,
-	})
-	test.AssertNotError(t, err, "sa.AddPrecertificate failed")
 
 	fc.Add(time.Hour * 24 * 4)
 	statuses, err := updater.findStaleOCSPResponses(fc.Now().Add(-time.Hour), 10)
 	test.AssertNotError(t, err, "findStaleOCSPResponses failed")
-	test.AssertEquals(t, len(statuses), 2)
+	test.AssertEquals(t, len(statuses), 1)
 	test.AssertEquals(t, *statuses[0].IssuerID, id)
-	test.Assert(t, *statuses[1].IssuerID == 0, "second status doesn't have zero IssuerID")
 
 	_, err = updater.generateResponse(context.Background(), statuses[0])
 	test.AssertNotError(t, err, "generateResponse failed")
 	test.Assert(t, m.gotIssuer, "generateResponse didn't send issuer information and serial")
-	_, err = updater.generateResponse(context.Background(), statuses[1])
-	test.AssertNotError(t, err, "generateResponse failed")
-	test.Assert(t, !m.gotIssuer, "generateResponse did send issuer information and serial when it shouldn't")
 }
 
 type brokenDB struct{}

cmd/orphan-finder/main.go

@@ -91,9 +91,21 @@ func (t orphanType) String() string {
 	}
 }
 
+// An orphaned cert log line must contain at least the following tokens:
+// "orphaning", "(pre)?certificate", "cert=[\w+]", "issuerID=[\d+]", and "regID=[\d]".
+// For example:
+// `[AUDIT] Failed RPC to store at SA, orphaning precertificate: serial=[04asdf1234], cert=[MIIdeafbeef], issuerID=[112358], regID=[1001], orderID=[1002], err=[Timed out]`
+// The orphan-finder does not care about the serial, error, or orderID.
+type parsedLine struct {
+	certDER  []byte
+	issuerID int64
+	regID    int64
+}
+
 var (
 	derOrphan        = regexp.MustCompile(`cert=\[([0-9a-f]+)\]`)
 	regOrphan        = regexp.MustCompile(`regID=\[(\d+)\]`)
+	issuerOrphan     = regexp.MustCompile(`issuerID=\[(\d+)\]`)
 	errAlreadyExists = fmt.Errorf("Certificate already exists in DB")
 )
 
@@ -147,6 +159,41 @@ func checkDER(sai certificateStorage, der []byte) (*x509.Certificate, orphanType
 	return nil, orphanTyp, fmt.Errorf("Existing %s lookup failed: %s", orphanTyp, err)
 }
 
+func parseLogLine(line string, logger blog.Logger) (parsedLine, error) {
+	derStr := derOrphan.FindStringSubmatch(line)
+	if len(derStr) <= 1 {
+		return parsedLine{}, fmt.Errorf("unable to find cert der: %s", line)
+	}
+	der, err := hex.DecodeString(derStr[1])
+	if err != nil {
+		return parsedLine{}, fmt.Errorf("unable to decode hex der from [%s]: %s", line, err)
+	}
+
+	regStr := regOrphan.FindStringSubmatch(line)
+	if len(regStr) <= 1 {
+		return parsedLine{}, fmt.Errorf("unable to find regID: %s", line)
+	}
+	regID, err := strconv.ParseInt(regStr[1], 10, 64)
+	if err != nil {
+		return parsedLine{}, fmt.Errorf("unable to parse regID from [%s]: %s", line, err)
+	}
+
+	issuerStr := issuerOrphan.FindStringSubmatch(line)
+	if len(issuerStr) <= 1 {
+		return parsedLine{}, fmt.Errorf("unable to find issuerID: %s", line)
+	}
+	issuerID, err := strconv.ParseInt(issuerStr[1], 10, 64)
+	if err != nil {
+		return parsedLine{}, fmt.Errorf("unable to parse issuerID from [%s]: %s", line, err)
+	}
+
+	return parsedLine{
+		certDER:  der,
+		regID:    regID,
+		issuerID: issuerID,
+	}, nil
+}
+
 // storeParsedLogLine attempts to parse one log line according to the format used when
 // orphaning certificates and precertificates. It returns two booleans and the
 // orphanType: The first boolean is true if the line was a match, and the second
@@ -154,33 +201,23 @@ func checkDER(sai certificateStorage, der []byte) (*x509.Certificate, orphanType
 // orphan to the DB, it requests a fresh OCSP response from the CA to store
 // alongside the precertificate/certificate.
 func storeParsedLogLine(sa certificateStorage, ca ocspGenerator, logger blog.Logger, line string) (found bool, added bool, typ orphanType) {
-	ctx := context.Background()
+	// At a minimum, the log line should contain the word "orphaning" and the token
+	// "cert=". If it doesn't have those, short-circuit.
+	if (!strings.Contains(line, fmt.Sprintf("orphaning %s", certOrphan)) &&
+		!strings.Contains(line, fmt.Sprintf("orphaning %s", precertOrphan))) ||
+		!strings.Contains(line, "cert=") {
+		return false, false, unknownOrphan
+	}
 
-	// The log line should contain a label indicating it is a cert or a precert
+	parsed, err := parseLogLine(line, logger)
-	// orphan. We will determine which it is in checkDER based on the DER instead
-	// of the log line label.
-	if !strings.Contains(line, fmt.Sprintf("orphaning %s", certOrphan)) &&
-		!strings.Contains(line, fmt.Sprintf("orphaning %s", precertOrphan)) {
-		return false, false, unknownOrphan
-	}
-	// The log line should also contain certificate DER
-	if !strings.Contains(line, "cert=") {
-		return false, false, unknownOrphan
-	}
-	// Extract and decode the orphan DER
-	derStr := derOrphan.FindStringSubmatch(line)
-	if len(derStr) <= 1 {
-		logger.AuditErrf("Didn't match regex for cert: %s", line)
-		return true, false, unknownOrphan
-	}
-	der, err := hex.DecodeString(derStr[1])
 	if err != nil {
-		logger.AuditErrf("Couldn't decode hex: %s, [%s]", err, line)
+		logger.AuditErr(fmt.Sprintf("Couldn't parse log line: %s", err))
 		return true, false, unknownOrphan
 	}
+
 	// Parse the DER, determine the orphan type, and ensure it doesn't already
 	// exist in the DB
-	cert, typ, err := checkDER(sa, der)
+	cert, typ, err := checkDER(sa, parsed.certDER)
 	if err != nil {
 		logFunc := logger.Errf
 		if err == errAlreadyExists {
@@ -189,22 +226,15 @@ func storeParsedLogLine(sa certificateStorage, ca ocspGenerator, logger blog.Log
 		logFunc("%s, [%s]", err, line)
 		return true, false, typ
 	}
-	// extract the regID
+
-	regStr := regOrphan.FindStringSubmatch(line)
+	// generate an OCSP response
-	if len(regStr) <= 1 {
+	ctx := context.Background()
-		logger.AuditErrf("regID variable is empty, [%s]", line)
+	response, err := generateOCSP(ctx, ca, parsed.certDER)
-		return true, false, typ
-	}
-	regID, err := strconv.ParseInt(regStr[1], 10, 64)
-	if err != nil {
-		logger.AuditErrf("Couldn't parse regID: %s, [%s]", err, line)
-		return true, false, typ
-	}
-	response, err := generateOCSP(ctx, ca, der)
 	if err != nil {
 		logger.AuditErrf("Couldn't generate OCSP: %s, [%s]", err, line)
 		return true, false, typ
 	}
+
 	// We use `cert.NotBefore` as the issued date to avoid the SA tagging this
 	// certificate with an issued date of the current time when we know it was an
 	// orphan issued in the past. Because certificates are backdated we need to
@@ -212,13 +242,14 @@ func storeParsedLogLine(sa certificateStorage, ca ocspGenerator, logger blog.Log
 	issuedDate := cert.NotBefore.Add(backdateDuration)
 	switch typ {
 	case certOrphan:
-		_, err = sa.AddCertificate(ctx, der, regID, response, &issuedDate)
+		_, err = sa.AddCertificate(ctx, parsed.certDER, parsed.regID, response, &issuedDate)
 	case precertOrphan:
 		_, err = sa.AddPrecertificate(ctx, &sapb.AddCertificateRequest{
-			Der:    der,
+			Der:      parsed.certDER,
-			RegID:  regID,
+			RegID:    parsed.regID,
-			Ocsp:   response,
+			Ocsp:     response,
-			Issued: issuedDate.UnixNano(),
+			Issued:   issuedDate.UnixNano(),
+			IssuerID: parsed.issuerID,
 		})
 	default:
 		// Shouldn't happen but be defensive anyway

cmd/orphan-finder/main_test.go

@@ -61,6 +61,9 @@ func (m *mockSA) GetCertificate(ctx context.Context, s string) (core.Certificate
 }
 
 func (m *mockSA) AddPrecertificate(ctx context.Context, req *sapb.AddCertificateRequest) (*corepb.Empty, error) {
+	if core.IsAnyNilOrZero(req.Der, req.Issued, req.RegID, req.IssuerID) {
+		return nil, berrors.InternalServerError("Incomplete request")
+	}
 	parsed, err := x509.ParseCertificate(req.Der)
 	if err != nil {
 		return nil, err
@@ -122,12 +125,12 @@ func TestParseLine(t *testing.T) {
 
 	testPreCertDER := "308204553082033da003020102021203e1dea6f3349009a90e0306dbb39c3e7ca2300d06092a864886f70d01010b0500304a310b300906035504061302555331163014060355040a130d4c6574277320456e6372797074312330210603550403131a4c6574277320456e637279707420417574686f72697479205833301e170d3139313031363132353431375a170d3230303131343132353431375a30133111300f060355040313086a756e74732e696f30820122300d06092a864886f70d01010105000382010f003082010a0282010100c91926403839aadbf2a73af4f85e3884df553880c7e9d11943121b941f284a2c805b6329a93d7fb2357c1298d811cfce61faa863c334149f948ff52a55a516e56b2d31d137b1d0319f2aabdea0e9d5e8630b54d7e53597e094c323e24a7ec1ab0db5d85651a641ec3fd7841fe5cbc675315c49b714238ead757e55409fd68c4b48d42f14c2124d381800fd2ec417ed7f363b00ab23aaddaf9113d5cf889bbf391431bffb91d425d11a1e79318b7007b8e75cc56633662c3d6c58175b5cab6225aa495361b1124642f19584820d215f23f46bd9fafa3341a0f7f387bf7cdecbccd7fcbcb3e917becb41562771e579884a0d8a1b170536f82ba90b398e9a6932150203010001a382016a30820166300e0603551d0f0101ff0404030205a0301d0603551d250416301406082b0601050507030106082b06010505070302300c0603551d130101ff04023000301d0603551d0e041604144d14d73117ca7f5a27394ed590b0d037eb5888a2301f0603551d23041830168014a84a6a63047dddbae6d139b7a64565eff3a8eca1306f06082b0601050507010104633061302e06082b060105050730018622687474703a2f2f6f6373702e696e742d78332e6c657473656e63727970742e6f7267302f06082b060105050730028623687474703a2f2f636572742e696e742d78332e6c657473656e63727970742e6f72672f30130603551d11040c300a82086a756e74732e696f304c0603551d20044530433008060667810c0102013037060b2b0601040182df130101013028302606082b06010505070201161a687474703a2f2f6370732e6c657473656e63727970742e6f72673013060a2b06010401d6790204030101ff04020500300d06092a864886f70d01010b0500038201010035f9d6620874966f2aa400f069c5f601dc11083f5859a15d20e9b1d2f9d87d3756a71a03cee0ab2a69b5173a4395b698163ba60394167c9eb4b66d20d9b3a76bf94995288e8d15c70bee969f77a71147718803e73df0a7832c1fcae1e3138601ebc61725bc7505c6d1e5b0eaf7797e09161d71e37d76370dc489312b1bf0600d1c952f846edb810c284c0d831f27481a8f2220ad178c87d8c4688023fa3798293dc9fdffa9e5b885a8107d8a2480226cd5f9121d6d7ea83b10292371ad6757e7729b27136a064f2901822b4f0ea52f8149a17860e37d3dc925488b1ba4aa26ef51e60de024e67e3d5e04ac97d8bd79a003e668ea2e1bd1c0b9d77c7cf7bfdc32"
 
-	logLine := func(typ orphanType, der, regID, orderID string) string {
+	logLine := func(typ orphanType, der, issuerID, regID, orderID string) string {
 		return fmt.Sprintf(
 			"0000-00-00T00:00:00+00:00 hostname boulder-ca[pid]: "+
 				"[AUDIT] Failed RPC to store at SA, orphaning %s: "+
-				"cert=[%s] err=[context deadline exceeded], regID=[%s], orderID=[%s]",
+				"serial=[unused], cert=[%s], issuerID=[%s], regID=[%s], orderID=[%s], err=[context deadline exceeded]",
-			typ, der, regID, orderID)
+			typ, der, issuerID, regID, orderID)
 	}
 
 	testCases := []struct {
@@ -148,21 +151,21 @@ func TestParseLine(t *testing.T) {
 		},
 		{
 			Name:           "Empty cert in line",
-			LogLine:        logLine(certOrphan, "", "1337", "0"),
+			LogLine:        logLine(certOrphan, "", "1", "1337", "0"),
 			ExpectFound:    true,
 			ExpectAdded:    false,
 			ExpectNoErrors: false,
 		},
 		{
 			Name:           "Invalid cert in line",
-			LogLine:        logLine(certOrphan, "deadbeef", "", ""),
+			LogLine:        logLine(certOrphan, "deadbeef", "", "", ""),
 			ExpectFound:    true,
 			ExpectAdded:    false,
 			ExpectNoErrors: false,
 		},
 		{
 			Name:           "Valid cert in line",
-			LogLine:        logLine(certOrphan, testCertDER, "1001", "0"),
+			LogLine:        logLine(certOrphan, testCertDER, "1", "1001", "0"),
 			ExpectFound:    true,
 			ExpectAdded:    true,
 			ExpectAddedDER: testCertDER,
@@ -171,7 +174,7 @@ func TestParseLine(t *testing.T) {
 		},
 		{
 			Name:        "Already inserted cert in line",
-			LogLine:     logLine(certOrphan, testCertDER, "1001", "0"),
+			LogLine:     logLine(certOrphan, testCertDER, "1", "1001", "0"),
 			ExpectFound: true,
 			// ExpectAdded is false because we have already added this cert in the
 			// previous "Valid cert in line" test case.
@@ -180,21 +183,21 @@ func TestParseLine(t *testing.T) {
 		},
 		{
 			Name:           "Empty precert in line",
-			LogLine:        logLine(precertOrphan, "", "1337", "0"),
+			LogLine:        logLine(precertOrphan, "", "1", "1337", "0"),
 			ExpectFound:    true,
 			ExpectAdded:    false,
 			ExpectNoErrors: false,
 		},
 		{
 			Name:           "Invalid precert in line",
-			LogLine:        logLine(precertOrphan, "deadbeef", "", ""),
+			LogLine:        logLine(precertOrphan, "deadbeef", "", "", ""),
 			ExpectFound:    true,
 			ExpectAdded:    false,
 			ExpectNoErrors: false,
 		},
 		{
 			Name:           "Valid precert in line",
-			LogLine:        logLine(precertOrphan, testPreCertDER, "9999", "0"),
+			LogLine:        logLine(precertOrphan, testPreCertDER, "1", "9999", "0"),
 			ExpectFound:    true,
 			ExpectAdded:    true,
 			ExpectAddedDER: testPreCertDER,
@@ -203,7 +206,7 @@ func TestParseLine(t *testing.T) {
 		},
 		{
 			Name:        "Already inserted precert in line",
-			LogLine:     logLine(precertOrphan, testPreCertDER, "1001", "0"),
+			LogLine:     logLine(precertOrphan, testPreCertDER, "1", "1001", "0"),
 			ExpectFound: true,
 			// ExpectAdded is false because we have already added this cert in the
 			// previous "Valid cert in line" test case.
@@ -212,11 +215,25 @@ func TestParseLine(t *testing.T) {
 		},
 		{
 			Name:           "Unknown orphan type",
-			LogLine:        logLine(unknownOrphan, testPreCertDER, "1001", "0"),
+			LogLine:        logLine(unknownOrphan, testPreCertDER, "1", "1001", "0"),
 			ExpectFound:    false,
 			ExpectAdded:    false,
 			ExpectNoErrors: false,
 		},
+		{
+			Name:           "Empty issuerID in line",
+			LogLine:        logLine(precertOrphan, testPreCertDER, "", "1001", "0"),
+			ExpectFound:    true,
+			ExpectAdded:    false,
+			ExpectNoErrors: false,
+		},
+		{
+			Name:           "Zero issuerID in line",
+			LogLine:        logLine(precertOrphan, testPreCertDER, "0", "1001", "0"),
+			ExpectFound:    true,
+			ExpectAdded:    false,
+			ExpectNoErrors: false,
+		},
 	}
 
 	for _, tc := range testCases {

sa/precertificates.go

@@ -37,7 +37,7 @@ func (ssa *SQLStorageAuthority) AddSerial(ctx context.Context, req *sapb.AddSeri
 
 // AddPrecertificate writes a record of a precertificate generation to the DB.
 func (ssa *SQLStorageAuthority) AddPrecertificate(ctx context.Context, req *sapb.AddCertificateRequest) (*corepb.Empty, error) {
-	if core.IsAnyNilOrZero(req.Der, req.Issued, req.RegID) {
+	if core.IsAnyNilOrZero(req.Der, req.Issued, req.RegID, req.IssuerID) {
 		return nil, errIncompleteRequest
 	}
 	parsed, err := x509.ParseCertificate(req.Der)

sa/precertificates_test.go

@@ -44,10 +44,11 @@ func TestAddPrecertificate(t *testing.T) {
 		regID := reg.ID
 		issuedTime := time.Date(2018, 4, 1, 7, 0, 0, 0, time.UTC)
 		_, err := sa.AddPrecertificate(ctx, &sapb.AddCertificateRequest{
-			Der:    testCert.Raw,
+			Der:      testCert.Raw,
-			RegID:  regID,
+			RegID:    regID,
-			Ocsp:   ocspResp,
+			Ocsp:     ocspResp,
-			Issued: issuedTime.UnixNano(),
+			Issued:   issuedTime.UnixNano(),
+			IssuerID: 1,
 		})
 		test.AssertNotError(t, err, "Couldn't add test cert")
 
@@ -86,10 +87,11 @@ func TestAddPrecertificate(t *testing.T) {
 		// Adding the same certificate with the same serial should result in an
 		// error
 		_, err = sa.AddPrecertificate(ctx, &sapb.AddCertificateRequest{
-			Der:    testCert.Raw,
+			Der:      testCert.Raw,
-			RegID:  regID,
+			RegID:    regID,
-			Ocsp:   ocspResp,
+			Ocsp:     ocspResp,
-			Issued: issuedTime.UnixNano(),
+			Issued:   issuedTime.UnixNano(),
+			IssuerID: 1,
 		})
 		if err == nil {
 			t.Fatalf("Expected error inserting duplicate precertificate, got none")
@@ -102,6 +104,30 @@ func TestAddPrecertificate(t *testing.T) {
 	addPrecert(true)
 }
 
+func TestAddPrecertificateIncomplete(t *testing.T) {
+	sa, _, cleanUp := initSA(t)
+	defer cleanUp()
+
+	reg := satest.CreateWorkingRegistration(t, sa)
+
+	// Create a throw-away self signed certificate with a random name and
+	// serial number
+	_, testCert := test.ThrowAwayCert(t, 1)
+
+	// Add the cert as a precertificate
+	ocspResp := []byte{0, 0, 1}
+	regID := reg.ID
+	_, err := sa.AddPrecertificate(ctx, &sapb.AddCertificateRequest{
+		Der:    testCert.Raw,
+		RegID:  regID,
+		Ocsp:   ocspResp,
+		Issued: time.Date(2018, 4, 1, 7, 0, 0, 0, time.UTC).UnixNano(),
+		// Leaving out IssuerID
+	})
+
+	test.AssertError(t, err, "Adding precert with no issuer did not fail")
+}
+
 func TestAddPrecertificateKeyHash(t *testing.T) {
 	sa, _, cleanUp := initSA(t)
 	defer cleanUp()
@@ -109,10 +135,11 @@ func TestAddPrecertificateKeyHash(t *testing.T) {
 
 	serial, testCert := test.ThrowAwayCert(t, 1)
 	_, err := sa.AddPrecertificate(ctx, &sapb.AddCertificateRequest{
-		Der:    testCert.Raw,
+		Der:      testCert.Raw,
-		RegID:  reg.ID,
+		RegID:    reg.ID,
-		Ocsp:   []byte{1, 2, 3},
+		Ocsp:     []byte{1, 2, 3},
-		Issued: testCert.NotBefore.UnixNano(),
+		Issued:   testCert.NotBefore.UnixNano(),
+		IssuerID: 1,
 	})
 	test.AssertNotError(t, err, "failed to add precert")
 

sa/sa_test.go

@@ -643,11 +643,11 @@ func TestPreviousCertificateExists(t *testing.T) {
 	test.AssertNotError(t, err, "reading cert DER")
 
 	issued := sa.clk.Now()
-	issuedUnix := issued.UnixNano()
 	_, err = sa.AddPrecertificate(ctx, &sapb.AddCertificateRequest{
-		Der:    certDER,
+		Der:      certDER,
-		Issued: issuedUnix,
+		Issued:   issued.UnixNano(),
-		RegID:  reg.ID,
+		RegID:    reg.ID,
+		IssuerID: 1,
 	})
 	test.AssertNotError(t, err, "Failed to add precertificate")
 	_, err = sa.AddCertificate(ctx, certDER, reg.ID, nil, &issued)
@@ -1563,12 +1563,12 @@ func TestRevokeCertificate(t *testing.T) {
 	// Add a cert to the DB to test with.
 	certDER, err := ioutil.ReadFile("www.eff.org.der")
 	test.AssertNotError(t, err, "Couldn't read example cert DER")
-	issued := sa.clk.Now().UnixNano()
 	_, err = sa.AddPrecertificate(ctx, &sapb.AddCertificateRequest{
-		Der:    certDER,
+		Der:      certDER,
-		RegID:  reg.ID,
+		RegID:    reg.ID,
-		Ocsp:   nil,
+		Ocsp:     nil,
-		Issued: issued,
+		Issued:   sa.clk.Now().UnixNano(),
+		IssuerID: 1,
 	})
 	test.AssertNotError(t, err, "Couldn't add www.eff.org.der")
 
@@ -1634,11 +1634,11 @@ func TestAddCertificateRenewalBit(t *testing.T) {
 	test.AssertNotError(t, tx.Commit(), "Failed to commit transaction")
 
 	// Add the certificate with the same names.
-	issuedUnix := issued.UnixNano()
 	_, err = sa.AddPrecertificate(ctx, &sapb.AddCertificateRequest{
-		Der:    certDER,
+		Der:      certDER,
-		Issued: issuedUnix,
+		Issued:   issued.UnixNano(),
-		RegID:  reg.ID,
+		RegID:    reg.ID,
+		IssuerID: 1,
 	})
 	test.AssertNotError(t, err, "Failed to add precertificate")
 	_, err = sa.AddCertificate(ctx, certDER, reg.ID, nil, &issued)
@@ -1671,11 +1671,11 @@ func TestAddCertificateRenewalBit(t *testing.T) {
 	test.AssertNotError(t, err, "Unexpected error parsing test-cert.der test file")
 	names = cert.DNSNames
 
-	issuedUnix = issued.UnixNano()
 	_, err = sa.AddPrecertificate(ctx, &sapb.AddCertificateRequest{
-		Der:    certDER,
+		Der:      certDER,
-		Issued: issuedUnix,
+		Issued:   issued.UnixNano(),
-		RegID:  reg.ID,
+		RegID:    reg.ID,
+		IssuerID: 1,
 	})
 	test.AssertNotError(t, err, "Failed to add precertificate")
 	_, err = sa.AddCertificate(ctx, certDER, reg.ID, nil, &issued)

test/integration/orphan_finder_test.go

@@ -23,8 +23,8 @@ import (
 	"github.com/letsencrypt/pkcs11key/v4"
 )
 
-var template = `[AUDIT] Failed RPC to store at SA, orphaning precertificate: serial=[%x] cert=[%x] err=[sa.StorageAuthority.AddCertificate timed out after 5000 ms], regID=[1], orderID=[1]
+var template = `[AUDIT] Failed RPC to store at SA, orphaning precertificate: serial=[%x] cert=[%x] err=[sa.StorageAuthority.AddCertificate timed out after 5000 ms], issuerID=[1], regID=[1], orderID=[1]
-[AUDIT] Failed RPC to store at SA, orphaning certificate: serial=[%x] cert=[%x] err=[sa.StorageAuthority.AddCertificate timed out after 5000 ms], regID=[1], orderID=[1]`
+[AUDIT] Failed RPC to store at SA, orphaning certificate: serial=[%x] cert=[%x] err=[sa.StorageAuthority.AddCertificate timed out after 5000 ms], issuerID=[1], regID=[1], orderID=[1]`
 
 // TestOrphanFinder runs the orphan-finder with an example input file. This must
 // be run after other tests so the account ID 1 exists (since the inserted