Browse code

mroute/management: repair mgmt client-kill for mroute with proto

Fix issue reported by Coverity:
CID 1641564: Uninitialized variables (UNINIT)
Using unitialized value "maddr.proto" when
calling "mroute_addr_equal()".

Due to changes at the mroute structure
which now includes the protocol, the mgmt
iface client-kill-by-addr feature has been
updated to include this new value along
with IP:port.

While at it, changed the
mroute_addr_print_ex() format to display
the protocol only in case of MR_WITH_PROTO
avoid doing it on virtual addresses when
MR_WITH_PORT is not specified.

Change-Id: I4be0ff4d308213d2ef8ba66bd3178eee1f60fff1
Signed-off-by: Gianmarco De Gregori <gianmarco@mandelbit.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20250129161609.19202-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg30716.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>

Gianmarco De Gregori authored on 2025/01/30 01:16:08
Showing 8 changed files
... ...
@@ -317,6 +317,9 @@ User-visible Changes
317 317
   settings will contradict the setting of allow-compression as this almost
318 318
   always results in a non-working connection.
319 319
 
320
+- The "kill" by addr management command now requires also the protocol
321
+  as string e.g. "udp", "tcp".
322
+
320 323
 Common errors with OpenSSL 3.0 and OpenVPN 2.6
321 324
 ----------------------------------------------
322 325
 Both OpenVPN 2.6 and OpenSSL 3.0 tighten the security considerable, so some
... ...
@@ -205,8 +205,12 @@ Command examples:
205 205
 
206 206
   kill Test-Client -- kill the client instance having a
207 207
                       common name of "Test-Client".
208
-  kill 1.2.3.4:4000 -- kill the client instance having a
209
-                       source address and port of 1.2.3.4:4000
208
+  kill tcp:1.2.3.4:4000 -- kill the client instance having a
209
+                           source address, port and proto of
210
+                           tcp:1.2.3.4:4000
211
+
212
+  Note that kill by address won't work for IPv6-connected
213
+  clients yet, so rely on kill by CN or CID instead.
210 214
 
211 215
 Use the "status" command to see which clients are connected.
212 216
 
... ...
@@ -544,45 +544,52 @@ man_kill(struct management *man, const char *victim)
544 544
         struct buffer buf;
545 545
         char p1[128];
546 546
         char p2[128];
547
+        char p3[128];
547 548
         int n_killed;
548 549
 
549 550
         buf_set_read(&buf, (uint8_t *) victim, strlen(victim) + 1);
550 551
         buf_parse(&buf, ':', p1, sizeof(p1));
551 552
         buf_parse(&buf, ':', p2, sizeof(p2));
553
+        buf_parse(&buf, ':', p3, sizeof(p3));
552 554
 
553
-        if (strlen(p1) && strlen(p2))
555
+        if (strlen(p1) && strlen(p2) && strlen(p3))
554 556
         {
555 557
             /* IP:port specified */
556 558
             bool status;
557
-            const in_addr_t addr = getaddr(GETADDR_HOST_ORDER|GETADDR_MSG_VIRT_OUT, p1, 0, &status, NULL);
559
+            const in_addr_t addr = getaddr(GETADDR_HOST_ORDER|GETADDR_MSG_VIRT_OUT, p2, 0, &status, NULL);
558 560
             if (status)
559 561
             {
560
-                const int port = atoi(p2);
561
-                if (port > 0 && port < 65536)
562
+                const int port = atoi(p3);
563
+                const int proto = (streq(p1, "tcp")) ? PROTO_TCP_SERVER :
564
+                                  (streq(p1, "udp")) ? PROTO_UDP : PROTO_NONE;
565
+
566
+                if ((port > 0 && port < 65536) && (proto != PROTO_NONE))
562 567
                 {
563
-                    n_killed = (*man->persist.callback.kill_by_addr)(man->persist.callback.arg, addr, port);
568
+                    n_killed = (*man->persist.callback.kill_by_addr)(man->persist.callback.arg, addr, port, proto);
564 569
                     if (n_killed > 0)
565 570
                     {
566
-                        msg(M_CLIENT, "SUCCESS: %d client(s) at address %s:%d killed",
571
+                        msg(M_CLIENT, "SUCCESS: %d client(s) at address %s:%s:%d killed",
567 572
                             n_killed,
573
+                            proto2ascii(proto, AF_UNSPEC, false),
568 574
                             print_in_addr_t(addr, 0, &gc),
569 575
                             port);
570 576
                     }
571 577
                     else
572 578
                     {
573
-                        msg(M_CLIENT, "ERROR: client at address %s:%d not found",
579
+                        msg(M_CLIENT, "ERROR: client at address %s:%s:%d not found",
580
+                            proto2ascii(proto, AF_UNSPEC, false),
574 581
                             print_in_addr_t(addr, 0, &gc),
575 582
                             port);
576 583
                     }
577 584
                 }
578 585
                 else
579 586
                 {
580
-                    msg(M_CLIENT, "ERROR: port number is out of range: %s", p2);
587
+                    msg(M_CLIENT, "ERROR: port number or protocol out of range: %s %s", p3, p1);
581 588
                 }
582 589
             }
583 590
             else
584 591
             {
585
-                msg(M_CLIENT, "ERROR: error parsing IP address: %s", p1);
592
+                msg(M_CLIENT, "ERROR: error parsing IP address: %s", p2);
586 593
             }
587 594
         }
588 595
         else if (strlen(p1))
... ...
@@ -180,7 +180,7 @@ struct management_callback
180 180
     void (*status) (void *arg, const int version, struct status_output *so);
181 181
     void (*show_net) (void *arg, const int msglevel);
182 182
     int (*kill_by_cn) (void *arg, const char *common_name);
183
-    int (*kill_by_addr) (void *arg, const in_addr_t addr, const int port);
183
+    int (*kill_by_addr) (void *arg, const in_addr_t addr, const int port, const int proto);
184 184
     void (*delete_event) (void *arg, event_t event);
185 185
     int (*n_clients) (void *arg);
186 186
     bool (*send_cc_message) (void *arg, const char *message, const char *parameter);
... ...
@@ -276,6 +276,10 @@ mroute_extract_openvpn_sockaddr(struct mroute_addr *addr,
276 276
                 addr->len = 6;
277 277
                 addr->v4.addr = osaddr->addr.in4.sin_addr.s_addr;
278 278
                 addr->v4.port = osaddr->addr.in4.sin_port;
279
+                if (addr->proto != PROTO_NONE)
280
+                {
281
+                    addr->type |= MR_WITH_PROTO;
282
+                }
279 283
             }
280 284
             else
281 285
             {
... ...
@@ -295,6 +299,10 @@ mroute_extract_openvpn_sockaddr(struct mroute_addr *addr,
295 295
                 addr->len = 18;
296 296
                 addr->v6.addr = osaddr->addr.in6.sin6_addr;
297 297
                 addr->v6.port = osaddr->addr.in6.sin6_port;
298
+                if (addr->proto != PROTO_NONE)
299
+                {
300
+                    addr->type |= MR_WITH_PROTO;
301
+                }
298 302
             }
299 303
             else
300 304
             {
... ...
@@ -403,6 +411,10 @@ mroute_addr_print_ex(const struct mroute_addr *ma,
403 403
                 {
404 404
                     buf_printf(&out, "ARP/");
405 405
                 }
406
+                if (maddr.type & MR_WITH_PROTO)
407
+                {
408
+                    buf_printf(&out, "%s:", proto2ascii(maddr.proto, AF_INET, false));
409
+                }
406 410
                 buf_printf(&out, "%s", print_in_addr_t(ntohl(maddr.v4.addr),
407 411
                                                        (flags & MAPF_IA_EMPTY_IF_UNDEF) ? IA_EMPTY_IF_UNDEF : 0, gc));
408 412
                 if (maddr.type & MR_WITH_NETBITS)
... ...
@@ -426,6 +438,10 @@ mroute_addr_print_ex(const struct mroute_addr *ma,
426 426
 
427 427
             case MR_ADDR_IPV6:
428 428
             {
429
+                if (maddr.type & MR_WITH_PROTO)
430
+                {
431
+                    buf_printf(&out, "%s:", proto2ascii(maddr.proto, AF_INET6, false));
432
+                }
429 433
                 if (IN6_IS_ADDR_V4MAPPED( &maddr.v6.addr ) )
430 434
                 {
431 435
                     buf_printf(&out, "%s", print_in_addr_t(maddr.v4mappedv6.addr,
... ...
@@ -454,7 +470,6 @@ mroute_addr_print_ex(const struct mroute_addr *ma,
454 454
                 buf_printf(&out, "UNKNOWN");
455 455
                 break;
456 456
         }
457
-        buf_printf(&out, "|%d", maddr.proto);
458 457
         return BSTR(&out);
459 458
     }
460 459
     else
... ...
@@ -72,6 +72,9 @@
72 72
 /* Indicates than IPv4 addr was extracted from ARP packet */
73 73
 #define MR_ARP                   16
74 74
 
75
+/* Address type mask indicating that proto # is part of address */
76
+#define MR_WITH_PROTO            32
77
+
75 78
 struct mroute_addr {
76 79
     uint8_t len;    /* length of address */
77 80
     uint8_t proto;
... ...
@@ -111,6 +111,7 @@ multi_tcp_instance_specific_init(struct multi_context *m, struct multi_instance
111 111
     ASSERT(mi->context.c2.link_sockets[0]->info.lsa->actual.dest.addr.sa.sa_family == AF_INET
112 112
            || mi->context.c2.link_sockets[0]->info.lsa->actual.dest.addr.sa.sa_family == AF_INET6
113 113
            );
114
+    mi->real.proto = mi->context.c2.link_sockets[0]->info.proto;
114 115
     if (!mroute_extract_openvpn_sockaddr(&mi->real,
115 116
                                          &mi->context.c2.link_sockets[0]->info.lsa->actual.dest,
116 117
                                          true))
... ...
@@ -794,7 +794,6 @@ multi_create_instance(struct multi_context *m, const struct mroute_addr *real,
794 794
         {
795 795
             goto err;
796 796
         }
797
-        mi->real.proto = ls->info.proto;
798 797
         generate_prefix(mi);
799 798
     }
800 799
 
... ...
@@ -3942,7 +3941,8 @@ management_callback_kill_by_cn(void *arg, const char *del_cn)
3942 3942
 }
3943 3943
 
3944 3944
 static int
3945
-management_callback_kill_by_addr(void *arg, const in_addr_t addr, const int port)
3945
+management_callback_kill_by_addr(void *arg, const in_addr_t addr,
3946
+                                 const int port, const int proto)
3946 3947
 {
3947 3948
     struct multi_context *m = (struct multi_context *) arg;
3948 3949
     struct hash_iterator hi;
... ...
@@ -3957,6 +3957,7 @@ management_callback_kill_by_addr(void *arg, const in_addr_t addr, const int port
3957 3957
     saddr.addr.in4.sin_port = htons(port);
3958 3958
     if (mroute_extract_openvpn_sockaddr(&maddr, &saddr, true))
3959 3959
     {
3960
+        maddr.proto = proto;
3960 3961
         hash_iterator_init(m->iter, &hi);
3961 3962
         while ((he = hash_iterator_next(&hi)))
3962 3963
         {