Bläddra i källkod

feat(api): ban registering domains covering foreign zones, fixes #382

Peter Thomassen 5 år sedan
förälder
incheckning
21d939c4e8

+ 19 - 10
api/desecapi/models.py

@@ -259,19 +259,17 @@ class Domain(ExportModelOperationsMixin('Domain'), models.Model):
         assert self.name == next(iter(private_domains), self.public_suffix)
 
         # Determine whether domain is covered by other users' zones
-        try:
-            owner = self.owner
-        except Domain.owner.RelatedObjectDoesNotExist:
-            owner = None
-        return Domain.objects.filter(Q(name__in=private_domains) & ~Q(owner=owner)).exists()
+        return Domain.objects.filter(Q(name__in=private_domains) & ~Q(owner=self._owner_or_none)).exists()
+
+    def covers_foreign_zone(self):
+        # Note: This is not completely accurate: Ideally, we should only consider zones with identical public suffix.
+        # (If a public suffix lies in between, it's ok.) However, as there could be many descendant zones, the accurate
+        # check is expensive, so currently not implemented (PSL lookups for each of them).
+        return Domain.objects.filter(Q(name__endswith=f'.{self.name}') & ~Q(owner=self._owner_or_none)).exists()
 
     def is_registrable(self):
         """
-        Returns False in any of the following cases:
-        (a) the domain name is under .internal,
-        (b) the domain_name appears on the public suffix list,
-        (c) the domain is descendant to a zone that belongs to any user different from the given one,
-            unless it's parent is a public suffix, either through the Internet PSL or local settings.
+        Returns False if the domain name is reserved, a public suffix, or covered by / covers another user's domain.
         Otherwise, True is returned.
         """
         self.clean()  # ensure .name is a domain name
@@ -288,6 +286,10 @@ class Domain(ExportModelOperationsMixin('Domain'), models.Model):
         if self.is_covered_by_foreign_zone():
             return False
 
+        # Domains that would cover another user's zone can't be registered
+        if self.covers_foreign_zone():
+            return False
+
         return True
 
     @property
@@ -310,6 +312,13 @@ class Domain(ExportModelOperationsMixin('Domain'), models.Model):
     def is_locally_registrable(self):
         return self.parent_domain_name in settings.LOCAL_PUBLIC_SUFFIXES
 
+    @property
+    def _owner_or_none(self):
+        try:
+            return self.owner
+        except Domain.owner.RelatedObjectDoesNotExist:
+            return None
+
     @property
     def parent_domain_name(self):
         return self._partitioned_name[1]

+ 1 - 1
api/desecapi/serializers.py

@@ -541,7 +541,7 @@ class DomainSerializer(serializers.ModelSerializer):
         user = user if not isinstance(user, AnonymousUser) else None
         if not models.Domain(name=domain_name, owner=user).is_registrable():
             raise serializers.ValidationError(
-                'This domain name is unavailable because it is already taken, or disallowed by policy.',
+                'This domain name conflicts with an existing zone, or is disallowed by policy.',
                 code='name_unavailable'
             )
 

+ 20 - 0
api/desecapi/tests/test_domains.py

@@ -60,6 +60,10 @@ class IsRegistrableTestCase(DesecTestCase, PublicSuffixMockMixin):
             self.assertRegistrable('something.else')
 
     def test_can_register_local_public_suffix(self):
+        # Avoid side effects from existing domains (such as dedyn.io.example.com being covered by the .com test below)
+        # Existing domains depend on environment variables. We may want to make the tests "stand-alone" at some point.
+        Domain.objects.filter(name__in=self.AUTO_DELEGATION_DOMAINS).delete()
+
         local_public_suffixes = ['something.else', 'our.public.suffix', 'com', 'com.uk']
         with self.mock(
             global_public_suffixes=['com', 'de', 'xxx', 'com.uk'],
@@ -83,6 +87,22 @@ class IsRegistrableTestCase(DesecTestCase, PublicSuffixMockMixin):
             self.assertNotRegistrable('b.a.public.suffix', user_b)
             self.assertRegistrable('b.a.public.suffix', user_a)
 
+    def test_cant_register_ancestors_of_registered_domains(self):
+        with self.mock(
+            global_public_suffixes={'public.suffix'},
+            local_public_suffixes={'public.suffix'},
+        ):
+            # let A own c.b.a.public.suffix
+            user_a = self.create_user()
+            self.assertRegistrable('c.b.a.public.suffix', user_a)
+            self.create_domain(owner=user_a, name='c.b.a.public.suffix')
+            # user B shall not register b.a.public.suffix or a.public.suffix, but A may
+            user_b = self.create_user()
+            self.assertNotRegistrable('b.a.public.suffix', user_b)
+            self.assertNotRegistrable('a.public.suffix', user_b)
+            self.assertRegistrable('b.a.public.suffix', user_a)
+            self.assertRegistrable('a.public.suffix', user_a)
+
     def test_can_register_public_suffixes_under_private_domains(self):
         with self.mock(
             global_public_suffixes={'public.suffix'},

+ 1 - 1
api/desecapi/tests/test_user_management.py

@@ -253,7 +253,7 @@ class UserManagementTestCase(DesecTestCase, PublicSuffixMockMixin):
     def assertRegistrationFailureDomainUnavailableResponse(self, response, domain):
         self.assertContains(
             response=response,
-            text='This domain name is unavailable',
+            text='This domain name conflicts with an existing zone, or is disallowed by policy.',
             status_code=status.HTTP_400_BAD_REQUEST,
             msg_prefix=str(response.data)
         )

+ 7 - 5
docs/dns/domains.rst

@@ -123,16 +123,18 @@ Only the ``name`` field is mandatory.
 Upon success, the response status code will be ``201 Created``, with the
 domain object contained in the response body.  If an improper request was
 sent, ``400 Bad Request`` is returned.  This can happen when the request
-payload was malformed, or when the requested domain name is unavailable or
-invalid (e.g. because another user owns it, or due to policy reasons).
+payload was malformed, or when the requested domain name is unavailable
+(because it conflicts with another user's zone) or invalid (due to policy, see
+below).
 
 If you have reached the maximum number of domains for your account, the API
 responds with ``403 Forbidden``.
 
 Restrictions on what is a valid domain name apply.  In particular, domains
-listed on the `Public Suffix List`_ cannot be registered.  (If you operate a
-public suffix and would like to host it with deSEC, that's certainly possible;
-please contact our support.)
+listed on the `Public Suffix List`_ such as ``co.uk`` cannot be registered.
+(If you operate a public suffix and would like to host it with deSEC, that's
+certainly possible; please contact support.) Also, domains ending with
+``.internal`` cannot be registered.
 
 .. _Public Suffix List: https://publicsuffix.org/