-
Notifications
You must be signed in to change notification settings - Fork 104
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||
} | ||
|
||
/** | ||
|
@@ -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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||
|
@@ -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) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Its wrong. I add test to check it. 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; | ||
|
@@ -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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. 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)) | ||
{ | ||
|
There was a problem hiding this comment.
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.