From f5f7cbc63f9235ce040687cdf102aeaef69ab422 Mon Sep 17 00:00:00 2001 From: Stanislav Laznicka Date: Thu, 1 Jun 2017 09:00:15 +0200 Subject: [PATCH] rpc: avoid possible recursion in create_connection There was a recursion in RPCClient.create_connection() which under rare circumstances would not have an ending condition. This commit removes it and cleans up the code a bit as well. https://pagure.io/freeipa/issue/6796 Reviewed-By: Florence Blanc-Renaud --- ipalib/rpc.py | 140 +++++++++++++++++++++++++++++++--------------------------- 1 file changed, 74 insertions(+), 66 deletions(-) diff --git a/ipalib/rpc.py b/ipalib/rpc.py index b12ce4c5365299332587ad0d2990ca30070217bf..e3b8d67d69c084ad1a43390b5f93061826a27e1d 100644 --- a/ipalib/rpc.py +++ b/ipalib/rpc.py @@ -999,77 +999,85 @@ class RPCClient(Connectible): # No session key, do full Kerberos auth pass urls = self.get_url_list(rpc_uri) - serverproxy = None + + proxy_kw = { + 'allow_none': True, + 'encoding': 'UTF-8', + 'verbose': verbose + } + for url in urls: - kw = { - 'allow_none': True, - 'encoding': 'UTF-8', - 'verbose': verbose - } - if url.startswith('https://'): - if delegate: - transport_class = DelegatedKerbTransport + # should we get ProtocolError (=> error in HTTP response) and + # 401 (=> Unauthorized), we'll be re-trying with new session + # cookies several times + for _try_num in range(0, 5): + if url.startswith('https://'): + if delegate: + transport_class = DelegatedKerbTransport + else: + transport_class = KerbTransport else: - transport_class = KerbTransport - else: - transport_class = LanguageAwareTransport - kw['transport'] = transport_class(protocol=self.protocol, - service='HTTP', ccache=ccache) - self.log.info('trying %s' % url) - setattr(context, 'request_url', url) - serverproxy = self.server_proxy_class(url, **kw) - if len(urls) == 1: - # if we have only 1 server and then let the - # main requester handle any errors. This also means it - # must handle a 401 but we save a ping. - return serverproxy - try: - command = getattr(serverproxy, 'ping') + transport_class = LanguageAwareTransport + proxy_kw['transport'] = transport_class( + protocol=self.protocol, service='HTTP', ccache=ccache) + self.log.info('trying %s' % url) + setattr(context, 'request_url', url) + serverproxy = self.server_proxy_class(url, **proxy_kw) + if len(urls) == 1: + # if we have only 1 server and then let the + # main requester handle any errors. This also means it + # must handle a 401 but we save a ping. + return serverproxy try: - command([], {}) - except Fault as e: - e = decode_fault(e) - if e.faultCode in errors_by_code: - error = errors_by_code[e.faultCode] - raise error(message=e.faultString) - else: - raise UnknownError( - code=e.faultCode, - error=e.faultString, - server=url, - ) - # We don't care about the response, just that we got one - break - except errors.KerberosError: - # kerberos error on one server is likely on all - raise - except ProtocolError as e: - if hasattr(context, 'session_cookie') and e.errcode == 401: - # Unauthorized. Remove the session and try again. - delattr(context, 'session_cookie') + command = getattr(serverproxy, 'ping') try: - delete_persistent_client_session_data(principal) - except Exception: - # This shouldn't happen if we have a session but it isn't fatal. - pass - return self.create_connection( - ccache, verbose, fallback, delegate) - if not fallback: + command([], {}) + except Fault as e: + e = decode_fault(e) + if e.faultCode in errors_by_code: + error = errors_by_code[e.faultCode] + raise error(message=e.faultString) + else: + raise UnknownError( + code=e.faultCode, + error=e.faultString, + server=url, + ) + # We don't care about the response, just that we got one + return serverproxy + except errors.KerberosError: + # kerberos error on one server is likely on all raise - else: - self.log.info('Connection to %s failed with %s', url, e) - serverproxy = None - except Exception as e: - if not fallback: - raise - else: - self.log.info('Connection to %s failed with %s', url, e) - serverproxy = None - - if serverproxy is None: - raise NetworkError(uri=_('any of the configured servers'), - error=', '.join(urls)) - return serverproxy + except ProtocolError as e: + if hasattr(context, 'session_cookie') and e.errcode == 401: + # Unauthorized. Remove the session and try again. + delattr(context, 'session_cookie') + try: + delete_persistent_client_session_data(principal) + except Exception: + # This shouldn't happen if we have a session but + # it isn't fatal. + pass + # try the same url once more with a new session cookie + continue + if not fallback: + raise + else: + self.log.info( + 'Connection to %s failed with %s', url, e) + # try the next url + break + except Exception as e: + if not fallback: + raise + else: + self.log.info( + 'Connection to %s failed with %s', url, e) + # try the next url + break + # finished all tries but no serverproxy was found + raise NetworkError(uri=_('any of the configured servers'), + error=', '.join(urls)) def destroy_connection(self): conn = getattr(context, self.id, None) -- 2.9.4