[Openvpn-devel,M] Change in openvpn[master]: mroute: properly print protocol at the end of the string

Message ID d8b9a4fe6b301cbd19def7d87804bc09e2215aeb-HTML@gerrit.openvpn.net
State Not Applicable
Headers show
Series [Openvpn-devel,M] Change in openvpn[master]: mroute: properly print protocol at the end of the string | expand

Commit Message

d12fk (Code Review) Nov. 15, 2023, 1:45 p.m. UTC
Attention is currently required from: flichtenheld, plaisthos.

Hello plaisthos, flichtenheld,

I'd like you to do a code review.
Please visit

    http://gerrit.openvpn.net/c/openvpn/+/439?usp=email

to review the following change.


Change subject: mroute: properly print protocol at the end of the string
......................................................................

mroute: properly print protocol at the end of the string

mroute: substitute unused field with proto

Rather than adding a new field 'proto', take advantage of the 'unused'
field and rename it.

Hashing will now start at the 'proto' field rather than 'type'.

MULTI: ensure we've got the correct protocol with virtual addresses

MULTI: ensure we've got the same value as protocol for vaddressed

Change-Id: I6688362d8461c112bf425ddfe488d511a64cc37e
Signed-off-by: Gianmarco De Gregori <gianmarco@mandelbit.com>
---
M src/openvpn/forward.c
M src/openvpn/mroute.c
M src/openvpn/mroute.h
M src/openvpn/mtcp.c
M src/openvpn/mudp.c
M src/openvpn/multi.c
M src/openvpn/ssl.c
7 files changed, 45 insertions(+), 29 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/39/439/1

Patch

diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
index 27415ee..63a684b 100644
--- a/src/openvpn/forward.c
+++ b/src/openvpn/forward.c
@@ -1114,13 +1114,16 @@ 
         decrypt_status = openvpn_decrypt(&c->c2.buf, c->c2.buffers->decrypt_buf,
                                          co, &c->c2.frame, ad_start);
 
-        if (!decrypt_status
-            /* all sockets are of the same type, so just check the first one */
-            && link_socket_connection_oriented(c->c2.link_sockets[0]))
+        for (int i = 0; i < c->c1.link_sockets_num; i++)
         {
-            /* decryption errors are fatal in TCP mode */
-            register_signal(c->sig, SIGUSR1, "decryption-error"); /* SOFT-SIGUSR1 -- decryption error in TCP mode */
-            msg(D_STREAM_ERRORS, "Fatal decryption error (process_incoming_link), restarting");
+            if (!decrypt_status
+                /* all sockets are of the same type, so just check the first one (not anymore!) */
+                && link_socket_connection_oriented(c->c2.link_sockets[i]))
+            {
+                /* decryption errors are fatal in TCP mode */
+                register_signal(c->sig, SIGUSR1, "decryption-error"); /* SOFT-SIGUSR1 -- decryption error in TCP mode */
+                msg(D_STREAM_ERRORS, "Fatal decryption error (process_incoming_link), restarting");
+            }
         }
     }
     else
@@ -2239,6 +2242,7 @@ 
 
             if (status > 0)
             {
+                /*printf("\nstatus: %d\n", status); */
                 int i;
                 mtcp->event_set_status = 0;
                 for (i = 0; i < status; ++i)
@@ -2275,10 +2279,6 @@ 
                 mtcp->event_set_status = ES_TIMEOUT;
             }
         }
-        else
-        {
-            mtcp->event_set_status = SOCKET_READ;
-        }
     }
 
     /* 'now' should always be a reasonably up-to-date timestamp */
diff --git a/src/openvpn/mroute.c b/src/openvpn/mroute.c
index 0017a48..c72fe10 100644
--- a/src/openvpn/mroute.c
+++ b/src/openvpn/mroute.c
@@ -421,7 +421,6 @@ 
                 {
                     buf_printf(&out, ":%d", ntohs(maddr.v4.port));
                 }
-                buf_printf(&out, ":%d", maddr.proto);
             }
             break;
 
@@ -454,6 +453,7 @@ 
                 buf_printf(&out, "UNKNOWN");
                 break;
         }
+        buf_printf(&out, "|%d", maddr.proto);
         return BSTR(&out);
     }
     else
diff --git a/src/openvpn/mroute.h b/src/openvpn/mroute.h
index 7c8972f..4e6d32c 100644
--- a/src/openvpn/mroute.h
+++ b/src/openvpn/mroute.h
@@ -74,9 +74,8 @@ 
 
 struct mroute_addr {
     uint8_t len;    /* length of address */
-    uint8_t unused;
-    uint8_t type;   /* MR_ADDR/MR_WITH flags */
     uint8_t proto;
+    uint8_t type;   /* MR_ADDR/MR_WITH flags */
     uint8_t netbits; /* number of bits in network part of address,
                       * valid if MR_WITH_NETBITS is set */
     union {
@@ -231,7 +230,7 @@ 
 mroute_addr_hash_ptr(const struct mroute_addr *a)
 {
     /* NOTE: depends on ordering of struct mroute_addr */
-    return (uint8_t *) &a->type;
+    return (uint8_t *) &a->proto;
 }
 
 static inline uint32_t
diff --git a/src/openvpn/mtcp.c b/src/openvpn/mtcp.c
index d4ce642..ba0905e 100644
--- a/src/openvpn/mtcp.c
+++ b/src/openvpn/mtcp.c
@@ -109,7 +109,7 @@ 
     mi = multi_create_instance(m, NULL, ls);
     if (mi && !proto_is_dgram(ls->info.proto))
     {
-        printf("\nTCP add\n");
+        mi->real.proto = ls->info.proto;
         struct hash_element *he;
         const uint32_t hv = hash_value(hash, &mi->real);
         struct hash_bucket *bucket = hash_bucket(hash, hv);
@@ -746,22 +746,26 @@ 
                                                             ev_arg->u.ls);
                             }
                         }
-                        multi_get_timeout(m, &m->top.c2.timeval);
-                        io_wait_udp(&m->top, m->mtcp, p2mp_iow_flags(m));
-                        MULTI_CHECK_SIG(m);
 
-                        multi_process_per_second_timers(m);
-
-                        if (m->mtcp->event_set_status == ES_TIMEOUT)
+                        while (true)
                         {
-                            multi_process_timeout(m, MPP_PRE_SELECT | MPP_CLOSE_ON_SIGNAL);
-                        }
-                        else
-                        {
-                            multi_process_io_udp(m);
+                            multi_get_timeout(m, &m->top.c2.timeval);
+                            io_wait_udp(&m->top, m->mtcp, p2mp_iow_flags(m));
                             MULTI_CHECK_SIG(m);
-                        }
 
+                            multi_process_per_second_timers(m);
+
+                            if (m->mtcp->event_set_status == ES_TIMEOUT)
+                            {
+                                multi_process_timeout(m, MPP_PRE_SELECT | MPP_CLOSE_ON_SIGNAL);
+                            }
+                            else
+                            {
+                                multi_process_io_udp(m);
+                                MULTI_CHECK_SIG(m);
+                                break;
+                            }
+                        }
                         break;
                     }
             }
diff --git a/src/openvpn/mudp.c b/src/openvpn/mudp.c
index e9182c8..4979751 100644
--- a/src/openvpn/mudp.c
+++ b/src/openvpn/mudp.c
@@ -193,6 +193,7 @@ 
     struct multi_instance *mi = NULL;
     struct hash *hash = m->hash;
     real.proto = ls->info.proto;
+    m->local.proto = real.proto;
 
     if (mroute_extract_openvpn_sockaddr(&real, &m->top.c2.from.dest, true)
         && m->top.c2.buf.len > 0)
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 3522206..5098581 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -1157,11 +1157,12 @@ 
  */
 static struct multi_instance *
 multi_get_instance_by_virtual_addr(struct multi_context *m,
-                                   const struct mroute_addr *addr,
+                                   struct mroute_addr *addr,
                                    bool cidr_routing)
 {
     struct multi_route *route;
     struct multi_instance *ret = NULL;
+    addr->proto = 0;
 
     /* check for local address */
     if (mroute_addr_equal(addr, &m->local))
@@ -1247,6 +1248,7 @@ 
     CLEAR(remote_si);
     remote_si.addr.in4.sin_family = AF_INET;
     remote_si.addr.in4.sin_addr.s_addr = htonl(a);
+    addr.proto = 0;
     ASSERT(mroute_extract_openvpn_sockaddr(&addr, &remote_si, false));
 
     if (netbits >= 0)
@@ -3351,6 +3353,14 @@ 
     bool ret = true;
     bool floated = false;
 
+    /*
+     * Since we don't really need the protocol on vaddresses for internal VPN
+     * payload packets, make sure we have the same value to void hashing insert
+     * and search issues.
+     */
+    src.proto = 0;
+    dest.proto = src.proto;
+
     if (m->pending)
     {
         return true;
@@ -3417,7 +3427,6 @@ 
                                                                0,
                                                                &c->c2.to_tun,
                                                                DEV_TYPE_TUN);
-
                 /* drop packet if extract failed */
                 if (!(mroute_flags & MROUTE_EXTRACT_SUCCEEDED))
                 {
@@ -3555,6 +3564,8 @@ 
         const int dev_type = TUNNEL_TYPE(m->top.c1.tuntap);
         int16_t vid = 0;
 
+        src.proto = 0;
+        dest.proto = src.proto;
 
 #ifdef MULTI_DEBUG_EVENT_LOOP
         printf("TUN -> TCP/UDP [%d]\n", BLEN(&m->top.c2.buf));
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index cee4afe..73d6db0 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -3926,6 +3926,7 @@ 
                     {
                         msg(D_MULTI_DROPPED,
                             "Incoming control channel packet too big, dropping.");
+                        printf("\nif (!buf_copy(in, buf))\n");
                         goto error;
                     }
                     reliable_mark_active_incoming(ks->rec_reliable, in, id, op);