Rate limit deactivating pending authzs (#7835)

When a client deactivates a pending authorization, count that towards
their FailedAuthorizationsPerDomainPerAccount and
FailedAuthorizationsForPausingPerDomainPerAccount rate limits. This
should help curb the few clients which constantly create new orders and
authzs, deactivate those pending authzs preventing reuse of them or
their orders, and then rinse and repeat.

Fixes #7834

Author: aarongable

ra/ra.go

@@ -2425,7 +2425,7 @@ func (ra *RegistrationAuthorityImpl) DeactivateRegistration(ctx context.Context,
 
 // DeactivateAuthorization deactivates a currently valid authorization
 func (ra *RegistrationAuthorityImpl) DeactivateAuthorization(ctx context.Context, req *corepb.Authorization) (*emptypb.Empty, error) {
-	if req == nil || req.Id == "" || req.Status == "" {
+	if core.IsAnyNilOrZero(req, req.Id, req.Status, req.RegistrationID) {
 		return nil, errIncompleteGRPCRequest
 	}
 	authzID, err := strconv.ParseInt(req.Id, 10, 64)
@@ -2435,6 +2435,17 @@ func (ra *RegistrationAuthorityImpl) DeactivateAuthorization(ctx context.Context
 	if _, err := ra.SA.DeactivateAuthorization2(ctx, &sapb.AuthorizationID2{Id: authzID}); err != nil {
 		return nil, err
 	}
+	if req.Status == string(core.StatusPending) {
+		// Some clients deactivate pending authorizations without attempting them.
+		// We're not sure exactly when this happens but it's most likely due to
+		// internal errors in the client. From our perspective this uses storage
+		// resources similar to how failed authorizations do, so we increment the
+		// failed authorizations limit.
+		err = ra.countFailedValidations(ctx, req.RegistrationID, identifier.NewDNS(req.DnsName))
+		if err != nil {
+			return nil, fmt.Errorf("failed to update rate limits: %w", err)
+		}
+	}
 	return &emptypb.Empty{}, nil
 }
 

ra/ra_test.go

@@ -865,13 +865,13 @@ func (msa *mockSAPaused) FinalizeAuthorization2(ctx context.Context, req *sapb.F
 }
 
 func TestPerformValidation_FailedValidationsTriggerPauseIdentifiersRatelimit(t *testing.T) {
-	if !strings.Contains(os.Getenv("BOULDER_CONFIG_DIR"), "test/config-next") {
-		t.Skip()
-	}
-
 	va, sa, ra, redisSrc, fc, cleanUp := initAuthorities(t)
 	defer cleanUp()
 
+	if ra.limiter == nil {
+		t.Skip("no redis limiter configured")
+	}
+
 	features.Set(features.Config{AutomaticallyPauseZombieClients: true})
 	defer features.Reset()
 
@@ -988,13 +988,13 @@ func TestPerformValidation_FailedValidationsTriggerPauseIdentifiersRatelimit(t *
 }
 
 func TestPerformValidation_FailedThenSuccessfulValidationResetsPauseIdentifiersRatelimit(t *testing.T) {
-	if !strings.Contains(os.Getenv("BOULDER_CONFIG_DIR"), "test/config-next") {
-		t.Skip()
-	}
-
 	va, sa, ra, redisSrc, fc, cleanUp := initAuthorities(t)
 	defer cleanUp()
 
+	if ra.limiter == nil {
+		t.Skip("no redis limiter configured")
+	}
+
 	features.Set(features.Config{AutomaticallyPauseZombieClients: true})
 	defer features.Reset()
 
@@ -1778,6 +1778,74 @@ func TestDeactivateAuthorization(t *testing.T) {
 	test.AssertEquals(t, deact.Status, string(core.StatusDeactivated))
 }
 
+type mockSARecordingPauses struct {
+	sapb.StorageAuthorityClient
+	recv *sapb.PauseRequest
+}
+
+func (sa *mockSARecordingPauses) PauseIdentifiers(ctx context.Context, req *sapb.PauseRequest, _ ...grpc.CallOption) (*sapb.PauseIdentifiersResponse, error) {
+	sa.recv = req
+	return &sapb.PauseIdentifiersResponse{Paused: int64(len(req.Identifiers))}, nil
+}
+
+func (sa *mockSARecordingPauses) DeactivateAuthorization2(_ context.Context, _ *sapb.AuthorizationID2, _ ...grpc.CallOption) (*emptypb.Empty, error) {
+	return nil, nil
+}
+
+func TestDeactivateAuthorization_Pausing(t *testing.T) {
+	_, _, ra, _, _, cleanUp := initAuthorities(t)
+	defer cleanUp()
+
+	if ra.limiter == nil {
+		t.Skip("no redis limiter configured")
+	}
+
+	msa := mockSARecordingPauses{}
+	ra.SA = &msa
+
+	features.Set(features.Config{AutomaticallyPauseZombieClients: true})
+	defer features.Reset()
+
+	// Override the default ratelimits to only allow one failed validation.
+	txnBuilder, err := ratelimits.NewTransactionBuilder("testdata/one-failed-validation-before-pausing.yml", "")
+	test.AssertNotError(t, err, "making transaction composer")
+	ra.txnBuilder = txnBuilder
+
+	// The first deactivation of a pending authz should work and nothing should
+	// get paused.
+	_, err = ra.DeactivateAuthorization(ctx, &corepb.Authorization{
+		Id:             "1",
+		RegistrationID: 1,
+		DnsName:        "example.com",
+		Status:         string(core.StatusPending),
+	})
+	test.AssertNotError(t, err, "mock deactivation should work")
+	test.AssertBoxedNil(t, msa.recv, "shouldn't be a pause request yet")
+
+	// Deactivating a valid authz shouldn't increment any limits or pause anything.
+	_, err = ra.DeactivateAuthorization(ctx, &corepb.Authorization{
+		Id:             "2",
+		RegistrationID: 1,
+		DnsName:        "example.com",
+		Status:         string(core.StatusValid),
+	})
+	test.AssertNotError(t, err, "mock deactivation should work")
+	test.AssertBoxedNil(t, msa.recv, "deactivating valid authz should never pause")
+
+	// Deactivating a second pending authz should surpass the limit and result
+	// in a pause request.
+	_, err = ra.DeactivateAuthorization(ctx, &corepb.Authorization{
+		Id:             "3",
+		RegistrationID: 1,
+		DnsName:        "example.com",
+		Status:         string(core.StatusPending),
+	})
+	test.AssertNotError(t, err, "mock deactivation should work")
+	test.AssertNotNil(t, msa.recv, "should have recorded a pause request")
+	test.AssertEquals(t, msa.recv.RegistrationID, int64(1))
+	test.AssertEquals(t, msa.recv.Identifiers[0].Value, "example.com")
+}
+
 func TestDeactivateRegistration(t *testing.T) {
 	_, _, ra, _, _, cleanUp := initAuthorities(t)
 	defer cleanUp()