From: Alan T. DeKok <aland@freeradius.org>
Date: Mon, 28 Apr 2025 15:46:50 +0000 (-0400)
Subject: packet verification is handled in the BIO callbacks
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c60f89b45cd23e97b725736acbc5d5ef6c1d2a51;p=thirdparty%2Ffreeradius-server.git

packet verification is handled in the BIO callbacks

by the rlm_radius_verify() function.

ideally, we should also move any tracking checks and decode
routines to that function, too
---

diff --git a/src/modules/rlm_radius/bio.c b/src/modules/rlm_radius/bio.c
index 4f36a03b2f..40b5b19cd6 100644
--- a/src/modules/rlm_radius/bio.c
+++ b/src/modules/rlm_radius/bio.c
@@ -37,13 +37,6 @@
 //#include "rlm_radius.h"
 #include "track.h"
 
-/*
- * Macro to simplify checking packets before calling decode(), so that
- * it gets a known valid length and no longer calls fr_radius_ok() itself.
- */
-#define check(_handle, _len_p) fr_radius_ok((_handle)->buffer, (size_t *)(_len_p), \
-					    (_handle)->ctx.inst->max_attributes, false, NULL)
-
 typedef struct {
 	char const		*module_name;	//!< the module that opened the connection
 	rlm_radius_t const	*inst;		//!< our instance
@@ -461,11 +454,7 @@ static void conn_init_readable(fr_event_list_t *el, UNUSED int fd, UNUSED int fl
 	 *	the response timer take care of progressing the
 	 *	connection attempt.
 	 */
-	if (slen < RADIUS_HEADER_LENGTH) {
-		ERROR("%s - Packet too short, expected at least %zu bytes got %zd bytes",
-		      h->ctx.module_name, (size_t)RADIUS_HEADER_LENGTH, slen);
-		return;
-	}
+	fr_assert(slen >= RADIUS_HEADER_LENGTH); /* checked in verify */
 
 	if (u->id != h->buffer[1]) {
 		ERROR("%s - Received response with incorrect or expired ID.  Expected %u, got %u",
@@ -473,8 +462,6 @@ static void conn_init_readable(fr_event_list_t *el, UNUSED int fd, UNUSED int fl
 		return;
 	}
 
-	if (!check(h, &slen)) return;
-
 	if (decode(h, &reply, &code,
 		   h, h->status_request, h->status_u, u->packet + RADIUS_AUTH_VECTOR_OFFSET,
 		   h->buffer, slen) != DECODE_FAIL_NONE) return;
@@ -1953,11 +1940,7 @@ static void request_demux(UNUSED fr_event_list_t *el, trunk_connection_t *tconn,
 			return;
 		}
 
-		if (slen < RADIUS_HEADER_LENGTH) {
-			ERROR("%s - Packet too short, expected at least %zu bytes got %zd bytes",
-			      h->ctx.module_name, (size_t)RADIUS_HEADER_LENGTH, slen);
-			continue;
-		}
+		fr_assert(slen >= RADIUS_HEADER_LENGTH); /* checked in verify */
 
 		/*
 		 *	Note that we don't care about packet codes.  All
@@ -1977,14 +1960,8 @@ static void request_demux(UNUSED fr_event_list_t *el, trunk_connection_t *tconn,
 		fr_assert(u == treq->preq);
 
 		/*
-		 *	Validate and decode the incoming packet
+		 *	Decode the incoming packet.
 		 */
-
-		if (!check(h, &slen)) {
-			RWARN("Ignoring malformed packet");
-			continue;
-		}
-
 		reason = decode(request->reply_ctx, &reply, &code, h, request, u, rr->vector, h->buffer, (size_t)slen);
 		if (reason != DECODE_FAIL_NONE) continue;