From eff7bad45a543f27c471a7e9bb699fabd83d7cbd Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 22 2016 21:41:31 +0000 Subject: [PATCH 1/45] pull requests bits from py3 work auth not working untrusted https not working --- diff --git a/koji/__init__.py b/koji/__init__.py index 54d7356..aceaeb5 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -44,6 +44,7 @@ import os.path import pwd import random import re +import requests import rpm import shutil import signal @@ -1834,53 +1835,13 @@ class ClientSession(object): self.baseurl = baseurl self.opts = opts self._connection = None - self._setup_connection() self.authtype = None self.setSession(sinfo) self.multicall = False self._calls = [] self.logger = logging.getLogger('koji') - - def _setup_connection(self): - uri = urlparse.urlsplit(self.baseurl) - scheme = uri[0] - self._host, _port = urllib.splitport(uri[1]) - self.explicit_port = bool(_port) - self._path = uri[2] - default_port = 80 - if self.opts.get('certs'): - ctx = ssl.SSLCommon.CreateSSLContext(self.opts['certs']) - cnxOpts = {'ssl_context' : ctx} - cnxClass = ssl.SSLCommon.PlgHTTPSConnection - default_port = 443 - elif scheme == 'https': - cnxOpts = {} - if sys.version_info[:3] >= (2, 7, 9): - #ctx = pyssl.SSLContext(pyssl.PROTOCOL_SSLv23) - ctx = pyssl._create_unverified_context() - # TODO - we should default to verifying where possible - cnxOpts['context'] = ctx - cnxClass = httplib.HTTPSConnection - default_port = 443 - elif scheme == 'http': - cnxOpts = {} - cnxClass = httplib.HTTPConnection - else: - raise IOError, "unsupported XML-RPC protocol" - # set a default 12 hour connection timeout. - # Some Koji operations can take a long time to return, but after 12 - # hours we can assume something is seriously wrong. - timeout = self.opts.setdefault('timeout', 60 * 60 * 12) - self._timeout_compat = False - if timeout: - if sys.version_info[:3] < (2, 6, 0) and 'ssl_context' not in cnxOpts: - self._timeout_compat = True - else: - cnxOpts['timeout'] = timeout - self._port = (_port and int(_port) or default_port) - self._cnxOpts = cnxOpts - self._cnxClass = cnxClass - self._close_connection() + self.rsession = requests.Session() + self.opts.setdefault('timeout', 60 * 60 * 12) def setSession(self, sinfo): """Set the session info @@ -1890,7 +1851,6 @@ class ClientSession(object): self.logged_in = False self.callnum = None # do we need to do anything else here? - self._setup_connection() self.authtype = None else: self.logged_in = True @@ -2076,11 +2036,11 @@ class ClientSession(object): sinfo = self.sinfo.copy() sinfo['callnum'] = self.callnum self.callnum += 1 - handler = "%s?%s" % (self._path, urllib.urlencode(sinfo)) + handler = "%s?%s" % (self.baseurl, urllib.urlencode(sinfo)) elif name == 'sslLogin': - handler = self._path + '/ssllogin' + handler = self.baseurl + '/ssllogin' else: - handler = self._path + handler = self.baseurl request = dumps(args, name, allow_none=1) headers = [ # connection class handles Host @@ -2106,54 +2066,34 @@ class ClientSession(object): def _sendOneCall(self, handler, headers, request): - cnx = self._get_connection() - if self.opts.get('debug_xmlrpc', False): - cnx.set_debuglevel(1) - cnx.putrequest('POST', handler) - for n, v in headers: - cnx.putheader(n, v) - cnx.endheaders() - cnx.send(request) - response = cnx.getresponse() + # TODO: honor debug_xmlrpc + request = request.encode('utf8') + headers = dict(headers) + callopts = { + 'headers': headers, + 'data': request, + 'stream': True, + } + cert = self.opts.get('certs', {}).get('peer_ca_cert') + if cert: + callopts['verify'] = cert + #else: + # callopts['verify'] = False + r = self.rsession.post(handler, **callopts) try: - ret = self._read_xmlrpc_response(response, handler) + ret = self._read_xmlrpc_response(r) finally: - response.close() + r.close() return ret - def _get_connection(self): - key = (self._cnxClass, self._host, self._port) - if self._connection and self.opts.get('keepalive'): - if key == self._connection[0]: - cnx = self._connection[1] - if getattr(cnx, 'sock', None): - return cnx - cnx = self._cnxClass(self._host, self._port, **self._cnxOpts) - self._connection = (key, cnx) - if self._timeout_compat: - # in python < 2.6 httplib does not support the timeout option - # but socket supports it since 2.3 - cnx.connect() - cnx.sock.settimeout(self.opts['timeout']) - return cnx - def _close_connection(self): if self._connection: self._connection[1].close() self._connection = None - def _read_xmlrpc_response(self, response, handler=''): - #XXX honor debug_xmlrpc - if response.status != 200: - if (response.getheader("content-length", 0)): - response.read() - raise xmlrpclib.ProtocolError(self._host + handler, - response.status, response.reason, response.msg) + def _read_xmlrpc_response(self, response): p, u = xmlrpclib.getparser() - while True: - chunk = response.read(8192) - if not chunk: - break + for chunk in response.iter_content(8192): if self.opts.get('debug_xmlrpc', False): print "body: %r" % chunk p.feed(chunk) From fc8a8c6582c5e3b7a8a3a4b887061ba7a3f150a1 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 22 2016 21:41:31 +0000 Subject: [PATCH 2/45] parse host from baseurl in _serverPrincipal() krb auth working now --- diff --git a/koji/__init__.py b/koji/__init__.py index aceaeb5..a7bf055 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -1945,13 +1945,13 @@ class ClientSession(object): def _serverPrincipal(self, cprinc): """Get the Kerberos principal of the server we're connecting to, based on baseurl.""" + + uri = urlparse.urlsplit(self.baseurl) + host, port = urllib.splitport(uri[1]) if self.opts.get('krb_rdns', True): - servername = socket.getfqdn(self._host) + servername = socket.getfqdn(host) else: - servername = self._host - #portspec = servername.find(':') - #if portspec != -1: - # servername = servername[:portspec] + servername = host realm = cprinc.realm service = self.opts.get('krbservice', 'host') From 37bd3c460b742f3a60605b0bae6e2ac8dcd54849 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 22 2016 21:41:31 +0000 Subject: [PATCH 3/45] ssl auth working --- diff --git a/koji/__init__.py b/koji/__init__.py index a7bf055..7e17468 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -1964,21 +1964,15 @@ class ClientSession(object): # FIXME: ca is not useful here and therefore ignored, can be removed # when API is changed - ctx = ssl.SSLCommon.CreateSSLContext(certs) - self._cnxOpts = {'ssl_context' : ctx} # 60 second timeout during login - old_timeout = self._cnxOpts.get('timeout') - self._cnxOpts['timeout'] = 60 + old_opts = self.opts + self.opts = old_opts.copy() + self.opts['timeout'] = 60 + self.opts['certs'] = certs try: - self._cnxClass = ssl.SSLCommon.PlgHTTPSConnection - if self._port == 80 and not self.explicit_port: - self._port = 443 sinfo = self.callMethod('sslLogin', proxyuser) finally: - if old_timeout is None: - del self._cnxOpts['timeout'] - else: - self._cnxOpts['timeout'] = old_timeout + self.opts = old_opts if not sinfo: raise AuthError, 'unable to obtain a session' @@ -2074,11 +2068,25 @@ class ClientSession(object): 'data': request, 'stream': True, } - cert = self.opts.get('certs', {}).get('peer_ca_cert') + verify = self.opts.get('certs', {}).get('peer_ca_cert') + if verify: + callopts['verify'] = verify + else: + callopts['verify'] = False + # XXX - not great, but this is the previous behavior + cert = self.opts.get('certs', {}).get('key_and_cert') if cert: - callopts['verify'] = cert + # TODO: we really only need to do this for ssllogin calls + callopts['cert'] = cert + timeout = self.opts.get('timeout') + if timeout: + callopts['timeout'] = timeout #else: # callopts['verify'] = False + if self.opts.get('debug_xmlrpc', False): + print "url: %s" % handler + for _key in callopts: + print "%s: %s" % (_key, callopts[_key]) r = self.rsession.post(handler, **callopts) try: ret = self._read_xmlrpc_response(r) From 9af4c393f51001c672fb3faa5b03beb022a600b2 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 22 2016 21:41:31 +0000 Subject: [PATCH 4/45] move is_cert_error(); require python-requests --- diff --git a/koji.spec b/koji.spec index 5d12d27..3beff7e 100644 --- a/koji.spec +++ b/koji.spec @@ -28,6 +28,7 @@ BuildArch: noarch Requires: python-krbV >= 1.0.13 Requires: rpm-python Requires: pyOpenSSL +Requires: python-requests Requires: python-urlgrabber Requires: python-dateutil BuildRequires: python diff --git a/koji/__init__.py b/koji/__init__.py index 7e17468..15a3633 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -38,6 +38,7 @@ import imp import logging import logging.handlers from koji.util import md5_constructor +from OpenSSL.SSL import Error as SSL_Error import optparse import os import os.path @@ -49,7 +50,6 @@ import rpm import shutil import signal import socket -import ssl.SSLCommon try: from ssl import ssl as pyssl except ImportError: # pragma: no cover @@ -1811,6 +1811,44 @@ class PathInfo(object): pathinfo = PathInfo() + +def is_cert_error(e): + """Determine if an OpenSSL error is due to a bad cert""" + + if not isinstance(e, SSL_Error): + return False + + # pyOpenSSL doesn't use different exception + # subclasses, we have to actually parse the args + for arg in e.args: + # First, check to see if 'arg' is iterable because + # it can be anything.. + try: + iter(arg) + except TypeError: + continue + + # We do all this so that we can detect cert expiry + # so we can avoid retrying those over and over. + for items in arg: + try: + iter(items) + except TypeError: + continue + + if len(items) != 3: + continue + + _, _, ssl_reason = items + + if ('certificate revoked' in ssl_reason or + 'certificate expired' in ssl_reason): + return True + + #otherwise + return False + + class VirtualMethod(object): # some magic to bind an XML-RPC method to an RPC server. # supports "nested" methods (e.g. examples.getStateName) @@ -2157,7 +2195,7 @@ class ClientSession(object): tb_str = ''.join(traceback.format_exception(*sys.exc_info())) self._close_connection() - if ssl.SSLCommon.is_cert_error(e): + if is_cert_error(e): # There's no point in retrying for this raise diff --git a/koji/ssl/SSLCommon.py b/koji/ssl/SSLCommon.py index fa5e64a..ffcd5e9 100644 --- a/koji/ssl/SSLCommon.py +++ b/koji/ssl/SSLCommon.py @@ -28,43 +28,6 @@ def our_verify(connection, x509, errNum, errDepth, preverifyOK): return preverifyOK -def is_cert_error(e): - """Determine if an OpenSSL error is due to a bad cert""" - - if not isinstance(e, SSL.Error): - return False - - # pyOpenSSL doesn't use different exception - # subclasses, we have to actually parse the args - for arg in e.args: - # First, check to see if 'arg' is iterable because - # it can be anything.. - try: - iter(arg) - except TypeError: - continue - - # We do all this so that we can detect cert expiry - # so we can avoid retrying those over and over. - for items in arg: - try: - iter(items) - except TypeError: - continue - - if len(items) != 3: - continue - - _, _, ssl_reason = items - - if ('certificate revoked' in ssl_reason or - 'certificate expired' in ssl_reason): - return True - - #otherwise - return False - - def CreateSSLContext(certs): key_and_cert = certs['key_and_cert'] peer_ca_cert = certs['peer_ca_cert'] From aed2a1a0542175f866efd72fd2d95df60983a31b Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 22 2016 21:41:31 +0000 Subject: [PATCH 5/45] honor debug_xmlrpc --- diff --git a/koji/__init__.py b/koji/__init__.py index 15a3633..55abb15 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -2098,8 +2098,6 @@ class ClientSession(object): def _sendOneCall(self, handler, headers, request): - # TODO: honor debug_xmlrpc - request = request.encode('utf8') headers = dict(headers) callopts = { 'headers': headers, @@ -2119,12 +2117,13 @@ class ClientSession(object): timeout = self.opts.get('timeout') if timeout: callopts['timeout'] = timeout - #else: - # callopts['verify'] = False if self.opts.get('debug_xmlrpc', False): print "url: %s" % handler for _key in callopts: - print "%s: %s" % (_key, callopts[_key]) + _val = callopts[_key] + if _key == 'data' and len(_val) > 1024: + _val = _val[:1024] + '...' + print "%s: %r" % (_key, _val) r = self.rsession.post(handler, **callopts) try: ret = self._read_xmlrpc_response(r) From 6f08b8d064933dedb20c13feee4cdc6b709758cd Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 22 2016 21:41:31 +0000 Subject: [PATCH 6/45] fix handler url for upload --- diff --git a/koji/__init__.py b/koji/__init__.py index 55abb15..8317a60 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -2318,7 +2318,7 @@ class ClientSession(object): args['overwrite'] = "1" size = len(chunk) self.callnum += 1 - handler = "%s?%s" % (self._path, urllib.urlencode(args)) + handler = "%s?%s" % (self.baseurl, urllib.urlencode(args)) headers = [ ('User-Agent', 'koji/1.7'), #XXX ("Content-Type", "application/octet-stream"), From 9b4ccd18ecb14aa9a64a254b7b727d98f4893a0a Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 22 2016 21:41:31 +0000 Subject: [PATCH 7/45] stop mocking removed function in tests --- diff --git a/tests/test_krbv.py b/tests/test_krbv.py index b8a88e1..0fea679 100644 --- a/tests/test_krbv.py +++ b/tests/test_krbv.py @@ -9,8 +9,7 @@ import koji class KrbVTestCase(unittest.TestCase): @mock.patch('koji.krbV', new=None) - @mock.patch('koji.ClientSession._setup_connection') - def test_krbv_disabled(self, krbV): + def test_krbv_disabled(self): """ Test that when krbV is absent, we behave rationally. """ self.assertEquals(koji.krbV, None) session = koji.ClientSession('whatever') From ce768fc59ecb5a3c37e112094662e98cbc65bcc7 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 22 2016 21:41:31 +0000 Subject: [PATCH 8/45] stub for compatrequests --- diff --git a/koji/compatrequests.py b/koji/compatrequests.py new file mode 100644 index 0000000..4dfd23a --- /dev/null +++ b/koji/compatrequests.py @@ -0,0 +1,23 @@ +""" +koji.compatrequests +~~~~~~~~~~~~~~~~~~~ + +This module contains a *very limited* partial implemention of the requests +module that is based on the older codepaths in koji. It only provides +the bits that koji needs. +""" + + +class Session(object): + + def post(self, **kwargs): + pass + + +class Response(object): + + def __init__(self, session): + self.session = session + + def iter_content(self, blocksize=8192): + pass From a211a564014980bb213e420040a979e5503674f2 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 22 2016 21:41:31 +0000 Subject: [PATCH 9/45] More work on compatrequests --- diff --git a/koji/compatrequests.py b/koji/compatrequests.py index 4dfd23a..a8d066d 100644 --- a/koji/compatrequests.py +++ b/koji/compatrequests.py @@ -10,14 +10,96 @@ the bits that koji needs. class Session(object): - def post(self, **kwargs): - pass + def __init__(self): + self.connection = None + + def post(self, url, data=None, headers=None, stream=None, verify=None, + cert=None, timeout=None): + cnx = self.get_connection(url, cert, timeout) + # TODO get handler from url + cnx.putrequest('POST', handler) + if headers: + for k in headers: + cnx.putheader(k, headers[k]) + cnx.endheaders() + if data is not None: + cnx.send(data) + response = cnx.getresponse() + return Response(self, response) + + def get_connection(self, url, cert, timeout): + key = (url, cert, timeout) + if self.connection and self.opts.get('keepalive'): + if key == self.connection[0]: + cnx = self._connection[1] + if getattr(cnx, 'sock', None): + return cnx + # Otherwise we make a new one + uri = urlparse.urlsplit(url) + scheme = uri[0] + host, port = urllib.splitport(uri[1]) + path = uri[2] + default_port = 80 + if cert: + ctx = ssl.SSLCommon.CreateSSLContext(cert) #XXX + cnxOpts = {'ssl_context' : ctx} + cnxClass = ssl.SSLCommon.PlgHTTPSConnection + default_port = 443 + elif scheme == 'https': + cnxOpts = {} + if sys.version_info[:3] >= (2, 7, 9): + ctx = pyssl._create_unverified_context() + # TODO - we should default to verifying where possible + cnxOpts['context'] = ctx + cnxClass = httplib.HTTPSConnection + default_port = 443 + elif scheme == 'http': + cnxOpts = {} + cnxClass = httplib.HTTPConnection + else: + raise IOError, "unsupported protocol: %s" % scheme + + timeout_compat = False + if timeout: + if sys.version_info[:3] < (2, 6, 0) and 'ssl_context' not in cnxOpts: + timeout_compat = True + else: + cnxOpts['timeout'] = timeout + # no need to close connection + cnx = cnxClass(host, port, **cnxOpts) + self.connection = (key, cnx) + if timeout_compat: + # in python < 2.6 httplib does not support the timeout option + # but socket supports it since 2.3 + cnx.connect() + cnx.sock.settimeout(timeout) + + def close_connection(self): + if self.connection: + self.connection[1].close() + self.connection = None class Response(object): - def __init__(self, session): + def __init__(self, session, response): self.session = session + self.response = response def iter_content(self, blocksize=8192): - pass + # should we check this in Session.post()? + # should we even check this here? + if self.response.status != 200: + if (self.response.getheader("content-length", 0)): + self.response.read() + # XXX wrong exception + raise xmlrpclib.ProtocolError(self._host + handler, + response.status, response.reason, response.msg) + while True: + chunk = response.read(blocksize) + if not chunk: + break + yield chunk + + + From 45dd3bf1878f4dddf03deb3a5cc6a43fa43c729c Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 22 2016 21:41:31 +0000 Subject: [PATCH 10/45] working? --- diff --git a/koji/compatrequests.py b/koji/compatrequests.py index a8d066d..c9fe989 100644 --- a/koji/compatrequests.py +++ b/koji/compatrequests.py @@ -7,6 +7,16 @@ module that is based on the older codepaths in koji. It only provides the bits that koji needs. """ +import httplib +import urlparse +import urllib +import sys +import ssl +try: + from ssl import ssl as pyssl +except ImportError: + pass + class Session(object): @@ -15,9 +25,10 @@ class Session(object): def post(self, url, data=None, headers=None, stream=None, verify=None, cert=None, timeout=None): - cnx = self.get_connection(url, cert, timeout) - # TODO get handler from url - cnx.putrequest('POST', handler) + uri = urlparse.urlsplit(url) + path = uri[2] + cnx = self.get_connection(uri, cert, timeout) + cnx.putrequest('POST', path) #XXX if headers: for k in headers: cnx.putheader(k, headers[k]) @@ -27,18 +38,16 @@ class Session(object): response = cnx.getresponse() return Response(self, response) - def get_connection(self, url, cert, timeout): - key = (url, cert, timeout) + def get_connection(self, uri, cert, timeout): + scheme = uri[0] + host, port = urllib.splitport(uri[1]) + key = (scheme, host, cert, timeout) if self.connection and self.opts.get('keepalive'): if key == self.connection[0]: cnx = self._connection[1] if getattr(cnx, 'sock', None): return cnx # Otherwise we make a new one - uri = urlparse.urlsplit(url) - scheme = uri[0] - host, port = urllib.splitport(uri[1]) - path = uri[2] default_port = 80 if cert: ctx = ssl.SSLCommon.CreateSSLContext(cert) #XXX @@ -73,6 +82,7 @@ class Session(object): # but socket supports it since 2.3 cnx.connect() cnx.sock.settimeout(timeout) + return cnx def close_connection(self): if self.connection: @@ -93,10 +103,9 @@ class Response(object): if (self.response.getheader("content-length", 0)): self.response.read() # XXX wrong exception - raise xmlrpclib.ProtocolError(self._host + handler, - response.status, response.reason, response.msg) + raise Exception("Server status: %s" % self.response.status) while True: - chunk = response.read(blocksize) + chunk = self.response.read(blocksize) if not chunk: break yield chunk From 6bbf9afdc88b1630a7aca3a091b021f652c11501 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 22 2016 21:41:31 +0000 Subject: [PATCH 11/45] don't assume content type is given --- diff --git a/hub/kojixmlrpc.py b/hub/kojixmlrpc.py index f3057a1..295a197 100644 --- a/hub/kojixmlrpc.py +++ b/hub/kojixmlrpc.py @@ -750,7 +750,7 @@ def application(environ, start_response): except Exception: return offline_reply(start_response, msg="database outage") h = ModXMLRPCRequestHandler(registry) - if environ['CONTENT_TYPE'] == 'application/octet-stream': + if environ.get('CONTENT_TYPE') == 'application/octet-stream': response = h._wrap_handler(h.handle_upload, environ) else: response = h._wrap_handler(h.handle_rpc, environ) From 6b02479210ae0348909711732914cff2c8b86cb7 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 22 2016 21:41:31 +0000 Subject: [PATCH 12/45] compat mode works anonymously --- diff --git a/koji/__init__.py b/koji/__init__.py index 8317a60..ed93437 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -1878,7 +1878,11 @@ class ClientSession(object): self.multicall = False self._calls = [] self.logger = logging.getLogger('koji') - self.rsession = requests.Session() + if opts.get('use_old_ssl', True): + import koji.compatrequests + self.rsession = koji.compatrequests.Session() + else: + self.rsession = requests.Session() self.opts.setdefault('timeout', 60 * 60 * 12) def setSession(self, sinfo): diff --git a/koji/compatrequests.py b/koji/compatrequests.py index c9fe989..1a5a500 100644 --- a/koji/compatrequests.py +++ b/koji/compatrequests.py @@ -42,9 +42,10 @@ class Session(object): scheme = uri[0] host, port = urllib.splitport(uri[1]) key = (scheme, host, cert, timeout) - if self.connection and self.opts.get('keepalive'): + #if self.connection and self.opts.get('keepalive'): + if self.connection: # XXX honor keepalive if key == self.connection[0]: - cnx = self._connection[1] + cnx = self.connection[1] if getattr(cnx, 'sock', None): return cnx # Otherwise we make a new one @@ -84,7 +85,7 @@ class Session(object): cnx.sock.settimeout(timeout) return cnx - def close_connection(self): + def close(self): if self.connection: self.connection[1].close() self.connection = None @@ -110,5 +111,8 @@ class Response(object): break yield chunk + def close(self): + self.response.close() + From 33da7cf84d35b9f1701ca44ddeffda424881d47d Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 22 2016 21:41:31 +0000 Subject: [PATCH 13/45] trying to fix auth --- diff --git a/koji/compatrequests.py b/koji/compatrequests.py index 1a5a500..b0859db 100644 --- a/koji/compatrequests.py +++ b/koji/compatrequests.py @@ -11,7 +11,7 @@ import httplib import urlparse import urllib import sys -import ssl +import ssl.SSLCommon try: from ssl import ssl as pyssl except ImportError: @@ -27,7 +27,7 @@ class Session(object): cert=None, timeout=None): uri = urlparse.urlsplit(url) path = uri[2] - cnx = self.get_connection(uri, cert, timeout) + cnx = self.get_connection(uri, cert, verify, timeout) cnx.putrequest('POST', path) #XXX if headers: for k in headers: @@ -38,10 +38,10 @@ class Session(object): response = cnx.getresponse() return Response(self, response) - def get_connection(self, uri, cert, timeout): + def get_connection(self, uri, cert, verify, timeout): scheme = uri[0] host, port = urllib.splitport(uri[1]) - key = (scheme, host, cert, timeout) + key = (scheme, host, cert, verify, timeout) #if self.connection and self.opts.get('keepalive'): if self.connection: # XXX honor keepalive if key == self.connection[0]: @@ -50,17 +50,22 @@ class Session(object): return cnx # Otherwise we make a new one default_port = 80 + certs = {} + if isinstance(verify, basestring): + certs['peer_ca_cert'] = verify if cert: - ctx = ssl.SSLCommon.CreateSSLContext(cert) #XXX + certs['key_and_cert'] = cert + ctx = ssl.SSLCommon.CreateSSLContext(certs) cnxOpts = {'ssl_context' : ctx} cnxClass = ssl.SSLCommon.PlgHTTPSConnection default_port = 443 elif scheme == 'https': cnxOpts = {} - if sys.version_info[:3] >= (2, 7, 9): + if sys.version_info[:3] >= (2, 7, 9) and not verify: ctx = pyssl._create_unverified_context() # TODO - we should default to verifying where possible cnxOpts['context'] = ctx + # TODO honor verify cnxClass = httplib.HTTPSConnection default_port = 443 elif scheme == 'http': From 17e79df68b45e0a33dcc9e596beb18f4e33e1097 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 22 2016 21:41:31 +0000 Subject: [PATCH 14/45] use correct port --- diff --git a/koji/compatrequests.py b/koji/compatrequests.py index b0859db..766b79c 100644 --- a/koji/compatrequests.py +++ b/koji/compatrequests.py @@ -81,6 +81,7 @@ class Session(object): else: cnxOpts['timeout'] = timeout # no need to close connection + port = (port and int(port) or default_port) cnx = cnxClass(host, port, **cnxOpts) self.connection = (key, cnx) if timeout_compat: From c3556dee43c697bc25f26398c609702ff87905a1 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 22 2016 21:41:31 +0000 Subject: [PATCH 15/45] add config for use_old_ssl. make new session on errors --- diff --git a/cli/koji b/cli/koji index 67900b7..f7a6387 100755 --- a/cli/koji +++ b/cli/koji @@ -7175,7 +7175,7 @@ if __name__ == "__main__": for k in ('user', 'password', 'krbservice', 'debug_xmlrpc', 'debug', 'max_retries', 'retry_interval', 'offline_retry', 'offline_retry_interval', 'anon_retry', 'keepalive', 'timeout', 'use_fast_upload', - 'upload_blocksize', 'krb_rdns'): + 'upload_blocksize', 'krb_rdns', 'use_old_ssl'): value = getattr(options,k) if value is not None: session_opts[k] = value diff --git a/koji/__init__.py b/koji/__init__.py index ed93437..6f5cd04 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -1563,6 +1563,7 @@ def read_config(profile_name, user_config=None): 'anon_retry' : None, 'offline_retry' : None, 'offline_retry_interval' : None, + 'use_old_ssl' : False, 'keepalive' : True, 'timeout' : None, 'use_fast_upload': False, @@ -1624,7 +1625,8 @@ def read_config(profile_name, user_config=None): #options *can* be set via the config file. Such options should #not have a default value set in the option parser. if result.has_key(name): - if name in ('anon_retry', 'offline_retry', 'keepalive', 'use_fast_upload', 'krb_rdns'): + if name in ('anon_retry', 'offline_retry', 'keepalive', + 'use_fast_upload', 'krb_rdns', 'use_old_ssl'): result[name] = config.getboolean(profile_name, name) elif name in ('max_retries', 'retry_interval', 'offline_retry_interval', 'poll_interval', 'timeout', @@ -1872,18 +1874,23 @@ class ClientSession(object): opts = opts.copy() self.baseurl = baseurl self.opts = opts - self._connection = None self.authtype = None self.setSession(sinfo) self.multicall = False self._calls = [] self.logger = logging.getLogger('koji') - if opts.get('use_old_ssl', True): + self.rsession = None + self.new_session() + self.opts.setdefault('timeout', 60 * 60 * 12) + + def new_session(self): + if self.rsession: + self.rsession.close() + if self.opts.get('use_old_ssl', False): import koji.compatrequests self.rsession = koji.compatrequests.Session() else: self.rsession = requests.Session() - self.opts.setdefault('timeout', 60 * 60 * 12) def setSession(self, sinfo): """Set the session info @@ -2092,11 +2099,11 @@ class ClientSession(object): try: return self._sendOneCall(handler, headers, request) except socket.error, e: - self._close_connection() + self.new_session() if i or getattr(e, 'errno', None) not in (errno.ECONNRESET, errno.ECONNABORTED, errno.EPIPE): raise except httplib.BadStatusLine: - self._close_connection() + self.new_session() if i: raise @@ -2135,11 +2142,6 @@ class ClientSession(object): r.close() return ret - def _close_connection(self): - if self._connection: - self._connection[1].close() - self._connection = None - def _read_xmlrpc_response(self, response): p, u = xmlrpclib.getparser() for chunk in response.iter_content(8192): @@ -2196,7 +2198,7 @@ class ClientSession(object): raise except Exception, e: tb_str = ''.join(traceback.format_exception(*sys.exc_info())) - self._close_connection() + self.new_session() if is_cert_error(e): # There's no point in retrying for this From b13994082a83508db38e9d79f918b36f1db064d0 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 22 2016 21:41:31 +0000 Subject: [PATCH 16/45] use compatrequests if requests no present --- diff --git a/koji/__init__.py b/koji/__init__.py index 6f5cd04..2fb8991 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -45,15 +45,14 @@ import os.path import pwd import random import re -import requests +try: + import requests +except ImportError: #pragma: no cover + requests = None import rpm import shutil import signal import socket -try: - from ssl import ssl as pyssl -except ImportError: # pragma: no cover - pass import struct import tempfile import time @@ -1886,7 +1885,7 @@ class ClientSession(object): def new_session(self): if self.rsession: self.rsession.close() - if self.opts.get('use_old_ssl', False): + if self.opts.get('use_old_ssl', False) or requests is None: import koji.compatrequests self.rsession = koji.compatrequests.Session() else: diff --git a/koji/compatrequests.py b/koji/compatrequests.py index 766b79c..2f4c91e 100644 --- a/koji/compatrequests.py +++ b/koji/compatrequests.py @@ -28,6 +28,7 @@ class Session(object): uri = urlparse.urlsplit(url) path = uri[2] cnx = self.get_connection(uri, cert, verify, timeout) + #cnx.set_debuglevel(1) cnx.putrequest('POST', path) #XXX if headers: for k in headers: From f8c58e9f64b22ffa5a2663703b88a305d5b44f56 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 22 2016 21:41:31 +0000 Subject: [PATCH 17/45] fix post handler --- diff --git a/koji/compatrequests.py b/koji/compatrequests.py index 2f4c91e..5feeb55 100644 --- a/koji/compatrequests.py +++ b/koji/compatrequests.py @@ -26,10 +26,13 @@ class Session(object): def post(self, url, data=None, headers=None, stream=None, verify=None, cert=None, timeout=None): uri = urlparse.urlsplit(url) - path = uri[2] + if uri[3]: + handler = "%s?%s" % (uri[2], uri[3]) + else: + handler = uri[2] cnx = self.get_connection(uri, cert, verify, timeout) #cnx.set_debuglevel(1) - cnx.putrequest('POST', path) #XXX + cnx.putrequest('POST', handler) if headers: for k in headers: cnx.putheader(k, headers[k]) From be7c97eacd4916a21c41bb6caebb84c061a07097 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 22 2016 21:41:31 +0000 Subject: [PATCH 18/45] move cert date directly into client session opts --- diff --git a/koji/__init__.py b/koji/__init__.py index 2fb8991..1885694 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -2005,10 +2005,13 @@ class ClientSession(object): return '%s/%s@%s' % (service, servername, realm) - def ssl_login(self, cert, ca, serverca, proxyuser=None): - certs = {} - certs['key_and_cert'] = cert - certs['peer_ca_cert'] = serverca + def ssl_login(self, cert=None, ca=None, serverca=None, proxyuser=None): + cert = cert or self.opts.get('cert') + serverca = serverca or self.opts.get('serverca') + if cert is None: + raise AuthError('No certification provided') + if serverca is None: + raise AuthError('No server CA provided') # FIXME: ca is not useful here and therefore ignored, can be removed # when API is changed @@ -2016,7 +2019,8 @@ class ClientSession(object): old_opts = self.opts self.opts = old_opts.copy() self.opts['timeout'] = 60 - self.opts['certs'] = certs + self.opts['cert'] = cert + self.opts['serverca'] = serverca try: sinfo = self.callMethod('sslLogin', proxyuser) finally: @@ -2024,7 +2028,8 @@ class ClientSession(object): if not sinfo: raise AuthError, 'unable to obtain a session' - self.opts['certs'] = certs + self.opts['cert'] = cert + self.opts['serverca'] = serverca self.setSession(sinfo) self.authtype = AUTHTYPE_SSL @@ -2114,13 +2119,13 @@ class ClientSession(object): 'data': request, 'stream': True, } - verify = self.opts.get('certs', {}).get('peer_ca_cert') + verify = self.opts.get('serverca') if verify: callopts['verify'] = verify else: callopts['verify'] = False # XXX - not great, but this is the previous behavior - cert = self.opts.get('certs', {}).get('key_and_cert') + cert = self.opts.get('cert') if cert: # TODO: we really only need to do this for ssllogin calls callopts['cert'] = cert From d85fc2bed0059330daff553d195f21f36d3c6be9 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 22 2016 21:41:31 +0000 Subject: [PATCH 19/45] always pass serverca --- diff --git a/cli/koji b/cli/koji index f7a6387..5fc6d60 100755 --- a/cli/koji +++ b/cli/koji @@ -7175,7 +7175,7 @@ if __name__ == "__main__": for k in ('user', 'password', 'krbservice', 'debug_xmlrpc', 'debug', 'max_retries', 'retry_interval', 'offline_retry', 'offline_retry_interval', 'anon_retry', 'keepalive', 'timeout', 'use_fast_upload', - 'upload_blocksize', 'krb_rdns', 'use_old_ssl'): + 'upload_blocksize', 'krb_rdns', 'use_old_ssl', 'serverca'): value = getattr(options,k) if value is not None: session_opts[k] = value From 2ac8d7a7010eed3618d515908d86098f5a2aca9c Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 22 2016 21:41:31 +0000 Subject: [PATCH 20/45] only use default certs if they exist --- diff --git a/koji/__init__.py b/koji/__init__.py index 1885694..3dce6bf 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -1570,9 +1570,9 @@ def read_config(profile_name, user_config=None): 'poll_interval': 5, 'krbservice': 'host', 'krb_rdns': True, - 'cert': '~/.koji/client.crt', + 'cert': None, 'ca': '', # FIXME: remove in next major release - 'serverca': '~/.koji/serverca.crt', + 'serverca': None, 'authtype': None } @@ -1642,6 +1642,19 @@ def read_config(profile_name, user_config=None): sys.stderr.write("Warning: no configuration for profile name: %s\n" % profile_name) sys.stderr.flush() + # special handling for cert defaults + cert_defaults = { + 'cert': '~/.koji/client.crt', + 'serverca': '~/.koji/serverca.crt', + } + for name in cert_defaults: + if result.get(name) is None: + fn = cert_defaults[name] + if os.path.exists(fn): + result[name] = fn + else: + result[name] = '' + return result From c1e91486bd1710c553e25497c4b5c1acf2ae2dfb Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 22 2016 21:41:31 +0000 Subject: [PATCH 21/45] no_ssl_verify session opt --- diff --git a/koji/__init__.py b/koji/__init__.py index 3dce6bf..ba492bb 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -2135,7 +2135,7 @@ class ClientSession(object): verify = self.opts.get('serverca') if verify: callopts['verify'] = verify - else: + elif self.opts.get('no_ssl_verify'): callopts['verify'] = False # XXX - not great, but this is the previous behavior cert = self.opts.get('cert') From 4e872d67b762827bec7a8f29807ede0ae163628a Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 22 2016 21:41:31 +0000 Subject: [PATCH 22/45] suppress warnings in requests --- diff --git a/koji/__init__.py b/koji/__init__.py index ba492bb..1aa4c53 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -61,6 +61,7 @@ import urllib import urllib2 import urlparse import util +import warnings import xmlrpclib import xml.sax import xml.sax.handler @@ -2152,12 +2153,14 @@ class ClientSession(object): if _key == 'data' and len(_val) > 1024: _val = _val[:1024] + '...' print "%s: %r" % (_key, _val) - r = self.rsession.post(handler, **callopts) - try: - ret = self._read_xmlrpc_response(r) - finally: - r.close() - return ret + with warnings.catch_warnings(): + warnings.simplefilter("ignore") + r = self.rsession.post(handler, **callopts) + try: + ret = self._read_xmlrpc_response(r) + finally: + r.close() + return ret def _read_xmlrpc_response(self, response): p, u = xmlrpclib.getparser() From 1caa748a6db8ad8284c953303453039c8b3eadb7 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 22 2016 21:41:31 +0000 Subject: [PATCH 23/45] avoid with statement for now --- diff --git a/koji/__init__.py b/koji/__init__.py index 1aa4c53..cddd1f5 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -2153,14 +2153,23 @@ class ClientSession(object): if _key == 'data' and len(_val) > 1024: _val = _val[:1024] + '...' print "%s: %r" % (_key, _val) - with warnings.catch_warnings(): - warnings.simplefilter("ignore") + catcher = None + if hasattr(warnings, 'catch_warnings'): + # TODO: convert to a with statement when we drop 2.4.3 support + catcher = warnings.catch_warnings() + catcher.__enter__() + try: + if catcher: + warnings.simplefilter("ignore") r = self.rsession.post(handler, **callopts) try: ret = self._read_xmlrpc_response(r) finally: r.close() - return ret + finally: + if catcher: + catcher.__exit__() + return ret def _read_xmlrpc_response(self, response): p, u = xmlrpclib.getparser() From a6440820a615eda53045da2259db577a6eadf9fa Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 22 2016 21:41:31 +0000 Subject: [PATCH 24/45] honor verify in compatrequests --- diff --git a/koji/compatrequests.py b/koji/compatrequests.py index 5feeb55..9123dd0 100644 --- a/koji/compatrequests.py +++ b/koji/compatrequests.py @@ -65,11 +65,25 @@ class Session(object): default_port = 443 elif scheme == 'https': cnxOpts = {} - if sys.version_info[:3] >= (2, 7, 9) and not verify: + if verify: + if sys.version_info[:3] >= (2, 7, 9): + try: + proto = pyssl.PROTOCOL_TLS + except AttributeError: + proto = pyssl.PROTOCOL_SSLv23 + ctx = pyssl.SSLContext(proto) + ctx.load_verify_locations(cafile=verify) + ctx.verify_mode = pyssl.CERT_REQUIRED + cnxOpts['context'] = ctx + else: + cnxOpts['cert_file'] = verify + elif verify is None: + # not specified, leave as default + pass + elif sys.version_info[:3] >= (2, 7, 9): + # no verify ctx = pyssl._create_unverified_context() - # TODO - we should default to verifying where possible cnxOpts['context'] = ctx - # TODO honor verify cnxClass = httplib.HTTPSConnection default_port = 443 elif scheme == 'http': From 0a5650cf285ab23aa01920203f8c8c8963d9f507 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 22 2016 21:41:31 +0000 Subject: [PATCH 25/45] update connection error retry code see also: https://github.com/mikem23/keepalive-race --- diff --git a/koji/__init__.py b/koji/__init__.py index cddd1f5..d77353a 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -1864,6 +1864,35 @@ def is_cert_error(e): return False +def is_conn_error(e): + """Determine if an error seems to be from a dropped connection""" + if isinstance(e, socket.error): + if getattr(e, 'errno', None) in (errno.ECONNRESET, errno.ECONNABORTED, errno.EPIPE): + return True + # else + return False + if isinstance(e, httplib.BadStatusLine): + return True + if requests is not None: + try: + if isinstance(e, requests.exceptions.ConnectionError): + # we see errors like this in keep alive timeout races + # ConnectionError(ProtocolError('Connection aborted.', BadStatusLine("''",)),) + e2 = getattr(e, 'args', [None])[0] + if isinstance(e2, requests.packages.urllib3.exceptions.ProtocolError): + e3 = getattr(e2, 'args', [None, None])[1] + if isinstance(e3, httplib.BadStatusLine): + return True + if isinstance(e2, socket.error): + # same check as unwrapped socket error + if getattr(e, 'errno', None) in (errno.ECONNRESET, errno.ECONNABORTED, errno.EPIPE): + return True + except (TypeError, AttributeError): + pass + # otherwise + return False + + class VirtualMethod(object): # some magic to bind an XML-RPC method to an RPC server. # supports "nested" methods (e.g. examples.getStateName) @@ -1897,6 +1926,7 @@ class ClientSession(object): self.opts.setdefault('timeout', 60 * 60 * 12) def new_session(self): + self.logger.debug("Opening new requests session") if self.rsession: self.rsession.close() if self.opts.get('use_old_ssl', False) or requests is None: @@ -2116,15 +2146,11 @@ class ClientSession(object): for i in (0, 1): try: return self._sendOneCall(handler, headers, request) - except socket.error, e: - self.new_session() - if i or getattr(e, 'errno', None) not in (errno.ECONNRESET, errno.ECONNABORTED, errno.EPIPE): + except Exception, e: + if i or not is_conn_error(e): raise - except httplib.BadStatusLine: + self.logger.debug("Connection Error: %s", e) self.new_session() - if i: - raise - def _sendOneCall(self, handler, headers, request): headers = dict(headers) From 7b3332703006501bed85736313c2528aee048480 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 22 2016 21:41:31 +0000 Subject: [PATCH 26/45] avoid tickling race condition with common keep alive timeout setting The default KeepAliveTimeout in Apache httpd 2.4 is 5 seconds. Sleeping for that same time and then resuming calls can tickle a race condition. The previous commit /should/ catch that now, but there's no sense hitting that error so often. See also: https://github.com/mikem23/keepalive-race --- diff --git a/cli/koji b/cli/koji index 5fc6d60..405c4f0 100755 --- a/cli/koji +++ b/cli/koji @@ -4360,7 +4360,7 @@ def anon_handle_list_history(options, session, args): if not options.watch: break else: - time.sleep(5) + time.sleep(options.poll_interval) # repeat query for later events if last_event: kwargs['afterEvent'] = last_event diff --git a/koji/__init__.py b/koji/__init__.py index d77353a..df0b686 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -1568,7 +1568,7 @@ def read_config(profile_name, user_config=None): 'timeout' : None, 'use_fast_upload': False, 'upload_blocksize': 1048576, - 'poll_interval': 5, + 'poll_interval': 6, 'krbservice': 'host', 'krb_rdns': True, 'cert': None, From 33e1bdee97e9f677349303cf25d5904f644abb81 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 22 2016 21:41:31 +0000 Subject: [PATCH 27/45] force https when using ssl auth --- diff --git a/koji/__init__.py b/koji/__init__.py index df0b686..8286b3d 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -2059,6 +2059,11 @@ class ClientSession(object): # FIXME: ca is not useful here and therefore ignored, can be removed # when API is changed + # force https + uri = urlparse.urlsplit(self.baseurl) + if uri[0] != 'https': + self.baseurl = 'https://%s%s' % (uri[1], uri[2]) + # 60 second timeout during login old_opts = self.opts self.opts = old_opts.copy() From 3d16485c6fb92aeaa64b14a86e08e3ab661906f7 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 22 2016 21:41:31 +0000 Subject: [PATCH 28/45] grab_session_options function --- diff --git a/cli/koji b/cli/koji index 405c4f0..f093c50 100755 --- a/cli/koji +++ b/cli/koji @@ -7171,15 +7171,8 @@ if __name__ == "__main__": else: logger.setLevel(logging.WARN) - session_opts = {} - for k in ('user', 'password', 'krbservice', 'debug_xmlrpc', 'debug', 'max_retries', - 'retry_interval', 'offline_retry', 'offline_retry_interval', - 'anon_retry', 'keepalive', 'timeout', 'use_fast_upload', - 'upload_blocksize', 'krb_rdns', 'use_old_ssl', 'serverca'): - value = getattr(options,k) - if value is not None: - session_opts[k] = value - session = koji.ClientSession(options.server,session_opts) + session_opts = koji.grab_session_options(options) + session = koji.ClientSession(options.server, session_opts) rv = 0 try: rv = locals()[command].__call__(options, session, args) diff --git a/koji/__init__.py b/koji/__init__.py index 8286b3d..9aa05cb 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -1906,6 +1906,38 @@ class VirtualMethod(object): return self.__func(self.__name, args, opts) +def grab_session_options(options): + '''Convert optparse options to a dict that ClientSession can handle''' + s_opts = ( + 'user', + 'password', + 'krbservice', + 'debug_xmlrpc', + 'debug', + 'max_retries', + 'retry_interval', + 'offline_retry', + 'offline_retry_interval', + 'anon_retry', + 'keepalive', + 'timeout', + 'use_fast_upload', + 'upload_blocksize', + 'krb_rdns', + 'use_old_ssl', + 'no_ssl_verify', + 'serverca', + ) + # cert is omitted for now + ret = {} + for key in s_opts: + if not hasattr(options, key): + continue + value = getattr(options, key) + if value is not None: + ret[key] = value + + class ClientSession(object): def __init__(self, baseurl, opts=None, sinfo=None): @@ -1925,6 +1957,7 @@ class ClientSession(object): self.new_session() self.opts.setdefault('timeout', 60 * 60 * 12) + def new_session(self): self.logger.debug("Opening new requests session") if self.rsession: From de5fe4711123255ad63dd2b661e01c7304ce7f18 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 22 2016 21:41:31 +0000 Subject: [PATCH 29/45] koji-gc: remove --network-hack, use grab_session_options --- diff --git a/util/koji-gc b/util/koji-gc index 97232d1..8c30432 100755 --- a/util/koji-gc +++ b/util/koji-gc @@ -36,35 +36,6 @@ if optparse.__version__ == "1.4.1+": sys.exit(2) OptionParser.error = _op_error -class MySession(koji.ClientSession): - """This is a hack to work around server timeouts""" - - def _callMethod(self, name, args, kwargs): - retries = 10 - i = 0 - while True: - i += 1 - try: - return super(MySession, self)._callMethod(name, args, kwargs) - except (socket.timeout, socket.error, xmlrpclib.ProtocolError), e: - if i > retries: - raise - else: - print "Socket Error: %s [%i], retrying..." % (e, i) - time.sleep(60) - - #an even worse hack - def multiCall(self): - if not self.multicall: - raise GenericError, 'ClientSession.multicall must be set to True before calling multiCall()' - if len(self._calls) == 0: - return [] - - self.multicall = False - calls = self._calls - self._calls = [] - return self._callMethod('multiCall', (calls,), {}) - def _(args): """Stub function for translation""" @@ -88,7 +59,7 @@ def get_options(): parser.add_option("--noauth", action="store_true", default=False, help=_("do not authenticate")) parser.add_option("--network-hack", action="store_true", default=False, - help=_("enable hackish workaround for broken networks")) + help=optparse.SUPPRESS_HELP) # no longer used parser.add_option("--cert", default='/etc/koji-gc/client.crt', help=_("Client SSL certificate file for authentication")) parser.add_option("--ca", default='', @@ -926,15 +897,8 @@ if __name__ == "__main__": options, args = get_options() - session_opts = {} - for k in ('user', 'password', 'krbservice', 'krb_rdns', 'email_domain', - 'debug_xmlrpc', 'debug'): - session_opts[k] = getattr(options,k) - if options.network_hack: - socket.setdefaulttimeout(180) - session = MySession(options.server, session_opts) - else: - session = koji.ClientSession(options.server, session_opts) + session_opts = koji.grab_session_options(options) + session = koji.ClientSession(options.server, session_opts) rv = 0 try: if not options.skip_main: From ef790857adb3e7131af46c1012f7e922b34493bc Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 22 2016 21:41:31 +0000 Subject: [PATCH 30/45] fix return --- diff --git a/koji/__init__.py b/koji/__init__.py index 9aa05cb..ec6e6f3 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -1936,6 +1936,7 @@ def grab_session_options(options): value = getattr(options, key) if value is not None: ret[key] = value + return ret class ClientSession(object): From 89ca42f2245cf95aae68ed67cf91b56a98ac039a Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 22 2016 21:41:31 +0000 Subject: [PATCH 31/45] use grab_session_options everywhere --- diff --git a/builder/kojid b/builder/kojid index b5ff393..dc9d635 100755 --- a/builder/kojid +++ b/builder/kojid @@ -5074,16 +5074,8 @@ if __name__ == "__main__": if options.admin_emails: koji.add_mail_logger("koji", options.admin_emails) - #build session options - session_opts = {} - for k in ('user', 'password', 'krbservice', 'krb_rdns', 'debug_xmlrpc', 'debug', - 'retry_interval', 'max_retries', 'offline_retry', 'offline_retry_interval', - 'keepalive', 'timeout', 'use_fast_upload', - ): - v = getattr(options, k, None) - if v is not None: - session_opts[k] = v #start a session and login + session_opts = koji.grab_session_options(options) session = koji.ClientSession(options.server, session_opts) if os.path.isfile(options.cert): try: diff --git a/util/koji-shadow b/util/koji-shadow index a0f7cf4..bd5f967 100755 --- a/util/koji-shadow +++ b/util/koji-shadow @@ -1326,9 +1326,7 @@ if __name__ == "__main__": options, args = get_options() - session_opts = {} - for k in ('user', 'password', 'krbservice', 'debug_xmlrpc', 'debug'): - session_opts[k] = getattr(options, k) + session_opts = koji.grab_session_options(options) session = koji.ClientSession(options.server, session_opts) if not options.noauth: activate_session(session) diff --git a/util/kojira b/util/kojira index 9f776fb..470661b 100755 --- a/util/kojira +++ b/util/kojira @@ -791,10 +791,8 @@ if __name__ == "__main__": logger.setLevel(logging.ERROR) else: logger.setLevel(logging.WARNING) - session_opts = {} - for k in ('user', 'password', 'krbservice', 'krb_rdns', 'debug_xmlrpc', 'debug', - 'retry_interval', 'max_retries', 'offline_retry', 'offline_retry_interval'): - session_opts[k] = getattr(options,k) + + session_opts = koji.grab_session_options(options) session = koji.ClientSession(options.server,session_opts) if os.path.isfile(options.cert): # authenticate using SSL client certificates diff --git a/vm/kojivmd b/vm/kojivmd index 3cce429..373e34f 100755 --- a/vm/kojivmd +++ b/vm/kojivmd @@ -1055,14 +1055,8 @@ if __name__ == "__main__": if options.admin_emails: koji.add_mail_logger("koji", options.admin_emails) - #build session options - session_opts = {} - for k in ('user', 'password', 'krbservice', 'krb_rdns', 'debug_xmlrpc', 'debug', - 'retry_interval', 'max_retries', 'offline_retry', 'offline_retry_interval'): - v = getattr(options, k, None) - if v is not None: - session_opts[k] = v #start a session and login + session_opts = koji.grab_session_options(options) session = koji.ClientSession(options.server, session_opts) if os.path.isfile(options.cert): try: From 97264e690cd14a2c314eaaa90cb8e857f10e87eb Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 22 2016 21:41:31 +0000 Subject: [PATCH 32/45] let koji-gc config set more session options --- diff --git a/util/koji-gc b/util/koji-gc index 8c30432..b43f8aa 100755 --- a/util/koji-gc +++ b/util/koji-gc @@ -149,6 +149,9 @@ def get_options(): ['unprotected_keys', None, 'string'], ['grace_period', None, 'string'], ['trashcan_tag', None, 'string'], + ['use_old_ssl', None, 'boolean'], + ['no_ssl_verify', None, 'boolean'], + ['timeout', None, 'integer'], ] for name, alias, type in cfgmap: if alias is None: From 921e6303fb36b433327cf347b0261f1f693f2d54 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 22 2016 21:41:31 +0000 Subject: [PATCH 33/45] allow setting more session options in kojira --- diff --git a/util/kojira b/util/kojira index 470661b..905fec4 100755 --- a/util/kojira +++ b/util/kojira @@ -719,6 +719,8 @@ def get_options(): 'max_retries': 120, 'offline_retry': True, 'offline_retry_interval': 120, + 'use_old_ssl': False, + 'no_ssl_verify': False, 'max_delete_processes': 4, 'max_repo_tasks' : 4, 'max_repo_tasks_maven' : 2, @@ -737,7 +739,8 @@ def get_options(): 'max_delete_processes', 'max_repo_tasks_maven', 'delete_batch_size', ) str_opts = ('topdir', 'server', 'user', 'password', 'logfile', 'principal', 'keytab', 'krbservice', 'cert', 'ca', 'serverca', 'debuginfo_tags', 'source_tags') # FIXME: remove ca here - bool_opts = ('with_src','verbose','debug','ignore_stray_repos', 'offline_retry', 'krb_rdns') + bool_opts = ('with_src','verbose','debug','ignore_stray_repos', 'offline_retry', + 'krb_rdns', 'use_old_ssl', 'no_ssl_verify') for name in config.options(section): if name in int_opts: defaults[name] = config.getint(section, name) From 7582a08d5e7c40b347a1664e45da8a3555c18dc5 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 22 2016 21:41:31 +0000 Subject: [PATCH 34/45] allow setting more session options in kojid config --- diff --git a/builder/kojid b/builder/kojid index dc9d635..2858f05 100755 --- a/builder/kojid +++ b/builder/kojid @@ -4974,6 +4974,8 @@ def get_options(): 'offline_retry_interval': 120, 'keepalive' : True, 'timeout' : None, + 'use_old_ssl' : False, + 'no_ssl_verify' : False, 'use_fast_upload': True, 'use_createrepo_c': False, 'createrepo_skip_stat': True, @@ -5001,7 +5003,7 @@ def get_options(): elif name in ['offline_retry', 'use_createrepo_c', 'createrepo_skip_stat', 'createrepo_update', 'keepalive', 'use_fast_upload', 'support_rpm_source_layout', 'krb_rdns', - 'build_arch_can_fail']: + 'build_arch_can_fail', 'use_old_ssl', 'no_ssl_verify']: defaults[name] = config.getboolean('kojid', name) elif name in ['plugin', 'plugins']: defaults['plugin'] = value.split() From d81d293ed97b3b28276e40e036840232a353b66b Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 22 2016 21:41:31 +0000 Subject: [PATCH 35/45] allow setting more session options in kojivmd --- diff --git a/vm/kojivmd b/vm/kojivmd index 373e34f..50d48a3 100755 --- a/vm/kojivmd +++ b/vm/kojivmd @@ -125,6 +125,8 @@ def get_options(): 'server': None, 'user': None, 'password': None, + 'use_old_ssl': False, + 'no_ssl_verify': False, 'retry_interval': 60, 'max_retries': 120, 'offline_retry': True, @@ -142,7 +144,8 @@ def get_options(): defaults[name] = int(value) except ValueError: quit("value for %s option must be a valid integer" % name) - elif name in ['offline_retry', 'krb_rdns']: + elif name in ['offline_retry', 'krb_rdns', 'use_old_ssl', + 'no_ssl_verify']: defaults[name] = config.getboolean('kojivmd', name) elif name in ['plugin', 'plugins']: defaults['plugin'] = value.split() From f86ca05c0e4d2ec7591b949fabaa8376a5f84114 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 22 2016 21:41:31 +0000 Subject: [PATCH 36/45] force a new session in ssl_login --- diff --git a/koji/__init__.py b/koji/__init__.py index ec6e6f3..4b66b32 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -2098,6 +2098,9 @@ class ClientSession(object): if uri[0] != 'https': self.baseurl = 'https://%s%s' % (uri[1], uri[2]) + # Force a new session + self.new_session() + # 60 second timeout during login old_opts = self.opts self.opts = old_opts.copy() From 20d67b5c63c5065992b35700cd95d48cd2364093 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 22 2016 21:41:31 +0000 Subject: [PATCH 37/45] ssl hack 2 --- diff --git a/koji/__init__.py b/koji/__init__.py index 4b66b32..8465322 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -38,7 +38,11 @@ import imp import logging import logging.handlers from koji.util import md5_constructor -from OpenSSL.SSL import Error as SSL_Error +try: + from OpenSSL.SSL import Error as SSL_Error +except Exception: + # XXX + pass import optparse import os import os.path From f3a6bc86bc3b93ee1c3fe66ad3a598c05cb3b199 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 22 2016 21:41:31 +0000 Subject: [PATCH 38/45] more smarts for ssl bug workaround --- diff --git a/koji/__init__.py b/koji/__init__.py index 8465322..90b770c 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -38,10 +38,14 @@ import imp import logging import logging.handlers from koji.util import md5_constructor +SSL_Error = None try: from OpenSSL.SSL import Error as SSL_Error -except Exception: - # XXX +except Exception: #pragma: no cover + # the hub imports koji, and sometimes this import fails there + # see: https://cryptography.io/en/latest/faq/#starting-cryptography-using-mod-wsgi-produces-an-internalerror-during-a-call-in-register-osrandom-engine + # unfortunately the workaround at the above link does not always work, so + # we ignore it here pass import optparse import os @@ -1834,6 +1838,9 @@ pathinfo = PathInfo() def is_cert_error(e): """Determine if an OpenSSL error is due to a bad cert""" + if SSL_Error is None: #pragma: no cover + # import failed, so we can't determine + raise Exception("OpenSSL library did not load") if not isinstance(e, SSL_Error): return False From e9a8c60a3ad3aabd419f65d70c62bbb4b93b4eca Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 22 2016 21:41:31 +0000 Subject: [PATCH 39/45] some unit tests for koji.compatrequests --- diff --git a/tests/test_compatrequests.py b/tests/test_compatrequests.py new file mode 100644 index 0000000..1da3b85 --- /dev/null +++ b/tests/test_compatrequests.py @@ -0,0 +1,55 @@ +import mock +import unittest + +import koji.compatrequests + + +class TestResponse(unittest.TestCase): + + def setUp(self): + session = mock.MagicMock() + response = mock.MagicMock() + self.response = koji.compatrequests.Response(session, response) + + def tearDown(self): + del self.response + + def test_read(self): + self.response.response.status = 200 + data = [ + "Here's some data", + "Here's some mooore data", + "And look!", + "Here's a nice block of lorem text", + "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do " + "eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut " + "enim ad minim veniam, quis nostrud exercitation ullamco laboris " + "nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor " + "in reprehenderit in voluptate velit esse cillum dolore eu fugiat " + "nulla pariatur. Excepteur sint occaecat cupidatat non proident, " + "sunt in culpa qui officia deserunt mollit anim id est laborum.", + "", #eof + ] + self.response.response.read.side_effect = data + + result = list(self.response.iter_content(blocksize=10240)) + + self.assertEqual(result, data[:-1]) + rcalls = [mock.call(10240) for s in data] + self.response.response.read.assert_has_calls(rcalls) + + self.response.close() + self.response.response.close.assert_called_once() + + def test_error(self): + self.response.response.status = 404 + self.response.response.getheader.return_value = 0 + with self.assertRaises(Exception): + list(self.response.iter_content()) + self.response.response.read.assert_not_called() + + self.response.response.status = 404 + self.response.response.getheader.return_value = 42 + with self.assertRaises(Exception): + list(self.response.iter_content()) + self.response.response.read.assert_called_once() From 8eb69799ad850127c173df89142bba0315a63025 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 22 2016 21:41:31 +0000 Subject: [PATCH 40/45] more unit tests for compatrequests --- diff --git a/tests/test_compatrequests.py b/tests/test_compatrequests.py index 1da3b85..1b532b1 100644 --- a/tests/test_compatrequests.py +++ b/tests/test_compatrequests.py @@ -1,5 +1,6 @@ import mock import unittest +import urlparse import koji.compatrequests @@ -53,3 +54,81 @@ class TestResponse(unittest.TestCase): with self.assertRaises(Exception): list(self.response.iter_content()) self.response.response.read.assert_called_once() + + +class TestSessionPost(unittest.TestCase): + + def test_simple(self): + session = koji.compatrequests.Session() + url = 'https://www.fakedomain.org/KOJIHUB' + cnx = mock.MagicMock() + session.get_connection = mock.MagicMock() + session.get_connection.return_value = cnx + response = mock.MagicMock() + cnx.getresponse.return_value = response + + ret = session.post(url, data="data", headers={"foo": "bar"}) + cnx.putrequest.assert_called_once_with('POST', '/KOJIHUB') + cnx.putheader.assert_called_once_with('foo', 'bar') + cnx.send.assert_called_once_with("data") + self.assertEqual(ret.response, response) + + def test_less_simple(self): + session = koji.compatrequests.Session() + url = 'https://www.fakedomain.org/KOJIHUB?a=1&b=2' + cnx = mock.MagicMock() + session.get_connection = mock.MagicMock() + session.get_connection.return_value = cnx + response = mock.MagicMock() + cnx.getresponse.return_value = response + + ret = session.post(url, data="data", headers={"foo": "bar"}, + cert="cert", verify="verify", stream=True, timeout=1701) + cnx.putrequest.assert_called_once_with('POST', '/KOJIHUB?a=1&b=2') + cnx.putheader.assert_called_once_with('foo', 'bar') + cnx.send.assert_called_once_with("data") + self.assertEqual(ret.response, response) + + +class TestSessionConnection(unittest.TestCase): + + @mock.patch('httplib.HTTPConnection') + def test_http(self, HTTPConnection): + # no cert, no verify, no timeout + session = koji.compatrequests.Session() + url = 'http://www.fakedomain234234.org/KOJIHUB?a=1&b=2' + uri = urlparse.urlsplit(url) + + cnx = session.get_connection(uri, None, None, None) + HTTPConnection.assert_called_once_with('www.fakedomain234234.org', 80) + key = ('http', 'www.fakedomain234234.org', None, None, None) + self.assertEqual(session.connection, (key, cnx)) + + # and close it + session.close() + self.assertEqual(session.connection, None) + cnx.close.assert_called_with() + + # double close should not error + session.close() + + + def test_cached(self): + session = koji.compatrequests.Session() + url = 'http://www.fakedomain234234.org/KOJIHUB?a=1&b=2' + uri = urlparse.urlsplit(url) + key = ('http', 'www.fakedomain234234.org', None, None, None) + cnx = mock.MagicMock() + session.connection = (key, cnx) + + ret = session.get_connection(uri, None, None, None) + self.assertEqual(ret, cnx) + + def test_badproto(self): + session = koji.compatrequests.Session() + url = 'nosuchproto://www.fakedomain234234.org/KOJIHUB?a=1&b=2' + uri = urlparse.urlsplit(url) + + with self.assertRaises(IOError): + ret = session.get_connection(uri, None, None, None) + From ed986086dc6876571a033ca041427c9c830abd6e Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 22 2016 21:41:31 +0000 Subject: [PATCH 41/45] uncovered import exception --- diff --git a/koji/compatrequests.py b/koji/compatrequests.py index 9123dd0..2855da0 100644 --- a/koji/compatrequests.py +++ b/koji/compatrequests.py @@ -14,7 +14,7 @@ import sys import ssl.SSLCommon try: from ssl import ssl as pyssl -except ImportError: +except ImportError: # pragma: no cover pass From 29a7c9e8162fb95a64f77fb9ebfc9fbca2a2db1d Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 22 2016 21:41:31 +0000 Subject: [PATCH 42/45] still more tests --- diff --git a/tests/test_compatrequests.py b/tests/test_compatrequests.py index 1b532b1..0fd69e1 100644 --- a/tests/test_compatrequests.py +++ b/tests/test_compatrequests.py @@ -112,7 +112,6 @@ class TestSessionConnection(unittest.TestCase): # double close should not error session.close() - def test_cached(self): session = koji.compatrequests.Session() url = 'http://www.fakedomain234234.org/KOJIHUB?a=1&b=2' @@ -132,3 +131,91 @@ class TestSessionConnection(unittest.TestCase): with self.assertRaises(IOError): ret = session.get_connection(uri, None, None, None) + @mock.patch('httplib.HTTPConnection') + @mock.patch('sys.version_info', new=(2,7,12,'final', 0)) + def test_timeout(self, HTTPConnection): + # no cert, no verify + session = koji.compatrequests.Session() + url = 'http://www.fakedomain234234.org/KOJIHUB?a=1&b=2' + uri = urlparse.urlsplit(url) + timeout = 1701 + + cnx = session.get_connection(uri, None, None, 1701) + HTTPConnection.assert_called_once_with('www.fakedomain234234.org', 80, timeout=1701) + key = ('http', 'www.fakedomain234234.org', None, None, 1701) + self.assertEqual(session.connection, (key, cnx)) + + @mock.patch('httplib.HTTPConnection') + @mock.patch('sys.version_info', new=(2, 4, 3, 'final', 0)) + def test_timeout_compat(self, HTTPConnection): + # no cert, no verify + session = koji.compatrequests.Session() + url = 'http://www.fakedomain234234.org/KOJIHUB?a=1&b=2' + uri = urlparse.urlsplit(url) + timeout = 1701 + + cnx = session.get_connection(uri, None, None, 1701) + HTTPConnection.assert_called_once_with('www.fakedomain234234.org', 80) + key = ('http', 'www.fakedomain234234.org', None, None, 1701) + self.assertEqual(session.connection, (key, cnx)) + cnx.connect.assert_called_once() + cnx.sock.settimeout.assert_called_with(1701) + + @mock.patch('httplib.HTTPSConnection') + def test_https(self, HTTPSConnection): + # no cert, no verify, no timeout + session = koji.compatrequests.Session() + url = 'https://www.fakedomain234234.org/KOJIHUB?a=1&b=2' + uri = urlparse.urlsplit(url) + + cnx = session.get_connection(uri, None, None, None) + HTTPSConnection.assert_called_once_with('www.fakedomain234234.org', 443) + key = ('https', 'www.fakedomain234234.org', None, None, None) + self.assertEqual(session.connection, (key, cnx)) + + @mock.patch('koji.ssl.SSLCommon.CreateSSLContext') + @mock.patch('koji.ssl.SSLCommon.PlgHTTPSConnection') + def test_cert(self, PlgHTTPSConnection, CreateSSLContext): + # no verify, no timeout + session = koji.compatrequests.Session() + url = 'https://www.fakedomain234234.org/KOJIHUB?a=1&b=2' + uri = urlparse.urlsplit(url) + cert = '/path/to/cert/file' + context = mock.MagicMock() + CreateSSLContext.return_value = context + + cnx = session.get_connection(uri, cert, None, None) + PlgHTTPSConnection.assert_called_once_with('www.fakedomain234234.org', 443, ssl_context=context) + key = ('https', 'www.fakedomain234234.org', cert, None, None) + self.assertEqual(session.connection, (key, cnx)) + + @mock.patch('ssl._create_unverified_context') + @mock.patch('httplib.HTTPSConnection') + @mock.patch('sys.version_info', new=(2,7,12,'final', 0)) + def test_unverified(self, HTTPSConnection, create_unverified_context): + # no cert, verify=False, no timeout + session = koji.compatrequests.Session() + url = 'https://www.fakedomain234234.org/KOJIHUB?a=1&b=2' + uri = urlparse.urlsplit(url) + context = mock.MagicMock() + create_unverified_context.return_value = context + + cnx = session.get_connection(uri, None, False, None) + create_unverified_context.assert_called_once() + HTTPSConnection.assert_called_once_with('www.fakedomain234234.org', 443, context=context) + key = ('https', 'www.fakedomain234234.org', None, False, None) + self.assertEqual(session.connection, (key, cnx)) + + @mock.patch('httplib.HTTPSConnection') + @mock.patch('sys.version_info', new=(2, 4, 3, 'final', 0)) + def test_unverified_compat(self, HTTPSConnection): + # no cert, verify=False, no timeout + session = koji.compatrequests.Session() + url = 'https://www.fakedomain234234.org/KOJIHUB?a=1&b=2' + uri = urlparse.urlsplit(url) + + cnx = session.get_connection(uri, None, False, None) + HTTPSConnection.assert_called_once_with('www.fakedomain234234.org', 443) + key = ('https', 'www.fakedomain234234.org', None, False, None) + self.assertEqual(session.connection, (key, cnx)) + From d97d568c39ebaac5080b58fbd6e8919d821493e9 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 22 2016 21:41:31 +0000 Subject: [PATCH 43/45] more compatrequests unit tests --- diff --git a/tests/test_compatrequests.py b/tests/test_compatrequests.py index 0fd69e1..f835d79 100644 --- a/tests/test_compatrequests.py +++ b/tests/test_compatrequests.py @@ -219,3 +219,41 @@ class TestSessionConnection(unittest.TestCase): key = ('https', 'www.fakedomain234234.org', None, False, None) self.assertEqual(session.connection, (key, cnx)) + @mock.patch('ssl._create_unverified_context') + @mock.patch('ssl.SSLContext') + @mock.patch('httplib.HTTPSConnection') + @mock.patch('sys.version_info', new=(2,7,12,'final', 0)) + def test_verify(self, HTTPSConnection, SSLContext, create_unverified_context): + # no cert, no timeout + session = koji.compatrequests.Session() + url = 'https://www.fakedomain234234.org/KOJIHUB?a=1&b=2' + uri = urlparse.urlsplit(url) + context = mock.MagicMock() + SSLContext.return_value = context + verify = '/path/to/verify/cert' + + cnx = session.get_connection(uri, None, verify, None) + create_unverified_context.assert_not_called() + SSLContext.assert_called_once() + context.load_verify_locations.called_once_with(cafile=verify) + HTTPSConnection.assert_called_once_with('www.fakedomain234234.org', 443, context=context) + key = ('https', 'www.fakedomain234234.org', None, verify, None) + self.assertEqual(session.connection, (key, cnx)) + + @mock.patch('ssl._create_unverified_context') + @mock.patch('ssl.SSLContext') + @mock.patch('httplib.HTTPSConnection') + @mock.patch('sys.version_info', new=(2, 4, 3, 'final', 0)) + def test_verify_compat(self, HTTPSConnection, SSLContext, create_unverified_context): + # no cert, no timeout + session = koji.compatrequests.Session() + url = 'https://www.fakedomain234234.org/KOJIHUB?a=1&b=2' + uri = urlparse.urlsplit(url) + verify = '/path/to/verify/cert' + + cnx = session.get_connection(uri, None, verify, None) + create_unverified_context.assert_not_called() + SSLContext.assert_not_called() + HTTPSConnection.assert_called_once_with('www.fakedomain234234.org', 443, cert_file=verify) + key = ('https', 'www.fakedomain234234.org', None, verify, None) + self.assertEqual(session.connection, (key, cnx)) From 53d315788de5a02d378611ace406ec241428e8ac Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 22 2016 21:41:31 +0000 Subject: [PATCH 44/45] add test for ClientSession.serverPrincipal --- diff --git a/tests/test_client_session.py b/tests/test_client_session.py new file mode 100644 index 0000000..dc226f2 --- /dev/null +++ b/tests/test_client_session.py @@ -0,0 +1,35 @@ +import mock +import unittest + +import koji + + +class TestClientSession(unittest.TestCase): + + @mock.patch('socket.getfqdn') + def test_server_principal_rdns(self, getfqdn): + opts = {'krb_rdns': True} + session = koji.ClientSession('http://koji.example.com/kojihub', opts) + cprinc = mock.MagicMock() + cprinc.realm = "REALM" + getfqdn.return_value = 'koji02.example.com' + + princ = session._serverPrincipal(cprinc) + self.assertEqual(princ, 'host/koji02.example.com@REALM') + getfqdn.assert_called_with('koji.example.com') + + @mock.patch('socket.getfqdn') + def test_server_principal_no_rdns(self, getfqdn): + opts = {'krb_rdns': False} + session = koji.ClientSession('http://koji.example.com/kojihub', opts) + cprinc = mock.MagicMock() + cprinc.realm = "REALM" + getfqdn.return_value = 'koji02.example.com' + + princ = session._serverPrincipal(cprinc) + self.assertEqual(princ, 'host/koji.example.com@REALM') + getfqdn.assert_not_called() + + + + From feb619f4607039a4c83d320680378dd4e9ecd529 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 22 2016 21:41:31 +0000 Subject: [PATCH 45/45] unit tests for new_session() --- diff --git a/tests/test_client_session.py b/tests/test_client_session.py index dc226f2..3931891 100644 --- a/tests/test_client_session.py +++ b/tests/test_client_session.py @@ -30,6 +30,36 @@ class TestClientSession(unittest.TestCase): self.assertEqual(princ, 'host/koji.example.com@REALM') getfqdn.assert_not_called() + @mock.patch('koji.compatrequests.Session') + @mock.patch('requests.Session') + def test_new_session(self, rsession, compat_session): + opts = {'use_old_ssl': False} + ksession = koji.ClientSession('http://koji.example.com/kojihub', opts) + # init should have called new_session for us + rsession.assert_called_once() + compat_session.assert_not_called() + @mock.patch('koji.compatrequests.Session') + @mock.patch('requests.Session') + def test_new_session_old(self, rsession, compat_session): + opts = {'use_old_ssl': True} + ksession = koji.ClientSession('http://koji.example.com/kojihub', opts) + + # init should have called new_session for us + + rsession.assert_not_called() + compat_session.assert_called_once() + + @mock.patch('koji.compatrequests.Session') + @mock.patch('requests.Session') + def test_new_session_close(self, rsession, compat_session): + opts = {'use_old_ssl': True} + ksession = koji.ClientSession('http://koji.example.com/kojihub', opts) + my_rsession = mock.MagicMock() + ksession.rsession = my_rsession + + ksession.new_session() + my_rsession.close.assert_called() + self.assertNotEqual(ksession.rsession, my_rsession)