From ba1f41d89d38286f769cfdf40b337fe5c7dad578 Mon Sep 17 00:00:00 2001 From: Brian Cline Date: Fri, 11 Apr 2014 03:27:42 -0500 Subject: [PATCH] Fixes "bad format" in replicator for valid hosts Due to a bad regex, the glance-replicator tool rejects any source or target host:port combination that contains a period. As a result, only hostnames without periods can be used. This fix expands the host/port checks to: - allow periods in hostnames - allow and verify IPv6 addr/port pairs (i.e., [fe80::f00d:face]:1234) - allow and verify IPv4 addr/port pairs (i.e., 172.17.17.2:1234) - sanity-check port numbers This also includes extensive tests for each component of the parsing, and adds network_utils to openstack-common.conf. Change-Id: I94fdd7a57a4cb0aa5d79f66d68be159d1f1266d1 Closes-Bug: #1216247 --- glance/cmd/replicator.py | 48 ++------ glance/common/utils.py | 74 +++++++++++++ glance/tests/unit/common/test_utils.py | 146 +++++++++++++++++++++++++ requirements.txt | 1 + 4 files changed, 230 insertions(+), 39 deletions(-) diff --git a/glance/cmd/replicator.py b/glance/cmd/replicator.py index 4ea377102f..5e8c5b130a 100755 --- a/glance/cmd/replicator.py +++ b/glance/cmd/replicator.py @@ -1,6 +1,7 @@ #!/usr/bin/env python # Copyright 2012 Michael Still and Canonical Inc +# Copyright 2014 SoftLayer Technologies, Inc. # All Rights Reserved. # # Licensed under the Apache License, Version 2.0 (the "License"); you may @@ -23,12 +24,12 @@ import logging.config import logging.handlers import optparse import os -import re import sys import uuid import six.moves.urllib.parse as urlparse +from glance.common import utils from glance.openstack.common import jsonutils # If ../glance/__init__.py exists, add ../ to Python search path, so that @@ -58,8 +59,6 @@ IMAGE_ALREADY_PRESENT_MESSAGE = _('The image %s is already present on ' 'do not have permissions to see all ' 'the images on the slave server.') -SERVER_PORT_REGEX = '\w+:\w+' - class AuthenticationException(Exception): pass @@ -281,12 +280,7 @@ def replication_size(options, args): if len(args) < 1: raise TypeError(_("Too few arguments.")) - server_port = args.pop() - - if not re.match(SERVER_PORT_REGEX, server_port): - raise ValueError(_("Bad format of the given arguments.")) - - server, port = server_port.split(':') + server, port = utils.parse_valid_host_port(args.pop()) total_size = 0 count = 0 @@ -319,12 +313,7 @@ def replication_dump(options, args): raise TypeError(_("Too few arguments.")) path = args.pop() - server_port = args.pop() - - if not re.match(SERVER_PORT_REGEX, server_port): - raise ValueError(_("Bad format of the given arguments.")) - - server, port = server_port.split(':') + server, port = utils.parse_valid_host_port(args.pop()) imageservice = get_image_service() client = imageservice(httplib.HTTPConnection(server, port), @@ -404,12 +393,7 @@ def replication_load(options, args): raise TypeError(_("Too few arguments.")) path = args.pop() - server_port = args.pop() - - if not re.match(SERVER_PORT_REGEX, server_port): - raise ValueError(_("Bad format of the given arguments.")) - - server, port = server_port.split(':') + server, port = utils.parse_valid_host_port(args.pop()) imageservice = get_image_service() client = imageservice(httplib.HTTPConnection(server, port), @@ -482,20 +466,13 @@ def replication_livecopy(options, args): if len(args) < 2: raise TypeError(_("Too few arguments.")) - slave_server_port = args.pop() - master_server_port = args.pop() - - if not re.match(SERVER_PORT_REGEX, slave_server_port) or \ - not re.match(SERVER_PORT_REGEX, master_server_port): - raise ValueError(_("Bad format of the given arguments.")) - imageservice = get_image_service() - slave_server, slave_port = slave_server_port.split(':') + slave_server, slave_port = utils.parse_valid_host_port(args.pop()) slave_conn = httplib.HTTPConnection(slave_server, slave_port) slave_client = imageservice(slave_conn, options.slavetoken) - master_server, master_port = master_server_port.split(':') + master_server, master_port = utils.parse_valid_host_port(args.pop()) master_conn = httplib.HTTPConnection(master_server, master_port) master_client = imageservice(master_conn, options.mastertoken) @@ -559,20 +536,13 @@ def replication_compare(options, args): if len(args) < 2: raise TypeError(_("Too few arguments.")) - slave_server_port = args.pop() - master_server_port = args.pop() - - if not re.match(SERVER_PORT_REGEX, slave_server_port) or \ - not re.match(SERVER_PORT_REGEX, master_server_port): - raise ValueError(_("Bad format of the given arguments.")) - imageservice = get_image_service() - slave_server, slave_port = slave_server_port.split(':') + slave_server, slave_port = utils.parse_valid_host_port(args.pop()) slave_conn = httplib.HTTPConnection(slave_server, slave_port) slave_client = imageservice(slave_conn, options.slavetoken) - master_server, master_port = master_server_port.split(':') + master_server, master_port = utils.parse_valid_host_port(args.pop()) master_conn = httplib.HTTPConnection(master_server, master_port) master_client = imageservice(master_conn, options.mastertoken) diff --git a/glance/common/utils.py b/glance/common/utils.py index df12eeb20b..b22b31f2ce 100644 --- a/glance/common/utils.py +++ b/glance/common/utils.py @@ -1,5 +1,6 @@ # Copyright 2010 United States Government as represented by the # Administrator of the National Aeronautics and Space Administration. +# Copyright 2014 SoftLayer Technologies, Inc. # All Rights Reserved. # # Licensed under the Apache License, Version 2.0 (the "License"); you may @@ -29,10 +30,12 @@ from eventlet.green import socket import functools import os import platform +import re import subprocess import sys import uuid +import netaddr from OpenSSL import crypto from oslo.config import cfg from webob import exc @@ -40,6 +43,7 @@ from webob import exc from glance.common import exception from glance.openstack.common import excutils import glance.openstack.common.log as logging +from glance.openstack.common import network_utils from glance.openstack.common import strutils CONF = cfg.CONF @@ -559,3 +563,73 @@ def is_uuid_like(val): return str(uuid.UUID(val)) == val except (TypeError, ValueError, AttributeError): return False + + +def is_valid_port(port): + """Verify that port represents a valid port number.""" + return str(port).isdigit() and int(port) > 0 and int(port) <= 65535 + + +def is_valid_ipv4(address): + """Verify that address represents a valid IPv4 address.""" + try: + return netaddr.valid_ipv4(address) + except Exception: + return False + + +def is_valid_ipv6(address): + """Verify that address represents a valid IPv6 address.""" + try: + return netaddr.valid_ipv6(address) + except Exception: + return False + + +def is_valid_hostname(hostname): + """Verify whether a hostname (not an FQDN) is valid.""" + return re.match('^[a-zA-Z0-9-]+$', hostname) is not None + + +def is_valid_fqdn(fqdn): + """Verify whether a host is a valid FQDN.""" + return re.match('^[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$', fqdn) is not None + + +def parse_valid_host_port(host_port): + """ + Given a "host:port" string, attempts to parse it as intelligently as + possible to determine if it is valid. This includes IPv6 [host]:port form, + IPv4 ip:port form, and hostname:port or fqdn:port form. + + Invalid inputs will raise a ValueError, while valid inputs will return + a (host, port) tuple where the port will always be of type int. + """ + + try: + try: + host, port = network_utils.parse_host_port(host_port) + except Exception: + raise ValueError(_('Host and port "%s" is not valid.') % host_port) + + if not is_valid_port(port): + raise ValueError(_('Port "%s" is not valid.') % port) + + # First check for valid IPv6 and IPv4 addresses, then a generic + # hostname. Failing those, if the host includes a period, then this + # should pass a very generic FQDN check. The FQDN check for letters at + # the tail end will weed out any hilariously absurd IPv4 addresses. + + if not (is_valid_ipv6(host) or is_valid_ipv4(host) or + is_valid_hostname(host) or is_valid_fqdn(host)): + raise ValueError(_('Host "%s" is not valid.') % host) + + except Exception as ex: + raise ValueError(_('%s ' + 'Please specify a host:port pair, where host is an ' + 'IPv4 address, IPv6 address, hostname, or FQDN. If ' + 'using an IPv6 address, enclose it in brackets ' + 'separately from the port (i.e., ' + '"[fe80::a:b:c]:9876").') % ex) + + return (host, int(port)) diff --git a/glance/tests/unit/common/test_utils.py b/glance/tests/unit/common/test_utils.py index e34c0b7f9c..14b55c1037 100644 --- a/glance/tests/unit/common/test_utils.py +++ b/glance/tests/unit/common/test_utils.py @@ -217,6 +217,152 @@ class TestUtils(test_utils.BaseTestCase): utils.validate_key_cert, keyf.name, keyf.name) + def test_valid_port(self): + valid_inputs = [1, '1', 2, '3', '5', 8, 13, 21, + '80', '3246', '65535'] + for input_str in valid_inputs: + self.assertTrue(utils.is_valid_port(input_str)) + + def test_valid_port_fail(self): + invalid_inputs = ['-32768', '0', 0, '65536', 528491, '528491', + '528.491', 'thirty-seven'] + for input_str in invalid_inputs: + self.assertFalse(utils.is_valid_port(input_str)) + + def test_valid_ipv4(self): + valid_inputs = ['10.11.12.13', + '172.17.17.1'] + for input_str in valid_inputs: + self.assertTrue(utils.is_valid_ipv4(input_str)) + + def test_valid_ipv4_fail(self): + invalid_pairs = ['', + '290.12.52.80', + 'a.b.c.d', + u'\u2601', + u'\u2603:8080', + 'fe80::1', + '[fe80::2]', + ':5673', + 'fe80:a:b:c:d:e:f:1:2:3:4', + 'fe80:a:b:c:d:e:f:g', + 'fe80::1:8080', + '[fe80:a:b:c:d:e:f:g]:9090', + '[a:b:s:u:r:d]:fe80'] + + for pair in invalid_pairs: + self.assertRaises(ValueError, + utils.parse_valid_host_port, + pair) + + def test_valid_ipv6(self): + valid_inputs = ['fe80::1', + 'fe80:0000:0000:0000:0000:0000:0000:0002', + 'fe80:a:b:c:d:e:f:0', + 'fe80::a:b:c:d', + 'fe80::1:8080'] + + for input_str in valid_inputs: + self.assertTrue(utils.is_valid_ipv6(input_str)) + + def test_valid_ipv6_fail(self): + invalid_pairs = ['', + '[fe80::2]', + '', + 'fe80:::a', + 'fe80:a:b:c:d:e:f:1:2:3:4', + 'fe80:a:b:c:d:e:f:g', + 'fe80::1:8080', + 'i:n:s:a:n:i:t:y'] + + for pair in invalid_pairs: + self.assertRaises(ValueError, + utils.parse_valid_host_port, + pair) + + def test_valid_hostname(self): + valid_inputs = ['localhost', + 'glance04-a' + 'G', + '528491'] + + for input_str in valid_inputs: + self.assertTrue(utils.is_valid_hostname(input_str)) + + def test_valid_hostname_fail(self): + invalid_inputs = ['localhost.localdomain', + '192.168.0.1', + u'\u2603', + 'glance02.stack42.local'] + + for input_str in invalid_inputs: + self.assertFalse(utils.is_valid_hostname(input_str)) + + def test_valid_fqdn(self): + valid_inputs = ['localhost.localdomain', + 'glance02.stack42.local' + 'glance04-a.stack47.local', + 'img83.glance.xn--penstack-r74e.org'] + + for input_str in valid_inputs: + self.assertTrue(utils.is_valid_fqdn(input_str)) + + def test_valid_fqdn_fail(self): + invalid_inputs = ['localhost', + '192.168.0.1', + '999.88.77.6', + u'\u2603.local', + 'glance02.stack42'] + + for input_str in invalid_inputs: + self.assertFalse(utils.is_valid_fqdn(input_str)) + + def test_valid_host_port_string(self): + valid_pairs = ['10.11.12.13:80', + '172.17.17.1:65535', + '[fe80::a:b:c:d]:9990', + 'localhost:9990', + 'localhost.localdomain:9990', + 'glance02.stack42.local:1234', + 'glance04-a.stack47.local:1234', + 'img83.glance.xn--penstack-r74e.org:13080'] + + for pair_str in valid_pairs: + host, port = utils.parse_valid_host_port(pair_str) + + escaped = pair_str.startswith('[') + expected_host = '%s%s%s' % ('[' if escaped else '', host, + ']' if escaped else '') + + self.assertTrue(pair_str.startswith(expected_host)) + self.assertTrue(port > 0) + + expected_pair = '%s:%d' % (expected_host, port) + self.assertEqual(expected_pair, pair_str) + + def test_valid_host_port_string_fail(self): + invalid_pairs = ['', + '10.11.12.13', + '172.17.17.1:99999', + '290.12.52.80:5673', + 'absurd inputs happen', + u'\u2601', + u'\u2603:8080', + 'fe80::1', + '[fe80::2]', + ':5673', + '[fe80::a:b:c:d]9990', + 'fe80:a:b:c:d:e:f:1:2:3:4', + 'fe80:a:b:c:d:e:f:g', + 'fe80::1:8080', + '[fe80:a:b:c:d:e:f:g]:9090', + '[a:b:s:u:r:d]:fe80'] + + for pair in invalid_pairs: + self.assertRaises(ValueError, + utils.parse_valid_host_port, + pair) + class UUIDTestCase(test_utils.BaseTestCase): diff --git a/requirements.txt b/requirements.txt index 103e06d9de..cdf2d7ab90 100644 --- a/requirements.txt +++ b/requirements.txt @@ -22,6 +22,7 @@ iso8601>=0.1.9 ordereddict oslo.config>=1.2.0 stevedore>=0.14 +netaddr>=0.7.6 # For Swift storage backend. python-swiftclient>=1.6