mbasti-rh's pull request #78: "Fix Ip-addr validation" was opened
PR body: """ """ See the full pull-request at https://github.com/freeipa/freeipa/pull/78 ... or pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/78/head:pr78 git checkout pr78
From 21df12c0c17265ac64cdcd5c962c8d7a46593827 Mon Sep 17 00:00:00 2001 From: Martin Basti <mba...@redhat.com> Date: Mon, 12 Sep 2016 14:36:14 +0200 Subject: [PATCH 1/4] Fix attribute error caused by config.ips in replica install config.ips is defined only when --setup-dns is not used https://fedorahosted.org/freeipa/ticket/5814 --- ipaserver/install/server/replicainstall.py | 38 +++++++++++++++--------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py index 5edc002..b0af221 100644 --- a/ipaserver/install/server/replicainstall.py +++ b/ipaserver/install/server/replicainstall.py @@ -1329,25 +1329,25 @@ 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 - ) + 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.") From 64cb464f3ff626eee37250b29b6b3845054f99d4 Mon Sep 17 00:00:00 2001 From: Martin Basti <mba...@redhat.com> Date: Mon, 12 Sep 2016 14:38:12 +0200 Subject: [PATCH 2/4] Abstract procedures for IP address warnings Originaly there should be only two occurencees of this warning, one for server, one for client. But obviously is not possible with current installers to achive this goal, so I have to extract code to not mess with 5 times copy and paste. https://fedorahosted.org/freeipa/ticket/5814 --- client/ipa-client-install | 19 +++--------- ipalib/util.py | 27 +++++++++++++++- ipaserver/install/server/install.py | 21 +++++-------- ipaserver/install/server/replicainstall.py | 49 +++++------------------------- 4 files changed, 46 insertions(+), 70 deletions(-) diff --git a/client/ipa-client-install b/client/ipa-client-install index 6330f1d..535fe65 100755 --- a/client/ipa-client-install +++ b/client/ipa-client-install @@ -55,7 +55,9 @@ try: from ipalib import api, errors from ipalib import x509, certstore from ipalib.util import ( - normalize_hostname, validate_domain_name, verify_host_resolvable) + normalize_hostname, validate_domain_name, verify_host_resolvable, + network_ip_address_warning, broadcast_ip_address_warning + ) from ipalib.constants import CACERT from ipapython.dn import DN from ipapython.ssh import SSHPublicKey @@ -1651,19 +1653,8 @@ 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) + network_ip_address_warning(update_ips) + broadcast_ip_address_warning(update_ips) update_txt = "debug\n" update_txt += ipautil.template_str(DELETE_TEMPLATE_A, diff --git a/ipalib/util.py b/ipalib/util.py index 8057740..785dd5f 100644 --- a/ipalib/util.py +++ b/ipalib/util.py @@ -21,7 +21,10 @@ Various utility functions. """ -from __future__ import absolute_import +from __future__ import ( + absolute_import, + print_function, +) import os import socket @@ -29,6 +32,7 @@ import decimal import dns import encodings +import sys from weakref import WeakKeyDictionary import netaddr @@ -45,6 +49,7 @@ from ipapython.dn import DN, RDN from ipapython.dnsutil import DNSName from ipapython.dnsutil import resolve_ip_addresses +from ipapython.ipa_log_manager import root_logger if six.PY3: unicode = str @@ -994,3 +999,23 @@ def check_principal_realm_in_trust_namespace(api_instance, *keys): name='krbprincipalname', error=_('realm or UPN suffix overlaps with trusted domain ' 'namespace')) + + +def network_ip_address_warning(addr_list): + for ip in addr_list: + 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) + + +def broadcast_ip_address_warning(addr_list): + for ip in addr_list: + 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) diff --git a/ipaserver/install/server/install.py b/ipaserver/install/server/install.py index 2ef9bdb..7733106 100644 --- a/ipaserver/install/server/install.py +++ b/ipaserver/install/server/install.py @@ -31,7 +31,11 @@ from ipaplatform.tasks import tasks from ipalib import api, constants, errors, x509 from ipalib.constants import CACERT -from ipalib.util import validate_domain_name +from ipalib.util import ( + validate_domain_name, + network_ip_address_warning, + broadcast_ip_address_warning, +) import ipaclient.ntpconf from ipaserver.install import ( bindinstance, ca, cainstance, certs, dns, dsinstance, @@ -609,19 +613,8 @@ 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) + network_ip_address_warning(ip_addresses) + broadcast_ip_address_warning(ip_addresses) # installer needs to update hosts file when DNS subsystem will be # installed or custom addresses are used diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py index b0af221..d835052 100644 --- a/ipaserver/install/server/replicainstall.py +++ b/ipaserver/install/server/replicainstall.py @@ -13,7 +13,6 @@ import os import shutil import socket -import sys import tempfile import six @@ -28,6 +27,10 @@ from ipaplatform.tasks import tasks from ipaplatform.paths import paths from ipalib import api, certstore, constants, create_api, errors, rpc, x509 +from ipalib.util import ( + network_ip_address_warning, + broadcast_ip_address_warning, +) import ipaclient.ipachangeconf import ipaclient.ntpconf from ipaserver.install import ( @@ -736,27 +739,8 @@ 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 - ) + network_ip_address_warning(config.ips) + broadcast_ip_address_warning(config.ips) except errors.ACIError: raise ScriptError("\nThe password provided is incorrect for LDAP server " @@ -1329,25 +1313,8 @@ 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 - ) + network_ip_address_warning(config.ips) + broadcast_ip_address_warning(config.ips) except errors.ACIError: raise ScriptError("\nInsufficient privileges to promote the server.") From 47ffcfa38f043256207050cb99201b971dec1e7b Mon Sep 17 00:00:00 2001 From: Martin Basti <mba...@redhat.com> Date: Tue, 13 Sep 2016 16:06:11 +0200 Subject: [PATCH 3/4] Fix missing config.ips in promote_check When replica is installed with --setup-dns config.ips is not defined. https://fedorahosted.org/freeipa/ticket/5814 --- ipaserver/install/server/replicainstall.py | 1 + 1 file changed, 1 insertion(+) diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py index d835052..571b860 100644 --- a/ipaserver/install/server/replicainstall.py +++ b/ipaserver/install/server/replicainstall.py @@ -1308,6 +1308,7 @@ def promote_check(installer): if options.setup_dns: dns.install_check(False, remote_api, True, options, config.host_name) + config.ips = dns.ip_addresses else: config.ips = installutils.get_server_ip_address( config.host_name, not installer.interactive, From c4f370ef5394ff4c9984b261de4cd5835678943e Mon Sep 17 00:00:00 2001 From: Martin Basti <mba...@redhat.com> Date: Tue, 13 Sep 2016 17:52:07 +0200 Subject: [PATCH 4/4] Add check for IP addresses into DNS installer https://fedorahosted.org/freeipa/ticket/5814 --- ipaserver/install/dns.py | 3 +++ ipaserver/install/server/install.py | 5 +++-- ipaserver/install/server/replicainstall.py | 10 ++++++---- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/ipaserver/install/dns.py b/ipaserver/install/dns.py index fe66274..dedafec 100644 --- a/ipaserver/install/dns.py +++ b/ipaserver/install/dns.py @@ -260,6 +260,9 @@ def install_check(standalone, api, replica, options, hostname): ip_addresses = get_server_ip_address(hostname, options.unattended, True, options.ip_addresses) + util.network_ip_address_warning(ip_addresses) + util.broadcast_ip_address_warning(ip_addresses) + if not options.forward_policy: # user did not specify policy, derive it: default is 'first' but # if any of local IP addresses belongs to private ranges use 'only' diff --git a/ipaserver/install/server/install.py b/ipaserver/install/server/install.py index 7733106..ff50bef 100644 --- a/ipaserver/install/server/install.py +++ b/ipaserver/install/server/install.py @@ -613,8 +613,9 @@ def install_check(installer): not installer.interactive, False, options.ip_addresses) - network_ip_address_warning(ip_addresses) - broadcast_ip_address_warning(ip_addresses) + # check addresses here, dns module is doing own check + network_ip_address_warning(ip_addresses) + broadcast_ip_address_warning(ip_addresses) # installer needs to update hosts file when DNS subsystem will be # installed or custom addresses are used diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py index 571b860..aefe158 100644 --- a/ipaserver/install/server/replicainstall.py +++ b/ipaserver/install/server/replicainstall.py @@ -739,8 +739,9 @@ def install_check(installer): config.host_name, not installer.interactive, False, options.ip_addresses) - network_ip_address_warning(config.ips) - broadcast_ip_address_warning(config.ips) + # check addresses here, dns module is doing own check + network_ip_address_warning(config.ips) + broadcast_ip_address_warning(config.ips) except errors.ACIError: raise ScriptError("\nThe password provided is incorrect for LDAP server " @@ -1314,8 +1315,9 @@ def promote_check(installer): config.host_name, not installer.interactive, False, options.ip_addresses) - network_ip_address_warning(config.ips) - broadcast_ip_address_warning(config.ips) + # check addresses here, dns module is doing own check + network_ip_address_warning(config.ips) + broadcast_ip_address_warning(config.ips) except errors.ACIError: raise ScriptError("\nInsufficient privileges to promote the server.")
-- 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
