From: Alan T. DeKok Date: Sat, 18 Feb 2023 01:38:01 +0000 (-0500) Subject: allow encoding and decoding of VENDOR attributes in TACACS X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=402469cceee65684873d2e4fad4de5b02a1f1014;p=thirdparty%2Ffreeradius-server.git allow encoding and decoding of VENDOR attributes in TACACS right now the vendor attr has to be passed to the decode function. The encoder should arguably be also passed a vendor, and then only encode that vendor. But for now it's OK --- diff --git a/src/listen/tacacs/proto_tacacs.c b/src/listen/tacacs/proto_tacacs.c index 80ce2451f98..a1b439f5071 100644 --- a/src/listen/tacacs/proto_tacacs.c +++ b/src/listen/tacacs/proto_tacacs.c @@ -195,6 +195,7 @@ static int mod_decode(void const *instance, request_t *request, uint8_t *const d fr_tacacs_packet_t const *pkt = (fr_tacacs_packet_t const *)data; char const *secret; size_t secretlen = 0; + fr_dict_attr_t const *dv = NULL; RHEXDUMP3(data, data_len, "proto_tacacs decode packet"); @@ -230,12 +231,21 @@ static int mod_decode(void const *instance, request_t *request, uint8_t *const d secretlen = talloc_array_length(client->secret) - 1; } + /* + * See if there's a client-specific vendor in the "nas_type" field. + * + * If there's no such vendor, too bad for you. + */ + if (client->nas_type) { + dv = fr_dict_attr_by_name(NULL, fr_dict_root(dict_tacacs), client->nas_type); + } + /* * Note that we don't set a limit on max_attributes here. * That MUST be set and checked in the underlying * transport, via a call to ??? */ - if (fr_tacacs_decode(request->request_ctx, &request->request_pairs, + if (fr_tacacs_decode(request->request_ctx, &request->request_pairs, dv, request->packet->data, request->packet->data_len, NULL, secret, secretlen, &code) < 0) { RPEDEBUG("Failed decoding packet"); diff --git a/src/modules/rlm_tacacs/rlm_tacacs_tcp.c b/src/modules/rlm_tacacs/rlm_tacacs_tcp.c index 36bb999c8fb..c820987b303 100644 --- a/src/modules/rlm_tacacs/rlm_tacacs_tcp.c +++ b/src/modules/rlm_tacacs/rlm_tacacs_tcp.c @@ -565,7 +565,7 @@ static ssize_t decode(TALLOC_CTX *ctx, fr_pair_list_t *reply, uint8_t *response_ * This only fails if the packet is strangely malformed, * or if we run out of memory. */ - packet_len = fr_tacacs_decode(ctx, reply, data, data_len, NULL, inst->secret, inst->secretlen, &code); + packet_len = fr_tacacs_decode(ctx, reply, NULL, data, data_len, NULL, inst->secret, inst->secretlen, &code); if (packet_len < 0) { RPEDEBUG("Failed decoding TACACS+ reply packet"); fr_pair_list_free(reply); diff --git a/src/protocols/tacacs/decode.c b/src/protocols/tacacs/decode.c index 8f3dc05e423..851a2c0a033 100644 --- a/src/protocols/tacacs/decode.c +++ b/src/protocols/tacacs/decode.c @@ -233,9 +233,12 @@ static int tacacs_decode_args(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dict_attr * Argument-List += "name=value" */ if (parent) { - da = fr_dict_attr_by_name(NULL, fr_dict_root(dict_tacacs), (char *) buffer); + da = fr_dict_attr_by_name(NULL, parent, (char *) buffer); if (!da) goto raw; + vp = fr_pair_afrom_da(ctx, da); + if (!vp) goto oom; + /* * If it's OCTETS or STRING type, then just copy the value verbatim, as the * contents are (should be?) binary-safe. But if it's zero length, then don't need to @@ -290,6 +293,7 @@ static int tacacs_decode_args(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dict_attr raw: vp = fr_pair_afrom_da(ctx, attr_tacacs_argument_list); if (!vp) { + oom: fr_strerror_const("Out of Memory"); return -1; } @@ -354,7 +358,7 @@ static int tacacs_decode_field(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dict_att /** * Decode a TACACS+ packet */ -ssize_t fr_tacacs_decode(TALLOC_CTX *ctx, fr_pair_list_t *out, uint8_t const *buffer, size_t buffer_len, +ssize_t fr_tacacs_decode(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dict_attr_t const *vendor, uint8_t const *buffer, size_t buffer_len, const uint8_t *original, char const * const secret, size_t secret_len, int *code) { fr_tacacs_packet_t const *pkt; @@ -810,7 +814,7 @@ ssize_t fr_tacacs_decode(TALLOC_CTX *ctx, fr_pair_list_t *out, uint8_t const *bu /* * Decode 'arg_N' arguments (horrible format) */ - if (tacacs_decode_args(ctx, out, NULL, + if (tacacs_decode_args(ctx, out, vendor, pkt->author_req.arg_cnt, argv, attrs, end) < 0) goto fail; } else if (packet_is_author_reply(pkt)) { @@ -863,7 +867,7 @@ ssize_t fr_tacacs_decode(TALLOC_CTX *ctx, fr_pair_list_t *out, uint8_t const *bu /* * Decode 'arg_N' arguments (horrible format) */ - if (tacacs_decode_args(ctx, out, NULL, + if (tacacs_decode_args(ctx, out, vendor, pkt->author_reply.arg_cnt, argv, attrs, end) < 0) goto fail; } else { @@ -932,7 +936,7 @@ ssize_t fr_tacacs_decode(TALLOC_CTX *ctx, fr_pair_list_t *out, uint8_t const *bu /* * Decode 'arg_N' arguments (horrible format) */ - if (tacacs_decode_args(ctx, out, NULL, + if (tacacs_decode_args(ctx, out, vendor, pkt->acct_req.arg_cnt, argv, attrs, end) < 0) goto fail; } else if (packet_is_acct_reply(pkt)) { @@ -990,8 +994,12 @@ ssize_t fr_tacacs_decode(TALLOC_CTX *ctx, fr_pair_list_t *out, uint8_t const *bu static ssize_t fr_tacacs_decode_proto(TALLOC_CTX *ctx, fr_pair_list_t *out, uint8_t const *data, size_t data_len, void *proto_ctx) { fr_tacacs_ctx_t *test_ctx = talloc_get_type_abort(proto_ctx, fr_tacacs_ctx_t); + fr_dict_attr_t const *dv; + + dv = fr_dict_attr_by_name(NULL, fr_dict_root(dict_tacacs), "Test"); + fr_assert(!dv || (dv->type == FR_TYPE_VENDOR)); - return fr_tacacs_decode(ctx, out, data, data_len, NULL, + return fr_tacacs_decode(ctx, out, dv, data, data_len, NULL, test_ctx->secret, (talloc_array_length(test_ctx->secret)-1), false); } diff --git a/src/protocols/tacacs/encode.c b/src/protocols/tacacs/encode.c index 67c3015a9c7..0386e502ac8 100644 --- a/src/protocols/tacacs/encode.c +++ b/src/protocols/tacacs/encode.c @@ -151,12 +151,9 @@ static uint8_t tacacs_encode_body_arg_cnt(fr_pair_list_t *vps, fr_dict_attr_t co continue; } - /* - * Maybe it's still a TACACS+ attribute? - */ - if (fr_dict_by_da(vp->da) != dict_tacacs) continue; + fr_assert(fr_dict_by_da(vp->da) == dict_tacacs); - if (!((vp->da->attr >= 1000) && (vp->da->attr <= 65000))) continue; + if (vp->da->parent->type != FR_TYPE_VENDOR) continue; arg_cnt++; } @@ -172,7 +169,7 @@ static ssize_t tacacs_encode_body_arg_n(fr_dbuff_t *dbuff, uint8_t arg_cnt, uint for (vp = fr_pair_list_head(vps); vp; - vp = fr_pair_list_next(vps,vp)) { + vp = fr_pair_list_next(vps, vp)) { int len; if (i == 255) break; @@ -192,10 +189,7 @@ static ssize_t tacacs_encode_body_arg_n(fr_dbuff_t *dbuff, uint8_t arg_cnt, uint FR_PROTO_TRACE("arg[%d] --> %s", i, vp->vp_strvalue); len = vp->vp_length; - } else if (fr_dict_by_da(vp->da) != dict_tacacs) { - continue; - - } else if (!((vp->da->attr >= 1000) && (vp->da->attr <= 65000))) { + } else if (!vp->da->parent || (vp->da->parent->type != FR_TYPE_VENDOR)) { continue; } else { diff --git a/src/protocols/tacacs/tacacs.h b/src/protocols/tacacs/tacacs.h index d43d91e1d73..9999a31e2fa 100644 --- a/src/protocols/tacacs/tacacs.h +++ b/src/protocols/tacacs/tacacs.h @@ -337,7 +337,7 @@ ssize_t fr_tacacs_encode(fr_dbuff_t *dbuff, uint8_t const *original, char const int fr_tacacs_code_to_packet(fr_tacacs_packet_t *pkt, uint32_t code); /* decode.c */ -ssize_t fr_tacacs_decode(TALLOC_CTX *ctx, fr_pair_list_t *out, uint8_t const *buffer, size_t buffer_len, +ssize_t fr_tacacs_decode(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dict_attr_t const *vendor, uint8_t const *buffer, size_t buffer_len, UNUSED const uint8_t *original, char const * const secret, size_t secret_len, int *code); int fr_tacacs_packet_to_code(fr_tacacs_packet_t const *pkt); diff --git a/src/tests/unit/protocols/tacacs/dictionary.test b/src/tests/unit/protocols/tacacs/dictionary.test index fddd8643807..0e8c2feb1a3 100644 --- a/src/tests/unit/protocols/tacacs/dictionary.test +++ b/src/tests/unit/protocols/tacacs/dictionary.test @@ -1,9 +1,7 @@ -# -# The numbers here don't matter. For now, just use 30000...40000 -# -# We will document that users can define their own attributes in the space -# 1000...9999 -# -# @todo - define some magical *tacacs* format which doesn't use numbers? -# -ATTRIBUTE addr 60000 ipv4addr +VENDOR Test 0xdeadbeef + +BEGIN-VENDOR Test + +DEFINE addr ipv4addr + +END-VENDOR Test diff --git a/src/tests/unit/protocols/tacacs/typed.txt.ignore b/src/tests/unit/protocols/tacacs/typed.txt similarity index 98% rename from src/tests/unit/protocols/tacacs/typed.txt.ignore rename to src/tests/unit/protocols/tacacs/typed.txt index 6d17359284f..305722ffcf1 100644 --- a/src/tests/unit/protocols/tacacs/typed.txt.ignore +++ b/src/tests/unit/protocols/tacacs/typed.txt @@ -29,7 +29,7 @@ match c0 02 01 00 e1 66 78 e6 00 00 00 35 4b c5 ea 62 13 cc ca a6 6a 03 3c 8e 3f # Authorization - Response: (Client <- Server) # decode-proto c0 02 02 00 e1 66 78 e6 00 00 00 13 02 59 f9 90 38 81 e1 bb 9d a6 13 93 fc 86 7e 4a 14 1c 24 -match Packet.Version-Major = Plus, Packet.Version-Minor = 0, Packet.Packet-Type = Authorization, Packet.Sequence-Number = 2, Packet.Flags = None, Packet.Session-Id = 3781589222, Packet.Length = 19, Packet-Body-Type = Response, Authorization-Status = Pass-Add, Server-Message = "", Data = 0x, addr = 1.2.3.4 +match Packet.Version-Major = Plus, Packet.Version-Minor = 0, Packet.Packet-Type = Authorization, Packet.Sequence-Number = 2, Packet.Flags = None, Packet.Session-Id = 3781589222, Packet.Length = 19, Packet-Body-Type = Response, Authorization-Status = Pass-Add, Server-Message = "", Data = 0x, Test.addr = 1.2.3.4 encode-proto - match c0 02 02 00 e1 66 78 e6 00 00 00 13 02 59 f9 90 38 81 e1 bb 9d a6 13 93 fc 86 7e 4a 14 1c 24