From: Alan T. DeKok Date: Tue, 7 Feb 2023 15:41:12 +0000 (-0500) Subject: get the synthesized packet code from the decrypted packet #4882 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=bbb1277972ea368ffe77730a9bc046efb2d9aedf;p=thirdparty%2Ffreeradius-server.git get the synthesized packet code from the decrypted packet #4882 not from the encrypted packet. --- diff --git a/src/listen/tacacs/proto_tacacs.c b/src/listen/tacacs/proto_tacacs.c index 44fa86753cb..93579d836d0 100644 --- a/src/listen/tacacs/proto_tacacs.c +++ b/src/listen/tacacs/proto_tacacs.c @@ -176,7 +176,7 @@ static int mod_decode(void const *instance, request_t *request, uint8_t *const d fr_io_track_t const *track = talloc_get_type_abort_const(request->async->packet_ctx, fr_io_track_t); fr_io_address_t const *address = track->address; RADCLIENT const *client; - int code; + int code = -1; fr_tacacs_packet_t const *pkt = (fr_tacacs_packet_t const *)data; char const *secret; size_t secretlen = 0; @@ -200,23 +200,6 @@ static int mod_decode(void const *instance, request_t *request, uint8_t *const d return -1; } - /* - * RFC 8907 Section 3.6 says: - * - * If an error occurs but the type of the incoming packet cannot be determined, a packet with the - * identical cleartext header but with a sequence number incremented by one and the length set to - * zero MUST be returned to indicate an error. - * - * This is substantially retarded. It should instead just close the connection. - */ - - code = fr_tacacs_packet_to_code(pkt); - if (code < 0) { - RPEDEBUG("Invalid packet"); - return -1; - } - - request->packet->code = code; request->packet->id = data[2]; // seq_no request->reply->id = data[2]; // seq_no @@ -233,11 +216,24 @@ static int mod_decode(void const *instance, request_t *request, uint8_t *const d */ if (fr_tacacs_decode(request->request_ctx, &request->request_pairs, request->packet->data, request->packet->data_len, - NULL, secret, secretlen) < 0) { + NULL, secret, secretlen, &code) < 0) { RPEDEBUG("Failed decoding packet"); return -1; } + request->packet->code = code; + + /* + * RFC 8907 Section 3.6 says: + * + * If an error occurs but the type of the incoming packet cannot be determined, a packet with the + * identical cleartext header but with a sequence number incremented by one and the length set to + * zero MUST be returned to indicate an error. + * + * This is substantially retarded. It should instead just close the connection. + */ + + /* * Set the rest of the fields. */ diff --git a/src/modules/rlm_tacacs/rlm_tacacs_tcp.c b/src/modules/rlm_tacacs/rlm_tacacs_tcp.c index 4223a92586e..36bb999c8fb 100644 --- a/src/modules/rlm_tacacs/rlm_tacacs_tcp.c +++ b/src/modules/rlm_tacacs/rlm_tacacs_tcp.c @@ -543,12 +543,12 @@ static int8_t request_prioritise(void const *one, void const *two) * - >0 for how many bytes were decoded */ static ssize_t decode(TALLOC_CTX *ctx, fr_pair_list_t *reply, uint8_t *response_code, - udp_handle_t *h, request_t *request, udp_request_t *u, - uint8_t *data, size_t data_len) + udp_handle_t *h, request_t *request, udp_request_t *u, + uint8_t *data, size_t data_len) { rlm_tacacs_tcp_t const *inst = h->thread->inst; ssize_t packet_len; - uint8_t code; + int code; *response_code = 0; /* Initialise to keep the rest of the code happy */ @@ -565,15 +565,13 @@ 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); + packet_len = fr_tacacs_decode(ctx, reply, data, data_len, NULL, inst->secret, inst->secretlen, &code); if (packet_len < 0) { RPEDEBUG("Failed decoding TACACS+ reply packet"); fr_pair_list_free(reply); return -1; } - code = data[1]; - RDEBUG("Received %s ID %d length %ld reply packet on connection %s", fr_tacacs_packet_names[code], code, packet_len, h->name); log_request_pair_list(L_DBG_LVL_2, request, NULL, reply, NULL); diff --git a/src/protocols/tacacs/decode.c b/src/protocols/tacacs/decode.c index ea3abb0a0e2..f7344ff7c53 100644 --- a/src/protocols/tacacs/decode.c +++ b/src/protocols/tacacs/decode.c @@ -338,7 +338,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, - const uint8_t *original, char const * const secret, size_t secret_len) + const uint8_t *original, char const * const secret, size_t secret_len, int *code) { fr_tacacs_packet_t const *pkt; fr_pair_t *vp; @@ -474,6 +474,11 @@ ssize_t fr_tacacs_decode(TALLOC_CTX *ctx, fr_pair_list_t *out, uint8_t const *bu decrypted[3] |= FR_TAC_PLUS_UNENCRYPTED_FLAG; FR_PROTO_HEX_DUMP(decrypted, buffer_len, "fr_tacacs_packet_t (unencrypted)"); + + if (code) { + *code = fr_tacacs_packet_to_code((fr_tacacs_packet_t const *) decrypted); + if (*code < 0) return -1; + } } #ifndef NDEBUG @@ -879,7 +884,7 @@ static ssize_t fr_tacacs_decode_proto(TALLOC_CTX *ctx, fr_pair_list_t *out, uint fr_tacacs_ctx_t *test_ctx = talloc_get_type_abort(proto_ctx, fr_tacacs_ctx_t); return fr_tacacs_decode(ctx, out, data, data_len, NULL, - test_ctx->secret, (talloc_array_length(test_ctx->secret)-1)); + test_ctx->secret, (talloc_array_length(test_ctx->secret)-1), false); } static int _encode_test_ctx(fr_tacacs_ctx_t *proto_ctx) diff --git a/src/protocols/tacacs/tacacs.h b/src/protocols/tacacs/tacacs.h index 07ee64752a5..6d505e7d0a7 100644 --- a/src/protocols/tacacs/tacacs.h +++ b/src/protocols/tacacs/tacacs.h @@ -318,7 +318,7 @@ 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, - UNUSED const uint8_t *original, char const * const secret, size_t secret_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);