From: Alan T. DeKok Date: Wed, 24 Jan 2024 13:31:44 +0000 (-0500) Subject: pass original vector to verify / sign X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=081382c99897e0696acad5174caec0142d54e337;p=thirdparty%2Ffreeradius-server.git pass original vector to verify / sign instead of the complete packet. This makes later changes easier --- diff --git a/src/listen/radius/proto_radius.c b/src/listen/radius/proto_radius.c index cfab04bda36..d26e516fdd7 100644 --- a/src/listen/radius/proto_radius.c +++ b/src/listen/radius/proto_radius.c @@ -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; diff --git a/src/modules/rlm_radius/rlm_radius_udp.c b/src/modules/rlm_radius/rlm_radius_udp.c index f437580f1a4..a5e6a2f6745 100644 --- a/src/modules/rlm_radius/rlm_radius_udp.c +++ b/src/modules/rlm_radius/rlm_radius_udp.c @@ -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); diff --git a/src/protocols/radius/base.c b/src/protocols/radius/base.c index b4b6456ea34..40246c2928d 100644 --- a/src/protocols/radius/base.c +++ b/src/protocols/radius/base.c @@ -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"); diff --git a/src/protocols/radius/decode.c b/src/protocols/radius/decode.c index 569a85e0a75..fdf3c77f69d 100644 --- a/src/protocols/radius/decode.c +++ b/src/protocols/radius/decode.c @@ -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); } diff --git a/src/protocols/radius/packet.c b/src/protocols/radius/packet.c index 163c9fb4c79..d90519c8478 100644 --- a/src/protocols/radius/packet.c +++ b/src/protocols/radius/packet.c @@ -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; diff --git a/src/protocols/radius/radius.h b/src/protocols/radius/radius.h index fd8f2f696b7..c8dcc1487a0 100644 --- a/src/protocols/radius/radius.h +++ b/src/protocols/radius/radius.h @@ -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);