Vendored Docker SDK for Python updates (#434)

* utils: fix IPv6 address w/ port parsing

This was using a deprecated function (`urllib.splitnport`),
ostensibly to work around issues with brackets on IPv6 addresses.

Ironically, its usage was broken, and would result in mangled IPv6
addresses if they had a port specified in some instances.

Usage of the deprecated function has been eliminated and extra test
cases added where missing. All existing cases pass as-is. (The only
other change to the test was to improve assertion messages.)

Cherry-picked from
f16c4e1147

Co-authored-by: Milas Bowman <milas.bowman@docker.com>

* client: fix exception semantics in _raise_for_status

We want "The above exception was the direct cause of the following exception:" instead of "During handling of the above exception, another exception occurred:"

Cherry-picked from
bb11197ee3

Co-authored-by: Maor Kleinberger <kmaork@gmail.com>

* tls: use auto-negotiated highest version

Specific TLS versions are deprecated in latest Python, which
causes test failures due to treating deprecation errors as
warnings.

Luckily, the fix here is straightforward: we can eliminate some
custom version selection logic by using `PROTOCOL_TLS_CLIENT`,
which is the recommended method and will select the highest TLS
version supported by both client and server.

Cherry-picked from
56dd6de7df

Co-authored-by: Milas Bowman <milas.bowman@docker.com>

* transport: fix ProxyCommand for SSH conn

Cherry-picked from
4e19cc48df

Co-authored-by: Guy Lichtman <glicht@users.noreply.github.com>

* ssh: do not create unnecessary subshell on exec

Cherry-picked from
bb40ba051f

Co-authored-by: liubo <liubo@uniontech.com>

* ssh: reject unknown host keys when using Python SSH impl

In the Secure Shell (SSH) protocol, host keys are used to verify the identity of remote hosts. Accepting unknown host keys may leave the connection open to man-in-the-middle attacks.

Do not accept unknown host keys. In particular, do not set the default missing host key policy for the Paramiko library to either AutoAddPolicy or WarningPolicy. Both of these policies continue even when the host key is unknown. The default setting of RejectPolicy is secure because it throws an exception when it encounters an unknown host key.

Reference: https://cwe.mitre.org/data/definitions/295.html

NOTE: This only affects SSH connections using the native Python SSH implementation (Paramiko), when `use_ssh_client=False` (default). If using the system SSH client (`use_ssh_client=True`), the host configuration
(e.g. `~/.ssh/config`) will apply.

Cherry-picked from
d9298647d9

Co-authored-by: Audun Nes <audun.nes@gmail.com>

* lint: fix deprecation warnings from threading package

Set `daemon` attribute instead of using `setDaemon` method that
was deprecated in Python 3.10.

Cherry-picked from
adf5a97b12

Co-authored-by: Karthikeyan Singaravelan <tir.karthi@gmail.com>

* api: preserve cause when re-raising error

Use `from e` to ensure that the error context is propagated
correctly.

Cherry-picked from
05e143429e

Co-authored-by: Milas Bowman <milas.bowman@docker.com>

* build: trim trailing whitespace from dockerignore entries

Cherry-picked from
3ee3a2486f

Co-authored-by: Clément Loiselet <clement.loiselet@capgemini.com>

* Improve formulation, also mention the security change as a breaking change.

Co-authored-by: Milas Bowman <milas.bowman@docker.com>
Co-authored-by: Maor Kleinberger <kmaork@gmail.com>
Co-authored-by: Guy Lichtman <glicht@users.noreply.github.com>
Co-authored-by: liubo <liubo@uniontech.com>
Co-authored-by: Audun Nes <audun.nes@gmail.com>
Co-authored-by: Karthikeyan Singaravelan <tir.karthi@gmail.com>
Co-authored-by: Clément Loiselet <clement.loiselet@capgemini.com>
This commit is contained in:
Felix Fontein 2022-07-31 17:09:18 +02:00 committed by GitHub
parent a33e51e04a
commit ae708a7333
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 62 additions and 33 deletions

View File

@ -0,0 +1,11 @@
bugfixes:
- "modules and plugins communicating directly with the Docker daemon - fix parsing of IPv6 addresses with a port in ``docker_host``. This is only a change relative to older community.docker 3.0.0 pre-releases or with respect to Docker SDK for Python < 6.0.0. Docker SDK for Python 6.0.0 will also include this change (https://github.com/ansible-collections/community.docker/pull/434)."
- "modules and plugins communicating directly with the Docker daemon - fix ``ProxyCommand`` handling for SSH connections when not using ``use_ssh_client=true``. This is only a change relative to older community.docker 3.0.0 pre-releases or with respect to Docker SDK for Python < 6.0.0. Docker SDK for Python 6.0.0 will also include this change (https://github.com/ansible-collections/community.docker/pull/434)."
- "modules and plugins communicating directly with the Docker daemon - do not create a subshell for SSH connections when using ``use_ssh_client=true``. This is only a change relative to older community.docker 3.0.0 pre-releases or with respect to Docker SDK for Python < 6.0.0. Docker SDK for Python 6.0.0 will also include this change (https://github.com/ansible-collections/community.docker/pull/434)."
- "docker_image - when composing the build context, trim trailing whitespace from ``.dockerignore`` entries. This is only a change relative to older community.docker 3.0.0 pre-releases or with respect to Docker SDK for Python < 6.0.0. Docker SDK for Python 6.0.0 will also include this change (https://github.com/ansible-collections/community.docker/pull/434)."
minor_changes:
- "modules and plugins communicating directly with the Docker daemon - improve default TLS version selection for Python 3.6 and newer. This is only a change relative to older community.docker 3.0.0 pre-releases or with respect to Docker SDK for Python < 6.0.0. Docker SDK for Python 6.0.0 will also include this change (https://github.com/ansible-collections/community.docker/pull/434)."
security_fixes:
- "modules and plugins communicating directly with the Docker daemon - when connecting by SSH and not using ``use_ssh_client=true``, reject unknown host keys instead of accepting them. This is only a change relative to older community.docker 3.0.0 pre-releases or with respect to Docker SDK for Python < 6.0.0. Docker SDK for Python 6.0.0 will also include this change (https://github.com/ansible-collections/community.docker/pull/434)."
breaking_changes:
- "modules and plugins communicating directly with the Docker daemon - when connecting by SSH and not using ``use_ssh_client=true``, reject unknown host keys instead of accepting them. This is only a breaking change relative to older community.docker 3.0.0 pre-releases or with respect to Docker SDK for Python < 6.0.0. Docker SDK for Python 6.0.0 will also include this change (https://github.com/ansible-collections/community.docker/pull/434)."

View File

@ -15,7 +15,7 @@ import logging
import struct import struct
from functools import partial from functools import partial
from ansible.module_utils.six import PY3, binary_type, iteritems, string_types from ansible.module_utils.six import PY3, binary_type, iteritems, string_types, raise_from
from ansible.module_utils.six.moves.urllib.parse import quote from ansible.module_utils.six.moves.urllib.parse import quote
from .. import auth from .. import auth
@ -258,7 +258,7 @@ class APIClient(
try: try:
response.raise_for_status() response.raise_for_status()
except _HTTPError as e: except _HTTPError as e:
raise create_api_error_from_http_exception(e) raise_from(create_api_error_from_http_exception(e), e)
def _result(self, response, json=False, binary=False): def _result(self, response, json=False, binary=False):
if json and binary: if json and binary:

View File

@ -12,6 +12,8 @@ __metaclass__ = type
from ._import_helper import HTTPError as _HTTPError from ._import_helper import HTTPError as _HTTPError
from ansible.module_utils.six import raise_from
class DockerException(Exception): class DockerException(Exception):
""" """
@ -40,7 +42,7 @@ def create_api_error_from_http_exception(e):
cls = ImageNotFound cls = ImageNotFound
else: else:
cls = NotFound cls = NotFound
raise cls(e, response=response, explanation=explanation) raise_from(cls(e, response=response, explanation=explanation), e)
class APIError(_HTTPError, DockerException): class APIError(_HTTPError, DockerException):

View File

@ -12,6 +12,7 @@ __metaclass__ = type
import os import os
import ssl import ssl
import sys
from . import errors from . import errors
from .transport.ssladapter import SSLHTTPAdapter from .transport.ssladapter import SSLHTTPAdapter
@ -49,15 +50,10 @@ class TLSConfig(object):
self.assert_hostname = assert_hostname self.assert_hostname = assert_hostname
self.assert_fingerprint = assert_fingerprint self.assert_fingerprint = assert_fingerprint
# TODO(dperny): according to the python docs, PROTOCOL_TLSvWhatever is
# depcreated, and it's recommended to use OPT_NO_TLSvWhatever instead
# to exclude versions. But I think that might require a bigger
# architectural change, so I've opted not to pursue it at this time
# If the user provides an SSL version, we should use their preference # If the user provides an SSL version, we should use their preference
if ssl_version: if ssl_version:
self.ssl_version = ssl_version self.ssl_version = ssl_version
else: elif (sys.version_info.major, sys.version_info.minor) < (3, 6):
# If the user provides no ssl version, we should default to # If the user provides no ssl version, we should default to
# TLSv1_2. This option is the most secure, and will work for the # TLSv1_2. This option is the most secure, and will work for the
# majority of users with reasonably up-to-date software. However, # majority of users with reasonably up-to-date software. However,
@ -73,6 +69,8 @@ class TLSConfig(object):
# SSLv23 fails in mysterious ways: # SSLv23 fails in mysterious ways:
# https://github.com/docker/docker-py/issues/963 # https://github.com/docker/docker-py/issues/963
self.ssl_version = ssl.PROTOCOL_TLSv1 self.ssl_version = ssl.PROTOCOL_TLSv1
else:
self.ssl_version = ssl.PROTOCOL_TLS_CLIENT
# "client_cert" must have both or neither cert/key files. In # "client_cert" must have both or neither cert/key files. In
# either case, Alert the user when both are expected, but any are # either case, Alert the user when both are expected, but any are

View File

@ -78,9 +78,8 @@ class SSHSocket(socket.socket):
env.pop('SSL_CERT_FILE', None) env.pop('SSL_CERT_FILE', None)
self.proc = subprocess.Popen( self.proc = subprocess.Popen(
' '.join(args), args,
env=env, env=env,
shell=True,
stdout=subprocess.PIPE, stdout=subprocess.PIPE,
stdin=subprocess.PIPE, stdin=subprocess.PIPE,
preexec_fn=preexec_func) preexec_fn=preexec_func)
@ -225,7 +224,7 @@ class SSHHTTPAdapter(BaseHTTPAdapter):
host_config = conf.lookup(base_url.hostname) host_config = conf.lookup(base_url.hostname)
if 'proxycommand' in host_config: if 'proxycommand' in host_config:
self.ssh_params["sock"] = paramiko.ProxyCommand( self.ssh_params["sock"] = paramiko.ProxyCommand(
self.ssh_conf['proxycommand'] host_config['proxycommand']
) )
if 'hostname' in host_config: if 'hostname' in host_config:
self.ssh_params['hostname'] = host_config['hostname'] self.ssh_params['hostname'] = host_config['hostname']
@ -237,7 +236,7 @@ class SSHHTTPAdapter(BaseHTTPAdapter):
self.ssh_params['key_filename'] = host_config['identityfile'] self.ssh_params['key_filename'] = host_config['identityfile']
self.ssh_client.load_system_host_keys() self.ssh_client.load_system_host_keys()
self.ssh_client.set_missing_host_key_policy(paramiko.WarningPolicy()) self.ssh_client.set_missing_host_key_policy(paramiko.RejectPolicy())
def _connect(self): def _connect(self):
if self.ssh_client: if self.ssh_client:

View File

@ -244,6 +244,9 @@ class Pattern(object):
@classmethod @classmethod
def normalize(cls, p): def normalize(cls, p):
# Remove trailing spaces
p = p.strip()
# Leading and trailing slashes are not relevant. Yes, # Leading and trailing slashes are not relevant. Yes,
# "foo.py/" must exclude the "foo.py" regular file. "." # "foo.py/" must exclude the "foo.py" regular file. "."
# components are not relevant either, even if the whole # components are not relevant either, even if the whole

View File

@ -11,6 +11,7 @@ from __future__ import (absolute_import, division, print_function)
__metaclass__ = type __metaclass__ = type
import base64 import base64
import collections
import json import json
import os import os
import os.path import os.path
@ -22,17 +23,22 @@ from ansible_collections.community.docker.plugins.module_utils.version import St
from ansible.module_utils.six import PY2, PY3, binary_type, integer_types, iteritems, string_types, text_type from ansible.module_utils.six import PY2, PY3, binary_type, integer_types, iteritems, string_types, text_type
from .. import errors from .. import errors
from .. import tls
from ..constants import DEFAULT_HTTP_HOST from ..constants import DEFAULT_HTTP_HOST
from ..constants import DEFAULT_UNIX_SOCKET from ..constants import DEFAULT_UNIX_SOCKET
from ..constants import DEFAULT_NPIPE from ..constants import DEFAULT_NPIPE
from ..constants import BYTE_UNITS from ..constants import BYTE_UNITS
from ..tls import TLSConfig
if PY2: if PY2:
from urllib import splitnport from urlparse import urlparse, urlunparse
from urlparse import urlparse
else: else:
from urllib.parse import splitnport, urlparse from urllib.parse import urlparse, urlunparse
URLComponents = collections.namedtuple(
'URLComponents',
'scheme netloc url params query fragment',
)
def create_ipam_pool(*args, **kwargs): def create_ipam_pool(*args, **kwargs):
@ -220,10 +226,6 @@ def parse_repository_tag(repo_name):
def parse_host(addr, is_win32=False, tls=False): def parse_host(addr, is_win32=False, tls=False):
path = ''
port = None
host = None
# Sensible defaults # Sensible defaults
if not addr and is_win32: if not addr and is_win32:
return DEFAULT_NPIPE return DEFAULT_NPIPE
@ -282,20 +284,20 @@ def parse_host(addr, is_win32=False, tls=False):
# to be valid and equivalent to unix:///path # to be valid and equivalent to unix:///path
path = '/'.join((parsed_url.hostname, path)) path = '/'.join((parsed_url.hostname, path))
netloc = parsed_url.netloc
if proto in ('tcp', 'ssh'): if proto in ('tcp', 'ssh'):
# parsed_url.hostname strips brackets from IPv6 addresses, port = parsed_url.port or 0
# which can be problematic hence our use of splitnport() instead. if port <= 0:
host, port = splitnport(parsed_url.netloc)
if port is None or port < 0:
if proto != 'ssh': if proto != 'ssh':
raise errors.DockerException( raise errors.DockerException(
'Invalid bind address format: port is required:' 'Invalid bind address format: port is required:'
' {0}'.format(addr) ' {0}'.format(addr)
) )
port = 22 port = 22
netloc = '{0}:{1}'.format(parsed_url.netloc, port)
if not host: if not parsed_url.hostname:
host = DEFAULT_HTTP_HOST netloc = '{0}:{1}'.format(DEFAULT_HTTP_HOST, port)
# Rewrite schemes to fit library internals (requests adapters) # Rewrite schemes to fit library internals (requests adapters)
if proto == 'tcp': if proto == 'tcp':
@ -305,7 +307,14 @@ def parse_host(addr, is_win32=False, tls=False):
if proto in ('http+unix', 'npipe'): if proto in ('http+unix', 'npipe'):
return "{0}://{1}".format(proto, path).rstrip('/') return "{0}://{1}".format(proto, path).rstrip('/')
return '{0}://{1}:{2}{3}'.format(proto, host, port, path).rstrip('/') return urlunparse(URLComponents(
scheme=proto,
netloc=netloc,
url=path,
params='',
query='',
fragment='',
)).rstrip('/')
def parse_devices(devices): def parse_devices(devices):
@ -370,7 +379,7 @@ def kwargs_from_env(ssl_version=None, assert_hostname=None, environment=None):
# so if it's not set already then set it to false. # so if it's not set already then set it to false.
assert_hostname = False assert_hostname = False
params['tls'] = tls.TLSConfig( params['tls'] = TLSConfig(
client_cert=(os.path.join(cert_path, 'cert.pem'), client_cert=(os.path.join(cert_path, 'cert.pem'),
os.path.join(cert_path, 'key.pem')), os.path.join(cert_path, 'key.pem')),
ca_cert=os.path.join(cert_path, 'ca.pem'), ca_cert=os.path.join(cert_path, 'ca.pem'),

View File

@ -393,7 +393,7 @@ class UnixSocketStreamTest(unittest.TestCase):
self.server_socket = self._setup_socket() self.server_socket = self._setup_socket()
self.stop_server = False self.stop_server = False
server_thread = threading.Thread(target=self.run_server) server_thread = threading.Thread(target=self.run_server)
server_thread.setDaemon(True) server_thread.daemon = True
server_thread.start() server_thread.start()
self.response = None self.response = None
self.request_handler = None self.request_handler = None
@ -519,7 +519,7 @@ class TCPSocketStreamTest(unittest.TestCase):
cls.server = six.moves.socketserver.ThreadingTCPServer( cls.server = six.moves.socketserver.ThreadingTCPServer(
('', 0), cls.get_handler_class()) ('', 0), cls.get_handler_class())
cls.thread = threading.Thread(target=cls.server.serve_forever) cls.thread = threading.Thread(target=cls.server.serve_forever)
cls.thread.setDaemon(True) cls.thread.daemon = True
cls.thread.start() cls.thread.start()
cls.address = 'http://{0}:{1}'.format( cls.address = 'http://{0}:{1}'.format(
socket.gethostname(), cls.server.server_address[1]) socket.gethostname(), cls.server.server_address[1])

View File

@ -285,17 +285,24 @@ class ParseHostTest(unittest.TestCase):
'[fd12::82d1]:2375/docker/engine': ( '[fd12::82d1]:2375/docker/engine': (
'http://[fd12::82d1]:2375/docker/engine' 'http://[fd12::82d1]:2375/docker/engine'
), ),
'ssh://[fd12::82d1]': 'ssh://[fd12::82d1]:22',
'ssh://user@[fd12::82d1]:8765': 'ssh://user@[fd12::82d1]:8765',
'ssh://': 'ssh://127.0.0.1:22', 'ssh://': 'ssh://127.0.0.1:22',
'ssh://user@localhost:22': 'ssh://user@localhost:22', 'ssh://user@localhost:22': 'ssh://user@localhost:22',
'ssh://user@remote': 'ssh://user@remote:22', 'ssh://user@remote': 'ssh://user@remote:22',
} }
for host in invalid_hosts: for host in invalid_hosts:
with pytest.raises(DockerException): msg = 'Should have failed to parse invalid host: {0}'.format(host)
with self.assertRaises(DockerException, msg=msg):
parse_host(host, None) parse_host(host, None)
for host, expected in valid_hosts.items(): for host, expected in valid_hosts.items():
assert parse_host(host, None) == expected self.assertEqual(
parse_host(host, None),
expected,
msg='Failed to parse valid host: {0}'.format(host),
)
def test_parse_host_empty_value(self): def test_parse_host_empty_value(self):
unix_socket = 'http+unix:///var/run/docker.sock' unix_socket = 'http+unix:///var/run/docker.sock'