hoist Proxy-State checks to main encoder

in preparation for moving rlm_radius to the new BIO code

diff --git a/src/modules/rlm_radius/rlm_radius.c b/src/modules/rlm_radius/rlm_radius.c
index 8c67924e0135b05aef61758c0f78f49f72468782..81ed6af31508a99cfc37180f7fee41bf863d9260 100644 (file)
--- a/src/modules/rlm_radius/rlm_radius.c
+++ b/src/modules/rlm_radius/rlm_radius.c
@@ -364,7 +364,7 @@ static void radius_fixups(rlm_radius_t const *inst, request_t *request)
        /*
         *      Check for proxy loops.
         */
-       if (RDEBUG_ENABLED) {
+       if (!inst->originate && RDEBUG_ENABLED) {
                fr_dcursor_t cursor;
 
                for (vp = fr_pair_dcursor_by_da_init(&cursor, &request->request_pairs, attr_proxy_state);

diff --git a/src/modules/rlm_radius/rlm_radius_udp.c b/src/modules/rlm_radius/rlm_radius_udp.c
index 1b831ea85af087759df3ca5719967da0d6e839ae..73476fe1f891b016ad048ce0435b619115e89025 100644 (file)
--- a/src/modules/rlm_radius/rlm_radius_udp.c
+++ b/src/modules/rlm_radius/rlm_radius_udp.c
@@ -1206,7 +1206,6 @@ static decode_fail_t decode(TALLOC_CTX *ctx, fr_pair_list_t *reply, uint8_t *res
 static int encode(rlm_radius_udp_t const *inst, request_t *request, udp_request_t *u, uint8_t id)
 {
        ssize_t                 packet_len;
-       int                     proxy_state = 6;
        fr_radius_encode_ctx_t  encode_ctx;
 
        fr_assert(inst->parent->allowed[u->code]);
@@ -1233,25 +1232,17 @@ static int encode(rlm_radius_udp_t const *inst, request_t *request, udp_request_
        if (u->status_check) {
                fr_pair_t *vp;
 
-               proxy_state = 0;
                vp = fr_pair_find_by_da(&request->request_pairs, NULL, attr_event_timestamp);
                if (vp) vp->vp_date = fr_time_to_unix_time(u->retry.updated);
 
                if (u->code == FR_RADIUS_CODE_STATUS_SERVER) u->can_retransmit = false;
-
-       } else if (inst->parent->originate) {
-               /*
-                *      We're originating packets instead of proxying
-                *      them.  We don't add a Proxy-State attribute.
-                */
-               proxy_state = 0;
        }
 
        /*
         *      We should have at minimum 64-byte packets, so don't
         *      bother doing run-time checks here.
         */
-       fr_assert(u->packet_len >= (size_t) (RADIUS_HEADER_LENGTH + proxy_state));
+       fr_assert(u->packet_len >= (size_t) RADIUS_HEADER_LENGTH);
 
        encode_ctx = (fr_radius_encode_ctx_t) {
                .common = &inst->common_ctx,
@@ -1266,7 +1257,7 @@ static int encode(rlm_radius_udp_t const *inst, request_t *request, udp_request_
        /*
         *      Encode it, leaving room for Proxy-State if necessary.
         */
-       packet_len = fr_radius_encode(&FR_DBUFF_TMP(u->packet, u->packet_len - proxy_state),
+       packet_len = fr_radius_encode(&FR_DBUFF_TMP(u->packet, u->packet_len),
                                      &request->request_pairs, &encode_ctx);
        if (fr_pair_encode_is_error(packet_len)) {
                RPERROR("Failed encoding packet");
@@ -1280,7 +1271,7 @@ static int encode(rlm_radius_udp_t const *inst, request_t *request, udp_request_
                size_t have;
                size_t need;
 
-               have = u->packet_len - proxy_state;
+               have = u->packet_len;
                need = have - packet_len;
 
                if (need > RADIUS_MAX_PACKET_SIZE) {
@@ -1296,7 +1287,7 @@ static int encode(rlm_radius_udp_t const *inst, request_t *request, udp_request_
        /*
         *      The encoded packet should NOT over-run the input buffer.
         */
-       fr_assert((size_t) (packet_len + proxy_state) <= u->packet_len);
+       fr_assert((size_t) packet_len <= u->packet_len);
 
        /*
         *      Add Proxy-State to the tail end of the packet.
@@ -1305,44 +1296,11 @@ static int encode(rlm_radius_udp_t const *inst, request_t *request, udp_request_
         *      request->request_pairs, because multiple modules
         *      may be sending the packets at the same time.
         */
-       if (proxy_state) {
-               uint8_t         *attr = u->packet + packet_len;
+       if (inst->common_ctx.add_proxy_state) {
                fr_pair_t       *vp;
-               fr_dcursor_t    cursor;
-               int             count = 0;
-
-               /*
-                *      Count how many Proxy-State attributes have
-                *      *our* magic number.  Note that we also add a
-                *      counter to each Proxy-State, so we're double
-                *      sure that it's a loop.
-                */
-               if (DEBUG_ENABLED) {
-                       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 == 5) && (memcmp(vp->vp_octets, &inst->parent->proxy_state, 4) == 0)) {
-                                       count++;
-                               }
-                       }
-
-                       /*
-                        *      Some configurations may proxy to
-                        *      ourselves for tests / simplicity.  But
-                        *      warn if there are a large number of
-                        *      identical Proxy-State attributes.
-                        */
-                       if (count >= 4) RWARN("Potential proxy loop detected!  Please recheck your configuration.");
-               }
-
-               attr[0] = (uint8_t)attr_proxy_state->attr;
-               attr[1] = 7;
-               memcpy(attr + 2, &inst->parent->proxy_state, 4);
-               attr[6] = count & 0xff;
-               packet_len += 7;
 
                MEM(vp = fr_pair_afrom_da(u->packet, attr_proxy_state));
-               fr_pair_value_memdup(vp, attr + 2, 5, true);
+               fr_pair_value_memdup(vp, &inst->common_ctx.proxy_state, sizeof(inst->common_ctx.proxy_state), false);
                fr_pair_append(&u->extra, vp);
        }
 
@@ -2775,6 +2733,8 @@ static int mod_instantiate(module_inst_ctx_t const *mctx)
        inst->common_ctx = (fr_radius_ctx_t) {
                .secret = inst->secret,
                .secret_length = talloc_array_length(inst->secret) - 1,
+               .add_proxy_state = !inst->parent->originate,
+               .proxy_state = inst->parent->proxy_state,
        };
 
        /*

diff --git a/src/protocols/radius/base.c b/src/protocols/radius/base.c
index 93d632c443f238d898dfd9fde68f4b94c0f67acd..51bac0f3982f45e3721214f7ca468a25f5201623 100644 (file)
--- a/src/protocols/radius/base.c
+++ b/src/protocols/radius/base.c
@@ -1026,6 +1026,14 @@ ssize_t fr_radius_encode(fr_dbuff_t *dbuff, fr_pair_list_t *vps, fr_radius_encod
                if (slen < 0) return slen;
        } /* done looping over all attributes */
 
+       /*
+        *      Add Proxy-State to the end of the packet if the caller requested it.
+        */
+       if (packet_ctx->common->add_proxy_state) {
+               FR_DBUFF_IN_BYTES_RETURN(&work_dbuff, FR_PROXY_STATE, 6);
+               FR_DBUFF_IN_RETURN(&work_dbuff, packet_ctx->common->proxy_state);
+       }
+
        /*
         *      Fill in the length field we zeroed out earlier.
         *

diff --git a/src/protocols/radius/radius.h b/src/protocols/radius/radius.h
index d39de66f3096923312ec74d88898f8ef79d98bea..f3ae728aa297aef5bc0800eb2f257340210be16f 100644 (file)
--- a/src/protocols/radius/radius.h
+++ b/src/protocols/radius/radius.h
@@ -143,7 +143,7 @@ typedef struct {
        bool                    secure_transport;       //!< for TLS
 
        bool                    add_proxy_state;        //!< do we add a Proxy-State?
-       uint64_t                my_proxy_state;         //!< if so, this is its value
+       uint32_t                proxy_state;            //!< if so, this is its value
 } fr_radius_ctx_t;
 
 typedef struct {