memleak in gss_add_cred_with_password in krb 1.12.1 and 1.13.1

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

memleak in gss_add_cred_with_password in krb 1.12.1 and 1.13.1

Sorin Manolache
Hello,

I think I've found a memory leak in gss_add_cred_with_password, in krb5
1.12.1 and 1.13.1.

The gss_OID_set target_mechs in gss_add_cred_with_password
(lib/gssapi/mechglue/g_acquire_cred_with_pw.c) is not released if the
function returns GSS_S_COMPLETE.

Sorin
_______________________________________________
krbdev mailing list             [hidden email]
https://mailman.mit.edu/mailman/listinfo/krbdev
Reply | Threaded
Open this post in threaded view
|

Re: memleak in gss_add_cred_with_password in krb 1.12.1 and 1.13.1

Greg Hudson
On 06/18/2015 12:13 PM, Sorin Manolache wrote:
> I think I've found a memory leak in gss_add_cred_with_password, in krb5
> 1.12.1 and 1.13.1.
>
> The gss_OID_set target_mechs in gss_add_cred_with_password
> (lib/gssapi/mechglue/g_acquire_cred_with_pw.c) is not released if the
> function returns GSS_S_COMPLETE.

Thanks; I have filed a pull request.  This should be fixed in 1.13.3 and
probably also a 1.12.x patch release.

Be aware that we are planning to change the behavior of
gss_acquire_cred_with_password in 1.14 as discussed here:

    http://krbdev.mit.edu/rt/Ticket/Display.html?id=8152
_______________________________________________
krbdev mailing list             [hidden email]
https://mailman.mit.edu/mailman/listinfo/krbdev
Reply | Threaded
Open this post in threaded view
|

Ticket #8152 gss_acquire_cred_with_password() ignores expired creds

Sorin Manolache
On 2015-06-19 23:35, Greg Hudson wrote:

> On 06/18/2015 12:13 PM, Sorin Manolache wrote:
>> I think I've found a memory leak in gss_add_cred_with_password, in krb5
>> 1.12.1 and 1.13.1.
>>
>> The gss_OID_set target_mechs in gss_add_cred_with_password
>> (lib/gssapi/mechglue/g_acquire_cred_with_pw.c) is not released if the
>> function returns GSS_S_COMPLETE.
>
> Thanks; I have filed a pull request.  This should be fixed in 1.13.3 and
> probably also a 1.12.x patch release.
>
> Be aware that we are planning to change the behavior of
> gss_acquire_cred_with_password in 1.14 as discussed here:
>
>      http://krbdev.mit.edu/rt/Ticket/Display.html?id=8152
>

Hello,

Thank you for the information. However I didn't get how you intend to
change the behaviour.

The ticket mentions checking with Heimdal. Here's what I could
understand from the Heimdal code:

It traverses all caches (krb5_cc_cache_match) in order to match the
principal.
    if found => link the found cache to the cred
If not found => checks if the principal of the dflt cache matches
    if not => fetch creds from KDC, create a new unique memory cache,
store the creds there, link the new cache to the gss_cred structure. The
destroy_cache_on_release flag is set on the gss_cred structure.
    if yes => link the dflt cache to the cred.

So newly fetched creds are not stored in the default cache. They are
stored in a new memory cache that is destroyed when the gss_cred_id_t is
released.

Because the newly created cache is destroyed when the gss_cred is
released, a new invocation of gss_acquire_cred_with_password will fetch
the credentials again from the KDC.

One would benefit of the cache only if the principal of the default
cache matches the principal of the gss_acquire_cred_with_password.

Heimdal does not fetch new credentials from the KDC when the cache
contains expired credentials.

Regards,
Sorin
_______________________________________________
krbdev mailing list             [hidden email]
https://mailman.mit.edu/mailman/listinfo/krbdev
Reply | Threaded
Open this post in threaded view
|

Re: Ticket #8152 gss_acquire_cred_with_password() ignores expired creds

Benjamin Kaduk-2
On Sat, 20 Jun 2015, Sorin Manolache wrote:

> Thank you for the information. However I didn't get how you intend to
> change the behaviour.
>
> The ticket mentions checking with Heimdal. Here's what I could
> understand from the Heimdal code:
>
> It traverses all caches (krb5_cc_cache_match) in order to match the
> principal.
>     if found => link the found cache to the cred
> If not found => checks if the principal of the dflt cache matches
>     if not => fetch creds from KDC, create a new unique memory cache,
> store the creds there, link the new cache to the gss_cred structure. The
> destroy_cache_on_release flag is set on the gss_cred structure.
>     if yes => link the dflt cache to the cred.
>
> So newly fetched creds are not stored in the default cache. They are
> stored in a new memory cache that is destroyed when the gss_cred_id_t is
> released.
>
> Because the newly created cache is destroyed when the gss_cred is
> released, a new invocation of gss_acquire_cred_with_password will fetch
> the credentials again from the KDC.
>
> One would benefit of the cache only if the principal of the default
> cache matches the principal of the gss_acquire_cred_with_password.
>
> Heimdal does not fetch new credentials from the KDC when the cache
> contains expired credentials.

The proposed change will result in gss_acquire_cred_with_password() never
using cached credentials, and always fetching new credentials from the
KDC.  The motivation is that the routine can be used to implement
password-checking functionality, and using cached credentials could allow
an incorrect password to be erroneously accepted.

-Ben
_______________________________________________
krbdev mailing list             [hidden email]
https://mailman.mit.edu/mailman/listinfo/krbdev
Reply | Threaded
Open this post in threaded view
|

Re: Ticket #8152 gss_acquire_cred_with_password() ignores expired creds

Greg Hudson
In reply to this post by Sorin Manolache
On 06/20/2015 01:38 PM, Sorin Manolache wrote:
> Thank you for the information. However I didn't get how you intend to
> change the behaviour.

Sorry if the ticket wasn't clear.  As Ben explained, credentials will be
fetched into a unique memory ccache.  The idea is that if you want to
interact with the shared cache, you can use gss_acquire_cred()
beforehand and gss_store_cred() afterwards.  This new behavior matches
the original behavior of the function when it was introduced in Solaris;
I misguidedly changed it when we first introduced the function into MIT
krb5.

> The ticket mentions checking with Heimdal. Here's what I could
> understand from the Heimdal code:

Heimdal is also changing its behavior:


https://github.com/heimdal/heimdal/commit/db2ba88384dbf79cfeda339d9b6f8c1cc9032871
_______________________________________________
krbdev mailing list             [hidden email]
https://mailman.mit.edu/mailman/listinfo/krbdev