]> git.ipfire.org Git - thirdparty/freeradius-server.git/commitdiff
pass original vector to verify / sign
authorAlan T. DeKok <aland@freeradius.org>
Wed, 24 Jan 2024 13:31:44 +0000 (08:31 -0500)
committerAlan T. DeKok <aland@freeradius.org>
Wed, 24 Jan 2024 15:11:31 +0000 (10:11 -0500)
instead of the complete packet.  This makes later changes easier

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/packet.c
src/protocols/radius/radius.h

index cfab04bda3698fa650df2c0f4fd6f0781bc0bb88..d26e516fdd79c5a16c06214d3d001ceb1e710aa7 100644 (file)
@@ -396,7 +396,7 @@ static ssize_t mod_encode(UNUSED void const *instance, request_t *request, uint8
                return -1;
        }
 
-       if (fr_radius_sign(buffer, request->packet->data,
+       if (fr_radius_sign(buffer, request->packet->data + 4,
                           (uint8_t const *) client->secret, talloc_array_length(client->secret) - 1) < 0) {
                RPEDEBUG("Failed signing RADIUS reply");
                return -1;
index f437580f1a401581f5a8bfb2f6b9e6285b5028b5..a5e6a2f6745cd89cd6dd3b5a3611c076e6618ce1 100644 (file)
@@ -1159,7 +1159,6 @@ static decode_fail_t decode(TALLOC_CTX *ctx, fr_pair_list_t *reply, uint8_t *res
        rlm_radius_udp_t const *inst = h->thread->inst;
        size_t                  packet_len;
        uint8_t                 code;
-       uint8_t                 original[RADIUS_HEADER_LENGTH];
 
        *response_code = 0;     /* Initialise to keep the rest of the code happy */
 
@@ -1188,13 +1187,7 @@ static decode_fail_t decode(TALLOC_CTX *ctx, fr_pair_list_t *reply, uint8_t *res
                return DECODE_FAIL_UNKNOWN_PACKET_CODE;
        }
 
-       original[0] = u->code;
-       original[1] = 0;                        /* not looked at by fr_radius_verify() */
-       original[2] = 0;
-       original[3] = RADIUS_HEADER_LENGTH;     /* for debugging */
-       memcpy(original + RADIUS_AUTH_VECTOR_OFFSET, request_authenticator, RADIUS_AUTH_VECTOR_LENGTH);
-
-       if (fr_radius_verify(data, original,
+       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;
@@ -1205,7 +1198,7 @@ static decode_fail_t decode(TALLOC_CTX *ctx, fr_pair_list_t *reply, uint8_t *res
         *      This only fails if the packet is strangely malformed,
         *      or if we run out of memory.
         */
-       if (fr_radius_decode(ctx, reply, data, packet_len, original,
+       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);
index b4b6456ea3474125467a6cbef27cab828a0b971e..40246c2928d5da7203d9324752b8c13bc4e83e86 100644 (file)
@@ -156,7 +156,7 @@ char const *fr_radius_packet_names[FR_RADIUS_CODE_MAX] = {
  * encrypting passwords to RADIUS.
  */
 ssize_t fr_radius_ascend_secret(fr_dbuff_t *dbuff, uint8_t const *in, size_t inlen,
-                               char const *secret, uint8_t const vector[static RADIUS_AUTH_VECTOR_LENGTH])
+                               char const *secret, uint8_t const *vector)
 {
        fr_md5_ctx_t            *md5_ctx;
        size_t                  i;
@@ -259,14 +259,14 @@ invalid:
  * in the message-authenticator value if the attribute is present in the encoded packet.
  *
  * @param[in,out] packet       (request or response).
- * @param[in] original         request (only if this is a response).
+ * @param[in] vector           original packet vector to use
  * @param[in] secret           to sign the packet with.
  * @param[in] secret_len       The length of the secret.
  * @return
  *     - <0 on error
  *     - 0 on success
  */
-int fr_radius_sign(uint8_t *packet, uint8_t const *original,
+int fr_radius_sign(uint8_t *packet, uint8_t const *vector,
                   uint8_t const *secret, size_t secret_len)
 {
        uint8_t         *msg, *end;
@@ -329,8 +329,8 @@ int fr_radius_sign(uint8_t *packet, uint8_t const *original,
                case FR_RADIUS_CODE_DISCONNECT_NAK:
                case FR_RADIUS_CODE_COA_ACK:
                case FR_RADIUS_CODE_COA_NAK:
-                       if (!original) goto need_original;
-                       memcpy(packet + 4, original + 4, RADIUS_AUTH_VECTOR_LENGTH);
+                       if (!vector) goto need_original;
+                       memcpy(packet + 4, vector, RADIUS_AUTH_VECTOR_LENGTH);
                        break;
 
                case FR_RADIUS_CODE_ACCESS_REQUEST:
@@ -371,12 +371,12 @@ int fr_radius_sign(uint8_t *packet, uint8_t const *original,
        case FR_RADIUS_CODE_COA_ACK:
        case FR_RADIUS_CODE_COA_NAK:
        case FR_RADIUS_CODE_PROTOCOL_ERROR:
-               if (!original) {
+               if (!vector) {
                need_original:
                        fr_strerror_const("Cannot sign response packet without a request packet");
                        return -1;
                }
-               memcpy(packet + 4, original + 4, RADIUS_AUTH_VECTOR_LENGTH);
+               memcpy(packet + 4, vector, RADIUS_AUTH_VECTOR_LENGTH);
                break;
 
                /*
@@ -683,7 +683,7 @@ finish:
  *  If they differ, there's a problem.
  *
  * @param[in] packet           the raw RADIUS packet (request or response)
- * @param[in] original         the raw original request (if this is a response)
+ * @param[in] vector           the original packet vector
  * @param[in] secret           the shared secret
  * @param[in] secret_len       the length of the secret
  * @param[in] require_ma       whether we require Message-Authenticator.
@@ -693,7 +693,7 @@ finish:
  *          was in some other way malformed.
  *     - 0 on success.
  */
-int fr_radius_verify(uint8_t *packet, uint8_t const *original,
+int fr_radius_verify(uint8_t *packet, uint8_t const *vector,
                     uint8_t const *secret, size_t secret_len, bool require_ma)
 {
        bool found_ma;
@@ -766,7 +766,7 @@ int fr_radius_verify(uint8_t *packet, uint8_t const *original,
         *      slightly more CPU work than having verify-specific
         *      functions, but it ends up being cleaner in the code.
         */
-       rcode = fr_radius_sign(packet, original, secret, secret_len);
+       rcode = fr_radius_sign(packet, vector, secret, secret_len);
        if (rcode < 0) {
                fr_strerror_const_push("Failed calculating correct authenticator");
                return -1;
@@ -801,7 +801,7 @@ int fr_radius_verify(uint8_t *packet, uint8_t const *original,
         */
        if (fr_digest_cmp(request_authenticator, packet + 4, sizeof(request_authenticator)) != 0) {
                memcpy(packet + 4, request_authenticator, sizeof(request_authenticator));
-               if (original) {
+               if (vector) {
                        fr_strerror_const("invalid Response Authenticator (shared secret is incorrect)");
                } else {
                        fr_strerror_const("invalid Request Authenticator (shared secret is incorrect)");
@@ -977,7 +977,7 @@ ssize_t fr_radius_encode_dbuff(fr_dbuff_t *dbuff, uint8_t const *original,
  *
  */
 ssize_t fr_radius_decode(TALLOC_CTX *ctx, fr_pair_list_t *out,
-                        uint8_t const *packet, size_t packet_len, uint8_t const *original,
+                        uint8_t const *packet, size_t packet_len, uint8_t const *vector,
                         char const *secret, size_t secret_len)
 {
        ssize_t                 slen;
@@ -987,7 +987,7 @@ ssize_t fr_radius_decode(TALLOC_CTX *ctx, fr_pair_list_t *out,
 
        common_ctx.secret = secret;
        common_ctx.secret_length = secret_len;
-       memcpy(common_ctx.vector, original ? original + 4 : packet + 4, sizeof(common_ctx.vector));
+       memcpy(common_ctx.vector, vector ? vector : packet + 4, sizeof(common_ctx.vector));
 
        packet_ctx.common = &common_ctx;
        packet_ctx.tmp_ctx = talloc_init_const("tmp");
index 569a85e0a756d0d66111a0bd7b5ac33221f2e474..fdf3c77f69d84aaab4e8d0557ed0864d8c26b4a0 100644 (file)
@@ -2095,7 +2095,6 @@ static ssize_t fr_radius_decode_proto(TALLOC_CTX *ctx, fr_pair_list_t *out,
        decode_fail_t   reason;
        fr_pair_t       *vp;
        size_t          packet_len = data_len;
-       uint8_t         original[20];
 
        if (!fr_radius_ok(data, &packet_len, 200, false, &reason)) {
                return -1;
@@ -2120,11 +2119,9 @@ static ssize_t fr_radius_decode_proto(TALLOC_CTX *ctx, fr_pair_list_t *out,
        (void) fr_pair_value_memdup(vp, data + 4, 16, true);
        fr_pair_append(out, vp);
 
-       memset(original, 0, 4);
-       memcpy(original + 4, test_ctx->common->vector, sizeof(test_ctx->common->vector));
        test_ctx->end = data + packet_len;
 
-       return fr_radius_decode(ctx, out, data, packet_len, original,
+       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);
 }
 
index 163c9fb4c79415f1825ad12a5dc3ab390bc3a1c8..d90519c8478200efc3ecf50a1f1444c54ea7e073 100644 (file)
@@ -282,19 +282,12 @@ bool fr_radius_packet_ok(fr_radius_packet_t *packet, uint32_t max_attributes, bo
  *
  */
 int fr_radius_packet_verify(fr_radius_packet_t *packet, fr_radius_packet_t *original, char const *secret)
-{
-       uint8_t const   *original_data;
+{      
        char            buffer[INET6_ADDRSTRLEN];
 
        if (!packet->data) return -1;
 
-       if (original) {
-               original_data = original->data;
-       } else {
-               original_data = NULL;
-       }
-
-       if (fr_radius_verify(packet->data, original_data,
+       if (fr_radius_verify(packet->data, original ? original->data + 4 : NULL,
                             (uint8_t const *) secret, talloc_array_length(secret) - 1, false) < 0) {
                fr_strerror_printf_push("Received invalid packet from %s",
                                        inet_ntop(packet->socket.inet.src_ipaddr.af, &packet->socket.inet.src_ipaddr.addr,
@@ -313,13 +306,6 @@ int fr_radius_packet_sign(fr_radius_packet_t *packet, fr_radius_packet_t const *
                          char const *secret)
 {
        int ret;
-       uint8_t const *original_data;
-
-       if (original) {
-               original_data = original->data;
-       } else {
-               original_data = NULL;
-       }
 
        /*
         *      Copy the random vector to the packet.  Other packet
@@ -331,7 +317,7 @@ int fr_radius_packet_sign(fr_radius_packet_t *packet, fr_radius_packet_t const *
                memcpy(packet->data + 4, packet->vector, sizeof(packet->vector));
        }
 
-       ret = fr_radius_sign(packet->data, original_data,
+       ret = fr_radius_sign(packet->data, original ? original->data + 4 : NULL,
                               (uint8_t const *) secret, talloc_array_length(secret) - 1);
        if (ret < 0) return ret;
 
index fd8f2f696b7c3c67fdb41e7fd44771fa833369f6..c8dcc1487a0d390af38b47fd8a4fa44dda8f2bab 100644 (file)
@@ -105,15 +105,15 @@ enum {
 /*
  *     protocols/radius/base.c
  */
-int            fr_radius_sign(uint8_t *packet, uint8_t const *original,
+int            fr_radius_sign(uint8_t *packet, uint8_t const *vector,
                               uint8_t const *secret, size_t secret_len) CC_HINT(nonnull (1,3));
-int            fr_radius_verify(uint8_t *packet, uint8_t const *original,
+int            fr_radius_verify(uint8_t *packet, uint8_t const *vector,
                                 uint8_t const *secret, size_t secret_len, bool require_ma) CC_HINT(nonnull (1,3));
 bool           fr_radius_ok(uint8_t const *packet, size_t *packet_len_p,
                             uint32_t max_attributes, bool require_ma, decode_fail_t *reason) CC_HINT(nonnull (1,2));
 
 ssize_t                fr_radius_ascend_secret(fr_dbuff_t *dbuff, uint8_t const *in, size_t inlen,
-                                       char const *secret, uint8_t const vector[static RADIUS_AUTH_VECTOR_LENGTH]);
+                                       char const *secret, uint8_t const *vector);
 
 ssize_t                fr_radius_recv_header(int sockfd, fr_ipaddr_t *src_ipaddr, uint16_t *src_port, unsigned int *code);
 
@@ -124,7 +124,7 @@ 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 *original,
+                                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));
 
 int            fr_radius_init(void);