From: Arran Cudbard-Bell Date: Wed, 31 Dec 2014 16:25:51 +0000 (-0500) Subject: Perform implicit conversion from unknown attribute format to known attributes in... X-Git-Tag: release_3_0_7~387 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=3312f0abbc7e177b47e8107fe987fbe11b2c1588;p=thirdparty%2Ffreeradius-server.git Perform implicit conversion from unknown attribute format to known attributes in conditions and update sections e.g. Attr-1 is treated the same as User-Name --- diff --git a/src/include/tmpl.h b/src/include/tmpl.h index fb62cd6e008..b587852e6e0 100644 --- 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 e699af76983..d70996064c2 100644 --- 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 74cdecef59a..66a93111130 100644 --- 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 b8b2e1fd66c..9cbd920bd85 100644 --- 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 46e17d210fd..0cf5b5e3731 100644 --- 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 4f3db62fbef..068473c6135 100644 --- 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 278c5e478af..ee60e16ffc9 100644 --- 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 dcaec7f7813..29503cc947f 100644 --- 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 c031649b679..eb9659123ae 100644 --- 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 235a03c2368..92f648a9302 100644 --- a/src/tests/unit/condition.txt +++ b/src/tests/unit/condition.txt @@ -607,3 +607,12 @@ data ¬-a-packet.User-Name == ¬-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