From: Arran Cudbard-Bell <a.cudbardb@freeradius.org>
Date: Tue, 18 Jun 2024 02:48:12 +0000 (-0600)
Subject: Fix "RADIUS checksum" mode in radsniff, when dealing with Access-Requests
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e1ef7481c20e1742dce09f22c23936ceb802bc6f;p=thirdparty%2Ffreeradius-server.git

Fix "RADIUS checksum" mode in radsniff, when dealing with Access-Requests
---

diff --git a/src/bin/radsniff.c b/src/bin/radsniff.c
index fa4685a91cd..03f1bf36b00 100644
--- a/src/bin/radsniff.c
+++ b/src/bin/radsniff.c
@@ -77,8 +77,8 @@ static int rs_useful_codes[] = {
 	FR_RADIUS_CODE_DISCONNECT_ACK,			//!< RFC3575/RFC5176 - Disconnect-Ack (positive)
 	FR_RADIUS_CODE_DISCONNECT_NAK,			//!< RFC3575/RFC5176 - Disconnect-Nak (not willing to perform)
 	FR_RADIUS_CODE_COA_REQUEST,			//!< RFC3575/RFC5176 - CoA-Request
-	FR_RADIUS_CODE_COA_ACK,			//!< RFC3575/RFC5176 - CoA-Ack (positive)
-	FR_RADIUS_CODE_COA_NAK,			//!< RFC3575/RFC5176 - CoA-Nak (not willing to perform)
+	FR_RADIUS_CODE_COA_ACK,				//!< RFC3575/RFC5176 - CoA-Ack (positive)
+	FR_RADIUS_CODE_COA_NAK,				//!< RFC3575/RFC5176 - CoA-Nak (not willing to perform)
 };
 
 static fr_table_num_sorted_t const rs_events[] = {
@@ -1467,7 +1467,7 @@ static void rs_packet_process(uint64_t count, rs_event_t *event, struct pcap_pkt
 			return;
 		}
 
-		if (conf->verify_radius_authenticator && original) {
+		if (conf->verify_radius_authenticator) {
 			int ret;
 			FILE *log_fp = fr_log_fp;
 
@@ -1600,6 +1600,7 @@ static void rs_packet_process(uint64_t count, rs_event_t *event, struct pcap_pkt
 
 		if (conf->verify_radius_authenticator) {
 			switch (packet->code) {
+			case FR_RADIUS_CODE_ACCESS_REQUEST: /* Even though this is just random bytes, we still might need to check Message-Authenticator */
 			case FR_RADIUS_CODE_ACCOUNTING_REQUEST:
 			case FR_RADIUS_CODE_COA_REQUEST:
 			case FR_RADIUS_CODE_DISCONNECT_REQUEST:
diff --git a/src/protocols/radius/base.c b/src/protocols/radius/base.c
index ce7c2d49009..c1cdc15ed9e 100644
--- a/src/protocols/radius/base.c
+++ b/src/protocols/radius/base.c
@@ -784,10 +784,8 @@ int fr_radius_verify(uint8_t *packet, uint8_t const *vector,
 	}
 
 	/*
-	 *	Implement verification as a signature, followed by
-	 *	checking our signature against the sent one.  This is
-	 *	slightly more CPU work than having verify-specific
-	 *	functions, but it ends up being cleaner in the code.
+	 *	Overwrite the contents of Message-Authenticator
+	 *	with the one we calculate.
 	 */
 	rcode = fr_radius_sign(packet, vector, secret, secret_len);
 	if (rcode < 0) {
@@ -801,6 +799,10 @@ int fr_radius_verify(uint8_t *packet, uint8_t const *vector,
 	 *	If it's invalid, restore the original
 	 *	Message-Authenticator and Request Authenticator
 	 *	fields.
+	 *
+	 *	If it's valid the original and calculated
+	 *	message authenticators are the same, so we don't
+	 *	need to do anything.
 	 */
 	if ((msg < end) &&
 	    (fr_digest_cmp(message_authenticator, msg + 2, sizeof(message_authenticator)) != 0)) {
@@ -1141,9 +1143,7 @@ int fr_radius_global_init(void)
 
 void fr_radius_global_free(void)
 {
-	fr_assert(instance_count > 0);
-
-	if (--instance_count > 0) return;
+	if (--instance_count != 0) return;
 
 	fr_dict_autofree(libfreeradius_radius_dict);
 }