Authenticating against target server hangs the thread when sending a POST request (that has a non empty body)

[fralken]

Recently I stumbled upon an unusual use case, where there is a remote server that accepts plain http connections (not https) and requires NTLM authentication. Cntlm tries and authenticate but hangs the thread (mainly because it should send the request body twice, but the second time it is not available any more).

By removing this code block...

cntlm/direct.c

Lines 333 to 380 in d6047b6

	if (loop == 1 && data[1]->code == 401 && hlist_subcmp_all(data[1]->headers, "WWW-Authenticate", "NTLM")) {
	/*
	* Server closing the connection after 401?
	* Should never happen.
	*/
	if (hlist_subcmp(data[1]->headers, "Connection", "close")) {
	if (debug)
	printf("Reconnect before WWW auth\n");
	close(sd);
	sd = host_connect(data[0]->hostname, data[0]->port);
	if (sd < 0) {
	tmp = gen_502_page(data[0]->http, "WWW authentication reconnect failed");
	(void) write_wrapper(cd, tmp, strlen(tmp));
	free(tmp);
	rc = (void *)-1;
	goto bailout;
	}
	}
	if (!www_authenticate(*wsocket[0], data[0], data[1], tcreds)) {
	if (debug)
	printf("WWW auth connection error.\n");
	tmp = gen_502_page(data[1]->http, data[1]->errmsg ? data[1]->errmsg : "Error during WWW-Authenticate");
	(void) write_wrapper(cd, tmp, strlen(tmp));
	free(tmp);
	free_rr_data(&data[0]);
	free_rr_data(&data[1]);
	rc = (void *)-1;
	goto bailout;
	} else if (data[1]->code == 401) {
	/*
	* Server giving 401 after auth?
	* Request basic auth
	*/
	tmp = gen_401_page(data[1]->http, data[0]->hostname, data[0]->port);
	(void) write_wrapper(cd, tmp, strlen(tmp));
	free(tmp);
	free_rr_data(&data[0]);
	free_rr_data(&data[1]);
	rc = (void *)-1;
	goto bailout;
	}
	}

...it works because the authentication is delegated to the client.

This use case is very rare because usually connections are in https and cntlm simply tunnels them.

In my opinion this behaviour is out of scope because cntlm should deal with authentications against proxies and not against target servers.

I propose to remove this code block completely. (I guess nobody has ever tested this piece of code).

[jschwartzenberg]

I didn't know there was a feature to authenticate against non-proxy servers.

It would seem best to remove that code indeed. If there is any documentation hinting at authentication against non-proxy servers, it should be removed along with it.

[fralken]

Indeed the original documentation claims that it supports transparent connection to ntlm authenticated servers:

Cntlm integrates TCP/IP port forwarding (HTTP tunneling), SOCKS5 proxy mode, standalone proxy allowing you to browse intranet as well as Internet and to access corporate web servers with NTLM protection.

So authentication against target servers is an advertised feature, and the original author tested a lot.

Cntlm has been tested against various ISA servers, WinGate, NetCache, Squid and Tinyproxy with and without NTLM auth.

I guess the issue is with POST requests, where you need to send the body request twice (the second time after you completed the authentication sequence). For sure it works with GET requests (without a body).

So for now I change the title of this issue. Let's see if there is a way to keep this feature and make it more robust (maybe disabling the authentication for POST requests).

[jschwartzenberg]

So for now I change the title of this issue. Let's see if there is a way to keep this feature and make it more robust (maybe disabling the authentication for POST requests).

It depends on the complexity of the code. HTTP without SSL is disappearing, so at some point this code would not really be used while it does contribute to the complexity of the program.

[fralken]

Yes, a quick fix is to add a test that the request body is empty:

if (loop == 1 && data[1]->code == 401 && hlist_subcmp_all(data[1]->headers, "WWW-Authenticate", "NTLM") && !http_has_body(data[0], NULL)) {

In this case the feature is still working, and does not break with POST requests.

I agree that this requirement is obsolete, probably nowadays there are no more clients not able to deal with ntlm authentication, but since the code is there and a quick fix prevents it from breaking, I think for now we can keep it.