mbasti-rh's pull request #58: "Ip addr validation" was opened PR body: """
""" See the full pull-request at https://github.com/freeipa/freeipa/pull/58 ... or pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/58/head:pr58 git checkout pr58
From 7febc1af14637623a649bf0c43a540023744c057 Mon Sep 17 00:00:00 2001 From: Martin Basti <mba...@redhat.com> Date: Fri, 2 Sep 2016 13:25:19 +0200 Subject: [PATCH 1/4] Allow network ip addresses Currently cloud environments uses heavily prefix /32 (/128) what makes IPA validators to fail. IPA should not care if IP address is network or not. This commit allows usage of network addresses in: * host plugin * dns plugin * server-installer * client-installer https://fedorahosted.org/freeipa/ticket/5814 --- ipapython/ipautil.py | 9 +++++---- ipaserver/plugins/dns.py | 5 ++--- ipatests/test_ipapython/test_ipautil.py | 6 ++++-- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py index 9536543..6624e3c 100644 --- a/ipapython/ipautil.py +++ b/ipapython/ipautil.py @@ -132,8 +132,8 @@ class CheckedIPAddress(UnsafeIPAddress): Reserved or link-local addresses are never accepted. """ def __init__(self, addr, match_local=False, parse_netmask=True, - allow_network=False, allow_loopback=False, - allow_broadcast=False, allow_multicast=False): + allow_loopback=False, allow_broadcast=False, + allow_multicast=False): super(CheckedIPAddress, self).__init__(addr) if isinstance(addr, CheckedIPAddress): @@ -194,14 +194,15 @@ def __init__(self, addr, match_local=False, parse_netmask=True, elif self.version == 6: self._net = netaddr.IPNetwork(str(self) + '/64') - if not allow_network and self == self._net.network: - raise ValueError("cannot use IP network address {}".format(addr)) if not allow_broadcast and (self.version == 4 and self == self._net.broadcast): raise ValueError("cannot use broadcast IP address {}".format(addr)) self.prefixlen = self._net.prefixlen + def is_network_addr(self): + return self == self._net.network + def valid_ip(addr): return netaddr.valid_ipv4(addr) or netaddr.valid_ipv6(addr) diff --git a/ipaserver/plugins/dns.py b/ipaserver/plugins/dns.py index 6f1bd71..3b8d80b 100644 --- a/ipaserver/plugins/dns.py +++ b/ipaserver/plugins/dns.py @@ -413,8 +413,7 @@ def _validate_bind_aci(ugettext, bind_acis): bind_aci = bind_aci[1:] try: - ip = CheckedIPAddress(bind_aci, parse_netmask=True, - allow_network=True, allow_loopback=True) + CheckedIPAddress(bind_aci, parse_netmask=True, allow_loopback=True) except (netaddr.AddrFormatError, ValueError) as e: return unicode(e) except UnboundLocalError: @@ -439,7 +438,7 @@ def _normalize_bind_aci(bind_acis): try: ip = CheckedIPAddress(bind_aci, parse_netmask=True, - allow_network=True, allow_loopback=True) + allow_loopback=True) if '/' in bind_aci: # addr with netmask netmask = "/%s" % ip.prefixlen else: diff --git a/ipatests/test_ipapython/test_ipautil.py b/ipatests/test_ipapython/test_ipautil.py index 8c0b9c4..ea9251b 100644 --- a/ipatests/test_ipapython/test_ipautil.py +++ b/ipatests/test_ipapython/test_ipautil.py @@ -44,6 +44,7 @@ def check_ipaddress(): def test_ip_address(): addrs = [ + ('0.0.0.0/0',), ('10.11.12.13', (10, 11, 12, 13), 8), ('10.11.12.13/14', (10, 11, 12, 13), 14), ('10.11.12.13%zoneid',), @@ -53,10 +54,11 @@ def test_ip_address(): ('127.0.0.1',), ('241.1.2.3',), ('169.254.1.2',), - ('10.11.12.0/24',), + ('10.11.12.0/24', (10, 11, 12, 0), 24), ('224.5.6.7',), ('10.11.12.255/24',), + ('::/0',), ('2001::1', (0x2001, 0, 0, 0, 0, 0, 0, 1), 64), ('2001::1/72', (0x2001, 0, 0, 0, 0, 0, 0, 1), 72), ('2001::1%zoneid', (0x2001, 0, 0, 0, 0, 0, 0, 1), 64), @@ -66,7 +68,7 @@ def test_ip_address(): ('::1',), ('6789::1',), ('fe89::1',), - ('2001::/64',), + ('2001::/64', (0x2001, 0, 0, 0, 0, 0, 0, 0), 64), ('ff01::1',), ('junk',) From caccb00eded5bac23876c6c3ed1ec927c0e89197 Mon Sep 17 00:00:00 2001 From: Martin Basti <mba...@redhat.com> Date: Fri, 2 Sep 2016 17:07:03 +0200 Subject: [PATCH 2/4] Allow broadcast ip addresses Currently environments may use prefix /31 on point-to-point connections what makes IPA validators to fail. IPA should not care if IP address is broadcast or not. In some cases (when prefix is not specified) IPA cannot decide properly if broadcast address is really broadcast. This commit allows usage of broadcast addresses in: * host plugin * dns plugin * server-installer * client-installer https://fedorahosted.org/freeipa/ticket/5814 --- ipapython/ipautil.py | 10 ++++------ ipatests/test_ipapython/test_ipautil.py | 2 ++ 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py index 6624e3c..365c0fa 100644 --- a/ipapython/ipautil.py +++ b/ipapython/ipautil.py @@ -132,8 +132,7 @@ class CheckedIPAddress(UnsafeIPAddress): Reserved or link-local addresses are never accepted. """ def __init__(self, addr, match_local=False, parse_netmask=True, - allow_loopback=False, allow_broadcast=False, - allow_multicast=False): + allow_loopback=False, allow_multicast=False): super(CheckedIPAddress, self).__init__(addr) if isinstance(addr, CheckedIPAddress): @@ -194,15 +193,14 @@ def __init__(self, addr, match_local=False, parse_netmask=True, elif self.version == 6: self._net = netaddr.IPNetwork(str(self) + '/64') - if not allow_broadcast and (self.version == 4 and - self == self._net.broadcast): - raise ValueError("cannot use broadcast IP address {}".format(addr)) - self.prefixlen = self._net.prefixlen def is_network_addr(self): return self == self._net.network + def is_broadcast_addr(self): + return self.version == 4 and self == self._net.broadcast + def valid_ip(addr): return netaddr.valid_ipv4(addr) or netaddr.valid_ipv6(addr) diff --git a/ipatests/test_ipapython/test_ipautil.py b/ipatests/test_ipapython/test_ipautil.py index ea9251b..ac7bfc9 100644 --- a/ipatests/test_ipapython/test_ipautil.py +++ b/ipatests/test_ipapython/test_ipautil.py @@ -55,8 +55,10 @@ def test_ip_address(): ('241.1.2.3',), ('169.254.1.2',), ('10.11.12.0/24', (10, 11, 12, 0), 24), + ('10.0.0.255', (10, 0, 0, 255), 24), ('224.5.6.7',), ('10.11.12.255/24',), + ('255.255.255.255',), ('::/0',), ('2001::1', (0x2001, 0, 0, 0, 0, 0, 0, 1), 64), From d1ad8ba025b527472766e9f16a023145568d9bdd Mon Sep 17 00:00:00 2001 From: Martin Basti <mba...@redhat.com> Date: Fri, 2 Sep 2016 17:22:24 +0200 Subject: [PATCH 3/4] Allow multicast addresses in A/AAAA records There is no reason (RFC) why we should prevent users to add multicast addresses to A/AAAA records https://fedorahosted.org/freeipa/ticket/5814 --- ipaserver/plugins/dns.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ipaserver/plugins/dns.py b/ipaserver/plugins/dns.py index 3b8d80b..5cbfe6e 100644 --- a/ipaserver/plugins/dns.py +++ b/ipaserver/plugins/dns.py @@ -566,7 +566,8 @@ def add_records_for_host_validation(option_name, host, domain, ip_addresses, che for ip_address in ip_addresses: try: - ip = CheckedIPAddress(ip_address, match_local=False) + ip = CheckedIPAddress( + ip_address, match_local=False, allow_multicast=True) except Exception as e: raise errors.ValidationError(name=option_name, error=unicode(e)) @@ -597,7 +598,8 @@ def add_records_for_host(host, domain, ip_addresses, add_forward=True, add_rever ip_addresses = [ip_addresses] for ip_address in ip_addresses: - ip = CheckedIPAddress(ip_address, match_local=False) + ip = CheckedIPAddress( + ip_address, match_local=False, allow_multicast=True) if add_forward: add_forward_record(domain, host, unicode(ip)) From fd8fdf38103fbac388460b896c17693a3edd7dae Mon Sep 17 00:00:00 2001 From: Martin Basti <mba...@redhat.com> Date: Fri, 2 Sep 2016 17:39:01 +0200 Subject: [PATCH 4/4] Show warning when net/broadcast IP address is used in installer https://fedorahosted.org/freeipa/ticket/5814 --- client/ipa-client-install | 14 ++++++++++++++ ipaserver/install/server/install.py | 14 ++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/client/ipa-client-install b/client/ipa-client-install index 4a263b3..6330f1d 100755 --- a/client/ipa-client-install +++ b/client/ipa-client-install @@ -1651,6 +1651,20 @@ def update_dns(server, hostname, options): root_logger.info("Failed to determine this machine's ip address(es).") return + for ip in update_ips: + if ip.is_network_addr(): + root_logger.warning("IP address %s might be network address", ip) + # fixme: once when loggers will be fixed, we can remove this print + print( + "WARNING: IP address {} might be network address".format(ip), + file=sys.stderr) + if ip.is_broadcast_addr(): + root_logger.warning("IP address %s might be broadcast address", ip) + # fixme: once when loggers will be fixed, we can remove this print + print( + "WARNING: IP address {} might be broadcast address".format(ip), + file=sys.stderr) + update_txt = "debug\n" update_txt += ipautil.template_str(DELETE_TEMPLATE_A, dict(HOSTNAME=hostname)) diff --git a/ipaserver/install/server/install.py b/ipaserver/install/server/install.py index 6644a6b..2ef9bdb 100644 --- a/ipaserver/install/server/install.py +++ b/ipaserver/install/server/install.py @@ -609,6 +609,20 @@ def install_check(installer): not installer.interactive, False, options.ip_addresses) + for ip in ip_addresses: + if ip.is_network_addr(): + root_logger.warning("IP address %s might be network address", ip) + # fixme: once when loggers will be fixed, we can remove this print + print( + "WARNING: IP address {} might be network address".format(ip), + file=sys.stderr) + if ip.is_broadcast_addr(): + root_logger.warning("IP address %s might be broadcast address", ip) + # fixme: once when loggers will be fixed, we can remove this print + print( + "WARNING: IP address {} might be broadcast address".format(ip), + file=sys.stderr) + # installer needs to update hosts file when DNS subsystem will be # installed or custom addresses are used if options.ip_addresses or options.setup_dns:
-- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
