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.
This commit is contained in:
Willy Tarreau 2021-09-07 14:24:07 +02:00
parent f1cb0ebe3e
commit 7978c5c422
4 changed files with 29 additions and 11 deletions

View File

@ -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.

View File

@ -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`

View File

@ -25,6 +25,9 @@
#include <haproxy/sample_data-t.h>
#include <haproxy/thread-t.h>
/* flags used when setting/clearing variables */
#define VF_UPDATEONLY 0x00000001 // SCOPE_PROC variables are only updated
enum vars_scope {
SCOPE_SESS = 0,
SCOPE_TXN,

View File

@ -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;
}