koji

#1187 Add ctx option to ClientSession.krb_login()
Merged by mikem. Opened by mprahl.
Unknown source master

Download 1187.patch

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.

+1 Looks good!

@mikem, @tkopecek, or @breilly could you please review?

:thumbsup:

+1

@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 f04346e4 fixes this pull-request

Pull-Request has been merged by mikem
Metadata
None
No Tags
Static archive. pagure.io was sunset in 2026; this is a read-only snapshot.
Red Hat Logo

Fedora is sponsored by Red Hat. Learn more about the relationship between Red Hat and Fedora »

© Red Hat, Inc. and others.