Comments inline.

On 2015-07-23 21:29, Robbie Harwood wrote:
Some comments from Solly and I inline:

Michael Šimáček <msima...@redhat.com> writes:

On 2015-07-22 15:47, Simo Sorce wrote:
Comments inline.

----- Original Message -----
From: "Michael Simacek" <msima...@redhat.com>
To: freeipa-devel@redhat.com
Sent: Tuesday, July 21, 2015 8:02:26 AM
Subject: [Freeipa-devel] [PATCH] Port from python-kerberos library to   
python-gssapi

diff --git a/ipalib/util.py b/ipalib/util.py
index 649a487..aea3ba9 100644
--- a/ipalib/util.py
+++ b/ipalib/util.py
@@ -63,15 +63,15 @@ def json_serialize(obj):

   def get_current_principal():
       try:
-        import kerberos
-        rc, vc = kerberos.authGSSClientInit("notempty")
-        rc = kerberos.authGSSClientInquireCred(vc)
-        username = kerberos.authGSSClientUserName(vc)
-        kerberos.authGSSClientClean(vc)
+        import gssapi
+        cred = gssapi.raw.acquire_cred(usage='initiate').creds
+        name = gssapi.raw.inquire_cred(cred, lifetime=False, usage=False,
+                                       mechs=False).name
+        username = gssapi.raw.display_name(name, name_type=False).name

Same as above.
Create a credential and inquire it with the high level api

Done, but I still use raw.display_name as I don't see how to get it from
high-level API (besides parsing repr).

I believe one can call `str()`.  See
http://pythonhosted.org/gssapi/gssapi.html#gssapi.names.Name


You're of course right. I'm sorry I missed such an obvious thing.


@@ -548,14 +552,12 @@ class KerbTransport(SSLTransport):
          service = "HTTP@" + host.split(':')[0]

          try:
-            (rc, vc) = kerberos.authGSSClientInit(service=service,
-                                                  gssflags=self.flags)
-        except kerberos.GSSError, e:
-            self._handle_exception(e)
-
-        try:
-            kerberos.authGSSClientStep(vc, "")
-        except kerberos.GSSError, e:
+            name = gssapi.Name(service, gssapi.NameType.hostbased_service)
+            sec_context = gssapi.SecurityContext(name=name, flags=self.flags)
+            # gssapi defers errors to next step, we want them now
+            sec_context.__DEFER_STEP_ERRORS__ = False

As a class-level flag, this should probably be used as such.  Preferable
to using it would be to check complete, though - is there a reason not
to do that here?

Otherwise, looks good!


It would probably be nicer to do the full cycle, but I'd like to avoid changes in behavior when porting from one library to another. And the code above doesn't actually hold any connection, so it would require more refactoring to make that happen. For now I would follow what the original code was doing. As for the exceptions, I think it would actually be justifiable to use the raw api's init_sec_context, because the high level api would just do the same call + the exception handling magic, which we want to avoid for now. Please let me know what do you think. Attaching updated patch that uses 'unicode' instead of raw.display_name and reverts back to using init_sec_context.

Thank you.

--
Michael Simacek
From abdee5c742cf0adaa287b8932f25d701219f71a0 Mon Sep 17 00:00:00 2001
From: Michael Simacek <msima...@redhat.com>
Date: Thu, 16 Jul 2015 18:22:00 +0200
Subject: [PATCH] Port from python-kerberos to python-gssapi

kerberos library doesn't support Python 3 and probably never will.
python-gssapi library is Python 3 compatible.

https://fedorahosted.org/freeipa/ticket/5147
---
 BUILD.txt            |  2 +-
 freeipa.spec.in      |  2 +-
 ipalib/rpc.py        | 43 ++++++++++++++++++++++---------------------
 ipalib/util.py       | 13 +++++--------
 ipapython/ipautil.py | 17 -----------------
 5 files changed, 29 insertions(+), 48 deletions(-)

diff --git a/BUILD.txt b/BUILD.txt
index 6a28beba1e0844971fb5625c0e1adf3f0c0fc0e3..53012b14d05673d4fbc4d0567e877348d5e78444 100644
--- a/BUILD.txt
+++ b/BUILD.txt
@@ -20,7 +20,7 @@ systemd-units samba-devel samba-python libwbclient-devel libtalloc-devel \
 libtevent-devel nspr-devel nss-devel openssl-devel openldap-devel krb5-devel \
 krb5-workstation libuuid-devel libcurl-devel xmlrpc-c-devel popt-devel \
 autoconf automake m4 libtool gettext python-devel python-ldap \
-python-setuptools python-krbV python-nss python-netaddr python-kerberos \
+python-setuptools python-krbV python-nss python-netaddr python-gssapi \
 python-rhsm pyOpenSSL pylint python-polib libipa_hbac-python python-memcached \
 sssd python-lxml python-pyasn1 python-qrcode-core python-dns m2crypto \
 check libsss_idmap-devel libsss_nss_idmap-devel java-headless rhino \
diff --git a/freeipa.spec.in b/freeipa.spec.in
index 928425fdc65a092f67a28d97101c32b7392bf1c8..e2bbd79360ac626db93fe7e957b0ef3be043da4f 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -72,7 +72,7 @@ BuildRequires:  python-krbV
 BuildRequires:  python-nss
 BuildRequires:  python-cryptography
 BuildRequires:  python-netaddr
-BuildRequires:  python-kerberos >= 1.1-14
+BuildRequires:  python-gssapi >= 1.1.1
 BuildRequires:  python-rhsm
 BuildRequires:  pyOpenSSL
 BuildRequires:  pylint >= 1.0
diff --git a/ipalib/rpc.py b/ipalib/rpc.py
index 466b49a6dd60370db4d588389acba8dcaa493aa1..43c7760616c502f07321ad0a3c69e89c6db6f0f9 100644
--- a/ipalib/rpc.py
+++ b/ipalib/rpc.py
@@ -44,7 +44,7 @@ from urllib2 import urlparse
 
 from xmlrpclib import (Binary, Fault, DateTime, dumps, loads, ServerProxy,
         Transport, ProtocolError, MININT, MAXINT)
-import kerberos
+import gssapi
 from dns import resolver, rdatatype
 from dns.exception import DNSException
 from nss.error import NSPRError
@@ -510,24 +510,28 @@ class KerbTransport(SSLTransport):
     """
     Handles Kerberos Negotiation authentication to an XML-RPC server.
     """
-    flags = kerberos.GSS_C_MUTUAL_FLAG | kerberos.GSS_C_SEQUENCE_FLAG
+    flags = [gssapi.RequirementFlag.mutual_authentication,
+             gssapi.RequirementFlag.out_of_sequence_detection]
 
     def _handle_exception(self, e, service=None):
-        (major, minor) = ipautil.get_gsserror(e)
-        if minor[1] == KRB5KDC_ERR_S_PRINCIPAL_UNKNOWN:
+        # kerberos library coerced error codes to signed, gssapi uses unsigned
+        minor = e.min_code
+        if minor & (1 << 31):
+            minor -= 1 << 32
+        if minor == KRB5KDC_ERR_S_PRINCIPAL_UNKNOWN:
             raise errors.ServiceError(service=service)
-        elif minor[1] == KRB5_FCC_NOFILE:
+        elif minor == KRB5_FCC_NOFILE:
             raise errors.NoCCacheError()
-        elif minor[1] == KRB5KRB_AP_ERR_TKT_EXPIRED:
+        elif minor == KRB5KRB_AP_ERR_TKT_EXPIRED:
             raise errors.TicketExpired()
-        elif minor[1] == KRB5_FCC_PERM:
+        elif minor == KRB5_FCC_PERM:
             raise errors.BadCCachePerms()
-        elif minor[1] == KRB5_CC_FORMAT:
+        elif minor == KRB5_CC_FORMAT:
             raise errors.BadCCacheFormat()
-        elif minor[1] == KRB5_REALM_CANT_RESOLVE:
+        elif minor == KRB5_REALM_CANT_RESOLVE:
             raise errors.CannotResolveKDC()
         else:
-            raise errors.KerberosError(major=major, minor=minor)
+            raise errors.KerberosError(major=e.maj_code, minor=minor)
 
     def get_host_info(self, host):
         """
@@ -548,14 +552,10 @@ class KerbTransport(SSLTransport):
         service = "HTTP@" + host.split(':')[0]
 
         try:
-            (rc, vc) = kerberos.authGSSClientInit(service=service,
-                                                  gssflags=self.flags)
-        except kerberos.GSSError, e:
-            self._handle_exception(e)
-
-        try:
-            kerberos.authGSSClientStep(vc, "")
-        except kerberos.GSSError, e:
+            name = gssapi.Name(service, gssapi.NameType.hostbased_service)
+            sec_context = gssapi.raw.init_sec_context(name, flags=self.flags)
+            response = sec_context.token
+        except gssapi.exceptions.GSSError as e:
             self._handle_exception(e, service=service)
 
         for (h, v) in extra_headers:
@@ -564,7 +564,7 @@ class KerbTransport(SSLTransport):
                 break
 
         extra_headers.append(
-            ('Authorization', 'negotiate %s' % kerberos.authGSSClientResponse(vc))
+            ('Authorization', 'negotiate %s' % base64.b64encode(response))
         )
 
         return (host, extra_headers, x509)
@@ -632,8 +632,9 @@ class DelegatedKerbTransport(KerbTransport):
     Handles Kerberos Negotiation authentication and TGT delegation to an
     XML-RPC server.
     """
-    flags = kerberos.GSS_C_DELEG_FLAG |  kerberos.GSS_C_MUTUAL_FLAG | \
-            kerberos.GSS_C_SEQUENCE_FLAG
+    flags = [gssapi.RequirementFlag.delegate_to_peer,
+             gssapi.RequirementFlag.mutual_authentication,
+             gssapi.RequirementFlag.out_of_sequence_detection]
 
 
 class RPCClient(Connectible):
diff --git a/ipalib/util.py b/ipalib/util.py
index 649a4875fde0b44844749946cce53d81f7f6eea4..5a670146e0ea569458bf0086b075df3d4b7b4b5b 100644
--- a/ipalib/util.py
+++ b/ipalib/util.py
@@ -63,15 +63,12 @@ def json_serialize(obj):
 
 def get_current_principal():
     try:
-        import kerberos
-        rc, vc = kerberos.authGSSClientInit("notempty")
-        rc = kerberos.authGSSClientInquireCred(vc)
-        username = kerberos.authGSSClientUserName(vc)
-        kerberos.authGSSClientClean(vc)
-        return unicode(username)
+        import gssapi
+        cred = gssapi.Credentials(usage='initiate')
+        return unicode(cred.name)
     except ImportError:
-        raise RuntimeError('python-kerberos is not available.')
-    except kerberos.GSSError, e:
+        raise RuntimeError('python-gssapi is not available.')
+    except gssapi.exceptions.GSSError:
         #TODO: do a kinit?
         raise errors.CCacheError()
 
diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index 88e89706b8e2aa6dea80809510d88bceaa836e85..05a7eebf012b27e3e34cb74778c2724e9deaf616 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -783,23 +783,6 @@ def user_input(prompt, default = None, allow_empty = True):
                 return ret
 
 
-def get_gsserror(e):
-    """
-    A GSSError exception looks differently in python 2.4 than it does
-    in python 2.5. Deal with it.
-    """
-
-    try:
-       major = e[0]
-       minor = e[1]
-    except:
-       major = e[0][0]
-       minor = e[0][1]
-
-    return (major, minor)
-
-
-
 def host_port_open(host, port, socket_type=socket.SOCK_STREAM, socket_timeout=None):
     for res in socket.getaddrinfo(host, port, socket.AF_UNSPEC, socket_type):
         af, socktype, proto, canonname, sa = res
-- 
2.1.0

-- 
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

Reply via email to