From: Alan T. DeKok Date: Thu, 25 Jan 2024 13:21:45 +0000 (-0500) Subject: hoist reply checks to core code X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0684e33fbec46a494d6b2c41fe8baf9350aeb37e;p=thirdparty%2Ffreeradius-server.git hoist reply checks to core code --- diff --git a/src/modules/rlm_radius/rlm_radius_udp.c b/src/modules/rlm_radius/rlm_radius_udp.c index fd628962e02..c7dd69033e0 100644 --- a/src/modules/rlm_radius/rlm_radius_udp.c +++ b/src/modules/rlm_radius/rlm_radius_udp.c @@ -228,25 +228,6 @@ fr_dict_attr_autoload_t rlm_radius_udp_dict_attr[] = { { NULL } }; -/** If we get a reply, the request must come from one of a small - * number of packet types. - */ -static fr_radius_packet_code_t allowed_replies[FR_RADIUS_CODE_MAX] = { - [FR_RADIUS_CODE_ACCESS_ACCEPT] = FR_RADIUS_CODE_ACCESS_REQUEST, - [FR_RADIUS_CODE_ACCESS_CHALLENGE] = FR_RADIUS_CODE_ACCESS_REQUEST, - [FR_RADIUS_CODE_ACCESS_REJECT] = FR_RADIUS_CODE_ACCESS_REQUEST, - - [FR_RADIUS_CODE_ACCOUNTING_RESPONSE] = FR_RADIUS_CODE_ACCOUNTING_REQUEST, - - [FR_RADIUS_CODE_COA_ACK] = FR_RADIUS_CODE_COA_REQUEST, - [FR_RADIUS_CODE_COA_NAK] = FR_RADIUS_CODE_COA_REQUEST, - - [FR_RADIUS_CODE_DISCONNECT_ACK] = FR_RADIUS_CODE_DISCONNECT_REQUEST, - [FR_RADIUS_CODE_DISCONNECT_NAK] = FR_RADIUS_CODE_DISCONNECT_REQUEST, - - [FR_RADIUS_CODE_PROTOCOL_ERROR] = FR_RADIUS_CODE_PROTOCOL_ERROR, /* Any */ -}; - /** Turn a reply code into a module rcode; * */ @@ -1165,27 +1146,6 @@ static decode_fail_t decode(TALLOC_CTX *ctx, fr_pair_list_t *reply, uint8_t *res RHEXDUMP3(data, data_len, "Read packet"); - code = data[0]; - if (!allowed_replies[code]) { - REDEBUG("%s packet received unknown reply code %s", - fr_radius_packet_names[u->code], fr_radius_packet_names[code]); - return DECODE_FAIL_UNKNOWN_PACKET_CODE; - } - - /* - * Protocol error can reply to any packet. - * - * Status-Server can get any reply. - * - * Otherwise the reply code must be associated with the request code we sent. - */ - if ((code != FR_RADIUS_CODE_PROTOCOL_ERROR) && (u->code != FR_RADIUS_CODE_STATUS_SERVER) && - (allowed_replies[code] != u->code)) { - REDEBUG("%s packet received invalid reply code %s", - fr_radius_packet_names[u->code], fr_radius_packet_names[code]); - return DECODE_FAIL_UNKNOWN_PACKET_CODE; - } - common_ctx = (fr_radius_ctx_t) { .secret = inst->secret, .secret_length = talloc_array_length(inst->secret) - 1, @@ -1212,8 +1172,10 @@ static decode_fail_t decode(TALLOC_CTX *ctx, fr_pair_list_t *reply, uint8_t *res } talloc_free(decode_ctx.tmp_ctx); + code = data[0]; + RDEBUG("Received %s ID %d length %ld reply packet on connection %s", - fr_radius_packet_names[code], code, data_len, h->name); + fr_radius_packet_names[code], data[1], 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 fef38148a0b..801c42455a4 100644 --- a/src/protocols/radius/base.c +++ b/src/protocols/radius/base.c @@ -148,6 +148,25 @@ char const *fr_radius_packet_names[FR_RADIUS_CODE_MAX] = { }; +/** If we get a reply, the request must come from one of a small + * number of packet types. + */ +static const fr_radius_packet_code_t allowed_replies[FR_RADIUS_CODE_MAX] = { + [FR_RADIUS_CODE_ACCESS_ACCEPT] = FR_RADIUS_CODE_ACCESS_REQUEST, + [FR_RADIUS_CODE_ACCESS_CHALLENGE] = FR_RADIUS_CODE_ACCESS_REQUEST, + [FR_RADIUS_CODE_ACCESS_REJECT] = FR_RADIUS_CODE_ACCESS_REQUEST, + + [FR_RADIUS_CODE_ACCOUNTING_RESPONSE] = FR_RADIUS_CODE_ACCOUNTING_REQUEST, + + [FR_RADIUS_CODE_COA_ACK] = FR_RADIUS_CODE_COA_REQUEST, + [FR_RADIUS_CODE_COA_NAK] = FR_RADIUS_CODE_COA_REQUEST, + + [FR_RADIUS_CODE_DISCONNECT_ACK] = FR_RADIUS_CODE_DISCONNECT_REQUEST, + [FR_RADIUS_CODE_DISCONNECT_NAK] = FR_RADIUS_CODE_DISCONNECT_REQUEST, + + [FR_RADIUS_CODE_PROTOCOL_ERROR] = FR_RADIUS_CODE_PROTOCOL_ERROR, /* Any */ +}; + /** Do Ascend-Send / Recv-Secret calculation. * * The secret is hidden by xoring with a MD5 digest created from @@ -996,16 +1015,46 @@ ssize_t fr_radius_decode(TALLOC_CTX *ctx, fr_pair_list_t *out, break; default: + no_vector: fr_strerror_const("No authentication vector passed for packet decode"); return -1; } } + if (decode_ctx->request_code) { + int code = packet[0]; + + fr_assert(code < FR_RADIUS_CODE_MAX); /* checked by fr_radius_ok() */ + fr_assert(decode_ctx->request_code < FR_RADIUS_CODE_MAX); /* checked by fr_radius_ok() */ + + if (!allowed_replies[code]) { + fr_strerror_printf("%s packet received unknown reply code %s", + fr_radius_packet_names[decode_ctx->request_code], fr_radius_packet_names[code]); + return DECODE_FAIL_UNKNOWN_PACKET_CODE; + } + + /* + * Protocol error can reply to any packet. + * + * Status-Server can get any reply. + * + * Otherwise the reply code must be associated with the request code we sent. + */ + if ((code != FR_RADIUS_CODE_PROTOCOL_ERROR) && (decode_ctx->request_code != FR_RADIUS_CODE_STATUS_SERVER) && + (allowed_replies[code] != decode_ctx->request_code)) { + fr_strerror_printf("%s packet received invalid reply code %s", + fr_radius_packet_names[decode_ctx->request_code], fr_radius_packet_names[code]); + return DECODE_FAIL_UNKNOWN_PACKET_CODE; + } + } + /* * We can skip verification for dynamic client checks, and where packets are unsigned as with * RADIUS/1.1. */ if (decode_ctx->verify) { + if (!decode_ctx->request_authenticator) goto no_vector; + 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) {