On 01/31/2013 05:01 PM, Alexander Bokovoy wrote:
> On Wed, 30 Jan 2013, Martin Kosek wrote:
>> Some parts of install scripts used only ccache name as returned by
>> krbV.CCache.name attribute. However, when this name is used again
>> to initialize krbV.CCache object or when it is used in KRB5CCNAME
>> environmental variable, it fails for new DIR type of CCACHE.
>>
>> We should always use both CCACHE type and name when referring to
>> them to avoid these crashes. ldap2 backend was also updated to
>> accept directly krbV.CCache object which contains everything we need
>> to authenticate with ccache.
>>
>> https://fedorahosted.org/freeipa/ticket/3381
> Minor comment: there are few cleanups of 'import krbV' in places where
> Kerberos functions are not used. Maybe it would be better to separate
> them into their own patch to avoid rebasing issues in future?

Sure, good idea. Attaching both patches.

> 
>> Please note, that this fix is rather a short/medium-term fix for Fedora 18. 
>> In
>> a long term we should consolidate our CCACHE manipulation code, it now uses
>> several different wrappers or just uses krbV python library directly. I did 
>> not
>> do any global refactoring in this patch, this should be done after we decide 
>> if
>> we want to create a new, more usable krb5 library bindings as was already
>> discussed in the past.
> Yes. John has published his current code for new Python bindings to
> libkrb5 at https://github.com/jdennis/python-krb. It is far from
> finished but gives more pythony feeling and additional contributions are
> highly welcomed.
> 
> Once it is ready, we can start looking migrating to it.

Agreed. During the migration, it would then make sense to also refactor and
consolidate a our CCACHE manupulation code.


> 
>> from ipalib import api, errors
>> from ipalib.crud import CrudBackend
>> from ipalib.request import context
>> @@ -783,7 +781,7 @@ class ldap2(CrudBackend):
>>
>>         Keyword arguments:
>>         ldapuri -- the LDAP server to connect to
>> -        ccache -- Kerberos V5 ccache name
>> +        ccache -- Kerberos V5 ccache object or name
>>         bind_dn -- dn used to bind to the server
>>         bind_pw -- password used to bind to the server
>>         debug_level -- LDAP debug level option
>> @@ -821,10 +819,17 @@ class ldap2(CrudBackend):
>>                 if maxssf < minssf:
>>                     conn.set_option(_ldap.OPT_X_SASL_SSF_MAX, minssf)
>>             if ccache is not None:
>> +                if isinstance(ccache, krbV.CCache):
>> +                    principal = ccache.principal().name
>> +                    # get a fully qualified CCACHE name (schema+name)
>> +                    ccache = "%(type)s:%(name)s" % dict(type=ccache.type,
>> +                                                        name=ccache.name)
> May be a comment could be added here that we don't use krbV.CCache
> instance afterwards and it is OK to override refernce to it by a
> string?

Comment added.

> 
>> +                else:
>> +                    principal = krbV.CCache(name=ccache,
>> +                        context=krbV.default_context()).principal().name
>> +
>>                 os.environ['KRB5CCNAME'] = ccache
>>                 conn.sasl_interactive_bind_s(None, SASL_AUTH)
>> -                principal = krbV.CCache(name=ccache,
>> -                            context=krbV.default_context()).principal().name
>>                 setattr(context, 'principal', principal)
>>             else:
>>                 # no kerberos ccache, use simple bind or external sasl
> 

Updated patches attached.

Martin
From 386eaebe74ae55fd51615ac072675fcf185a3b9a Mon Sep 17 00:00:00 2001
From: Martin Kosek <mko...@redhat.com>
Date: Thu, 31 Jan 2013 17:16:32 +0100
Subject: [PATCH 1/2] Remove unused krbV imports

https://fedorahosted.org/freeipa/ticket/3381
---
 install/certmonger/dogtag-ipa-retrieve-agent-submit | 1 -
 install/restart_scripts/renew_ca_cert               | 1 -
 install/tools/ipa-upgradeconfig                     | 1 -
 ipaserver/plugins/ldap2.py                          | 2 --
 4 files changed, 5 deletions(-)

diff --git a/install/certmonger/dogtag-ipa-retrieve-agent-submit b/install/certmonger/dogtag-ipa-retrieve-agent-submit
index 6d54000d6ec15b89557af144fe1d72c14c3128ac..3781fc5d01da12ce2dc01e17fc60143e82fbedc6 100644
--- a/install/certmonger/dogtag-ipa-retrieve-agent-submit
+++ b/install/certmonger/dogtag-ipa-retrieve-agent-submit
@@ -26,7 +26,6 @@ import os
 import sys
 import shutil
 import tempfile
-import krbV
 import syslog
 from ipalib import api
 from ipapython.dn import DN
diff --git a/install/restart_scripts/renew_ca_cert b/install/restart_scripts/renew_ca_cert
index b7e4ebaae89472dd12f3767616e004f96358df7e..b1efd8f9d5c211315c140915fa51e17bae4c0436 100644
--- a/install/restart_scripts/renew_ca_cert
+++ b/install/restart_scripts/renew_ca_cert
@@ -23,7 +23,6 @@ import os
 import sys
 import shutil
 import tempfile
-import krbV
 import syslog
 import random
 import time
diff --git a/install/tools/ipa-upgradeconfig b/install/tools/ipa-upgradeconfig
index 8ec6248b32871a6dc6271243880b7424e0219697..f71d834e5aff9f1b433b5bb7a31711e40e830cce 100644
--- a/install/tools/ipa-upgradeconfig
+++ b/install/tools/ipa-upgradeconfig
@@ -41,7 +41,6 @@ try:
     from ipaserver.install import certs
     from ipaserver.install import sysupgrade
     import ldap
-    import krbV
     import re
     import os
     import shutil
diff --git a/ipaserver/plugins/ldap2.py b/ipaserver/plugins/ldap2.py
index 8e8e1604ff0a3d36fe3501ec6f54abdb717d78ae..9b438944610f2f441104e29662f67d3b32d93506 100644
--- a/ipaserver/plugins/ldap2.py
+++ b/ipaserver/plugins/ldap2.py
@@ -67,8 +67,6 @@ except ImportError:
 # for backward compatibility
 from ipalib import _
 
-import krbV
-
 from ipalib import api, errors
 from ipalib.crud import CrudBackend
 from ipalib.request import context
-- 
1.7.11.7

From 46d6b60569afc0438054e96b002185ff0812241b Mon Sep 17 00:00:00 2001
From: Martin Kosek <mko...@redhat.com>
Date: Thu, 31 Jan 2013 17:18:35 +0100
Subject: [PATCH 2/2] Use fully qualified CCACHE names

Some parts of install scripts used only ccache name as returned by
krbV.CCache.name attribute. However, when this name is used again
to initialize krbV.CCache object or when it is used in KRB5CCNAME
environmental variable, it fails for new DIR type of CCACHE.

We should always use both CCACHE type and name when referring to
them to avoid these crashes. ldap2 backend was also updated to
accept directly krbV.CCache object which contains everything we need
to authenticate with ccache.

https://fedorahosted.org/freeipa/ticket/3381
---
 install/tools/ipa-adtrust-install |  2 +-
 install/tools/ipa-dns-install     |  2 +-
 install/tools/ipa-replica-manage  |  2 +-
 ipalib/plugins/kerberos.py        |  9 ++++++---
 ipaserver/plugins/ldap2.py        | 15 ++++++++++++---
 5 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/install/tools/ipa-adtrust-install b/install/tools/ipa-adtrust-install
index 17f2f0e98d08863c9e48595d219bffb148490921..6985bba2798ea908be8d834d89d08f9b67a441e2 100755
--- a/install/tools/ipa-adtrust-install
+++ b/install/tools/ipa-adtrust-install
@@ -299,7 +299,7 @@ def main():
         sys.exit("Must have Kerberos credentials to setup AD trusts on server")
 
     try:
-        api.Backend.ldap2.connect(ccache.name)
+        api.Backend.ldap2.connect(ccache)
     except errors.ACIError, e:
         sys.exit("Outdated Kerberos credentials. Use kdestroy and kinit to update your ticket")
     except errors.DatabaseError, e:
diff --git a/install/tools/ipa-dns-install b/install/tools/ipa-dns-install
index 71592d4899d702606b33e0ac89592d91f99c5e29..2ab90b648030857520b0023207031cf9922d908e 100755
--- a/install/tools/ipa-dns-install
+++ b/install/tools/ipa-dns-install
@@ -205,7 +205,7 @@ def main():
         api.Backend.ldap2.connect(bind_dn=DN(('cn', 'Directory Manager')), bind_pw=bind.dm_password)
     else:
         # See if our LDAP server is up and we can talk to it over GSSAPI
-        ccache = krbV.default_context().default_ccache().name
+        ccache = krbV.default_context().default_ccache()
         api.Backend.ldap2.connect(ccache)
 
     if options.reverse_zone:
diff --git a/install/tools/ipa-replica-manage b/install/tools/ipa-replica-manage
index 2422891082ac5be1521426e40772bc23e8d1dc3a..0dad143198db6f721688ca7823db2fefd1e5a831 100755
--- a/install/tools/ipa-replica-manage
+++ b/install/tools/ipa-replica-manage
@@ -667,7 +667,7 @@ def del_master(realm, hostname, options):
                 api.Backend.ldap2.connect(bind_dn=DN(('cn', 'Directory Manager')),
                                           bind_pw=options.dirman_passwd)
             else:
-                ccache = krbV.default_context().default_ccache().name
+                ccache = krbV.default_context().default_ccache()
                 api.Backend.ldap2.connect(ccache=ccache)
             bind = bindinstance.BindInstance()
             bind.remove_master_dns_records(hostname, realm, realm.lower())
diff --git a/ipalib/plugins/kerberos.py b/ipalib/plugins/kerberos.py
index e6f775b97d4b1d89073f9d3f67991ec43f135200..7ae63b930d6296b3dd2d9fdadfbd8a64328cdfd4 100644
--- a/ipalib/plugins/kerberos.py
+++ b/ipalib/plugins/kerberos.py
@@ -66,14 +66,17 @@ class krb(Backend):
 
     def default_ccname(self):
         """
-        Return the default ccache file name.
+        Return the default ccache file name (schema+name).
 
-        This will return something like '/tmp/krb5cc_500'.
+        This will return something like 'FILE:/tmp/krb5cc_500'.
 
         This cannot return anything meaningful if used in the server as a
         request is processed.
         """
-        return self.__default_ccache().name
+        default_ccache = self.__default_ccache()
+        ccname = "%(type)s:%(name)s" % dict(type=default_ccache.type,
+                                            name=default_ccache.name)
+        return ccname
 
     def default_principal(self):
         """
diff --git a/ipaserver/plugins/ldap2.py b/ipaserver/plugins/ldap2.py
index 9b438944610f2f441104e29662f67d3b32d93506..8c296fc1190c1807390a8e7e78598aa22df1dcaf 100644
--- a/ipaserver/plugins/ldap2.py
+++ b/ipaserver/plugins/ldap2.py
@@ -781,7 +781,7 @@ class ldap2(CrudBackend):
 
         Keyword arguments:
         ldapuri -- the LDAP server to connect to
-        ccache -- Kerberos V5 ccache name
+        ccache -- Kerberos V5 ccache object or name
         bind_dn -- dn used to bind to the server
         bind_pw -- password used to bind to the server
         debug_level -- LDAP debug level option
@@ -819,10 +819,19 @@ class ldap2(CrudBackend):
                 if maxssf < minssf:
                     conn.set_option(_ldap.OPT_X_SASL_SSF_MAX, minssf)
             if ccache is not None:
+                if isinstance(ccache, krbV.CCache):
+                    principal = ccache.principal().name
+                    # Get a fully qualified CCACHE name (schema+name)
+                    # As we do not use the krbV.CCache object later,
+                    # we can safely overwrite it
+                    ccache = "%(type)s:%(name)s" % dict(type=ccache.type,
+                                                        name=ccache.name)
+                else:
+                    principal = krbV.CCache(name=ccache,
+                        context=krbV.default_context()).principal().name
+
                 os.environ['KRB5CCNAME'] = ccache
                 conn.sasl_interactive_bind_s(None, SASL_AUTH)
-                principal = krbV.CCache(name=ccache,
-                            context=krbV.default_context()).principal().name
                 setattr(context, 'principal', principal)
             else:
                 # no kerberos ccache, use simple bind or external sasl
-- 
1.7.11.7

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to