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 c2756dc43d233c58445a102b52cbf6b1bf861527..2d8b41735104cc7d5bc89cc467b3166c1f934d3a 100644 (file)
--- 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 d1534b31c6cb839a912219c59b3237cb9a28242e..aeba8e4c9cd6bb5a425ff957fdca0fc1e620ccdc 100644 (file)
--- 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;