Fix hostname validation for nameservers

Fixes the hostname validation to align with the RFC's demands[1].
This was done by replacing the full regex with a function that
broke the FQDN into individual components that were easier to
reason about with regular expressions.

Also added several test cases for domains so if someone wants
to convert it back to pure regex there will be better test vectors.

1. RFC 1123 says an all-digit hostname is allowed in section 2.1. It
   says that this more liberal syntax MUST be supported.

Conflicts:
	neutron/tests/unit/test_attributes.py

Closes-Bug: #1396932
Change-Id: I003cf14d95070707e43e40d55da62e11a28dfa4e
(cherry picked from commit 35662d07628452d14306f5197871ad64f6396ff3)

2 files changed, 72 insertions, 23 deletions

diff --git a/neutron/api/v2/attributes.py b/neutron/api/v2/attributes.py
index 21486dbd39..85e9b80cf1 100644
--- a/neutron/api/v2/attributes.py
+++ b/neutron/api/v2/attributes.py
@@ -235,27 +235,38 @@ def _validate_fixed_ips(data, valid_values=None):
                 return msg
 
 
+def _validate_ip_or_hostname(host):
+    ip_err = _validate_ip_address(host)
+    if not ip_err:
+        return
+    name_err = _validate_hostname(host)
+    if not name_err:
+        return
+    msg = _("%(host)s is not a valid IP or hostname. Details: "
+            "%(ip_err)s, %(name_err)s") % {'ip_err': ip_err, 'host': host,
+                                           'name_err': name_err}
+    return msg
+
+
 def _validate_nameservers(data, valid_values=None):
     if not hasattr(data, '__iter__'):
         msg = _("Invalid data format for nameserver: '%s'") % data
         LOG.debug(msg)
         return msg
 
-    ips = []
-    for ip in data:
-        msg = _validate_ip_address(ip)
+    hosts = []
+    for host in data:
+        # This may be an IP or a hostname
+        msg = _validate_ip_or_hostname(host)
         if msg:
-            # This may be a hostname
-            msg = _validate_regex(ip, HOSTNAME_PATTERN)
-            if msg:
-                msg = _("'%s' is not a valid nameserver") % ip
-                LOG.debug(msg)
-                return msg
-        if ip in ips:
-            msg = _("Duplicate nameserver '%s'") % ip
+            msg = _("'%(host)s' is not a valid nameserver. %(msg)s") % {
+                'host': host, 'msg': msg}
+            return msg
+        if host in hosts:
+            msg = _("Duplicate nameserver '%s'") % host
             LOG.debug(msg)
             return msg
-        ips.append(ip)
+        hosts.append(host)
 
 
 def _validate_hostroutes(data, valid_values=None):
@@ -332,6 +343,41 @@ def _validate_subnet_or_none(data, valid_values=None):
     return _validate_subnet(data, valid_values)
 
 
+def _validate_hostname(data):
+    # NOTE: An individual name regex instead of an entire FQDN was used
+    # because its easier to make correct. Feel free to replace with a
+    # full regex solution. The logic should validate that the hostname
+    # matches RFC 1123 (section 2.1) and RFC 952.
+    hostname_pattern = "[a-zA-Z0-9-]{1,63}$"
+    try:
+        # Trailing periods are allowed to indicate that a name is fully
+        # qualified per RFC 1034 (page 7).
+        trimmed = data if data[-1] != '.' else data[:-1]
+        if len(trimmed) > 255:
+            raise TypeError(
+                _("'%s' exceeds the 255 character hostname limit") % trimmed)
+        names = trimmed.split('.')
+        for name in names:
+            if not name:
+                raise TypeError(_("Encountered an empty component."))
+            if name[-1] == '-' or name[0] == '-':
+                raise TypeError(
+                    _("Name '%s' must not start or end with a hyphen.") % name)
+            if not re.match(hostname_pattern, name):
+                raise TypeError(
+                    _("Name '%s' must be 1-63 characters long, each of "
+                      "which can only be alphanumeric or a hyphen.") % name)
+        # RFC 1123 hints that a TLD can't be all numeric. last is a TLD if
+        # it's an FQDN.
+        if len(names) > 1 and re.match("^[0-9]+$", names[-1]):
+            raise TypeError(_("TLD '%s' must not be all numeric") % names[-1])
+    except TypeError as e:
+        msg = _("'%(data)s' is not a valid hostname. Reason: %(reason)s") % {
+            'data': data, 'reason': e.message}
+        LOG.debug(msg)
+        return msg
+
+
 def _validate_regex(data, valid_values=None):
     try:
         if re.match(valid_values, data):
@@ -540,9 +586,6 @@ def convert_to_list(data):
         return [data]
 
 
-HOSTNAME_PATTERN = ("(?=^.{1,254}$)(^(?:(?!\d+.|-)[a-zA-Z0-9_\-]{1,62}"
-                    "[a-zA-Z0-9]\.?)+(?:[a-zA-Z]{2,})$)")
-
 HEX_ELEM = '[0-9A-Fa-f]'
 UUID_PATTERN = '-'.join([HEX_ELEM + '{8}', HEX_ELEM + '{4}',
                          HEX_ELEM + '{4}', HEX_ELEM + '{4}',
diff --git a/neutron/tests/unit/test_attributes.py b/neutron/tests/unit/test_attributes.py
index f8cb462b3e..2707af27f9 100644
--- a/neutron/tests/unit/test_attributes.py
+++ b/neutron/tests/unit/test_attributes.py
@@ -279,9 +279,7 @@ class TestAttributes(base.BaseTestCase):
     def test_validate_nameservers(self):
         ns_pools = [['1.1.1.2', '1.1.1.2'],
                     ['www.hostname.com', 'www.hostname.com'],
-                    ['77.hostname.com'],
                     ['1000.0.0.1'],
-                    ['111111111111111111111111111111111111111111111111111111111111'],  # noqa
                     None]
 
         for ns in ns_pools:
@@ -292,6 +290,8 @@ class TestAttributes(base.BaseTestCase):
                     ['www.hostname.com'],
                     ['www.great.marathons.to.travel'],
                     ['valid'],
+                    ['77.hostname.com'],
+                    ['1' * 59],
                     ['www.internal.hostname.com']]
 
         for ns in ns_pools:
@@ -342,13 +342,19 @@ class TestAttributes(base.BaseTestCase):
         self.assertEqual(msg, "'%s' is not a valid IP address" % ip_addr)
 
     def test_hostname_pattern(self):
-        data = '@openstack'
-        msg = attributes._validate_regex(data, attributes.HOSTNAME_PATTERN)
-        self.assertIsNotNone(msg)
+        bad_values = ['@openstack', 'ffff.abcdefg' * 26, 'f' * 80, '-hello',
+                      'goodbye-', 'example..org']
+        for data in bad_values:
+            msg = attributes._validate_hostname(data)
+            self.assertIsNotNone(msg)
 
-        data = 'www.openstack.org'
-        msg = attributes._validate_regex(data, attributes.HOSTNAME_PATTERN)
-        self.assertIsNone(msg)
+        # All numeric hostnames are allowed per RFC 1123 section 2.1
+        good_values = ['www.openstack.org', '1234x', '1234',
+                       'openstack-1', 'v.xyz', '1' * 50, 'a1a',
+                       'x.x1x', 'x.yz', 'example.org.']
+        for data in good_values:
+            msg = attributes._validate_hostname(data)
+            self.assertIsNone(msg)
 
     def test_uuid_pattern(self):
         data = 'garbage'