On Fri, Jul 03, 2015 at 02:12:34PM -0400, Simo Sorce wrote:
On Fri, 2015-07-03 at 11:59 +0200, Jakub Hrozek wrote:
> On Fri, Jul 03, 2015 at 11:54:46AM +0200, Jakub Hrozek wrote:
> > Hi,
> >
> > the attached patches fix
https://fedorahosted.org/sssd/ticket/2701
> >
> > The first patch just adds a common function instead of copying the same
> > pattern again to the new test.
> >
> > The second adds a new request krb5_auth_queue_send() that wraps
> > krb5_auth_send() and also uses the Kerberos authentication queue. I hope
> > the unit tests cover a lot of use-cases, if not, please suggest more!
> >
> > btw I was thinking that the chaining might not always be necessary if
> > the ccache is of type MEMORY and I hope that the serializaton wouldn't
> > be perceived as performance regression for users. Shall we say that
> > Pavel's cached auth patches are a more systematic solution that
doesn't
> > rely on properties of the ccache type in that case?
>
> I'm sorry, but CI fails on Debian because of wrong linking with
> libraries. I'm already testing a fix. Review of the rest is appreciated
> :-)
If we are serializing all authentications then a busy server would be
swamped. I do not see a per-user/per-cache queue so I would tentatively
NACK the approach sorry.
Thanks for the feedback!
I had my doubts about serialization by default as well, but I would like
to note that serialization had been the default in the krb5 responder,
just not IPA or AD responder since 2010...
Please note I'm not bashing the existing code, I think the serialization
is the only good approach for old platforms that use FILE: ccache by
default..which I guess is why we used it.
If clobbering is the only issues
we have then what we can do is to
always use a custom memory ccache for each auth attempt and then use a
locking mechanism to copy from the memory ccache to the actual ccache.
This is an issue on old platforms like RHEL-6 where FILE is the only
(supported) ccache. So memory cache might not be available..
For better or worse, these platforms are used in critical environments
and high-load servers and are hitting the bug.
But we should really do either per-ccache queuing
Can you elaborate a bit? Are you suggesting to use ccache as the key in
the hash table? I would have to think about a bit, but so far it seems
like a bit of an overkill, because even now if the same ccache was used
for different users we would clobber it anyway I think.
(maybe not per user as
in pathological cases we may have the same ccache for different users ?)
or use memory ccaches and copy them with locking
Yes, but memory ccache is not available on all platforms..
, but fully serializing
all authentications is not really a solution, when a full auth may
require multiple network roundtrips.
That's why I was suggesting using the cached auth feature as a remedy.
But I do see your point. Would serializing only for FILE: ccache be an
accebtable option for you? Something like:
def krb5_queue_auth_send()
if ccache_type == FILE:
queue()
else:
send_auth()
Then we could document for admins that on busy servers that are
unfortunate to only support FILE ccache, use cached auth. If you're
running on a recent platform (RHEL-7+), though, serialization is
not used.