From f5f7cbc63f9235ce040687cdf102aeaef69ab422 Mon Sep 17 00:00:00 2001

From: Stanislav Laznicka <slaznick@redhat.com>

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 <frenaud@redhat.com>

---

 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