Rework the code that receives the client request.

Collect all the code in cnt_wait()

Add a new -3 return code from HTC which says that all we've seen so
far is white space.  Use the RFC2616 "LWS" definition of white-space.

Rename "sess_timeout" parameter to "timeout_idle" and describe it more
precisely.  Make it a double, so more precise values can be specified.
The sessions is considered idle even if we receive white-space.

Rename "session_linger" parameter to "timeout_linger", make it a double
and give it units of seconds like other timeouts.

Add new parameter "timeout_req" which controls how long time we give
the client to send the request headers, from the first non-white char.
Set it to two seconds.  Experimentation/Experience requested.

"timeout_req" also gives us positive control of any kind of
"slowlaris" attack scenarios.

I'm trying to group parameters more sensibly, since we sort them
by name, calling them all timeout_*, pool_* etc, makes some sense I hope.

bin/varnishd/cache/cache_acceptor.c

@@ -276,6 +276,7 @@ VCA_SetupSess(struct worker *w)
 	sp->vsl_id = wa->acceptsock | VSL_CLIENTMARKER ;
 	wa->acceptsock = -1;
 	sp->t_open = VTIM_real();
+	sp->t_req = sp->t_open;
 	sp->t_idle = sp->t_open;
 	sp->mylsock = wa->acceptlsock;
 	CHECK_OBJ_NOTNULL(sp->mylsock, LISTEN_SOCK_MAGIC);
@@ -293,7 +294,7 @@ static void *
 vca_acct(void *arg)
 {
 #ifdef SO_RCVTIMEO_WORKS
-	double sess_timeout = 0;
+	double timeout_idle = 0;
 #endif
 #ifdef SO_SNDTIMEO_WORKS
 	double send_timeout = 0;
@@ -333,10 +334,10 @@ vca_acct(void *arg)
 		}
 #endif
 #ifdef SO_RCVTIMEO_WORKS
-		if (cache_param->sess_timeout != sess_timeout) {
+		if (cache_param->timeout_idle != timeout_idle) {
 			need_test = 1;
-			sess_timeout = cache_param->sess_timeout;
-			tv_rcvtimeo = VTIM_timeval(sess_timeout);
+			timeout_idle = cache_param->timeout_idle;
+			tv_rcvtimeo = VTIM_timeval(timeout_idle);
 			VTAILQ_FOREACH(ls, &heritage.socks, list) {
 				if (ls->sock < 0)
 					continue;

bin/varnishd/cache/cache_center.c

@@ -98,9 +98,10 @@ DOT }
 static int
 cnt_wait(struct sess *sp)
 {
-	int i;
+	int i, j, tmo;
 	struct pollfd pfd[1];
 	struct worker *wrk;
+	double now, when;
 
 	CHECK_OBJ_NOTNULL(sp, SESS_MAGIC);
 	wrk = sp->wrk;
@@ -110,33 +111,59 @@ cnt_wait(struct sess *sp)
 	AZ(sp->esi_level);
 	assert(sp->xid == 0);
 
-	i = HTC_Complete(sp->htc);
-	if (i == 0 && cache_param->session_linger > 0) {
+	assert(!isnan(sp->t_req));
+	tmo = (int)(1e3 * cache_param->timeout_linger);
+	while (1) {
 		pfd[0].fd = sp->fd;
 		pfd[0].events = POLLIN;
 		pfd[0].revents = 0;
-		i = poll(pfd, 1, cache_param->session_linger);
-		if (i)
+		j = poll(pfd, 1, tmo);
+		assert(j >= 0);
+		if (j != 0)
 			i = HTC_Rx(sp->htc);
+		else
+			i = HTC_Complete(sp->htc);
+		if (i == -1) {
+			SES_Close(sp, "EOF");
+			break;
+		}
+		if (i == -2) {
+			SES_Close(sp, "overflow");
+			break;
+		}
+		now = VTIM_real();
+		if (i == 1) {
+			/* Got it, run with it */
+			sp->t_req = now;
+			sp->step = STP_START;
+			return (0);
+		}
+		if (i == -3) {
+			/* Nothing but whitespace */
+			when = sp->t_idle + cache_param->timeout_idle;
+			if (when < now) {
+				SES_Close(sp, "timeout");
+				break;
+			}
+			when = sp->t_idle + cache_param->timeout_linger;
+			tmo = (int)(1e3 * (when - now));
+			if (when < now || tmo == 0) {
+				sp->t_req = NAN;
+				wrk->stats.sess_herd++;
+				SES_Charge(sp);
+				WAIT_Enter(sp);
+				return (1);
+			}
+		} else {
+			/* Working on it */
+			when = sp->t_req + cache_param->timeout_req;
+			tmo = (int)(1e3 * (when - now));
+			if (when < now || tmo == 0) {
+				SES_Close(sp, "req timeout");
+				break;
+			}
+		}
 	}
-	if (i == 0) {
-		WSP(sp, SLT_Debug, "herding");
-		wrk->stats.sess_herd++;
-		SES_Charge(sp);
-		WAIT_Enter(sp);
-		return (1);
-	}
-	if (i == 1) {
-		sp->step = STP_START;
-		return (0);
-	}
-	if (i == -2) {
-		SES_Close(sp, "overflow");
-	} else if (i == -1 && Tlen(sp->htc->rxbuf) == 0 &&
-	    (errno == 0 || errno == ECONNRESET))
-		SES_Close(sp, "EOF");
-	else
-		SES_Close(sp, "error");
 	sp->step = STP_DONE;
 	return (0);
 }
@@ -366,8 +393,6 @@ cnt_done(struct sess *sp)
 
 
 	sp->t_idle = W_TIM_real(wrk);
-WSP(sp, SLT_Debug, "PHK req %.9f resp %.9f end %.9f open %.9f",
-    sp->t_req, sp->t_resp, sp->t_idle,  sp->t_open);
 	if (sp->xid == 0) {
 		sp->t_resp = sp->t_idle;
 	} else {
@@ -418,17 +443,20 @@ WSP(sp, SLT_Debug, "PHK req %.9f resp %.9f end %.9f open %.9f",
 	i = HTC_Reinit(sp->htc);
 	if (i == 1) {
 		wrk->stats.sess_pipeline++;
+		sp->t_req = sp->t_idle;
 		sp->step = STP_START;
 		return (0);
 	}
 	if (Tlen(sp->htc->rxbuf)) {
 		wrk->stats.sess_readahead++;
 		sp->step = STP_WAIT;
+		sp->t_req = sp->t_idle;
 		return (0);
 	}
-	if (cache_param->session_linger > 0) {
+	if (cache_param->timeout_linger > 0.) {
 		wrk->stats.sess_linger++;
 		sp->step = STP_WAIT;
+		sp->t_req = sp->t_idle;		// XXX: not quite correct
 		return (0);
 	}
 	wrk->stats.sess_herd++;
@@ -1574,7 +1602,7 @@ cnt_start(struct sess *sp)
 
 	/* Update stats of various sorts */
 	wrk->stats.client_req++;
-	sp->t_req = W_TIM_real(wrk);
+	assert(!isnan(sp->t_req));
 	wrk->acct_tmp.req++;
 
 	/* Assign XID and log */

bin/varnishd/cache/cache_httpconn.c

@@ -50,6 +50,7 @@
  * Check if we have a complete HTTP request or response yet
  *
  * Return values:
+ *	-3  All whitespace so far
  *	 0  No, keep trying
  *	>0  Yes, it is this many bytes long.
  */
@@ -62,13 +63,13 @@ htc_header_complete(txt *t)
 	Tcheck(*t);
 	assert(*t->e == '\0');
 	/* Skip any leading white space */
-	for (p = t->b ; vct_issp(*p); p++)
+	for (p = t->b ; vct_islws(*p); p++)
 		continue;
 	if (p == t->e) {
 		/* All white space */
 		t->e = t->b;
 		*t->e = '\0';
-		return (0);
+		return (-3);
 	}
 	while (1) {
 		p = strchr(p, '\n');
@@ -133,6 +134,8 @@ HTC_Reinit(struct http_conn *htc)
 }
 
 /*--------------------------------------------------------------------
+ * Return -3 if it's all whitespace so far
+ * Return 0 if we need more text
  * Return 1 if we have a complete HTTP procol header
  */
 
@@ -143,9 +146,8 @@ HTC_Complete(struct http_conn *htc)
 
 	CHECK_OBJ_NOTNULL(htc, HTTP_CONN_MAGIC);
 	i = htc_header_complete(&htc->rxbuf);
-	assert(i >= 0);
-	if (i == 0)
-		return (0);
+	if (i <= 0)
+		return (i);
 	WS_ReleaseP(htc->ws, htc->rxbuf.e);
 	AZ(htc->pipeline.b);
 	AZ(htc->pipeline.e);
@@ -160,8 +162,9 @@ HTC_Complete(struct http_conn *htc)
 /*--------------------------------------------------------------------
  * Receive more HTTP protocol bytes
  * Returns:
+ *	-3 all whitespace so far
  *	-2 overflow
- *	-1 error
+ *	-1 error/EOF
  *	 0 more needed
  *	 1 got complete HTTP header
  */

bin/varnishd/common/params.h

@@ -85,8 +85,9 @@ struct params {
 
 	unsigned		shm_reclen;
 
-	/* Acceptor hints */
-	unsigned		sess_timeout;
+	double			timeout_linger;
+	double			timeout_idle;
+	double			timeout_req;
 	unsigned		pipe_timeout;
 	unsigned		send_timeout;
 	unsigned		idle_send_timeout;
@@ -140,9 +141,6 @@ struct params {
 	double			first_byte_timeout;
 	double			between_bytes_timeout;
 
-	/* How long to linger on sessions */
-	unsigned		session_linger;
-
 	/* CLI buffer size */
 	unsigned		cli_buffer;
 

bin/varnishd/mgt/mgt_param.c

@@ -773,12 +773,19 @@ static const struct parspec input_parspec[] = {
 		"cache at the end of ttl+grace+keep.",
 		DELAYED_EFFECT,
 		"0", "seconds" },
-	{ "sess_timeout", tweak_timeout, &mgt_param.sess_timeout, 0, 0,
-		"Idle timeout for persistent sessions. "
-		"If a HTTP request has not been received in this many "
-		"seconds, the session is closed.",
+	{ "timeout_idle", tweak_timeout_double, &mgt_param.timeout_idle,
+		0, UINT_MAX,
+		"Idle timeout for client connections.\n"
+		"A connection is considered idle, until we receive"
+		" a non-white-space character on it.",
 		0,
 		"5", "seconds" },
+	{ "timeout_req", tweak_timeout_double, &mgt_param.timeout_req,
+		0, UINT_MAX,
+		"Max time to receive clients request header, measured"
+		" from first non-white-space character to double CRNL.",
+		0,
+		"2", "seconds" },
 	{ "expiry_sleep", tweak_timeout_double, &mgt_param.expiry_sleep, 0, 60,
 		"How long the expiry thread sleeps when there is nothing "
 		"for it to do.\n",
@@ -993,18 +1000,18 @@ static const struct parspec input_parspec[] = {
 		"it.\n",
 		0,
 		"100000", "sessions" },
-	{ "session_linger", tweak_uint,
-		&mgt_param.session_linger,0, UINT_MAX,
-		"How long time the workerthread lingers on the session "
-		"to see if a new request appears right away.\n"
-		"If sessions are reused, as much as half of all reuses "
+	{ "timeout_linger", tweak_timeout_double, &mgt_param.timeout_linger,
+		0, UINT_MAX,
+		"How long time the workerthread lingers on an idle session "
+		"before handing it over to the waiter.\n"
+		"When sessions are reused, as much as half of all reuses "
 		"happen within the first 100 msec of the previous request "
 		"completing.\n"
 		"Setting this too high results in worker threads not doing "
 		"anything for their keep, setting it too low just means that "
 		"more sessions take a detour around the waiter.",
 		EXPERIMENTAL,
-		"50", "ms" },
+		"0.050", "seconds" },
 	{ "log_hashstring", tweak_bool, &mgt_param.log_hash, 0, 0,
 		"Log the hash string components to shared memory log.\n",
 		0,

bin/varnishd/waiter/cache_waiter_epoll.c

@@ -183,7 +183,7 @@ vwe_thread(void *priv)
 			continue;
 
 		/* check for timeouts */
-		deadline = now - cache_param->sess_timeout;
+		deadline = now - cache_param->timeout_idle;
 		for (;;) {
 			sp = VTAILQ_FIRST(&vwe->sesshead);
 			if (sp == NULL)
@@ -201,13 +201,13 @@ vwe_thread(void *priv)
 /*--------------------------------------------------------------------*/
 
 static void *
-vwe_sess_timeout_ticker(void *priv)
+vwe_timeout_idle_ticker(void *priv)
 {
 	char ticker = 'R';
 	struct vwe *vwe;
 
 	CAST_OBJ_NOTNULL(vwe, priv, VWE_MAGIC);
-	THR_SetName("cache-epoll-sess_timeout_ticker");
+	THR_SetName("cache-epoll-timeout_idle_ticker");
 
 	while (1) {
 		/* ticking */
@@ -255,7 +255,7 @@ vwe_init(void)
 	assert(i != -1);
 
 	AZ(pthread_create(&vwe->timer_thread,
-	    NULL, vwe_sess_timeout_ticker, vwe));
+	    NULL, vwe_timeout_idle_ticker, vwe));
 	AZ(pthread_create(&vwe->epoll_thread, NULL, vwe_thread, vwe));
 	return(vwe);
 }

bin/varnishd/waiter/cache_waiter_kqueue.c

@@ -195,7 +195,7 @@ vwk_thread(void *priv)
 		 * would not know we meant "the old fd of this number".
 		 */
 		vwk_kq_flush(vwk);
-		deadline = now - cache_param->sess_timeout;
+		deadline = now - cache_param->timeout_idle;
 		for (;;) {
 			sp = VTAILQ_FIRST(&vwk->sesshead);
 			if (sp == NULL)

bin/varnishd/waiter/cache_waiter_poll.c

@@ -141,7 +141,7 @@ vwp_main(void *priv)
 		v = poll(vwp->pollfd, vwp->hpoll + 1, 100);
 		assert(v >= 0);
 		now = VTIM_real();
-		deadline = now - cache_param->sess_timeout;
+		deadline = now - cache_param->timeout_idle;
 		v2 = v;
 		VTAILQ_FOREACH_SAFE(sp, &vwp->sesshead, list, sp2) {
 			if (v != 0 && v2 == 0)

bin/varnishd/waiter/cache_waiter_ports.c

@@ -191,7 +191,7 @@ vws_thread(void *priv)
 			vws_port_ev(vws, ev + ei, now);
 
 		/* check for timeouts */
-		deadline = now - cache_param->sess_timeout;
+		deadline = now - cache_param->timeout_idle;
 
 		/*
 		 * This loop assumes that the oldest sessions are always at the
@@ -220,7 +220,7 @@ vws_thread(void *priv)
 
 		if (sp) {
 			double tmo =
-			    (sp->t_open + cache_param->sess_timeout) - now;
+			    (sp->t_open + cache_param->timeout_idle) - now;
 
 			/*
 			 * we should have removed all sps whose timeout

bin/varnishtest/tests/r00262.vtc

@@ -7,7 +7,7 @@ server s1 {
 		-body "012345\n"
 } -start
 
-varnish v1 -arg "-p session_linger=20" -vcl+backend { } -start 
+varnish v1 -arg "-p timeout_linger=20" -vcl+backend { } -start 
 
 client c1 {
 	send "GET / HTTP/1.1\r\n\r\n\r\n"

bin/varnishtest/tests/r00524.vtc

@@ -21,7 +21,7 @@ varnish v1 -vcl+backend {
 	sub vcl_fetch {
 		set beresp.do_esi = true;
 	}
-} -cliok "param.set sess_timeout 60" -start
+} -cliok "param.set timeout_idle 60" -start
 
 client c1 {
 	txreq -proto HTTP/1.0 -hdr "Connection: kEep-alive"

bin/varnishtest/tests/r00641.vtc

@@ -18,7 +18,7 @@ varnish v1 -vcl+backend {
 	sub vcl_fetch {
 		set beresp.do_esi = true;
 	}
-} -cliok "param.set sess_timeout 60" -start
+} -cliok "param.set timeout_idle 60" -start
 
 client c1 {
 	txreq -proto HTTP/1.1