From c426a805632dc206a002aacd95d3a990737fd568 Mon Sep 17 00:00:00 2001 From: jianglai Date: Mon, 13 Mar 2017 14:21:08 -0700 Subject: [PATCH] Add a new reservation type to support nameserver restrictions A new field (allowedNameservers) is added to ReservedListEntry that stores the allow nameservers for the label. The field itself is a comma separated string, but the actual lines within a reserved list file (from which the field is parsed) uses colon to separate nameservers, to avoid conflicting with the commas used as primary separators in a CSV file. Combined with upcoming update(s) that enables locking down an entire TLD to only delegate domains with a nameserver restricted reservation type, this change will enable us to restrict domain delegation to nameservers specifically specified in the allowed nameservers list, in order to prevent malicious delegation in case the registrar for a brand TLD is compromised. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=149989330 --- .../reserved-list-management.md | 33 ++++-- .../registry/label/DomainLabelEntry.java | 2 +- .../model/registry/label/PremiumList.java | 4 +- .../model/registry/label/ReservationType.java | 11 +- .../model/registry/label/ReservedList.java | 96 +++++++++++++++-- .../registry/label/ReservedListTest.java | 102 +++++++++++++++--- javatests/google/registry/model/schema.txt | 2 + 7 files changed, 208 insertions(+), 42 deletions(-) diff --git a/docs/operational-procedures/reserved-list-management.md b/docs/operational-procedures/reserved-list-management.md index 1cf9608f8..50fdeebad 100644 --- a/docs/operational-procedures/reserved-list-management.md +++ b/docs/operational-procedures/reserved-list-management.md @@ -12,6 +12,11 @@ a price, it has a reservation type. The valid values for reservation types are: * **`UNRESERVED`** - The default value for any label that isn't reserved. Labels that aren't explictly under any other status implictly have this value. +* **`NAMESERVER_RESTRICTED`** - Only nameservers included here can be set on a + domain with this label. If the a label in this type exists on multiple + reserved lists that are applied to the same TLD. The set of allowed + nameservers for that label in that TLD is the intersection of all applicable + nameservers. * **`ALLOWED_IN_SUNRISE`** - The label can be registered during the sunrise period by a registrant with a valid claim but it is reserved thereafter. * **`MISTAKEN_PREMIUM`** - The label is reserved because it was mistakenly put @@ -19,7 +24,9 @@ a price, it has a reservation type. The valid values for reservation types are: a valid claim but is reserved thereafter. * **`RESERVED_FOR_ANCHOR_TENANT`** - The label is reserved for the use of an anchor tenant, and can only be registered by someone sending along the EPP - passcode specified here at time of registration. + passcode specified here at time of registration. If a label has different + passcodes in different lists that are applied to the same TLD, an error will + occur. * **`NAME_COLLISION`** - The label is reserved because it is on an [ICANN collision list](https://www.icann.org/resources/pages/name-collision-2013-12-06-en). @@ -28,23 +35,29 @@ a price, it has a reservation type. The valid values for reservation types are: * **`FULLY_BLOCKED`** - The label is fully reserved, no further reason specified. -The reservation types are listed in order of increasing precedence, so if a -label is included on different lists that are applied to a single TLD, whichever -reservation type is later in the list takes precedence. E.g. a label being fully -blocked in one list always supersedes it being allowed in sunrise from another -list. In general `FULLY_BLOCKED` is by far the most widely used reservation type -for typical TLD use cases. +The reservation types are listed in order of increasing precedence, but if a +label is included in different lists that are applied to a single TLD, all +reservation types of the label are returned when queried. The order of the +reservation types only affects the message a domain check EPP request receives, +which is the one with the highest precedence. E.g. a label with name collision +reservation type in one list and allowed in sunrise reservation type in another +list will have both reservation types, but domain check will report that the +label is reserved due to name collision (with message "Cannot be delegated"). In +general `FULLY_BLOCKED` is by far the most widely used reservation type for +typical TLD use cases. Here's an example of a small reserved list. Note that -`RESERVED_FOR_ANCHOR_TENANT` is the only reservation type that has a third entry -on the line, that entry being the EPP passcode required to register the domain -(`hunter2` in this case): +`RESERVED_FOR_ANCHOR_TENANT` has a third entry on the line, being the EPP +passcode required to register the domain (`hunter2` in this case); and that +`NAMESERVER_RESERVED` also has a third entry, a colon separated list of +nameservers that the label can be delegated to: ``` reserveddomain,FULLY_BLOCKED availableinga,ALLOWED_IN_SUNRISE fourletterword,FULLY_BLOCKED acmecorp,RESERVED_FOR_ANCHOR_TENANT,hunter2 +internaldomain,NAMESERVER_RESTRICTED,ns1.internal.tld:ns1.internal.tld ``` There are two types of reserved lists: Those that are intended to apply to a diff --git a/java/google/registry/model/registry/label/DomainLabelEntry.java b/java/google/registry/model/registry/label/DomainLabelEntry.java index c4d26039f..ae6f38d68 100644 --- a/java/google/registry/model/registry/label/DomainLabelEntry.java +++ b/java/google/registry/model/registry/label/DomainLabelEntry.java @@ -22,7 +22,7 @@ import google.registry.model.Buildable.GenericBuilder; import google.registry.model.ImmutableObject; /** - * Represents a label entry parsed from a line in a Reserved List txt file. + * Represents a label entry parsed from a line in a reserved/premium list txt file. * * @param The type of the value stored for the domain label, e.g. {@link ReservationType}. */ diff --git a/java/google/registry/model/registry/label/PremiumList.java b/java/google/registry/model/registry/label/PremiumList.java index 767111d6b..d156756fd 100644 --- a/java/google/registry/model/registry/label/PremiumList.java +++ b/java/google/registry/model/registry/label/PremiumList.java @@ -242,9 +242,7 @@ public final class PremiumList extends BaseDomainLabelList { public Builder() {} diff --git a/java/google/registry/model/registry/label/ReservationType.java b/java/google/registry/model/registry/label/ReservationType.java index 3b408da2e..0683b6ef9 100644 --- a/java/google/registry/model/registry/label/ReservationType.java +++ b/java/google/registry/model/registry/label/ReservationType.java @@ -28,11 +28,12 @@ public enum ReservationType { // severity. UNRESERVED(null, 0), - ALLOWED_IN_SUNRISE("Reserved for non-sunrise", 1), - MISTAKEN_PREMIUM("Reserved", 2), - RESERVED_FOR_ANCHOR_TENANT("Reserved", 3), - NAME_COLLISION("Cannot be delegated", 4), - FULLY_BLOCKED("Reserved", 5); + NAMESERVER_RESTRICTED("Nameserver restricted", 1), + ALLOWED_IN_SUNRISE("Reserved for non-sunrise", 2), + MISTAKEN_PREMIUM("Reserved", 3), + RESERVED_FOR_ANCHOR_TENANT("Reserved", 4), + NAME_COLLISION("Cannot be delegated", 5), + FULLY_BLOCKED("Reserved", 6); @Nullable private final String messageForCheck; diff --git a/java/google/registry/model/registry/label/ReservedList.java b/java/google/registry/model/registry/label/ReservedList.java index 6ae68df9d..a53412423 100644 --- a/java/google/registry/model/registry/label/ReservedList.java +++ b/java/google/registry/model/registry/label/ReservedList.java @@ -23,6 +23,7 @@ import static google.registry.model.common.EntityGroupRoot.getCrossTldKey; import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.model.ofy.Ofy.RECOMMENDED_MEMCACHE_EXPIRATION; import static google.registry.model.registry.label.ReservationType.FULLY_BLOCKED; +import static google.registry.model.registry.label.ReservationType.NAMESERVER_RESTRICTED; import static google.registry.model.registry.label.ReservationType.RESERVED_FOR_ANCHOR_TENANT; import static google.registry.model.registry.label.ReservationType.UNRESERVED; import static google.registry.util.CollectionUtils.nullToEmpty; @@ -30,6 +31,7 @@ import static java.util.concurrent.TimeUnit.MILLISECONDS; import static org.joda.time.DateTimeZone.UTC; import com.google.common.base.Function; +import com.google.common.base.Joiner; import com.google.common.base.Optional; import com.google.common.base.Splitter; import com.google.common.cache.CacheBuilder; @@ -85,6 +87,18 @@ public final class ReservedList */ String authCode; + /** + * Contains a comma-delimited list of the fully qualified hostnames of the nameservers that can + * be set on a domain with this label (only applicable to NAMESERVER_RESTRICTED). + * + *

A String field is persisted because Objectify 4 does not allow multi-dimensional + * collections in embedded entities. + * + * @see Embedding + */ + String allowedNameservers; + /** Mapper for use with @Mapify */ static class LabelMapper implements Mapper { @Override @@ -93,26 +107,56 @@ public final class ReservedList } } + /** + * Creates a {@link ReservedListEntry} from label, reservation type, and optionally additional + * restrictions + * + *

The additional restricitno can be the authCode for anchor tenant or the allowed + * nameservers (in a colon-separated string) for nameserver-restricted domains. + */ public static ReservedListEntry create( String label, ReservationType reservationType, - @Nullable String authCode, + @Nullable String restrictions, String comment) { - if (authCode != null) { - checkArgument(reservationType == RESERVED_FOR_ANCHOR_TENANT, - "Only anchor tenant reservations should have an auth code configured"); + ReservedListEntry entry = new ReservedListEntry(); + if (restrictions != null) { + checkArgument( + reservationType == RESERVED_FOR_ANCHOR_TENANT + || reservationType == NAMESERVER_RESTRICTED, + "Only anchor tenant and nameserver restricted reservations " + + "should have restrictions imposed"); + if (reservationType == RESERVED_FOR_ANCHOR_TENANT) { + entry.authCode = restrictions; + } else if (reservationType == NAMESERVER_RESTRICTED) { + Set allowedNameservers = + ImmutableSet.copyOf(Splitter.on(':').trimResults().split(restrictions)); + checkNameserversAreValid(allowedNameservers); + entry.allowedNameservers = Joiner.on(',').join(allowedNameservers); + } } else { checkArgument(reservationType != RESERVED_FOR_ANCHOR_TENANT, "Anchor tenant reservations must have an auth code configured"); + checkArgument( + reservationType != NAMESERVER_RESTRICTED, + "Nameserver restricted reservations must have at least one nameserver configured"); } - ReservedListEntry entry = new ReservedListEntry(); entry.label = label; - entry.reservationType = reservationType; - entry.authCode = authCode; entry.comment = comment; + entry.reservationType = reservationType; return entry; } + private static void checkNameserversAreValid(Set nameservers) { + for (String nameserver : nameservers) { + // A domain name with fewer than two parts cannot be a hostname, as a nameserver should be. + checkArgument( + InternetDomainName.from(nameserver).parts().size() >= 3, + "%s is not a valid nameserver hostname", + nameserver); + } + } + @Override public ReservationType getValue() { return reservationType; @@ -121,6 +165,10 @@ public final class ReservedList public String getAuthCode() { return authCode; } + + public ImmutableSet getAllowedNameservers() { + return ImmutableSet.copyOf(Splitter.on(',').splitToList(allowedNameservers)); + } } @Override @@ -209,6 +257,31 @@ public final class ReservedList return !domainAuthCodes.isEmpty() && getOnlyElement(domainAuthCodes).equals(authCode); } + /** + * Returns the set of nameservers that can be set on the given domain. + * + *

The allowed nameservers are the intersection of all allowed nameservers for the given domain + * across all reserved lists. Returns an empty set if not applicable, i. e. the label for the + * domain is not set with {@code NAMESERVER_RESTRICTED} reservation type. + */ + public static ImmutableSet getAllowedNameservers(InternetDomainName domainName) { + HashSet allowedNameservers = new HashSet<>(); + boolean foundFirstNameserverRestricted = false; + for (ReservedListEntry entry : + getReservedListEntries(domainName.parts().get(0), domainName.parent().toString())) { + if (entry.reservationType == NAMESERVER_RESTRICTED) { + if (foundFirstNameserverRestricted) { + allowedNameservers.retainAll(entry.getAllowedNameservers()); + } else { + allowedNameservers = new HashSet(entry.getAllowedNameservers()); + foundFirstNameserverRestricted = true; + } + } + } + return ImmutableSet.copyOf(allowedNameservers); + } + + /** * Helper function to retrieve the entries associated with this label and TLD, or an empty set if * no such entry exists. @@ -271,8 +344,9 @@ public final class ReservedList * Gets the {@link ReservationType} of a label in a single ReservedList, or returns an absent * Optional if none exists in the list. * - *

Note that this logic is significantly less complicated than the getReservation() methods, - * which are applicable to an entire Registry, and need to check across multiple reserved lists. + *

Note that this logic is significantly less complicated than the {@link #getReservationTypes} + * methods, which are applicable to an entire Registry, and need to check across multiple reserved + * lists. */ public Optional getReservationInList(String label) { ReservedListEntry entry = getReservedListEntries().get(label); @@ -293,8 +367,8 @@ public final class ReservedList "Could not parse line in reserved list: %s", originalLine); String label = parts.get(0); ReservationType reservationType = ReservationType.valueOf(parts.get(1)); - String authCode = (parts.size() > 2) ? parts.get(2) : null; - return ReservedListEntry.create(label, reservationType, authCode, comment); + String restrictions = (parts.size() > 2) ? parts.get(2) : null; + return ReservedListEntry.create(label, reservationType, restrictions, comment); } @Override diff --git a/javatests/google/registry/model/registry/label/ReservedListTest.java b/javatests/google/registry/model/registry/label/ReservedListTest.java index 01dc1ee3c..982c411f8 100644 --- a/javatests/google/registry/model/registry/label/ReservedListTest.java +++ b/javatests/google/registry/model/registry/label/ReservedListTest.java @@ -22,9 +22,11 @@ import static google.registry.model.registry.label.DomainLabelMetrics.reservedLi import static google.registry.model.registry.label.ReservationType.ALLOWED_IN_SUNRISE; import static google.registry.model.registry.label.ReservationType.FULLY_BLOCKED; import static google.registry.model.registry.label.ReservationType.MISTAKEN_PREMIUM; +import static google.registry.model.registry.label.ReservationType.NAMESERVER_RESTRICTED; import static google.registry.model.registry.label.ReservationType.NAME_COLLISION; import static google.registry.model.registry.label.ReservationType.RESERVED_FOR_ANCHOR_TENANT; import static google.registry.model.registry.label.ReservationType.UNRESERVED; +import static google.registry.model.registry.label.ReservedList.getAllowedNameservers; import static google.registry.model.registry.label.ReservedList.getReservationTypes; import static google.registry.model.registry.label.ReservedList.matchesAnchorTenantReservation; import static google.registry.monitoring.metrics.contrib.EventMetricSubject.assertThat; @@ -163,22 +165,59 @@ public class ReservedListTest { .hasNoOtherValues(); } + @Test + public void testGetAllowedNameservers() throws Exception { + ReservedList rl1 = + persistReservedList( + "reserved1", + "lol,NAMESERVER_RESTRICTED,ns1.nameserver.com", + "lol1,NAMESERVER_RESTRICTED,ns1.nameserver.com:ns2.domain.tld:ns3.domain.tld", + "lol2,NAMESERVER_RESTRICTED,ns.name.tld # This is a comment"); + ReservedList rl2 = + persistReservedList( + "reserved2", + "lol1,NAMESERVER_RESTRICTED,ns3.nameserver.com:ns2.domain.tld:ns3.domain.tld", + "lol2,NAMESERVER_RESTRICTED,ns3.nameserver.com:ns4.domain.tld", + "lol3,NAMESERVER_RESTRICTED,ns3.nameserver.com"); + ReservedList rl3 = + persistReservedList( + "reserved3", "lol1,NAMESERVER_RESTRICTED,ns3.domain.tld", "lol4,ALLOWED_IN_SUNRISE"); + persistResource(Registry.get("tld").asBuilder().setReservedLists(rl1, rl2, rl3).build()); + assertThat(getReservationTypes("lol", "tld")).containsExactly(NAMESERVER_RESTRICTED); + assertThat(getReservationTypes("lol1", "tld")).containsExactly(NAMESERVER_RESTRICTED); + assertThat(getReservationTypes("lol2", "tld")).containsExactly(NAMESERVER_RESTRICTED); + assertThat(getReservationTypes("lol3", "tld")).containsExactly(NAMESERVER_RESTRICTED); + assertThat(getAllowedNameservers(InternetDomainName.from("lol.tld"))) + .containsExactly("ns1.nameserver.com"); + assertThat(getAllowedNameservers(InternetDomainName.from("lol1.tld"))) + .containsExactly("ns3.domain.tld"); + assertThat(getAllowedNameservers(InternetDomainName.from("lol2.tld"))).isEmpty(); + assertThat(getAllowedNameservers(InternetDomainName.from("lol3.tld"))) + .containsExactly("ns3.nameserver.com"); + assertThat(getAllowedNameservers(InternetDomainName.from("lol4.tld"))).isEmpty(); + } + @Test public void testMatchesAnchorTenantReservation_falseOnOtherReservationTypes() throws Exception { - persistResource(Registry.get("tld").asBuilder() - .setReservedLists(ImmutableSet.of( - persistReservedList( - "reserved2", - "lol,FULLY_BLOCKED", - "lol2,NAME_COLLISION", - "lol3,MISTAKEN_PREMIUM", - "lol4,ALLOWED_IN_SUNRISE"))) - .build()); + persistResource( + Registry.get("tld") + .asBuilder() + .setReservedLists( + ImmutableSet.of( + persistReservedList( + "reserved2", + "lol,FULLY_BLOCKED", + "lol2,NAME_COLLISION", + "lol3,MISTAKEN_PREMIUM", + "lol4,ALLOWED_IN_SUNRISE", + "lol5,NAMESERVER_RESTRICTED,na1.domain.tld"))) + .build()); assertThat(matchesAnchorTenantReservation(InternetDomainName.from("lol.tld"), "")).isFalse(); assertThat(matchesAnchorTenantReservation(InternetDomainName.from("lol2.tld"), "")).isFalse(); assertThat(matchesAnchorTenantReservation(InternetDomainName.from("lol3.tld"), "")).isFalse(); assertThat(matchesAnchorTenantReservation(InternetDomainName.from("lol4.tld"), "")).isFalse(); assertThat(matchesAnchorTenantReservation(InternetDomainName.from("lol5.tld"), "")).isFalse(); + assertThat(matchesAnchorTenantReservation(InternetDomainName.from("lol6.tld"), "")).isFalse(); assertThat(reservedListChecks) .hasValueForLabels(1, "tld", "1", "reserved2", FULLY_BLOCKED.toString()) .and() @@ -188,6 +227,8 @@ public class ReservedListTest { .and() .hasValueForLabels(1, "tld", "1", "reserved2", ALLOWED_IN_SUNRISE.toString()) .and() + .hasValueForLabels(1, "tld", "1", "reserved2", NAMESERVER_RESTRICTED.toString()) + .and() .hasValueForLabels(1, "tld", "0", "(none)", UNRESERVED.toString()) .and() .hasNoOtherValues(); @@ -200,6 +241,8 @@ public class ReservedListTest { .and() .hasAnyValueForLabels("tld", "1", "reserved2", ALLOWED_IN_SUNRISE.toString()) .and() + .hasAnyValueForLabels("tld", "1", "reserved2", NAMESERVER_RESTRICTED.toString()) + .and() .hasAnyValueForLabels("tld", "0", "(none)", UNRESERVED.toString()) .and() .hasNoOtherValues(); @@ -212,6 +255,8 @@ public class ReservedListTest { .and() .hasValueForLabels(1, "tld", "reserved2", ALLOWED_IN_SUNRISE.toString()) .and() + .hasValueForLabels(1, "tld", "reserved2", NAMESERVER_RESTRICTED.toString()) + .and() .hasNoOtherValues(); } @@ -435,9 +480,11 @@ public class ReservedListTest { } @Test - public void testSave_passwordWithNonAnchorTenantReservation() throws Exception { - thrown.expect(IllegalArgumentException.class, - "Only anchor tenant reservations should have an auth code configured"); + public void testSave_additionalRestrictionWithIncompatibleReservationType() throws Exception { + thrown.expect( + IllegalArgumentException.class, + "Only anchor tenant and nameserver restricted reservations " + + "should have restrictions imposed"); persistResource( Registry.get("tld") .asBuilder() @@ -446,6 +493,24 @@ public class ReservedListTest { .build()); } + @Test + public void testSave_badNameservers_invalidSyntax() throws Exception { + thrown.expect(IllegalArgumentException.class, "Not a valid domain name: 'ns@.domain.tld'"); + persistReservedList( + "reserved1", + "lol,NAMESERVER_RESTRICTED,ns1.domain.tld:ns2.domain.tld", + "lol1,NAMESERVER_RESTRICTED,ns1.domain.tld:ns@.domain.tld"); + } + + @Test + public void testSave_badNameservers_tooFewPartsForHostname() throws Exception { + thrown.expect(IllegalArgumentException.class, "domain.tld is not a valid nameserver hostname"); + persistReservedList( + "reserved1", + "lol,NAMESERVER_RESTRICTED,ns1.domain.tld:ns2.domain.tld", + "lol1,NAMESERVER_RESTRICTED,ns1.domain.tld:domain.tld"); + } + @Test public void testSave_noPasswordWithAnchorTenantReservation() throws Exception { thrown.expect(IllegalArgumentException.class, @@ -458,6 +523,19 @@ public class ReservedListTest { .build()); } + @Test + public void testSave_noNameserversWithNameserverRestrictedReservation() throws Exception { + thrown.expect( + IllegalArgumentException.class, + "Nameserver restricted reservations must have at least one nameserver configured"); + persistResource( + Registry.get("tld") + .asBuilder() + .setReservedLists( + ImmutableSet.of(persistReservedList("reserved1", "lol,NAMESERVER_RESTRICTED"))) + .build()); + } + @Test public void testParse_cannotIncludeDuplicateLabels() { ReservedList rl = new ReservedList.Builder().setName("blah").build(); diff --git a/javatests/google/registry/model/schema.txt b/javatests/google/registry/model/schema.txt index 858692955..705f5d22d 100644 --- a/javatests/google/registry/model/schema.txt +++ b/javatests/google/registry/model/schema.txt @@ -748,6 +748,7 @@ enum google.registry.model.registry.label.ReservationType { ALLOWED_IN_SUNRISE; FULLY_BLOCKED; MISTAKEN_PREMIUM; + NAMESERVER_RESTRICTED; NAME_COLLISION; RESERVED_FOR_ANCHOR_TENANT; UNRESERVED; @@ -764,6 +765,7 @@ class google.registry.model.registry.label.ReservedList { class google.registry.model.registry.label.ReservedList$ReservedListEntry { @Id java.lang.String label; google.registry.model.registry.label.ReservationType reservationType; + java.lang.String allowedNameservers; java.lang.String authCode; java.lang.String comment; }