diff --git a/include/proto/acl.h b/include/proto/acl.h index 1471dae42..39cbf46af 100644 --- a/include/proto/acl.h +++ b/include/proto/acl.h @@ -97,10 +97,20 @@ struct acl_cond *build_acl_cond(const char *file, int line, struct proxy *px, co */ int acl_exec_cond(struct acl_cond *cond, struct proxy *px, struct session *l4, void *l7, unsigned int opt); -/* Reports a pointer to the first ACL used in condition which requires - * at least one of the USE_FLAGS in . Returns NULL if none matches. +/* Returns a pointer to the first ACL conflicting with usage at place + * which is one of the SMP_VAL_* bits indicating a check place, or NULL if + * no conflict is found. Only full conflicts are detected (ACL is not usable). + * Use the next function to check for useless keywords. */ -struct acl *cond_find_require(const struct acl_cond *cond, unsigned int require); +const struct acl *acl_cond_conflicts(const struct acl_cond *cond, unsigned int where); + +/* Returns a pointer to the first ACL and its first keyword to conflict with + * usage at place which is one of the SMP_VAL_* bits indicating a check + * place. Returns true if a conflict is found, with and set (if non + * null), or false if not conflict is found. The first useless keyword is + * returned. + */ +int acl_cond_kw_conflicts(const struct acl_cond *cond, unsigned int where, struct acl const **acl, struct acl_keyword const **kw); /* * Find targets for userlist and groups in acl. Function returns the number diff --git a/include/proto/sample.h b/include/proto/sample.h index 9dc631abb..a5696f6fb 100644 --- a/include/proto/sample.h +++ b/include/proto/sample.h @@ -35,7 +35,7 @@ struct sample *sample_fetch_string(struct proxy *px, struct session *l4, void *l void sample_register_fetches(struct sample_fetch_kw_list *psl); void sample_register_convs(struct sample_conv_kw_list *psl); const char *sample_src_names(unsigned int use); -const char *sample_src_names(unsigned int use); +const char *sample_ckp_names(unsigned int use); struct sample_fetch *find_sample_fetch(const char *kw, int len); #endif /* _PROTO_SAMPLE_H */ diff --git a/include/types/acl.h b/include/types/acl.h index 68c3d78d7..5e4f84824 100644 --- a/include/types/acl.h +++ b/include/types/acl.h @@ -255,7 +255,8 @@ struct acl { char *name; /* acl name */ struct list expr; /* list of acl_exprs */ int cache_idx; /* ACL index in cache */ - unsigned int requires; /* or'ed bit mask of all acl_expr's ACL_USE_* */ + unsigned int use; /* or'ed bit mask of all acl_expr's SMP_USE_* */ + unsigned int val; /* or'ed bit mask of all acl_expr's SMP_VAL_* */ }; /* the condition will be linked to from an action in a proxy */ @@ -274,7 +275,8 @@ struct acl_cond { struct list list; /* Some specific tests may use multiple conditions */ struct list suites; /* list of acl_term_suites */ int pol; /* polarity: ACL_COND_IF / ACL_COND_UNLESS */ - unsigned int requires; /* or'ed bit mask of all acl's ACL_USE_* */ + unsigned int use; /* or'ed bit mask of all suites's SMP_USE_* */ + unsigned int val; /* or'ed bit mask of all suites's SMP_VAL_* */ const char *file; /* config file where the condition is declared */ int line; /* line in the config file where the condition is declared */ }; diff --git a/src/acl.c b/src/acl.c index 58363fa16..686eadfbb 100644 --- a/src/acl.c +++ b/src/acl.c @@ -1209,7 +1209,12 @@ struct acl *parse_acl(const char **args, struct list *known_acl, char **err) cur_acl->name = name; } - cur_acl->requires |= acl_expr->kw->requires; + /* We want to know what features the ACL needs (typically HTTP parsing), + * and where it may be used. If an ACL relies on multiple matches, it is + * OK if at least one of them may match in the context where it is used. + */ + cur_acl->use |= acl_expr->kw->smp->use; + cur_acl->val |= acl_expr->kw->smp->val; LIST_ADDQ(&cur_acl->expr, &acl_expr->list); return cur_acl; @@ -1294,7 +1299,8 @@ struct acl *find_acl_default(const char *acl_name, struct list *known_acl, char } cur_acl->name = name; - cur_acl->requires |= acl_expr->kw->requires; + cur_acl->use |= acl_expr->kw->smp->use; + cur_acl->val |= acl_expr->kw->smp->val; LIST_INIT(&cur_acl->expr); LIST_ADDQ(&cur_acl->expr, &acl_expr->list); if (known_acl) @@ -1342,6 +1348,7 @@ struct acl_cond *parse_acl_cond(const char **args, struct list *known_acl, int p struct acl_term *cur_term; struct acl_term_suite *cur_suite; struct acl_cond *cond; + unsigned int suite_val; cond = (struct acl_cond *)calloc(1, sizeof(*cond)); if (cond == NULL) { @@ -1352,8 +1359,10 @@ struct acl_cond *parse_acl_cond(const char **args, struct list *known_acl, int p LIST_INIT(&cond->list); LIST_INIT(&cond->suites); cond->pol = pol; + cond->val = 0; cur_suite = NULL; + suite_val = ~0U; neg = 0; for (arg = 0; *args[arg]; arg++) { word = args[arg]; @@ -1372,6 +1381,8 @@ struct acl_cond *parse_acl_cond(const char **args, struct list *known_acl, int p if (strcasecmp(word, "or") == 0 || strcmp(word, "||") == 0) { /* new term suite */ + cond->val |= suite_val; + suite_val = ~0U; cur_suite = NULL; neg = 0; continue; @@ -1408,6 +1419,7 @@ struct acl_cond *parse_acl_cond(const char **args, struct list *known_acl, int p /* note that parse_acl() must have filled here */ goto out_free_suite; } + word = args[arg + 1]; arg = arg_end; } else { @@ -1434,7 +1446,18 @@ struct acl_cond *parse_acl_cond(const char **args, struct list *known_acl, int p cur_term->acl = cur_acl; cur_term->neg = neg; - cond->requires |= cur_acl->requires; + + /* Here it is a bit complex. The acl_term_suite is a conjunction + * of many terms. It may only be used if all of its terms are + * usable at the same time. So the suite's validity domain is an + * AND between all ACL keywords' ones. But, the global condition + * is valid if at least one term suite is OK. So it's an OR between + * all of their validity domains. We could emit a warning as soon + * as suite_val is null because it means that the last ACL is not + * compatible with the previous ones. Let's remain simple for now. + */ + cond->use |= cur_acl->use; + suite_val &= cur_acl->val; if (!cur_suite) { cur_suite = (struct acl_term_suite *)calloc(1, sizeof(*cur_suite)); @@ -1449,6 +1472,7 @@ struct acl_cond *parse_acl_cond(const char **args, struct list *known_acl, int p neg = 0; } + cond->val |= suite_val; return cond; out_free_term: @@ -1498,7 +1522,7 @@ struct acl_cond *build_acl_cond(const char *file, int line, struct proxy *px, co cond->file = file; cond->line = line; - px->http_needed |= !!(cond->requires & ACL_USE_L7_ANY); + px->http_needed |= !!(cond->use & SMP_USE_HTTP_ANY); return cond; } @@ -1638,15 +1662,12 @@ int acl_exec_cond(struct acl_cond *cond, struct proxy *px, struct session *l4, v return cond_res; } - -/* Reports a pointer to the first ACL used in condition which requires - * at least one of the USE_FLAGS in . Returns NULL if none matches. - * The construct is almost the same as for acl_exec_cond() since we're walking - * down the ACL tree as well. It is important that the tree is really walked - * through and never cached, because that way, this function can be used as a - * late check. +/* Returns a pointer to the first ACL conflicting with usage at place + * which is one of the SMP_VAL_* bits indicating a check place, or NULL if + * no conflict is found. Only full conflicts are detected (ACL is not usable). + * Use the next function to check for useless keywords. */ -struct acl *cond_find_require(const struct acl_cond *cond, unsigned int require) +const struct acl *acl_cond_conflicts(const struct acl_cond *cond, unsigned int where) { struct acl_term_suite *suite; struct acl_term *term; @@ -1655,13 +1676,41 @@ struct acl *cond_find_require(const struct acl_cond *cond, unsigned int require) list_for_each_entry(suite, &cond->suites, list) { list_for_each_entry(term, &suite->terms, list) { acl = term->acl; - if (acl->requires & require) + if (!(acl->val & where)) return acl; } } return NULL; } +/* Returns a pointer to the first ACL and its first keyword to conflict with + * usage at place which is one of the SMP_VAL_* bits indicating a check + * place. Returns true if a conflict is found, with and set (if non + * null), or false if not conflict is found. The first useless keyword is + * returned. + */ +int acl_cond_kw_conflicts(const struct acl_cond *cond, unsigned int where, struct acl const **acl, struct acl_keyword const **kw) +{ + struct acl_term_suite *suite; + struct acl_term *term; + struct acl_expr *expr; + + list_for_each_entry(suite, &cond->suites, list) { + list_for_each_entry(term, &suite->terms, list) { + list_for_each_entry(expr, &term->acl->expr, list) { + if (!(expr->kw->smp->val & where)) { + if (acl) + *acl = term->acl; + if (kw) + *kw = expr->kw; + return 1; + } + } + } + } + return 0; +} + /* * Find targets for userlist and groups in acl. Function returns the number * of errors or OK if everything is fine. diff --git a/src/cfgparse.c b/src/cfgparse.c index e277b12b4..bd3382cb8 100644 --- a/src/cfgparse.c +++ b/src/cfgparse.c @@ -414,41 +414,41 @@ int warnif_misplaced_reqadd(struct proxy *proxy, const char *file, int line, con warnif_rule_after_use_backend(proxy, file, line, arg); } -/* Report it if a request ACL condition uses some response-only parameters. It - * returns either 0 or ERR_WARN so that its result can be or'ed with err_code. - * Note that may be NULL and then will be ignored. +/* Report it if a request ACL condition uses some keywords that are incompatible + * with the place where the ACL is used. It returns either 0 or ERR_WARN so that + * its result can be or'ed with err_code. Note that may be NULL and then + * will be ignored. */ -static int warnif_cond_requires_resp(const struct acl_cond *cond, const char *file, int line) +static int warnif_cond_conflicts(const struct acl_cond *cond, unsigned int where, const char *file, int line) { - struct acl *acl; + const struct acl *acl; + const struct acl_keyword *kw; - if (!cond || !(cond->requires & ACL_USE_RTR_ANY)) + if (!cond) return 0; - acl = cond_find_require(cond, ACL_USE_RTR_ANY); - Warning("parsing [%s:%d] : acl '%s' involves some response-only criteria which will be ignored.\n", - file, line, acl ? acl->name : "(unknown)"); - return ERR_WARN; -} - -/* Report it if a request ACL condition uses some request-only volatile parameters. - * It returns either 0 or ERR_WARN so that its result can be or'ed with err_code. - * Note that may be NULL and then will be ignored. - */ -static int warnif_cond_requires_req(const struct acl_cond *cond, const char *file, int line) -{ - struct acl *acl; - - if (!cond || !(cond->requires & ACL_USE_REQ_VOLATILE)) + acl = acl_cond_conflicts(cond, where); + if (acl) { + if (acl->name && *acl->name) + Warning("parsing [%s:%d] : acl '%s' will never match because it only involves keywords that are incompatible with '%s'\n", + file, line, acl->name, sample_ckp_names(where)); + else + Warning("parsing [%s:%d] : anonymous acl will never match because it uses keyword '%s' which is incompatible with '%s'\n", + file, line, LIST_ELEM(acl->expr.n, struct acl_expr *, list)->kw->kw, sample_ckp_names(where)); + return ERR_WARN; + } + if (!acl_cond_kw_conflicts(cond, where, &acl, &kw)) return 0; - acl = cond_find_require(cond, ACL_USE_REQ_VOLATILE); - Warning("parsing [%s:%d] : acl '%s' involves some volatile request-only criteria which will be ignored.\n", - file, line, acl ? acl->name : "(unknown)"); + if (acl->name && *acl->name) + Warning("parsing [%s:%d] : acl '%s' involves keywords '%s' which is incompatible with '%s'\n", + file, line, acl->name, kw->kw, sample_ckp_names(where)); + else + Warning("parsing [%s:%d] : anonymous acl involves keyword '%s' which is incompatible with '%s'\n", + file, line, kw->kw, sample_ckp_names(where)); return ERR_WARN; } - /* * parse a line in a section. Returns the error code, 0 if OK, or * any combination of : @@ -1380,10 +1380,11 @@ static int create_cond_regex_rule(const char *file, int line, goto err; } - if (dir == SMP_OPT_DIR_REQ) - err_code |= warnif_cond_requires_resp(cond, file, line); - else - err_code |= warnif_cond_requires_req(cond, file, line); + err_code |= warnif_cond_conflicts(cond, + (dir == SMP_OPT_DIR_REQ) ? + ((px->cap & PR_CAP_FE) ? SMP_VAL_FE_HRQ_HDR : SMP_VAL_BE_HRQ_HDR) : + ((px->cap & PR_CAP_BE) ? SMP_VAL_BE_HRS_HDR : SMP_VAL_FE_HRS_HDR), + file, line); preg = calloc(1, sizeof(regex_t)); if (!preg) { @@ -2627,7 +2628,10 @@ int cfg_parse_listen(const char *file, int linenum, char **args, int kwm) goto out; } - err_code |= warnif_cond_requires_resp(rule->cond, file, linenum); + err_code |= warnif_cond_conflicts(rule->cond, + (curproxy->cap & PR_CAP_FE) ? SMP_VAL_FE_HRQ_HDR : SMP_VAL_BE_HRQ_HDR, + file, linenum); + LIST_ADDQ(&curproxy->http_req_rules, &rule->list); } else if (!strcmp(args[0], "http-send-name-header")) { /* send server name in request header */ @@ -2689,7 +2693,9 @@ int cfg_parse_listen(const char *file, int linenum, char **args, int kwm) LIST_ADDQ(&curproxy->redirect_rules, &rule->list); err_code |= warnif_rule_after_use_backend(curproxy, file, linenum, args[0]); - err_code |= warnif_cond_requires_resp(rule->cond, file, linenum); + err_code |= warnif_cond_conflicts(rule->cond, + (curproxy->cap & PR_CAP_FE) ? SMP_VAL_FE_HRQ_HDR : SMP_VAL_BE_HRQ_HDR, + file, linenum); } else if (!strcmp(args[0], "use_backend")) { struct switching_rule *rule; @@ -2723,7 +2729,7 @@ int cfg_parse_listen(const char *file, int linenum, char **args, int kwm) goto out; } - err_code |= warnif_cond_requires_resp(cond, file, linenum); + err_code |= warnif_cond_conflicts(cond, SMP_VAL_FE_SET_BCK, file, linenum); rule = (struct switching_rule *)calloc(1, sizeof(*rule)); rule->cond = cond; @@ -2763,7 +2769,7 @@ int cfg_parse_listen(const char *file, int linenum, char **args, int kwm) goto out; } - err_code |= warnif_cond_requires_resp(cond, file, linenum); + err_code |= warnif_cond_conflicts(cond, SMP_VAL_BE_SET_SRV, file, linenum); rule = (struct server_rule *)calloc(1, sizeof(*rule)); rule->cond = cond; @@ -2799,7 +2805,10 @@ int cfg_parse_listen(const char *file, int linenum, char **args, int kwm) goto out; } - err_code |= warnif_cond_requires_resp(cond, file, linenum); + /* note: BE_REQ_CNT is the first one after FE_SET_BCK, which is + * where force-persist is applied. + */ + err_code |= warnif_cond_conflicts(cond, SMP_VAL_BE_REQ_CNT, file, linenum); rule = (struct persist_rule *)calloc(1, sizeof(*rule)); rule->cond = cond; @@ -3064,9 +3073,9 @@ int cfg_parse_listen(const char *file, int linenum, char **args, int kwm) goto out; } if (flags & STK_ON_RSP) - err_code |= warnif_cond_requires_req(cond, file, linenum); + err_code |= warnif_cond_conflicts(cond, SMP_VAL_BE_STO_RUL, file, linenum); else - err_code |= warnif_cond_requires_resp(cond, file, linenum); + err_code |= warnif_cond_conflicts(cond, SMP_VAL_BE_SET_SRV, file, linenum); rule = (struct sticking_rule *)calloc(1, sizeof(*rule)); rule->cond = cond; @@ -3116,7 +3125,9 @@ int cfg_parse_listen(const char *file, int linenum, char **args, int kwm) goto out; } - err_code |= warnif_cond_requires_resp(cond, file, linenum); + err_code |= warnif_cond_conflicts(cond, + (curproxy->cap & PR_CAP_FE) ? SMP_VAL_FE_HRQ_HDR : SMP_VAL_BE_HRQ_HDR, + file, linenum); rule = (struct stats_admin_rule *)calloc(1, sizeof(*rule)); rule->cond = cond; @@ -3185,7 +3196,9 @@ int cfg_parse_listen(const char *file, int linenum, char **args, int kwm) goto out; } - err_code |= warnif_cond_requires_resp(rule->cond, file, linenum); + err_code |= warnif_cond_conflicts(rule->cond, + (curproxy->cap & PR_CAP_FE) ? SMP_VAL_FE_HRQ_HDR : SMP_VAL_BE_HRQ_HDR, + file, linenum); LIST_ADDQ(&curproxy->uri_auth->http_req_rules, &rule->list); } else if (!strcmp(args[1], "auth")) { @@ -5259,7 +5272,9 @@ stats_error_parsing: err_code |= ERR_ALERT | ERR_FATAL; goto out; } - err_code |= warnif_cond_requires_resp(cond, file, linenum); + err_code |= warnif_cond_conflicts(cond, + (curproxy->cap & PR_CAP_FE) ? SMP_VAL_FE_HRQ_HDR : SMP_VAL_BE_HRQ_HDR, + file, linenum); } else if (*args[2]) { Alert("parsing [%s:%d] : '%s' : Expecting nothing, 'if', or 'unless', got '%s'.\n", @@ -5354,7 +5369,9 @@ stats_error_parsing: err_code |= ERR_ALERT | ERR_FATAL; goto out; } - err_code |= warnif_cond_requires_req(cond, file, linenum); + err_code |= warnif_cond_conflicts(cond, + (curproxy->cap & PR_CAP_BE) ? SMP_VAL_BE_HRS_HDR : SMP_VAL_FE_HRS_HDR, + file, linenum); } else if (*args[2]) { Alert("parsing [%s:%d] : '%s' : Expecting nothing, 'if', or 'unless', got '%s'.\n", diff --git a/src/proto_tcp.c b/src/proto_tcp.c index 3cf0d81ad..dfae8e3b8 100644 --- a/src/proto_tcp.c +++ b/src/proto_tcp.c @@ -1189,6 +1189,8 @@ static int tcp_parse_tcp_rep(char **args, int section_type, struct proxy *curpx, int arg; struct tcp_rule *rule; unsigned int where; + const struct acl *acl; + const struct acl_keyword *kw; if (!*args[1]) { memprintf(err, "missing argument for '%s' in %s '%s'", @@ -1237,16 +1239,30 @@ static int tcp_parse_tcp_rep(char **args, int section_type, struct proxy *curpx, if (tcp_parse_response_rule(args, arg, section_type, curpx, defpx, rule, err, where) < 0) goto error; - if (rule->cond && (rule->cond->requires & ACL_USE_L6REQ_VOLATILE)) { - struct acl *acl; - const char *name; + acl = rule->cond ? acl_cond_conflicts(rule->cond, where) : NULL; + if (acl) { + if (acl->name && *acl->name) + memprintf(err, + "acl '%s' will never match in '%s %s' because it only involves keywords that are incompatible with '%s'", + acl->name, args[0], args[1], sample_ckp_names(where)); + else + memprintf(err, + "anonymous acl will never match in '%s %s' because it uses keyword '%s' which is incompatible with '%s'", + args[0], args[1], + LIST_ELEM(acl->expr.n, struct acl_expr *, list)->kw->kw, + sample_ckp_names(where)); - acl = cond_find_require(rule->cond, ACL_USE_L6REQ_VOLATILE); - name = acl ? acl->name : "(unknown)"; - - memprintf(err, - "acl '%s' involves some request-only criteria which will be ignored in '%s %s'", - name, args[0], args[1]); + warn++; + } + else if (rule->cond && acl_cond_kw_conflicts(rule->cond, where, &acl, &kw)) { + if (acl->name && *acl->name) + memprintf(err, + "acl '%s' involves keyword '%s' which is incompatible with '%s'", + acl->name, kw->kw, sample_ckp_names(where)); + else + memprintf(err, + "anonymous acl involves keyword '%s' which is incompatible with '%s'", + kw->kw, sample_ckp_names(where)); warn++; } @@ -1279,6 +1295,8 @@ static int tcp_parse_tcp_req(char **args, int section_type, struct proxy *curpx, int arg; struct tcp_rule *rule; unsigned int where; + const struct acl *acl; + const struct acl_keyword *kw; if (!*args[1]) { if (curpx == defpx) @@ -1330,16 +1348,30 @@ static int tcp_parse_tcp_req(char **args, int section_type, struct proxy *curpx, if (tcp_parse_request_rule(args, arg, section_type, curpx, defpx, rule, err, where) < 0) goto error; - if (rule->cond && (rule->cond->requires & ACL_USE_RTR_ANY)) { - struct acl *acl; - const char *name; + acl = rule->cond ? acl_cond_conflicts(rule->cond, where) : NULL; + if (acl) { + if (acl->name && *acl->name) + memprintf(err, + "acl '%s' will never match in '%s %s' because it only involves keywords that are incompatible with '%s'", + acl->name, args[0], args[1], sample_ckp_names(where)); + else + memprintf(err, + "anonymous acl will never match in '%s %s' because it uses keyword '%s' which is incompatible with '%s'", + args[0], args[1], + LIST_ELEM(acl->expr.n, struct acl_expr *, list)->kw->kw, + sample_ckp_names(where)); - acl = cond_find_require(rule->cond, ACL_USE_RTR_ANY); - name = acl ? acl->name : "(unknown)"; - - memprintf(err, - "acl '%s' involves some response-only criteria which will be ignored in '%s %s'", - name, args[0], args[1]); + warn++; + } + else if (rule->cond && acl_cond_kw_conflicts(rule->cond, where, &acl, &kw)) { + if (acl->name && *acl->name) + memprintf(err, + "acl '%s' involves keyword '%s' which is incompatible with '%s'", + acl->name, kw->kw, sample_ckp_names(where)); + else + memprintf(err, + "anonymous acl involves keyword '%s' which is incompatible with '%s'", + kw->kw, sample_ckp_names(where)); warn++; } @@ -1359,24 +1391,30 @@ static int tcp_parse_tcp_req(char **args, int section_type, struct proxy *curpx, if (tcp_parse_request_rule(args, arg, section_type, curpx, defpx, rule, err, where) < 0) goto error; - if (rule->cond && (rule->cond->requires & (ACL_USE_RTR_ANY|ACL_USE_L6_ANY|ACL_USE_L7_ANY))) { - struct acl *acl; - const char *name; - - acl = cond_find_require(rule->cond, ACL_USE_RTR_ANY|ACL_USE_L6_ANY|ACL_USE_L7_ANY); - name = acl ? acl->name : "(unknown)"; - - if (acl->requires & (ACL_USE_L6_ANY|ACL_USE_L7_ANY)) { + acl = rule->cond ? acl_cond_conflicts(rule->cond, where) : NULL; + if (acl) { + if (acl->name && *acl->name) memprintf(err, - "'%s %s' may not reference acl '%s' which makes use of " - "payload in %s '%s'. Please use '%s content' for this.", - args[0], args[1], name, proxy_type_str(curpx), curpx->id, args[0]); - goto error; - } - if (acl->requires & ACL_USE_RTR_ANY) + "acl '%s' will never match in '%s %s' because it only involves keywords that are incompatible with '%s'", + acl->name, args[0], args[1], sample_ckp_names(where)); + else memprintf(err, - "acl '%s' involves some response-only criteria which will be ignored in '%s %s'", - name, args[0], args[1]); + "anonymous acl will never match in '%s %s' because it uses keyword '%s' which is incompatible with '%s'", + args[0], args[1], + LIST_ELEM(acl->expr.n, struct acl_expr *, list)->kw->kw, + sample_ckp_names(where)); + + warn++; + } + else if (rule->cond && acl_cond_kw_conflicts(rule->cond, where, &acl, &kw)) { + if (acl->name && *acl->name) + memprintf(err, + "acl '%s' involves keyword '%s' which is incompatible with '%s'", + acl->name, kw->kw, sample_ckp_names(where)); + else + memprintf(err, + "anonymous acl involves keyword '%s' which is incompatible with '%s'", + kw->kw, sample_ckp_names(where)); warn++; }