From 327b1676a8ea43f3add465b230b86f6cde07aed1 Mon Sep 17 00:00:00 2001 From: Toshio Kuratomi Date: Tue, 14 Jul 2015 11:48:41 -0700 Subject: [PATCH] Add support for SNI and TLS-1.1 and TLS-1.2 to the fetch_url() helper Fixes #1716 Fixes #1695 --- lib/ansible/module_utils/urls.py | 75 +++++++++++++++---- .../roles/test_get_url/tasks/main.yml | 32 ++++++++ .../integration/roles/test_uri/tasks/main.yml | 7 +- 3 files changed, 97 insertions(+), 17 deletions(-) diff --git a/lib/ansible/module_utils/urls.py b/lib/ansible/module_utils/urls.py index cf9a652ed1..2ba19b629f 100644 --- a/lib/ansible/module_utils/urls.py +++ b/lib/ansible/module_utils/urls.py @@ -95,9 +95,16 @@ except: try: import ssl - HAS_SSL=True + HAS_SSL = True except: - HAS_SSL=False + HAS_SSL = False + +try: + # SNI Handling needs python2.7.9's SSLContext + from ssl import create_default_context, SSLContext + HAS_SSLCONTEXT = True +except ImportError: + HAS_SSLCONTEXT = False HAS_MATCH_HOSTNAME = True try: @@ -277,6 +284,13 @@ class NoSSLError(SSLValidationError): class CustomHTTPSConnection(httplib.HTTPSConnection): + def __init__(self, *args, **kwargs): + httplib.HTTPSConnection.__init__(self, *args, **kwargs) + if HAS_SSLCONTEXT: + self.context = create_default_context() + if self.cert_file: + self.context.load_cert_chain(self.cert_file, self.key_file) + def connect(self): "Connect to a host on a given (SSL) port." @@ -287,7 +301,10 @@ class CustomHTTPSConnection(httplib.HTTPSConnection): if self._tunnel_host: self.sock = sock self._tunnel() - self.sock = ssl.wrap_socket(sock, keyfile=self.key_file, certfile=self.cert_file, ssl_version=ssl.PROTOCOL_TLSv1) + if HAS_SSLCONTEXT: + self.sock = self.context.wrap_socket(sock, server_hostname=self.host) + else: + self.sock = ssl.wrap_socket(sock, keyfile=self.key_file, certfile=self.cert_file, ssl_version=ssl.PROTOCOL_TLSv1) class CustomHTTPSHandler(urllib2.HTTPSHandler): @@ -462,9 +479,17 @@ class SSLValidationHandler(urllib2.BaseHandler): return False return True + def _make_context(self, tmp_ca_cert_path): + context = create_default_context() + context.load_verify_locations(tmp_ca_cert_path) + return context + def http_request(self, req): tmp_ca_cert_path, paths_checked = self.get_ca_certs() https_proxy = os.environ.get('https_proxy') + context = None + if HAS_SSLCONTEXT: + context = self._make_context(tmp_ca_cert_path) # Detect if 'no_proxy' environment variable is set and if our URL is included use_proxy = self.detect_no_proxy(req.get_full_url()) @@ -486,14 +511,20 @@ class SSLValidationHandler(urllib2.BaseHandler): s.sendall('\r\n') connect_result = s.recv(4096) self.validate_proxy_response(connect_result) - ssl_s = ssl.wrap_socket(s, ca_certs=tmp_ca_cert_path, cert_reqs=ssl.CERT_REQUIRED) - match_hostname(ssl_s.getpeercert(), self.hostname) + if context: + ssl_s = context.wrap_socket(s, server_hostname=proxy_parts.get('hostname')) + else: + ssl_s = ssl.wrap_socket(s, ca_certs=tmp_ca_cert_path, cert_reqs=ssl.CERT_REQUIRED, ssl_version=ssl.PROTOCOL_TLSv1) + match_hostname(ssl_s.getpeercert(), self.hostname) else: raise ProxyError('Unsupported proxy scheme: %s. Currently ansible only supports HTTP proxies.' % proxy_parts.get('scheme')) else: s.connect((self.hostname, self.port)) - ssl_s = ssl.wrap_socket(s, ca_certs=tmp_ca_cert_path, cert_reqs=ssl.CERT_REQUIRED) - match_hostname(ssl_s.getpeercert(), self.hostname) + if context: + ssl_s = context.wrap_socket(s, server_hostname=self.hostname) + else: + ssl_s = ssl.wrap_socket(s, ca_certs=tmp_ca_cert_path, cert_reqs=ssl.CERT_REQUIRED, ssl_version=ssl.PROTOCOL_TLSv1) + match_hostname(ssl_s.getpeercert(), self.hostname) # close the ssl connection #ssl_s.unwrap() s.close() @@ -502,9 +533,14 @@ class SSLValidationHandler(urllib2.BaseHandler): if 'connection refused' in str(e).lower(): raise ConnectionError('Failed to connect to %s:%s.' % (self.hostname, self.port)) else: - raise SSLValidationError('Failed to validate the SSL certificate for %s:%s. ' - 'Use validate_certs=False (insecure) or make sure your managed systems have a valid CA certificate installed. ' - 'Paths checked for this platform: %s' % (self.hostname, self.port, ", ".join(paths_checked)) + raise SSLValidationError('Failed to validate the SSL certificate for %s:%s.' + ' Make sure your managed systems have a valid CA' + ' certificate installed. If the website serving the url' + ' uses SNI you need python >= 2.7.9 on your managed' + ' machine. You can use validate_certs=False if you do' + ' not need to confirm the server\s identity but this is' + ' unsafe and not recommended' + ' Paths checked for this platform: %s' % (self.hostname, self.port, ", ".join(paths_checked)) ) except CertificateError: raise SSLValidationError("SSL Certificate does not belong to %s. Make sure the url has a certificate that belongs to it or use validate_certs=False (insecure)" % self.hostname) @@ -534,8 +570,6 @@ def open_url(url, data=None, headers=None, method=None, use_proxy=True, if parsed[0] == 'https' and validate_certs: if not HAS_SSL: raise NoSSLError('SSL validation is not available in your version of python. You can use validate_certs=False, however this is unsafe and not recommended') - if not HAS_MATCH_HOSTNAME: - raise SSLValidationError('Available SSL validation does not check that the certificate matches the hostname. You can install backports.ssl_match_hostname or update your managed machine to python-2.7.9 or newer. You could also use validate_certs=False, however this is unsafe and not recommended') # do the cert validation netloc = parsed[1] @@ -630,13 +664,22 @@ def open_url(url, data=None, headers=None, method=None, use_proxy=True, for header in headers: request.add_header(header, headers[header]) - if sys.version_info < (2,6,0): + urlopen_args = [request, None] + if sys.version_info >= (2,6,0): # urlopen in python prior to 2.6.0 did not # have a timeout parameter - r = urllib2.urlopen(request, None) - else: - r = urllib2.urlopen(request, None, timeout) + urlopen_args.append(timeout) + if HAS_SSLCONTEXT and not validate_certs: + # In 2.7.9, the default context validates certificates + context = SSLContext(ssl.PROTOCOL_SSLv23) + context.options |= ssl.OP_NO_SSLv2 + context.options |= ssl.OP_NO_SSLv3 + context.verify_mode = ssl.CERT_NONE + context.check_hostname = False + urlopen_args += (None, None, None, context) + + r = urllib2.urlopen(*urlopen_args) return r # diff --git a/test/integration/roles/test_get_url/tasks/main.yml b/test/integration/roles/test_get_url/tasks/main.yml index 88ff3b2e21..6e3842f6ab 100644 --- a/test/integration/roles/test_get_url/tasks/main.yml +++ b/test/integration/roles/test_get_url/tasks/main.yml @@ -60,3 +60,35 @@ that: - "result.changed == true" - "stat_result.stat.exists == true" + +# SNI Tests +# SNI is only built into the stdlib from python-2.7.9 onwards +- name: Test that SNI works + get_url: + # A test site that returns a page with information on what SNI information + # the client sent. A failure would have the string: did not send a TLS server name indication extension + url: 'https://foo.sni.velox.ch/' + dest: "{{ output_dir }}/sni.html" + register: get_url_result + ignore_errors: True + +- command: "grep 'sent the following TLS server name indication extension' {{ output_dir}}/sni.html" + register: data_result + when: "{{ ansible_python_version | version_compare('2.7.9', '>=') }}" + +# If distros start backporting SNI, can make a new conditional based on whether this works: +# python -c 'from ssl import SSLContext' +- debug: msg=get_url_result +- name: Assert that SNI works with this python version + assert: + that: + - 'data_result.rc == 0' + - '"failed" not in get_url_result' + when: "{{ ansible_python_version | version_compare('2.7.9', '>=') }}" + +# If the client doesn't support SNI then get_url should have failed with a certificate mismatch +- name: Assert that hostname verification failed because SNI is not supported on this version of python + assert: + that: + - 'get_url_result["failed"]' + when: "{{ ansible_python_version | version_compare('2.7.9', '<') }}" diff --git a/test/integration/roles/test_uri/tasks/main.yml b/test/integration/roles/test_uri/tasks/main.yml index 99c6048a59..7300578982 100644 --- a/test/integration/roles/test_uri/tasks/main.yml +++ b/test/integration/roles/test_uri/tasks/main.yml @@ -110,6 +110,11 @@ - "'certificate does not match ' in result.msg" - "stat_result.stat.exists == false" +- name: Clean up any cruft from the results directory + file: + name: "{{ output_dir }}/kreitz.html" + state: absent + - name: test https fetch to a site with mismatched hostname and certificate and validate_certs=no get_url: url: "https://kennethreitz.org/" @@ -124,5 +129,5 @@ - name: Assert that the file was downloaded assert: that: - - "result.changed == true" - "stat_result.stat.exists == true" + - "result.changed == true"