[PATCH] httpd: fix username verification with md5 auth

[Peter Korsgaard]

>>>>> "Denys" == Denys Vlasenko <vda.linux at googlemail.com> writes:

[resend with proper from:]

Hi,
 >> /* path match found.  Check request */
 >> /* for check next /path:user:password */
 >> prev = p0;
 >> -			u = strchr(request, ':');
 >> -			if (u == NULL) {
 >> -				/* bad request, ':' required */
 >> -				break;
 >> -			}

 Denys> I don't see what ensures ':' is always there.

Ahh, sorry - it's only checked in the MD5 auth case, seems like I
didn't test that good enough.

We'll need to push that check out of the MD5 case, and error out like
before.

 >> -					char *encrypted = pw_encrypt(u+1, ++pp, 1);
 >> -					int r = strcmp(encrypted, pp);
 >> +				 && pp[3] == '$' && pp[4]) {
 >> +					char *encrypted;
 >> +					int r, len;
 >> +
 >> +					len = 1 + pp - p;
 >> +					if (strncmp(p, request, len) != 0) {
 >> +						/* user doesn't match */
 >> +						continue;
 >> +					}
 >> +
 >> +					encrypted = pw_encrypt(request+len, p+len, 1);
 >> +					r = strcmp(encrypted, p+len);

 Denys> I spent some ten minutes understanding what is done here.

Yeah, it isn't the cleanest code ;) For a simple cleartext password it
just strcmp's what's provided by the client and what's in the conf
file, and for the md5 auth case, it strcmp's the username, encrypts
the client password and compares the hashes.

 Denys> Here where request without ':' will bite us.

 Denys> BTW: any idea why this routine locks on first found path?
 Denys> This code part:

Sorry, I don't.

 Denys> I applied your patch, and then did heavy editing on top -
 Denys> variable names, comments, etc in config file parsing were
 Denys> atrocious.

I'll take a look and test.

-- 
Bye, Peter Korsgaard