diff --git a/core/src/main/java/google/registry/flows/host/HostCreateFlow.java b/core/src/main/java/google/registry/flows/host/HostCreateFlow.java index 28b946575e5..04c21e6e19e 100644 --- a/core/src/main/java/google/registry/flows/host/HostCreateFlow.java +++ b/core/src/main/java/google/registry/flows/host/HostCreateFlow.java @@ -20,6 +20,7 @@ import static google.registry.flows.host.HostFlowUtils.lookupSuperordinateDomain; import static google.registry.flows.host.HostFlowUtils.validateHostName; import static google.registry.flows.host.HostFlowUtils.verifySuperordinateDomainNotInPendingDelete; +import static google.registry.flows.host.HostFlowUtils.verifySuperordinateDomainNotServerUpdateProhibited; import static google.registry.flows.host.HostFlowUtils.verifySuperordinateDomainOwnership; import static google.registry.model.EppResourceUtils.createRepoId; import static google.registry.model.reporting.HistoryEntry.Type.HOST_CREATE; @@ -73,6 +74,7 @@ * @error {@link HostFlowUtils.HostNameNotPunyCodedException} * @error {@link HostFlowUtils.SuperordinateDomainDoesNotExistException} * @error {@link HostFlowUtils.SuperordinateDomainInPendingDeleteException} + * @error {@link HostFlowUtils.SuperordinateDomainServerUpdateProhibitedException} * @error {@link SubordinateHostMustHaveIpException} * @error {@link UnexpectedExternalHostIpException} */ @@ -107,6 +109,7 @@ public EppResponse run() throws EppException { Optional superordinateDomain = lookupSuperordinateDomain(validateHostName(targetId), now); verifySuperordinateDomainNotInPendingDelete(superordinateDomain.orElse(null)); + verifySuperordinateDomainNotServerUpdateProhibited(superordinateDomain.orElse(null)); verifySuperordinateDomainOwnership(registrarId, superordinateDomain.orElse(null)); boolean willBeSubordinate = superordinateDomain.isPresent(); boolean hasIpAddresses = !isNullOrEmpty(command.getInetAddresses()); diff --git a/core/src/main/java/google/registry/flows/host/HostDeleteFlow.java b/core/src/main/java/google/registry/flows/host/HostDeleteFlow.java index 88348c985e8..d6764331116 100644 --- a/core/src/main/java/google/registry/flows/host/HostDeleteFlow.java +++ b/core/src/main/java/google/registry/flows/host/HostDeleteFlow.java @@ -22,6 +22,7 @@ import static google.registry.flows.ResourceFlowUtils.verifyNoDisallowedStatuses; import static google.registry.flows.ResourceFlowUtils.verifyResourceOwnership; import static google.registry.flows.host.HostFlowUtils.validateHostName; +import static google.registry.flows.host.HostFlowUtils.verifySuperordinateDomainNotServerUpdateProhibited; import static google.registry.model.eppoutput.Result.Code.SUCCESS; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; @@ -33,7 +34,7 @@ import google.registry.flows.FlowModule.TargetId; import google.registry.flows.MutatingFlow; import google.registry.flows.annotations.ReportingSpec; -import google.registry.model.EppResource; +import google.registry.model.domain.Domain; import google.registry.model.domain.metadata.MetadataExtension; import google.registry.model.eppcommon.StatusValue; import google.registry.model.eppcommon.Trid; @@ -62,6 +63,7 @@ * @error {@link HostFlowUtils.HostNameNotLowerCaseException} * @error {@link HostFlowUtils.HostNameNotNormalizedException} * @error {@link HostFlowUtils.HostNameNotPunyCodedException} + * @error {@link HostFlowUtils.SuperordinateDomainServerUpdateProhibitedException} */ @ReportingSpec(ActivityReportField.HOST_DELETE) public final class HostDeleteFlow implements MutatingFlow { @@ -90,12 +92,15 @@ public EppResponse run() throws EppException { if (!isSuperuser) { verifyNoDisallowedStatuses(existingHost, DELETE_PROHIBITED_STATUSES); // Hosts transfer with their superordinate domains, so for hosts with a superordinate domain, - // the client id, needs to be read off of it. - EppResource owningResource = - existingHost.isSubordinate() - ? tm().loadByKey(existingHost.getSuperordinateDomain()).cloneProjectedAtTime(now) - : existingHost; - verifyResourceOwnership(registrarId, owningResource); + // the client id needs to be read off of it. + if (existingHost.isSubordinate()) { + Domain superordinateDomain = + tm().loadByKey(existingHost.getSuperordinateDomain()).cloneProjectedAtTime(now); + verifyResourceOwnership(registrarId, superordinateDomain); + verifySuperordinateDomainNotServerUpdateProhibited(superordinateDomain); + } else { + verifyResourceOwnership(registrarId, existingHost); + } } Host newHost = existingHost.asBuilder().setStatusValues(null).setDeletionTime(now).build(); if (existingHost.isSubordinate()) { diff --git a/core/src/main/java/google/registry/flows/host/HostFlowUtils.java b/core/src/main/java/google/registry/flows/host/HostFlowUtils.java index 447f2176de4..f73af01bd7c 100644 --- a/core/src/main/java/google/registry/flows/host/HostFlowUtils.java +++ b/core/src/main/java/google/registry/flows/host/HostFlowUtils.java @@ -174,6 +174,31 @@ public SuperordinateDomainInPendingDeleteException() { } } + /** + * Ensure that the superordinate domain does not have {@code serverUpdateProhibited} status. + * + *

Subordinate hosts publish glue A/AAAA records in the TLD zone on behalf of their + * superordinate domain. If the superordinate domain is registry-locked (carries {@link + * StatusValue#SERVER_UPDATE_PROHIBITED}), non-superuser modifications to its subordinate hosts + * must be rejected as well; otherwise a compromised sponsoring registrar could hijack DNS for a + * locked domain by swapping the glue addresses of its in-bailiwick nameservers. + */ + static void verifySuperordinateDomainNotServerUpdateProhibited(Domain superordinateDomain) + throws EppException { + if ((superordinateDomain != null) + && superordinateDomain.getStatusValues().contains(StatusValue.SERVER_UPDATE_PROHIBITED)) { + throw new SuperordinateDomainServerUpdateProhibitedException(); + } + } + + /** Superordinate domain for this hostname has serverUpdateProhibited status. */ + static class SuperordinateDomainServerUpdateProhibitedException + extends StatusProhibitsOperationException { + public SuperordinateDomainServerUpdateProhibitedException() { + super("Superordinate domain for this hostname has serverUpdateProhibited status"); + } + } + /** Host names are limited to 253 characters. */ static class HostNameTooLongException extends ParameterValueRangeErrorException { public HostNameTooLongException() { diff --git a/core/src/main/java/google/registry/flows/host/HostUpdateFlow.java b/core/src/main/java/google/registry/flows/host/HostUpdateFlow.java index c5a2c243f26..b20ddf4cde9 100644 --- a/core/src/main/java/google/registry/flows/host/HostUpdateFlow.java +++ b/core/src/main/java/google/registry/flows/host/HostUpdateFlow.java @@ -28,6 +28,7 @@ import static google.registry.flows.host.HostFlowUtils.lookupSuperordinateDomain; import static google.registry.flows.host.HostFlowUtils.validateHostName; import static google.registry.flows.host.HostFlowUtils.verifySuperordinateDomainNotInPendingDelete; +import static google.registry.flows.host.HostFlowUtils.verifySuperordinateDomainNotServerUpdateProhibited; import static google.registry.flows.host.HostFlowUtils.verifySuperordinateDomainOwnership; import static google.registry.model.reporting.HistoryEntry.Type.HOST_UPDATE; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; @@ -98,6 +99,7 @@ * @error {@link HostFlowUtils.InvalidHostNameException} * @error {@link HostFlowUtils.SuperordinateDomainDoesNotExistException} * @error {@link HostFlowUtils.SuperordinateDomainInPendingDeleteException} + * @error {@link HostFlowUtils.SuperordinateDomainServerUpdateProhibitedException} * @error {@link CannotAddIpToExternalHostException} * @error {@link CannotRemoveSubordinateHostLastIpException} * @error {@link CannotRenameExternalHostException} @@ -152,7 +154,12 @@ public EppResponse run() throws EppException { verifySuperordinateDomainNotInPendingDelete(newSuperordinateDomain.orElse(null)); EppResource owningResource = firstNonNull(oldSuperordinateDomain, existingHost); verifyUpdateAllowed( - command, existingHost, newSuperordinateDomain.orElse(null), owningResource, isHostRename); + command, + existingHost, + oldSuperordinateDomain, + newSuperordinateDomain.orElse(null), + owningResource, + isHostRename); if (isHostRename && ForeignKeyUtils.loadKey(Host.class, newHostName, now).isPresent()) { throw new HostAlreadyExistsException(newHostName); } @@ -211,30 +218,38 @@ public EppResponse run() throws EppException { private void verifyUpdateAllowed( Update command, Host existingHost, + Domain oldSuperordinateDomain, Domain newSuperordinateDomain, EppResource owningResource, boolean isHostRename) throws EppException { - if (!isSuperuser) { - // Verify that the host belongs to this registrar, either directly or because it is currently - // subordinate to a domain owned by this registrar. - verifyResourceOwnership(registrarId, owningResource); - if (isHostRename && !existingHost.isSubordinate()) { - throw new CannotRenameExternalHostException(); - } - // Verify that the new superordinate domain belongs to this registrar. - verifySuperordinateDomainOwnership(registrarId, newSuperordinateDomain); - ImmutableSet statusesToAdd = command.getInnerAdd().getStatusValues(); - ImmutableSet statusesToRemove = command.getInnerRemove().getStatusValues(); - // If the resource is marked with clientUpdateProhibited, and this update does not clear that - // status, then the update must be disallowed. - if (existingHost.getStatusValues().contains(StatusValue.CLIENT_UPDATE_PROHIBITED) - && !statusesToRemove.contains(StatusValue.CLIENT_UPDATE_PROHIBITED)) { - throw new ResourceHasClientUpdateProhibitedException(); - } - verifyAllStatusesAreClientSettable(union(statusesToAdd, statusesToRemove)); - } verifyNoDisallowedStatuses(existingHost, DISALLOWED_STATUSES); + if (isSuperuser) { + return; + } + // Verify that the host belongs to this registrar, either directly or because it is currently + // subordinate to a domain owned by this registrar. + verifyResourceOwnership(registrarId, owningResource); + if (isHostRename && !existingHost.isSubordinate()) { + throw new CannotRenameExternalHostException(); + } + // Verify that the new superordinate domain belongs to this registrar. + verifySuperordinateDomainOwnership(registrarId, newSuperordinateDomain); + // Subordinate hosts inherit the registry-lock protection of their superordinate domain: if + // either the current or the target superordinate domain carries SERVER_UPDATE_PROHIBITED, + // non-superuser updates (including glue IP changes and renames) must be rejected so that a + // compromised sponsoring registrar cannot hijack DNS for a registry-locked domain. + verifySuperordinateDomainNotServerUpdateProhibited(oldSuperordinateDomain); + verifySuperordinateDomainNotServerUpdateProhibited(newSuperordinateDomain); + ImmutableSet statusesToAdd = command.getInnerAdd().getStatusValues(); + ImmutableSet statusesToRemove = command.getInnerRemove().getStatusValues(); + // If the resource is marked with clientUpdateProhibited, and this update does not clear that + // status, then the update must be disallowed. + if (existingHost.getStatusValues().contains(StatusValue.CLIENT_UPDATE_PROHIBITED) + && !statusesToRemove.contains(StatusValue.CLIENT_UPDATE_PROHIBITED)) { + throw new ResourceHasClientUpdateProhibitedException(); + } + verifyAllStatusesAreClientSettable(union(statusesToAdd, statusesToRemove)); } private static void verifyHasIpsIffIsExternal(Update command, Host existingHost, Host newHost) diff --git a/core/src/test/java/google/registry/flows/host/HostCreateFlowTest.java b/core/src/test/java/google/registry/flows/host/HostCreateFlowTest.java index 6ac065e64a3..5d23cbd4410 100644 --- a/core/src/test/java/google/registry/flows/host/HostCreateFlowTest.java +++ b/core/src/test/java/google/registry/flows/host/HostCreateFlowTest.java @@ -51,6 +51,7 @@ import google.registry.flows.host.HostFlowUtils.IpAddressNotRoutableException; import google.registry.flows.host.HostFlowUtils.SuperordinateDomainDoesNotExistException; import google.registry.flows.host.HostFlowUtils.SuperordinateDomainInPendingDeleteException; +import google.registry.flows.host.HostFlowUtils.SuperordinateDomainServerUpdateProhibitedException; import google.registry.model.ForeignKeyUtils; import google.registry.model.domain.Domain; import google.registry.model.eppcommon.StatusValue; @@ -413,6 +414,22 @@ void testFailure_anyLocalInetAddress_ipv4() { .marshalsToXml(); } + @Test + void testFailure_superordinateDomainIsServerUpdateProhibited() { + createTld("tld"); + persistResource( + DatabaseHelper.newDomain("example.tld") + .asBuilder() + .setStatusValues(ImmutableSet.of(StatusValue.SERVER_UPDATE_PROHIBITED)) + .build()); + setEppHostCreateInputWithIps("ns1.example.tld"); + SuperordinateDomainServerUpdateProhibitedException thrown = + assertThrows(SuperordinateDomainServerUpdateProhibitedException.class, this::runFlow); + assertThat(thrown) + .hasMessageThat() + .contains("Superordinate domain for this hostname has serverUpdateProhibited status"); + } + @Test void testIcannActivityReportField_getsLogged() throws Exception { runFlow(); diff --git a/core/src/test/java/google/registry/flows/host/HostDeleteFlowTest.java b/core/src/test/java/google/registry/flows/host/HostDeleteFlowTest.java index bb00d9a96cc..dfe92ef79cf 100644 --- a/core/src/test/java/google/registry/flows/host/HostDeleteFlowTest.java +++ b/core/src/test/java/google/registry/flows/host/HostDeleteFlowTest.java @@ -42,6 +42,7 @@ import google.registry.flows.host.HostFlowUtils.HostNameNotLowerCaseException; import google.registry.flows.host.HostFlowUtils.HostNameNotNormalizedException; import google.registry.flows.host.HostFlowUtils.HostNameNotPunyCodedException; +import google.registry.flows.host.HostFlowUtils.SuperordinateDomainServerUpdateProhibitedException; import google.registry.model.domain.Domain; import google.registry.model.domain.GracePeriod; import google.registry.model.domain.rgp.GracePeriodStatus; @@ -360,6 +361,25 @@ void testFailure_nonCanonicalHostname() { assertAboutEppExceptions().that(thrown).marshalsToXml(); } + @Test + void testFailure_superordinateDomainIsServerUpdateProhibited() { + createTld("tld"); + Domain domain = + persistResource( + DatabaseHelper.newDomain("example.tld") + .asBuilder() + .setStatusValues(ImmutableSet.of(StatusValue.SERVER_UPDATE_PROHIBITED)) + .build()); + persistResource( + newHost("ns1.example.tld").asBuilder().setSuperordinateDomain(domain.createVKey()).build()); + clock.advanceOneMilli(); + SuperordinateDomainServerUpdateProhibitedException thrown = + assertThrows(SuperordinateDomainServerUpdateProhibitedException.class, this::runFlow); + assertThat(thrown) + .hasMessageThat() + .contains("Superordinate domain for this hostname has serverUpdateProhibited status"); + } + @Test void testIcannActivityReportField_getsLogged() throws Exception { persistActiveHost("ns1.example.tld"); diff --git a/core/src/test/java/google/registry/flows/host/HostUpdateFlowTest.java b/core/src/test/java/google/registry/flows/host/HostUpdateFlowTest.java index 4c5890df678..161df88ecc4 100644 --- a/core/src/test/java/google/registry/flows/host/HostUpdateFlowTest.java +++ b/core/src/test/java/google/registry/flows/host/HostUpdateFlowTest.java @@ -68,6 +68,7 @@ import google.registry.flows.host.HostFlowUtils.IpAddressNotRoutableException; import google.registry.flows.host.HostFlowUtils.SuperordinateDomainDoesNotExistException; import google.registry.flows.host.HostFlowUtils.SuperordinateDomainInPendingDeleteException; +import google.registry.flows.host.HostFlowUtils.SuperordinateDomainServerUpdateProhibitedException; import google.registry.flows.host.HostUpdateFlow.CannotAddIpToExternalHostException; import google.registry.flows.host.HostUpdateFlow.CannotRemoveSubordinateHostLastIpException; import google.registry.flows.host.HostUpdateFlow.CannotRenameExternalHostException; @@ -889,6 +890,30 @@ void testFailure_superordinateInPendingDelete() throws Exception { .contains("Superordinate domain for this hostname is in pending delete"); } + @Test + void testFailure_superordinateDomainIsServerUpdateProhibited() throws Exception { + setEppHostUpdateInput( + "ns1.example.tld", + "ns2.example.tld", + "192.0.2.22", + "1080:0:0:0:8:800:200C:417A"); + createTld("tld"); + Domain domain = + persistResource( + DatabaseHelper.newDomain("example.tld") + .asBuilder() + .setSubordinateHosts(ImmutableSet.of(oldHostName())) + .setStatusValues(ImmutableSet.of(StatusValue.SERVER_UPDATE_PROHIBITED)) + .build()); + persistActiveSubordinateHost(oldHostName(), domain); + clock.advanceOneMilli(); + SuperordinateDomainServerUpdateProhibitedException thrown = + assertThrows(SuperordinateDomainServerUpdateProhibitedException.class, this::runFlow); + assertThat(thrown) + .hasMessageThat() + .contains("Superordinate domain for this hostname has serverUpdateProhibited status"); + } + @Test void testFailure_neverExisted() throws Exception { ResourceDoesNotExistException thrown =