[krbdev.mit.edu #1525] API (inadvertant?) change in krb5_get_in_tkt_with_password leads to memory leaks

[Ezra Peisach via RT]

There has been an undocumented API change in 1.3 with
krb5_get_in_tkt_with_password.

This function has the unfortunate API of using a krb5_creds * for both
input and output. In particular the creds->server and client are used on
input.

This function was recently changed to invoke krb5_get_init_creds.
In get_in_tkt.c about line 1040 there is the following comment and code:

    /* XXX this should be inside stash_as_reply, but as long as
       get_in_tkt is still around using that arg as an in/out, I can't
       do that */
    memset(creds, 0, sizeof(*creds));

    if ((ret = stash_as_reply(context, time_now, &request, local_as_reply,
			      creds, NULL)))

stash_as_reply will copy over the client and server creds - if they are
not set (which is okay).

It is the memset that can lead to memory leaks - as it clears the
pointers to the client and server.

What to do to fix this?
a) Don't do the memset....
b) memset all but the client and server
c) Modify krb5_get_in_tkt_with_password and krb5_get_in_tkt_with_keytab
(and possibly others) to save a pointer to the client and server
portions so they can krb5_free_principal on return from init_creds and
then restore the pointers.
d) Change krb5_get_in_tkt_with_password and krb5_get_in_tkt_with_keytab
back to not use krb5_get_init_creds.


In terms of code reuse - I think  (c) might be the best option as
changes (a) and (b) would make a change in krb5_get_init_creds from the
1.2 series....

(for those wanting to test the code bug - tests/hammer/kdc5_hammer
currently leaks memory in this fashion - when it did not in the past)

I believe this needs to be fixed before 1.3 goes out.

	  Ezra