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

[Robbie Harwood]

Michael Šimáček <msimacek at redhat.com> writes:

> GSSAPI doesn't provide any method (that I'm aware of) to get default
> ccache name. In most cases this is not needed as we can simply not
> pass any name and it will use the default. The ldap plugin had to be
> adjusted for this - the connect method now takes new use_gssapi
> argument, which can turn on gssapi support without the need to supply
> explicit ccache name. The only place where the ccache name is really
> needed is the test server, where I use system klist command to obtain
> it.

This is sub-optimal, but not a huge deal if it's only in the test
suite.

> It's also not possible to directly get default realm name, what I do
> is importing nonexistent name, cannonicalizing it and extracting the
> realm from it. Which should work but is ugly. It would be better if we
> could modify the places that use it to not need it at all, but it's
> mostly used in ldap code and I don't understand that part of FreeIPA.
> Alternative would be parsing /etc/krb.conf.

Please try not to do this.  DEFINITELY do not parse krb.conf.
Unfortunately, I do not know enough about the LDAP code to know why this
is needed or to suggest an alternate solution.

> Sorry for long patch, but I'm afraid it cannot be reasonably split.

This is indeed really long and difficult to work through.  I have
probably missed some things; apologies if they come through in a later
round.

> +try:
> +    cred = kinit_keytab(principal, keytab_name, ccache_name)
> +    # would raise exception if expired
> +    cred.lifetime
> +except gssapi.exceptions.ExpiredCredentialsError:
> +    # delete stale ccache and try again
> +    os.unlink(ccache_name)
> +    cred = kinit_keytab(principal, keytab_name, ccache_name)

See next comment.

> -    # The keytab may have stale key material (from older trust-add run)
> -    if not os.path.isfile(oneway_ccache_name):
> -        oneway_ccache = kinit_keytab(oneway_principal, oneway_keytab_name, oneway_ccache_name)
> -except krbV.Krb5Error as e:
> +    try:
> +        # The keytab may have stale key material (from older trust-add run)
> +        cred = kinit_keytab(oneway_principal, oneway_keytab_name, oneway_ccache_name)
> +        # would raise exception if expired
> +        cred.lifetime
> +    except gssapi.exceptions.ExpiredCredentialsError:
> +        cred = kinit_keytab(oneway_principal, oneway_keytab_name, oneway_ccache_name)
> +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)

In general, it's bad practice to catch *all* possible GSS errors unless
you intend to display their status and/or abort/raise.  If there's a
specific state you want to cope with here, catch the exception related
to it, not all of them.  Up above is a place where I think this is done
right.

> -        ctx = krbV.default_context()
> -        ccache = ctx.default_ccache()
> -        principal = ccache.principal()
> -    except krbV.Krb5Error, e:
> +        principal = krb_utils.get_principal()
> +    except errors.CCacheError:
>          sys.exit("Must have Kerberos credentials to setup AD trusts on server")

Based on how GSSAPI error messages are being packed into CCache errors
(the name of which is itself unfortunate...), it would be nice to give
some hint of the problem here if it were GSSAPI; otherwise, to my eye,
it looks like the GSSAPI status is being dropped.

> +def get_credentials(name=None, ccache_name=None):
>      '''
> -    Kerberos stores a TGT (Ticket Granting Ticket) and the service
> -    tickets bound to it in a ccache (credentials cache). ccaches are
> -    bound to a Kerberos user principal. This class opens a Kerberos
> -    ccache and allows one to manipulate it. Most useful is the
> -    extraction of ticket entries (cred's) in the ccache and the
> -    ability to examine their attributes.
> +    Obtains GSSAPI credentials with given principal name from ccache. When no
> +    principal name specified, it retrieves the default one for given
> +    credentials cache.
> +
> +    :parameters:
> +      name
> +        gssapi.Name object specifying principal or None for the default
> +      ccache_name
> +        string specifying Kerberos credentials cache name or None for the
> +        default
> +    :returns:
> +      gssapi.Credentials object
> +    ''''
> +    store = None
> +    if ccache_name:
> +        store = {'ccache': ccache_name}
> +    try:
> +        return gssapi.Credentials(usage='initiate', name=name, store=store)
> +    except gssapi.exceptions.GSSError as e:
> +        if e.min_code == KRB5_FCC_NOFILE:
> +            raise ValueError('"%s", ccache="%s"' % (e.message, ccache_name))
> +        raise errors.CCacheError()

This is another case where it stands out that the specific error from
GSSAPI should probably be checked.

> +    # FIXME this is a temporary workaround. We should find some nicer solution
> +    name = gssapi.Name('notempty', gssapi.NameType.user)
> +    can_name = unicode(name.canonicalize(gssapi.MechType.kerberos))
> +    return can_name.partition('@')[2] or None

As mentioned in my email to you, I do not think we can guarantee that
the realm will actually be present after the '@' in all cases, so
hopefully everything copes with a None here.  But if it copes with a
None, why have this code?

> +# 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)
>  
>  
>  class KRBCheater(object):
>      def __init__(self, app):
>          self.app = app
>          self.url = app.url
> -        self.ccname = api.Backend.krb.default_ccname()
> +        self.ccname = get_default_ccache_name()
>      def __call__(self, environ, start_response):
>          environ['KRB5CCNAME'] = self.ccname

If all you're doing is using this to set $KRB5CCNAME (as __call__
suggests), that variable when unset is the default value and the
song-and-dance to get the default ccache name isn't necessary.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 818 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150729/46b2176b/attachment.sig>