From: YAMAMOTO Takashi Date: Fri, 21 Nov 2014 05:16:03 +0000 (+0900) Subject: attributes: Additional IP address validation X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=2794bb89d664355ae1194a0b1f8346c1538caef8;p=openstack-build%2Fneutron-build.git 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 8959120b9..d270c797b 100644 --- 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 64d7a9b8b..daa7b0930 100644 --- 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'}],