Thread safety issue in krb5_verify_init_creds() when passing ccache pointer to null?

[Greg Hudson]

On 5/28/21 5:14 AM, Thomas Sondergaard wrote:
> It calls krb5_parse_name to create a principal from princ_name and then
> krb5_get_init_creds_passwords to create a krb5_creds. It then calls
> krb5_verify_init_creds(). If that checks out it then calls
> krb5_cc_get_principal to dig the principal out of the returned krb5_ccache
> and then calls krb5_unparse_name and returns that to the caller. Is there
> any situation where this returned principal will be different from the one
> the caller of the function provided?

There isn't.  The returned ccache is initialized with creds->client
(vfy_increds.c:162), so reading its principal will just give that back
again.

>> With what version of the krb5 libraries?  The issue you identified below
>> is a concern, but a crash indicates a problem in the ccache layer.
>> Ticket #8202 (fixed in 1.17) addresses a bug that could explain the crash.
>>
> 
> It is on CentOS 8.3.2011 with krb5-libs-1.18.2-5.el8.x86_64

In that case, if you have the ability to get a stack trace for the
crash, I'd be interested in seeing it.

> krb5_mcc_close(krb5_context context, krb5_ccache id) in
> krb5/src/lib/krb5/ccache/cc_memory.c is the function that is called when
> calling krb5_cc_close on a "MEMORY" type ccache, isn't it?

Yes.

>     count = --d->refcount;
>     k5_cc_mutex_unlock(context, &d->lock);
>     if (count == 0) {
>         /* This is the last active handle referencing d and d has been
> removed
>          * from the table, so we can release it. */

> When I call krb5_cc_close() on the only and therefore last handle to my
> ccache created with krb5_cc_new_unique() I expect the refcount to reach 0
> therefore calling empty_mcc_cache() to free the cache?

No, the table gets its own reference.  So when you create a new memory
ccache handle with krb5_cc_resolve(), it has a refcount of 2 (one for
the table, one for the handle).  If you close it, it has a refcount of 1
(for the table).  Only a krb5_cc_destroy() operation can allow the
refcount to hit 0 and release the cache object.

> So I was a little
> unsure what is leaked. I also looked at krb5_mcc_destroy() in the same file
> and it mentions removing the ccache from a table. Is it this empty table
> entry that is leaked?

It's not empty; the populated cache persists until all references to it
are removed, including the table's.

> So the conclusion is that I should call krb5_cc_destroy() on the ccache
> created with krb5_cc_new_unique() to get rid of it, right?

Yes.  Otherwise the cache still lives in the table and, if anyone
remembers its name, resolvable in order to get at its contents.