From: Christopher Faulet Date: Wed, 18 Dec 2019 08:20:16 +0000 (+0100) Subject: MINOR: http-rule/tcp-rules: Make track-sc* custom actions X-Git-Tag: v2.2-dev1~36 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ac98d81f4;p=thirdparty%2Fhaproxy.git MINOR: http-rule/tcp-rules: Make track-sc* custom actions Now, these actions use their own dedicated function and are no longer handled "in place" during the TCP/HTTP rules evaluation. Thus the action names ACT_ACTION_TRK_SC0 and ACT_ACTION_TRK_SCMAX are removed. The action type is now the tracking index. Thus the function trk_idx() is no longer needed. --- diff --git a/include/proto/action.h b/include/proto/action.h index d0b2c5d2a2..b7339f4e8b 100644 --- a/include/proto/action.h +++ b/include/proto/action.h @@ -72,15 +72,7 @@ static inline void action_build_list(struct list *keywords, *p = '\0'; } -/* for an action ACT_ACTION_TRK_SC*, return a tracking index starting at zero - * for SC0. Unknown actions also return zero. - */ -static inline int trk_idx(int trk_action) -{ - return trk_action - ACT_ACTION_TRK_SC0; -} - -/* Find and check the target table used by an action ACT_ACTION_TRK_*. This +/* Find and check the target table used by an action track-sc*. This * function should be called during the configuration validity check. * * The function returns 1 in success case, otherwise, it returns 0 and err is diff --git a/include/types/action.h b/include/types/action.h index 0016796b8b..1077ec967c 100644 --- a/include/types/action.h +++ b/include/types/action.h @@ -94,11 +94,6 @@ enum act_name { ACT_TCP_EXPECT_CIP, ACT_TCP_CLOSE, /* close at the sender's */ ACT_TCP_CAPTURE, /* capture a fetched sample */ - - /* track stick counters */ - ACT_ACTION_TRK_SC0, - /* SC1, SC2, ... SCn */ - ACT_ACTION_TRK_SCMAX = ACT_ACTION_TRK_SC0 + MAX_SESS_STKCTR - 1, }; /* NOTE: if <.action_ptr> is defined, the referenced function will always be diff --git a/src/action.c b/src/action.c index 76842022c8..544d3e4680 100644 --- a/src/action.c +++ b/src/action.c @@ -16,13 +16,14 @@ #include #include +#include #include #include #include #include -/* Find and check the target table used by an action ACT_ACTION_TRK_*. This +/* Find and check the target table used by an action track-sc*. This * function should be called during the configuration validity check. * * The function returns 1 in success case, otherwise, it returns 0 and err is @@ -40,19 +41,19 @@ int check_trk_action(struct act_rule *rule, struct proxy *px, char **err) if (!target) { memprintf(err, "unable to find table '%s' referenced by track-sc%d", rule->arg.trk_ctr.table.n ? rule->arg.trk_ctr.table.n : px->id, - trk_idx(rule->action)); + rule->action); return 0; } if (!stktable_compatible_sample(rule->arg.trk_ctr.expr, target->type)) { memprintf(err, "stick-table '%s' uses a type incompatible with the 'track-sc%d' rule", rule->arg.trk_ctr.table.n ? rule->arg.trk_ctr.table.n : px->id, - trk_idx(rule->action)); + rule->action); return 0; } else if (target->proxy && (px->bind_proc & ~target->proxy->bind_proc)) { memprintf(err, "stick-table '%s' referenced by 'track-sc%d' rule not present on all processes covered by proxy '%s'", - target->id, trk_idx(rule->action), px->id); + target->id, rule->action, px->id); return 0; } else { @@ -67,6 +68,16 @@ int check_trk_action(struct act_rule *rule, struct proxy *px, char **err) * right here using stktable_alloc_data_type(). */ } + + if (rule->from == ACT_F_TCP_REQ_CNT && (px->cap & PR_CAP_FE) && !px->tcp_req.inspect_delay && + !(rule->arg.trk_ctr.expr->fetch->val & SMP_VAL_FE_SES_ACC)) { + ha_warning("config : %s '%s' : a 'tcp-request content track-sc*' rule explicitly depending on request" + " contents without any 'tcp-request inspect-delay' setting." + " This means that this rule will randomly find its contents. This can be fixed by" + " setting the tcp-request inspect-delay.\n", + proxy_type_str(px), px->id); + } + return 1; } diff --git a/src/cfgparse.c b/src/cfgparse.c index fdc19f4171..3ff8a8d353 100644 --- a/src/cfgparse.c +++ b/src/cfgparse.c @@ -3489,9 +3489,6 @@ out_uri_auth_compat: if (arule->action == ACT_TCP_CAPTURE && !(arule->arg.cap.expr->fetch->val & SMP_VAL_FE_SES_ACC)) break; - if ((arule->action >= ACT_ACTION_TRK_SC0 && arule->action <= ACT_ACTION_TRK_SCMAX) && - !(arule->arg.trk_ctr.expr->fetch->val & SMP_VAL_FE_SES_ACC)) - break; } if (&arule->list != &curproxy->tcp_req.inspect_rules) { diff --git a/src/http_act.c b/src/http_act.c index 60b65c4f0f..986e542166 100644 --- a/src/http_act.c +++ b/src/http_act.c @@ -1447,6 +1447,75 @@ static enum act_parse_ret parse_http_set_map(const char **args, int *orig_arg, s return ACT_RET_PRS_OK; } +/* This function executes a track-sc* actions. On success, it returns + * ACT_RET_CONT. Otherwsize ACT_RET_ERR is returned. + */ +static enum act_return http_action_track_sc(struct act_rule *rule, struct proxy *px, + struct session *sess, struct stream *s, int flags) +{ + struct stktable *t; + struct stksess *ts; + struct stktable_key *key; + void *ptr1, *ptr2, *ptr3, *ptr4; + int opt; + + ptr1 = ptr2 = ptr3 = ptr4 = NULL; + opt = ((rule->from == ACT_F_HTTP_REQ) ? SMP_OPT_DIR_REQ : SMP_OPT_DIR_RES) | SMP_OPT_FINAL; + + t = rule->arg.trk_ctr.table.t; + key = stktable_fetch_key(t, s->be, sess, s, opt, rule->arg.trk_ctr.expr, NULL); + + if (!key) + goto end; + ts = stktable_get_entry(t, key); + if (!ts) + goto end; + + stream_track_stkctr(&s->stkctr[rule->action], t, ts); + + /* let's count a new HTTP request as it's the first time we do it */ + ptr1 = stktable_data_ptr(t, ts, STKTABLE_DT_HTTP_REQ_CNT); + ptr2 = stktable_data_ptr(t, ts, STKTABLE_DT_HTTP_REQ_RATE); + + /* When the client triggers a 4xx from the server, it's most often due + * to a missing object or permission. These events should be tracked + * because if they happen often, it may indicate a brute force or a + * vulnerability scan. Normally this is done when receiving the response + * but here we're tracking after this ought to have been done so we have + * to do it on purpose. + */ + if (rule->from == ACT_F_HTTP_RES && (unsigned)(s->txn->status - 400) < 100) { + ptr3 = stktable_data_ptr(t, ts, STKTABLE_DT_HTTP_ERR_CNT); + ptr4 = stktable_data_ptr(t, ts, STKTABLE_DT_HTTP_ERR_RATE); + } + + if (ptr1 || ptr2 || ptr3 || ptr4) { + HA_RWLOCK_WRLOCK(STK_SESS_LOCK, &ts->lock); + + if (ptr1) + stktable_data_cast(ptr1, http_req_cnt)++; + if (ptr2) + update_freq_ctr_period(&stktable_data_cast(ptr2, http_req_rate), + t->data_arg[STKTABLE_DT_HTTP_REQ_RATE].u, 1); + if (ptr3) + stktable_data_cast(ptr3, http_err_cnt)++; + if (ptr4) + update_freq_ctr_period(&stktable_data_cast(ptr4, http_err_rate), + t->data_arg[STKTABLE_DT_HTTP_ERR_RATE].u, 1); + + HA_RWLOCK_WRUNLOCK(STK_SESS_LOCK, &ts->lock); + + /* If data was modified, we need to touch to re-schedule sync */ + stktable_touch_local(t, ts, 0); + } + + stkctr_set_flags(&s->stkctr[rule->action], STKCTR_TRACK_CONTENT); + if (sess->fe != s->be) + stkctr_set_flags(&s->stkctr[rule->action], STKCTR_TRACK_BACKEND); + + end: + return ACT_RET_CONT; +} /* Parse a "track-sc*" actions. It returns ACT_RET_PRS_OK on success, * ACT_RET_PRS_ERR on error. @@ -1494,8 +1563,9 @@ static enum act_parse_ret parse_http_track_sc(const char **args, int *orig_arg, cur_arg++; } + rule->action = tsc_num; rule->arg.trk_ctr.expr = expr; - rule->action = ACT_ACTION_TRK_SC0 + tsc_num; + rule->action_ptr = http_action_track_sc; rule->check_ptr = check_trk_action; *orig_arg = cur_arg; diff --git a/src/http_ana.c b/src/http_ana.c index d21d19e1fd..e100589a88 100644 --- a/src/http_ana.c +++ b/src/http_ana.c @@ -2959,50 +2959,6 @@ static enum rule_result http_req_get_intercept_rule(struct proxy *px, struct lis http_remove_header(htx, &ctx); break; - case ACT_ACTION_TRK_SC0 ... ACT_ACTION_TRK_SCMAX: - /* Note: only the first valid tracking parameter of each - * applies. - */ - - if (stkctr_entry(&s->stkctr[trk_idx(rule->action)]) == NULL) { - struct stktable *t; - struct stksess *ts; - struct stktable_key *key; - void *ptr1, *ptr2; - - t = rule->arg.trk_ctr.table.t; - key = stktable_fetch_key(t, s->be, sess, s, SMP_OPT_DIR_REQ | SMP_OPT_FINAL, - rule->arg.trk_ctr.expr, NULL); - - if (key && (ts = stktable_get_entry(t, key))) { - stream_track_stkctr(&s->stkctr[trk_idx(rule->action)], t, ts); - - /* let's count a new HTTP request as it's the first time we do it */ - ptr1 = stktable_data_ptr(t, ts, STKTABLE_DT_HTTP_REQ_CNT); - ptr2 = stktable_data_ptr(t, ts, STKTABLE_DT_HTTP_REQ_RATE); - if (ptr1 || ptr2) { - HA_RWLOCK_WRLOCK(STK_SESS_LOCK, &ts->lock); - - if (ptr1) - stktable_data_cast(ptr1, http_req_cnt)++; - - if (ptr2) - update_freq_ctr_period(&stktable_data_cast(ptr2, http_req_rate), - t->data_arg[STKTABLE_DT_HTTP_REQ_RATE].u, 1); - - HA_RWLOCK_WRUNLOCK(STK_SESS_LOCK, &ts->lock); - - /* If data was modified, we need to touch to re-schedule sync */ - stktable_touch_local(t, ts, 0); - } - - stkctr_set_flags(&s->stkctr[trk_idx(rule->action)], STKCTR_TRACK_CONTENT); - if (sess->fe != s->be) - stkctr_set_flags(&s->stkctr[trk_idx(rule->action)], STKCTR_TRACK_BACKEND); - } - } - break; - /* other flags exists, but normally, they never be matched. */ default: break; @@ -3150,65 +3106,6 @@ resume_execution: rule_ret = HTTP_RULE_RES_ERROR; goto end; - case ACT_ACTION_TRK_SC0 ... ACT_ACTION_TRK_SCMAX: - /* Note: only the first valid tracking parameter of each - * applies. - */ - if (stkctr_entry(&s->stkctr[trk_idx(rule->action)]) == NULL) { - struct stktable *t; - struct stksess *ts; - struct stktable_key *key; - void *ptr; - - t = rule->arg.trk_ctr.table.t; - key = stktable_fetch_key(t, s->be, sess, s, SMP_OPT_DIR_RES | SMP_OPT_FINAL, - rule->arg.trk_ctr.expr, NULL); - - if (key && (ts = stktable_get_entry(t, key))) { - stream_track_stkctr(&s->stkctr[trk_idx(rule->action)], t, ts); - - HA_RWLOCK_WRLOCK(STK_SESS_LOCK, &ts->lock); - - /* let's count a new HTTP request as it's the first time we do it */ - ptr = stktable_data_ptr(t, ts, STKTABLE_DT_HTTP_REQ_CNT); - if (ptr) - stktable_data_cast(ptr, http_req_cnt)++; - - ptr = stktable_data_ptr(t, ts, STKTABLE_DT_HTTP_REQ_RATE); - if (ptr) - update_freq_ctr_period(&stktable_data_cast(ptr, http_req_rate), - t->data_arg[STKTABLE_DT_HTTP_REQ_RATE].u, 1); - - /* When the client triggers a 4xx from the server, it's most often due - * to a missing object or permission. These events should be tracked - * because if they happen often, it may indicate a brute force or a - * vulnerability scan. Normally this is done when receiving the response - * but here we're tracking after this ought to have been done so we have - * to do it on purpose. - */ - if ((unsigned)(txn->status - 400) < 100) { - ptr = stktable_data_ptr(t, ts, STKTABLE_DT_HTTP_ERR_CNT); - if (ptr) - stktable_data_cast(ptr, http_err_cnt)++; - - ptr = stktable_data_ptr(t, ts, STKTABLE_DT_HTTP_ERR_RATE); - if (ptr) - update_freq_ctr_period(&stktable_data_cast(ptr, http_err_rate), - t->data_arg[STKTABLE_DT_HTTP_ERR_RATE].u, 1); - } - - HA_RWLOCK_WRUNLOCK(STK_SESS_LOCK, &ts->lock); - - /* If data was modified, we need to touch to re-schedule sync */ - stktable_touch_local(t, ts, 0); - - stkctr_set_flags(&s->stkctr[trk_idx(rule->action)], STKCTR_TRACK_CONTENT); - if (sess->fe != s->be) - stkctr_set_flags(&s->stkctr[trk_idx(rule->action)], STKCTR_TRACK_BACKEND); - } - } - break; - /* other flags exists, but normally, they never be matched. */ default: break; diff --git a/src/tcp_rules.c b/src/tcp_rules.c index fae2e152bc..fbd8768573 100644 --- a/src/tcp_rules.c +++ b/src/tcp_rules.c @@ -101,8 +101,6 @@ int tcp_inspect_request(struct stream *s, struct channel *req, int an_bit) { struct session *sess = s->sess; struct act_rule *rule; - struct stksess *ts; - struct stktable *t; int partial; int act_opts = 0; @@ -187,29 +185,6 @@ resume_execution: else if (rule->action == ACT_ACTION_DENY) { goto deny; } - else if (rule->action >= ACT_ACTION_TRK_SC0 && rule->action <= ACT_ACTION_TRK_SCMAX) { - /* Note: only the first valid tracking parameter of each - * applies. - */ - struct stktable_key *key; - struct sample smp; - - if (stkctr_entry(&s->stkctr[trk_idx(rule->action)])) - continue; - - t = rule->arg.trk_ctr.table.t; - key = stktable_fetch_key(t, s->be, sess, s, SMP_OPT_DIR_REQ | partial, rule->arg.trk_ctr.expr, &smp); - - if ((smp.flags & SMP_F_MAY_CHANGE) && !(partial & SMP_OPT_FINAL)) - goto missing_data; /* key might appear later */ - - if (key && (ts = stktable_get_entry(t, key))) { - stream_track_stkctr(&s->stkctr[trk_idx(rule->action)], t, ts); - stkctr_set_flags(&s->stkctr[trk_idx(rule->action)], STKCTR_TRACK_CONTENT); - if (sess->fe != s->be) - stkctr_set_flags(&s->stkctr[trk_idx(rule->action)], STKCTR_TRACK_BACKEND); - } - } else if (rule->action == ACT_TCP_CAPTURE) { struct sample *key; struct cap_hdr *h = rule->arg.cap.hdr; @@ -464,8 +439,6 @@ resume_execution: int tcp_exec_l4_rules(struct session *sess) { struct act_rule *rule; - struct stksess *ts; - struct stktable *t = NULL; struct connection *conn = objt_conn(sess->origin); int result = 1; enum acl_test_res ret; @@ -521,21 +494,6 @@ int tcp_exec_l4_rules(struct session *sess) result = 0; goto end; } - else if (rule->action >= ACT_ACTION_TRK_SC0 && rule->action <= ACT_ACTION_TRK_SCMAX) { - /* Note: only the first valid tracking parameter of each - * applies. - */ - struct stktable_key *key; - - if (stkctr_entry(&sess->stkctr[trk_idx(rule->action)])) - continue; - - t = rule->arg.trk_ctr.table.t; - key = stktable_fetch_key(t, sess->fe, sess, NULL, SMP_OPT_DIR_REQ|SMP_OPT_FINAL, rule->arg.trk_ctr.expr, NULL); - - if (key && (ts = stktable_get_entry(t, key))) - stream_track_stkctr(&sess->stkctr[trk_idx(rule->action)], t, ts); - } else if (rule->action == ACT_TCP_EXPECT_PX) { if (!(conn->flags & (CO_FL_HANDSHAKE_NOSSL))) { if (xprt_add_hs(conn) < 0) { @@ -570,8 +528,6 @@ int tcp_exec_l4_rules(struct session *sess) int tcp_exec_l5_rules(struct session *sess) { struct act_rule *rule; - struct stksess *ts; - struct stktable *t = NULL; int result = 1; enum acl_test_res ret; @@ -623,21 +579,6 @@ int tcp_exec_l5_rules(struct session *sess) result = 0; goto end; } - else if (rule->action >= ACT_ACTION_TRK_SC0 && rule->action <= ACT_ACTION_TRK_SCMAX) { - /* Note: only the first valid tracking parameter of each - * applies. - */ - struct stktable_key *key; - - if (stkctr_entry(&sess->stkctr[trk_idx(rule->action)])) - continue; - - t = rule->arg.trk_ctr.table.t; - key = stktable_fetch_key(t, sess->fe, sess, NULL, SMP_OPT_DIR_REQ|SMP_OPT_FINAL, rule->arg.trk_ctr.expr, NULL); - - if (key && (ts = stktable_get_entry(t, key))) - stream_track_stkctr(&sess->stkctr[trk_idx(rule->action)], t, ts); - } } } end: @@ -708,6 +649,44 @@ static int tcp_parse_response_rule(char **args, int arg, int section_type, } +/* This function executes a track-sc* actions. On success, it returns + * ACT_RET_CONT. If it must yield, it return ACT_RET_YIELD. Otherwsize + * ACT_RET_ERR is returned. + */ +static enum act_return tcp_action_track_sc(struct act_rule *rule, struct proxy *px, + struct session *sess, struct stream *s, int flags) +{ + struct stksess *ts; + struct stktable *t; + struct stktable_key *key; + struct sample smp; + int opt; + + opt = ((rule->from == ACT_F_TCP_REQ_CNT) ? SMP_OPT_DIR_REQ : SMP_OPT_DIR_RES); + if (flags & ACT_FLAG_FINAL) + opt |= SMP_OPT_FINAL; + + if (stkctr_entry(&s->stkctr[rule->action])) + goto end; + + t = rule->arg.trk_ctr.table.t; + key = stktable_fetch_key(t, s->be, sess, s, opt, rule->arg.trk_ctr.expr, &smp); + + if ((smp.flags & SMP_F_MAY_CHANGE) && !(flags & ACT_FLAG_FINAL)) + return ACT_RET_YIELD; /* key might appear later */ + + if (key && (ts = stktable_get_entry(t, key))) { + stream_track_stkctr(&s->stkctr[rule->action], t, ts); + if (rule->from == ACT_F_TCP_REQ_CNT) { + stkctr_set_flags(&s->stkctr[rule->action], STKCTR_TRACK_CONTENT); + if (sess->fe != s->be) + stkctr_set_flags(&s->stkctr[rule->action], STKCTR_TRACK_BACKEND); + } + } + + end: + return ACT_RET_CONT; +} /* Parse a tcp-request rule. Return a negative value in case of failure */ static int tcp_parse_request_rule(char **args, int arg, int section_type, @@ -864,8 +843,9 @@ static int tcp_parse_request_rule(char **args, int arg, int section_type, rule->arg.trk_ctr.table.n = strdup(args[arg]); arg++; } + rule->action = tsc_num; rule->arg.trk_ctr.expr = expr; - rule->action = ACT_ACTION_TRK_SC0 + tsc_num; + rule->action_ptr = tcp_action_track_sc; rule->check_ptr = check_trk_action; } else if (strcmp(args[arg], "expect-proxy") == 0) {