add immutable flag, and check it most places

the main purpose of immutable flags is to prevent users from
modifying values that the server wants to keep around.

As a result, the main checks for immutable values are in the
various editing routines:

    src/lib/util/calc.c	- cannot store results to immutable leaves
    src/lib/util/edit.c - cannot edit immutable values
			  or delete immutable leaves from lists
    src/lib/unlang/edit.c - cannot store to immutable leaves

Many of the internal value box / pair APIs will ignore the
immutable flag, and happily over-write values.  This behavior
is likely good enough for now.  The intention is to stop admins
from doing stupid things, and not to prevent the internal code
from doing what makes sense.

There is currently no flag in structural VPs which says "contains
an immutable child".  The edit code therefore has to check each
time by walking the list recursively.  That's fine for now.

There is as yet no code to set the immutable flag, or unit tests.
The goal is to have the decoders set the immutable flag as necessary,
which means that they don't need to save / restore attributes with
special meaning.

diff --git a/src/lib/unlang/edit.c b/src/lib/unlang/edit.c
index ab2f0def4832d166b71b8796893100c7a70c98b8..d833c347d00b873350c79503615fff966c3f8ac3 100644 (file)
--- a/src/lib/unlang/edit.c
+++ b/src/lib/unlang/edit.c
@@ -1336,6 +1336,11 @@ static int check_lhs(request_t *request, unlang_frame_state_edit_t *state, edit_
                // &control := ...
        }
 
+       if (fr_pair_immutable(vp)) {
+               RWDEBUG("Cannot modify immutable value for %s", current->lhs.vpt->name);
+               return -1;
+       }
+
        /*
         *      We found an existing attribute, with a modification operator.
         */

diff --git a/src/lib/util/calc.c b/src/lib/util/calc.c
index ef2a939a747c321ea675f6eaa385b43c34b2ef78..b413a178d4a2541c814f01bd848836de463962a5 100644 (file)
--- a/src/lib/util/calc.c
+++ b/src/lib/util/calc.c
@@ -2119,6 +2119,11 @@ int fr_value_calc_assignment_op(TALLOC_CTX *ctx, fr_value_box_t *dst, fr_token_t
        if (!fr_type_is_leaf(dst->type)) return invalid_type(dst->type);
        if (!fr_type_is_leaf(src->type)) return invalid_type(src->type);
 
+       if (dst->immutable) {
+               fr_strerror_printf("Cannot modify immutable value");
+               return -1;
+       }
+
        /*
         *      These operators are included here for testing and completeness.  But see comments in
         *      fr_edit_list_apply_pair_assignment() for what the caller should be doing.
@@ -2175,6 +2180,11 @@ int fr_value_calc_unary_op(TALLOC_CTX *ctx, fr_value_box_t *dst, fr_token_t op,
 
        if (!fr_type_is_numeric(src->type)) return invalid_type(src->type);
 
+       if (dst->immutable) {
+               fr_strerror_printf("Cannot modify immutable value");
+               return -1;
+       }
+
        if (op == T_OP_INCRM) {
                /*
                 *      Add 1 or subtract 1 means RHS is always 1.

diff --git a/src/lib/util/edit.c b/src/lib/util/edit.c
index 527b0ac8c7c5af6dd650951483b0019fb57e1064..5959c03cc720d33e8b79d87af5ee611c56f75a72 100644 (file)
--- a/src/lib/util/edit.c
+++ b/src/lib/util/edit.c
@@ -265,6 +265,11 @@ static int edit_record(fr_edit_list_t *el, fr_edit_op_t op, fr_pair_t *vp, fr_pa
                case FR_EDIT_CLEAR:
                        if (!fr_type_is_structural(vp->vp_type)) return 0;
 
+                       if (fr_pair_immutable(vp)) {
+                               fr_strerror_printf("Cannot modify immutable value for %s", vp->da->name);
+                               return -1;
+                       }
+
                        fr_pair_list_free(&vp->vp_group);
                        return 0;
 
@@ -542,6 +547,11 @@ int fr_edit_list_insert_pair_after(fr_edit_list_t *el, fr_pair_list_t *list, fr_
  */
 int fr_edit_list_pair_delete(fr_edit_list_t *el, fr_pair_list_t *list, fr_pair_t *vp)
 {
+       if (fr_pair_immutable(vp)) {
+               fr_strerror_printf("Cannot modify immutable value for %s", vp->da->name);
+               return -1;
+       }
+
        if (!el) {
                fr_pair_delete(list, vp);
                return 0;
@@ -565,6 +575,8 @@ int fr_edit_list_pair_delete_by_da(fr_edit_list_t *el, fr_pair_list_t *list, fr_
         *      Delete all VPs with a matching da.
         */
        fr_pair_list_foreach(list, vp) {
+               if (fr_pair_immutable(vp)) continue;
+
                if (vp->da != da) continue;
 
                (void) fr_pair_remove(list, vp);
@@ -597,6 +609,11 @@ int fr_edit_list_replace_pair_value(fr_edit_list_t *el, fr_pair_t *vp, fr_value_
 {
        if (!fr_type_is_leaf(vp->vp_type)) return -1;
 
+       if (vp->data.immutable) {
+               fr_strerror_printf("Cannot modify immutable value for %s", vp->da->name);
+               return -1;
+       }
+
        if (el && (edit_record(el, FR_EDIT_VALUE, vp, NULL, NULL) < 0)) return -1;
 
        if (!fr_type_is_fixed_size(vp->vp_type)) fr_value_box_clear(&vp->data);
@@ -615,6 +632,11 @@ int fr_edit_list_replace_pair(fr_edit_list_t *el, fr_pair_list_t *list, fr_pair_
 {
        if (to_replace->da != vp->da) return -1;
 
+       if (fr_pair_immutable(to_replace)) {
+               fr_strerror_printf("Cannot modify immutable value for %s", vp->da->name);
+               return -1;
+       }
+
        if (!el) {
                if (fr_pair_insert_after(list, to_replace, vp) < 0) return -1;
                fr_pair_delete(list, to_replace);

diff --git a/src/lib/util/pair.c b/src/lib/util/pair.c
index 89e70e12e6b6c81beaaef1e9c13294b7ec2cde1f..fa54dffa8df7b806d84ad327d6f324bb81fbc8a3 100644 (file)
--- a/src/lib/util/pair.c
+++ b/src/lib/util/pair.c
@@ -1624,6 +1624,8 @@ int fr_pair_delete_by_da(fr_pair_list_t *list, fr_dict_attr_t const *da)
 
        fr_pair_list_foreach(list, vp) {
                if (da == vp->da) {
+                       if (fr_pair_immutable(vp)) continue;
+
                        cnt++;
                        fr_pair_delete(list, vp);
                }
@@ -2084,6 +2086,30 @@ bool fr_pair_validate_relaxed(fr_pair_t const *failed[2], fr_pair_list_t *filter
        return true;
 }
 
+/**
+ *
+ * @param[in] vp       the pair to check
+ * @return
+ *     - true          the pair is immutable, or has an immutable child
+ *     - false         the pair is not immutable, or has no immutable children.
+ */
+bool fr_pair_immutable(fr_pair_t const *vp)
+{
+       if (fr_type_is_leaf(vp->vp_type)) return vp->data.immutable;
+
+       fr_pair_list_foreach(&vp->vp_group, child) {
+               if (fr_type_is_leaf(child->vp_type)) {
+                       if (child->data.immutable) return true;
+
+                       continue;
+               }
+
+               if (fr_pair_immutable(child)) return true;
+       }
+
+       return false;
+}
+
 /** Steal a list of pairs to a new context
  *
  */

diff --git a/src/lib/util/pair.h b/src/lib/util/pair.h
index 22ceec8014a64e8884089013ac33a68c15869e94..08c504145fda8871d5ddf9bcd79661fd96285933 100644 (file)
--- a/src/lib/util/pair.h
+++ b/src/lib/util/pair.h
@@ -628,6 +628,9 @@ bool                fr_pair_validate(fr_pair_t const *failed[2], fr_pair_list_t *filter,
 bool           fr_pair_validate_relaxed(fr_pair_t const *failed[2], fr_pair_list_t *filter,
                                         fr_pair_list_t *list) CC_HINT(nonnull(2,3));
 
+bool           fr_pair_immutable(fr_pair_t const *vp) CC_HINT(nonnull);
+
+
 /* Lists */
 int            fr_pair_list_copy(TALLOC_CTX *ctx, fr_pair_list_t *to, fr_pair_list_t const *from);
 

diff --git a/src/lib/util/value.h b/src/lib/util/value.h
index da727fca499706e1cdcc02da4bfbf09ae4611304..02b8fa5459b73770ccb9a5be9c496199b6e75261 100644 (file)
--- a/src/lib/util/value.h
+++ b/src/lib/util/value.h
@@ -155,8 +155,11 @@ struct value_box_s {
        /** Type and flags should appear together for packing efficiency
         */
        fr_type_t               _CONST          type;                   //!< Type of this value-box, at the start, see pair.h
+
        unsigned int                            tainted : 1;            //!< i.e. did it come from an untrusted source
        unsigned int                            secret : 1;             //!< Same as #fr_dict_attr_flags_t secret
+       unsigned int                            immutable : 1;          //!< once set, the value cannot be changed
+
        uint16_t                _CONST          safe;                   //!< more detailed safety
 
        fr_value_box_entry_t                    entry;                  //!< Doubly linked list entry.
@@ -466,6 +469,7 @@ void fr_value_box_init(fr_value_box_t *vb, fr_type_t type, fr_dict_attr_t const
                        .enumv = enumv,
                        .tainted = tainted,
                        .secret = enumv && enumv->flags.secret,
+                       /* don't set the immutable flag.  The caller has to do it once he's finished editing the values */
                }, sizeof(*vb));
        fr_value_box_list_entry_init(vb);
 
@@ -980,6 +984,12 @@ void fr_value_box_set_secret(fr_value_box_t *box, bool secret)
        box->secret = secret;
 }
 
+static inline CC_HINT(nonnull, always_inline)
+void fr_value_box_set_immutable(fr_value_box_t *box)
+{
+       box->immutable = true;
+}
+
 
 /** @name Assign and manipulate binary-unsafe C strings
  *