From: Alan T. DeKok Date: Thu, 25 Jan 2024 01:04:19 +0000 (-0500) Subject: move to using decode_ctx for public APIs, v2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0d4b93441f985a3a78281065633a72db5ef996a1;p=thirdparty%2Ffreeradius-server.git move to using decode_ctx for public APIs, v2 --- diff --git a/src/listen/radius/proto_radius.c b/src/listen/radius/proto_radius.c index d26e516fdd7..ee0f1bddac6 100644 --- a/src/listen/radius/proto_radius.c +++ b/src/listen/radius/proto_radius.c @@ -201,7 +201,9 @@ static int mod_decode(UNUSED void const *instance, request_t *request, uint8_t * { 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; - fr_client_t const *client; + fr_client_t const *client; + fr_radius_ctx_t common_ctx; + fr_radius_decode_ctx_t decode_ctx; fr_assert(data[0] < FR_RADIUS_CODE_MAX); @@ -214,19 +216,24 @@ static int mod_decode(UNUSED void const *instance, request_t *request, uint8_t * client = address->radclient; - /* - * !client->active means a fake packet defining a dynamic client - so there will - * be no secret defined yet - so can't verify. - */ - if (client->active && - fr_radius_verify(data, NULL, (uint8_t const *) client->secret, talloc_array_length(client->secret) - 1, - client->message_authenticator) < 0) { - RPEDEBUG("Failed verifying packet signature."); - return -1; - } + common_ctx = (fr_radius_ctx_t) { + .secret = client->secret, + .secret_length = talloc_array_length(client->secret) - 1, + }; + + decode_ctx = (fr_radius_decode_ctx_t) { + .common = &common_ctx, + .tmp_ctx = talloc(request, uint8_t), + /* decode figures out request_authenticator */ + .end = data + data_len, + .verify = client->active, + .require_message_authenticator = client->message_authenticator, + }; /* - * Hacks for now until we have a lower-level decode routine. + * The verify() routine over-writes the request packet vector. + * + * @todo - That needs to be changed. */ request->packet->code = data[0]; request->packet->id = data[1]; @@ -237,16 +244,16 @@ static int mod_decode(UNUSED void const *instance, request_t *request, uint8_t * request->packet->data_len = data_len; /* - * 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 fr_radius_ok(). + * !client->active means a fake packet defining a dynamic client - so there will + * be no secret defined yet - so can't verify. */ if (fr_radius_decode(request->request_ctx, &request->request_pairs, - request->packet->data, request->packet->data_len, NULL, - client->secret, talloc_array_length(client->secret) - 1) < 0) { - RPEDEBUG("Failed decoding packet"); + data, data_len, &decode_ctx) < 0) { + talloc_free(decode_ctx.tmp_ctx); + RPEDEBUG("Failed reading packet"); return -1; } + talloc_free(decode_ctx.tmp_ctx); /* * Set the rest of the fields. diff --git a/src/modules/rlm_radius/rlm_radius_udp.c b/src/modules/rlm_radius/rlm_radius_udp.c index a5e6a2f6745..fd628962e02 100644 --- a/src/modules/rlm_radius/rlm_radius_udp.c +++ b/src/modules/rlm_radius/rlm_radius_udp.c @@ -1157,14 +1157,13 @@ static decode_fail_t decode(TALLOC_CTX *ctx, fr_pair_list_t *reply, uint8_t *res uint8_t *data, size_t data_len) { rlm_radius_udp_t const *inst = h->thread->inst; - size_t packet_len; uint8_t code; + fr_radius_ctx_t common_ctx; + fr_radius_decode_ctx_t decode_ctx; *response_code = 0; /* Initialise to keep the rest of the code happy */ - packet_len = data_len; - - RHEXDUMP3(data, packet_len, "Read packet"); + RHEXDUMP3(data, data_len, "Read packet"); code = data[0]; if (!allowed_replies[code]) { @@ -1187,26 +1186,34 @@ static decode_fail_t decode(TALLOC_CTX *ctx, fr_pair_list_t *reply, uint8_t *res return DECODE_FAIL_UNKNOWN_PACKET_CODE; } - if (fr_radius_verify(data, request_authenticator, - (uint8_t const *) inst->secret, talloc_array_length(inst->secret) - 1, false) < 0) { - RPWDEBUG("Ignoring response with invalid signature"); - return DECODE_FAIL_MA_INVALID; - } + common_ctx = (fr_radius_ctx_t) { + .secret = inst->secret, + .secret_length = talloc_array_length(inst->secret) - 1, + }; + + decode_ctx = (fr_radius_decode_ctx_t) { + .common = &common_ctx, + .request_code = u->code, + .request_authenticator = request_authenticator, + .tmp_ctx = talloc(ctx, uint8_t), + .end = data + data_len, + .verify = true, + }; /* - * Decode the attributes, in the context of the reply. - * This only fails if the packet is strangely malformed, - * or if we run out of memory. + * !client->active means a fake packet defining a dynamic client - so there will + * be no secret defined yet - so can't verify. */ - if (fr_radius_decode(ctx, reply, data, packet_len, request_authenticator, - inst->secret, talloc_array_length(inst->secret) - 1) < 0) { - REDEBUG("Failed decoding attributes for packet"); - fr_pair_list_free(reply); + if (fr_radius_decode(request->request_ctx, &request->request_pairs, + data, data_len, &decode_ctx) < 0) { + talloc_free(decode_ctx.tmp_ctx); + RPEDEBUG("Failed reading packet"); return DECODE_FAIL_UNKNOWN; } + talloc_free(decode_ctx.tmp_ctx); RDEBUG("Received %s ID %d length %ld reply packet on connection %s", - fr_radius_packet_names[code], code, packet_len, h->name); + fr_radius_packet_names[code], code, data_len, h->name); log_request_pair_list(L_DBG_LVL_2, request, NULL, reply, NULL); *response_code = code; diff --git a/src/protocols/radius/base.c b/src/protocols/radius/base.c index 40246c2928d..2d5c5b31032 100644 --- a/src/protocols/radius/base.c +++ b/src/protocols/radius/base.c @@ -973,25 +973,44 @@ ssize_t fr_radius_encode_dbuff(fr_dbuff_t *dbuff, uint8_t const *original, return fr_dbuff_set(dbuff, &work_dbuff); } -/** Decode a raw RADIUS packet into VPs. - * - */ -ssize_t fr_radius_decode(TALLOC_CTX *ctx, fr_pair_list_t *out, - uint8_t const *packet, size_t packet_len, uint8_t const *vector, - char const *secret, size_t secret_len) +ssize_t fr_radius_decode(TALLOC_CTX *ctx, fr_pair_list_t *out, + uint8_t *packet, size_t packet_len, + fr_radius_decode_ctx_t *decode_ctx) { ssize_t slen; uint8_t const *attr, *end; - fr_radius_ctx_t common_ctx = {}; - fr_radius_decode_ctx_t packet_ctx = {}; + static const uint8_t zeros[RADIUS_AUTH_VECTOR_LENGTH] = {}; - common_ctx.secret = secret; - common_ctx.secret_length = secret_len; - memcpy(common_ctx.vector, vector ? vector : packet + 4, sizeof(common_ctx.vector)); + if (!decode_ctx->request_authenticator) { + switch (packet[0]) { + case FR_RADIUS_CODE_ACCESS_REQUEST: + case FR_RADIUS_CODE_STATUS_SERVER: + decode_ctx->request_authenticator = packet + 4; + break; - packet_ctx.common = &common_ctx; - packet_ctx.tmp_ctx = talloc_init_const("tmp"); - packet_ctx.end = packet + packet_len; + case FR_RADIUS_CODE_ACCOUNTING_REQUEST: + case FR_RADIUS_CODE_COA_REQUEST: + case FR_RADIUS_CODE_DISCONNECT_REQUEST: + decode_ctx->request_authenticator = zeros; + break; + + default: + fr_strerror_const("No authentication vector passed for packet decode"); + return -1; + } + } + + /* + * We can skip verification for dynamic client checks, and where packets are unsigned as with + * RADIUS/1.1. + */ + if (decode_ctx->verify) { + if (fr_radius_verify(packet, decode_ctx->request_authenticator, + (uint8_t const *) decode_ctx->common->secret, decode_ctx->common->secret_length, + decode_ctx->require_message_authenticator) < 0) { + return -1; + } + } attr = packet + 20; end = packet + packet_len; @@ -1001,34 +1020,28 @@ ssize_t fr_radius_decode(TALLOC_CTX *ctx, fr_pair_list_t *out, * he doesn't, all hell breaks loose. */ while (attr < end) { - slen = fr_radius_decode_pair(ctx, out, attr, (end - attr), &packet_ctx); - if (slen < 0) { - fail: - talloc_free(packet_ctx.tmp_ctx); - talloc_free(packet_ctx.tags); - return slen; - } + slen = fr_radius_decode_pair(ctx, out, attr, (end - attr), decode_ctx); + if (slen < 0) return slen; /* * If slen is larger than the room in the packet, * all kinds of bad things happen. */ if (!fr_cond_assert(slen <= (end - attr))) { - goto fail; + return -slen; } attr += slen; - talloc_free_children(packet_ctx.tmp_ctx); + talloc_free_children(decode_ctx->tmp_ctx); } /* * We've parsed the whole packet, return that. */ - talloc_free(packet_ctx.tmp_ctx); - talloc_free(packet_ctx.tags); return packet_len; } + int fr_radius_init(void) { if (instance_count > 0) { diff --git a/src/protocols/radius/decode.c b/src/protocols/radius/decode.c index fdf3c77f69d..60b61fd58c7 100644 --- a/src/protocols/radius/decode.c +++ b/src/protocols/radius/decode.c @@ -1624,8 +1624,10 @@ ssize_t fr_radius_decode_pair_value(TALLOC_CTX *ctx, fr_pair_list_t *out, * User-Password */ case FLAG_ENCRYPT_USER_PASSWORD: + if (!packet_ctx->request_authenticator) goto raw; + fr_radius_decode_password((char *)buffer, attr_len, - packet_ctx->common->secret, packet_ctx->common->vector); + packet_ctx->common->secret, packet_ctx->request_authenticator); buffer[253] = '\0'; /* @@ -1659,8 +1661,10 @@ ssize_t fr_radius_decode_pair_value(TALLOC_CTX *ctx, fr_pair_list_t *out, */ case FLAG_TAGGED_TUNNEL_PASSWORD: case FLAG_ENCRYPT_TUNNEL_PASSWORD: + if (!packet_ctx->request_authenticator) goto raw; + if (fr_radius_decode_tunnel_password(buffer, &data_len, - packet_ctx->common->secret, packet_ctx->common->vector, + packet_ctx->common->secret, packet_ctx->request_authenticator, packet_ctx->tunnel_password_zeros) < 0) { goto raw; } @@ -1670,9 +1674,11 @@ ssize_t fr_radius_decode_pair_value(TALLOC_CTX *ctx, fr_pair_list_t *out, * Ascend-Send-Secret * Ascend-Receive-Secret */ - case FLAG_ENCRYPT_ASCEND_SECRET: + case FLAG_ENCRYPT_ASCEND_SECRET: + if (!packet_ctx->request_authenticator) goto raw; + fr_radius_ascend_secret(&FR_DBUFF_TMP(buffer, sizeof(buffer)), p, data_len, - packet_ctx->common->secret, packet_ctx->common->vector); + packet_ctx->common->secret, packet_ctx->request_authenticator); buffer[RADIUS_AUTH_VECTOR_LENGTH] = '\0'; data_len = strlen((char *) buffer); break; @@ -2079,7 +2085,7 @@ static int decode_test_ctx(void **out, TALLOC_CTX *ctx) test_ctx->common->secret = talloc_strdup(test_ctx->common, "testing123"); test_ctx->common->secret_length = talloc_array_length(test_ctx->common->secret); - memcpy(test_ctx->common->vector, vector, sizeof(test_ctx->common->vector)); + test_ctx->request_authenticator = vector; test_ctx->tmp_ctx = talloc_zero(test_ctx, uint8_t); talloc_set_destructor(test_ctx, _test_ctx_free); @@ -2121,8 +2127,7 @@ static ssize_t fr_radius_decode_proto(TALLOC_CTX *ctx, fr_pair_list_t *out, test_ctx->end = data + packet_len; - return fr_radius_decode(ctx, out, data, packet_len, test_ctx->common->vector, - test_ctx->common->secret, talloc_array_length(test_ctx->common->secret) - 1); + return fr_radius_decode(ctx, out, UNCONST(uint8_t *, data), packet_len, test_ctx); } static ssize_t decode_pair(TALLOC_CTX *ctx, fr_pair_list_t *out, NDEBUG_UNUSED fr_dict_attr_t const *parent, diff --git a/src/protocols/radius/radius.h b/src/protocols/radius/radius.h index 0c3acf4240d..c8b5019a2d1 100644 --- a/src/protocols/radius/radius.h +++ b/src/protocols/radius/radius.h @@ -102,6 +102,54 @@ enum { #define flag_long_extended(_flags) (!(_flags)->extra && (_flags)->subtype == FLAG_LONG_EXTENDED_ATTR) #define flag_tunnel_password(_flags) (!(_flags)->extra && (((_flags)->subtype == FLAG_ENCRYPT_TUNNEL_PASSWORD) || ((_flags)->subtype == FLAG_TAGGED_TUNNEL_PASSWORD))) +typedef struct { + fr_pair_t *parent; + fr_dcursor_t cursor; +} fr_radius_tag_ctx_t; + +typedef struct { + char const *secret; + size_t secret_length; + + bool add_proxy_state; //!< do we add a Proxy-State? + uint64_t my_proxy_state; //!< if so, this is its value + + uint32_t acct_delay_time; //!< additional time to add to acct_delay_time + + uint8_t vector[RADIUS_AUTH_VECTOR_LENGTH]; //!< vector for authenticating the reply +} fr_radius_ctx_t; + +typedef struct { + fr_radius_ctx_t *common; + + fr_fast_rand_t rand_ctx; //!< for tunnel passwords + int salt_offset; //!< for tunnel passwords + + uint8_t tag; //!< current tag for encoding + + bool disallow_tunnel_passwords; //!< not all packets can have tunnel passwords + bool seen_message_authenticator; +} fr_radius_encode_ctx_t; + +typedef struct { + fr_radius_ctx_t *common; + + uint8_t const *request_authenticator; + + TALLOC_CTX *tmp_ctx; //!< for temporary things cleaned up during decoding + uint8_t const *end; //!< end of the packet + + uint8_t request_code; //!< original code for the request. + + bool tunnel_password_zeros; //!< check for trailing zeros on decode + bool verify; //!< can skip verify for dynamic clients + bool require_message_authenticator; + + fr_radius_tag_ctx_t **tags; //!< for decoding tagged attributes + fr_pair_list_t *tag_root; //!< Where to insert tag attributes. + TALLOC_CTX *tag_root_ctx; //!< Where to allocate new tag attributes. +} fr_radius_decode_ctx_t; + /* * protocols/radius/base.c */ @@ -124,8 +172,8 @@ ssize_t fr_radius_encode_dbuff(fr_dbuff_t *dbuff, uint8_t const *original, char const *secret, UNUSED size_t secret_len, int code, int id, fr_pair_list_t *vps); ssize_t fr_radius_decode(TALLOC_CTX *ctx, fr_pair_list_t *out, - uint8_t const *packet, size_t packet_len, uint8_t const *vector, - char const *secret, UNUSED size_t secret_len) CC_HINT(nonnull(1,2,3,6)); + uint8_t *packet, size_t packet_len, + fr_radius_decode_ctx_t *decode_ctx) CC_HINT(nonnull); int fr_radius_init(void); @@ -156,47 +204,6 @@ int fr_radius_packet_send(fr_radius_packet_t *packet, fr_pair_list_t *list, #define fr_radius_packet_log_hex(_log, _packet) _fr_radius_packet_log_hex(_log, _packet, __FILE__, __LINE__) void _fr_radius_packet_log_hex(fr_log_t const *log, fr_radius_packet_t const *packet, char const *file, int line) CC_HINT(nonnull); -typedef struct { - fr_pair_t *parent; - fr_dcursor_t cursor; -} fr_radius_tag_ctx_t; - -typedef struct { - char const *secret; - size_t secret_length; - - bool add_proxy_state; //!< do we add a Proxy-State? - uint64_t my_proxy_state; //!< if so, this is its value - - uint32_t acct_delay_time; //!< additional time to add to acct_delay_time - - uint8_t vector[RADIUS_AUTH_VECTOR_LENGTH]; //!< vector for authenticating the reply -} fr_radius_ctx_t; - -typedef struct { - fr_radius_ctx_t *common; - - fr_fast_rand_t rand_ctx; //!< for tunnel passwords - int salt_offset; //!< for tunnel passwords - - uint8_t tag; //!< current tag for encoding - bool disallow_tunnel_passwords; //!< not all packets can have tunnel passwords - bool seen_message_authenticator; -} fr_radius_encode_ctx_t; - -typedef struct { - fr_radius_ctx_t *common; - - TALLOC_CTX *tmp_ctx; //!< for temporary things cleaned up during decoding - uint8_t const *end; //!< end of the packet - - bool tunnel_password_zeros; //!< check for trailing zeros on decode - - fr_radius_tag_ctx_t **tags; //!< for decoding tagged attributes - fr_pair_list_t *tag_root; //!< Where to insert tag attributes. - TALLOC_CTX *tag_root_ctx; //!< Where to allocate new tag attributes. -} fr_radius_decode_ctx_t; - /* * protocols/radius/abinary.c */