Verify unblockables are truly unblockable (#2381)

* Verify unblockables are truly unblockable

Unblockable domains may become blockable due to deregistration or
removal from the reserved list. The BSA refresh job is responsible
for removing them from the database. This PR verifies that the refreshes
are correct.

Note that recent changes since last refresh are not reflected in the
result, and inconsistency due to recent deregistrations are ignored.
Changes in reserved status or IDN validity are not timestamped,
therefore we cannot ignore recent inconsistencies. However, these
changes are rare.

* Addressing code review

* Addressing code review
This commit is contained in:
Weimin Yu 2024-03-20 14:52:17 -04:00 committed by GitHub
parent 5c62dc78ba
commit de3af34b66
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 172 additions and 2 deletions

View file

@ -16,27 +16,42 @@ package google.registry.bsa;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Throwables.getStackTraceAsString;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static google.registry.bsa.BsaTransactions.bsaQuery;
import static google.registry.bsa.ReservedDomainsUtils.isReservedDomain;
import static google.registry.bsa.persistence.Queries.batchReadBsaLabelText;
import static google.registry.persistence.transaction.TransactionManagerFactory.replicaTm;
import static google.registry.request.Action.Method.GET;
import static google.registry.request.Action.Method.POST;
import static javax.servlet.http.HttpServletResponse.SC_OK;
import com.google.common.base.Joiner;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.Sets;
import com.google.common.collect.Sets.SetView;
import com.google.common.flogger.FluentLogger;
import com.google.common.net.InternetDomainName;
import google.registry.bsa.api.UnblockableDomain;
import google.registry.bsa.api.UnblockableDomain.Reason;
import google.registry.bsa.persistence.DownloadScheduler;
import google.registry.bsa.persistence.Queries;
import google.registry.config.RegistryConfig.Config;
import google.registry.model.ForeignKeyUtils;
import google.registry.model.domain.Domain;
import google.registry.persistence.VKey;
import google.registry.request.Action;
import google.registry.request.Response;
import google.registry.request.auth.Auth;
import google.registry.util.Clock;
import java.util.Objects;
import java.util.Optional;
import java.util.stream.Stream;
import javax.inject.Inject;
import org.joda.time.DateTime;
import org.joda.time.Duration;
/** Validates the BSA data in the database against the most recent block lists. */
@Action(
@ -53,6 +68,8 @@ public class BsaValidateAction implements Runnable {
private final IdnChecker idnChecker;
private final BsaEmailSender emailSender;
private final int transactionBatchSize;
private final Duration maxStaleness;
private final Clock clock;
private final BsaLock bsaLock;
private final Response response;
@ -62,12 +79,16 @@ public class BsaValidateAction implements Runnable {
IdnChecker idnChecker,
BsaEmailSender emailSender,
@Config("bsaTxnBatchSize") int transactionBatchSize,
@Config("bsaValidationMaxStaleness") Duration maxStaleness,
Clock clock,
BsaLock bsaLock,
Response response) {
this.gcsClient = gcsClient;
this.idnChecker = idnChecker;
this.emailSender = emailSender;
this.transactionBatchSize = transactionBatchSize;
this.maxStaleness = maxStaleness;
this.clock = clock;
this.bsaLock = bsaLock;
this.response = response;
}
@ -103,6 +124,7 @@ public class BsaValidateAction implements Runnable {
ImmutableList.Builder<String> errorsBuilder = new ImmutableList.Builder<>();
errorsBuilder.addAll(checkBsaLabels(downloadJobName.get()));
errorsBuilder.addAll(checkUnblockableDomains());
ImmutableList<String> errors = errorsBuilder.build();
@ -150,6 +172,66 @@ public class BsaValidateAction implements Runnable {
return errors.build();
}
ImmutableList<String> checkUnblockableDomains() {
ImmutableList.Builder<String> errors = new ImmutableList.Builder<>();
Optional<UnblockableDomain> lastRead = Optional.empty();
ImmutableList<UnblockableDomain> batch;
do {
batch = Queries.batchReadUnblockableDomains(lastRead, transactionBatchSize);
ImmutableMap<String, VKey<Domain>> activeDomains =
ForeignKeyUtils.load(
Domain.class,
batch.stream().map(UnblockableDomain::domainName).collect(toImmutableList()),
clock.nowUtc());
for (var unblockable : batch) {
verifyDomainStillUnblockableWithReason(unblockable, activeDomains).ifPresent(errors::add);
}
if (!batch.isEmpty()) {
lastRead = Optional.of(Iterables.getLast(batch));
}
} while (batch.size() == transactionBatchSize);
return errors.build();
}
Optional<String> verifyDomainStillUnblockableWithReason(
UnblockableDomain domain, ImmutableMap<String, VKey<Domain>> activeDomains) {
DateTime now = clock.nowUtc();
boolean isRegistered = activeDomains.containsKey(domain.domainName());
boolean isReserved = isReservedDomain(domain.domainName(), now);
InternetDomainName domainName = InternetDomainName.from(domain.domainName());
boolean isInvalid = idnChecker.getAllValidIdns(domainName.parts().get(0)).isEmpty();
Reason expectedReason =
isRegistered
? Reason.REGISTERED
: (isReserved ? Reason.RESERVED : (isInvalid ? Reason.INVALID : null));
if (Objects.equals(expectedReason, domain.reason())) {
return Optional.empty();
}
if (isRegistered || domain.reason().equals(Reason.REGISTERED)) {
if (isStalenessAllowed(isRegistered, activeDomains.get(domain.domainName()))) {
return Optional.empty();
}
}
return Optional.of(
String.format(
"%s: should be %s, found %s",
domain.domainName(),
expectedReason != null ? expectedReason.name() : "BLOCKABLE",
domain.reason()));
}
boolean isStalenessAllowed(boolean isNewDomain, VKey<Domain> domainVKey) {
Domain domain = bsaQuery(() -> replicaTm().loadByKey(domainVKey));
var now = clock.nowUtc();
if (isNewDomain) {
return domain.getCreationTime().plus(maxStaleness).isAfter(now);
} else {
return domain.getDeletionTime().isBefore(now)
&& domain.getDeletionTime().plus(maxStaleness).isAfter(now);
}
}
/** Returns unique labels across all block lists in the download specified by {@code jobName}. */
ImmutableSet<String> fetchDownloadedLabels(String jobName) {
ImmutableSet.Builder<String> labelsBuilder = new ImmutableSet.Builder<>();
@ -171,10 +253,11 @@ public class BsaValidateAction implements Runnable {
Optional<String> lastRead = Optional.empty();
do {
batch = batchReadBsaLabelText(lastRead, batchSize);
batch.forEach(labelsBuilder::add);
labelsBuilder.addAll(batch);
if (!batch.isEmpty()) {
lastRead = Optional.of(Iterables.getLast(batch));
}
} while (batch.size() == batchSize);
return labelsBuilder.build();
}

View file

@ -15,6 +15,7 @@
package google.registry.bsa.persistence;
import static com.google.common.base.Verify.verify;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static google.registry.bsa.BsaStringUtils.DOMAIN_SPLITTER;
import static google.registry.bsa.BsaTransactions.bsaQuery;
import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
@ -22,6 +23,7 @@ import static google.registry.persistence.transaction.TransactionManagerFactory.
import com.google.common.collect.ImmutableCollection;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import google.registry.bsa.api.UnblockableDomain;
import google.registry.model.CreateAutoTimestamp;
import java.util.List;
import java.util.Optional;
@ -45,7 +47,6 @@ public final class Queries {
public static ImmutableList<String> batchReadBsaLabelText(
Optional<String> lastRead, int batchSize) {
return ImmutableList.copyOf(
bsaQuery(
() ->
@ -58,6 +59,13 @@ public final class Queries {
.getResultList()));
}
public static ImmutableList<UnblockableDomain> batchReadUnblockableDomains(
Optional<UnblockableDomain> lastRead, int batchSize) {
return batchReadUnblockables(lastRead.map(BsaUnblockableDomain::of), batchSize).stream()
.map(BsaUnblockableDomain::toUnblockableDomain)
.collect(toImmutableList());
}
static Stream<BsaUnblockableDomain> queryBsaUnblockableDomainByLabels(
ImmutableCollection<String> labels) {
return ((Stream<?>)

View file

@ -1488,6 +1488,12 @@ public final class RegistryConfig {
return Duration.standardSeconds(config.bsa.domainCreateTxnCommitTimeLagSeconds);
}
@Provides
@Config("bsaValidationMaxStaleness")
public static Duration provideBsaValidationMaxStaleness(RegistryConfigSettings config) {
return Duration.standardSeconds(config.bsa.bsaValidationMaxStalenessSeconds);
}
@Provides
@Config("bsaAuthUrl")
public static String provideBsaAuthUrl(RegistryConfigSettings config) {

View file

@ -275,6 +275,7 @@ public class RegistryConfigSettings {
public int bsaMaxNopIntervalHours;
public int bsaTxnBatchSize;
public int domainCreateTxnCommitTimeLagSeconds;
public int bsaValidationMaxStalenessSeconds;
public String authUrl;
public int authTokenExpirySeconds;
public Map<String, String> dataUrls;

View file

@ -640,6 +640,9 @@ bsa:
# Number of entities (labels and unblockable domains) to process in a single
# DB transaction.
bsaTxnBatchSize: 1000
# Used by `BsaValidateAction`: ignore inconsistencies caused by recent events
# in the past. This is roughly equal to two `BsaRefreshAction` runs.
bsaValidationMaxStalenessSeconds: 3600
# Http endpoint for acquiring Auth tokens.
authUrl: "https://"

View file

@ -16,7 +16,12 @@ package google.registry.bsa;
import static com.google.common.base.Throwables.getStackTraceAsString;
import static com.google.common.truth.Truth.assertThat;
import static google.registry.bsa.persistence.BsaTestingUtils.persistBsaLabel;
import static google.registry.bsa.persistence.BsaTestingUtils.persistDownloadSchedule;
import static google.registry.bsa.persistence.BsaTestingUtils.persistUnblockableDomain;
import static google.registry.testing.DatabaseHelper.createTld;
import static google.registry.testing.DatabaseHelper.persistActiveDomain;
import static google.registry.testing.DatabaseHelper.persistDeletedDomain;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.startsWith;
@ -31,9 +36,12 @@ import com.google.cloud.storage.contrib.nio.testing.LocalStorageHelper;
import com.google.common.base.Joiner;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import google.registry.bsa.api.UnblockableDomain;
import google.registry.bsa.api.UnblockableDomain.Reason;
import google.registry.bsa.persistence.BsaTestingUtils;
import google.registry.gcs.GcsUtils;
import google.registry.groups.GmailClient;
import google.registry.model.domain.Domain;
import google.registry.persistence.transaction.JpaTestExtensions;
import google.registry.persistence.transaction.JpaTestExtensions.JpaIntegrationWithCoverageExtension;
import google.registry.request.Response;
@ -43,6 +51,7 @@ import google.registry.util.EmailMessage;
import java.util.concurrent.Callable;
import javax.mail.internet.InternetAddress;
import org.joda.time.DateTime;
import org.joda.time.Duration;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
@ -58,6 +67,8 @@ public class BsaValidateActionTest {
private static final String DOWNLOAD_JOB_NAME = "job";
private static final Duration MAX_STALENESS = Duration.standardMinutes(1);
FakeClock fakeClock = new FakeClock(DateTime.parse("2023-11-09T02:08:57.880Z"));
@RegisterExtension
@ -90,8 +101,11 @@ public class BsaValidateActionTest {
idnChecker,
new BsaEmailSender(gmailClient, emailRecipient),
/* transactionBatchSize= */ 500,
MAX_STALENESS,
fakeClock,
bsaLock,
response);
createTld("app");
}
static void createBlockList(GcsClient gcsClient, BlockListType blockListType, String content)
@ -214,6 +228,61 @@ public class BsaValidateActionTest {
assertThat(allErrors).contains("Found 1 unexpected labels in the DB. Examples: [test3]");
}
@Test
void isStalenessAllowed_newDomain_allowed() {
persistBsaLabel("label");
Domain domain = persistActiveDomain("label.app", fakeClock.nowUtc());
fakeClock.advanceBy(MAX_STALENESS.minus(Duration.standardSeconds(1)));
assertThat(action.isStalenessAllowed(true, domain.createVKey())).isTrue();
}
@Test
void isStalenessAllowed_newDomain_notAllowed() {
persistBsaLabel("label");
Domain domain = persistActiveDomain("label.app", fakeClock.nowUtc());
fakeClock.advanceBy(MAX_STALENESS);
assertThat(action.isStalenessAllowed(true, domain.createVKey())).isFalse();
}
@Test
void isStalenessAllowed_deletedDomain_allowed() {
persistBsaLabel("label");
Domain domain = persistDeletedDomain("label.app", fakeClock.nowUtc());
fakeClock.advanceBy(MAX_STALENESS.minus(Duration.standardSeconds(1)));
assertThat(action.isStalenessAllowed(false, domain.createVKey())).isTrue();
}
@Test
void isStalenessAllowed_deletedDomain_notAllowed() {
persistBsaLabel("label");
Domain domain = persistDeletedDomain("label.app", fakeClock.nowUtc());
fakeClock.advanceBy(MAX_STALENESS);
assertThat(action.isStalenessAllowed(false, domain.createVKey())).isFalse();
}
@Test
void checkUnblockableDomain_noError() {
createTld("app");
persistActiveDomain("label.app");
persistBsaLabel("label");
persistUnblockableDomain(UnblockableDomain.of("label", "app", Reason.REGISTERED));
when(idnChecker.getAllValidIdns(anyString())).thenReturn(ImmutableSet.of(IdnTableEnum.JA));
assertThat(action.checkUnblockableDomains()).isEmpty();
}
@Test
void checkUnblockableDomain_error() {
createTld("app");
persistActiveDomain("label.app");
persistBsaLabel("label");
persistUnblockableDomain(UnblockableDomain.of("label", "app", Reason.RESERVED));
when(idnChecker.getAllValidIdns(anyString())).thenReturn(ImmutableSet.of(IdnTableEnum.JA));
assertThat(action.checkUnblockableDomains())
.containsExactly("label.app: should be REGISTERED, found RESERVED");
}
@Test
void notificationSent_cannotAcquireLock() {
when(bsaLock.executeWithLock(any())).thenReturn(false);