From: Alan T. DeKok Date: Sat, 9 Sep 2023 15:43:56 +0000 (-0400) Subject: clean up in preparation for further raw cleanups X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8c8fa46411b8e6c6faf398b0a377f32878ae42e2;p=thirdparty%2Ffreeradius-server.git clean up in preparation for further raw cleanups fr_dict_unknown_afrom_oid_substr() now continues from where the previous parser left off. The raw handling is now essentially all out of the dictionaries --- diff --git a/src/lib/util/dict.h b/src/lib/util/dict.h index 66fb377c0af..f4dc781891c 100644 --- a/src/lib/util/dict.h +++ b/src/lib/util/dict.h @@ -392,10 +392,10 @@ fr_dict_attr_t *fr_dict_unknown_attr_afrom_da(TALLOC_CTX *ctx, fr_dict_attr_t c fr_slen_t fr_dict_unknown_afrom_oid_substr(TALLOC_CTX *ctx, - fr_dict_attr_err_t *err, fr_dict_attr_t **out, + fr_dict_attr_t **out, fr_dict_attr_t const *parent, - fr_sbuff_t *in, fr_sbuff_term_t const *tt) - CC_HINT(nonnull(3,4,5)); + fr_sbuff_t *in) + CC_HINT(nonnull); int fr_dict_attr_unknown_parent_to_known(fr_dict_attr_t *da, fr_dict_attr_t const *parent); diff --git a/src/lib/util/dict_unknown.c b/src/lib/util/dict_unknown.c index bbafa4b622e..d91e6531ec5 100644 --- a/src/lib/util/dict_unknown.c +++ b/src/lib/util/dict_unknown.c @@ -381,99 +381,30 @@ fr_dict_attr_t *fr_dict_unknown_attr_afrom_da(TALLOC_CTX *ctx, fr_dict_attr_t co * and will be use the unknown da as its talloc parent. * * @param[in] ctx to alloc new attribute in. - * @param[out] err Where to write error codes. * @param[out] out Where to write the head of the chain unknown * dictionary attributes. * @param[in] parent Attribute to use as the root for resolving OIDs in. * Usually the root of a protocol dictionary. * @param[in] in of attribute. - * @param[in] tt Terminal strings. * @return * - The number of bytes parsed on success. * - <= 0 on failure. Negative offset indicates parse error position. */ fr_slen_t fr_dict_unknown_afrom_oid_substr(TALLOC_CTX *ctx, - fr_dict_attr_err_t *err, fr_dict_attr_t **out, + fr_dict_attr_t **out, fr_dict_attr_t const *parent, - fr_sbuff_t *in, fr_sbuff_term_t const *tt) + fr_sbuff_t *in) { fr_sbuff_t our_in = FR_SBUFF(in); - fr_dict_attr_t const *our_parent; + fr_dict_attr_t const *our_parent = parent; fr_dict_attr_t *n = NULL; - fr_dict_attr_err_t our_err; fr_dict_attr_flags_t flags = { - .is_unknown = true + .is_unknown = true, + .is_raw = true, }; - bool is_raw; *out = NULL; - is_raw = fr_sbuff_adv_past_str_literal(&our_in, "raw"); - - /* - * Resolve all the known bits first... - */ - (void)fr_dict_attr_by_oid_substr(&our_err, &our_parent, parent, &our_in, tt); - switch (our_err) { - /* - * Um this is awkward, we were asked to - * produce an unknown but all components - * are known... - * - * Just exit and pass back the known - * attribute, unless we got a raw prefix - * in which case process that. - */ - case FR_DICT_ATTR_OK: - if (is_raw) { - *out = fr_dict_unknown_attr_afrom_da(ctx, our_parent); - if (!*out) { - if (err) *err = FR_DICT_ATTR_PARSE_ERROR; - } else { - (*out)->flags.is_raw = 1; - if (err) *err = FR_DICT_ATTR_OK; - } - } else { - *out = fr_dict_attr_unconst(our_parent); /* Which is the resolved attribute in this case */ - if (err) *err = FR_DICT_ATTR_OK; - } - - FR_SBUFF_SET_RETURN(in, &our_in); - - /* - * This is what we want... Everything - * up to the non-matching OID was valid. - * - * our_parent should be left pointing - * to the last known attribute, or be - * so to NULL if we couldn't resolve - * anything. - */ - case FR_DICT_ATTR_NOTFOUND: - if (our_parent) { - switch (parent->type) { - case FR_TYPE_STRUCTURAL: - break; - - default: - fr_strerror_printf("Parent OID component (%s) specified a non-structural type (%s)", - our_parent->name, - fr_type_to_str(our_parent->type)); - goto error; - } - } else { - our_parent = parent; - } - break; - - /* - * All other errors are fatal. - */ - default: - if (err) *err = our_err; - FR_SBUFF_ERROR_RETURN(&our_in); - } - /* * Allocate the final attribute first, so that any * unknown parents can be freed when this da is freed. @@ -489,22 +420,6 @@ fr_slen_t fr_dict_unknown_afrom_oid_substr(TALLOC_CTX *ctx, */ n = dict_attr_alloc_null(ctx); - /* - * fr_dict_attr_by_oid_substr parsed *something* - * we expected the next component to be a '.'. - */ - if (fr_sbuff_ahead(&our_in) > 0) { - if (!fr_sbuff_next_if_char(&our_in, '.')) { /* this is likely a logic bug if the test fails ? */ - fr_strerror_printf("Missing OID component separator %.*s", (int)fr_sbuff_remaining(&our_in), fr_sbuff_current(&our_in)); - error: - if (err) *err = FR_DICT_ATTR_PARSE_ERROR; - talloc_free(n); - FR_SBUFF_ERROR_RETURN(&our_in); - } - } else if (fr_sbuff_next_if_char(&our_in, '.')) { - our_parent = fr_dict_root(fr_dict_by_da(parent)); /* From the root */ - } - /* * Loop until there's no more component separators. */ @@ -526,7 +441,11 @@ fr_slen_t fr_dict_unknown_afrom_oid_substr(TALLOC_CTX *ctx, if (fr_sbuff_next_if_char(&our_in, '.')) { ni = fr_dict_unknown_vendor_afrom_num(n, our_parent, num); - if (!ni) goto error; + if (!ni) { + error: + talloc_free(n); + FR_SBUFF_ERROR_RETURN(&our_in); + } our_parent = ni; continue; } @@ -562,7 +481,6 @@ fr_slen_t fr_dict_unknown_afrom_oid_substr(TALLOC_CTX *ctx, fr_type_to_str(our_parent->type)); goto error; } - flags.is_raw = is_raw; if (dict_attr_init(&n, our_parent, NULL, num, FR_TYPE_OCTETS, &(dict_attr_args_t){ .flags = &flags }) < 0) goto error; break; diff --git a/src/lib/util/dict_util.c b/src/lib/util/dict_util.c index eeff7600c60..a915854c375 100644 --- a/src/lib/util/dict_util.c +++ b/src/lib/util/dict_util.c @@ -1944,6 +1944,7 @@ fr_slen_t fr_dict_attr_by_oid_substr(fr_dict_attr_err_t *err, fr_dict_attr_t const *child; if ((fr_dict_oid_component(err, &child, our_parent, &our_in, tt) < 0) || !child) { + *out = our_parent; fr_sbuff_set(&our_in, &m_c); /* Reset to the start of the last component */ break; /* Resolved as much as we can */ } diff --git a/src/lib/util/pair_legacy.c b/src/lib/util/pair_legacy.c index 8f3c3b99384..95b291c6407 100644 --- a/src/lib/util/pair_legacy.c +++ b/src/lib/util/pair_legacy.c @@ -111,6 +111,7 @@ static ssize_t fr_pair_list_afrom_substr(TALLOC_CTX *ctx, fr_dict_attr_t const * p = buffer; while (true) { + bool is_raw = false; ssize_t slen; fr_token_t op; fr_dict_attr_t const *da, *my_parent; @@ -167,32 +168,77 @@ static ssize_t fr_pair_list_afrom_substr(TALLOC_CTX *ctx, fr_dict_attr_t const * /* * Raw attributes get a special parser. */ - if (strncmp(p, "raw.", 4) == 0) goto do_unknown; + if (strncmp(p, "raw.", 4) == 0) { + p += 4; + is_raw = true; + } } /* * Parse the name. */ slen = fr_dict_attr_by_oid_substr(&err, &da, my_parent, &FR_SBUFF_IN(p, (end - p)), &bareword_terminals); - if ((err != FR_DICT_ATTR_OK) && internal) { - slen = fr_dict_attr_by_oid_substr(&err, &da, internal, - &FR_SBUFF_IN(p, (end - p)), &bareword_terminals); + if (err == FR_DICT_ATTR_NOTFOUND) { + if (is_raw) { + /* + * We have something like raw.KNOWN.26, let's go parse the unknown OID + * portion, starting from where the parsing failed. + */ + if (((slen > 0) && (p[slen] == '.') && isdigit((int) p[slen + 1])) || + ((slen == 0) && isdigit((int) *p))) { + char const *q = p + slen + (slen > 0); + + slen = fr_dict_unknown_afrom_oid_substr(ctx, &da_unknown, da, &FR_SBUFF_IN(q, (end - q))); + if (slen < 0) goto error; + + p = q; + da = da_unknown; + goto do_next; + } + + goto notfound; + } + + /* + * We have an internal dictionary, look up the attribute there. Note that we + * can't have raw internal attributes. + */ + if (internal) { + slen = fr_dict_attr_by_oid_substr(&err, &da, internal, + &FR_SBUFF_IN(p, (end - p)), &bareword_terminals); + } } if (err != FR_DICT_ATTR_OK) { - do_unknown: - slen = fr_dict_unknown_afrom_oid_substr(ctx, NULL, &da_unknown, my_parent, - &FR_SBUFF_IN(p, (end - p)), &bareword_terminals); - if (slen < 0) { - p += -slen; - - error: - *token = T_INVALID; - return -(p - buffer); + if (slen < 0) slen = -slen; + p += slen; + + /* + * Regenerate the error message so that it's for the correct parent. + */ + if (err == FR_DICT_ATTR_NOTFOUND) { + uint8_t const *q; + + notfound: + for (q = (uint8_t const *) p; q < (uint8_t const *) end && fr_dict_attr_allowed_chars[*q]; q++) { + /* nothing */ + } + fr_strerror_printf("Unknown attribute \"%.*s\" for parent \"%s\"", (int) (q - ((uint8_t const *) p)), p, my_parent->name); } + error: + *token = T_INVALID; + return -(p - buffer); + } + /* + * If we force it to be raw, then only do that if it's not already unknown. + */ + if (is_raw && !da_unknown) { + da_unknown = fr_dict_unknown_attr_afrom_da(ctx, da); + if (!da_unknown) goto error; da = da_unknown; } + do_next: next = p + slen; rhs[0] = '\0'; diff --git a/src/tests/unit/protocols/radius/vendor.txt b/src/tests/unit/protocols/radius/vendor.txt index a19fed1d15b..ed06bb729fb 100644 --- a/src/tests/unit/protocols/radius/vendor.txt +++ b/src/tests/unit/protocols/radius/vendor.txt @@ -112,23 +112,32 @@ match 1a 0b 00 00 00 09 01 05 66 6f 6f 1a 0b 00 00 00 09 01 05 62 61 72 # # Unknown attributes with TLVs # -encode-pair 26.6809.1 = 0xabcdef +encode-pair raw.26.6809.1 = 0xabcdef match 1a 0b 00 00 1a 99 01 05 ab cd ef decode-pair - match raw.Vendor-Specific.6809.1 = 0xabcdef -encode-pair 26.6809.1.2 = 0xabcdef +pair raw.26.6809.1.2 = 0xabcdef +match Vendor-Specific = { raw.6809 = { raw.1 = { raw.2 = 0xabcdef } } } + +# +# @todo - pair_legacy - We can't currently encode this, because the pair_legacy code creates "octets" for "raw.1", and not "tlv". +# +#encode-pair - +#match 1a 0d 00 00 1a 99 01 07 02 05 ab cd ef + +encode-pair raw.26.6809.1.2 = 0xabcdef match 1a 0d 00 00 1a 99 01 07 02 05 ab cd ef decode-pair - match raw.Vendor-Specific.6809.1 = { raw.2 = 0xabcdef } -encode-pair 26.6809.1.2.3 = 0xabcdef +encode-pair raw.26.6809.1.2.3 = 0xabcdef match 1a 0f 00 00 1a 99 01 09 02 07 03 05 ab cd ef decode-pair - match raw.Vendor-Specific.6809.1 = { raw.2 = { raw.3 = 0xabcdef } } count -match 64 +match 66