]> git.ipfire.org Git - thirdparty/freeradius-server.git/commitdiff
move to using decode_ctx for public APIs, v2
authorAlan T. DeKok <aland@freeradius.org>
Thu, 25 Jan 2024 01:04:19 +0000 (20:04 -0500)
committerAlan T. DeKok <aland@freeradius.org>
Thu, 25 Jan 2024 01:04:19 +0000 (20:04 -0500)
src/listen/radius/proto_radius.c
src/modules/rlm_radius/rlm_radius_udp.c
src/protocols/radius/base.c
src/protocols/radius/decode.c
src/protocols/radius/radius.h

index d26e516fdd79c5a16c06214d3d001ceb1e710aa7..ee0f1bddac6833cfc2deb0402a33260f33272fb4 100644 (file)
@@ -201,7 +201,9 @@ static int mod_decode(UNUSED void const *instance, request_t *request, uint8_t *
 {
        fr_io_track_t const     *track = talloc_get_type_abort_const(request->async->packet_ctx, fr_io_track_t);
        fr_io_address_t const   *address = track->address;
-       fr_client_t const               *client;
+       fr_client_t const       *client;
+       fr_radius_ctx_t         common_ctx;
+       fr_radius_decode_ctx_t  decode_ctx;
 
        fr_assert(data[0] < FR_RADIUS_CODE_MAX);
 
@@ -214,19 +216,24 @@ static int mod_decode(UNUSED void const *instance, request_t *request, uint8_t *
 
        client = address->radclient;
 
-       /*
-        *      !client->active means a fake packet defining a dynamic client - so there will
-        *      be no secret defined yet - so can't verify.
-        */
-       if (client->active &&
-           fr_radius_verify(data, NULL, (uint8_t const *) client->secret, talloc_array_length(client->secret) - 1,
-                            client->message_authenticator) < 0) {
-               RPEDEBUG("Failed verifying packet signature.");
-               return -1;
-       }
+       common_ctx = (fr_radius_ctx_t) {
+               .secret = client->secret,
+               .secret_length = talloc_array_length(client->secret) - 1,
+       };
+
+       decode_ctx = (fr_radius_decode_ctx_t) {
+               .common = &common_ctx,
+               .tmp_ctx = talloc(request, uint8_t),
+               /* decode figures out request_authenticator */
+               .end = data + data_len,
+               .verify = client->active,
+               .require_message_authenticator = client->message_authenticator,
+       };
 
        /*
-        *      Hacks for now until we have a lower-level decode routine.
+        *      The verify() routine over-writes the request packet vector.
+        *
+        *      @todo - That needs to be changed.
         */
        request->packet->code = data[0];
        request->packet->id = data[1];
@@ -237,16 +244,16 @@ static int mod_decode(UNUSED void const *instance, request_t *request, uint8_t *
        request->packet->data_len = data_len;
 
        /*
-        *      Note that we don't set a limit on max_attributes here.
-        *      That MUST be set and checked in the underlying
-        *      transport, via a call to fr_radius_ok().
+        *      !client->active means a fake packet defining a dynamic client - so there will
+        *      be no secret defined yet - so can't verify.
         */
        if (fr_radius_decode(request->request_ctx, &request->request_pairs,
-                            request->packet->data, request->packet->data_len, NULL,
-                            client->secret, talloc_array_length(client->secret) - 1) < 0) {
-               RPEDEBUG("Failed decoding packet");
+                            data, data_len, &decode_ctx) < 0) {
+               talloc_free(decode_ctx.tmp_ctx);
+               RPEDEBUG("Failed reading packet");
                return -1;
        }
+       talloc_free(decode_ctx.tmp_ctx);
 
        /*
         *      Set the rest of the fields.
index a5e6a2f6745cd89cd6dd3b5a3611c076e6618ce1..fd628962e02da016ffdf66eb31b93e40e6c004a3 100644 (file)
@@ -1157,14 +1157,13 @@ static decode_fail_t decode(TALLOC_CTX *ctx, fr_pair_list_t *reply, uint8_t *res
                            uint8_t *data, size_t data_len)
 {
        rlm_radius_udp_t const *inst = h->thread->inst;
-       size_t                  packet_len;
        uint8_t                 code;
+       fr_radius_ctx_t         common_ctx;
+       fr_radius_decode_ctx_t  decode_ctx;
 
        *response_code = 0;     /* Initialise to keep the rest of the code happy */
 
-       packet_len = data_len;
-
-       RHEXDUMP3(data, packet_len, "Read packet");
+       RHEXDUMP3(data, data_len, "Read packet");
 
        code = data[0];
        if (!allowed_replies[code]) {
@@ -1187,26 +1186,34 @@ static decode_fail_t decode(TALLOC_CTX *ctx, fr_pair_list_t *reply, uint8_t *res
                return DECODE_FAIL_UNKNOWN_PACKET_CODE;
        }
 
-       if (fr_radius_verify(data, request_authenticator,
-                            (uint8_t const *) inst->secret, talloc_array_length(inst->secret) - 1, false) < 0) {
-               RPWDEBUG("Ignoring response with invalid signature");
-               return DECODE_FAIL_MA_INVALID;
-       }
+       common_ctx = (fr_radius_ctx_t) {
+               .secret = inst->secret,
+               .secret_length = talloc_array_length(inst->secret) - 1,
+       };
+
+       decode_ctx = (fr_radius_decode_ctx_t) {
+               .common = &common_ctx,
+               .request_code = u->code,
+               .request_authenticator = request_authenticator,
+               .tmp_ctx = talloc(ctx, uint8_t),
+               .end = data + data_len,
+               .verify = true,
+       };
 
        /*
-        *      Decode the attributes, in the context of the reply.
-        *      This only fails if the packet is strangely malformed,
-        *      or if we run out of memory.
+        *      !client->active means a fake packet defining a dynamic client - so there will
+        *      be no secret defined yet - so can't verify.
         */
-       if (fr_radius_decode(ctx, reply, data, packet_len, request_authenticator,
-                            inst->secret, talloc_array_length(inst->secret) - 1) < 0) {
-               REDEBUG("Failed decoding attributes for packet");
-               fr_pair_list_free(reply);
+       if (fr_radius_decode(request->request_ctx, &request->request_pairs,
+                            data, data_len, &decode_ctx) < 0) {
+               talloc_free(decode_ctx.tmp_ctx);
+               RPEDEBUG("Failed reading packet");
                return DECODE_FAIL_UNKNOWN;
        }
+       talloc_free(decode_ctx.tmp_ctx);
 
        RDEBUG("Received %s ID %d length %ld reply packet on connection %s",
-              fr_radius_packet_names[code], code, packet_len, h->name);
+              fr_radius_packet_names[code], code, data_len, h->name);
        log_request_pair_list(L_DBG_LVL_2, request, NULL, reply, NULL);
 
        *response_code = code;
index 40246c2928d5da7203d9324752b8c13bc4e83e86..2d5c5b310322092b9712a969113276146cc6b801 100644 (file)
@@ -973,25 +973,44 @@ ssize_t fr_radius_encode_dbuff(fr_dbuff_t *dbuff, uint8_t const *original,
        return fr_dbuff_set(dbuff, &work_dbuff);
 }
 
-/** Decode a raw RADIUS packet into VPs.
- *
- */
-ssize_t fr_radius_decode(TALLOC_CTX *ctx, fr_pair_list_t *out,
-                        uint8_t const *packet, size_t packet_len, uint8_t const *vector,
-                        char const *secret, size_t secret_len)
+ssize_t        fr_radius_decode(TALLOC_CTX *ctx, fr_pair_list_t *out,
+                        uint8_t *packet, size_t packet_len,
+                        fr_radius_decode_ctx_t *decode_ctx)
 {
        ssize_t                 slen;
        uint8_t const           *attr, *end;
-       fr_radius_ctx_t         common_ctx = {};
-       fr_radius_decode_ctx_t  packet_ctx = {};
+       static const uint8_t    zeros[RADIUS_AUTH_VECTOR_LENGTH] = {};
 
-       common_ctx.secret = secret;
-       common_ctx.secret_length = secret_len;
-       memcpy(common_ctx.vector, vector ? vector : packet + 4, sizeof(common_ctx.vector));
+       if (!decode_ctx->request_authenticator) {
+               switch (packet[0]) {
+               case FR_RADIUS_CODE_ACCESS_REQUEST:
+               case FR_RADIUS_CODE_STATUS_SERVER:
+                       decode_ctx->request_authenticator = packet + 4;
+                       break;
 
-       packet_ctx.common = &common_ctx;
-       packet_ctx.tmp_ctx = talloc_init_const("tmp");
-       packet_ctx.end = packet + packet_len;
+               case FR_RADIUS_CODE_ACCOUNTING_REQUEST:
+               case FR_RADIUS_CODE_COA_REQUEST:
+               case FR_RADIUS_CODE_DISCONNECT_REQUEST:
+                       decode_ctx->request_authenticator = zeros;
+                       break;
+
+               default:
+                       fr_strerror_const("No authentication vector passed for packet decode");
+                       return -1;
+               }
+       }
+
+       /*
+        *      We can skip verification for dynamic client checks, and where packets are unsigned as with
+        *      RADIUS/1.1.
+        */
+       if (decode_ctx->verify) {
+               if (fr_radius_verify(packet, decode_ctx->request_authenticator,
+                                    (uint8_t const *) decode_ctx->common->secret, decode_ctx->common->secret_length,
+                                    decode_ctx->require_message_authenticator) < 0) {
+                       return -1;
+               }
+       }
 
        attr = packet + 20;
        end = packet + packet_len;
@@ -1001,34 +1020,28 @@ ssize_t fr_radius_decode(TALLOC_CTX *ctx, fr_pair_list_t *out,
         *      he doesn't, all hell breaks loose.
         */
        while (attr < end) {
-               slen = fr_radius_decode_pair(ctx, out, attr, (end - attr), &packet_ctx);
-               if (slen < 0) {
-               fail:
-                       talloc_free(packet_ctx.tmp_ctx);
-                       talloc_free(packet_ctx.tags);
-                       return slen;
-               }
+               slen = fr_radius_decode_pair(ctx, out, attr, (end - attr), decode_ctx);
+               if (slen < 0) return slen;
 
                /*
                 *      If slen is larger than the room in the packet,
                 *      all kinds of bad things happen.
                 */
                 if (!fr_cond_assert(slen <= (end - attr))) {
-                        goto fail;
+                        return -slen;
                 }
 
                attr += slen;
-               talloc_free_children(packet_ctx.tmp_ctx);
+               talloc_free_children(decode_ctx->tmp_ctx);
        }
 
        /*
         *      We've parsed the whole packet, return that.
         */
-       talloc_free(packet_ctx.tmp_ctx);
-       talloc_free(packet_ctx.tags);
        return packet_len;
 }
 
+
 int fr_radius_init(void)
 {
        if (instance_count > 0) {
index fdf3c77f69d84aaab4e8d0557ed0864d8c26b4a0..60b61fd58c77e8177d13144790e052d62aa00314 100644 (file)
@@ -1624,8 +1624,10 @@ ssize_t fr_radius_decode_pair_value(TALLOC_CTX *ctx, fr_pair_list_t *out,
                 *  User-Password
                 */
                case FLAG_ENCRYPT_USER_PASSWORD:
+                       if (!packet_ctx->request_authenticator) goto raw;
+
                        fr_radius_decode_password((char *)buffer, attr_len,
-                                                 packet_ctx->common->secret, packet_ctx->common->vector);
+                                                 packet_ctx->common->secret, packet_ctx->request_authenticator);
                        buffer[253] = '\0';
 
                        /*
@@ -1659,8 +1661,10 @@ ssize_t fr_radius_decode_pair_value(TALLOC_CTX *ctx, fr_pair_list_t *out,
                 */
                case FLAG_TAGGED_TUNNEL_PASSWORD:
                case FLAG_ENCRYPT_TUNNEL_PASSWORD:
+                       if (!packet_ctx->request_authenticator) goto raw;       
+
                        if (fr_radius_decode_tunnel_password(buffer, &data_len,
-                                                            packet_ctx->common->secret, packet_ctx->common->vector,
+                                                            packet_ctx->common->secret, packet_ctx->request_authenticator,
                                                             packet_ctx->tunnel_password_zeros) < 0) {
                                goto raw;
                        }
@@ -1670,9 +1674,11 @@ ssize_t fr_radius_decode_pair_value(TALLOC_CTX *ctx, fr_pair_list_t *out,
                 *      Ascend-Send-Secret
                 *      Ascend-Receive-Secret
                 */
-               case FLAG_ENCRYPT_ASCEND_SECRET:
+               case FLAG_ENCRYPT_ASCEND_SECRET:        
+                       if (!packet_ctx->request_authenticator) goto raw;
+
                        fr_radius_ascend_secret(&FR_DBUFF_TMP(buffer, sizeof(buffer)), p, data_len,
-                                               packet_ctx->common->secret, packet_ctx->common->vector);
+                                               packet_ctx->common->secret, packet_ctx->request_authenticator);
                        buffer[RADIUS_AUTH_VECTOR_LENGTH] = '\0';
                        data_len = strlen((char *) buffer);
                        break;
@@ -2079,7 +2085,7 @@ static int decode_test_ctx(void **out, TALLOC_CTX *ctx)
        test_ctx->common->secret = talloc_strdup(test_ctx->common, "testing123");
        test_ctx->common->secret_length = talloc_array_length(test_ctx->common->secret);
 
-       memcpy(test_ctx->common->vector, vector, sizeof(test_ctx->common->vector));
+       test_ctx->request_authenticator = vector;
        test_ctx->tmp_ctx = talloc_zero(test_ctx, uint8_t);
        talloc_set_destructor(test_ctx, _test_ctx_free);
 
@@ -2121,8 +2127,7 @@ static ssize_t fr_radius_decode_proto(TALLOC_CTX *ctx, fr_pair_list_t *out,
 
        test_ctx->end = data + packet_len;
 
-       return fr_radius_decode(ctx, out, data, packet_len, test_ctx->common->vector,
-                               test_ctx->common->secret, talloc_array_length(test_ctx->common->secret) - 1);
+       return fr_radius_decode(ctx, out, UNCONST(uint8_t *, data), packet_len, test_ctx);
 }
 
 static ssize_t decode_pair(TALLOC_CTX *ctx, fr_pair_list_t *out, NDEBUG_UNUSED fr_dict_attr_t const *parent,
index 0c3acf4240d8c9de94e55165a457dd621db586d3..c8b5019a2d1ad630007d4cc4eac48dfabc28db42 100644 (file)
@@ -102,6 +102,54 @@ enum {
 #define flag_long_extended(_flags)   (!(_flags)->extra && (_flags)->subtype == FLAG_LONG_EXTENDED_ATTR)
 #define flag_tunnel_password(_flags) (!(_flags)->extra && (((_flags)->subtype == FLAG_ENCRYPT_TUNNEL_PASSWORD) || ((_flags)->subtype == FLAG_TAGGED_TUNNEL_PASSWORD)))
 
+typedef struct {
+       fr_pair_t       *parent;
+       fr_dcursor_t    cursor;
+} fr_radius_tag_ctx_t;
+
+typedef struct {
+       char const      *secret;
+       size_t          secret_length;
+
+       bool            add_proxy_state;                //!< do we add a Proxy-State?
+       uint64_t        my_proxy_state;                 //!< if so, this is its value
+
+       uint32_t        acct_delay_time;                //!< additional time to add to acct_delay_time
+
+       uint8_t         vector[RADIUS_AUTH_VECTOR_LENGTH]; //!< vector for authenticating the reply
+} fr_radius_ctx_t;
+
+typedef struct {
+       fr_radius_ctx_t         *common;
+
+       fr_fast_rand_t          rand_ctx;               //!< for tunnel passwords
+       int                     salt_offset;            //!< for tunnel passwords
+
+       uint8_t                 tag;                    //!< current tag for encoding
+
+       bool                    disallow_tunnel_passwords; //!< not all packets can have tunnel passwords
+       bool                    seen_message_authenticator;
+} fr_radius_encode_ctx_t;
+
+typedef struct {
+       fr_radius_ctx_t         *common;
+
+       uint8_t const           *request_authenticator;
+
+       TALLOC_CTX              *tmp_ctx;               //!< for temporary things cleaned up during decoding
+       uint8_t const           *end;                   //!< end of the packet
+
+       uint8_t                 request_code;           //!< original code for the request.
+
+       bool                    tunnel_password_zeros;  //!< check for trailing zeros on decode
+       bool                    verify;                 //!< can skip verify for dynamic clients
+       bool                    require_message_authenticator;
+
+       fr_radius_tag_ctx_t     **tags;                 //!< for decoding tagged attributes
+       fr_pair_list_t          *tag_root;              //!< Where to insert tag attributes.
+       TALLOC_CTX              *tag_root_ctx;          //!< Where to allocate new tag attributes.
+} fr_radius_decode_ctx_t;
+
 /*
  *     protocols/radius/base.c
  */
@@ -124,8 +172,8 @@ ssize_t             fr_radius_encode_dbuff(fr_dbuff_t *dbuff, uint8_t const *original,
                                 char const *secret, UNUSED size_t secret_len, int code, int id, fr_pair_list_t *vps);
 
 ssize_t                fr_radius_decode(TALLOC_CTX *ctx, fr_pair_list_t *out,
-                                uint8_t const *packet, size_t packet_len, uint8_t const *vector,
-                                char const *secret, UNUSED size_t secret_len) CC_HINT(nonnull(1,2,3,6));
+                                uint8_t *packet, size_t packet_len,
+                                fr_radius_decode_ctx_t *decode_ctx) CC_HINT(nonnull);
 
 int            fr_radius_init(void);
 
@@ -156,47 +204,6 @@ int                fr_radius_packet_send(fr_radius_packet_t *packet, fr_pair_list_t *list,
 #define fr_radius_packet_log_hex(_log, _packet) _fr_radius_packet_log_hex(_log, _packet, __FILE__, __LINE__)
 void           _fr_radius_packet_log_hex(fr_log_t const *log, fr_radius_packet_t const *packet, char const *file, int line) CC_HINT(nonnull);
 
-typedef struct {
-       fr_pair_t       *parent;
-       fr_dcursor_t    cursor;
-} fr_radius_tag_ctx_t;
-
-typedef struct {
-       char const      *secret;
-       size_t          secret_length;
-
-       bool            add_proxy_state;                //!< do we add a Proxy-State?
-       uint64_t        my_proxy_state;                 //!< if so, this is its value
-
-       uint32_t        acct_delay_time;                //!< additional time to add to acct_delay_time
-
-       uint8_t         vector[RADIUS_AUTH_VECTOR_LENGTH]; //!< vector for authenticating the reply
-} fr_radius_ctx_t;
-
-typedef struct {
-       fr_radius_ctx_t         *common;
-
-       fr_fast_rand_t          rand_ctx;               //!< for tunnel passwords
-       int                     salt_offset;            //!< for tunnel passwords
-
-       uint8_t                 tag;                    //!< current tag for encoding
-       bool                    disallow_tunnel_passwords; //!< not all packets can have tunnel passwords
-       bool                    seen_message_authenticator;
-} fr_radius_encode_ctx_t;
-
-typedef struct {
-       fr_radius_ctx_t         *common;
-
-       TALLOC_CTX              *tmp_ctx;               //!< for temporary things cleaned up during decoding
-       uint8_t const           *end;                   //!< end of the packet
-
-       bool                    tunnel_password_zeros;  //!< check for trailing zeros on decode
-
-       fr_radius_tag_ctx_t     **tags;                 //!< for decoding tagged attributes
-       fr_pair_list_t          *tag_root;              //!< Where to insert tag attributes.
-       TALLOC_CTX              *tag_root_ctx;          //!< Where to allocate new tag attributes.
-} fr_radius_decode_ctx_t;
-
 /*
  *     protocols/radius/abinary.c
  */