hoist proxy loop checks to one location

and apply the CHAP-Challenge etc. fixups to %proxy.sendto.ipaddr()

diff --git a/src/modules/rlm_radius/bio.c b/src/modules/rlm_radius/bio.c
index bfaedf3d9698f11a4fa0eec5b90b3be2e1bff31f..7103fd0964e5502beb66d8958babe676ca435030 100644 (file)
--- a/src/modules/rlm_radius/bio.c
+++ b/src/modules/rlm_radius/bio.c
@@ -1517,7 +1517,6 @@ static void request_mux(UNUSED fr_event_list_t *el,
 {
        bio_handle_t            *h = talloc_get_type_abort(conn->h, bio_handle_t);
        trunk_request_t         *treq;
-       bio_request_t           *u;
        request_t               *request;
 
        if (unlikely(trunk_connection_pop_request(&treq, tconn) < 0)) return;
@@ -1529,42 +1528,6 @@ static void request_mux(UNUSED fr_event_list_t *el,
 
        request = treq->request;
 
-       u = treq->preq;
-       fr_assert(u != NULL);
-
-       /*
-        *      Warn people about misconfigurations and loops.
-        *
-        *      There should _never_ be two instances of the same Proxy-State in the packet.
-        */
-       if (RDEBUG_ENABLED && u->proxied) {
-               unsigned int count = 0;
-
-               fr_pair_list_foreach(&request->request_pairs, vp) {
-                       if (vp->vp_length != sizeof(h->ctx.radius_ctx.proxy_state)) continue;
-
-                       if (memcmp(vp->vp_octets, &h->ctx.radius_ctx.proxy_state,
-                                  sizeof(h->ctx.radius_ctx.proxy_state)) == 0) {
-
-                               /*
-                                *      Cancel proxying when there are two instances of the same Proxy-State
-                                *      in the packet.  This limitation could be configurable, but it likely
-                                *      doesn't make sense to make it configurable.
-                                */
-                               if (count == 1) {
-                                       RWARN("Canceling proxy due to loop of multiple %pV", vp);
-                                       trunk_request_signal_cancel(treq);
-                                       u->treq = NULL;
-                                       return;
-                               }
-
-                               RWARN("Proxied packet contains our own %pV", vp);
-                               RWARN("Check if there is a proxy loop.  Perhaps the server has been configured to proxy to itself.");
-                               count++;
-                       }
-               }
-       }
-
        mod_write(request, treq, h);
 }
 
@@ -2320,10 +2283,12 @@ static int mod_enqueue(bio_request_t **p_u, fr_retry_config_t const **p_retry_co
        fr_assert(request->packet->code > 0);
        fr_assert(request->packet->code < FR_RADIUS_CODE_MAX);
 
-       if (request->packet->code == FR_RADIUS_CODE_STATUS_SERVER) {
-               RWDEBUG("Status-Server is reserved for internal use, and cannot be sent manually.");
-               return 0;
-       }
+       /*
+        *      Do any necessary RADIUS level fixups
+        *      - check Proxy-State
+        *      - do CHAP-Challenge fixups
+        */
+       if (radius_fixups(inst, request) < 0) return 0;
 
        treq = trunk_request_alloc(trunk, request);
        if (!treq) {
@@ -2694,7 +2659,7 @@ static xlat_action_t xlat_radius_client(UNUSED TALLOC_CTX *ctx, UNUSED fr_dcurso
         */
        if (ipaddr->vb_ip.af != thread->bio.info->socket.af) {
                RDEBUG("Invalid destination IP address family in %pV", ipaddr);
-               return -1;
+               return XLAT_ACTION_DONE;
        }
 
        home = fr_rb_find(&thread->bio.expires.tree, &(home_server_t) {
@@ -2778,7 +2743,9 @@ static xlat_action_t xlat_radius_client(UNUSED TALLOC_CTX *ctx, UNUSED fr_dcurso
         *      Enqueue the packet on the per-home-server trunk.
         */
        rcode = mod_enqueue(&u, &retry_config, inst, home->ctx.trunk, request);
-       if (rcode <= 0) {
+       if (rcode == 0) return XLAT_ACTION_DONE;
+
+       if (rcode < 0) {
                REDEBUG("Failed enqueuing packet");
                return XLAT_ACTION_FAIL;
        }

diff --git a/src/modules/rlm_radius/rlm_radius.c b/src/modules/rlm_radius/rlm_radius.c
index 01a17e297a0f4e5143063b4fa7d54623093ce402..dc2d89dbd0b4937fd8c4e415226ad33bf7546992 100644 (file)
--- a/src/modules/rlm_radius/rlm_radius.c
+++ b/src/modules/rlm_radius/rlm_radius.c
@@ -36,6 +36,7 @@ static int mode_parse(TALLOC_CTX *ctx, void *out, UNUSED void *parent, CONF_ITEM
 static int type_parse(TALLOC_CTX *ctx, void *out, UNUSED void *parent, CONF_ITEM *ci, conf_parser_t const *rule);
 static int status_check_type_parse(TALLOC_CTX *ctx, void *out, UNUSED void *parent, CONF_ITEM *ci, conf_parser_t const *rule);
 static int status_check_update_parse(TALLOC_CTX *ctx, void *out, UNUSED void *parent, CONF_ITEM *ci, conf_parser_t const *rule);
+static int radius_fixups(rlm_radius_t const *inst, request_t *request);
 
 static conf_parser_t const status_check_config[] = {
        { FR_CONF_OFFSET_TYPE_FLAGS("type", FR_TYPE_VOID, 0, rlm_radius_t, status_check),
@@ -461,35 +462,64 @@ static int status_check_update_parse(TALLOC_CTX *ctx, void *out, UNUSED void *pa
 /** Do any RADIUS-layer fixups for proxying.
  *
  */
-static void radius_fixups(rlm_radius_t const *inst, request_t *request)
+static int radius_fixups(rlm_radius_t const *inst, request_t *request)
 {
        fr_pair_t *vp;
 
+       if (request->packet->code == FR_RADIUS_CODE_STATUS_SERVER) {
+               RWDEBUG("Status-Server is reserved for internal use, and cannot be sent manually.");
+               return 0;
+       }
+
+       if (!inst->allowed[request->packet->code]) {
+               REDEBUG("Packet code %s is disallowed by the configuration",
+                      fr_radius_packet_name[request->packet->code]);
+               return -1;
+       }
+
        /*
         *      Check for proxy loops.
+        *
+        *      There should _never_ be two instances of the same Proxy-State in the packet.
         */
        if ((inst->mode == RLM_RADIUS_MODE_PROXY) && RDEBUG_ENABLED) {
+               unsigned int count = 0;
                fr_dcursor_t cursor;
 
                for (vp = fr_pair_dcursor_by_da_init(&cursor, &request->request_pairs, attr_proxy_state);
                     vp;
                     vp = fr_dcursor_next(&cursor)) {
-                       if (vp->vp_length != 4) continue;
-
-                       if (memcmp(&inst->common_ctx.proxy_state, vp->vp_octets, 4) == 0) {
-                               RWARN("Possible proxy loop - please check server configuration.");
-                               break;
+                       if (vp->vp_length != sizeof(inst->common_ctx.proxy_state)) continue;
+
+                       if (memcmp(vp->vp_octets, &inst->common_ctx.proxy_state,
+                                  sizeof(inst->common_ctx.proxy_state)) == 0) {
+
+                               /*
+                                *      Cancel proxying when there are two instances of the same Proxy-State
+                                *      in the packet.  This limitation could be configurable, but it likely
+                                *      doesn't make sense to make it configurable.
+                                */
+                               if (count == 1) {
+                                       RWARN("Canceling proxy due to loop of multiple %pV", vp);
+                                       return -1;
+                               }
+
+                               RWARN("Proxied packet contains our own %pV", vp);
+                               RWARN("Check if there is a proxy loop.  Perhaps the server has been configured to proxy to itself.");
+                               count++;
                        }
                }
        }
 
-       if (request->packet->code != FR_RADIUS_CODE_ACCESS_REQUEST) return;
+       if (request->packet->code != FR_RADIUS_CODE_ACCESS_REQUEST) return 0;
 
        if (fr_pair_find_by_da(&request->request_pairs, NULL, attr_chap_password) &&
            !fr_pair_find_by_da(&request->request_pairs, NULL, attr_chap_challenge)) {
                MEM(pair_append_request(&vp, attr_chap_challenge) >= 0);
                fr_pair_value_memdup(vp, request->packet->vector, sizeof(request->packet->vector), true);
        }
+
+       return 0;
 }
 
 
@@ -510,15 +540,6 @@ static unlang_action_t CC_HINT(nonnull) mod_process(rlm_rcode_t *p_result, modul
                RETURN_MODULE_FAIL;
        }
 
-       /*
-        *      Reserve Status-Server for ourselves, for link-specific
-        *      signaling.
-        */
-       if (request->packet->code == FR_RADIUS_CODE_STATUS_SERVER) {
-               REDEBUG("Cannot proxy Status-Server packets");
-               RETURN_MODULE_FAIL;
-       }
-
        if ((request->packet->code >= FR_RADIUS_CODE_MAX) ||
            !fr_time_delta_ispos(inst->retry[request->packet->code].irt)) { /* can't be zero */
                REDEBUG("Invalid packet code %u", request->packet->code);
@@ -535,25 +556,12 @@ static unlang_action_t CC_HINT(nonnull) mod_process(rlm_rcode_t *p_result, modul
                RETURN_MODULE_FAIL;
        }
 
-       if (!inst->allowed[request->packet->code]) {
-               REDEBUG("Packet code %s is disallowed by the configuration",
-                      fr_radius_packet_name[request->packet->code]);
-               RETURN_MODULE_FAIL;
-       }
-
        client = client_from_request(request);
        if (client && client->dynamic && !client->active) {
                REDEBUG("Cannot proxy packets which define dynamic clients");
                RETURN_MODULE_FAIL;
        }
 
-       /*
-        *      Do any necessary RADIUS level fixups
-        *      - check Proxy-State
-        *      - do CHAP-Challenge fixups
-        */
-       radius_fixups(inst, request);
-
        /*
         *      Push the request and it's data to the IO submodule.
         *