From 101fcb55ce97d565617294b2a8b020db7165cf85 Mon Sep 17 00:00:00 2001 From: Michal Minar Date: Mon, 16 Dec 2013 14:40:01 +0100 Subject: [PATCH 1/4] fixed TOCTOU error when validating certificate Resolves: rhbz#1026891 --- pywbem-20130723-ssl_verify_host.patch | 232 ++++++++++++++++++++++++++ pywbem.spec | 9 +- 2 files changed, 240 insertions(+), 1 deletion(-) create mode 100644 pywbem-20130723-ssl_verify_host.patch diff --git a/pywbem-20130723-ssl_verify_host.patch b/pywbem-20130723-ssl_verify_host.patch new file mode 100644 index 0000000..41a58a4 --- /dev/null +++ b/pywbem-20130723-ssl_verify_host.patch @@ -0,0 +1,232 @@ +Index: pywbem-20131121/cim_http.py +=================================================================== +--- pywbem-20131121.orig/cim_http.py ++++ pywbem-20131121/cim_http.py +@@ -28,6 +28,7 @@ being transferred is XML. It is up to t + data and interpret the result. + ''' + ++from M2Crypto import SSL, Err + import sys, string, re, os, socket, getpass + from stat import S_ISSOCK + import cim_obj +@@ -74,8 +75,25 @@ def parse_url(url): + + return host, port, ssl + ++def get_default_ca_certs(): ++ """ ++ Try to find out system path with ca certificates. This path is cached and ++ returned. If no path is found out, None is returned. ++ """ ++ if not hasattr(get_default_ca_certs, '_path'): ++ for path in ( ++ '/etc/ssl/certs', ++ '/etc/ssl/certificates'): ++ if os.path.exists(path): ++ get_default_ca_certs._path = path ++ break ++ else: ++ get_default_ca_certs._path = None ++ return get_default_ca_certs._path ++ + def wbem_request(url, data, creds, headers = [], debug = 0, x509 = None, +- verify_callback = None): ++ verify_callback = None, ca_certs = None, ++ no_verification = False): + """Send XML data over HTTP to the specified url. Return the + response in XML. Uses Python's build-in httplib. x509 may be a + dictionary containing the location of the SSL certificate and key +@@ -105,10 +123,35 @@ def wbem_request(url, data, creds, heade + + class HTTPSConnection(HTTPBaseConnection, httplib.HTTPSConnection): + def __init__(self, host, port=None, key_file=None, cert_file=None, +- strict=None): ++ strict=None, ca_certs=None, verify_callback=None): + httplib.HTTPSConnection.__init__(self, host, port, key_file, + cert_file, strict) +- ++ self.ca_certs = ca_certs ++ self.verify_callback = verify_callback ++ ++ def connect(self): ++ "Connect to a host on a given (SSL) port." ++ sock = socket.create_connection((self.host, self.port), ++ self.timeout, self.source_address) ++ if self._tunnel_host: ++ self.sock = sock ++ self._tunnel() ++ ctx = SSL.Context('sslv3') ++ if self.cert_file: ++ ctx.load_cert(self.cert_file, keyfile=self.key_file) ++ if self.ca_certs: ++ ctx.set_verify(SSL.verify_peer | SSL.verify_fail_if_no_peer_cert, ++ depth=9, callback=verify_callback) ++ if os.path.isdir(self.ca_certs): ++ ctx.load_verify_locations(capath=self.ca_certs) ++ else: ++ ctx.load_verify_locations(cafile=self.ca_certs) ++ try: ++ self.sock = SSL.Connection(ctx) ++ self.sock.connect((self.host, self.port)) ++ except Err.SSLError, arg: ++ raise Error("SSL error: %s" % arg) ++ + class FileHTTPConnection(HTTPBaseConnection, httplib.HTTPConnection): + def __init__(self, uds_path): + httplib.HTTPConnection.__init__(self, 'localhost') +@@ -117,53 +160,14 @@ def wbem_request(url, data, creds, heade + self.sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) + self.sock.connect(self.uds_path) + +- host, port, ssl = parse_url(url) ++ host, port, use_ssl = parse_url(url) + + key_file = None + cert_file = None + +- if ssl: +- +- if x509 is not None: +- cert_file = x509.get('cert_file') +- key_file = x509.get('key_file') +- +- if verify_callback is not None: +- addr_ind = 0 +- # Temporary exception store +- addr_exc = None +- # Get a list of arguments for socket(). +- addr_list = socket.getaddrinfo(host, port, 0, socket.SOCK_STREAM) +- for addr_ind in xrange(len(addr_list)): +- family, socktype, proto, canonname, sockaddr = addr_list[addr_ind] +- try: +- from OpenSSL import SSL +- ctx = SSL.Context(SSL.SSLv3_METHOD) +- ctx.set_verify(SSL.VERIFY_PEER, verify_callback) +- ctx.set_default_verify_paths() +- # Add the key and certificate to the session +- if cert_file is not None and key_file is not None: +- ctx.use_certificate_file(cert_file) +- ctx.use_privatekey_file(key_file) +- s = SSL.Connection(ctx, socket.socket(family, socktype, proto)) +- s.connect((host, port)) +- s.do_handshake() +- s.shutdown() +- s.close() +- addr_exc = None +- break +- except (socket.gaierror, socket.error), arg: +- # Could not perform connect() call, store the exception object for +- # later use. +- addr_exc = arg +- continue +- except socket.sslerror, arg: +- raise Error("SSL error: %s" % (arg,)) +- +- # Did we try all the addresses from getaddrinfo() and no successful +- # connection performed? +- if addr_exc: +- raise Error("Socket error: %s" % (addr_exc),) ++ if use_ssl and x509 is not None: ++ cert_file = x509.get('cert_file') ++ key_file = x509.get('key_file') + + numTries = 0 + localAuthHeader = None +@@ -171,10 +175,19 @@ def wbem_request(url, data, creds, heade + + data = '\n' + data + ++ if not no_verification and ca_certs is None: ++ ca_certs = get_default_ca_certs() ++ elif no_verification: ++ ca_certs = None ++ + local = False +- if ssl: +- h = HTTPSConnection(host, port = port, key_file = key_file, +- cert_file = cert_file) ++ if use_ssl: ++ h = HTTPSConnection(host, ++ port = port, ++ key_file = key_file, ++ cert_file = cert_file, ++ ca_certs = ca_certs, ++ verify_callback = verify_callback) + else: + if url.startswith('http'): + h = HTTPConnection(host, port = port) +Index: pywbem-20131121/cim_operations.py +=================================================================== +--- pywbem-20131121.orig/cim_operations.py ++++ pywbem-20131121/cim_operations.py +@@ -78,12 +78,12 @@ class WBEMConnection(object): + the request before it is sent, and the reply before it is + unpacked. + +- verify_callback is used to verify the server certificate. +- It is passed to OpenSSL.SSL.set_verify, and is called during the SSL +- handshake. verify_callback should take five arguments: A Connection +- object, an X509 object, and three integer variables, which are in turn +- potential error number, error depth and return code. verify_callback +- should return True if verification passes and False otherwise. ++ verify_callback is used to verify the server certificate. It is passed to ++ M2Crypto.SSL.Context.set_verify, and is called during the SSL handshake. ++ verify_callback should take five arguments: An SSL Context object, an X509 ++ object, and three integer variables, which are in turn potential error ++ number, error depth and return code. verify_callback should return True if ++ verification passes and False otherwise. + + The value of the x509 argument is used only when the url contains + 'https'. x509 must be a dictionary containing the keys 'cert_file' +@@ -91,14 +91,27 @@ class WBEMConnection(object): + filename of an certificate and the value of 'key_file' must consist + of a filename containing the private key belonging to the public key + that is part of the certificate in cert_file. ++ ++ ca_certs specifies where CA certificates for verification purposes are ++ located. These are trusted certificates. Note that the certificates have to ++ be in PEM format. Either it is a directory prepared using the c_rehash tool ++ included with OpenSSL or an pemfile. If None, default system path will be ++ used. ++ ++ no_verification allows to disable peer's verification. This is insecure and ++ should be avoided. If True, peer's certificate is not verified and ca_certs ++ argument is ignored. + """ + + def __init__(self, url, creds = None, default_namespace = DEFAULT_NAMESPACE, +- x509 = None, verify_callback = None): ++ x509 = None, verify_callback = None, ca_certs = None, ++ no_verification = False): + self.url = url + self.creds = creds + self.x509 = x509 + self.verify_callback = verify_callback ++ self.ca_certs = ca_certs ++ self.no_verification = no_verification + self.last_request = self.last_reply = '' + self.default_namespace = default_namespace + self.debug = False +@@ -164,7 +177,9 @@ class WBEMConnection(object): + resp_xml = cim_http.wbem_request(self.url, req_xml.toxml(), + self.creds, headers, + x509 = self.x509, +- verify_callback = self.verify_callback) ++ verify_callback = self.verify_callback, ++ ca_certs = self.ca_certs, ++ no_verification = self.no_verification) + except cim_http.AuthError: + raise + except cim_http.Error, arg: +@@ -321,7 +336,9 @@ class WBEMConnection(object): + resp_xml = cim_http.wbem_request(self.url, req_xml.toxml(), + self.creds, headers, + x509 = self.x509, +- verify_callback = self.verify_callback) ++ verify_callback = self.verify_callback, ++ ca_certs = self.ca_certs, ++ no_verification = self.no_verification) + except cim_http.Error, arg: + # Convert cim_http exceptions to CIMError exceptions + raise CIMError(0, str(arg)) diff --git a/pywbem.spec b/pywbem.spec index 099ea84..20a8615 100644 --- a/pywbem.spec +++ b/pywbem.spec @@ -4,7 +4,7 @@ Name: pywbem Version: 0.7.0 -Release: 17.%{revdate}svn%{svnrev}%{?dist} +Release: 18.%{revdate}svn%{svnrev}%{?dist} Summary: Python WBEM Client and Provider Interface Group: Development/Libraries License: LGPLv2 @@ -16,9 +16,11 @@ URL: http://pywbem.sourceforge.net Source0: %{name}-%{revdate}.tar.xz BuildRequires: python-setuptools-devel BuildArch: noarch +Requires: m2crypto # fix module imports in /usr/bin/mofcomp Patch0: pywbem-20130411-mof_compiler-import.patch +Patch1: pywbem-20130723-ssl_verify_host.patch %description A Python library for making CIM (Common Information Model) operations over HTTP @@ -50,6 +52,7 @@ twisted.protocols.http.HTTPClient base class. %prep %setup -q -n %{name}-%{revdate} %patch0 -p1 -b .mofcomp-imports +%patch1 -p1 -b .ssl_verify_host %build # dirty workaround to fix the mof_compiler.py module path @@ -80,6 +83,10 @@ rm -rf %{buildroot} %{python_sitelib}/pywbem/twisted_client.py* %changelog +* Mon Dec 16 2013 Michal Minar 0.7.0-18.20131121svn656 +- Fixes TOCTOU vulnerability in certificate validation. +- Resolves: rhbz#1026891 + * Thu Nov 21 2013 Jan Safranek 0.7.0-17.20131121svn626 - Added '-d' option to /usr/bin/mofcomp to just check mof files and their includes. From b00fe8f78116d799f2be9ba977d99014a612ae83 Mon Sep 17 00:00:00 2001 From: Michal Minar Date: Mon, 16 Dec 2013 15:24:11 +0100 Subject: [PATCH 2/4] tweaked ssl_verify_host patch a bit Added dependency on m2crypto package to setup. --- pywbem-20130723-ssl_verify_host.patch | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/pywbem-20130723-ssl_verify_host.patch b/pywbem-20130723-ssl_verify_host.patch index 41a58a4..621ec2f 100644 --- a/pywbem-20130723-ssl_verify_host.patch +++ b/pywbem-20130723-ssl_verify_host.patch @@ -10,7 +10,7 @@ Index: pywbem-20131121/cim_http.py import sys, string, re, os, socket, getpass from stat import S_ISSOCK import cim_obj -@@ -74,8 +75,25 @@ def parse_url(url): +@@ -74,8 +75,26 @@ def parse_url(url): return host, port, ssl @@ -21,6 +21,7 @@ Index: pywbem-20131121/cim_http.py + """ + if not hasattr(get_default_ca_certs, '_path'): + for path in ( ++ '/etc/pki/tls/certs', + '/etc/ssl/certs', + '/etc/ssl/certificates'): + if os.path.exists(path): @@ -28,7 +29,7 @@ Index: pywbem-20131121/cim_http.py + break + else: + get_default_ca_certs._path = None -+ return get_default_ca_certs._path ++ return get_default_ca_certs._path + def wbem_request(url, data, creds, headers = [], debug = 0, x509 = None, - verify_callback = None): @@ -37,7 +38,7 @@ Index: pywbem-20131121/cim_http.py """Send XML data over HTTP to the specified url. Return the response in XML. Uses Python's build-in httplib. x509 may be a dictionary containing the location of the SSL certificate and key -@@ -105,10 +123,35 @@ def wbem_request(url, data, creds, heade +@@ -105,10 +124,35 @@ def wbem_request(url, data, creds, heade class HTTPSConnection(HTTPBaseConnection, httplib.HTTPSConnection): def __init__(self, host, port=None, key_file=None, cert_file=None, @@ -75,7 +76,7 @@ Index: pywbem-20131121/cim_http.py class FileHTTPConnection(HTTPBaseConnection, httplib.HTTPConnection): def __init__(self, uds_path): httplib.HTTPConnection.__init__(self, 'localhost') -@@ -117,53 +160,14 @@ def wbem_request(url, data, creds, heade +@@ -117,53 +161,14 @@ def wbem_request(url, data, creds, heade self.sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) self.sock.connect(self.uds_path) @@ -133,7 +134,7 @@ Index: pywbem-20131121/cim_http.py numTries = 0 localAuthHeader = None -@@ -171,10 +175,19 @@ def wbem_request(url, data, creds, heade +@@ -171,10 +176,19 @@ def wbem_request(url, data, creds, heade data = '\n' + data @@ -230,3 +231,15 @@ Index: pywbem-20131121/cim_operations.py except cim_http.Error, arg: # Convert cim_http exceptions to CIMError exceptions raise CIMError(0, str(arg)) +Index: pywbem-20131121/setup.py +=================================================================== +--- pywbem-20131121.orig/setup.py ++++ pywbem-20131121/setup.py +@@ -37,6 +37,7 @@ args = {'name': 'pywbem', + 'version': '0.7.0', + 'license': 'LGPLv2', + 'packages': ['pywbem'], ++ 'install_requires': ['M2Crypto'], + # Make packages in root dir appear in pywbem module + 'package_dir': {'pywbem': ''}, + # Make extensions in root dir appear in pywbem module From 414b922d8ee37fdf57f8a74a7c8e6b30f6180e67 Mon Sep 17 00:00:00 2001 From: Michal Minar Date: Tue, 17 Dec 2013 10:20:53 +0100 Subject: [PATCH 3/4] tweaked the ssl_verify_host patch M2Crypto exceptions could propagage to client code which is not desired because it's an API change. Thery are now catched and turned into pywbem exceptions. --- pywbem-20130723-ssl_verify_host.patch | 2 +- pywbem.spec | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/pywbem-20130723-ssl_verify_host.patch b/pywbem-20130723-ssl_verify_host.patch index 621ec2f..20d4218 100644 --- a/pywbem-20130723-ssl_verify_host.patch +++ b/pywbem-20130723-ssl_verify_host.patch @@ -70,7 +70,7 @@ Index: pywbem-20131121/cim_http.py + try: + self.sock = SSL.Connection(ctx) + self.sock.connect((self.host, self.port)) -+ except Err.SSLError, arg: ++ except (Err.SSLError, SSL.SSLError, SSL.SSLTimeoutError), arg: + raise Error("SSL error: %s" % arg) + class FileHTTPConnection(HTTPBaseConnection, httplib.HTTPConnection): diff --git a/pywbem.spec b/pywbem.spec index 20a8615..83bffb1 100644 --- a/pywbem.spec +++ b/pywbem.spec @@ -4,7 +4,7 @@ Name: pywbem Version: 0.7.0 -Release: 18.%{revdate}svn%{svnrev}%{?dist} +Release: 19.%{revdate}svn%{svnrev}%{?dist} Summary: Python WBEM Client and Provider Interface Group: Development/Libraries License: LGPLv2 @@ -83,6 +83,9 @@ rm -rf %{buildroot} %{python_sitelib}/pywbem/twisted_client.py* %changelog +* Tue Dec 17 2013 Michal Minar 0.7.0-19.20131121svn656 +- Tweaked the ssl_verify_host patch. + * Mon Dec 16 2013 Michal Minar 0.7.0-18.20131121svn656 - Fixes TOCTOU vulnerability in certificate validation. - Resolves: rhbz#1026891 From 961fb49d2c84211bfe5ea1bd1b7dd6104091f37d Mon Sep 17 00:00:00 2001 From: Michal Minar Date: Tue, 17 Dec 2013 10:47:53 +0100 Subject: [PATCH 4/4] fixed build added missing BuildRequires --- pywbem.spec | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pywbem.spec b/pywbem.spec index 83bffb1..b7982be 100644 --- a/pywbem.spec +++ b/pywbem.spec @@ -4,7 +4,7 @@ Name: pywbem Version: 0.7.0 -Release: 19.%{revdate}svn%{svnrev}%{?dist} +Release: 20.%{revdate}svn%{svnrev}%{?dist} Summary: Python WBEM Client and Provider Interface Group: Development/Libraries License: LGPLv2 @@ -15,6 +15,7 @@ URL: http://pywbem.sourceforge.net # tar -cJvf pywbem-20130128.tar.xz pywbem-20130128 Source0: %{name}-%{revdate}.tar.xz BuildRequires: python-setuptools-devel +BuildRequires: m2crypto BuildArch: noarch Requires: m2crypto @@ -83,7 +84,7 @@ rm -rf %{buildroot} %{python_sitelib}/pywbem/twisted_client.py* %changelog -* Tue Dec 17 2013 Michal Minar 0.7.0-19.20131121svn656 +* Tue Dec 17 2013 Michal Minar 0.7.0-20.20131121svn656 - Tweaked the ssl_verify_host patch. * Mon Dec 16 2013 Michal Minar 0.7.0-18.20131121svn656