aggregate tags across the entire packet

not just one attribute

the previous tests were decoding lists of pairs, which worked.
updating the tests to decoding packets should the issue.

note that packet encode with tags is still failing.  That will be
fixed next.

diff --git a/src/protocols/radius/base.c b/src/protocols/radius/base.c
index a0ca7b1b10cda39cad6cd05d83e1452534352ef8..7f452d77e76b04b3b1c99789c379c2c2abc89dc6 100644 (file)
--- a/src/protocols/radius/base.c
+++ b/src/protocols/radius/base.c
@@ -1076,13 +1076,13 @@ ssize_t fr_radius_decode(TALLOC_CTX *ctx, uint8_t const *packet, size_t packet_l
 
                attr += slen;
                talloc_free_children(packet_ctx.tmp_ctx);
-               packet_ctx.tags = NULL;
        }
 
        /*
         *      We've parsed the whole packet, return that.
         */
        talloc_free(packet_ctx.tmp_ctx);
+       talloc_free(packet_ctx.tags);
        return packet_len;
 }
 

diff --git a/src/protocols/radius/decode.c b/src/protocols/radius/decode.c
index d85c4ec03683736450eeab74791272f4d1453405..b4724efc9f9794d625c584756c848e7c7182c36e 100644 (file)
--- a/src/protocols/radius/decode.c
+++ b/src/protocols/radius/decode.c
@@ -1034,10 +1034,6 @@ ssize_t fr_radius_decode_pair_value(TALLOC_CTX *ctx, fr_cursor_t *cursor, fr_dic
                return -1;
        }
 
-#ifdef __clang_analyzer__
-       if (!packet_ctx || !packet_ctx->tmp_ctx) return -1;
-#endif
-
        FR_PROTO_HEX_DUMP(data, attr_len, "%s", __FUNCTION__ );
 
        FR_PROTO_TRACE("Parent %s len %zu ... %zu", parent->name, attr_len, packet_len);
@@ -1087,12 +1083,27 @@ ssize_t fr_radius_decode_pair_value(TALLOC_CTX *ctx, fr_cursor_t *cursor, fr_dic
                        }
 
                        p = buffer;
-               }
+
+               } /* else the field is >=0x20, so it's not a tag */
        }
 
        if (tag) {
+               /*
+                *      The EAP chbind code calls us with a NULL
+                *      packet ctx.  Which means that we can't decode
+                *      tags.  Which is OK, because the channel
+                *      binding attributes used there don't have tags.
+                */
+               if (!packet_ctx) goto raw;
+
                if (!packet_ctx->tags) {
-                       packet_ctx->tags = talloc_zero_array(packet_ctx->tmp_ctx, fr_radius_tag_ctx_t *, 32);
+                       /*
+                        *      This should NOT be packet_ctx.tmp_ctx,
+                        *      as that is freed after decoding every
+                        *      packet.  We wish to aggregate the tags
+                        *      across multiple attributes.
+                        */
+                       packet_ctx->tags = talloc_zero_array(NULL, fr_radius_tag_ctx_t *, 32);
                        if (!packet_ctx->tags) {
                                fr_pair_list_free(&vp);
                                fr_strerror_printf("%s: Internal sanity check %d", __FUNCTION__, __LINE__);
@@ -1421,6 +1432,8 @@ ssize_t fr_radius_decode_pair_value(TALLOC_CTX *ctx, fr_cursor_t *cursor, fr_dic
        {
                fr_dict_attr_t *raw;
 
+               if (!packet_ctx) goto raw;
+
                /*
                 *      Create an unknown attribute, and decode it as
                 *      "octets".  Note that we have to account for
@@ -1436,6 +1449,7 @@ ssize_t fr_radius_decode_pair_value(TALLOC_CTX *ctx, fr_cursor_t *cursor, fr_dic
                if (!child) goto raw;
 
                raw->flags.is_raw = 1;
+
                /*
                 *      "long" extended.  Decode the value.
                 */
@@ -1478,6 +1492,7 @@ ssize_t fr_radius_decode_pair_value(TALLOC_CTX *ctx, fr_cursor_t *cursor, fr_dic
        if (!tag) {
                vp = fr_pair_afrom_da(ctx, parent);
        } else {
+               fr_assert(packet_ctx != NULL);
                fr_assert(packet_ctx->tags != NULL);
                fr_assert(packet_ctx->tags[tag] != NULL);
                vp = fr_pair_afrom_da(packet_ctx->tags[tag]->parent, parent);
@@ -1593,6 +1608,7 @@ done:
                return attr_len;
        }
 
+       fr_assert(packet_ctx != NULL);
        fr_assert(packet_ctx->tags != NULL);
        fr_assert(packet_ctx->tags[tag] != NULL);
        fr_cursor_append(&packet_ctx->tags[tag]->cursor, vp);

diff --git a/src/protocols/radius/packet.c b/src/protocols/radius/packet.c
index 94ab1d85a98fdf9a84af24e0d1b8b2c549580ca4..ab60c2c8d89de6e979473e55c2f841255c2477e8 100644 (file)
--- a/src/protocols/radius/packet.c
+++ b/src/protocols/radius/packet.c
@@ -245,6 +245,7 @@ int fr_radius_packet_decode(fr_radius_packet_t *packet, fr_pair_list_t *list,
         */
        fr_rand_seed(packet->data, RADIUS_HEADER_LENGTH);
        talloc_free(packet_ctx.tmp_ctx);
+       talloc_free(packet_ctx.tags);
 
        return 0;
 }

diff --git a/src/tests/unit/protocols/radius/rfc2868.txt b/src/tests/unit/protocols/radius/rfc2868.txt
index 0fc7775db8130f4f4f435da178584d373b605b13..62e4a1b11bb1185635d55e0f22fa8ecc71a6afba 100644 (file)
--- a/src/tests/unit/protocols/radius/rfc2868.txt
+++ b/src/tests/unit/protocols/radius/rfc2868.txt
@@ -72,6 +72,18 @@ match Tag-1 = { Tunnel-Type = PPTP, Tunnel-Medium-Type = IPv4 }, Tag-2 = { Tunne
 encode-pair -
 match 40 06 01 00 00 01 41 06 01 00 00 01 40 06 02 00 00 01
 
+#
+#  Tags are aggregated across the entire packet, too.
+#
+decode-proto 01 01 00 26 00000000000000000000000000000000 40 06 01 00 00 01 40 06 02 00 00 01 41 06 01 00 00 01
+match Packet-Type = Access-Request, Packet-Authentication-Vector = 0x00000000000000000000000000000000, Tag-1 = { Tunnel-Type = PPTP, Tunnel-Medium-Type = IPv4 }, Tag-2 = { Tunnel-Type = PPTP }
+
+#
+#  This currently fails.  :(
+#
+#encode-proto -
+#match 01 00 00 14 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+
 # Invalid tag on decoding
 decode-pair 40 06 20 00 00 01
 match raw.Tunnel-Type = 0x20000001
@@ -96,4 +108,4 @@ decode-pair -
 match Tunnel-Client-Endpoint = "\001\002\003"
 
 count
-match 40
+match 42