Support for RFC-1035 label length validation #76
Comments
|
As a starting point for discussion, I've put the attached patch together. Probably still needs some work, but the basics for the concepts described above are present; interested in your thoughts/suggestions. Admin Edit, converting patch to code block diff --git a/nautobot_dns_models/__init__.py b/nautobot_dns_models/__init__.py
index da570d9..4a01c22 100644
--- a/nautobot_dns_models/__init__.py
+++ b/nautobot_dns_models/__init__.py
@@ -27,6 +27,15 @@ class NautobotDnsModelsConfig(NautobotAppConfig):
default_settings = {}
caching_config = {}
docs_view_name = "plugins:nautobot_dns_models:docs"
+ default_settings = {
+ "rfc1035_validation": False,
+ "validation_regex": "",
+ }
+
+ def ready(self):
+ super().ready()
+
+ import nautobot_dns_models.signals # noqa: F401
config = NautobotDnsModelsConfig # pylint:disable=invalid-name
diff --git a/nautobot_dns_models/custom_validators.py b/nautobot_dns_models/custom_validators.py
new file mode 100644
index 0000000..675a6eb
--- /dev/null
+++ b/nautobot_dns_models/custom_validators.py
@@ -0,0 +1,121 @@
+"""Validation classes for DNS records."""
+import re
+
+from django.conf import settings
+from nautobot.apps.models import CustomValidator
+
+from . import config as app_config
+from .models import DNSZoneModel
+
+
+class DNSNameValidator(CustomValidator):
+ """Base class for validating DNS records."""
+ model: str = ""
+
+ def clean(self):
+ """Run the validation."""
+ name = self.context["object"].name
+ zone = self.context["object"].zone
+ self._validate_name(name, zone)
+
+ def _validate_fqdn_length(self, fqdn: str):
+ print(f"Validating FQDN '{fqdn}'")
+ if len(f"{fqdn}.") > 255:
+ self.validation_error(
+ {"name": "FQDN is > 255 characters"}
+ )
+
+ def _validate_name(self, name: str, zone: DNSZoneModel):
+ plugin_config = settings.PLUGINS_CONFIG[app_config.name]
+
+ self._validate_length(name, zone)
+
+ if plugin_config["rfc1035_validation"]:
+ self._validate_rfc1035()
+
+ if plugin_config["validation_regex"]:
+ self._validate_regex(plugin_config["validation_regex"])
+
+ def _validate_length(self, name: str, zone: DNSZoneModel):
+ """Validate the overall length of the FQDN and the length of each label."""
+ label_list = name.split(".")
+
+ #
+ # The one validation everyone agrees on is the limits defined by
+ # wire format.
+ self._validate_fqdn_length(f"{name}.{zone.name}")
+ for label in label_list:
+ if len(label) > 63:
+ self.validation_error(
+ {"name": "Contains label > 63 characters"}
+ )
+
+ def _validate_rfc1035(self):
+ """Perform character-requirement validation defined by RFC-1035."""
+ #
+ # RFC1035 §2.3.1
+ regex = "^(?:[a-z][a-z0-9-]+[a-z0-9])$"
+
+ name = self.context["object"].name
+ label_list = name.split(".")
+
+ for label in label_list:
+ print(f"[{name}] Checking if label '{label}' matches regex '{regex}'")
+ if not re.search(regex, label, re.IGNORECASE):
+ self.validation_error(
+ {"name": "Does not meet RFC-1035 requirements"}
+ )
+
+ def _validate_regex(self, regex):
+ """User-supplied regex validation."""
+ if not re.search(regex, self.context["object"].name):
+ self.validation_error(
+ {"name": f"Does not match regex '{regex}'"}
+ )
+
+
+class RDataFQDNValidator(DNSNameValidator):
+ """Class for validating record types which have FQDNs in RDATA."""
+ rdata_field_name: str = ""
+
+ def clean(self):
+ """Run the validation."""
+ super().clean()
+
+ self._validate_fqdn_length(
+ getattr(self.context["object"], self.rdata_field_name)
+ )
+
+
+class ARecordModelValidator(DNSNameValidator):
+ """A record validator."""
+ model: str = "nautobot_dns_models.arecordmodel"
+
+
+class AAAARecordModelValidator(DNSNameValidator):
+ """AAAA record validator."""
+ model: str = "nautobot_dns_models.aaaarecordmodel"
+
+
+
+class NSRecordModelValidator(RDataFQDNValidator):
+ """NS Record validation (not yet implemented)."""
+ rdata_field_name: str = "server"
+
+
+class CNAMERecordModelValidator(RDataFQDNValidator):
+ """CNAME record validator."""
+ rdata_field_name: str = "alias"
+
+
+class MXRecordModelValidator(RDataFQDNValidator):
+ """MX record validator."""
+ rdata_field_name: str = "mail_server"
+
+
+custom_validators = [
+ ARecordModelValidator,
+ AAAARecordModelValidator,
+ CNAMERecordModelValidator,
+ MXRecordModelValidator,
+] |
|
I do see the value of implementing/enforcing RFC 1035. I'd argue against the custom regex placeholder though as this is already achievable with the Data Validation Engine App and re-implementing the functionality specifically for DNS would be non-optimal. |
|
Yeah; somewhere in the Many, Many™ browser tabs I currently have open is the start of a reply to this ticket which says more or less the same thing. I didn't learn about that plugin until after I submitted this ticket. Could/should the DNS plugin be set up to create the appropriate objects in the data validator automatically if the data validator plugin is installed? |
|
In general, we're trying to keep cross-plugin dependencies at a minimum so I doubt if that's the path forward. Here's how I think of it:
Makes sense? |
|
It does and I agree that the RFC1035 wire-protocol checks should be required, although a bit of care may be needed to properly support non-delegated subdomains, e.g. someone creates a hostname called Related questions:
|
|
Here's an updated patch which only enforces the wire protocol length requirements from the RFC. Thoughts? diff --git a/nautobot_dns_models/models.py b/nautobot_dns_models/models.py
index 72ae172..fdb7506 100644
--- a/nautobot_dns_models/models.py
+++ b/nautobot_dns_models/models.py
@@ -1,5 +1,6 @@
"""Models for Nautobot DNS Models."""
+from django.core.exceptions import ValidationError
from django.core.validators import MaxValueValidator, MinValueValidator
from django.db import models
from nautobot.apps.models import PrimaryModel, extras_features
@@ -90,6 +91,25 @@ class DNSZoneModel(DNSModel):
verbose_name = "DNS Zone"
verbose_name_plural = "DNS Zones"
+ def clean(self):
+ """Validate the zone name conforms to DNS label length restrictions.
+
+ DNS name length restrictions (RFC 1035 §3.1):
+ - Each label is limited to 63 octets
+ - Empty labels are not allowed
+ """
+ super().clean()
+
+ # Split name into labels
+ zone_label_list = self.name.split(".")
+
+ # Check each label
+ for zone_label in zone_label_list:
+ if len(zone_label) > 63:
+ raise ValidationError({"name": f"Label '{zone_label}' exceeds maximum length of 63 characters"})
+ if not zone_label:
+ raise ValidationError({"name": "Empty labels are not allowed"})
+
class DNSRecordModel(DNSModel): # pylint: disable=too-many-ancestors
"""Primary Dns Record model for plugin."""
@@ -102,6 +122,45 @@ class DNSRecordModel(DNSModel): # pylint: disable=too-many-ancestors
description = models.TextField(help_text="Description of the Record.", blank=True)
comment = models.CharField(max_length=200, help_text="Comment for the Record.", blank=True)
+ def clean(self):
+ """Validate the record name conforms to DNS label length restrictions.
+
+ DNS name length restrictions (RFC 1035 §3.1):
+ - Each label is limited to 63 octets
+ - The total length of a domain name is limited to 255 octets
+ - The length of each label is stored in a single octet
+ - The final length octet must be zero (root)
+
+ Wire format calculation:
+ - Each label: 1 octet (length) + label content
+ - Final root: 1 octet (zero length)
+ - Total must not exceed 255 octets
+ """
+ super().clean()
+
+ # Split names into labels
+ record_label_list = self.name.split(".")
+ zone_label_list = self.zone.name.split(".")
+
+ # Check each label in the relative name
+ for record_label in record_label_list:
+ if len(record_label) > 63:
+ raise ValidationError({"name": f"Label '{record_label}' exceeds maximum length of 63 characters"})
+ if not record_label:
+ raise ValidationError({"name": "Empty labels are not allowed"})
+
+ # Calculate wire format length including zone name
+ # - 1 length octet + label length for each record label
+ # - 1 length octet + label length for each zone label
+ # - 1 octet for root label (zero length)
+ wire_length = (
+ sum(1 + len(record_label) for record_label in record_label_list)
+ + sum(1 + len(zone_label) for zone_label in zone_label_list)
+ + 1 # final zero length
+ )
+ if wire_length > 255:
+ raise ValidationError({"name": "Total length of DNS name cannot exceed 255 characters"})
+
class Meta:
"""Meta attributes for DnsRecordModel.""" |
nrnvgh
changed the title
nrnvgh
changed the title
|
LMK what you think; if it works for you, I'll put together a PR. |
6 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Environment
Proposed Functionality
Background
As-in, the app allows records to be created, even if they violate RFC. While allowed characters used in the DNS may vary from org to org, it's (almost?) universally agreed in that DNS records shouldn't be allowed if they violate the wire format. Additionally, many orgs lock down their names RFC-1035 §2.3.1 (
SRVrecords being the notable exception, which aren't currently supported by the app.Proposal:
Use Case
While I'm always open to being convinced, I can't think of a good reason to allow DNS names to be created in Nautobot which violate the spec (i.e. FQDN and label lengths). Additionally, orgs may want to apply their own restrictions as to what constitutes a valid DNS name.
The text was updated successfully, but these errors were encountered: