From 7978c5c42204adb9fa0d9c57c9288c579691482a Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Tue, 7 Sep 2021 14:24:07 +0200 Subject: [PATCH] MEDIUM: vars: make the ifexist variant of set-var only apply to the proc scope When setting variables, there are currently two variants, one which will always create the variable, and another one, "ifexist", which will only create or update a variable if a similarly named variable in any scope already existed before. The goal was to limit the risk of injecting random names in the proc scope, but it was achieved by making use of the somewhat limited name indexing model, which explains the scope-agnostic restriction. With this change, we're moving the check downwards in the chain, at the variable level, and only variables under the scope "proc" will be subject to the restriction. A new set of VF_* flags was added to adjust how variables are set, and VF_UPDATEONLY is used to mention this restriction. In this exact state of affairs, this is not completely exact, as if a similar name was not known in any scope, the variable will continue to be rejected like before, but this will change soon. --- doc/SPOE.txt | 3 ++- doc/lua-api/index.rst | 13 +++++++++---- include/haproxy/vars-t.h | 3 +++ src/vars.c | 21 +++++++++++++++------ 4 files changed, 29 insertions(+), 11 deletions(-) diff --git a/doc/SPOE.txt b/doc/SPOE.txt index 4792963f4..2330c499d 100644 --- a/doc/SPOE.txt +++ b/doc/SPOE.txt @@ -276,7 +276,8 @@ no option dontlog-normal option force-set-var By default, SPOE filter only register already known variables (mainly from - parsing of the configuration). If you want that haproxy trusts the agent and + parsing of the configuration), and process-wide variables (those of scope + "proc") cannot be created. If you want that haproxy trusts the agent and registers all variables (ex: can be useful for LUA workload), activate this option. diff --git a/doc/lua-api/index.rst b/doc/lua-api/index.rst index d671b3222..991dc6f2f 100644 --- a/doc/lua-api/index.rst +++ b/doc/lua-api/index.rst @@ -2023,8 +2023,9 @@ TXN class integer. :param boolean ifexist: If this parameter is set to a truthy value the variable will only be set if it was defined elsewhere (i.e. used - within the configuration). It is highly recommended to - always set this to true. + within the configuration). For global variables (using the + "proc" scope), they will only be updated and never created. + It is highly recommended to always set this to true. .. js:function:: TXN.unset_var(TXN, var) @@ -2831,7 +2832,9 @@ AppletHTTP class integer. :param boolean ifexist: If this parameter is set to a truthy value the variable will only be set if it was defined elsewhere (i.e. used - within the configuration). It is highly recommended to + within the configuration). For global variables (using the + "proc" scope), they will only be updated and never created. + It is highly recommended to always set this to true. :see: :js:func:`AppletHTTP.unset_var` :see: :js:func:`AppletHTTP.get_var` @@ -2946,7 +2949,9 @@ AppletTCP class integer. :param boolean ifexist: If this parameter is set to a truthy value the variable will only be set if it was defined elsewhere (i.e. used - within the configuration). It is highly recommended to + within the configuration). For global variables (using the + "proc" scope), they will only be updated and never created. + It is highly recommended to always set this to true. :see: :js:func:`AppletTCP.unset_var` :see: :js:func:`AppletTCP.get_var` diff --git a/include/haproxy/vars-t.h b/include/haproxy/vars-t.h index e1e04524c..0e35ade19 100644 --- a/include/haproxy/vars-t.h +++ b/include/haproxy/vars-t.h @@ -25,6 +25,9 @@ #include #include +/* flags used when setting/clearing variables */ +#define VF_UPDATEONLY 0x00000001 // SCOPE_PROC variables are only updated + enum vars_scope { SCOPE_SESS = 0, SCOPE_TXN, diff --git a/src/vars.c b/src/vars.c index 8a54c20d8..df26ac160 100644 --- a/src/vars.c +++ b/src/vars.c @@ -352,9 +352,14 @@ static int smp_fetch_var(const struct arg *args, struct sample *smp, const char * and the stream may be NULL when scope is SCOPE_SESS. In case there wouldn't * be enough memory to store the sample while the variable was already created, * it would be changed to a bool (which is memory-less). + * + * Flags is a bitfield that may contain one of the following flags: + * - VF_UPDATEONLY: if the scope is SCOPE_PROC, the variable may only be + * updated but not created. + * * It returns 0 on failure, non-zero on success. */ -static int var_set(const char *name, enum vars_scope scope, struct sample *smp) +static int var_set(const char *name, enum vars_scope scope, struct sample *smp, uint flags) { struct vars *vars; struct var *var; @@ -383,6 +388,10 @@ static int var_set(const char *name, enum vars_scope scope, struct sample *smp) -var->data.u.meth.str.data); } } else { + /* creation permitted for proc ? */ + if (flags & VF_UPDATEONLY && scope == SCOPE_PROC) + goto unlock; + /* Check memory available. */ if (!var_accounting_add(vars, smp->sess, smp->strm, sizeof(struct var))) goto unlock; @@ -487,7 +496,7 @@ static int var_unset(const char *name, enum vars_scope scope, struct sample *smp /* Returns 0 if fails, else returns 1. */ static int smp_conv_store(const struct arg *args, struct sample *smp, void *private) { - return var_set(args[0].data.var.name, args[0].data.var.scope, smp); + return var_set(args[0].data.var.name, args[0].data.var.scope, smp, 0); } /* Returns 0 if fails, else returns 1. */ @@ -541,7 +550,7 @@ int vars_set_by_name_ifexist(const char *name, size_t len, struct sample *smp) if (!name) return 0; - return var_set(name, scope, smp); + return var_set(name, scope, smp, VF_UPDATEONLY); } @@ -557,7 +566,7 @@ int vars_set_by_name(const char *name, size_t len, struct sample *smp) if (!name) return 0; - return var_set(name, scope, smp); + return var_set(name, scope, smp, 0); } /* This function unset a variable if it was already defined. @@ -717,7 +726,7 @@ static enum act_return action_store(struct act_rule *rule, struct proxy *px, smp_set_owner(&smp, px, sess, s, 0); smp.data.type = SMP_T_STR; smp.data.u.str = *fmtstr; - var_set(rule->arg.vars.name, rule->arg.vars.scope, &smp); + var_set(rule->arg.vars.name, rule->arg.vars.scope, &smp, 0); } else { /* an expression is used */ @@ -727,7 +736,7 @@ static enum act_return action_store(struct act_rule *rule, struct proxy *px, } /* Store the sample, and ignore errors. */ - var_set(rule->arg.vars.name, rule->arg.vars.scope, &smp); + var_set(rule->arg.vars.name, rule->arg.vars.scope, &smp, 0); free_trash_chunk(fmtstr); return ACT_RET_CONT; }