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