[Freeipa-devel] [PATCH 0002] Port from python-krbV to python-gssapi

[Simo Sorce]

On Thu, 2015-08-20 at 14:42 -0400, Robbie Harwood wrote:
> Michael Šimáček <msimacek at redhat.com> writes:
> 
> > On 2015-08-20 12:32, Michael Šimáček wrote:
> >
> >>>>> Michael Šimáček <msimacek at 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.
> 
> > +    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?

Lifetime == 0 means the credentials are expired, it should never be
returned but if it were we'd have to raise ourself
gssapi.exceptions.ExpiredCredentialsError

> >          # 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.
> 
> > +# 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.

Well turns out there is a gssapi_krb5 extension to get a ccache name:
	gss_krb5_ccache_name()

Robbie,
do you think we should expose it in python-gssapi if available ?

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York