Re: [Freeipa-devel] [PATCH] 357 Use fully qualified CCACHE names

2013-01-31 Thread Alexander Bokovoy

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?


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.


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?


+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


--
/ Alexander Bokovoy

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


Re: [Freeipa-devel] [PATCH] 357 Use fully qualified CCACHE names

2013-01-31 Thread Martin Kosek
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 

Re: [Freeipa-devel] [PATCH] 357 Use fully qualified CCACHE names

2013-01-31 Thread Alexander Bokovoy

On Thu, 31 Jan 2013, Martin Kosek wrote:

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.

ACK to both now. Thanks!

--
/ Alexander Bokovoy

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


Re: [Freeipa-devel] [PATCH] 357 Use fully qualified CCACHE names

2013-01-31 Thread Martin Kosek
On 01/31/2013 07:07 PM, Alexander Bokovoy wrote:
 On Thu, 31 Jan 2013, Martin Kosek wrote:
 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.
 ACK to both now. Thanks!
 

Pushed to master, ipa-3-1.

Martin

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