attributes: Additional IP address validation

Introduce an additional IP address validation instead of assuming
that netaddr provides it.  Namely, it ensures that an address
either has ':' (IPv6) or 3 periods like 'xx.xx.xx.xx'. (IPv4)

The "'1' * 59" test case recently introduced by
commit 1681f62ec91b6c3705a14393815542dc1746de71 fails on
some platforms because it's considered a valid address by
their inet_aton.  Examples of such platforms: NetBSD, OS X

While one might argue it's a fault of the platforms, this is
a historical behavior which is probably too late to change there.

(The breakage has been hidden by later UT changes in
commit 35662d07628452d14306f5197871ad64f6396ff3 .
This commit includes a UT change to uncover the problem again.)

Closes-Bug: #1394867
Related-Bug: #1378450
Change-Id: Ibe02f8b7c4d437bf7abbe7304ca138bdcf4bfdb9

diff --git a/neutron/api/v2/attributes.py b/neutron/api/v2/attributes.py
index 8959120b90a6928df246ebb599f8604e0f740e2f..d270c797b2d6eb768bca373dab44be00f3c4be31 100644 (file)
--- a/neutron/api/v2/attributes.py
+++ b/neutron/api/v2/attributes.py
@@ -173,6 +173,21 @@ def _validate_mac_address_or_none(data, valid_values=None):
 def _validate_ip_address(data, valid_values=None):
     try:
         netaddr.IPAddress(_validate_no_whitespace(data))
+        # The followings are quick checks for IPv6 (has ':') and
+        # IPv4.  (has 3 periods like 'xx.xx.xx.xx')
+        # NOTE(yamamoto): netaddr uses libraries provided by the underlying
+        # platform to convert addresses.  For example, inet_aton(3).
+        # Some platforms, including NetBSD and OS X, have inet_aton
+        # implementation which accepts more varying forms of addresses than
+        # we want to accept here.  The following check is to reject such
+        # addresses.  For Example:
+        #   >>> netaddr.IPAddress('1' * 59)
+        #   IPAddress('199.28.113.199')
+        #   >>> netaddr.IPAddress(str(int('1' * 59) & 0xffffffff))
+        #   IPAddress('199.28.113.199')
+        #   >>>
+        if ':' not in data and data.count('.') != 3:
+            raise ValueError()
     except Exception:
         msg = _("'%s' is not a valid IP address") % data
         LOG.debug(msg)

diff --git a/neutron/tests/unit/test_attributes.py b/neutron/tests/unit/test_attributes.py
index 64d7a9b8ba23f22b486981b8b78f21bc743c67c9..daa7b0930e425402fe518c9293e9241125d36d78 100644 (file)
--- a/neutron/tests/unit/test_attributes.py
+++ b/neutron/tests/unit/test_attributes.py
@@ -16,6 +16,8 @@
 import string
 import testtools
 
+import mock
+
 from neutron.api.v2 import attributes
 from neutron.common import exceptions as n_exc
 from neutron.tests import base
@@ -215,6 +217,12 @@ class TestAttributes(base.BaseTestCase):
         msg = attributes._validate_ip_address(ip_addr)
         self.assertEqual("'%s' is not a valid IP address" % ip_addr, msg)
 
+        # Depending on platform to run UTs, this case might or might not be
+        # an equivalent to test_validate_ip_address_bsd.
+        ip_addr = '1' * 59
+        msg = attributes._validate_ip_address(ip_addr)
+        self.assertEqual("'%s' is not a valid IP address" % ip_addr, msg)
+
         ip_addr = '1.1.1.1 has whitespace'
         msg = attributes._validate_ip_address(ip_addr)
         self.assertEqual("'%s' is not a valid IP address" % ip_addr, msg)
@@ -237,6 +245,18 @@ class TestAttributes(base.BaseTestCase):
             msg = attributes._validate_ip_address(ip_addr)
             self.assertEqual("'%s' is not a valid IP address" % ip_addr, msg)
 
+    def test_validate_ip_address_bsd(self):
+        # NOTE(yamamoto):  On NetBSD and OS X, netaddr.IPAddress() accepts
+        # '1' * 59 as a valid address.  The behaviour is inherited from
+        # libc behaviour there.  This test ensures that our validator reject
+        # such addresses on such platforms by mocking netaddr to emulate
+        # the behaviour.
+        ip_addr = '1' * 59
+        with mock.patch('netaddr.IPAddress') as ip_address_cls:
+            msg = attributes._validate_ip_address(ip_addr)
+        ip_address_cls.assert_called_once_with(ip_addr)
+        self.assertEqual("'%s' is not a valid IP address" % ip_addr, msg)
+
     def test_validate_ip_pools(self):
         pools = [[{'end': '10.0.0.254'}],
                  [{'start': '10.0.0.254'}],