ca: deprecate StoreLintingCertificateInsteadOfPrecertificate (#6970)

And turn off the orphan queue in config-next.

Author: jsha

ca/ca.go

@@ -220,43 +220,14 @@ func (ca *certificateAuthorityImpl) IssuePrecertificate(ctx context.Context, iss
 		return nil, err
 	}
 
-	precertDER, issuer, err := ca.issuePrecertificateInner(ctx, issueReq, serialBigInt, validity)
+	precertDER, _, err := ca.issuePrecertificateInner(ctx, issueReq, serialBigInt, validity)
 	if err != nil {
 		return nil, err
 	}
-	issuerNameID := issuer.Cert.NameID()
 
-	req := &sapb.AddCertificateRequest{
-		Der:          precertDER,
-		RegID:        regID,
-		Issued:       nowNanos,
-		IssuerNameID: int64(issuerNameID),
-	}
-
-	if features.Enabled(features.StoreLintingCertificateInsteadOfPrecertificate) {
-		_, err = ca.sa.SetCertificateStatusReady(ctx, &sapb.Serial{Serial: serialHex})
-		if err != nil {
-			return nil, err
-		}
-	} else {
-		_, err = ca.sa.AddPrecertificate(ctx, req)
-		if err != nil {
-			ca.orphanCount.With(prometheus.Labels{"type": "precert"}).Inc()
-			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], issuerID=[%d], regID=[%d], orderID=[%d], err=[%v]",
-				serialHex, hex.EncodeToString(precertDER), issuerNameID, issueReq.RegistrationID, issueReq.OrderID, err)
-			if ca.orphanQueue != nil {
-				ca.queueOrphan(&orphanedCert{
-					DER:      precertDER,
-					RegID:    regID,
-					Precert:  true,
-					IssuerID: int64(issuerNameID),
-				})
-			}
-			return nil, err
-		}
+	_, err = ca.sa.SetCertificateStatusReady(ctx, &sapb.Serial{Serial: serialHex})
+	if err != nil {
+		return nil, err
 	}
 
 	return &capb.IssuePrecertificateResponse{
@@ -460,18 +431,16 @@ func (ca *certificateAuthorityImpl) issuePrecertificateInner(ctx context.Context
 		return nil, nil, berrors.InternalServerError("failed to prepare precertificate signing: %s", err)
 	}
 
-	if features.Enabled(features.StoreLintingCertificateInsteadOfPrecertificate) {
-		nowNanos := ca.clk.Now().UnixNano()
-		_, err = ca.sa.AddPrecertificate(context.Background(), &sapb.AddCertificateRequest{
-			Der:          lintCertBytes,
-			RegID:        issueReq.RegistrationID,
-			Issued:       nowNanos,
-			IssuerNameID: int64(issuer.Cert.NameID()),
-			OcspNotReady: true,
-		})
-		if err != nil {
-			return nil, nil, err
-		}
+	nowNanos := ca.clk.Now().UnixNano()
+	_, err = ca.sa.AddPrecertificate(context.Background(), &sapb.AddCertificateRequest{
+		Der:          lintCertBytes,
+		RegID:        issueReq.RegistrationID,
+		Issued:       nowNanos,
+		IssuerNameID: int64(issuer.Cert.NameID()),
+		OcspNotReady: true,
+	})
+	if err != nil {
+		return nil, nil, err
 	}
 
 	certDER, err := issuer.Issue(issuanceToken)

ca/ca_test.go

@@ -24,8 +24,6 @@ import (
 	"github.com/jmhodges/clock"
 	"github.com/prometheus/client_golang/prometheus"
 	"google.golang.org/grpc"
-	"google.golang.org/grpc/codes"
-	"google.golang.org/grpc/status"
 	"google.golang.org/protobuf/types/known/emptypb"
 
 	capb "github.com/letsencrypt/boulder/ca/proto"
@@ -150,7 +148,7 @@ func (m *mockSA) GetCertificate(ctx context.Context, req *sapb.Serial, _ ...grpc
 }
 
 func (m *mockSA) SetCertificateStatusReady(ctx context.Context, req *sapb.Serial, _ ...grpc.CallOption) (*emptypb.Empty, error) {
-	return nil, status.Error(codes.Unimplemented, "unimplemented mock")
+	return &emptypb.Empty{}, nil
 }
 
 var caKey crypto.Signer
@@ -946,74 +944,6 @@ func (qsa *queueSA) AddPrecertificate(ctx context.Context, req *sapb.AddCertific
 	return nil, nil
 }
 
-// TestPrecertOrphanQueue tests that IssuePrecertificate writes precertificates
-// to the orphan queue if storage fails, and that `integrateOrphan` later
-// successfully writes those precertificates to the database. To do this, it
-// uses the `queueSA` mock, which allows us to flip on and off a "fail" bit that
-// decides whether it errors in response to storage requests.
-func TestPrecertOrphanQueue(t *testing.T) {
-	tmpDir := t.TempDir()
-	orphanQueue, err := goque.OpenQueue(tmpDir)
-	test.AssertNotError(t, err, "Failed to open orphaned certificate queue")
-
-	qsa := &queueSA{fail: true}
-	testCtx := setup(t)
-	fakeNow := time.Date(2019, 9, 20, 0, 0, 0, 0, time.UTC)
-	testCtx.fc.Set(fakeNow)
-	ca, err := NewCertificateAuthorityImpl(
-		qsa,
-		testCtx.pa,
-		testCtx.boulderIssuers,
-		nil,
-		testCtx.certExpiry,
-		testCtx.certBackdate,
-		testCtx.serialPrefix,
-		testCtx.maxNames,
-		testCtx.keyPolicy,
-		orphanQueue,
-		testCtx.logger,
-		testCtx.stats,
-		testCtx.signatureCount,
-		testCtx.signErrorCount,
-		testCtx.fc)
-	test.AssertNotError(t, err, "Failed to create CA")
-
-	err = ca.integrateOrphan()
-	if err != goque.ErrEmpty {
-		t.Fatalf("Unexpected error, wanted %q, got %q", goque.ErrEmpty, err)
-	}
-
-	_, err = ca.IssuePrecertificate(context.Background(), &capb.IssueCertificateRequest{
-		RegistrationID: 1,
-		OrderID:        1,
-		Csr:            CNandSANCSR,
-	})
-	test.AssertError(t, err, "Expected IssuePrecertificate to fail with `qsa.fail = true`")
-
-	matches := testCtx.logger.GetAllMatching(`orphaning precertificate.* regID=\[1\], orderID=\[1\]`)
-	if len(matches) != 1 {
-		t.Errorf("no log line, or incorrect log line for orphaned precertificate:\n%s",
-			strings.Join(testCtx.logger.GetAllMatching(".*"), "\n"))
-	}
-
-	test.AssertMetricWithLabelsEquals(
-		t, ca.orphanCount, prometheus.Labels{"type": "precert"}, 1)
-
-	qsa.fail = false
-	err = ca.integrateOrphan()
-	test.AssertNotError(t, err, "integrateOrphan failed")
-	if !qsa.issuedPrecert.Equal(fakeNow) {
-		t.Errorf("expected issued time to be %s, got %s", fakeNow, qsa.issuedPrecert)
-	}
-	err = ca.integrateOrphan()
-	if err != goque.ErrEmpty {
-		t.Fatalf("Unexpected error, wanted %q, got %q", goque.ErrEmpty, err)
-	}
-
-	test.AssertMetricWithLabelsEquals(
-		t, ca.adoptedOrphanCount, prometheus.Labels{"type": "precert"}, 1)
-}
-
 func TestOrphanQueue(t *testing.T) {
 	tmpDir := t.TempDir()
 	orphanQueue, err := goque.OpenQueue(tmpDir)

features/featureflag_string.go

@@ -16,6 +16,7 @@ const (
 	StoreRevokerInfo
 	ROCSPStage6
 	ROCSPStage7
+	StoreLintingCertificateInsteadOfPrecertificate
 
 	//   Currently in-use features
 	// Check CAA and respect validationmethods parameter.
@@ -67,14 +68,6 @@ const (
 	// According to the BRs Section 7.1.4.2.2(a), the commonName field is
 	// Deprecated, and its inclusion is discouraged but not (yet) prohibited.
 	RequireCommonName
-
-	// StoreLintingCertificateInsteadOfPrecertificate stores a copy of the fake
-	// certificate we use for linting in the `precertificates` table. This
-	// allows us to write something useful to the database before signing
-	// anything with a real issuer certificate, so we can treat it as if it
-	// was issued even if there is a power outage or other error between
-	// signing the precertificate and writing it to the database.
-	StoreLintingCertificateInsteadOfPrecertificate
 )
 
 // List of features and their default value, protected by fMu

features/features.go

@@ -111,13 +111,11 @@
 			"blockedKeyFile": "test/example-blocked-keys.yaml",
 			"fermatRounds": 100
 		},
-		"orphanQueueDir": "/tmp/orphaned-certificates-a",
 		"ocspLogMaxLength": 4000,
 		"ocspLogPeriod": "500ms",
 		"ecdsaAllowListFilename": "test/config-next/ecdsaAllowList.yml",
 		"ctLogListFile": "test/ct-test-srv/log_list.json",
 		"features": {
-			"StoreLintingCertificateInsteadOfPrecertificate": true,
 			"RequireCommonName": false
 		}
 	},

test/config-next/ca-a.json

@@ -111,13 +111,11 @@
 			"blockedKeyFile": "test/example-blocked-keys.yaml",
 			"fermatRounds": 100
 		},
-		"orphanQueueDir": "/tmp/orphaned-certificates-b",
 		"ocspLogMaxLength": 4000,
 		"ocspLogPeriod": "500ms",
 		"ecdsaAllowListFilename": "test/config-next/ecdsaAllowList.yml",
 		"ctLogListFile": "test/ct-test-srv/log_list.json",
 		"features": {
-			"StoreLintingCertificateInsteadOfPrecertificate": true,
 			"RequireCommonName": false
 		}
 	},