From afc19894e1006780d2f248e325f7ecae84bb4f14 Mon Sep 17 00:00:00 2001 From: Toshio Kuratomi Date: Thu, 28 May 2015 12:35:37 -0700 Subject: [PATCH] Make fetch_url check the server's certificate on https connections --- lib/ansible/module_utils/urls.py | 49 ++++++++++++------- .../roles/test_get_url/tasks/main.yml | 20 ++++++++ 2 files changed, 52 insertions(+), 17 deletions(-) diff --git a/lib/ansible/module_utils/urls.py b/lib/ansible/module_utils/urls.py index d56cc89395e..18317e86aeb 100644 --- a/lib/ansible/module_utils/urls.py +++ b/lib/ansible/module_utils/urls.py @@ -50,6 +50,15 @@ try: except: HAS_SSL=False +HAS_MATCH_HOSTNAME = True +try: + from ssl import match_hostname, CertificateError +except ImportError: + try: + from backports.ssl_match_hostname import match_hostname, CertificateError + except ImportError: + HAS_MATCH_HOSTNAME = False + import httplib import os import re @@ -293,11 +302,13 @@ class SSLValidationHandler(urllib2.BaseHandler): 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) else: self.module.fail_json(msg='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) # close the ssl connection #ssl_s.unwrap() s.close() @@ -311,6 +322,9 @@ class SSLValidationHandler(urllib2.BaseHandler): 'Use validate_certs=no or make sure your managed systems have a valid CA certificate installed. ' + \ 'Paths checked for this platform: %s' % ", ".join(paths_checked) ) + except CertificateError: + self.module.fail_json(msg="SSL Certificate does not belong to %s. Make sure the url has a certificate that belongs to it or use validate_certs=no (insecure)" % self.hostname) + try: # cleanup the temp file created, don't worry # if it fails for some reason @@ -363,28 +377,29 @@ def fetch_url(module, url, data=None, headers=None, method=None, # FIXME: change the following to use the generic_urlparse function # to remove the indexed references for 'parsed' parsed = urlparse.urlparse(url) - if parsed[0] == 'https': - if not HAS_SSL and validate_certs: + if parsed[0] == 'https' and validate_certs: + if not HAS_SSL: if distribution == 'Redhat': module.fail_json(msg='SSL validation is not available in your version of python. You can use validate_certs=no, however this is unsafe and not recommended. You can also install python-ssl from EPEL') else: module.fail_json(msg='SSL validation is not available in your version of python. You can use validate_certs=no, however this is unsafe and not recommended') + if not HAS_MATCH_HOSTNAME: + module.fail_json(msg='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=no, however this is unsafe and not recommended') - elif validate_certs: - # do the cert validation - netloc = parsed[1] - if '@' in netloc: - netloc = netloc.split('@', 1)[1] - if ':' in netloc: - hostname, port = netloc.split(':', 1) - port = int(port) - else: - hostname = netloc - port = 443 - # create the SSL validation handler and - # add it to the list of handlers - ssl_handler = SSLValidationHandler(module, hostname, port) - handlers.append(ssl_handler) + # do the cert validation + netloc = parsed[1] + if '@' in netloc: + netloc = netloc.split('@', 1)[1] + if ':' in netloc: + hostname, port = netloc.split(':', 1) + port = int(port) + else: + hostname = netloc + port = 443 + # create the SSL validation handler and + # add it to the list of handlers + ssl_handler = SSLValidationHandler(module, hostname, port) + handlers.append(ssl_handler) if parsed[0] != 'ftp': username = module.params.get('url_username', '') diff --git a/test/integration/roles/test_get_url/tasks/main.yml b/test/integration/roles/test_get_url/tasks/main.yml index 1aa4b287ea7..6d016fe6be3 100644 --- a/test/integration/roles/test_get_url/tasks/main.yml +++ b/test/integration/roles/test_get_url/tasks/main.yml @@ -25,3 +25,23 @@ that: - result.changed - '"OK" in result.msg' + +- name: test https fetch to a site with invalid domain + get_url: + url: "https://kennethreitz.org/" + dest: "{{ output_dir }}/shouldnotexist.html" + ignore_errors: True + register: result + +- stat: + path: "{{ output_dir }}/shouldnotexist.html" + register: stat_result + +- debug: var=result + +- name: Assert that the file was not downloaded + assert: + that: + - "result.failed == true" + - "'Certificate does not belong to ' in result.msg" + - "stat_result.stat.exists == false"