From 0a169b1bea997b489979f624f31057facfcfd984 Mon Sep 17 00:00:00 2001
From: Stanislav Levin <slev@altlinux.org>
Date: Fri, 4 Jun 2021 12:53:25 +0300
Subject: [PATCH] krb_utils: Simplify get_credentials

Previously, `get_credentials` raises either ValueError or re-raises
GSSError. The former makes the handling of this function more difficult
without a good reason.

With this change:
- `get_credentials` no longer handles exceptions by itself, but delegates
this to the callers (which already process GSS errors).
- `get_credentials_if_valid` doesn't raise any expected exceptions, but
return valid credentials (on the moment of calling) or None. This makes
it consistent with docs.

Related: https://pagure.io/freeipa/issue/8873
Signed-off-by: Stanislav Levin <slev@altlinux.org>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
---
 install/tools/ipa-ccache-sweeper.in |  9 ++-----
 ipalib/krb_utils.py                 | 42 ++++++++++++++++-------------
 ipaserver/rpcserver.py              |  3 +++
 3 files changed, 28 insertions(+), 26 deletions(-)

diff --git a/install/tools/ipa-ccache-sweeper.in b/install/tools/ipa-ccache-sweeper.in
index 4a1e1fef1..4b304d242 100644
--- a/install/tools/ipa-ccache-sweeper.in
+++ b/install/tools/ipa-ccache-sweeper.in
@@ -43,13 +43,8 @@ def should_delete(fname, t, minlife):
     # is not provided in cred_store the result of gss_acquire_cred_from
     # is KRB5_FCC_NOFILE, which is mapped by gssproxy to
     # 0x04200000 + KRB5_FCC_NOFILE.
-    try:
-        creds = get_credentials_if_valid(ccache_name=fname)
-        return creds is None
-    except ValueError:
-        return True
-
-    return False
+    creds = get_credentials_if_valid(ccache_name=fname)
+    return creds is None
 
 
 if __name__ == "__main__":
diff --git a/ipalib/krb_utils.py b/ipalib/krb_utils.py
index 21078ef3e..8fe600827 100644
--- a/ipalib/krb_utils.py
+++ b/ipalib/krb_utils.py
@@ -19,14 +19,10 @@
 import time
 import re
 
-import six
 import gssapi
 
 from ipalib import errors
 
-if six.PY3:
-    unicode = str
-
 #-------------------------------------------------------------------------------
 
 # Kerberos error codes
@@ -149,14 +145,24 @@ def get_credentials(name=None, ccache_name=None):
     store = None
     if ccache_name:
         store = {'ccache': ccache_name}
-    try:
-        return gssapi.Credentials(usage='initiate', name=name, store=store)
-    except gssapi.exceptions.GSSError as e:
-        if e.min_code in (  # pylint: disable=no-member
-            KRB5_FCC_NOFILE, GSSPROXY_KRB5_FCC_NOFILE, KRB5_CC_NOTFOUND,
-        ):
-            raise ValueError('"%s", ccache="%s"' % (e, ccache_name))
-        raise
+    """
+    https://datatracker.ietf.org/doc/html/rfc2744.html#section-5.2
+    gss_acquire_cred:
+        If credential acquisition is time-consuming for a mechanism, the
+    mechanism may choose to delay the actual acquisition until the
+    credential is required (e.g. by gss_init_sec_context or
+    gss_accept_sec_context). Such mechanism-specific implementation
+    decisions should be invisible to the calling application; thus a call
+    of gss_inquire_cred immediately following the call of
+    gss_acquire_cred must return valid credential data, and may therefore
+    incur the overhead of a deferred credential acquisition.
+
+    So, as gssapi.Credentials() calls only gss_acquire_cred it is not
+    guaranteed to have valid(not expired) returned creds and all the
+    callers of this function have to deal with GSSAPI exceptions by
+    themselves, for example, to handle ExpiredCredentialsError.
+    """
+    return gssapi.Credentials(usage="initiate", name=name, store=store)
 
 def get_principal(ccache_name=None):
     '''
@@ -174,9 +180,9 @@ def get_principal(ccache_name=None):
     '''
     try:
         creds = get_credentials(ccache_name=ccache_name)
-        return unicode(creds.name)
-    except ValueError as e:
-        raise errors.CCacheError(message=unicode(e))
+        return str(creds.name)
+    except gssapi.exceptions.GSSError as e:
+        raise errors.CCacheError(message=str(e))
 
 def get_credentials_if_valid(name=None, ccache_name=None):
     '''
@@ -199,8 +205,6 @@ def get_credentials_if_valid(name=None, ccache_name=None):
         creds = get_credentials(name=name, ccache_name=ccache_name)
         if creds.lifetime > 0:
             return creds
+    except gssapi.exceptions.GSSError:
         return None
-    except gssapi.exceptions.ExpiredCredentialsError:
-        return None
-    except ValueError:
-        return None
+    return None
diff --git a/ipaserver/rpcserver.py b/ipaserver/rpcserver.py
index 9911e9457..b121316bf 100644
--- a/ipaserver/rpcserver.py
+++ b/ipaserver/rpcserver.py
@@ -913,6 +913,9 @@ class jsonserver_session(jsonserver, KerberosSession):
             else:
                 return self.service_unavailable(environ, start_response, msg)
 
+        except CCacheError:
+            return self.need_login(start_response)
+
         try:
             response = super(jsonserver_session, self).__call__(environ, start_response)
         finally: