Perform implicit conversion from unknown attribute format to known attributes in...

e.g. Attr-1 is treated the same as User-Name

diff --git a/src/include/tmpl.h b/src/include/tmpl.h
index fb62cd6e0080ae45ac3769b06298c33b33007654..b587852e6e08065cf804d80b3a483c9fa8cfb993 100644 (file)
--- a/src/include/tmpl.h
+++ b/src/include/tmpl.h
@@ -218,15 +218,18 @@ value_pair_tmpl_t *tmpl_alloc(TALLOC_CTX *ctx, tmpl_type_t type, char const *nam
  *
  */
 ssize_t                        tmpl_from_attr_substr(value_pair_tmpl_t *vpt, char const *name,
-                                             request_refs_t request_def, pair_lists_t list_def, bool allow_undefined);
+                                             request_refs_t request_def, pair_lists_t list_def,
+                                             bool allow_unknown, bool allow_undefined);
 
 ssize_t                        tmpl_from_attr_str(value_pair_tmpl_t *vpt, char const *name,
                                           request_refs_t request_def,
-                                          pair_lists_t list_def, bool allow_undefined);
+                                          pair_lists_t list_def,
+                                          bool allow_unknown, bool allow_undefined);
 
 ssize_t                        tmpl_afrom_attr_str(TALLOC_CTX *ctx, value_pair_tmpl_t **out, char const *name,
                                            request_refs_t request_def,
-                                           pair_lists_t list_def, bool allow_undefined);
+                                           pair_lists_t list_def,
+                                           bool allow_unknown, bool allow_undefined);
 
 /*
  *     Parses any type of string into a template

diff --git a/src/lib/dict.c b/src/lib/dict.c
index e699af76983ac8ae41111e42f920c63f095dbc52..d70996064c251fec5c343f6367a68ef9db1563b9 100644 (file)
--- a/src/lib/dict.c
+++ b/src/lib/dict.c
@@ -2739,8 +2739,6 @@ DICT_ATTR const *dict_unknown_afrom_fields(TALLOC_CTX *ctx, unsigned int attr, u
  *  - Vendor-%d-Attr-%d
  *  - VendorName-Attr-%d
  *
- * @todo should check attr/vendor against dictionary and return the real da.
- *
  * @param[in] da to initialise.
  * @param[in] name of attribute.
  * @return 0 on success -1 on failure.
@@ -2954,8 +2952,6 @@ int dict_unknown_from_str(DICT_ATTR *da, char const *name)
  *  - Vendor-%d-Attr-%d
  *  - VendorName-Attr-%d
  *
- * @todo should check attr/vendor against dictionary and return the real da.
- *
  * @param[in] ctx to alloc new attribute in.
  * @param[in] name of attribute.
  * @return 0 on success -1 on failure.

diff --git a/src/main/map.c b/src/main/map.c
index 74cdecef59afd23e68d3aa1e18e6dceabdc237cf..66a931111306438ed98501d382f67cefe1e9c17e 100644 (file)
--- a/src/main/map.c
+++ b/src/main/map.c
@@ -185,7 +185,7 @@ int map_afrom_cp(TALLOC_CTX *ctx, value_pair_map_t **out, CONF_PAIR *cp,
        /*
         *      LHS must always be an attribute reference.
         */
-       slen = tmpl_afrom_attr_str(ctx, &map->lhs, attr, dst_request_def, dst_list_def, true);
+       slen = tmpl_afrom_attr_str(ctx, &map->lhs, attr, dst_request_def, dst_list_def, true, true);
        if (slen <= 0) {
                char *spaces, *text;
 

diff --git a/src/main/pair.c b/src/main/pair.c
index b8b2e1fd66ccf935a9cc08a804b723b0362b165d..9cbd920bd858b48aa38e75cb7cf4c3bb737ed545 100644 (file)
--- a/src/main/pair.c
+++ b/src/main/pair.c
@@ -758,7 +758,7 @@ int radius_get_vp(VALUE_PAIR **out, REQUEST *request, char const *name)
 
        *out = NULL;
 
-       if (tmpl_from_attr_str(&vpt, name, REQUEST_CURRENT, PAIR_LIST_REQUEST, false) <= 0) {
+       if (tmpl_from_attr_str(&vpt, name, REQUEST_CURRENT, PAIR_LIST_REQUEST, false, false) <= 0) {
                return -4;
        }
 
@@ -784,7 +784,7 @@ int radius_copy_vp(TALLOC_CTX *ctx, VALUE_PAIR **out, REQUEST *request, char con
 
        *out = NULL;
 
-       if (tmpl_from_attr_str(&vpt, name, REQUEST_CURRENT, PAIR_LIST_REQUEST, false) <= 0) {
+       if (tmpl_from_attr_str(&vpt, name, REQUEST_CURRENT, PAIR_LIST_REQUEST, false, false) <= 0) {
                return -4;
        }
 

diff --git a/src/main/tmpl.c b/src/main/tmpl.c
index 46e17d210fddcd009e772fb95b0c756b24a88384..0cf5b5e3731e8158542552754d9036a50565c4e1 100644 (file)
--- a/src/main/tmpl.c
+++ b/src/main/tmpl.c
@@ -775,12 +775,15 @@ value_pair_tmpl_t *tmpl_alloc(TALLOC_CTX *ctx, tmpl_type_t type, char const *nam
  * @param[in] name of attribute including qualifiers.
  * @param[in] request_def The default request to insert unqualified attributes into.
  * @param[in] list_def The default list to insert unqualified attributes into.
- * @param[in] allow_undefined If true, we don't generate a parse error on attributes
- *     that are not in the global dictionary, and are not in unknown attribute format.
+ * @param[in] allow_unknown If true attributes in the format accepted by dict_unknown_from_substr
+ *     will be allowed, even if they're not in the main dictionaries.
+ * @param[in] allow_undefined If true, we don't generate a parse error on unknown
+ *     attributes, and instead set type to TMPL_TYPE_ATTR_UNDEFINED.
  * @return <= 0 on error (offset as negative integer), > 0 on success (number of bytes parsed)
  */
 ssize_t tmpl_from_attr_substr(value_pair_tmpl_t *vpt, char const *name,
-                             request_refs_t request_def, pair_lists_t list_def, bool allow_undefined)
+                             request_refs_t request_def, pair_lists_t list_def,
+                             bool allow_unknown, bool allow_undefined)
 {
        char const *p;
        long num;
@@ -821,10 +824,34 @@ ssize_t tmpl_from_attr_substr(value_pair_tmpl_t *vpt, char const *name,
 
        attr.da = dict_attrbyname_substr(&p);
        if (!attr.da) {
+               char const *a;
+
+               /*
+                *      Record start of attribute in case we need to error out.
+                */
+               a = p;
+
                /*
                 *      Attr-1.2.3.4 is OK.
                 */
                if (dict_unknown_from_substr((DICT_ATTR *)&attr.unknown.da, &p) == 0) {
+                       /*
+                        *      Check what we just parsed really hasn't been defined
+                        *      in the main dictionaries.
+                        */
+                       attr.da = dict_attrbyvalue(((DICT_ATTR *)&attr.unknown.da)->attr,
+                                                  ((DICT_ATTR *)&attr.unknown.da)->vendor);
+                       if (attr.da) goto do_tag;
+
+
+                       fprintf(stderr, "\n%i:%i\n", ((DICT_ATTR *)&attr.unknown.da)->attr,
+                               ((DICT_ATTR *)&attr.unknown.da)->vendor);
+
+                       if (!allow_unknown) {
+                               fr_strerror_printf("Unknown attribute");
+                               return -(a - name);
+                       }
+
                        attr.da = (DICT_ATTR *)&attr.unknown.da;
                        goto skip_tag; /* unknown attributes can't have tags */
                }
@@ -834,8 +861,8 @@ ssize_t tmpl_from_attr_substr(value_pair_tmpl_t *vpt, char const *name,
                 *      let the caller decide.
                 */
                if (!allow_undefined) {
-                       fr_strerror_printf("Unknown attribute");
-                       return -(p - name);
+                       fr_strerror_printf("Undefined attribute");
+                       return -(a - name);
                }
 
                /*
@@ -852,6 +879,8 @@ ssize_t tmpl_from_attr_substr(value_pair_tmpl_t *vpt, char const *name,
 
                goto skip_tag;
        }
+
+do_tag:
        type = TMPL_TYPE_ATTR;
 
        /*
@@ -955,11 +984,12 @@ finish:
  * @return <= 0 on error (offset as negative integer), > 0 on success (number of bytes parsed)
  */
 ssize_t tmpl_from_attr_str(value_pair_tmpl_t *vpt, char const *name,
-                          request_refs_t request_def, pair_lists_t list_def, bool allow_undefined)
+                          request_refs_t request_def, pair_lists_t list_def,
+                          bool allow_unknown, bool allow_undefined)
 {
        ssize_t slen;
 
-       slen = tmpl_from_attr_substr(vpt, name, request_def, list_def, allow_undefined);
+       slen = tmpl_from_attr_substr(vpt, name, request_def, list_def, allow_unknown, allow_undefined);
        if (slen <= 0) return slen;
        if (name[slen] != '\0') {
                fr_strerror_printf("Unexpected text after attribute name");
@@ -982,19 +1012,22 @@ ssize_t tmpl_from_attr_str(value_pair_tmpl_t *vpt, char const *name,
  * @param[in] request_def The default request to insert unqualified
  *     attributes into.
  * @param[in] list_def The default list to insert unqualified attributes into.
+ * @param[in] allow_unknown If true attributes in the format accepted by dict_unknown_from_substr
+ *     will be allowed, even if they're not in the main dictionaries.
  * @param[in] allow_undefined If true, we don't generate a parse error on unknown
  *     attributes, and instead set type to TMPL_TYPE_ATTR_UNDEFINED.
  * @return <= 0 on error (offset as negative integer), > 0 on success (number of bytes parsed)
  */
 ssize_t tmpl_afrom_attr_str(TALLOC_CTX *ctx, value_pair_tmpl_t **out, char const *name,
-                           request_refs_t request_def, pair_lists_t list_def, bool allow_undefined)
+                           request_refs_t request_def, pair_lists_t list_def,
+                           bool allow_unknown, bool allow_undefined)
 {
        ssize_t slen;
        value_pair_tmpl_t *vpt;
 
        MEM(vpt = talloc(ctx, value_pair_tmpl_t)); /* tmpl_from_attr_substr zeros it */
 
-       slen = tmpl_from_attr_substr(vpt, name, request_def, list_def, allow_undefined);
+       slen = tmpl_from_attr_substr(vpt, name, request_def, list_def, allow_unknown, allow_undefined);
        if (slen <= 0) {
                tmpl_free(&vpt);
                return slen;
@@ -1242,7 +1275,7 @@ ssize_t tmpl_afrom_str(TALLOC_CTX *ctx, value_pair_tmpl_t **out, char const *nam
                 *      If we can parse it as an attribute, it's an attribute.
                 *      Otherwise, treat it as a literal.
                 */
-               slen = tmpl_afrom_attr_str(ctx, &vpt, name, request_def, list_def, (name[0] == '&'));
+               slen = tmpl_afrom_attr_str(ctx, &vpt, name, request_def, list_def, true, (name[0] == '&'));
                if ((name[0] == '&') && (slen <= 0)) return slen;
                if (slen > 0) break;
                /* FALL-THROUGH */

diff --git a/src/main/xlat.c b/src/main/xlat.c
index 4f3db62fbef658b3c915883a784a1b1bc810eef2..068473c6135541e282b0610e42d3df1653b4de53 100644 (file)
--- a/src/main/xlat.c
+++ b/src/main/xlat.c
@@ -316,7 +316,7 @@ static ssize_t xlat_debug_attr(UNUSED void *instance, REQUEST *request, char con
 
        while (isspace((int) *fmt)) fmt++;
 
-       if (tmpl_from_attr_str(&vpt, fmt, REQUEST_CURRENT, PAIR_LIST_REQUEST, false) <= 0) {
+       if (tmpl_from_attr_str(&vpt, fmt, REQUEST_CURRENT, PAIR_LIST_REQUEST, false, false) <= 0) {
                RDEBUG("%s", fr_strerror());
                return -1;
        }
@@ -1208,7 +1208,7 @@ static ssize_t xlat_tokenize_expansion(TALLOC_CTX *ctx, char *fmt, xlat_exp_t **
         *      - '[' - Which is an attribute index, so it must be an attribute.
         *      - '}' - The end of the expansion, which means it was a bareword.
         */
-       slen = tmpl_from_attr_substr(&node->attr, p, REQUEST_CURRENT, PAIR_LIST_REQUEST, true);
+       slen = tmpl_from_attr_substr(&node->attr, p, REQUEST_CURRENT, PAIR_LIST_REQUEST, true, true);
        if (slen <= 0) {
                /*
                 *      If the parse error occurred before the ':'

diff --git a/src/modules/rlm_expr/rlm_expr.c b/src/modules/rlm_expr/rlm_expr.c
index 278c5e478af65b2bf9b17d22c6a143182123cce8..ee60e16ffc9080c65c0a293fdd9f8f0a1db842fc 100644 (file)
--- a/src/modules/rlm_expr/rlm_expr.c
+++ b/src/modules/rlm_expr/rlm_expr.c
@@ -223,7 +223,7 @@ static bool get_number(REQUEST *request, char const **string, int64_t *answer)
 
                p += 1;
 
-               slen = tmpl_from_attr_substr(&vpt, p, REQUEST_CURRENT, PAIR_LIST_REQUEST, false);
+               slen = tmpl_from_attr_substr(&vpt, p, REQUEST_CURRENT, PAIR_LIST_REQUEST, false, false);
                if (slen < 0) {
                        RDEBUG("Failed parsing attribute name '%s': %s", p, fr_strerror());
                        return false;
@@ -1145,7 +1145,7 @@ static ssize_t pairs_xlat(UNUSED void *instance, REQUEST *request,
 
        VALUE_PAIR *vp;
 
-       if (tmpl_from_attr_str(&vpt, fmt, REQUEST_CURRENT, PAIR_LIST_REQUEST, false) <= 0) {
+       if (tmpl_from_attr_str(&vpt, fmt, REQUEST_CURRENT, PAIR_LIST_REQUEST, false, false) <= 0) {
                REDEBUG("%s", fr_strerror());
                return -1;
        }

diff --git a/src/modules/rlm_rest/rest.c b/src/modules/rlm_rest/rest.c
index dcaec7f7813ffe28c9cebadc9c87d10599bf9b18..29503cc947f761d0eb5362b56acf182f405975d1 100644 (file)
--- a/src/modules/rlm_rest/rest.c
+++ b/src/modules/rlm_rest/rest.c
@@ -1325,7 +1325,7 @@ static int json_pairmake(rlm_rest_t *instance, rlm_rest_section_t *section,
                 */
                RDEBUG2("Parsing attribute \"%s\"", name);
 
-               if (tmpl_from_attr_str(&dst, name, REQUEST_CURRENT, PAIR_LIST_REPLY, false) <= 0) {
+               if (tmpl_from_attr_str(&dst, name, REQUEST_CURRENT, PAIR_LIST_REPLY, false, false) <= 0) {
                        RWDEBUG("Failed parsing attribute: %s, skipping...", fr_strerror());
                        continue;
                }

diff --git a/src/tests/keywords/unknown b/src/tests/keywords/unknown
index c031649b6797eaff13672087a1f56103cb1b319d..eb9659123ae4d55ac37b59c9783aaf03e5f97c98 100644 (file)
--- a/src/tests/keywords/unknown
+++ b/src/tests/keywords/unknown
@@ -5,6 +5,14 @@ update control {
        Cleartext-Password := 'hello'
 }
 
+update reply {
+       Filter-Id := "filter"
+}
+
+update request {
+       FreeRADIUS-Proxied-To  := 127.0.0.2
+}
+
 #
 #  Check that a known but malformed attr is malformed
 #
@@ -14,7 +22,7 @@ update request {
 
 if (&Attr-26.24757.84.9.5.7 != 0xab) {
        update reply {
-               Filter-Id += "fail-1"
+               Filter-Id += 'Fail 1'
        }
 }
 
@@ -27,12 +35,50 @@ update request {
 
 if (&Attr-26.24757.84.9.5.15 != 0xabcdef) {
        update reply {
-               Filter-Id += "fail-2"
+               Filter-Id += 'Fail 2'
+       }
+}
+
+#
+#  Check that unknown attributes which are defined
+#  get automatically resolved to the real attribute.
+#
+if (&Vendor-11344-Attr-1 == 127.0.0.1) {
+       update reply {
+               Filter-Id += 'Fail 3'
+       }
+}
+
+if (&Vendor-11344-Attr-1 != 127.0.0.2) {
+       update reply {
+               Filter-Id += 'Fail 4'
+       }
+}
+
+update request {
+       &Vendor-11344-Attr-1 := 127.0.0.1
+}
+
+if (&FreeRADIUS-Proxied-To == 127.0.0.2) {
+       update reply {
+               Filter-Id += 'Fail 5'
+       }
+}
+
+if (&FreeRADIUS-Proxied-To != 127.0.0.1) {
+       update reply {
+               Filter-Id += 'Fail 6'
+       }
+}
+
+if (&Vendor-11344-Attr-1 == 127.0.0.2) {
+       update reply {
+               Filter-Id += 'Fail 7'
        }
 }
 
-if (!reply:Filter-Id) {
+if (&Vendor-11344-Attr-1 != 127.0.0.1) {
        update reply {
-               Filter-Id := "filter"
+               Filter-Id += 'Fail 8'
        }
 }

diff --git a/src/tests/unit/condition.txt b/src/tests/unit/condition.txt
index 235a03c2368baaf27ed15e23d9c0b3560ebfb360..92f648a93020e8ff8d8a40d130286d6d9f9ab537 100644 (file)
--- a/src/tests/unit/condition.txt
+++ b/src/tests/unit/condition.txt
@@ -607,3 +607,12 @@ data &not-a-packet.User-Name == &not-a-packet.User-Name
 #
 condition ('i have scary embedded things\000 inside me' == "i have scary embedded things\000 inside me")
 data false
+
+#
+#  'Unknown' attributes which are defined in the main dictionary
+#  should be resolved to their real names.
+condition &Attr-1 == 'bar'
+data &User-Name == 'bar'
+
+condition &Vendor-11344-Attr-1 == 127.0.0.1
+data &FreeRADIUS-Proxied-To == 127.0.0.1/32