mbasti-rh's pull request #58: "Ip addr validation" was synchronize
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 7fc0b28b05acca51ffbdfbb04a7e1dc4212ae9a0 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 8de9acf..8a9aa0e 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): @@ -199,14 +199,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 f048351..a5f11a4 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 e4167ec9df06a0508602968ea9d9b69b370a56c5 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 | 4 +++- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py index 8a9aa0e..6ef39ab 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): @@ -199,15 +198,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..be59665 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), 8), ('224.5.6.7',), - ('10.11.12.255/24',), + ('10.11.12.255/24', (10, 11, 12, 255), 24), + ('255.255.255.255',), ('::/0',), ('2001::1', (0x2001, 0, 0, 0, 0, 0, 0, 1), 64), From 8e415a96a239dc3a4f5ffdceb9b9c48c2cdd97cf 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 a5f11a4..5510a66 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 088b027aa6363b92805ca96ab73eee589e05fcba 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 ++++++++++ ipaserver/install/server/replicainstall.py | 43 ++++++++++++++++++++++++++++++ 3 files changed, 71 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: diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py index c73600c..5edc002 100644 --- a/ipaserver/install/server/replicainstall.py +++ b/ipaserver/install/server/replicainstall.py @@ -13,6 +13,7 @@ import os import shutil import socket +import sys import tempfile import six @@ -735,6 +736,28 @@ def install_check(installer): config.host_name, not installer.interactive, False, options.ip_addresses) + for ip in config.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 + ) + except errors.ACIError: raise ScriptError("\nThe password provided is incorrect for LDAP server " "%s" % config.master_host_name) @@ -1306,6 +1329,26 @@ def promote_check(installer): config.host_name, not installer.interactive, False, options.ip_addresses) + for ip in config.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 + ) + except errors.ACIError: raise ScriptError("\nInsufficient privileges to promote the server.") except errors.LDAPError:
-- 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
