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.

diff --git a/doc/SPOE.txt b/doc/SPOE.txt
index 4792963f487cbfa3526ce64f5d398984cd34a762..2330c499d9942f705805017ff0d8b6c073aaa9f4 100644 (file)
--- 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 d671b32227e4b0a233cbe7361103a6d69264f76d..991dc6f2f616c2055e87ced15485ce9975e4f967 100644 (file)
--- 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 e1e04524c71c2c3d6fbb68382b3d86da497bf813..0e35ade1988e3ea4efa642b11f11d6825ca6295e 100644 (file)
--- a/include/haproxy/vars-t.h
+++ b/include/haproxy/vars-t.h
@@ -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,

diff --git a/src/vars.c b/src/vars.c
index 8a54c20d831894f771f9b04fa21f5d093f9aedf1..df26ac160a8eb6a16b043d32d12b96a60df68b7f 100644 (file)
--- 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;
 }