Merge branch main into rm-returning

cmd/boulder-ca/main.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"flag"
 	"os"
-	"reflect"
 	"strconv"
 	"time"
 
@@ -39,36 +38,15 @@ type Config struct {
 			// configured certificate profile or boulder-ca will fail to start.
 			DefaultCertificateProfileName string `validate:"omitempty,alphanum,min=1,max=32"`
 
-			// TODO(#7414) Remove this deprecated field.
-			// Deprecated: Use CertProfiles instead. Profile implicitly takes
-			// the internal Boulder default value of ca.DefaultCertProfileName.
-			Profile issuance.ProfileConfig `validate:"required_without=CertProfiles,structonly"`
-
 			// One of the profile names must match the value of
 			// DefaultCertificateProfileName or boulder-ca will fail to start.
 			CertProfiles map[string]*issuance.ProfileConfig `validate:"dive,keys,alphanum,min=1,max=32,endkeys,required_without=Profile,structonly"`
 
 			// TODO(#7159): Make this required once all live configs are using it.
 			CRLProfile issuance.CRLProfileConfig `validate:"-"`
 			Issuers    []issuance.IssuerConfig   `validate:"min=1,dive"`
-
-			// LintConfig is a path to a zlint config file.
-			// Deprecated: Use CertProfiles.LintConfig instead.
-			LintConfig string
-			// IgnoredLints is a list of lint names for which any errors should be
-			// ignored.
-			// Deprecated: Use CertProfiles.IgnoredLints instead.
-			IgnoredLints []string
 		}
 
-		// How long issued certificates are valid for.
-		// Deprecated: Use Issuance.CertProfiles.MaxValidityPeriod instead.
-		Expiry config.Duration
-
-		// How far back certificates should be backdated.
-		// Deprecated: Use Issuance.CertProfiles.MaxValidityBackdate instead.
-		Backdate config.Duration
-
 		// What digits we should prepend to serials after randomly generating them.
 		// Deprecated: Use SerialPrefixHex instead.
 		SerialPrefix int `validate:"required_without=SerialPrefixHex,omitempty,min=1,max=127"`
@@ -94,12 +72,6 @@ type Config struct {
 		// Section 4.9.10, it MUST NOT be more than 10 days. Default 96h.
 		LifespanOCSP config.Duration
 
-		// LifespanCRL is how long CRLs are valid for. It should be longer than the
-		// `period` field of the CRL Updater. Per the BRs, Section 4.9.7, it MUST
-		// NOT be more than 10 days.
-		// Deprecated: Use Config.CA.Issuance.CRLProfile.ValidityInterval instead.
-		LifespanCRL config.Duration `validate:"-"`
-
 		// GoodKey is an embedded config stanza for the goodkey library.
 		GoodKey goodkey.Config
 
@@ -179,15 +151,6 @@ func main() {
 		c.CA.LifespanOCSP.Duration = 96 * time.Hour
 	}
 
-	// TODO(#7159): Remove these fallbacks once all live configs are setting the
-	// CRL validity interval inside the Issuance.CRLProfile Config.
-	if c.CA.Issuance.CRLProfile.ValidityInterval.Duration == 0 && c.CA.LifespanCRL.Duration != 0 {
-		c.CA.Issuance.CRLProfile.ValidityInterval = c.CA.LifespanCRL
-	}
-	if c.CA.Issuance.CRLProfile.MaxBackdate.Duration == 0 && c.CA.Backdate.Duration != 0 {
-		c.CA.Issuance.CRLProfile.MaxBackdate = c.CA.Backdate
-	}
-
 	scope, logger, oTelShutdown := cmd.StatsAndLogging(c.Syslog, c.OpenTelemetry, c.CA.DebugAddr)
 	defer oTelShutdown(context.Background())
 	logger.Info(cmd.VersionString())
@@ -226,27 +189,8 @@ func main() {
 	}
 	logger.Infof("Configured default certificate profile name set to: %s", c.CA.Issuance.DefaultCertificateProfileName)
 
-	// TODO(#7414) Remove this check.
-	if !reflect.ValueOf(c.CA.Issuance.Profile).IsZero() && len(c.CA.Issuance.CertProfiles) > 0 {
-		cmd.Fail("Only one of Issuance.Profile or Issuance.CertProfiles can be configured")
-	}
-
-	// If no individual cert profiles are configured, pretend that the deprecated
-	// top-level profile as the only individual profile instead.
-	// TODO(#7414) Remove this fallback.
 	if len(c.CA.Issuance.CertProfiles) == 0 {
-		c.CA.Issuance.CertProfiles = make(map[string]*issuance.ProfileConfig, 0)
-		c.CA.Issuance.CertProfiles[c.CA.Issuance.DefaultCertificateProfileName] = &c.CA.Issuance.Profile
-	}
-
-	// If any individual cert profile doesn't have its own lint configuration,
-	// instead copy in the deprecated top-level lint configuration.
-	// TODO(#7414): Remove this fallback.
-	for _, prof := range c.CA.Issuance.CertProfiles {
-		if prof.LintConfig == "" && len(prof.IgnoredLints) == 0 {
-			prof.LintConfig = c.CA.Issuance.LintConfig
-			prof.IgnoredLints = c.CA.Issuance.IgnoredLints
-		}
+		cmd.Fail("At least one profile must be configured")
 	}
 
 	tlsConfig, err := c.CA.TLS.Load(scope)

features/features.go

@@ -16,10 +16,11 @@ import (
 // package's global Config.
 type Config struct {
 	// Deprecated flags.
-	IncrementRateLimits      bool
-	UseKvLimitsForNewOrder   bool
-	DisableLegacyLimitWrites bool
-	InsertAuthzsIndividually bool
+	IncrementRateLimits         bool
+	UseKvLimitsForNewOrder      bool
+	DisableLegacyLimitWrites    bool
+	MultipleCertificateProfiles bool
+	InsertAuthzsIndividually    bool
 
 	// ServeRenewalInfo exposes the renewalInfo endpoint in the directory and for
 	// GET requests. WARNING: This feature is a draft and highly unstable.
@@ -56,14 +57,6 @@ type Config struct {
 	// make a valid/invalid decision with the results.
 	EnforceMultiCAA bool
 
-	// MultipleCertificateProfiles, when enabled, triggers the following
-	// behavior:
-	//   - SA.NewOrderAndAuthzs: upon receiving a NewOrderRequest with a
-	//     `certificateProfileName` value, will add that value to the database's
-	//     `orders.certificateProfileName` column. Values in this column are
-	//     allowed to be empty.
-	MultipleCertificateProfiles bool
-
 	// CheckIdentifiersPaused checks if any of the identifiers in the order are
 	// currently paused at NewOrder time. If any are paused, an error is
 	// returned to the Subscriber indicating that the order cannot be processed

ra/ra_test.go

@@ -15,7 +15,6 @@ import (
 	"fmt"
 	"math/big"
 	mrand "math/rand/v2"
-	"os"
 	"regexp"
 	"strconv"
 	"strings"
@@ -1389,8 +1388,9 @@ func TestNewOrder_OrderReusex(t *testing.T) {
 	// Create an initial order with regA and names
 	names := []string{"zombo.com", "welcome.to.zombo.com"}
 	orderReq := &rapb.NewOrderRequest{
-		RegistrationID: Registration.Id,
-		DnsNames:       names,
+		RegistrationID:         Registration.Id,
+		DnsNames:               names,
+		CertificateProfileName: "test",
 	}
 	firstOrder, err := ra.NewOrder(context.Background(), orderReq)
 	test.AssertNotError(t, err, "Adding an initial order for regA failed")
@@ -1406,33 +1406,53 @@ func TestNewOrder_OrderReusex(t *testing.T) {
 		Name           string
 		RegistrationID int64
 		DnsNames       []string
+		Profile        string
 		ExpectReuse    bool
 	}{
 		{
 			Name:           "Duplicate order, same regID",
 			RegistrationID: Registration.Id,
 			DnsNames:       names,
+			Profile:        "test",
 			// We expect reuse since the order matches firstOrder
 			ExpectReuse: true,
 		},
 		{
 			Name:           "Subset of order names, same regID",
 			RegistrationID: Registration.Id,
 			DnsNames:       names[:1],
+			Profile:        "test",
 			// We do not expect reuse because the order names don't match firstOrder
 			ExpectReuse: false,
 		},
 		{
 			Name:           "Superset of order names, same regID",
 			RegistrationID: Registration.Id,
 			DnsNames:       append(names, "blog.zombo.com"),
+			Profile:        "test",
 			// We do not expect reuse because the order names don't match firstOrder
 			ExpectReuse: false,
 		},
+		{
+			Name:           "Missing profile, same regID",
+			RegistrationID: Registration.Id,
+			DnsNames:       append(names, "blog.zombo.com"),
+			// We do not expect reuse because the profile is missing
+			ExpectReuse: false,
+		},
+		{
+			Name:           "Missing profile, same regID",
+			RegistrationID: Registration.Id,
+			DnsNames:       append(names, "blog.zombo.com"),
+			Profile:        "different",
+			// We do not expect reuse because a different profile is specified
+			ExpectReuse: false,
+		},
 		{
 			Name:           "Duplicate order, different regID",
 			RegistrationID: secondReg.Id,
 			DnsNames:       names,
+			Profile:        "test",
 			// We do not expect reuse because the order regID differs from firstOrder
 			ExpectReuse: false,
 		},
@@ -1443,8 +1463,9 @@ func TestNewOrder_OrderReusex(t *testing.T) {
 		t.Run(tc.Name, func(t *testing.T) {
 			// Add the order for the test request
 			order, err := ra.NewOrder(context.Background(), &rapb.NewOrderRequest{
-				RegistrationID: tc.RegistrationID,
-				DnsNames:       tc.DnsNames,
+				RegistrationID:         tc.RegistrationID,
+				DnsNames:               tc.DnsNames,
+				CertificateProfileName: tc.Profile,
 			})
 			test.AssertNotError(t, err, "NewOrder returned an unexpected error")
 			test.AssertNotNil(t, order.Id, "NewOrder returned an order with a nil Id")
@@ -1462,50 +1483,6 @@ func TestNewOrder_OrderReusex(t *testing.T) {
 	}
 }
 
-// TestNewOrder_OrderReuse_Profile tests that order reuse respects profiles.
-// This is not simply a test case in TestNewOrder_OrderReuse because it relies
-// on feature-flag gated behavior. It should be unified with that function when
-// the feature flag is removed.
-func TestNewOrder_OrderReuse_Profile(t *testing.T) {
-	// TODO(#7324): Integrate these cases into TestNewOrder_OrderReuse.
-	if !strings.Contains(os.Getenv("BOULDER_CONFIG_DIR"), "test/config-next") {
-		t.Skip("this test requires the db to have the certificateProfileName column in the orders table")
-	}
-
-	_, _, ra, _, _, cleanUp := initAuthorities(t)
-	defer cleanUp()
-
-	features.Set(features.Config{MultipleCertificateProfiles: true})
-	defer features.Reset()
-
-	// Create an initial order with a profile name.
-	extant, err := ra.NewOrder(context.Background(), &rapb.NewOrderRequest{
-		RegistrationID:         Registration.Id,
-		CertificateProfileName: "test",
-		DnsNames:               []string{"a.com", "b.com"},
-	})
-	test.AssertNotError(t, err, "creating test order")
-
-	// Creating an identical order should reuse the first one.
-	new, err := ra.NewOrder(context.Background(), &rapb.NewOrderRequest{
-		RegistrationID:         Registration.Id,
-		CertificateProfileName: "test",
-		DnsNames:               []string{"a.com", "b.com"},
-	})
-	test.AssertNotError(t, err, "creating test order")
-	test.AssertEquals(t, new.Id, extant.Id)
-
-	// Creating a new order for the same names but a different profile should not
-	// reuse the first one.
-	new, err = ra.NewOrder(context.Background(), &rapb.NewOrderRequest{
-		RegistrationID:         Registration.Id,
-		CertificateProfileName: "test2",
-		DnsNames:               []string{"a.com", "b.com"},
-	})
-	test.AssertNotError(t, err, "creating test order")
-	test.AssertNotEquals(t, new.Id, extant.Id)
-}
-
 // TestNewOrder_OrderReuse_Expired tests that expired orders are not reused.
 // This is not simply a test case in TestNewOrder_OrderReuse because it has
 // side effects.

sa/database.go

@@ -271,8 +271,7 @@ func initTables(dbMap *borp.DbMap) {
 	dbMap.AddTableWithName(core.Certificate{}, "certificates").SetKeys(true, "ID")
 	dbMap.AddTableWithName(core.CertificateStatus{}, "certificateStatus").SetKeys(true, "ID")
 	dbMap.AddTableWithName(core.FQDNSet{}, "fqdnSets").SetKeys(true, "ID")
-	dbMap.AddTableWithName(orderModelv2{}, "orders").SetKeys(true, "ID")
-	dbMap.AddTableWithName(orderModelv1{}, "orders").SetKeys(true, "ID")
+	dbMap.AddTableWithName(orderModel{}, "orders").SetKeys(true, "ID")
 	dbMap.AddTableWithName(orderToAuthzModel{}, "orderToAuthz").SetKeys(false, "OrderID", "AuthzID")
 	dbMap.AddTableWithName(orderFQDNSet{}, "orderFqdnSets").SetKeys(true, "ID")
 	dbMap.AddTableWithName(authzModel{}, "authz2").SetKeys(true, "ID")

sa/db-next/boulder_sa/20240304000000_CertificateProfiles.sql

@@ -0,0 +1 @@
+../../db/boulder_sa/20240304000000_CertificateProfiles.sql

sa/db/boulder_sa/20240304000000_CertificateProfiles.sql

@@ -0,0 +1,9 @@
+-- +migrate Up
+-- SQL in section 'Up' is executed when this migration is applied
+
+ALTER TABLE `orders` ADD COLUMN `certificateProfileName` varchar(32) DEFAULT NULL;
+
+-- +migrate Down
+-- SQL section 'Down' is executed when this migration is rolled back
+
+ALTER TABLE `orders` DROP COLUMN `certificateProfileName`;

sa/model.go

@@ -370,20 +370,9 @@ type lintingCertModel struct {
 	Expires        time.Time
 }
 
-// TODO(#7324) orderModelv1 is deprecated, use orderModelv2 moving forward.
-type orderModelv1 struct {
-	ID                int64
-	RegistrationID    int64
-	Expires           time.Time
-	Created           time.Time
-	Error             []byte
-	CertificateSerial string
-	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 {
+// orderModel represents one row in the orders table. The CertificateProfileName
+// column is a pointer because the column is NULL-able.
+type orderModel struct {
 	ID                     int64
 	RegistrationID         int64
 	Expires                time.Time
@@ -399,59 +388,11 @@ type orderToAuthzModel struct {
 	AuthzID int64
 }
 
-// TODO(#7324) orderToModelv1 is deprecated, use orderModelv2 moving forward.
-func orderToModelv1(order *corepb.Order) (*orderModelv1, error) {
-	om := &orderModelv1{
-		ID:                order.Id,
-		RegistrationID:    order.RegistrationID,
-		Expires:           order.Expires.AsTime(),
-		Created:           order.Created.AsTime(),
-		BeganProcessing:   order.BeganProcessing,
-		CertificateSerial: order.CertificateSerial,
-	}
-
-	if order.Error != nil {
-		errJSON, err := json.Marshal(order.Error)
-		if err != nil {
-			return nil, err
-		}
-		if len(errJSON) > mediumBlobSize {
-			return nil, fmt.Errorf("Error object is too large to store in the database")
-		}
-		om.Error = errJSON
-	}
-	return om, nil
-}
-
-// TODO(#7324) modelToOrderv1 is deprecated, use orderModelv2 moving forward.
-func modelToOrderv1(om *orderModelv1) (*corepb.Order, error) {
-	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,
-	}
-	if len(om.Error) > 0 {
-		var problem corepb.ProblemDetails
-		err := json.Unmarshal(om.Error, &problem)
-		if err != nil {
-			return &corepb.Order{}, badJSONError(
-				"failed to unmarshal order model's error",
-				om.Error,
-				err)
-		}
-		order.Error = &problem
-	}
-	return order, nil
-}
-
-func orderToModelv2(order *corepb.Order) (*orderModelv2, error) {
+func orderToModel(order *corepb.Order) (*orderModel, error) {
 	// Make a local copy so we can take a reference to it below.
 	profile := order.CertificateProfileName
 
-	om := &orderModelv2{
+	om := &orderModel{
 		ID:                     order.Id,
 		RegistrationID:         order.RegistrationID,
 		Expires:                order.Expires.AsTime(),
@@ -474,7 +415,7 @@ func orderToModelv2(order *corepb.Order) (*orderModelv2, error) {
 	return om, nil
 }
 
-func modelToOrderv2(om *orderModelv2) (*corepb.Order, error) {
+func modelToOrder(om *orderModel) (*corepb.Order, error) {
 	profile := ""
 	if om.CertificateProfileName != nil {
 		profile = *om.CertificateProfileName