Re: [Freeipa-devel] [PATCHES 0015-0017] consolidation of various Kerberos auth methods in FreeIPA code

[Martin Babinsky]

On 03/11/2015 12:42 PM, Petr Spacek wrote:
diff --git a/ipaserver/rpcserver.py b/ipaserver/rpcserver.py
index 
d6bc955b9d9910a24eec5df1def579310eb54786..36f16908ac8477d9982bfee613b77576853054eb
 100644
--- a/ipaserver/rpcserver.py
+++ b/ipaserver/rpcserver.py
@@ -958,8 +958,8 @@ class login_password(Backend, KerberosSession, HTTP_Status):
      def kinit(self, user, realm, password, ccache_name):
          # get http service ccache as an armor for FAST to enable OTP 
authentication
-        armor_principal = krb5_format_service_principal_name(
-            'HTTP', self.api.env.host, realm)
+        armor_principal = str(krb5_format_service_principal_name(
+            'HTTP', self.api.env.host, realm))
          keytab = paths.IPA_KEYTAB
          armor_name = "%sA_%s" % (krbccache_prefix, user)
          armor_path = os.path.join(krbccache_dir, armor_name)
@@ -967,34 +967,33 @@ class login_password(Backend, KerberosSession, 
HTTP_Status):
          self.debug('Obtaining armor ccache: principal=%s keytab=%s ccache=%s',
                     armor_principal, keytab, armor_path)

-        (stdout, stderr, returncode) = ipautil.run(
-            [paths.KINIT, '-kt', keytab, armor_principal],
-            env={'KRB5CCNAME': armor_path}, raiseonerr=False)
-
-        if returncode != 0:
-            raise CCacheError()
+        try:
+            ipautil.kinit_keytab(paths.IPA_KEYTAB, armor_path,
+                                 armor_principal)
+        except StandardError, e:
+            raise CCacheError(str(e))

          # Format the user as a kerberos principal
          principal = krb5_format_principal_name(user, realm)

-        (stdout, stderr, returncode) = ipautil.run(
-            [paths.KINIT, principal, '-T', armor_path],
-            env={'KRB5CCNAME': ccache_name, 'LC_ALL': 'C'},
-            stdin=password, raiseonerr=False)
+        try:
+            ipautil.kinit_password(principal, password,
+                                   env={'KRB5CCNAME': ccache_name,
+                                        'LC_ALL': 'C'},
+                                   armor_ccache=armor_path)

-        self.debug('kinit: principal=%s returncode=%s, stderr="%s"',
-                   principal, returncode, stderr)
-
-        self.debug('Cleanup the armor ccache')
-        ipautil.run(
-            [paths.KDESTROY, '-A', '-c', armor_path],
-            env={'KRB5CCNAME': armor_path},
-            raiseonerr=False)
-
-        if returncode != 0:
-            if stderr.strip() == 'kinit: Cannot read password while getting 
initial credentials':
-                raise PasswordExpired(principal=principal, 
message=unicode(stderr))
-            raise InvalidSessionPassword(principal=principal, 
message=unicode(stderr))
+            self.debug('Cleanup the armor ccache')
+            ipautil.run(
+                [paths.KDESTROY, '-A', '-c', armor_path],
+                env={'KRB5CCNAME': armor_path},
+                raiseonerr=False)
+        except ipautil.CalledProcessError, e:
+            if ('kinit: Cannot read password while '
+                    'getting initial credentials') in e.output:
I know it is not your code but please make sure it will work with non-English
LANG or LC_MESSAGE.

I have done some research about the way the environmental variables line 
LC_MESSAGE, LC_ALL, etc. work, 
(https://www.gnu.org/software/gettext/manual/gettext.html#Locale-Names 
and the following section).

It turns out that the CalledProcessError handling code will work also in 
non-english environment, because in the following code snippet, kinit is 
actually run using LC_ALL=C environment variable effectively disabling 
any localization and forcing the program to print out default (i.e. 
english) error messages.

>> +        try:
>> +            ipautil.kinit_password(principal, password,
>> +                                   env={'KRB5CCNAME': ccache_name,
>> +                                        'LC_ALL': 'C'},
>> +                                   armor_ccache=armor_path)

Thus when handling the exception, we may be sure that any error message 
returned by above is in default locale. This greatly simplifies the 
logic deciding what exception to raise further based on the error message.

>> +        except ipautil.CalledProcessError, e:
>> +            if ('kinit: Cannot read password while '
>> +                    'getting initial credentials') in e.output:

A very clever move by Nathaniel (according to git blame) to circumvent 
the locale-specific behavior when it's actualy not needed.

--
Martin^3 Babinsky

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code