On 23.8.2015 23:27, Michael Šimáček wrote:



On 2015-08-21 15:52, Michael Šimáček wrote:


On 2015-08-20 20:42, Robbie Harwood wrote:
Michael Šimáček <msima...@redhat.com> writes:

On 2015-08-20 12:32, Michael Šimáček wrote:

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

Attaching new revision of the patch. Changes from the previous:
- ldap2's connect now chooses the bind type same way as in ipaldap
- get_default_realm usages replaced by api.env.realm
- fixed missing third kinit attempt in trust-fetch-domains
- removed rewrapping gssapi errors to ccache errors in krb_utils
- updated some parts of exception handling

Rebased on top of current master.

One of the commits reintroduced krbV dependency that I didn't notice.
Attaching updated revision. Only changes against previous revision are
in files daemons/dnssec/ipa-dnskeysync-replica and
daemons/dnssec/ipa-ods-exporter.

This is much better, thanks!  I've got some comments inline.

+except gssapi.exceptions.GSSError:
      # If there was failure on using keytab, assume it is stale and
retrieve again
      retrieve_keytab(api, ccache_name, oneway_keytab_name,
oneway_principal)

This code still bothers me a bit, but I think fixing it is probably
beyond the scope of a python-gssapi port.

The code catches all GSSAPI exceptions and retries to do the same thing
with different keytab. So if there was a problem unrelated to keytab,
the same exception will be raised again. Nothing will be ignored
silently.


+    try:
+        creds = get_credentials(name=name, ccache_name=ccache_name)
+        # property access would raise exception if expired
+        if creds.lifetime > 0:
+            return creds
+    except gssapi.exceptions.ExpiredCredentialsError:
+        return None

Per rfc2744, lifetime is unsigned.  It's not immediately clear what will
happen when `creds.lifetime == 0`; perhaps an explicit `return Nune` in
that case?

I think the check is probably redundant, gssapi raises exception upon
inquiring expired credentials. In trust-fetch-domains I just access the
lifetime in try-except without using the value, so I could do the same
here. It would be nice if gssapi provided some 'is_valid' or
'is_expired' method, so I wouldn't need to rely on side-effects of
property access, which is hard to read and confuses pylint.


          # Setup LDAP connection
          try:
-            ctx = krbV.default_context()
-            ccache = ctx.default_ccache()
-            api.Backend.ldap2.connect(ccache)
+            api.Backend.ldap2.connect()
              cls.ldap = api.Backend.ldap2
-        except krbV.Krb5Error as e:
+        except gssapi.exceptions.GSSError:
              sys.exit("Must have Kerberos credentials to migrate
Winsync users.")

Can you log the error here?  The other places GSSError is being caught
are doing a great job of either filtering-and-raising or
logging-and-exiting, so thanks for fixing those.

Yes, I'll update it in next revision of the patch.


+# Ugly hack for test purposes only. GSSAPI has no way to get default
ccache
+# name, but we don't need it outside test server
+def get_default_ccache_name():
+    try:
+        out = check_output(['klist'])
+    except CalledProcessError:
+        raise RuntimeError("Default ccache not found. Did you kinit?")
+    match = re.match(r'^Ticket cache:\s*(\S+)', out)
+    if not match:
+        raise RuntimeError("Cannot obtain ccache name")
+    return match.group(1)

Yup, this is still ugly.  Ah well, it's only test code.


I was trying to modify the code to not need the variable and just use
the default, but it is used for manipulating it as a file - in
production it is always defined by mod_auth_gssapi. So I'd keep this
as is.



Next revision of the patch. Changes from previous rev: printing
exception in ipa_winsync_migrate and more thorough dealing with
credentials expiration in krb_utils and trust-fetch-domains.

1) There is a merge conflict in freeipa.spec.in, please rebase the patch on top of current master.


2) pylint fails with:

************* Module ipa-ods-exporter
daemons/dnssec/ipa-ods-exporter:23: [E0611(no-name-in-module), ] No name 'GSSError' in module 'gssapi')
************* Module ipa-dnskeysync-replica
daemons/dnssec/ipa-dnskeysync-replica:15: [E0611(no-name-in-module), ] No name 'GSSError' in module 'gssapi')

Both failures are caused by "from gssapi import GSSError" statement, which should read "from gssapi.exceptions import GSSError".


3) ipa-adtrust-install fails with:

admin password:

Unrecognized error during check of admin rights: ad...@abc.idm.lab.eng.brq.redhat.com: user not found

Apparently there is a "user-show ad...@abc.idm.lab.eng.brq.redhat.com" call where a "user-show admin" call should be.


4) ipa-client-automount fails with:

Failed to obtain host TGT: Major (851968): Unspecified GSS failure. Minor code may provide more information, Minor (2529639111): Bad format in credentials cache


Honza

--
Jan Cholasta

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