[PATCH] Allow Kerberos CHANGEPW request to fallback to TCP

todd stecher 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
updated patch.

> 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
> specific.
> 
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.


-- 
Todd Stecher

Isilon Systems 

220 W. Mercer St. | Seattle, WA 98119 
Isilon's hiring...  | www.isilon.com/Careers 


More information about the samba-technical mailing list