fix #49754 minion zmq connecting to master configured explicitly as IPv6 address or IPv6 only master

salt/minion.py

@@ -27,6 +27,7 @@
 # pylint: disable=import-error,no-name-in-module,redefined-builtin
 from salt.ext import six
 from salt._compat import ipaddress
+from salt.utils.network import parse_host_port
 from salt.ext.six.moves import range
 from salt.utils.zeromq import zmq, ZMQDefaultLoop, install_zmq, ZMQ_VERSION_INFO
 
@@ -243,27 +244,29 @@ def resolve_dns(opts, fallback=True):
 
 
 def prep_ip_port(opts):
+    '''
+    parse host:port values from opts['master'] and return valid:
+        master: ip address or hostname as a string
+        master_port: (optional) master returner port as integer
+
+    e.g.:
+      - master: 'localhost:1234' -> {'master': 'localhost', 'master_port': 1234}
+      - master: '127.0.0.1:1234' -> {'master': '127.0.0.1', 'master_port' :1234}
+      - master: '[::1]:1234' -> {'master': '::1', 'master_port': 1234}
+      - master: 'fe80::a00:27ff:fedc:ba98' -> {'master': 'fe80::a00:27ff:fedc:ba98'}
+    '''
     ret = {}
     # Use given master IP if "ip_only" is set or if master_ip is an ipv6 address without
     # a port specified. The is_ipv6 check returns False if brackets are used in the IP
     # definition such as master: '[::1]:1234'.
-    if opts['master_uri_format'] == 'ip_only' or salt.utils.network.is_ipv6(opts['master']):
-        ret['master'] = opts['master']
+    if opts['master_uri_format'] == 'ip_only':
+        ret['master'] = ipaddress.ip_address(opts['master'])
     else:
-        ip_port = opts['master'].rsplit(':', 1)
-        if len(ip_port) == 1:
-            # e.g. master: mysaltmaster
-            ret['master'] = ip_port[0]
-        else:
-            # e.g. master: localhost:1234
-            # e.g. master: 127.0.0.1:1234
-            # e.g. master: [::1]:1234
-            # Strip off brackets for ipv6 support
-            ret['master'] = ip_port[0].strip('[]')
-
-            # Cast port back to an int! Otherwise a TypeError is thrown
-            # on some of the socket calls elsewhere in the minion and utils code.
-            ret['master_port'] = int(ip_port[1])
+        host, port = parse_host_port(opts['master'])
+        ret = {'master': host}
+        if port:
+            ret.update({'master_port': port})
+
     return ret
 
 

salt/transport/zeromq.py

@@ -33,6 +33,7 @@
 import salt.transport.mixins.auth
 from salt.ext import six
 from salt.exceptions import SaltReqTimeoutError
+from salt._compat import ipaddress
 
 from salt.utils.zeromq import zmq, ZMQDefaultLoop, install_zmq, ZMQ_VERSION_INFO, LIBZMQ_VERSION_INFO
 import zmq.error
@@ -71,33 +72,38 @@ def _get_master_uri(master_ip,
     '''
     Return the ZeroMQ URI to connect the Minion to the Master.
     It supports different source IP / port, given the ZeroMQ syntax:
-
     // Connecting using a IP address and bind to an IP address
     rc = zmq_connect(socket, "tcp://192.168.1.17:5555;192.168.1.1:5555"); assert (rc == 0);
-
     Source: http://api.zeromq.org/4-1:zmq-tcp
     '''
-    if LIBZMQ_VERSION_INFO >= (4, 1, 6) and ZMQ_VERSION_INFO >= (16, 0, 1):
-        # The source:port syntax for ZeroMQ has been added in libzmq 4.1.6
-        # which is included in the pyzmq wheels starting with 16.0.1.
-        if source_ip or source_port:
+    from salt.utils.zeromq import ip_bracket
+
+    master_uri = 'tcp://{master_ip}:{master_port}'.format(
+                  master_ip=ip_bracket(master_ip), master_port=master_port)
+
+    if source_ip or source_port:
+        if LIBZMQ_VERSION_INFO >= (4, 1, 6) and ZMQ_VERSION_INFO >= (16, 0, 1):
+            # The source:port syntax for ZeroMQ has been added in libzmq 4.1.6
+            # which is included in the pyzmq wheels starting with 16.0.1.
             if source_ip and source_port:
-                return 'tcp://{source_ip}:{source_port};{master_ip}:{master_port}'.format(
-                        source_ip=source_ip, source_port=source_port,
-                        master_ip=master_ip, master_port=master_port)
+                master_uri = 'tcp://{source_ip}:{source_port};{master_ip}:{master_port}'.format(
+                             source_ip=ip_bracket(source_ip), source_port=source_port,
+                             master_ip=ip_bracket(master_ip), master_port=master_port)
             elif source_ip and not source_port:
-                return 'tcp://{source_ip}:0;{master_ip}:{master_port}'.format(
-                        source_ip=source_ip,
-                        master_ip=master_ip, master_port=master_port)
-            elif not source_ip and source_port:
-                return 'tcp://0.0.0.0:{source_port};{master_ip}:{master_port}'.format(
-                        source_port=source_port,
-                        master_ip=master_ip, master_port=master_port)
-    if source_ip or source_port:
-        log.warning('Unable to connect to the Master using a specific source IP / port')
-        log.warning('Consider upgrading to pyzmq >= 16.0.1 and libzmq >= 4.1.6')
-    return 'tcp://{master_ip}:{master_port}'.format(
-                master_ip=master_ip, master_port=master_port)
+                master_uri = 'tcp://{source_ip}:0;{master_ip}:{master_port}'.format(
+                             source_ip=ip_bracket(source_ip),
+                             master_ip=ip_bracket(master_ip), master_port=master_port)
+            elif source_port and not source_ip:
+                ip_any = '0.0.0.0' if ipaddress.ip_address(master_ip).version == 4 else ip_bracket('::')
+                master_uri = 'tcp://{ip_any}:{source_port};{master_ip}:{master_port}'.format(
+                             ip_any=ip_any, source_port=source_port,
+                             master_ip=ip_bracket(master_ip), master_port=master_port)
+        else:
+            log.warning('Unable to connect to the Master using a specific source IP / port')
+            log.warning('Consider upgrading to pyzmq >= 16.0.1 and libzmq >= 4.1.6')
+            log.warning('Specific source IP / port for connecting to master returner port: configuraion ignored')
+
+    return master_uri
 
 
 class AsyncZeroMQReqChannel(salt.transport.client.ReqChannel):

salt/utils/network.py

@@ -56,12 +56,12 @@
 
 
 def sanitize_host(host):
-    '''
+    """
     Sanitize host string.
-    '''
-    return ''.join([
-        c for c in host[0:255] if c in (ascii_letters + digits + '.-')
-    ])
+    https://tools.ietf.org/html/rfc1123#section-2.1
+    """
+    RFC952_characters = ascii_letters + digits + ".-"
+    return "".join([c for c in host[0:255] if c in RFC952_characters])
 
 
 def isportopen(host, port):
@@ -1870,14 +1870,14 @@ def dns_check(addr, port, safe=False, ipv6=None):
                 if h[0] == socket.AF_INET6 and ipv6 is False:
                     continue
 
-                candidate_addr = salt.utils.zeromq.ip_bracket(h[4][0])
+                candidate_addr = h[4][0]
 
                 if h[0] != socket.AF_INET6 or ipv6 is not None:
                     candidates.append(candidate_addr)
 
                 try:
                     s = socket.socket(h[0], socket.SOCK_STREAM)
-                    s.connect((candidate_addr.strip('[]'), port))
+                    s.connect((candidate_addr), port))
                     s.close()
 
                     resolved = candidate_addr
@@ -1906,3 +1906,55 @@ def dns_check(addr, port, safe=False, ipv6=None):
             raise SaltClientError()
         raise SaltSystemExit(code=42, msg=err)
     return resolved
+
+
+def parse_host_port(host_port):
+    """
+    Takes a string argument specifying host or host:port.
+
+    Returns a (hostname, port) or (ip_address, port) tuple. If no port is given,
+    the second (port) element of the returned tuple will be None.
+
+    host:port argument, for example, is accepted in the forms of:
+      - hostname
+      - hostname:1234
+      - hostname.domain.tld
+      - hostname.domain.tld:5678
+      - [1234::5]:5678
+      - 1234::5
+      - 10.11.12.13:4567
+      - 10.11.12.13
+    """
+    host, port = None, None  # default
+
+    _s_ = host_port[:]
+    if _s_[0] == "[":
+        if "]" in host_port:
+            host, _s_ = _s_.lstrip("[").rsplit("]", 1)
+            host = ipaddress.IPv6Address(host)
+            if _s_[0] == ":":
+                port = int(_s_.lstrip(":"))
+            else:
+                if len(_s_) > 1:
+                    raise ValueError('found ambiguous "{}" port in "{}"'.format(_s_, host_port))
+    else:
+        if _s_.count(":") == 1:
+            host, _hostport_separator_, port = _s_.partition(":")
+            try:
+                port = int(port)
+            except ValueError as _e_:
+                log.error('host_port "%s" port value "%s" is not an integer.', host_port, port)
+                raise _e_
+        else:
+            host = _s_
+    try:
+        if not isinstance(host, ipaddress._BaseAddress):
+            host_ip = ipaddress.ip_address(host)
+            host = host_ip
+    except ValueError:
+        log.debug('"%s" Not an IP address? Assuming it is a hostname.', host)
+        if host != sanitize_host(host):
+            log.error('bad hostname: "%s"', host)
+            raise ValueError('bad hostname: "{}"'.format(host))
+
+    return host, port

salt/utils/zeromq.py

@@ -8,6 +8,7 @@
 import logging
 import tornado.ioloop
 from salt.exceptions import SaltSystemExit
+from salt._compat import ipaddress
 
 log = logging.getLogger(__name__)
 
@@ -82,6 +83,5 @@ def ip_bracket(addr):
     Convert IP address representation to ZMQ (URL) format. ZMQ expects
     brackets around IPv6 literals, since they are used in URLs.
     '''
-    if addr and ':' in addr and not addr.startswith('['):
-        return '[{0}]'.format(addr)
-    return addr
+    addr = ipaddress.ip_address(addr)
+    return ('[{}]' if addr.version == 6 else '{}').format(addr)

tests/unit/transport/test_zeromq.py

@@ -317,11 +317,15 @@ def test_master_uri(self):
         '''
         test _get_master_uri method
         '''
+
         m_ip = '127.0.0.1'
         m_port = 4505
         s_ip = '111.1.0.1'
         s_port = 4058
 
+        m_ip6 = '1234:5678::9abc'
+        s_ip6 = '1234:5678::1:9abc'
+
         with patch('salt.transport.zeromq.LIBZMQ_VERSION_INFO', (4, 1, 6)), \
             patch('salt.transport.zeromq.ZMQ_VERSION_INFO', (16, 0, 1)):
             # pass in both source_ip and source_port
@@ -330,15 +334,27 @@ def test_master_uri(self):
                                                          source_ip=s_ip,
                                                          source_port=s_port) == 'tcp://{0}:{1};{2}:{3}'.format(s_ip, s_port, m_ip, m_port)
 
+            assert salt.transport.zeromq._get_master_uri(master_ip=m_ip6,
+                                                         master_port=m_port,
+                                                         source_ip=s_ip6,
+                                                         source_port=s_port) == 'tcp://[{0}]:{1};[{2}]:{3}'.format(s_ip6, s_port, m_ip6, m_port)
+
             # source ip and source_port empty
             assert salt.transport.zeromq._get_master_uri(master_ip=m_ip,
                                                          master_port=m_port) == 'tcp://{0}:{1}'.format(m_ip, m_port)
 
+            assert salt.transport.zeromq._get_master_uri(master_ip=m_ip6,
+                                                         master_port=m_port) == 'tcp://[{0}]:{1}'.format(m_ip6, m_port)
+
             # pass in only source_ip
             assert salt.transport.zeromq._get_master_uri(master_ip=m_ip,
                                                          master_port=m_port,
                                                          source_ip=s_ip) == 'tcp://{0}:0;{1}:{2}'.format(s_ip, m_ip, m_port)
 
+            assert salt.transport.zeromq._get_master_uri(master_ip=m_ip6,
+                                                         master_port=m_port,
+                                                         source_ip=s_ip6) == 'tcp://[{0}]:0;[{1}]:{2}'.format(s_ip6, m_ip6, m_port)
+
             # pass in only source_port
             assert salt.transport.zeromq._get_master_uri(master_ip=m_ip,
                                                          master_port=m_port,

tests/unit/utils/test_network.py

@@ -18,6 +18,7 @@
 
 # Import salt libs
 import salt.utils.network as network
+from salt._compat import ipaddress
 
 log = logging.getLogger(__name__)
 
@@ -202,6 +203,35 @@ def test_is_ipv6(self):
         self.assertFalse(network.is_ipv6('10.0.1.2'))
         self.assertFalse(network.is_ipv6('2001.0db8.85a3.0000.0000.8a2e.0370.7334'))
 
+    def test_parse_host_port(self):
+        _ip = ipaddress.ip_address
+        good_host_ports = {
+            '10.10.0.3': (_ip('10.10.0.3'), None),
+            '10.10.0.3:1234': (_ip('10.10.0.3'), 1234),
+            '2001:0db8:85a3::8a2e:0370:7334': (_ip('2001:0db8:85a3::8a2e:0370:7334'), None),
+            '[2001:0db8:85a3::8a2e:0370:7334]:1234': (_ip('2001:0db8:85a3::8a2e:0370:7334'), 1234),
+            '2001:0db8:85a3::7334': (_ip('2001:0db8:85a3::7334'), None),
+            '[2001:0db8:85a3::7334]:1234': (_ip('2001:0db8:85a3::7334'), 1234)
+        }
+        bad_host_ports = [
+            '10.10.0.3/24',
+            '10.10.0.3::1234',
+            '2001:0db8:0370:7334',
+            '2001:0db8:0370::7334]:1234',
+            '2001:0db8:0370:0:a:b:c:d:1234'
+        ]
+        for host_port, assertion_value in good_host_ports.items():
+            host = port = None
+            host, port = network.parse_host_port(host_port)
+            self.assertEqual((host, port), assertion_value)
+
+        for host_port in bad_host_ports:
+            try:
+                self.assertRaises(ValueError, network.parse_host_port, host_port)
+            except AssertionError as _e_:
+                log.error('bad host_port value: "%s" failed to trigger ValueError exception', host_port)
+                raise _e_
+
     def test_is_subnet(self):
         for subnet_data in (IPV4_SUBNETS, IPV6_SUBNETS):
             for item in subnet_data[True]:

tests/unit/utils/validate/test_net.py

@@ -50,6 +50,7 @@ def test_ipv6_addr(self):
         Test IPv6 address validation
         '''
         true_addrs = [
+            '::',
             '::1',
             '::1/32',
             '::1/32',
@@ -62,6 +63,8 @@ def test_ipv6_addr(self):
             '::1/0',
             '::1/32d',
             '::1/129',
+            '2a03:4000:c:10aa:1017:f00d:aaaa:a:4506',
+            '2a03::1::2',
         ]
 
         for addr in true_addrs: