Re: Patch to allow GSSAPI authentication to a socks5 server
Date: Fri, 23 Jan 2009 23:30:50 +0100 (CET)
On Tue, 13 Jan 2009, Markus Moeller wrote:
> I think I fixed the points above.
Ok, I edited in a few further changes I think it needed:
- Applied and synced with current CVS.
- I removed all uses of sprintf() and included the correct header for our
internal printf stuff. I think a few places could use aprintf() instead of
malloc() and memcpy()s etc, but...
- I also unified a few things between the two implementations, since I don't
think a lot of code needs to bother about the diffs. Thus they both provide
the same function name now.
- I modded some code to put the string
data->set.str[STRING_SOCKS5_GSSAPI_SERVICE] into a local variable as
repeated accesses to that long name makes the code awkward to read!
(Please verify that everything still works!)
But most importantly what is still left to fix:
* I added checks for the malloc() calls I could find with returns on failure.
These are not really fine since they will leak memory in case of failure.
Adding proper ways to bail out may require use of a few more functions than
* I'm not friends with large stack uses and 64K matches that description imo.
I would like that memory to get allocated. Then the pointer to the buffer
is passed on to the sub function with a pointer only - without a length,
and the Curl_SOCKS5_gssapi_negotiate() implementations just assume
sufficient length. That's asking a little too much for trouble in the
future. I would like to see a length argument being passed in to the
function as well, and the function of course make really sure that it never
writes beyond that boundary.
Would you rather like me to commit the work we have right now and you can base
further patches on that, or do you rather polish the complete patch like this?
-- / daniel.haxx.se
- TEXT/x-diff attachment: socks5-gssapi-3.patch