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

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

Commit Message

cron2 (Code Review) Sept. 23, 2024, 1:41 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/+/765?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 vaddresses

Change-Id: Ic66eccb5058fe9c0fae64d8e2ca88728068a92ab
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
6 files changed, 27 insertions(+), 12 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/65/765/1

Patch

diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
index 7559a71..1357cad 100644
--- a/src/openvpn/forward.c
+++ b/src/openvpn/forward.c
@@ -1129,13 +1129,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
diff --git a/src/openvpn/mroute.c b/src/openvpn/mroute.c
index 6c8e8dd..3a0224e 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 fd1dbfe..e844c21 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 {
@@ -221,7 +220,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 73f6bcc..1eb28ec 100644
--- a/src/openvpn/mtcp.c
+++ b/src/openvpn/mtcp.c
@@ -56,6 +56,7 @@ 
     mi = multi_create_instance(m, NULL, ls);
     if (mi && !proto_is_dgram(ls->info.proto))
     {
+        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);
diff --git a/src/openvpn/mudp.c b/src/openvpn/mudp.c
index a7e6e1d..f9efcd5 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 2d4fbe7..4a6dd52 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -1156,11 +1156,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))
@@ -1246,6 +1247,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)
@@ -3346,6 +3348,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;
@@ -3412,7 +3422,6 @@ 
                                                                0,
                                                                &c->c2.to_tun,
                                                                DEV_TYPE_TUN);
-
                 /* drop packet if extract failed */
                 if (!(mroute_flags & MROUTE_EXTRACT_SUCCEEDED))
                 {
@@ -3550,6 +3559,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));