Rewrite utils.parse_host to detect more invalid addresses.

The method now uses parsing methods from urllib to better split provided URLs.
Addresses containing query strings, parameters, passwords or fragments no longer fail silently.
SSH addresses containing paths are no longer accepted.

Signed-off-by: Joffrey F <[email protected]>

Author: shin-

docker/transport/sshconn.py

@@ -1,10 +1,7 @@
-import urllib.parse
-
 import paramiko
 import requests.adapters
 import six
 
-
 from .. import constants
 
 if six.PY3:
@@ -82,7 +79,7 @@ def __init__(self, base_url, timeout=60,
         self.ssh_client = paramiko.SSHClient()
         self.ssh_client.load_system_host_keys()
 
-        parsed = urllib.parse.urlparse(base_url)
+        parsed = six.moves.urllib_parse.urlparse(base_url)
         self.ssh_client.connect(
             parsed.hostname, parsed.port, parsed.username,
         )

docker/utils/utils.py

@@ -1,10 +1,11 @@
 import base64
+import json
 import os
 import os.path
-import json
 import shlex
-from distutils.version import StrictVersion
+import string
 from datetime import datetime
+from distutils.version import StrictVersion
 
 import six
 
@@ -13,11 +14,12 @@
 
 if six.PY2:
     from urllib import splitnport
+    from urlparse import urlparse
 else:
-    from urllib.parse import splitnport
+    from urllib.parse import splitnport, urlparse
 
 DEFAULT_HTTP_HOST = "127.0.0.1"
-DEFAULT_UNIX_SOCKET = "http+unix://var/run/docker.sock"
+DEFAULT_UNIX_SOCKET = "http+unix:///var/run/docker.sock"
 DEFAULT_NPIPE = 'npipe:////./pipe/docker_engine'
 
 BYTE_UNITS = {
@@ -212,81 +214,93 @@ def parse_repository_tag(repo_name):
     return repo_name, None
 
 
-# Based on utils.go:ParseHost http://tinyurl.com/nkahcfh
-# fd:// protocol unsupported (for obvious reasons)
-# Added support for http and https
-# Protocol translation: tcp -> http, unix -> http+unix
 def parse_host(addr, is_win32=False, tls=False):
-    proto = "http+unix"
-    port = None
     path = ''
+    port = None
+    host = None
 
+    # Sensible defaults
     if not addr and is_win32:
-        addr = DEFAULT_NPIPE
-
+        return DEFAULT_NPIPE
     if not addr or addr.strip() == 'unix://':
         return DEFAULT_UNIX_SOCKET
 
     addr = addr.strip()
-    if addr.startswith('http://'):
-        addr = addr.replace('http://', 'tcp://')
-    if addr.startswith('http+unix://'):
-        addr = addr.replace('http+unix://', 'unix://')
 
-    if addr == 'tcp://':
+    parsed_url = urlparse(addr)
+    proto = parsed_url.scheme
+    if not proto or any([x not in string.ascii_letters + '+' for x in proto]):
+        # https://bugs.python.org/issue754016
+        parsed_url = urlparse('//' + addr, 'tcp')
+        proto = 'tcp'
+
+    if proto == 'fd':
+        raise errors.DockerException('fd protocol is not implemented')
+
+    # These protos are valid aliases for our library but not for the
+    # official spec
+    if proto == 'http' or proto == 'https':
+        tls = proto == 'https'
+        proto = 'tcp'
+    elif proto == 'http+unix':
+        proto = 'unix'
+
+    if proto not in ('tcp', 'unix', 'npipe', 'ssh'):
         raise errors.DockerException(
-            "Invalid bind address format: {0}".format(addr)
+            "Invalid bind address protocol: {}".format(addr)
         )
-    elif addr.startswith('unix://'):
-        addr = addr[7:]
-    elif addr.startswith('tcp://'):
-        proto = 'http{0}'.format('s' if tls else '')
-        addr = addr[6:]
-    elif addr.startswith('https://'):
-        proto = "https"
-        addr = addr[8:]
-    elif addr.startswith('npipe://'):
-        proto = 'npipe'
-        addr = addr[8:]
-    elif addr.startswith('fd://'):
-        raise errors.DockerException("fd protocol is not implemented")
-    elif addr.startswith('ssh://'):
-        proto = 'ssh'
-        addr = addr[6:]
-    else:
-        if "://" in addr:
-            raise errors.DockerException(
-                "Invalid bind address protocol: {0}".format(addr)
-            )
-        proto = "https" if tls else "http"
 
-    if proto in ("http", "https", "ssh"):
-        address_parts = addr.split('/', 1)
-        host = address_parts[0]
-        if len(address_parts) == 2:
-            path = '/' + address_parts[1]
-        host, port = splitnport(host)
+    if proto == 'tcp' and not parsed_url.netloc:
+        # "tcp://" is exceptionally disallowed by convention;
+        # omitting a hostname for other protocols is fine
+        raise errors.DockerException(
+            'Invalid bind address format: {}'.format(addr)
+        )
 
+    if any([
+        parsed_url.params, parsed_url.query, parsed_url.fragment,
+        parsed_url.password
+    ]):
+        raise errors.DockerException(
+            'Invalid bind address format: {}'.format(addr)
+        )
+
+    if parsed_url.path and proto == 'ssh':
+        raise errors.DockerException(
+            'Invalid bind address format: no path allowed for this protocol:'
+            ' {}'.format(addr)
+        )
+    else:
+        path = parsed_url.path
+        if proto == 'unix' and parsed_url.hostname is not None:
+            # For legacy reasons, we consider unix://path
+            # to be valid and equivalent to unix:///path
+            path = '/'.join((parsed_url.hostname, path))
+
+    if proto in ('tcp', 'ssh'):
+        # parsed_url.hostname strips brackets from IPv6 addresses,
+        # which can be problematic hence our use of splitnport() instead.
+        host, port = splitnport(parsed_url.netloc)
         if port is None or port < 0:
-            if proto == 'ssh':
-                port = 22
-            else:
+            if proto != 'ssh':
                 raise errors.DockerException(
-                    "Invalid port: {0}".format(addr)
+                    'Invalid bind address format: port is required:'
+                    ' {}'.format(addr)
                 )
+            port = 22
 
         if not host:
             host = DEFAULT_HTTP_HOST
-    else:
-        host = addr
 
-    if proto in ("http", "https") and port == -1:
-        raise errors.DockerException(
-            "Bind address needs a port: {0}".format(addr))
+    # Rewrite schemes to fit library internals (requests adapters)
+    if proto == 'tcp':
+        proto = 'http{}'.format('s' if tls else '')
+    elif proto == 'unix':
+        proto = 'http+unix'
 
-    if proto == "http+unix" or proto == 'npipe':
-        return "{0}://{1}".format(proto, host).rstrip('/')
-    return "{0}://{1}:{2}{3}".format(proto, host, port, path).rstrip('/')
+    if proto in ('http+unix', 'npipe'):
+        return "{}://{}".format(proto, path).rstrip('/')
+    return '{0}://{1}:{2}{3}'.format(proto, host, port, path).rstrip('/')
 
 
 def parse_devices(devices):

tests/unit/utils_test.py

@@ -272,6 +272,11 @@ def test_parse_host(self):
             'tcp://',
             'udp://127.0.0.1',
             'udp://127.0.0.1:2375',
+            'ssh://:22/path',
+            'tcp://netloc:3333/path?q=1',
+            'unix:///sock/path#fragment',
+            'https://netloc:3333/path;params',
+            'ssh://:clearpassword@host:22',
         ]
 
         valid_hosts = {
@@ -281,7 +286,7 @@ def test_parse_host(self):
             'http://:7777': 'http://127.0.0.1:7777',
             'https://kokia.jp:2375': 'https://kokia.jp:2375',
             'unix:///var/run/docker.sock': 'http+unix:///var/run/docker.sock',
-            'unix://': 'http+unix://var/run/docker.sock',
+            'unix://': 'http+unix:///var/run/docker.sock',
             '12.234.45.127:2375/docker/engine': (
                 'http://12.234.45.127:2375/docker/engine'
             ),
@@ -294,6 +299,9 @@ def test_parse_host(self):
             '[fd12::82d1]:2375/docker/engine': (
                 'http://[fd12::82d1]:2375/docker/engine'
             ),
+            'ssh://': 'ssh://127.0.0.1:22',
+            'ssh://user@localhost:22': 'ssh://user@localhost:22',
+            'ssh://user@remote': 'ssh://user@remote:22',
         }
 
         for host in invalid_hosts:
@@ -304,7 +312,7 @@ def test_parse_host(self):
             assert parse_host(host, None) == expected
 
     def test_parse_host_empty_value(self):
-        unix_socket = 'http+unix://var/run/docker.sock'
+        unix_socket = 'http+unix:///var/run/docker.sock'
         npipe = 'npipe:////./pipe/docker_engine'
 
         for val in [None, '']: