#10 When computing MD5 digest, use the known length of the string

[Mikhail T.]

There are only two places in the current code, where the CMD5::compute() is invoked, and in both places, the length of the string is already known. However, this known value is not passed to the CMD5::compute() and so the function must call strlen() to compute it again.

The patch adds a new function:
void CMD5::compute(const char* input, size_t ilen, unsigned char result[16])
and modifies the two current callers to use it.
The old function is turned into simply a wrapper of the new one -- just in case.

[Mikhail T.]

Actually, if the length is known, even better (cheaper) to use data() instead of c_str() -- to allow the string-implementation to not bother with '\0'-termination of the buffer.

I got to admit, however, that I'm not sure, if this:
CMD5::compute(cookiestr.str().c_str(), cookiestr.str().length(), cookie);
makes sense. I don't know C++ well enough to be sure, there will be no TWO string-objects created (one -- for the first argument, and another -- for the second). Maybe, something like this should be used instead:

const string scookie = cookiestr.str();
CMD5::compute(scookie.data(), scookie.length(), cookie);

Not sure...

[Mikhail T.]

Finally, I'm not sure about this usage of MD5 digests at all. It seems, the code in core.cpp only uses 4 bytes (most common sizeof int) of the 16 bytes of digest computed.

The current implementation, it seems, depends on the sizeof int, which suggests, two hosts with different int-sizes may be unable to communicate...

If 4 bytes is deemed sufficient verification, perhaps the commonly used and computationally cheap crc32 should be used instead of MD5?

[Yunhong Gu]

I should probably have used all 16-byte. I think I was just trying to simplify the impl, while occasional cookie hit may not hurt much - just a dangling socket that will be garbage collected.

[Mikhail T.]

Changing it now will break connections from clients using the older implementation, would it not?

But it is not done for authentication anyway -- perhaps, the right thing would be to simply stop checking it altogether on receive.

The most troubling, I'm afraid, is that two machines with different sizeof(int) may be unable to communicate now, if I'm reading the code right. So, I'd say, change it to always process 4 bytes (the most common sizeof(int)).

[Mikhail T.]

The call to getnameinfo() was incorrect -- the caller of the function is not supposed to guess the socket size, but simply use sa->sa_len instead. Both Linux and FreeBSD man-pages for the function have the following example:

if (getnameinfo(sa, sa->sa_len, hbuf, sizeof(hbuf), sbuf,
    sizeof(sbuf), NI_NUMERICHOST | NI_NUMERICSERV) == 0)

The current call causes getnameinfo() to fail (at least, on FreeBSD).

In this new patch I modify core.cpp to:
1. Call getnameinfo() properly.
2. Report the failure of getnameinfo(), if any. (The call to warnx() would only work on Linux and BSD systems. Windows and older Unixes -- like Solaris -- wound need to use an fprintf() or something else.)
3. Use C-style string-manipulations for constructing cookie and its MD5.

[Yunhong Gu]

sa_len is not available on many platforms. Perhaps I can write a helper function to get the sockaddr length.