Merge branch 'main' into drop-tables

Author: jprenken

.github/workflows/release.yml

@@ -19,6 +19,7 @@ jobs:
     runs-on: ubuntu-20.04
     permissions:
       contents: write
+      packages: write
     steps:
       - uses: actions/checkout@v4
         with:

cmd/boulder-wfe2/main.go

@@ -42,16 +42,19 @@ type Config struct {
 		TLSListenAddress string `validate:"omitempty,hostname_port"`
 
 		// Timeout is the per-request overall timeout. This should be slightly
-		// lower than the upstream's timeout when making requests to the WFE.
+		// lower than the upstream's timeout when making requests to this service.
 		Timeout config.Duration `validate:"-"`
 
+		// ShutdownStopTimeout determines the maximum amount of time to wait
+		// for extant request handlers to complete before exiting. It should be
+		// greater than Timeout.
+		ShutdownStopTimeout config.Duration
+
 		ServerCertificatePath string `validate:"required_with=TLSListenAddress"`
 		ServerKeyPath         string `validate:"required_with=TLSListenAddress"`
 
 		AllowOrigins []string
 
-		ShutdownStopTimeout config.Duration
-
 		SubscriberAgreementURL string
 
 		TLS cmd.TLSConfig

cmd/ocsp-responder/main.go

@@ -51,10 +51,15 @@ type Config struct {
 		// OCSP requests. This has a default value of ":80".
 		ListenAddress string `validate:"omitempty,hostname_port"`
 
-		// When to timeout a request. This should be slightly lower than the
-		// upstream's timeout when making request to ocsp-responder.
+		// Timeout is the per-request overall timeout. This should be slightly
+		// lower than the upstream's timeout when making requests to this service.
 		Timeout config.Duration `validate:"-"`
 
+		// ShutdownStopTimeout determines the maximum amount of time to wait
+		// for extant request handlers to complete before exiting. It should be
+		// greater than Timeout.
+		ShutdownStopTimeout config.Duration
+
 		// How often a response should be signed when using Redis/live-signing
 		// path. This has a default value of 60h.
 		LiveSigningPeriod config.Duration `validate:"-"`
@@ -80,8 +85,6 @@ type Config struct {
 		// 40 * 5 / 0.02 = 10,000 requests before the oldest request times out.
 		MaxSigningWaiters int `validate:"min=0"`
 
-		ShutdownStopTimeout config.Duration
-
 		RequiredSerialPrefixes []string `validate:"omitempty,dive,hexadecimal"`
 
 		Features features.Config

cmd/sfe/main.go

@@ -25,11 +25,12 @@ type Config struct {
 		ListenAddress string `validate:"omitempty,hostname_port"`
 
 		// Timeout is the per-request overall timeout. This should be slightly
-		// lower than the upstream's timeout when making requests to the SFE.
+		// lower than the upstream's timeout when making requests to this service.
 		Timeout config.Duration `validate:"-"`
 
-		// ShutdownStopTimeout is the duration that the SFE will wait before
-		// shutting down any listening servers.
+		// ShutdownStopTimeout determines the maximum amount of time to wait
+		// for extant request handlers to complete before exiting. It should be
+		// greater than Timeout.
 		ShutdownStopTimeout config.Duration
 
 		TLS cmd.TLSConfig

ra/ra.go

@@ -319,6 +319,10 @@ type certificateRequestEvent struct {
 	// CertProfileHash is SHA256 sum over every exported field of an
 	// issuance.ProfileConfig, represented here as a hexadecimal string.
 	CertProfileHash string `json:",omitempty"`
+	// PreviousCertificateIssued is present when this certificate uses the same set
+	// of FQDNs as a previous certificate (from any account) and contains the
+	// notBefore of the most recent such certificate.
+	PreviousCertificateIssued time.Time `json:",omitempty"`
 }
 
 // certificateRevocationEvent is a struct for holding information that is logged
@@ -889,6 +893,10 @@ func (ra *RegistrationAuthorityImpl) FinalizeOrder(ctx context.Context, req *rap
 		// that it can wait for all goroutines to drain during shutdown.
 		ra.drainWG.Add(1)
 		go func() {
+			// The original context will be canceled in the RPC layer when FinalizeOrder returns,
+			// so split off a context that won't be canceled (and has its own timeout).
+			ctx, cancel := context.WithTimeout(context.WithoutCancel(ctx), ra.finalizeTimeout)
+			defer cancel()
 			_, err := ra.issueCertificateOuter(ctx, proto.Clone(order).(*corepb.Order), csr, logEvent)
 			if err != nil {
 				// We only log here, because this is in a background goroutine with
@@ -1014,9 +1022,23 @@ func (ra *RegistrationAuthorityImpl) issueCertificateOuter(
 	ra.inflightFinalizes.Inc()
 	defer ra.inflightFinalizes.Dec()
 
+	isRenewal := false
+	timestamps, err := ra.SA.FQDNSetTimestampsForWindow(ctx, &sapb.CountFQDNSetsRequest{
+		DnsNames: order.DnsNames,
+		Window:   durationpb.New(120 * 24 * time.Hour),
+		Limit:    1,
+	})
+	if err != nil {
+		return nil, fmt.Errorf("checking if certificate is a renewal: %w", err)
+	}
+	if len(timestamps.Timestamps) > 0 {
+		isRenewal = true
+		logEvent.PreviousCertificateIssued = timestamps.Timestamps[0].AsTime()
+	}
+
 	// Step 3: Issue the Certificate
 	cert, cpId, err := ra.issueCertificateInner(
-		ctx, csr, order.CertificateProfileName, accountID(order.RegistrationID), orderID(order.Id))
+		ctx, csr, isRenewal, order.CertificateProfileName, accountID(order.RegistrationID), orderID(order.Id))
 
 	// Step 4: Fail the order if necessary, and update metrics and log fields
 	var result string
@@ -1119,16 +1141,10 @@ type certProfileID struct {
 func (ra *RegistrationAuthorityImpl) issueCertificateInner(
 	ctx context.Context,
 	csr *x509.CertificateRequest,
+	isRenewal bool,
 	profileName string,
 	acctID accountID,
 	oID orderID) (*x509.Certificate, *certProfileID, error) {
-	if features.Get().AsyncFinalize {
-		// If we're in async mode, use a context with a much longer timeout.
-		var cancel func()
-		ctx, cancel = context.WithTimeout(context.WithoutCancel(ctx), ra.finalizeTimeout)
-		defer cancel()
-	}
-
 	// wrapError adds a prefix to an error. If the error is a boulder error then
 	// the problem detail is updated with the prefix. Otherwise a new error is
 	// returned with the message prefixed using `fmt.Errorf`
@@ -1164,12 +1180,6 @@ func (ra *RegistrationAuthorityImpl) issueCertificateInner(
 		return nil, nil, wrapError(err, "getting SCTs")
 	}
 
-	exists, err := ra.SA.FQDNSetExists(ctx, &sapb.FQDNSetExistsRequest{DnsNames: parsedPrecert.DNSNames})
-	if err != nil {
-		return nil, nil, wrapError(err, "checking if certificate is a renewal")
-	}
-	isRenewal := exists.Exists
-
 	cert, err := ra.CA.IssueCertificateForPrecertificate(ctx, &capb.IssueCertificateForPrecertificateRequest{
 		DER:             precert.DER,
 		SCTs:            scts,
@@ -1518,8 +1528,7 @@ func (ra *RegistrationAuthorityImpl) PerformValidation(
 	// Clock for start of PerformValidation.
 	vStart := ra.clk.Now()
 
-	// TODO(#7153): Check each value via core.IsAnyNilOrZero
-	if req.Authz == nil || req.Authz.Id == "" || req.Authz.DnsName == "" || req.Authz.Status == "" || core.IsAnyNilOrZero(req.Authz.Expires) {
+	if core.IsAnyNilOrZero(req.Authz, req.Authz.Id, req.Authz.DnsName, req.Authz.Status, req.Authz.Expires) {
 		return nil, errIncompleteGRPCRequest
 	}
 
@@ -2216,8 +2225,7 @@ func (ra *RegistrationAuthorityImpl) NewOrder(ctx context.Context, req *rapb.New
 	// Error if an incomplete order is returned.
 	if existingOrder != nil {
 		// Check to see if the expected fields of the existing order are set.
-		// TODO(#7153): Check each value via core.IsAnyNilOrZero
-		if existingOrder.Id == 0 || existingOrder.Status == "" || existingOrder.RegistrationID == 0 || len(existingOrder.DnsNames) == 0 || core.IsAnyNilOrZero(existingOrder.Created, existingOrder.Expires) {
+		if core.IsAnyNilOrZero(existingOrder.Id, existingOrder.Status, existingOrder.RegistrationID, existingOrder.DnsNames, existingOrder.Created, existingOrder.Expires) {
 			return nil, errIncompleteGRPCResponse
 		}
 

ra/ra_test.go

@@ -3295,7 +3295,7 @@ func TestIssueCertificateInnerErrs(t *testing.T) {
 			// Mock the CA
 			ra.CA = tc.Mock
 			// Attempt issuance
-			_, _, err = ra.issueCertificateInner(ctx, csrOb, order.CertificateProfileName, accountID(Registration.Id), orderID(order.Id))
+			_, _, err = ra.issueCertificateInner(ctx, csrOb, false, order.CertificateProfileName, accountID(Registration.Id), orderID(order.Id))
 			// We expect all of the testcases to fail because all use mocked CAs that deliberately error
 			test.AssertError(t, err, "issueCertificateInner with failing mock CA did not fail")
 			// If there is an expected `error` then match the error message
@@ -3338,8 +3338,12 @@ func (sa *mockSAWithFinalize) FinalizeOrder(ctx context.Context, req *sapb.Final
 	return &emptypb.Empty{}, nil
 }
 
-func (sa *mockSAWithFinalize) FQDNSetExists(ctx context.Context, in *sapb.FQDNSetExistsRequest, opts ...grpc.CallOption) (*sapb.Exists, error) {
-	return &sapb.Exists{}, nil
+func (sa *mockSAWithFinalize) FQDNSetTimestampsForWindow(ctx context.Context, in *sapb.CountFQDNSetsRequest, opts ...grpc.CallOption) (*sapb.Timestamps, error) {
+	return &sapb.Timestamps{
+		Timestamps: []*timestamppb.Timestamp{
+			timestamppb.Now(),
+		},
+	}, nil
 }
 
 func TestIssueCertificateInnerWithProfile(t *testing.T) {
@@ -3372,7 +3376,7 @@ func TestIssueCertificateInnerWithProfile(t *testing.T) {
 
 	// Call issueCertificateInner with the CSR generated above and the profile
 	// name "default", which will cause the mockCA to return a specific hash.
-	_, cpId, err := ra.issueCertificateInner(context.Background(), csr, "default", 1, 1)
+	_, cpId, err := ra.issueCertificateInner(context.Background(), csr, false, "default", 1, 1)
 	test.AssertNotError(t, err, "issuing cert with profile name")
 	test.AssertEquals(t, mockCA.profileName, cpId.name)
 	test.AssertByteEquals(t, mockCA.profileHash, cpId.hash)

sa/db/boulder_sa/20230419000000_CombinedSchema.sql

@@ -86,6 +86,8 @@ CREATE TABLE `fqdnSets` (
   `id` bigint(20) UNSIGNED NOT NULL AUTO_INCREMENT,
   `setHash` binary(32) NOT NULL,
   `serial` varchar(255) NOT NULL,
+  -- Note: This should actually be called "notBefore" since it is set
+  -- based on the certificate's notBefore field, not the issuance time.
   `issued` datetime NOT NULL,
   `expires` datetime NOT NULL,
   PRIMARY KEY (`id`),

sa/model.go

@@ -392,6 +392,8 @@ type orderModelv1 struct {
 	BeganProcessing   bool
 }
 
+// orderModelv2 represents one row in the orders table. The
+// CertificateProfileName column is a pointer because the column is NULL-able.
 type orderModelv2 struct {
 	ID                     int64
 	RegistrationID         int64
@@ -400,7 +402,7 @@ type orderModelv2 struct {
 	Error                  []byte
 	CertificateSerial      string
 	BeganProcessing        bool
-	CertificateProfileName string
+	CertificateProfileName *string
 }
 
 type orderToAuthzModel struct {
@@ -457,14 +459,17 @@ func modelToOrderv1(om *orderModelv1) (*corepb.Order, error) {
 }
 
 func orderToModelv2(order *corepb.Order) (*orderModelv2, error) {
+	// Make a local copy so we can take a reference to it below.
+	profile := order.CertificateProfileName
+
 	om := &orderModelv2{
 		ID:                     order.Id,
 		RegistrationID:         order.RegistrationID,
 		Expires:                order.Expires.AsTime(),
 		Created:                order.Created.AsTime(),
 		BeganProcessing:        order.BeganProcessing,
 		CertificateSerial:      order.CertificateSerial,
-		CertificateProfileName: order.CertificateProfileName,
+		CertificateProfileName: &profile,
 	}
 
 	if order.Error != nil {
@@ -481,14 +486,18 @@ func orderToModelv2(order *corepb.Order) (*orderModelv2, error) {
 }
 
 func modelToOrderv2(om *orderModelv2) (*corepb.Order, error) {
+	profile := ""
+	if om.CertificateProfileName != nil {
+		profile = *om.CertificateProfileName
+	}
 	order := &corepb.Order{
 		Id:                     om.ID,
 		RegistrationID:         om.RegistrationID,
 		Expires:                timestamppb.New(om.Expires),
 		Created:                timestamppb.New(om.Created),
 		CertificateSerial:      om.CertificateSerial,
 		BeganProcessing:        om.BeganProcessing,
-		CertificateProfileName: om.CertificateProfileName,
+		CertificateProfileName: profile,
 	}
 	if len(om.Error) > 0 {
 		var problem corepb.ProblemDetails

sa/sa.go

@@ -281,8 +281,7 @@ func (ssa *SQLStorageAuthority) UpdateRegistrationKey(ctx context.Context, req *
 
 // AddSerial writes a record of a serial number generation to the DB.
 func (ssa *SQLStorageAuthority) AddSerial(ctx context.Context, req *sapb.AddSerialRequest) (*emptypb.Empty, error) {
-	// TODO(#7153): Check each value via core.IsAnyNilOrZero
-	if req.Serial == "" || req.RegID == 0 || core.IsAnyNilOrZero(req.Created, req.Expires) {
+	if core.IsAnyNilOrZero(req.Serial, req.RegID, req.Created, req.Expires) {
 		return nil, errIncompleteRequest
 	}
 	err := ssa.dbMap.Insert(ctx, &recordedSerialModel{
@@ -332,8 +331,7 @@ func (ssa *SQLStorageAuthority) SetCertificateStatusReady(ctx context.Context, r
 // certificate multiple times. Calling code needs to first insert the cert's
 // serial into the Serials table to ensure uniqueness.
 func (ssa *SQLStorageAuthority) AddPrecertificate(ctx context.Context, req *sapb.AddCertificateRequest) (*emptypb.Empty, error) {
-	// TODO(#7153): Check each value via core.IsAnyNilOrZero
-	if len(req.Der) == 0 || req.RegID == 0 || req.IssuerNameID == 0 || core.IsAnyNilOrZero(req.Issued) {
+	if core.IsAnyNilOrZero(req.Der, req.RegID, req.IssuerNameID, req.Issued) {
 		return nil, errIncompleteRequest
 	}
 	parsed, err := x509.ParseCertificate(req.Der)
@@ -424,8 +422,7 @@ func (ssa *SQLStorageAuthority) AddPrecertificate(ctx context.Context, req *sapb
 // AddCertificate stores an issued certificate, returning an error if it is a
 // duplicate or if any other failure occurs.
 func (ssa *SQLStorageAuthority) AddCertificate(ctx context.Context, req *sapb.AddCertificateRequest) (*emptypb.Empty, error) {
-	// TODO(#7153): Check each value via core.IsAnyNilOrZero
-	if len(req.Der) == 0 || req.RegID == 0 || core.IsAnyNilOrZero(req.Issued) {
+	if core.IsAnyNilOrZero(req.Der, req.RegID, req.Issued) {
 		return nil, errIncompleteRequest
 	}
 	parsedCertificate, err := x509.ParseCertificate(req.Der)
@@ -619,7 +616,7 @@ func (ssa *SQLStorageAuthority) NewOrderAndAuthzs(ctx context.Context, req *sapb
 				RegistrationID:         req.NewOrder.RegistrationID,
 				Expires:                req.NewOrder.Expires.AsTime(),
 				Created:                created,
-				CertificateProfileName: req.NewOrder.CertificateProfileName,
+				CertificateProfileName: &req.NewOrder.CertificateProfileName,
 			}
 			err = tx.Insert(ctx, &omv2)
 			orderID = omv2.ID
@@ -836,8 +833,7 @@ func (ssa *SQLStorageAuthority) FinalizeOrder(ctx context.Context, req *sapb.Fin
 // the authorization is being moved to invalid the validationError field must be set. If the
 // authorization is being moved to valid the validationRecord and expires fields must be set.
 func (ssa *SQLStorageAuthority) FinalizeAuthorization2(ctx context.Context, req *sapb.FinalizeAuthorizationRequest) (*emptypb.Empty, error) {
-	// TODO(#7153): Check each value via core.IsAnyNilOrZero
-	if req.Status == "" || req.Attempted == "" || req.Id == 0 || core.IsAnyNilOrZero(req.Expires) {
+	if core.IsAnyNilOrZero(req.Status, req.Attempted, req.Id, req.Expires) {
 		return nil, errIncompleteRequest
 	}
 
@@ -959,8 +955,7 @@ func addRevokedCertificate(ctx context.Context, tx db.Executor, req *sapb.Revoke
 // RevokeCertificate stores revocation information about a certificate. It will only store this
 // information if the certificate is not already marked as revoked.
 func (ssa *SQLStorageAuthority) RevokeCertificate(ctx context.Context, req *sapb.RevokeCertificateRequest) (*emptypb.Empty, error) {
-	// TODO(#7153): Check each value via core.IsAnyNilOrZero
-	if req.Serial == "" || req.IssuerID == 0 || core.IsAnyNilOrZero(req.Date) {
+	if core.IsAnyNilOrZero(req.Serial, req.IssuerID, req.Date) {
 		return nil, errIncompleteRequest
 	}
 
@@ -1013,8 +1008,7 @@ func (ssa *SQLStorageAuthority) RevokeCertificate(ctx context.Context, req *sapb
 // cert is already revoked, if the new revocation reason is `KeyCompromise`,
 // and if the revokedDate is identical to the current revokedDate.
 func (ssa *SQLStorageAuthority) UpdateRevokedCertificate(ctx context.Context, req *sapb.RevokeCertificateRequest) (*emptypb.Empty, error) {
-	// TODO(#7153): Check each value via core.IsAnyNilOrZero
-	if req.Serial == "" || req.IssuerID == 0 || core.IsAnyNilOrZero(req.Date, req.Backdate) {
+	if core.IsAnyNilOrZero(req.Serial, req.IssuerID, req.Date, req.Backdate) {
 		return nil, errIncompleteRequest
 	}
 	if req.Reason != ocsp.KeyCompromise {