[PATCH] Allow Kerberos CHANGEPW request to fallback to TCP
tstecher at isilon.com
Thu Mar 23 18:58:01 GMT 2006
On Wed, 2006-03-22 at 15:46 -0800, Jeremy Allison wrote:
> Thought you'd want a little feedback... It's good to know that
> even the people who wrote Microsoft's krb5 implementation can't
> write error free krb5 code (JOKE, JOKE, ok ! :-). No one writes
> error free krb5 code - especially me :-) :-).
I think in the broad scope of things that the MS KDC may be doing the
wrong thing - so I count this as 2 Todd bugs, technically.
> There's a memory leak around the krb5_rd_error() call you added.
> The API docs say that the krb5_error struct returned from
> krb5_rd_error() must be freed with krb5_free_error(), which you
> do in the codepath where krberror->e_data.data == NULL, but if
> this isn't the case (can that happen?) then it'll leak memory
> and bleed into the rest of the "successful packet" code. After
> the Coverity static analyzer ripped us a new one over things
> like this I'm a little sensitive on these issues :-).
Good catch - thanks for reviewing this.
The edata is likely completely irrelevant for a KRB_ERROR returned for
the setpassword protocol, but my reading of the RFC is that it is
*possible* to send back errors in this field. I copied this code from
1.4.1 MIT Kerberos, which preserves the edata in case it contains one of
the 7 or 8 possible setpassword / changepassword errors.
It's fairly straightforward to check for this case and return the
correct error without leaking memory. I'll fix that up and resend the
> I think we need a krb5_free_error(), return XXX after this
> section of the code (but what should XXX be there ?). The
> other issue is that this probably won't work with Heimdal, as
> the ERROR_TABLE_BASE_krb5 macro is almost certainly MIT
Hmm... I hadn't considered that - however, the Kerberos Samba code is
already using errors from the MIT codebase's krb5_err.h, right? This
should be roughly equal to Heimdahl's implementation whenever receiving
kerberos errors off the wire. I'll triple check, but other viewpoints
are certainly welcome.
> Do you want me to have a go at fixing this and post it to
> the list for your review or do you want to address this
> yourself ?
I'll have this done tomorrow.
220 W. Mercer St. | Seattle, WA 98119
Isilon's hiring... | www.isilon.com/Careers
More information about the samba-technical