diff --git a/features/features.go b/features/features.go index b7053fb2373..20986c8d22b 100644 --- a/features/features.go +++ b/features/features.go @@ -102,6 +102,12 @@ type Config struct { // and pause issuance for each (account, hostname) pair that repeatedly // fails validation. AutomaticallyPauseZombieClients bool + + // NoPendingAuthzReuse causes the RA to only select already-validated authzs + // to attach to a newly created order. This preserves important client-facing + // functionality (valid authz reuse) while letting us simplify our code by + // removing pending authz reuse. + NoPendingAuthzReuse bool } var fMu = new(sync.RWMutex) diff --git a/ra/ra.go b/ra/ra.go index 4c38937df4e..ed559be508c 100644 --- a/ra/ra.go +++ b/ra/ra.go @@ -2562,12 +2562,22 @@ func (ra *RegistrationAuthorityImpl) NewOrder(ctx context.Context, req *rapb.New // from expiring. authzExpiryCutoff := ra.clk.Now().AddDate(0, 0, 1) - getAuthReq := &sapb.GetAuthorizationsRequest{ - RegistrationID: newOrder.RegistrationID, - ValidUntil: timestamppb.New(authzExpiryCutoff), - DnsNames: newOrder.DnsNames, + var existingAuthz *sapb.Authorizations + if features.Get().NoPendingAuthzReuse { + getAuthReq := &sapb.GetValidAuthorizationsRequest{ + RegistrationID: newOrder.RegistrationID, + ValidUntil: timestamppb.New(authzExpiryCutoff), + DnsNames: newOrder.DnsNames, + } + existingAuthz, err = ra.SA.GetValidAuthorizations2(ctx, getAuthReq) + } else { + getAuthReq := &sapb.GetAuthorizationsRequest{ + RegistrationID: newOrder.RegistrationID, + ValidUntil: timestamppb.New(authzExpiryCutoff), + DnsNames: newOrder.DnsNames, + } + existingAuthz, err = ra.SA.GetAuthorizations2(ctx, getAuthReq) } - existingAuthz, err := ra.SA.GetAuthorizations2(ctx, getAuthReq) if err != nil { return nil, err } diff --git a/ra/ra_test.go b/ra/ra_test.go index 8969ad8aa4a..d0d007e9523 100644 --- a/ra/ra_test.go +++ b/ra/ra_test.go @@ -394,7 +394,7 @@ func initAuthorities(t *testing.T) (*DummyValidationAuthority, sapb.StorageAutho 1, testKeyPolicy, limiter, txnBuilder, 100, 300*24*time.Hour, 7*24*time.Hour, nil, noopCAA{}, - 0, 5*time.Minute, + 7*24*time.Hour, 5*time.Minute, ctp, nil, nil) ra.SA = sa ra.VA = va @@ -1214,8 +1214,6 @@ func TestNewOrderRateLimiting(t *testing.T) { _, _, ra, _, fc, cleanUp := initAuthorities(t) defer cleanUp() - ra.orderLifetime = 5 * 24 * time.Hour - // Create a dummy rate limit config that sets a NewOrdersPerAccount rate // limit with a very low threshold/short window rateLimitDuration := 5 * time.Minute @@ -1264,7 +1262,6 @@ func TestNewOrderRateLimiting(t *testing.T) { func TestEarlyOrderRateLimiting(t *testing.T) { _, _, ra, _, _, cleanUp := initAuthorities(t) defer cleanUp() - ra.orderLifetime = 5 * 24 * time.Hour rateLimitDuration := 5 * time.Minute @@ -2185,7 +2182,6 @@ func TestRecheckCAAInternalServerError(t *testing.T) { func TestNewOrder(t *testing.T) { _, _, ra, _, fc, cleanUp := initAuthorities(t) defer cleanUp() - ra.orderLifetime = time.Hour now := fc.Now() orderA, err := ra.NewOrder(context.Background(), &rapb.NewOrderRequest{ @@ -2195,7 +2191,7 @@ func TestNewOrder(t *testing.T) { }) test.AssertNotError(t, err, "ra.NewOrder failed") test.AssertEquals(t, orderA.RegistrationID, int64(1)) - test.AssertEquals(t, orderA.Expires.AsTime(), now.Add(time.Hour)) + test.AssertEquals(t, orderA.Expires.AsTime(), now.Add(ra.orderLifetime)) test.AssertEquals(t, len(orderA.DnsNames), 3) test.AssertEquals(t, orderA.CertificateProfileName, "test") // We expect the order names to have been sorted, deduped, and lowercased @@ -2203,43 +2199,6 @@ func TestNewOrder(t *testing.T) { test.AssertEquals(t, orderA.Id, int64(1)) test.AssertEquals(t, numAuthorizations(orderA), 3) - // Reuse all existing authorizations - now = fc.Now() - orderB, err := ra.NewOrder(context.Background(), &rapb.NewOrderRequest{ - RegistrationID: Registration.Id, - DnsNames: []string{"b.com", "a.com", "C.COM"}, - }) - test.AssertNotError(t, err, "ra.NewOrder failed") - test.AssertEquals(t, orderB.RegistrationID, int64(1)) - test.AssertEquals(t, orderB.Expires.AsTime(), now.Add(time.Hour)) - // We expect orderB's ID to match orderA's because of pending order reuse - test.AssertEquals(t, orderB.Id, orderA.Id) - test.AssertEquals(t, len(orderB.DnsNames), 3) - test.AssertDeepEquals(t, orderB.DnsNames, []string{"a.com", "b.com", "c.com"}) - test.AssertEquals(t, numAuthorizations(orderB), 3) - test.AssertDeepEquals(t, orderB.V2Authorizations, orderA.V2Authorizations) - - // Reuse all of the existing authorizations from the previous order and - // add a new one - orderA.DnsNames = append(orderA.DnsNames, "d.com") - now = fc.Now() - orderC, err := ra.NewOrder(context.Background(), &rapb.NewOrderRequest{ - RegistrationID: Registration.Id, - DnsNames: orderA.DnsNames, - }) - test.AssertNotError(t, err, "ra.NewOrder failed") - test.AssertEquals(t, orderC.RegistrationID, int64(1)) - test.AssertEquals(t, orderC.Expires.AsTime(), now.Add(time.Hour)) - test.AssertEquals(t, len(orderC.DnsNames), 4) - test.AssertDeepEquals(t, orderC.DnsNames, []string{"a.com", "b.com", "c.com", "d.com"}) - // We expect orderC's ID to not match orderA/orderB's because it is for - // a different set of names - test.AssertNotEquals(t, orderC.Id, orderA.Id) - test.AssertEquals(t, numAuthorizations(orderC), 4) - // Abuse the order of the queries used to extract the reused authorizations - existing := orderC.V2Authorizations[:3] - test.AssertDeepEquals(t, existing, orderA.V2Authorizations) - _, err = ra.NewOrder(context.Background(), &rapb.NewOrderRequest{ RegistrationID: Registration.Id, DnsNames: []string{"a"}, @@ -2251,161 +2210,311 @@ func TestNewOrder(t *testing.T) { // TestNewOrderReuse tests that subsequent requests by an ACME account to create // an identical order results in only one order being created & subsequently // reused. -func TestNewOrderReuse(t *testing.T) { - _, _, ra, _, fc, cleanUp := initAuthorities(t) +func TestNewOrder_OrderReusex(t *testing.T) { + _, _, ra, _, _, cleanUp := initAuthorities(t) defer cleanUp() - ctx := context.Background() + // Create an initial order with regA and names names := []string{"zombo.com", "welcome.to.zombo.com"} - - // Configure the RA to use a short order lifetime - ra.orderLifetime = time.Hour - // Create a var with two times the order lifetime to reference later - doubleLifetime := ra.orderLifetime * 2 - - // Create an initial request with regA and names orderReq := &rapb.NewOrderRequest{ RegistrationID: Registration.Id, DnsNames: names, } + firstOrder, err := ra.NewOrder(context.Background(), orderReq) + test.AssertNotError(t, err, "Adding an initial order for regA failed") // Create a second registration to reference acctKeyB, err := AccountKeyB.MarshalJSON() test.AssertNotError(t, err, "failed to marshal account key") - input := &corepb.Registration{ - Key: acctKeyB, - } - secondReg, err := ra.NewRegistration(ctx, input) + input := &corepb.Registration{Key: acctKeyB} + secondReg, err := ra.NewRegistration(context.Background(), input) test.AssertNotError(t, err, "Error creating a second test registration") - // First, add an order with `names` for regA - firstOrder, err := ra.NewOrder(context.Background(), orderReq) - // It shouldn't fail - test.AssertNotError(t, err, "Adding an initial order for regA failed") - // It should have an ID - test.AssertNotNil(t, firstOrder.Id, "Initial order had a nil ID") testCases := []struct { - Name string - OrderReq *rapb.NewOrderRequest - ExpectReuse bool - AdvanceClock *time.Duration + Name string + RegistrationID int64 + DnsNames []string + ExpectReuse bool }{ { - Name: "Duplicate order, same regID", - OrderReq: orderReq, + Name: "Duplicate order, same regID", + RegistrationID: Registration.Id, + DnsNames: names, // We expect reuse since the order matches firstOrder ExpectReuse: true, }, { - Name: "Subset of order names, same regID", - OrderReq: &rapb.NewOrderRequest{ - RegistrationID: Registration.Id, - DnsNames: []string{names[1]}, - }, + Name: "Subset of order names, same regID", + RegistrationID: Registration.Id, + DnsNames: names[:1], // We do not expect reuse because the order names don't match firstOrder ExpectReuse: false, }, { - Name: "Duplicate order, different regID", - OrderReq: &rapb.NewOrderRequest{ - RegistrationID: secondReg.Id, - DnsNames: names, - }, - // We do not expect reuse because the order regID differs from firstOrder + Name: "Superset of order names, same regID", + RegistrationID: Registration.Id, + DnsNames: append(names, "blog.zombo.com"), + // We do not expect reuse because the order names don't match firstOrder ExpectReuse: false, }, { - Name: "Duplicate order, different profile", - OrderReq: &rapb.NewOrderRequest{ - RegistrationID: Registration.Id, - CertificateProfileName: "different", - DnsNames: names, - }, - // We do not expect reuse because the profile name differs from firstOrder + Name: "Duplicate order, different regID", + RegistrationID: secondReg.Id, + DnsNames: names, + // We do not expect reuse because the order regID differs from firstOrder ExpectReuse: false, }, - { - Name: "Duplicate order, same regID, first expired", - OrderReq: orderReq, - AdvanceClock: &doubleLifetime, - // We do not expect reuse because firstOrder has expired - ExpectReuse: true, - }, + // TODO(#7324): Integrate certificate profile variance into this test. } for _, tc := range testCases { t.Run(tc.Name, func(t *testing.T) { - // If the testcase specifies, advance the clock before adding the order - if tc.AdvanceClock != nil { - _ = fc.Now().Add(*tc.AdvanceClock) - } // Add the order for the test request - order, err := ra.NewOrder(ctx, tc.OrderReq) - // It shouldn't fail + order, err := ra.NewOrder(context.Background(), &rapb.NewOrderRequest{ + RegistrationID: tc.RegistrationID, + DnsNames: tc.DnsNames, + }) test.AssertNotError(t, err, "NewOrder returned an unexpected error") - // The order should not have a nil ID test.AssertNotNil(t, order.Id, "NewOrder returned an order with a nil Id") if tc.ExpectReuse { // If we expected order reuse for this testcase assert that the order // has the same ID as the firstOrder - test.AssertEquals(t, firstOrder.Id, order.Id) + test.AssertEquals(t, order.Id, firstOrder.Id) } else { // Otherwise assert that the order doesn't have the same ID as the // firstOrder - test.AssertNotEquals(t, firstOrder.Id, order.Id) + test.AssertNotEquals(t, order.Id, firstOrder.Id) } }) } } -func TestNewOrderReuseInvalidAuthz(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() - ctx := context.Background() - names := []string{"zombo.com"} + features.Set(features.Config{MultipleCertificateProfiles: true}) + defer features.Reset() - // Create an initial request with regA and names - orderReq := &rapb.NewOrderRequest{ + // 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. +func TestNewOrder_OrderReuse_Expired(t *testing.T) { + _, _, ra, _, fc, cleanUp := initAuthorities(t) + defer cleanUp() + + // Set the order lifetime to something short and known. + ra.orderLifetime = time.Hour + + // Create an initial order. + extant, err := ra.NewOrder(context.Background(), &rapb.NewOrderRequest{ RegistrationID: Registration.Id, - DnsNames: names, - } + DnsNames: []string{"a.com", "b.com"}, + }) + test.AssertNotError(t, err, "creating test order") - // First, add an order with `names` for regA - order, err := ra.NewOrder(ctx, orderReq) - // It shouldn't fail - test.AssertNotError(t, err, "Adding an initial order for regA failed") - // It should have an ID - test.AssertNotNil(t, order.Id, "Initial order had a nil ID") - // It should have one authorization - test.AssertEquals(t, numAuthorizations(order), 1) + // Transition the original order to status invalid by jumping forward in time + // to when it has expired. + fc.Set(extant.Expires.AsTime().Add(2 * time.Hour)) - _, err = ra.SA.FinalizeAuthorization2(ctx, &sapb.FinalizeAuthorizationRequest{ - Id: order.V2Authorizations[0], - Status: string(core.StatusInvalid), - Expires: order.Expires, - Attempted: string(core.ChallengeTypeDNS01), - AttemptedAt: timestamppb.New(ra.clk.Now()), + // Now a new order for the same names should not reuse the first one. + new, err := ra.NewOrder(context.Background(), &rapb.NewOrderRequest{ + RegistrationID: Registration.Id, + DnsNames: []string{"a.com", "b.com"}, }) - test.AssertNotError(t, err, "FinalizeAuthorization2 failed") + test.AssertNotError(t, err, "creating test order") + test.AssertNotEquals(t, new.Id, extant.Id) +} - // The order associated with the authz should now be invalid - updatedOrder, err := ra.SA.GetOrder(ctx, &sapb.OrderRequest{Id: order.Id}) - test.AssertNotError(t, err, "Error getting order to check status") - test.AssertEquals(t, updatedOrder.Status, "invalid") +// TestNewOrder_OrderReuse_Invalid tests that invalid orders are not reused. +// This is not simply a test case in TestNewOrder_OrderReuse because it has +// side effects. +func TestNewOrder_OrderReuse_Invalid(t *testing.T) { + _, sa, ra, _, _, cleanUp := initAuthorities(t) + defer cleanUp() - // Create a second order for the same names/regID - secondOrder, err := ra.NewOrder(ctx, orderReq) - // It shouldn't fail - test.AssertNotError(t, err, "Adding an initial order for regA failed") - // It should have a different ID than the first now-invalid order - test.AssertNotEquals(t, secondOrder.Id, order.Id) - // It should be status pending - test.AssertEquals(t, secondOrder.Status, "pending") - test.AssertEquals(t, numAuthorizations(secondOrder), 1) - // It should have a different authorization than the first order's now-invalid authorization - test.AssertNotEquals(t, secondOrder.V2Authorizations[0], order.V2Authorizations[0]) + // Create an initial order. + extant, err := ra.NewOrder(context.Background(), &rapb.NewOrderRequest{ + RegistrationID: Registration.Id, + DnsNames: []string{"a.com", "b.com"}, + }) + test.AssertNotError(t, err, "creating test order") + + // Transition the original order to status invalid by invalidating one of its + // authorizations. + _, err = sa.DeactivateAuthorization2(context.Background(), &sapb.AuthorizationID2{ + Id: extant.V2Authorizations[0], + }) + test.AssertNotError(t, err, "deactivating test authorization") + + // Now a new order for the same names should not reuse the first one. + new, err := ra.NewOrder(context.Background(), &rapb.NewOrderRequest{ + RegistrationID: Registration.Id, + DnsNames: []string{"a.com", "b.com"}, + }) + test.AssertNotError(t, err, "creating test order") + test.AssertNotEquals(t, new.Id, extant.Id) +} + +func TestNewOrder_AuthzReuse(t *testing.T) { + _, sa, ra, _, fc, cleanUp := initAuthorities(t) + defer cleanUp() + + // Create three initial authzs by creating an initial order, then updating + // the individual authz statuses. + const ( + pending = "a-pending.com" + valid = "b-valid.com" + invalid = "c-invalid.com" + ) + extant, err := ra.NewOrder(context.Background(), &rapb.NewOrderRequest{ + RegistrationID: Registration.Id, + DnsNames: []string{pending, valid, invalid}, + }) + test.AssertNotError(t, err, "creating test order") + extantAuthzs := map[string]int64{ + // Take advantage of the fact that authz IDs are returned in the same order + // as the lexicographically-sorted identifiers. + pending: extant.V2Authorizations[0], + valid: extant.V2Authorizations[1], + invalid: extant.V2Authorizations[2], + } + _, err = sa.FinalizeAuthorization2(context.Background(), &sapb.FinalizeAuthorizationRequest{ + Id: extantAuthzs[valid], + Status: string(core.StatusValid), + Attempted: "hello", + Expires: timestamppb.New(fc.Now().Add(48 * time.Hour)), + }) + test.AssertNotError(t, err, "marking test authz as valid") + _, err = sa.DeactivateAuthorization2(context.Background(), &sapb.AuthorizationID2{ + Id: extantAuthzs[invalid], + }) + test.AssertNotError(t, err, "marking test authz as invalid") + + // Create a second registration to reference later. + acctKeyB, err := AccountKeyB.MarshalJSON() + test.AssertNotError(t, err, "failed to marshal account key") + input := &corepb.Registration{Key: acctKeyB} + secondReg, err := ra.NewRegistration(context.Background(), input) + test.AssertNotError(t, err, "Error creating a second test registration") + + testCases := []struct { + Name string + RegistrationID int64 + DnsName string + ExpectReuse bool + }{ + { + Name: "Reuse pending authz", + RegistrationID: Registration.Id, + DnsName: pending, + ExpectReuse: true, // TODO(#7715): Invert this. + }, + { + Name: "Reuse valid authz", + RegistrationID: Registration.Id, + DnsName: valid, + ExpectReuse: true, + }, + { + Name: "Don't reuse invalid authz", + RegistrationID: Registration.Id, + DnsName: invalid, + ExpectReuse: false, + }, + { + Name: "Don't reuse valid authz from other acct", + RegistrationID: secondReg.Id, + DnsName: valid, + ExpectReuse: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.Name, func(t *testing.T) { + new, err := ra.NewOrder(context.Background(), &rapb.NewOrderRequest{ + RegistrationID: tc.RegistrationID, + DnsNames: []string{tc.DnsName}, + }) + test.AssertNotError(t, err, "creating test order") + test.AssertNotEquals(t, new.Id, extant.Id) + + if tc.ExpectReuse { + test.AssertEquals(t, new.V2Authorizations[0], extantAuthzs[tc.DnsName]) + } else { + test.AssertNotEquals(t, new.V2Authorizations[0], extantAuthzs[tc.DnsName]) + } + }) + } +} + +// TestNewOrder_AuthzReuse_NoPending tests that authz reuse doesn't reuse +// pending authzs when a feature flag is set. +// 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_AuthzReuse_NoPending(t *testing.T) { + // TODO(#7715): Integrate these cases into TestNewOrder_AuthzReuse. + _, _, ra, _, _, cleanUp := initAuthorities(t) + defer cleanUp() + + features.Set(features.Config{NoPendingAuthzReuse: true}) + defer features.Reset() + + // Create an initial order and two pending authzs. + extant, err := ra.NewOrder(context.Background(), &rapb.NewOrderRequest{ + RegistrationID: Registration.Id, + DnsNames: []string{"a.com", "b.com"}, + }) + test.AssertNotError(t, err, "creating test order") + + // With the feature flag enabled, creating a new order for one of these names + // should not reuse the existing pending authz. + new, err := ra.NewOrder(context.Background(), &rapb.NewOrderRequest{ + RegistrationID: Registration.Id, + DnsNames: []string{"a.com"}, + }) + test.AssertNotError(t, err, "creating test order") + test.AssertNotEquals(t, new.Id, extant.Id) + test.AssertNotEquals(t, new.V2Authorizations[0], extant.V2Authorizations[0]) } // mockSACountPendingFails has a CountPendingAuthorizations2 implementation @@ -2530,11 +2639,11 @@ func (msa *mockSAWithAuthzs) GetOrderForNames(ctx context.Context, req *sapb.Get return nil, berrors.NotFoundError("no such order") } -// GetAuthorizations2 returns a _bizarre_ authorization for "*.zombo.com" that +// GetValidAuthorizations2 returns a _bizarre_ authorization for "*.zombo.com" that // was validated by HTTP-01. This should never happen in real life since the // name is a wildcard. We use this mock to test that we reject this bizarre // situation correctly. -func (msa *mockSAWithAuthzs) GetAuthorizations2(ctx context.Context, req *sapb.GetAuthorizationsRequest, _ ...grpc.CallOption) (*sapb.Authorizations, error) { +func (msa *mockSAWithAuthzs) GetValidAuthorizations2(ctx context.Context, req *sapb.GetValidAuthorizationsRequest, _ ...grpc.CallOption) (*sapb.Authorizations, error) { resp := &sapb.Authorizations{} for _, v := range msa.authzs { authzPB, err := bgrpc.AuthzToPB(*v) @@ -2546,6 +2655,14 @@ func (msa *mockSAWithAuthzs) GetAuthorizations2(ctx context.Context, req *sapb.G return resp, nil } +func (msa *mockSAWithAuthzs) GetAuthorizations2(ctx context.Context, req *sapb.GetAuthorizationsRequest, _ ...grpc.CallOption) (*sapb.Authorizations, error) { + return msa.GetValidAuthorizations2(ctx, &sapb.GetValidAuthorizationsRequest{ + RegistrationID: req.RegistrationID, + DnsNames: req.DnsNames, + ValidUntil: req.ValidUntil, + }) +} + func (msa *mockSAWithAuthzs) GetAuthorization2(ctx context.Context, req *sapb.AuthorizationID2, _ ...grpc.CallOption) (*corepb.Authorization, error) { for _, authz := range msa.authzs { if authz.ID == fmt.Sprintf("%d", req.Id) { @@ -2663,7 +2780,6 @@ func TestNewOrderAuthzReuseSafety(t *testing.T) { func TestNewOrderWildcard(t *testing.T) { _, _, ra, _, _, cleanUp := initAuthorities(t) defer cleanUp() - ra.orderLifetime = time.Hour orderNames := []string{"example.com", "*.welcome.zombo.com"} wildcardOrderRequest := &rapb.NewOrderRequest{ @@ -2894,9 +3010,8 @@ func TestNewOrderExpiry(t *testing.T) { } func TestFinalizeOrder(t *testing.T) { - _, sa, ra, _, fc, cleanUp := initAuthorities(t) + _, sa, ra, _, _, cleanUp := initAuthorities(t) defer cleanUp() - ra.orderLifetime = time.Hour // Create one finalized authorization for not-example.com and one finalized // authorization for www.not-example.org @@ -2941,7 +3056,7 @@ func TestFinalizeOrder(t *testing.T) { Subject: pkix.Name{CommonName: "not-example.com"}, DNSNames: []string{"not-example.com", "www.not-example.com"}, PublicKey: testKey.Public(), - NotBefore: fc.Now(), + NotBefore: now, BasicConstraintsValid: true, ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageClientAuth}, } @@ -3182,7 +3297,6 @@ func TestFinalizeOrder(t *testing.T) { func TestFinalizeOrderWithMixedSANAndCN(t *testing.T) { _, sa, ra, _, _, cleanUp := initAuthorities(t) defer cleanUp() - ra.orderLifetime = time.Hour // Pick an expiry in the future now := ra.clk.Now() @@ -3398,12 +3512,9 @@ func TestIssueCertificateAuditLog(t *testing.T) { _, sa, ra, _, _, cleanUp := initAuthorities(t) defer cleanUp() - // Set up order and authz expiries - ra.orderLifetime = 24 * time.Hour - exp := ra.clk.Now().Add(24 * time.Hour) - // Make some valid authorizations for some names using different challenge types names := []string{"not-example.com", "www.not-example.com", "still.not-example.com", "definitely.not-example.com"} + exp := ra.clk.Now().Add(ra.orderLifetime) challs := []core.AcmeChallenge{core.ChallengeTypeHTTP01, core.ChallengeTypeDNS01, core.ChallengeTypeHTTP01, core.ChallengeTypeDNS01} var authzIDs []int64 for i, name := range names { @@ -3521,10 +3632,6 @@ func TestIssueCertificateCAACheckLog(t *testing.T) { _, sa, ra, _, fc, cleanUp := initAuthorities(t) defer cleanUp() - // Set up order and authz expiries. - ra.orderLifetime = 24 * time.Hour - ra.authorizationLifetime = 15 * time.Hour - exp := fc.Now().Add(24 * time.Hour) recent := fc.Now().Add(-1 * time.Hour) older := fc.Now().Add(-8 * time.Hour) @@ -3830,11 +3937,9 @@ func TestIssueCertificateInnerErrs(t *testing.T) { _, sa, ra, _, _, cleanUp := initAuthorities(t) defer cleanUp() - ra.orderLifetime = 24 * time.Hour - exp := ra.clk.Now().Add(24 * time.Hour) - // Make some valid authorizations for some names names := []string{"not-example.com", "www.not-example.com", "still.not-example.com", "definitely.not-example.com"} + exp := ra.clk.Now().Add(ra.orderLifetime) var authzIDs []int64 for _, name := range names { authzIDs = append(authzIDs, createFinalizedAuthorization(t, sa, name, exp, core.ChallengeTypeHTTP01, ra.clk.Now())) @@ -3999,11 +4104,9 @@ func TestIssueCertificateOuter(t *testing.T) { _, sa, ra, _, fc, cleanup := initAuthorities(t) defer cleanup() - ra.orderLifetime = 24 * time.Hour - exp := ra.clk.Now().Add(24 * time.Hour) - // Make some valid authorizations for some names names := []string{"not-example.com", "www.not-example.com", "still.not-example.com", "definitely.not-example.com"} + exp := ra.clk.Now().Add(ra.orderLifetime) var authzIDs []int64 for _, name := range names { authzIDs = append(authzIDs, createFinalizedAuthorization(t, sa, name, exp, core.ChallengeTypeHTTP01, ra.clk.Now())) @@ -4647,8 +4750,6 @@ func TestNewOrderRateLimitingExempt(t *testing.T) { _, _, ra, _, _, cleanUp := initAuthorities(t) defer cleanUp() - ra.orderLifetime = 5 * 24 * time.Hour - // Set up a rate limit policy that allows 1 order every 5 minutes. rateLimitDuration := 5 * time.Minute ra.rlPolicies = &dummyRateLimitConfig{ diff --git a/sa/database.go b/sa/database.go index 9d83875802e..5b516d1ebb5 100644 --- a/sa/database.go +++ b/sa/database.go @@ -13,7 +13,6 @@ import ( "github.com/letsencrypt/boulder/cmd" "github.com/letsencrypt/boulder/core" boulderDB "github.com/letsencrypt/boulder/db" - "github.com/letsencrypt/boulder/features" blog "github.com/letsencrypt/boulder/log" ) @@ -273,11 +272,8 @@ 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") - if features.Get().MultipleCertificateProfiles { - dbMap.AddTableWithName(orderModelv2{}, "orders").SetKeys(true, "ID") - } else { - dbMap.AddTableWithName(orderModelv1{}, "orders").SetKeys(true, "ID") - } + dbMap.AddTableWithName(orderModelv2{}, "orders").SetKeys(true, "ID") + dbMap.AddTableWithName(orderModelv1{}, "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") diff --git a/sa/saro.go b/sa/saro.go index d3605322368..c5a6dbf4364 100644 --- a/sa/saro.go +++ b/sa/saro.go @@ -722,6 +722,8 @@ func authzModelMapToPB(m map[string]authzModel) (*sapb.Authorizations, error) { // the given account for all given identifiers. If both a valid and pending // authorization exist only the valid one will be returned. Currently only dns // identifiers are supported. +// +// Deprecated: Use GetValidAuthorizations2, as we stop pending authz reuse. func (ssa *SQLStorageAuthorityRO) GetAuthorizations2(ctx context.Context, req *sapb.GetAuthorizationsRequest) (*sapb.Authorizations, error) { if core.IsAnyNilOrZero(req, req.RegistrationID, req.DnsNames, req.ValidUntil) { return nil, errIncompleteRequest diff --git a/test/config-next/ra.json b/test/config-next/ra.json index cacda398d5f..8ed02cfb8c1 100644 --- a/test/config-next/ra.json +++ b/test/config-next/ra.json @@ -128,7 +128,8 @@ "features": { "AsyncFinalize": true, "UseKvLimitsForNewOrder": true, - "AutomaticallyPauseZombieClients": true + "AutomaticallyPauseZombieClients": true, + "NoPendingAuthzReuse": true }, "ctLogs": { "stagger": "500ms",