#1187 Add ctx option to ClientSession.krb_login()
Merged 3 years ago by mikem. Opened 3 years ago by mprahl.
Unknown source krb_login-thread-safe  into  master

file modified
+6 -4
@@ -2107,7 +2107,7 @@

          sinfo = self.callMethod('subsession')

          return type(self)(self.baseurl, self.opts, sinfo)

  

-     def krb_login(self, principal=None, keytab=None, ccache=None, proxyuser=None):

+     def krb_login(self, principal=None, keytab=None, ccache=None, proxyuser=None, ctx=None):

          """Log in using Kerberos.  If principal is not None and keytab is

          not None, then get credentials for the given principal from the given keytab.

          If both are None, authenticate using existing local credentials (as obtained
@@ -2115,7 +2115,8 @@

          not specified, the default ccache will be used.  If proxyuser is specified,

          log in the given user instead of the user associated with the Kerberos

          principal.  The principal must be in the "ProxyPrincipals" list on

-         the server side."""

+         the server side.  ctx is the Kerberos context to use, and should be unique

+         per thread.  If ctx is not specified, the default context is used."""

  

          try:

              # Silently try GSSAPI first
@@ -2134,10 +2135,11 @@

                  "Please install python-krbV to use kerberos."

              )

  

-         ctx = krbV.default_context()

+         if not ctx:

+             ctx = krbV.default_context()

  

          if ccache != None:

-             ccache = krbV.CCache(name='FILE:' + ccache, context=ctx)

+             ccache = krbV.CCache(name=ccache, context=ctx)

          else:

              ccache = ctx.default_ccache()

  

The first commit adds a ctx kwarg to krb_login. Before this change, koji.ClientSession.krb_login always used the default context. This can be an issue when a multi-threaded application shares this context and the Kerberos cache is stored in the thread keyring.

In this scenario, the first thread to run krb_login will succeed while all others will get a "Permission denied" error. By adding the ctx kwarg, a thread can establish a context and tell krb_login to use it instead of the default context.

The second commit makes it so that you can pass in a non-file ccache location to krb_login. This is useful because a multi-threaded application may choose to store the Kerberos cache in the thread keyring [1] to avoid Kerberos cache corruption. Since krb_login prepends the passed in ccache with FILE:, the application must resort to setting the KRB_CCACHE environment variable. This is annoying and unnecessary because ccache defaults to FILE anyways if no Kerberos cache type is specified in the value for ccache [2].

1 - http://man7.org/linux/man-pages/man7/thread-keyring.7.html
2 - https://web.mit.edu/kerberos/krb5-1.12/doc/basic/ccache_def.html#ccache-types

These issues were encountered in the Module Build Service (MBS), and this is currently hot-fixed in our product deployment to resolve our issue.

@mikeb @mikem @tkopecek does this needs anything else before it can be merged?

+1 @mikem Is there some reason to have FILE hardcoded?

Is there some reason to have FILE hardcoded?

@mikeb do you recall?

Is there some reason to have FILE hardcoded?

@mikeb do you recall?

@tkopecek @mikem Yes, me being pedantic. I hadn't considered the possibility of using a different ccache type. The Kerberos docs say that FILE: is the default if no type is specified, so this change is completely backward-compatible.

https://web.mit.edu/kerberos/krb5-latest/doc/basic/ccache_def.html

Confirmed, works for me.

$ kdestroy
$ kinit -c /tmp/FOOBAR mikem
...
>>> session.krb_login(ccache='/tmp/FOOBAR')

I've edited the title because I think "thread-safe" is a bit far. This change makes it possible to use krb_login in a multi-threaded environment, but requires the client to do part of the work and still defaults to the old behavior.

Commit f04346e fixes this pull-request

Pull-Request has been merged by mikem

3 years ago