diff --git a/ra/ra.go b/ra/ra.go index 7564a1395ae..0aee052816c 100644 --- a/ra/ra.go +++ b/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 } diff --git a/ra/ra_test.go b/ra/ra_test.go index d1c98177bc1..594edc17ec4 100644 --- a/ra/ra_test.go +++ b/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()