From: Arran Cudbard-Bell Date: Tue, 12 Aug 2025 23:07:23 +0000 (-0600) Subject: Fix %internal.encode() X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=03c8e45202e5cc2122a7349568bb73713865979e;p=thirdparty%2Ffreeradius-server.git Fix %internal.encode() The xlat previously seemed to truncate output and skipped every other attribute --- diff --git a/src/lib/unlang/xlat_builtin.c b/src/lib/unlang/xlat_builtin.c index 7ddd7257787..856c21a154a 100644 --- a/src/lib/unlang/xlat_builtin.c +++ b/src/lib/unlang/xlat_builtin.c @@ -23,14 +23,11 @@ * @copyright 2000,2006 The FreeRADIUS server project * @copyright 2000 Alan DeKok (aland@freeradius.org) */ - - RCSID("$Id$") /** * @defgroup xlat_functions xlat expansion functions */ - #include #include #include @@ -40,6 +37,9 @@ RCSID("$Id$") #include #include +#include +#include +#include #include #ifdef HAVE_OPENSSL_EVP_H @@ -259,8 +259,8 @@ xlat_action_t xlat_transparent(UNUSED TALLOC_CTX *ctx, fr_dcursor_t *out, * @ingroup xlat_functions */ static xlat_action_t xlat_func_pairs_debug(UNUSED TALLOC_CTX *ctx, UNUSED fr_dcursor_t *out, - UNUSED xlat_ctx_t const *xctx, - request_t *request, fr_value_box_list_t *args) + UNUSED xlat_ctx_t const *xctx, + request_t *request, fr_value_box_list_t *args) { fr_pair_t *vp; fr_dcursor_t *cursor; @@ -4142,13 +4142,14 @@ static xlat_action_t protocol_encode_xlat(TALLOC_CTX *ctx, fr_dcursor_t *out, bool tainted = false; fr_value_box_t *encoded; - uint8_t binbuf[2048]; - uint8_t *p = binbuf, *end = p + sizeof(binbuf); + fr_dbuff_t *dbuff; ssize_t len = 0; fr_value_box_t *in_head; void *encode_ctx = NULL; fr_test_point_pair_encode_t const *tp_encode; + FR_DBUFF_TALLOC_THREAD_LOCAL(&dbuff, 2048, SIZE_MAX); + XLAT_ARGS(args, &in_head); memcpy(&tp_encode, xctx->inst, sizeof(tp_encode)); /* const issues */ @@ -4167,12 +4168,35 @@ static xlat_action_t protocol_encode_xlat(TALLOC_CTX *ctx, fr_dcursor_t *out, /* * Loop over the attributes, encoding them. */ - for (vp = fr_dcursor_current(cursor); - vp != NULL; - vp = fr_dcursor_next(cursor)) { - if (vp->da->flags.internal) continue; + RDEBUG2("Encoding attributes"); + if (RDEBUG_ENABLED2) { + RINDENT(); + for (vp = fr_dcursor_current(cursor); + vp != NULL; + vp = fr_dcursor_next(cursor)) { + RDEBUG2("%pP", vp); + } + REXDENT(); + } + + /* + * Encoders advance the cursor, so we just need to feed + * in the next pair. This was originally so we could + * extend the output buffer, but with dbuffs that's + * no longer necessary... we might want to refactor this + * in future. + */ + for (vp = fr_dcursor_head(cursor); + vp != NULL; + vp = fr_dcursor_current(cursor)) { /* + * + * Don't check for internal attributes, the + * encoders can skip them if they need to, and the + * internal encoder can encode anything, as can + * things like CBOR. + * * Don't check the dictionaries. By definition, * vp->da->dict==request->proto_dict, OR else we're * using the internal encoder and encoding a real @@ -4183,22 +4207,22 @@ static xlat_action_t protocol_encode_xlat(TALLOC_CTX *ctx, fr_dcursor_t *out, * as AKA/SIM and DHCPv6 encode "bool"s only if * their value is true. */ - - len = tp_encode->func(&FR_DBUFF_TMP(p, end), cursor, encode_ctx); + len = tp_encode->func(dbuff, cursor, encode_ctx); if (len < 0) { RPEDEBUG("Protocol encoding failed"); + REXDENT(); return XLAT_ACTION_FAIL; } tainted |= vp->vp_tainted; - p += len; } + REXDENT(); /* * Pass the options string back to the caller. */ MEM(encoded = fr_value_box_alloc_null(ctx)); - fr_value_box_memdup(encoded, encoded, NULL, binbuf, (size_t)len, tainted); + fr_value_box_memdup(encoded, encoded, NULL, fr_dbuff_start(dbuff), fr_dbuff_used(dbuff), tainted); fr_dcursor_append(out, encoded); return XLAT_ACTION_DONE; diff --git a/src/tests/keywords/internal-md5 b/src/tests/keywords/internal-md5 new file mode 100644 index 00000000000..8a918dbba12 --- /dev/null +++ b/src/tests/keywords/internal-md5 @@ -0,0 +1,40 @@ +group { + octets data + + reply.Reply-Message := 'foo' + reply.Tag-1.Tunnel-Private-Group-Id := 10 + reply.Tag-2.Tunnel-Private-Group-Id := 20 + reply.Vendor-Specific.FreeRADIUS.Acct-Unique-Session-Id := '0123456789' + + debug_reply + + # Encode the children of the reply list + # This is also a regression test, as %internal.encode used + # to skip every other attribute. + data := %internal.encode('reply.[*]') + + # STOP - Dont just change the MD5 to make the test past. + # Understand _EXACTLY_ what changed, and WARN all other + # core developers that you are making this change and _WHY_. + if ! (%md5(data) == 0x9a9b2606e346135a4e64030a1763a83e) { + test_fail + } + + reply := {} + + # Make sure we decode exactly four attributes + if (%internal.decode(data) != 4) { + test_fail + } + + # ...and that they match what we encoded. + # This is mostly to help debug if the MD5 checksum changes. + if (Reply-Message != 'foo' || + (Tag-1.Tunnel-Private-Group-Id != 10) || + (Tag-2.Tunnel-Private-Group-Id != 20) || + (Vendor-Specific.FreeRADIUS.Acct-Unique-Session-Id != '0123456789')) { + test_fail + } +} + +success