Browse Source

feat(api): enforce 500-byte limit on RR contents

See https://github.com/PowerDNS/pdns/issues/8012 and comments
in models.py
Peter Thomassen 5 years ago
parent
commit
591e15ec68
2 changed files with 24 additions and 3 deletions
  1. 11 3
      api/desecapi/models.py
  2. 13 0
      api/desecapi/tests/test_rrsets.py

+ 11 - 3
api/desecapi/models.py

@@ -422,9 +422,17 @@ class RRManager(Manager):
 class RR(ExportModelOperationsMixin('RR'), models.Model):
 class RR(ExportModelOperationsMixin('RR'), models.Model):
     created = models.DateTimeField(auto_now_add=True)
     created = models.DateTimeField(auto_now_add=True)
     rrset = models.ForeignKey(RRset, on_delete=models.CASCADE, related_name='records')
     rrset = models.ForeignKey(RRset, on_delete=models.CASCADE, related_name='records')
-    # max_length is determined based on the calculation in
-    # https://lists.isc.org/pipermail/bind-users/2008-April/070148.html
-    content = models.CharField(max_length=4092)
+    # The pdns lmdb backend used on our slaves does not only store the record contents itself, but other metadata (such
+    # as type etc.) Both together have to fit into the lmdb backend's current total limit of 512 bytes per RR, see
+    # https://github.com/PowerDNS/pdns/issues/8012
+    # I found the additional data to be 12 bytes (by trial and error). I believe these are the 12 bytes mentioned here:
+    # https://lists.isc.org/pipermail/bind-users/2008-April/070137.html So we can use 500 bytes for the actual content.
+    # Note: This is a conservative estimate, as record contents may be stored more efficiently depending on their type,
+    # effectively allowing a longer length in "presentation format". For example, A record contents take up 4 bytes,
+    # although the presentation format (usual IPv4 representation) takes up to 15 bytes. Similarly, OPENPGPKEY contents
+    # are base64-decoded before storage in binary format, so a "presentation format" value (which is the value our API
+    # sees) can have up to 668 bytes. Instead of introducing per-type limits, setting it to 500 should always work.
+    content = models.CharField(max_length=500)  #
 
 
     objects = RRManager()
     objects = RRManager()
 
 

+ 13 - 0
api/desecapi/tests/test_rrsets.py

@@ -196,6 +196,19 @@ class AuthenticatedRRSetTestCase(AuthenticatedRRSetBaseTestCase):
         response = self.client.post_rr_set(self.other_domain.name, **data)
         response = self.client.post_rr_set(self.other_domain.name, **data)
         self.assertStatus(response, status.HTTP_404_NOT_FOUND)
         self.assertStatus(response, status.HTTP_404_NOT_FOUND)
 
 
+    def test_create_my_rr_sets_too_long_content(self):
+        def _create_data(length):
+            content_string = 'A' * (length - 2)  # we have two quotes
+            return {'records': [f'"{content_string}"'], 'ttl': 3600, 'type': 'TXT', 'subname': f'name{length}'}
+
+        with self.assertPdnsRequests(self.requests_desec_rr_sets_update(self.my_empty_domain.name)):
+            response = self.client.post_rr_set(self.my_empty_domain.name, **_create_data(500))
+            self.assertStatus(response, status.HTTP_201_CREATED)
+
+        response = self.client.post_rr_set(self.my_empty_domain.name, **_create_data(501))
+        self.assertStatus(response, status.HTTP_400_BAD_REQUEST)
+        self.assertIn('Ensure this field has no more than 500 characters.', str(response.data))
+
     def test_create_my_rr_sets_twice(self):
     def test_create_my_rr_sets_twice(self):
         data = {'records': ['1.2.3.4'], 'ttl': 3660, 'type': 'A'}
         data = {'records': ['1.2.3.4'], 'ttl': 3660, 'type': 'A'}
         with self.assertPdnsRequests(self.requests_desec_rr_sets_update(self.my_empty_domain.name)):
         with self.assertPdnsRequests(self.requests_desec_rr_sets_update(self.my_empty_domain.name)):