From: Arran Cudbard-Bell <a.cudbardb@freeradius.org>
Date: Wed, 18 Oct 2023 01:37:15 +0000 (-0600)
Subject: Tweaks to the new radius_request_verify function
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a0b645e8a699542188bee4895b7889d6d617ac95;p=thirdparty%2Ffreeradius-server.git

Tweaks to the new radius_request_verify function

Rename to radius_secret_verify, as that's what it's doing.  The request has already been verified, we're trying different secrets against the request.
---

diff --git a/src/process/radius/base.c b/src/process/radius/base.c
index c2756dc43d2..2d8b4173510 100644
--- a/src/process/radius/base.c
+++ b/src/process/radius/base.c
@@ -31,6 +31,8 @@
 #include <freeradius-devel/server/pair.h>
 #include <freeradius-devel/server/protocol.h>
 #include <freeradius-devel/server/state.h>
+#include <freeradius-devel/server/log.h>
+#include <freeradius-devel/unlang/xlat.h>
 
 #include <freeradius-devel/unlang/module.h>
 #include <freeradius-devel/unlang/interpret.h>
@@ -940,7 +942,7 @@ static unlang_action_t mod_process(rlm_rcode_t *p_result, module_ctx_t const *mc
 	return state->recv(p_result, mctx, request);
 }
 
-static xlat_arg_parser_t const xlat_func_radius_request_verify_args[] = {
+static xlat_arg_parser_t const xlat_func_radius_secret_verify_args[] = {
         { .required = true, .single = true, .type = FR_TYPE_OCTETS },
         XLAT_ARG_PARSER_TERMINATOR
 };
@@ -952,23 +954,46 @@ static xlat_arg_parser_t const xlat_func_radius_request_verify_args[] = {
  *
  * Example:
 @verbatim
-%radius_request_verify(<secret>)
+%radius_secret_verify(<secret>)
 @endverbatim
  *
  * @ingroup xlat_functions
  */
-static xlat_action_t xlat_func_radius_request_verify(TALLOC_CTX *ctx, fr_dcursor_t *out, UNUSED xlat_ctx_t const *xctx,
-                                                     request_t *request, fr_value_box_list_t *args)
+static xlat_action_t xlat_func_radius_secret_verify(TALLOC_CTX *ctx, fr_dcursor_t *out, UNUSED xlat_ctx_t const *xctx,
+                                                    request_t *request, fr_value_box_list_t *args)
 {
 	fr_value_box_t  *secret, *vb;
+	int		ret;
+	bool		require_ma = false;
 
 	XLAT_ARGS(args, &secret);
 
 	if (request->dict != dict_radius) return XLAT_ACTION_FAIL;
 
 	MEM(vb = fr_value_box_alloc(ctx, FR_TYPE_BOOL, NULL));
-	vb->vb_bool = (fr_radius_verify(request->packet->data, NULL, secret->vb_octets,
-                                        secret->vb_length, true) == 0) ? true : false;
+
+	/*
+	 *	Only Access-Requests require a Message-Authenticator.
+	 *	All the other packet types are signed using the
+	 *	authenticator field.
+	 */
+	if (request->packet->code == FR_RADIUS_CODE_ACCESS_REQUEST) require_ma = true;
+
+	ret = fr_radius_verify(request->packet->data, NULL, secret->vb_octets, secret->vb_length, require_ma);
+	switch (ret) {
+	case 0:
+		vb->vb_bool = true;
+		break;
+
+	case -1:
+		RPEDEBUG("Failed verifying secret");
+		return XLAT_ACTION_FAIL;
+
+	case -2:
+		RPDEBUG2("Provided secret was not used to sign this packet");
+		vb->vb_bool = false;
+		break;
+	}
 	fr_dcursor_append(out, vb);
 
 	return XLAT_ACTION_DONE;
@@ -999,10 +1024,10 @@ static int mod_load(void)
 {
 	xlat_t	*xlat;
 
-	if (unlikely(!(xlat = xlat_func_register(NULL, "radius_request_verify", xlat_func_radius_request_verify,
+	if (unlikely(!(xlat = xlat_func_register(NULL, "radius_secret_verify", xlat_func_radius_secret_verify,
 						 FR_TYPE_BOOL)))) return -1;
 
-	xlat_func_args_set(xlat, xlat_func_radius_request_verify_args);
+	xlat_func_args_set(xlat, xlat_func_radius_secret_verify_args);
 
 	return 0;
 }
diff --git a/src/protocols/radius/base.c b/src/protocols/radius/base.c
index d1534b31c6c..aeba8e4c9cd 100644
--- a/src/protocols/radius/base.c
+++ b/src/protocols/radius/base.c
@@ -687,14 +687,16 @@ finish:
  *  comparing the signature in the packet with the one we calculated.
  *  If they differ, there's a problem.
  *
- * @param packet the raw RADIUS packet (request or response)
- * @param original the raw original request (if this is a response)
- * @param secret the shared secret
- * @param secret_len the length of the secret
+ * @param[in] packet		the raw RADIUS packet (request or response)
+ * @param[in] original		the raw original request (if this is a response)
+ * @param[in] secret		the shared secret
+ * @param[in] secret_len	the length of the secret
  * @param[in] require_ma	whether we require Message-Authenticator.
  * @return
- *	- <0 on error
- *	- 0 on success
+ *	- -2 if the message authenticator or request authenticator was invalid.
+ *	- -1 if we were unable to verify the shared secret, or the packet
+ *	     was in some other way malformed.
+ *	- 0 on success.
  */
 int fr_radius_verify(uint8_t *packet, uint8_t const *original,
 		     uint8_t const *secret, size_t secret_len, bool require_ma)
@@ -781,7 +783,7 @@ int fr_radius_verify(uint8_t *packet, uint8_t const *original,
 		memcpy(packet + 4, request_authenticator, sizeof(request_authenticator));
 
 		fr_strerror_const("invalid Message-Authenticator (shared secret is incorrect)");
-		return -1;
+		return -2;
 	}
 
 	/*
@@ -802,7 +804,7 @@ int fr_radius_verify(uint8_t *packet, uint8_t const *original,
 		} else {
 			fr_strerror_const("invalid Request Authenticator (shared secret is incorrect)");
 		}
-		return -1;
+		return -2;
 	}
 
 	return 0;