fix #899; check for sticky cookie before passing request to cache

tempesta_fw/http.c

@@ -2610,38 +2610,12 @@ tfw_http_req_cache_service(TfwHttpResp *resp)
 static void
 tfw_http_req_cache_cb(TfwHttpMsg *msg)
 {
-	int r;
 	TfwHttpReq *req = (TfwHttpReq *)msg;
 	TfwSrvConn *srv_conn = NULL;
 	LIST_HEAD(eq);
 
 	TFW_DBG2("%s: req = %p, resp = %p\n", __func__, req, req->resp);
 
-	/*
-	 * Sticky cookie module used for HTTP session identification may send
-	 * a response to the client when sticky cookie presence is enforced
-	 * and the cookie is missing from the request.
-	 *
-	 * HTTP session may be required for request scheduling, so obtain it
-	 * first. However, req->sess still may be NULL if sticky cookies are
-	 * not enabled.
-	 */
-	r = tfw_http_sess_obtain(req);
-	switch (r)
-	{
-	case TFW_HTTP_SESS_SUCCESS:
-		break;
-	case TFW_HTTP_SESS_REDIRECT_SENT:
-		/* Response sent, nothing to do. */
-		return;
-	case TFW_HTTP_SESS_VIOLATE:
-		goto drop_503;
-	case TFW_HTTP_SESS_JS_NOT_SUPPORTED:
-		goto send_503;
-	default:
-		goto send_500;
-	}
-
 	if (req->resp) {
 		tfw_http_req_cache_service(req->resp);
 		return;
@@ -2674,20 +2648,6 @@ tfw_http_req_cache_cb(TfwHttpMsg *msg)
 	tfw_http_req_zap_error(&eq);
 	goto conn_put;
 
-send_503:
-	/*
-	 * Requested resource can't be challenged. Don't break response-request
-	 * queue on client side by dropping the request.
-	 */
-	tfw_http_send_resp(req, 503, "request dropped:"
-			   " can't send JS challenge.");
-	TFW_INC_STAT_BH(clnt.msgs_filtout);
-	return;
-drop_503:
-	tfw_srv_client_drop(req, 503, "request dropped: invalid sticky cookie "
-				      "or js challenge");
-	TFW_INC_STAT_BH(clnt.msgs_filtout);
-	return;
 send_502:
 	tfw_http_send_resp(req, 502, "request dropped: processing error");
 	TFW_INC_STAT_BH(clnt.msgs_otherr);
@@ -3189,6 +3149,48 @@ tfw_http_req_process(TfwConn *conn, const TfwFsmData *data)
 			req->httperr.reason = "cannot find Vhost for request";
 			__HTTP_FSM_JUMP(Http_Msg_Conn_Drop);
 		}
+		/*
+		 * Sticky cookie module used for HTTP session identification
+		 * may send a response to the client when sticky cookie presence
+		 * is enforced and the cookie is missing from the request.
+		 *
+		 * Client can violate sticky cookie, block such client without
+		 * forwarding the request to backend server.
+		 */
+		switch (tfw_http_sess_obtain(req))
+		{
+		case TFW_HTTP_SESS_SUCCESS:
+			break;
+		case TFW_HTTP_SESS_REDIRECT_NEED:
+			/*
+			 * Response is build and stored in @req->resp,
+			 * process it later on forward stage.
+			 */
+			break;
+		case TFW_HTTP_SESS_VIOLATE:
+			TFW_INC_STAT_BH(clnt.msgs_filtout);
+			req->httperr.status = 503;
+			req->httperr.reason =
+				"request dropped: invalid sticky cookie or js challenge";
+			__HTTP_FSM_JUMP(Http_Msg_Conn_Drop);
+		case TFW_HTTP_SESS_JS_NOT_SUPPORTED:
+			/*
+			 * Requested resource can't be challenged. Don't break
+			 * response-request queue on client side by dropping
+			 * the request.
+			 */
+			TFW_INC_STAT_BH(clnt.msgs_filtout);
+			req->httperr.status = 503;
+			req->httperr.reason =
+				"request dropped: can't send JS challenge.";
+			__HTTP_FSM_JUMP(Http_Msg_Conn_Drop);
+		default:
+			TFW_INC_STAT_BH(clnt.msgs_otherr);
+			req->httperr.status = 500;
+			req->httperr.reason =
+				"request dropped: processing error";
+			__HTTP_FSM_JUMP(Http_Msg_Conn_Drop);
+		}
 
 		/*
 		 * Stream mode: message is not ended, but need to be forwarded

tempesta_fw/http_sess.c

@@ -127,7 +127,7 @@ static const unsigned short tfw_cfg_redirect_st_code_dflt = 302;
  * 'Accept: text/html' and GET method.
  */
 static bool
-tfw_http_sticky_redirect_allied(TfwHttpReq *req)
+tfw_http_sticky_redirect_applied(TfwHttpReq *req)
 {
 	if (!tfw_cfg_js_ch)
 		return true;
@@ -137,7 +137,7 @@ tfw_http_sticky_redirect_allied(TfwHttpReq *req)
 }
 
 static int
-tfw_http_sticky_send_redirect(TfwHttpReq *req, StickyVal *sv)
+tfw_http_sticky_build_redirect(TfwHttpReq *req, StickyVal *sv)
 {
 	unsigned long ts_be64 = cpu_to_be64(sv->ts);
 	TfwStr chunks[3], cookie = { 0 };
@@ -155,7 +155,7 @@ tfw_http_sticky_send_redirect(TfwHttpReq *req, StickyVal *sv)
 	 * Non-challengeable requests also must be rate limited.
 	 */
 
-	if (!tfw_http_sticky_redirect_allied(req))
+	if (!tfw_http_sticky_redirect_applied(req))
 		return TFW_HTTP_SESS_JS_NOT_SUPPORTED;
 
 	if (!(resp = tfw_http_msg_alloc_resp_light(req)))
@@ -190,9 +190,12 @@ tfw_http_sticky_send_redirect(TfwHttpReq *req, StickyVal *sv)
 		return TFW_HTTP_SESS_FAILURE;
 	}
 
-	tfw_http_resp_fwd(resp);
-
-	return TFW_HTTP_SESS_REDIRECT_SENT;
+	/*
+	 * Don't send @resp now: cookie check take place on very early @req
+	 * processing stage, store @resp as @req->resp, the response will be
+	 * sent as soon as @req will be fully processed.
+	 */
+	return TFW_HTTP_SESS_REDIRECT_NEED;
 }
 
 static int
@@ -388,7 +391,7 @@ tfw_http_sticky_add(TfwHttpResp *resp)
 		.flags = 3 << TFW_STR_CN_SHIFT
 	};
 
-	/* See comment from tfw_http_sticky_send_redirect(). */
+	/* See comment from tfw_http_sticky_build_redirect(). */
 	bin2hex(buf, &ts_be64, sizeof(ts_be64));
 	bin2hex(&buf[sizeof(ts_be64) * 2], sess->hmac, sizeof(sess->hmac));
 
@@ -429,7 +432,7 @@ tfw_http_sticky_notfound(TfwHttpReq *req)
 	if (tfw_http_sticky_calc(req, &sv) != 0)
 		return TFW_HTTP_SESS_FAILURE;
 
-	return tfw_http_sticky_send_redirect(req, &sv);
+	return tfw_http_sticky_build_redirect(req, &sv);
 }
 
 #define sess_warn(check, addr, fmt, ...)				\
@@ -530,7 +533,7 @@ tfw_http_sticky_req_process(TfwHttpReq *req, StickyVal *sv)
 		 * keep user experience intact.
 		 */
 		if (tfw_http_sticky_verify(req, &cookie_val, sv))
-			return tfw_http_sticky_send_redirect(req, sv);
+			return tfw_http_sticky_build_redirect(req, sv);
 		return TFW_HTTP_SESS_SUCCESS;
 	}
 	TFW_WARN("Multiple Tempesta sticky cookies found: %d\n", r);

tempesta_fw/http_sess.h

@@ -84,7 +84,7 @@ enum {
 	/* Session successfuly obtained. */
 	TFW_HTTP_SESS_SUCCESS = 0,
 	/* Can't obtain session: new client; a redirection message sent. */
-	TFW_HTTP_SESS_REDIRECT_SENT,
+	TFW_HTTP_SESS_REDIRECT_NEED,
 	/* Sticky cookie violated, client must be blocked. */
 	TFW_HTTP_SESS_VIOLATE,
 	/* JS challenge enabled, but request is not challengable. */

tempesta_fw/t/unit/test_http_sticky.c

@@ -266,11 +266,16 @@ TEST(http_sticky, sending_302_without_preparing)
 	TfwConn *c = mock.req->conn;
 
 	/* Cookie is calculated for zero HMAC. */
-	EXPECT_EQ(tfw_http_sticky_send_redirect(mock.req, &sv),
-		  TFW_HTTP_SESS_REDIRECT_SENT);
+	EXPECT_EQ(tfw_http_sticky_build_redirect(mock.req, &sv),
+		  TFW_HTTP_SESS_REDIRECT_NEED);
+	EXPECT_NOT_NULL(mock.req->resp);
+	if (!mock.req->resp)
+		goto err;
+	tfw_http_resp_fwd(mock.req->resp);
 
 	EXPECT_TRUE(mock.tfw_connection_send_was_called);
 
+err:
 	tfw_connection_put(c);
 	mock.req = NULL; /* already freed */
 }
@@ -292,14 +297,19 @@ TEST(http_sticky, sending_302)
 		mock.req->h_tbl->tbl[TFW_HTTP_HDR_HOST] = *hdr1;
 
 		EXPECT_EQ(__sticky_calc(mock.req, &sv), 0);
-		EXPECT_EQ(tfw_http_sticky_send_redirect(mock.req, &sv),
-			  TFW_HTTP_SESS_REDIRECT_SENT);
+		EXPECT_EQ(tfw_http_sticky_build_redirect(mock.req, &sv),
+			  TFW_HTTP_SESS_REDIRECT_NEED);
+		EXPECT_NOT_NULL(mock.req->resp);
+		if (!mock.req->resp)
+			goto err;
+		tfw_http_resp_fwd(mock.req->resp);
 
 		EXPECT_TRUE(mock.tfw_connection_send_was_called);
 		EXPECT_TRUE(mock.seen_set_cookie_header);
 		EXPECT_TRUE(mock.seen_cookie);
 		EXPECT_EQ(mock.http_status, 302);
 
+err:
 		tfw_connection_put(c);
 		mock.req = NULL; /* already freed */
 	}
@@ -492,7 +502,11 @@ TEST(http_sticky, req_no_cookie_enforce)
 
 	append_string_to_msg((TfwHttpMsg *)mock.req, s_req);
 	EXPECT_EQ(http_parse_req_helper(), 0);
-	EXPECT_EQ(tfw_http_sess_obtain(mock.req), TFW_HTTP_SESS_REDIRECT_SENT);
+	EXPECT_EQ(tfw_http_sess_obtain(mock.req), TFW_HTTP_SESS_REDIRECT_NEED);
+	EXPECT_NOT_NULL(mock.req->resp);
+	if (!mock.req->resp)
+		goto err;
+	tfw_http_resp_fwd(mock.req->resp);
 
 	/* in enforce mode, 302 response is sent to a client by Tempesta
 	 * before backend gets anything
@@ -501,6 +515,7 @@ TEST(http_sticky, req_no_cookie_enforce)
 	EXPECT_TRUE(mock.seen_set_cookie_header);
 	EXPECT_TRUE(mock.seen_cookie);
 
+err:
 	tfw_connection_put(c);
 	mock.req = NULL; /* already freed */
 }