Browse code

PUSH_UPDATE server: check IV_PROTO before sending the message to the client

Before sending the PUSH_UPDATE message to the client, we must verify that
the client has actually sent IV_PROTO_PUSH_UPDATE to the server, declaring that
it supports push-updates.

Also fixed a gc_arena memory leak in one of the error paths and asserted
mi->context.c2.tls_multi .

Change-Id: I7c28da72be11c7efbed3068fbfc65f2959227bec
Signed-off-by: Marco Baffo <marco@mandelbit.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1255
Message-Id: <20251009182855.18712-1-gert@greenie.muc.de>
URL: https://sourceforge.net/p/openvpn/mailman/message/59244566/
Signed-off-by: Gert Doering <gert@greenie.muc.de>

Marco Baffo authored on 2025/10/10 03:28:49
Showing 2 changed files
... ...
@@ -7,6 +7,7 @@
7 7
 
8 8
 #ifdef ENABLE_MANAGEMENT
9 9
 #include "multi.h"
10
+#include "ssl_util.h"
10 11
 #endif
11 12
 
12 13
 #if defined(__GNUC__) || defined(__clang__)
... ...
@@ -177,20 +178,32 @@ send_single_push_update(struct context *c, struct buffer *msgs, unsigned int *op
177 177
         buf_string_compare_advance(&tmp_msg, push_update_cmd);
178 178
         if (process_incoming_push_update(c, pull_permission_mask(c), option_types_found, &tmp_msg, true) == PUSH_MSG_ERROR)
179 179
         {
180
-            msg(M_WARN, "Failed to process push update message sent to client ID: %u",
181
-                c->c2.tls_multi ? c->c2.tls_multi->peer_id : UINT32_MAX);
180
+            msg(M_WARN, "Failed to process push update message sent to client ID: %u", c->c2.tls_multi->peer_id);
182 181
             continue;
183 182
         }
184 183
         c->options.push_option_types_found |= *option_types_found;
185 184
         if (!options_postprocess_pull(&c->options, c->c2.es))
186 185
         {
187
-            msg(M_WARN, "Failed to post-process push update message sent to client ID: %u",
188
-                c->c2.tls_multi ? c->c2.tls_multi->peer_id : UINT32_MAX);
186
+            msg(M_WARN, "Failed to post-process push update message sent to client ID: %u", c->c2.tls_multi->peer_id);
189 187
         }
190 188
     }
191 189
     return true;
192 190
 }
193 191
 
192
+/* Return true if the client supports push-update */
193
+static bool
194
+support_push_update(struct multi_instance *mi)
195
+{
196
+    ASSERT(mi->context.c2.tls_multi);
197
+    const unsigned int iv_proto_peer = extract_iv_proto(mi->context.c2.tls_multi->peer_info);
198
+    if (!(iv_proto_peer & IV_PROTO_PUSH_UPDATE))
199
+    {
200
+        return false;
201
+    }
202
+
203
+    return true;
204
+}
205
+
194 206
 int
195 207
 send_push_update(struct multi_context *m, const void *target, const char *msg, const push_update_type type, const int push_bundle_size)
196 208
 {
... ...
@@ -231,9 +244,17 @@ send_push_update(struct multi_context *m, const void *target, const char *msg, c
231 231
 
232 232
         if (!mi)
233 233
         {
234
+            gc_free(&gc);
234 235
             return -ENOENT;
235 236
         }
236 237
 
238
+        if (!support_push_update(mi))
239
+        {
240
+            msg(M_CLIENT, "PUSH_UPDATE: not sending message to unsupported peer with ID: %u", mi->context.c2.tls_multi->peer_id);
241
+            gc_free(&gc);
242
+            return 0;
243
+        }
244
+
237 245
         const char *old_ip = mi->context.options.ifconfig_local;
238 246
         const char *old_ipv6 = mi->context.options.ifconfig_ipv6_local;
239 247
         if (!mi->halt
... ...
@@ -262,7 +283,7 @@ send_push_update(struct multi_context *m, const void *target, const char *msg, c
262 262
     {
263 263
         struct multi_instance *curr_mi = he->value;
264 264
 
265
-        if (curr_mi->halt)
265
+        if (curr_mi->halt || !support_push_update(curr_mi))
266 266
         {
267 267
             continue;
268 268
         }
... ...
@@ -273,8 +294,7 @@ send_push_update(struct multi_context *m, const void *target, const char *msg, c
273 273
         const char *old_ipv6 = curr_mi->context.options.ifconfig_ipv6_local;
274 274
         if (!send_single_push_update(&curr_mi->context, msgs, &option_types_found))
275 275
         {
276
-            msg(M_CLIENT, "ERROR: Peer ID: %u has not been updated",
277
-                curr_mi->context.c2.tls_multi ? curr_mi->context.c2.tls_multi->peer_id : UINT32_MAX);
276
+            msg(M_CLIENT, "ERROR: Peer ID: %u has not been updated", curr_mi->context.c2.tls_multi->peer_id);
278 277
             continue;
279 278
         }
280 279
         if (option_types_found & OPT_P_UP)
... ...
@@ -144,7 +144,13 @@ mroute_extract_openvpn_sockaddr(struct mroute_addr *addr,
144 144
 {
145 145
     return true;
146 146
 }
147
-#endif /* ifndef ENABLE_MANAGEMENT */
147
+
148
+unsigned int
149
+extract_iv_proto(const char *peer_info)
150
+{
151
+    return IV_PROTO_PUSH_UPDATE;
152
+}
153
+#endif /* ifdef ENABLE_MANAGEMENT */
148 154
 
149 155
 /* tests */
150 156
 
... ...
@@ -464,6 +470,7 @@ setup2(void **state)
464 464
     struct multi_context *m = calloc(1, sizeof(struct multi_context));
465 465
     m->instances = calloc(1, sizeof(struct multi_instance *));
466 466
     struct multi_instance *mi = calloc(1, sizeof(struct multi_instance));
467
+    mi->context.c2.tls_multi = calloc(1, sizeof(struct tls_multi));
467 468
     *(m->instances) = mi;
468 469
     m->top.options.disable_dco = true;
469 470
     *state = m;
... ...
@@ -474,6 +481,7 @@ static int
474 474
 teardown2(void **state)
475 475
 {
476 476
     struct multi_context *m = *state;
477
+    free((*(m->instances))->context.c2.tls_multi);
477 478
     free(*(m->instances));
478 479
     free(m->instances);
479 480
     free(m);