Make host flows only accept canonicalized host names as input

This now throws errors when a non-lower-cased, non-puny-coded, or non-canonicalized host name is passed in as an input parameter.

The approach we'll take is to first notify registrars which hosts we'll be renaming, then
issue EPP host update commands to effect those renames as superuser, then push this code
live to production.

This fixes #38 on GitHub.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=138441130

docs/flows.md

@@ -967,6 +967,9 @@ are enqueued to update DNS accordingly.
         addresses.
 *   2005
     *   Invalid host name.
+    *   Host names must be in lower-case.
+    *   Host names must be in normalized format.
+    *   Host names must be puny-coded.
 *   2201
     *   The specified resource belongs to another client.
 *   2302
@@ -996,6 +999,10 @@ see the information for any host.
 
 ### Errors
 
+*   2005
+    *   Host names must be in lower-case.
+    *   Host names must be in normalized format.
+    *   Host names must be puny-coded.
 *   2303
     *   Resource with this id does not exist.
 
@@ -1015,6 +1022,10 @@ or failure message when the process is complete.
 
 ### Errors
 
+*   2005
+    *   Host names must be in lower-case.
+    *   Host names must be in normalized format.
+    *   Host names must be puny-coded.
 *   2201
     *   The specified resource belongs to another client.
 *   2303
@@ -1047,6 +1058,9 @@ allows creating a host name, and if necessary enqueues tasks to update DNS.
     *   External hosts must not have ip addresses.
 *   2005
     *   Invalid host name.
+    *   Host names must be in lower-case.
+    *   Host names must be in normalized format.
+    *   Host names must be puny-coded.
 *   2302
     *   Resource with this id already exists.
 *   2303

java/google/registry/flows/host/HostCreateFlow.java

@@ -65,6 +65,9 @@ import org.joda.time.DateTime;
  * @error {@link HostFlowUtils.HostNameTooLongException}
  * @error {@link HostFlowUtils.HostNameTooShallowException}
  * @error {@link HostFlowUtils.InvalidHostNameException}
+ * @error {@link HostFlowUtils.HostNameNotLowerCaseException}
+ * @error {@link HostFlowUtils.HostNameNotNormalizedException}
+ * @error {@link HostFlowUtils.HostNameNotPunyCodedException}
  * @error {@link HostFlowUtils.SuperordinateDomainDoesNotExistException}
  * @error {@link SubordinateHostMustHaveIpException}
  * @error {@link UnexpectedExternalHostIpException}

java/google/registry/flows/host/HostDeleteFlow.java

@@ -19,6 +19,7 @@ import static google.registry.flows.ResourceFlowUtils.failfastForAsyncDelete;
 import static google.registry.flows.ResourceFlowUtils.loadAndVerifyExistence;
 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.model.eppoutput.Result.Code.SUCCESS_WITH_ACTION_PENDING;
 import static google.registry.model.ofy.ObjectifyService.ofy;
 
@@ -54,6 +55,9 @@ import org.joda.time.DateTime;
  * @error {@link google.registry.flows.ResourceFlowUtils.ResourceNotOwnedException}
  * @error {@link google.registry.flows.exceptions.ResourceStatusProhibitsOperationException}
  * @error {@link google.registry.flows.exceptions.ResourceToDeleteIsReferencedException}
+ * @error {@link HostFlowUtils.HostNameNotLowerCaseException}
+ * @error {@link HostFlowUtils.HostNameNotNormalizedException}
+ * @error {@link HostFlowUtils.HostNameNotPunyCodedException}
  */
 public final class HostDeleteFlow implements TransactionalFlow {
 
@@ -85,6 +89,7 @@ public final class HostDeleteFlow implements TransactionalFlow {
     extensionManager.validate();
     validateClientIsLoggedIn(clientId);
     DateTime now = ofy().getTransactionTime();
+    validateHostName(targetId);
     failfastForAsyncDelete(targetId, now, HostResource.class, GET_NAMESERVERS);
     HostResource existingHost = loadAndVerifyExistence(HostResource.class, targetId, now);
     verifyNoDisallowedStatuses(existingHost, DISALLOWED_STATUSES);

java/google/registry/flows/host/HostFlowUtils.java

@@ -17,7 +17,9 @@ package google.registry.flows.host;
 import static google.registry.model.EppResourceUtils.isActive;
 import static google.registry.model.EppResourceUtils.loadByForeignKey;
 import static google.registry.model.registry.Registries.findTldForName;
+import static google.registry.util.PreconditionsUtils.checkArgumentNotNull;
 
+import com.google.common.base.Ascii;
 import com.google.common.base.Joiner;
 import com.google.common.base.Optional;
 import com.google.common.collect.Iterables;
@@ -29,6 +31,7 @@ import google.registry.flows.EppException.ParameterValuePolicyErrorException;
 import google.registry.flows.EppException.ParameterValueRangeErrorException;
 import google.registry.flows.EppException.ParameterValueSyntaxErrorException;
 import google.registry.model.domain.DomainResource;
+import google.registry.util.Idn;
 import org.joda.time.DateTime;
 
 /** Static utility functions for host flows. */
@@ -36,14 +39,23 @@ public class HostFlowUtils {
 
   /** Checks that a host name is valid. */
   static InternetDomainName validateHostName(String name) throws EppException {
-    if (name == null) {
+    checkArgumentNotNull(name, "Must specify host name to validate");
-      return null;
-    }
     if (name.length() > 253) {
       throw new HostNameTooLongException();
     }
+    String hostNameLowerCase = Ascii.toLowerCase(name);
+    if (!name.equals(hostNameLowerCase)) {
+      throw new HostNameNotLowerCaseException(hostNameLowerCase);
+    }
+    String hostNamePunyCoded = Idn.toASCII(name);
+    if (!name.equals(hostNamePunyCoded)) {
+      throw new HostNameNotPunyCodedException(hostNamePunyCoded);
+    }
     try {
       InternetDomainName hostName = InternetDomainName.from(name);
+      if (!name.equals(hostName.toString())) {
+        throw new HostNameNotNormalizedException(hostName.toString());
+      }
       // Checks whether a hostname is deep enough. Technically a host can be just one under a
       // public suffix (e.g. example.com) but we require by policy that it has to be at least one
       // part beyond that (e.g. ns1.example.com). The public suffix list includes all current
@@ -135,4 +147,26 @@ public class HostFlowUtils {
       super("Invalid host name");
     }
   }
+
+  /** Host names must be in lower-case. */
+  static class HostNameNotLowerCaseException extends ParameterValueSyntaxErrorException {
+    public HostNameNotLowerCaseException(String expectedHostName) {
+      super(String.format("Host names must be in lower-case; expected %s", expectedHostName));
+    }
+  }
+
+  /** Host names must be puny-coded. */
+  static class HostNameNotPunyCodedException extends ParameterValueSyntaxErrorException {
+    public HostNameNotPunyCodedException(String expectedHostName) {
+      super(String.format("Host names must be puny-coded; expected %s", expectedHostName));
+    }
+  }
+
+  /** Host names must be in normalized format. */
+  static class HostNameNotNormalizedException extends ParameterValueSyntaxErrorException {
+    public HostNameNotNormalizedException(String expectedHostName) {
+      super(
+          String.format("Host names must be in normalized format; expected %s", expectedHostName));
+    }
+  }
 }

java/google/registry/flows/host/HostInfoFlow.java

@@ -16,6 +16,7 @@ package google.registry.flows.host;
 
 import static google.registry.flows.FlowUtils.validateClientIsLoggedIn;
 import static google.registry.flows.ResourceFlowUtils.loadAndVerifyExistence;
+import static google.registry.flows.host.HostFlowUtils.validateHostName;
 import static google.registry.model.EppResourceUtils.cloneResourceWithLinkedStatus;
 
 import google.registry.flows.EppException;
@@ -36,6 +37,9 @@ import org.joda.time.DateTime;
  * transfer if it has ever been transferred. Any registrar can see the information for any host.
  *
  * @error {@link google.registry.flows.ResourceFlowUtils.ResourceDoesNotExistException}
+ * @error {@link HostFlowUtils.HostNameNotLowerCaseException}
+ * @error {@link HostFlowUtils.HostNameNotNormalizedException}
+ * @error {@link HostFlowUtils.HostNameNotPunyCodedException}
  */
 public final class HostInfoFlow implements Flow {
 
@@ -50,6 +54,7 @@ public final class HostInfoFlow implements Flow {
   public EppResponse run() throws EppException {
     extensionManager.validate();  // There are no legal extensions for this flow.
     validateClientIsLoggedIn(clientId); 
+    validateHostName(targetId);
     DateTime now = clock.nowUtc();
     HostResource host = loadAndVerifyExistence(HostResource.class, targetId, now);
     return responseBuilder.setResData(cloneResourceWithLinkedStatus(host, now)).build();

java/google/registry/flows/host/HostUpdateFlow.java

@@ -82,6 +82,9 @@ import org.joda.time.DateTime;
  * @error {@link google.registry.flows.exceptions.ResourceStatusProhibitsOperationException}
  * @error {@link HostFlowUtils.HostNameTooShallowException}
  * @error {@link HostFlowUtils.InvalidHostNameException}
+ * @error {@link HostFlowUtils.HostNameNotLowerCaseException}
+ * @error {@link HostFlowUtils.HostNameNotNormalizedException}
+ * @error {@link HostFlowUtils.HostNameNotPunyCodedException}
  * @error {@link HostFlowUtils.SuperordinateDomainDoesNotExistException}
  * @error {@link CannotAddIpToExternalHostException}
  * @error {@link CannotRemoveSubordinateHostLastIpException}
@@ -120,6 +123,11 @@ public final class HostUpdateFlow implements TransactionalFlow {
     Change change = command.getInnerChange();
     String suppliedNewHostName = change.getFullyQualifiedHostName();
     DateTime now = ofy().getTransactionTime();
+    // Validation is disabled for superusers to allow renaming of existing invalid hostnames.
+    // TODO(b/32328995): Remove superuser override once all bad data in prod has been fixed.
+    if (!isSuperuser) {
+      validateHostName(targetId);
+    }
     HostResource existingHost = loadAndVerifyExistence(HostResource.class, targetId, now);
     boolean isHostRename = suppliedNewHostName != null;
     String oldHostName = targetId;

javatests/google/registry/flows/host/HostCreateFlowTest.java

@@ -32,6 +32,9 @@ import google.registry.flows.ResourceFlowTestCase;
 import google.registry.flows.exceptions.ResourceAlreadyExistsException;
 import google.registry.flows.host.HostCreateFlow.SubordinateHostMustHaveIpException;
 import google.registry.flows.host.HostCreateFlow.UnexpectedExternalHostIpException;
+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.HostNameTooLongException;
 import google.registry.flows.host.HostFlowUtils.HostNameTooShallowException;
 import google.registry.flows.host.HostFlowUtils.InvalidHostNameException;
@@ -169,6 +172,27 @@ public class HostCreateFlowTest extends ResourceFlowTestCase<HostCreateFlow, Hos
     runFlow();
   }
 
+  @Test
+  public void testFailure_nonLowerCaseHostname() throws Exception {
+    setEppHostCreateInput("ns1.EXAMPLE.tld", null);
+    thrown.expect(HostNameNotLowerCaseException.class);
+    runFlow();
+  }
+
+  @Test
+  public void testFailure_nonPunyCodedHostname() throws Exception {
+    setEppHostCreateInput("ns1.çauçalito.みんな", null);
+    thrown.expect(HostNameNotPunyCodedException.class, "expected ns1.xn--aualito-txac.xn--q9jyb4c");
+    runFlow();
+  }
+
+  @Test
+  public void testFailure_nonCanonicalHostname() throws Exception {
+    setEppHostCreateInput("ns1.example.tld.", null);
+    thrown.expect(HostNameNotNormalizedException.class);
+    runFlow();
+  }
+
   @Test
   public void testFailure_longHostName() throws Exception {
     setEppHostCreateInputWithIps("a" + Strings.repeat(".labelpart", 25) + ".tld");

javatests/google/registry/flows/host/HostDeleteFlowTest.java

@@ -24,6 +24,7 @@ import static google.registry.testing.DatastoreHelper.persistResource;
 import static google.registry.testing.HostResourceSubject.assertAboutHosts;
 import static google.registry.testing.TaskQueueHelper.assertNoDnsTasksEnqueued;
 
+import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.googlecode.objectify.Key;
 import google.registry.flows.ResourceFlowTestCase;
@@ -31,6 +32,9 @@ import google.registry.flows.ResourceFlowUtils.ResourceDoesNotExistException;
 import google.registry.flows.ResourceFlowUtils.ResourceNotOwnedException;
 import google.registry.flows.exceptions.ResourceStatusProhibitsOperationException;
 import google.registry.flows.exceptions.ResourceToDeleteIsReferencedException;
+import google.registry.flows.host.HostFlowUtils.HostNameNotLowerCaseException;
+import google.registry.flows.host.HostFlowUtils.HostNameNotNormalizedException;
+import google.registry.flows.host.HostFlowUtils.HostNameNotPunyCodedException;
 import google.registry.model.eppcommon.StatusValue;
 import google.registry.model.host.HostResource;
 import google.registry.model.reporting.HistoryEntry;
@@ -42,7 +46,7 @@ public class HostDeleteFlowTest extends ResourceFlowTestCase<HostDeleteFlow, Hos
 
   @Before
   public void initFlowTest() {
-    setEppInput("host_delete.xml");
+    setEppInput("host_delete.xml", ImmutableMap.of("HOSTNAME", "ns1.example.tld"));
   }
 
   private void doFailingStatusTest(StatusValue statusValue, Class<? extends Exception> exception)
@@ -158,4 +162,25 @@ public class HostDeleteFlowTest extends ResourceFlowTestCase<HostDeleteFlow, Hos
     thrown.expect(ResourceToDeleteIsReferencedException.class);
     runFlow();
   }
+
+  @Test
+  public void testFailure_nonLowerCaseHostname() throws Exception {
+    setEppInput("host_delete.xml", ImmutableMap.of("HOSTNAME", "NS1.EXAMPLE.NET"));
+    thrown.expect(HostNameNotLowerCaseException.class);
+    runFlow();
+  }
+
+  @Test
+  public void testFailure_nonPunyCodedHostname() throws Exception {
+    setEppInput("host_delete.xml", ImmutableMap.of("HOSTNAME", "ns1.çauçalito.tld"));
+    thrown.expect(HostNameNotPunyCodedException.class, "expected ns1.xn--aualito-txac.tld");
+    runFlow();
+  }
+
+  @Test
+  public void testFailure_nonCanonicalHostname() throws Exception {
+    setEppInput("host_delete.xml", ImmutableMap.of("HOSTNAME", "ns1.example.tld."));
+    thrown.expect(HostNameNotNormalizedException.class);
+    runFlow();
+  }
 }

javatests/google/registry/flows/host/HostInfoFlowTest.java

@@ -21,11 +21,15 @@ import static google.registry.testing.DatastoreHelper.createTld;
 import static google.registry.testing.DatastoreHelper.newDomainResource;
 import static google.registry.testing.DatastoreHelper.persistResource;
 
+import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.net.InetAddresses;
 import com.googlecode.objectify.Key;
 import google.registry.flows.ResourceFlowTestCase;
 import google.registry.flows.ResourceFlowUtils.ResourceDoesNotExistException;
+import google.registry.flows.host.HostFlowUtils.HostNameNotLowerCaseException;
+import google.registry.flows.host.HostFlowUtils.HostNameNotNormalizedException;
+import google.registry.flows.host.HostFlowUtils.HostNameNotPunyCodedException;
 import google.registry.model.domain.DomainResource;
 import google.registry.model.eppcommon.StatusValue;
 import google.registry.model.host.HostResource;
@@ -38,7 +42,7 @@ import org.junit.Test;
 public class HostInfoFlowTest extends ResourceFlowTestCase<HostInfoFlow, HostResource> {
 
   public HostInfoFlowTest() {
-    setEppInput("host_info.xml");
+    setEppInput("host_info.xml", ImmutableMap.of("HOSTNAME", "ns1.example.tld"));
   }
 
   @Before
@@ -158,4 +162,25 @@ public class HostInfoFlowTest extends ResourceFlowTestCase<HostInfoFlow, HostRes
         String.format("(%s)", getUniqueIdFromCommand()));
     runFlow();
   }
+
+  @Test
+  public void testFailure_nonLowerCaseHostname() throws Exception {
+    setEppInput("host_info.xml", ImmutableMap.of("HOSTNAME", "NS1.EXAMPLE.NET"));
+    thrown.expect(HostNameNotLowerCaseException.class);
+    runFlow();
+  }
+
+  @Test
+  public void testFailure_nonPunyCodedHostname() throws Exception {
+    setEppInput("host_info.xml", ImmutableMap.of("HOSTNAME", "ns1.çauçalito.tld"));
+    thrown.expect(HostNameNotPunyCodedException.class, "expected ns1.xn--aualito-txac.tld");
+    runFlow();
+  }
+
+  @Test
+  public void testFailure_nonCanonicalHostname() throws Exception {
+    setEppInput("host_info.xml", ImmutableMap.of("HOSTNAME", "ns1.example.tld."));
+    thrown.expect(HostNameNotNormalizedException.class);
+    runFlow();
+  }
 }

javatests/google/registry/flows/host/HostUpdateFlowTest.java

@@ -48,6 +48,9 @@ import google.registry.flows.ResourceFlowUtils.ResourceNotOwnedException;
 import google.registry.flows.ResourceFlowUtils.StatusNotClientSettableException;
 import google.registry.flows.exceptions.ResourceHasClientUpdateProhibitedException;
 import google.registry.flows.exceptions.ResourceStatusProhibitsOperationException;
+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.HostNameTooShallowException;
 import google.registry.flows.host.HostFlowUtils.InvalidHostNameException;
 import google.registry.flows.host.HostFlowUtils.SuperordinateDomainDoesNotExistException;
@@ -357,6 +360,19 @@ public class HostUpdateFlowTest extends ResourceFlowTestCase<HostUpdateFlow, Hos
         .hasStatusValue(StatusValue.SERVER_UPDATE_PROHIBITED);
   }
 
+  @Test
+  public void testSuccess_superuserCanSpecifyInvalidExistingHostName() throws Exception {
+    persistActiveHost("NS1.çiça199.tld.");
+    clock.advanceOneMilli();
+    setEppHostUpdateInput("NS1.çiça199.tld.", "ns1.xn--ia199-xrab.tld", null, null);
+    runFlowAssertResponse(
+        CommitMode.LIVE, UserPrivileges.SUPERUSER, readFile("host_update_response.xml"));
+    clock.advanceOneMilli();
+    assertThat(loadByForeignKey(HostResource.class, "NS1.çiça199.tld.", clock.nowUtc())).isNull();
+    assertThat(loadByForeignKey(HostResource.class, "ns1.xn--ia199-xrab.tld", clock.nowUtc()))
+        .isNotNull();
+  }
+
   @Test
   public void testSuccess_subordToSubord_lastTransferTimeFromPreviousSuperordinateWinsOut()
       throws Exception {
@@ -700,6 +716,54 @@ public class HostUpdateFlowTest extends ResourceFlowTestCase<HostUpdateFlow, Hos
     runFlow();
   }
 
+  @Test
+  public void testFailure_referToNonLowerCaseHostname() throws Exception {
+    persistActiveHost("ns1.EXAMPLE.tld");
+    setEppHostUpdateInput("ns1.EXAMPLE.tld", "ns2.example.tld", null, null);
+    thrown.expect(HostNameNotLowerCaseException.class);
+    runFlow();
+  }
+
+  @Test
+  public void testFailure_renameToNonLowerCaseHostname() throws Exception {
+    persistActiveHost("ns1.example.tld");
+    setEppHostUpdateInput("ns1.example.tld", "ns2.EXAMPLE.tld", null, null);
+    thrown.expect(HostNameNotLowerCaseException.class);
+    runFlow();
+  }
+
+  @Test
+  public void testFailure_referToNonPunyCodedHostname() throws Exception {
+    persistActiveHost("ns1.çauçalito.tld");
+    setEppHostUpdateInput("ns1.çauçalito.tld", "ns1.sausalito.tld", null, null);
+    thrown.expect(HostNameNotPunyCodedException.class, "expected ns1.xn--aualito-txac.tld");
+    runFlow();
+  }
+
+  @Test
+  public void testFailure_renameToNonPunyCodedHostname() throws Exception {
+    persistActiveHost("ns1.sausalito.tld");
+    setEppHostUpdateInput("ns1.sausalito.tld", "ns1.çauçalito.tld", null, null);
+    thrown.expect(HostNameNotPunyCodedException.class, "expected ns1.xn--aualito-txac.tld");
+    runFlow();
+  }
+
+  @Test
+  public void testFailure_referToNonCanonicalHostname() throws Exception {
+    persistActiveHost("ns1.example.tld.");
+    setEppHostUpdateInput("ns1.example.tld.", "ns2.example.tld", null, null);
+    thrown.expect(HostNameNotNormalizedException.class);
+    runFlow();
+  }
+
+  @Test
+  public void testFailure_renameToNonCanonicalHostname() throws Exception {
+    persistActiveHost("ns1.example.tld");
+    setEppHostUpdateInput("ns1.example.tld", "ns2.example.tld.", null, null);
+    thrown.expect(HostNameNotNormalizedException.class);
+    runFlow();
+  }
+
   @Test
   public void testFailure_clientUpdateProhibited() throws Exception {
     persistResource(

javatests/google/registry/flows/host/testdata/host_delete.xml

@@ -3,7 +3,7 @@
     <delete>
       <host:delete
        xmlns:host="urn:ietf:params:xml:ns:host-1.0">
-        <host:name>ns1.example.tld</host:name>
+        <host:name>%HOSTNAME%</host:name>
       </host:delete>
     </delete>
     <clTRID>ABC-12345</clTRID>

javatests/google/registry/flows/host/testdata/host_info.xml

@@ -3,7 +3,7 @@
     <info>
       <host:info
        xmlns:host="urn:ietf:params:xml:ns:host-1.0">
-        <host:name>ns1.example.tld</host:name>
+        <host:name>%HOSTNAME%</host:name>
       </host:info>
     </info>
     <clTRID>ABC-12345</clTRID>