Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implementation of the ability to send js challenge according with custom rules. #2131

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 37 additions & 27 deletions fw/http.c
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ static struct {
* here, because it refers to HTTP layer.
*/
unsigned int max_header_list_size = 0;
bool is_jsch_global = true;

#define S_CRLFCRLF "\r\n\r\n"
#define S_HTTP "http://"
Expand Down Expand Up @@ -6029,6 +6030,13 @@ tfw_http_req_process(TfwConn *conn, TfwStream *stream, struct sk_buff *skb,
"request has been filtered out via http table",
HTTP2_ECODE_PROTO);
}
if (res.type == TFW_HTTP_RES_JSCH) {
req->need_jsch = true;
req->vhost = res.vhost;
}
else {
req->need_jsch = is_jsch_global;
}
if (res.type == TFW_HTTP_RES_VHOST) {
req->vhost = res.vhost;
}
Expand Down Expand Up @@ -6134,37 +6142,39 @@ tfw_http_req_process(TfwConn *conn, TfwStream *stream, struct sk_buff *skb,
* to GET. We should send js challenge to the client because the real
* method, expected by the client is GET.
*/
switch (tfw_http_sess_obtain(req)) {
case TFW_HTTP_SESS_SUCCESS:
break;
if (req->need_jsch) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check here is wrong. tfw_http_sess_obtain called not only because of js challenge! see cookie learned. I add new tests for it.

switch (tfw_http_sess_obtain(req)) {
case TFW_HTTP_SESS_SUCCESS:
break;

case TFW_HTTP_SESS_REDIRECT_NEED:
/* Response is built and stored in @req->resp. */
break;
case TFW_HTTP_SESS_REDIRECT_NEED:
/* Response is built and stored in @req->resp. */
break;

case TFW_HTTP_SESS_VIOLATE:
TFW_INC_STAT_BH(clnt.msgs_filtout);
return tfw_http_req_parse_block(req, 403, NULL,
HTTP2_ECODE_PROTO);
case TFW_HTTP_SESS_VIOLATE:
TFW_INC_STAT_BH(clnt.msgs_filtout);
return tfw_http_req_parse_block(req, 403, NULL,
HTTP2_ECODE_PROTO);

case TFW_HTTP_SESS_JS_NOT_SUPPORTED:
/*
* Requested resource can't be challenged, try service it
* from cache.
*/
T_DBG("Can't send JS challenge for request since a "
"non-challengeable resource (e.g. image) was requested");
__set_bit(TFW_HTTP_B_JS_NOT_SUPPORTED, req->flags);
break;
case TFW_HTTP_SESS_JS_NOT_SUPPORTED:
/*
* Requested resource can't be challenged, try service it
* from cache.
*/
T_DBG("Can't send JS challenge for request since a "
"non-challengeable resource (e.g. image) was requested");
__set_bit(TFW_HTTP_B_JS_NOT_SUPPORTED, req->flags);
break;

case TFW_HTTP_SESS_FAILURE:
TFW_INC_STAT_BH(clnt.msgs_otherr);
return tfw_http_req_parse_drop_with_fin(req, 500,
"request dropped: internal error"
" in Sticky module",
HTTP2_ECODE_PROTO);
default:
BUG();
case TFW_HTTP_SESS_FAILURE:
TFW_INC_STAT_BH(clnt.msgs_otherr);
return tfw_http_req_parse_drop_with_fin(req, 500,
"request dropped: internal error"
" in Sticky module",
HTTP2_ECODE_PROTO);
default:
BUG();
}
}

if (TFW_MSG_H2(req))
Expand Down
2 changes: 2 additions & 0 deletions fw/http.h
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,7 @@ struct tfw_http_req_t {
unsigned char method_override;
unsigned int header_list_sz;
unsigned int headers_cnt;
bool need_jsch;
};

#define TFW_IDX_BITS 24
Expand Down Expand Up @@ -722,6 +723,7 @@ typedef void (*tfw_http_cache_cb_t)(TfwHttpMsg *);
(TFW_MSG_H2(hmmsg) ? HTTP2_EXTRA_HDR_OVERHEAD : 0)

extern unsigned int max_header_list_size;
extern bool is_jsch_global;

/* External HTTP functions. */
int tfw_http_msg_process(TfwConn *conn, struct sk_buff *skb,
Expand Down
1 change: 1 addition & 0 deletions fw/http_match.h
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ typedef enum {
TFW_HTTP_MATCH_ACT_BLOCK,
TFW_HTTP_MATCH_ACT_FLAG,
TFW_HTTP_MATCH_ACT_CACHE_TTL,
TFW_HTTP_MATCH_ACT_JSCH,
_TFW_HTTP_MATCH_ACT_COUNT
} tfw_http_rule_act_t;

Expand Down
56 changes: 31 additions & 25 deletions fw/http_sess.c
Original file line number Diff line number Diff line change
Expand Up @@ -588,17 +588,19 @@ tfw_http_sess_resp_process(TfwHttpResp *resp, bool cache)
{
return 0;
}
BUG_ON(!req->sess);

/*
* RFC 6265 4.1.1 and 4.1.2 says that we should not set session cookie
* if it's not necessary. Since client didn't send up the cookie and
* it seems that we don't enforce them, we can just set the cookie in
* each response forwarded to the client.
*/
if (test_bit(TFW_HTTP_B_HAS_STICKY, req->flags))
return 0;
return tfw_http_sticky_add(resp, cache);
if (req->need_jsch) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is called for all types of cookies which are set by tempesta. It is not only about js_challenge, so this check is wrong

BUG_ON(!req->sess);
/*
* RFC 6265 4.1.1 and 4.1.2 says that we should not set session cookie
* if it's not necessary. Since client didn't send up the cookie and
* it seems that we don't enforce them, we can just set the cookie in
* each response forwarded to the client.
*/
if (test_bit(TFW_HTTP_B_HAS_STICKY, req->flags))
return 0;
return tfw_http_sticky_add(resp, cache);
}
return 0;
}

/**
Expand Down Expand Up @@ -666,6 +668,8 @@ tfw_http_sess_check_jsch(StickyVal *sv, TfwHttpReq* req)
{
unsigned long min_time;
TfwCfgJsCh *js_ch = req->vhost->cookie->js_challenge;
if (!req->need_jsch)
Copy link
Contributor

@EvgeniiMekhanik EvgeniiMekhanik Jul 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please combine it with the next check in one if

return 0;

if (!js_ch)
return 0;
Expand Down Expand Up @@ -843,20 +847,22 @@ tfw_http_sess_obtain(TfwHttpReq *req)
* We leave this for administrator decision or more progressive DDoS
* mitigation techniques.
*/
r = tfw_http_sticky_req_process(req, sv, c_val);
switch (r) {
case TFW_HTTP_SESS_SUCCESS:
break;
case TFW_HTTP_SESS_FAILURE:
return r;
default:
/*
* Any js challenge processing error: cookie not found
* or invalid or request comes not in time. We increment
* max_misses and restart js challenge.
*/
BUG_ON(r < __TFW_HTTP_SESS_PUB_CODE_MAX);
return tfw_http_sticky_challenge_start(req);
if (req->need_jsch) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check is wrong here. We get cookies for cookie learn also.

r = tfw_http_sticky_req_process(req, sv, c_val);
switch (r) {
case TFW_HTTP_SESS_SUCCESS:
break;
case TFW_HTTP_SESS_FAILURE:
return r;
default:
/*
* Any js challenge processing error: cookie not found
* or invalid or request comes not in time. We increment
* max_misses and restart js challenge.
*/
BUG_ON(r < __TFW_HTTP_SESS_PUB_CODE_MAX);
return tfw_http_sticky_challenge_start(req);
}
}

if (req->vhost->cookie->learn) {
Expand Down
8 changes: 8 additions & 0 deletions fw/http_tbl.c
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,10 @@ tfw_http_tbl_scan(TfwMsg *msg, TfwHttpTable *table, TfwHttpActionResult *action)
return 0;
case TFW_HTTP_MATCH_ACT_BLOCK:
return -1;
case TFW_HTTP_MATCH_ACT_JSCH:
action->type = TFW_HTTP_RES_JSCH;
action->vhost = tfw_vhost_lookup_default();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its wrong. I add test to check it.
http_chain {{
uri == "*1.html" -> jsch;
host == "good" -> good;
-> default;
}}

For such configuration if host is good and uri == "*1.html" it should go to good vhost not default!

return 0;

default:
action->type = TFW_HTTP_RES_VHOST;
Expand Down Expand Up @@ -499,6 +503,10 @@ tfw_cfgop_http_rule(TfwCfgSpec *cs, TfwCfgEntry *e)
rule->act.type = TFW_HTTP_MATCH_ACT_CACHE_TTL;
rule->act.cache_ttl = act_val_parsed;
}
else if (!strcasecmp(action, "jsch")) {
is_jsch_global = false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is inconsistent behavior - you change global variable only if one jsch is set.
For example:
config = """
http_chain {
host == "aaa" -> jsch;
host == "bbb" -> good;
}

If host == "aaa" ->jsch is set we don't support js challenge for "bbb". But if it is not set we support it! Why? How host == "aaa" -> jsch; deal with host "bbb"?

rule->act.type = TFW_HTTP_MATCH_ACT_JSCH;
}
else if (action && action_val &&
!tfw_cfg_parse_uint(action, &rule->act.redir.resp_code))
{
Expand Down
1 change: 1 addition & 0 deletions fw/http_tbl.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@

#define TFW_HTTP_RES_VHOST 0
#define TFW_HTTP_RES_REDIR 1
#define TFW_HTTP_RES_JSCH 2

typedef struct {
TfwStr url;
Expand Down