Thread: [Openvpn-devel] [M] Change in openvpn[master]: multipeer: introduce asymmetric peer-id

SourceForge Headquarters 225 Broadway Suite 1600 San Diego, CA 92101 +1 (858) 422-6466

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/+/1089?usp=email

to review the following change.


Change subject: multipeer: introduce asymmetric peer-id
......................................................................

multipeer: introduce asymmetric peer-id

In order to achieve a multipeer functionality, peers now
use separate IDs for sending (tx_peer_id) and receiving
(rx_peer_id).

Each peer announces its own ID through pushing peer-info
using 'ID=7f1' hex format so identification can still
happen even if IP/port changes.

In P2P mode, peer switch to using the announced IDs after
mutual exchange.
In P2MP mode, clients always announce their ID, and servers
can optionally respond with their own to enable the same
behavior.

Change-Id: I0a13ee90b6706acf20eabcee3bab3f2dff639bf9
Signed-off-by: Gianmarco De Gregori <gia...@ma...>
---
M src/openvpn/dco.c
M src/openvpn/init.c
M src/openvpn/misc.c
M src/openvpn/multi.c
M src/openvpn/push.c
M src/openvpn/ssl.c
M src/openvpn/ssl_common.h
M src/openvpn/ssl_ncp.c
M src/openvpn/ssl_util.c
M src/openvpn/ssl_util.h
M tests/unit_tests/openvpn/test_crypto.c
11 files changed, 65 insertions(+), 27 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/89/1089/1

diff --git a/src/openvpn/dco.c b/src/openvpn/dco.c
index 98cbb72..3687f4a 100644
--- a/src/openvpn/dco.c
+++ b/src/openvpn/dco.c
@@ -513,7 +513,7 @@
         c->c2.tls_multi->dco_peer_id = -1;
     }
 #endif
-    int ret = dco_new_peer(&c->c1.tuntap->dco, multi->peer_id, sock->sd, NULL,
+    int ret = dco_new_peer(&c->c1.tuntap->dco, multi->rx_peer_id, sock->sd, NULL,
                            proto_is_dgram(sock->info.proto) ? remoteaddr : NULL,
                            NULL, NULL);
     if (ret < 0)
@@ -521,7 +521,7 @@
         return ret;
     }
 
-    c->c2.tls_multi->dco_peer_id = multi->peer_id;
+    c->c2.tls_multi->dco_peer_id = multi->rx_peer_id;
 
     return 0;
 }
@@ -595,7 +595,7 @@
 {
     struct context *c = &mi->context;
 
-    int peer_id = c->c2.tls_multi->peer_id;
+    int peer_id = c->c2.tls_multi->rx_peer_id;
     struct sockaddr *remoteaddr, *localaddr = NULL;
     struct sockaddr_storage local = { 0 };
     int sd = c->c2.link_sockets[0]->sd;
@@ -667,7 +667,7 @@
     if (addrtype == MR_ADDR_IPV6)
     {
 #if defined(_WIN32)
-        dco_win_add_iroute_ipv6(&c->c1.tuntap->dco, addr->v6.addr, addr->netbits, c->c2.tls_multi->peer_id);
+        dco_win_add_iroute_ipv6(&c->c1.tuntap->dco, addr->v6.addr, addr->netbits, c->c2.tls_multi->rx_peer_id);
 #else
         net_route_v6_add(&m->top.net_ctx, &addr->v6.addr, addr->netbits,
                          &mi->context.c2.push_ifconfig_ipv6_local, c->c1.tuntap->actual_name, 0,
@@ -677,7 +677,7 @@
     else if (addrtype == MR_ADDR_IPV4)
     {
 #if defined(_WIN32)
-        dco_win_add_iroute_ipv4(&c->c1.tuntap->dco, addr->v4.addr, addr->netbits, c->c2.tls_multi->peer_id);
+        dco_win_add_iroute_ipv4(&c->c1.tuntap->dco, addr->v4.addr, addr->netbits, c->c2.tls_multi->rx_peer_id);
 #else
         in_addr_t dest = htonl(addr->v4.addr);
         net_route_v4_add(&m->top.net_ctx, &dest, addr->netbits,
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 77747a2..543eaf9 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -2328,7 +2328,7 @@
 
     if (o->use_peer_id)
     {
-        buf_printf(&out, ", peer-id: %d", o->peer_id);
+        buf_printf(&out, ", rx_peer-id: %u, tx_peer-id: %u", c->c2.tls_multi->rx_peer_id, c->c2.tls_multi->tx_peer_id);
     }
 
 #ifdef USE_COMP
@@ -2778,7 +2778,7 @@
     {
         msg(D_PUSH_DEBUG, "OPTIONS IMPORT: peer-id set");
         c->c2.tls_multi->use_peer_id = true;
-        c->c2.tls_multi->peer_id = c->options.peer_id;
+        c->c2.tls_multi->tx_peer_id = c->options.peer_id;
     }
 
     /* process (potentially) pushed options */
diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c
index 4695700..122ca74 100644
--- a/src/openvpn/misc.c
+++ b/src/openvpn/misc.c
@@ -777,7 +777,8 @@
     {
         chomp(line);
         if (validate_peer_info_line(line)
-            && (strncmp(line, "IV_", 3) == 0 || strncmp(line, "UV_", 3) == 0) )
+            && (strncmp(line, "IV_", 3) == 0 || strncmp(line, "UV_", 3) == 0
+                || strncmp(line, "ID", 2) == 0))
         {
             msg(M_INFO, "peer info: %s", line);
             env_set_add(es, line);
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index a760e07..6987dc5 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -479,7 +479,7 @@
             && check_debug_level(D_DCO_DEBUG)
             && dco_enabled(&mi->context.options))
         {
-            buf_printf(&out, " peer-id=%d", mi->context.c2.tls_multi->peer_id);
+            buf_printf(&out, " rx_peer-id=%d", mi->context.c2.tls_multi->rx_peer_id);
         }
         return BSTR(&out);
     }
@@ -655,9 +655,9 @@
         }
 #endif
 
-        if (mi->context.c2.tls_multi->peer_id != MAX_PEER_ID)
+        if (mi->context.c2.tls_multi->rx_peer_id != MAX_PEER_ID)
         {
-            m->instances[mi->context.c2.tls_multi->peer_id] = NULL;
+            m->instances[mi->context.c2.tls_multi->rx_peer_id] = NULL;
         }
 
         schedule_remove_entry(m->schedule, (struct schedule_entry *) mi);
@@ -972,7 +972,7 @@
 #else
                                   sep,
 #endif
-                                  sep, mi->context.c2.tls_multi ? mi->context.c2.tls_multi->peer_id : UINT32_MAX,
+                                  sep, mi->context.c2.tls_multi ? mi->context.c2.tls_multi->rx_peer_id : UINT32_MAX,
                                   sep, translate_cipher_name_to_openvpn(mi->context.options.ciphername));
                 }
                 gc_free(&gc);
@@ -1813,6 +1813,12 @@
     {
         tls_multi->use_peer_id = true;
         o->use_peer_id = true;
+        uint32_t peer_id = extract_asymmetric_peer_id(peer_info);
+        if (peer_id)
+        {
+            tls_multi->tx_peer_id = peer_id;
+            tls_multi->use_asymmetric_peer_id = true;
+        }
     }
     else if (dco_enabled(o))
     {
@@ -3256,7 +3262,7 @@
     }
 
     msg(D_MULTI_MEDIUM, "peer %" PRIu32 " (%s) floated from %s to %s",
-        mi->context.c2.tls_multi->peer_id,
+        mi->context.c2.tls_multi->rx_peer_id,
         tls_common_name(mi->context.c2.tls_multi, false),
         mroute_addr_print(&mi->real, &gc),
         print_link_socket_actual(&m->top.c2.from, &gc));
@@ -4235,7 +4241,11 @@
     {
         if (!m->instances[i])
         {
-            mi->context.c2.tls_multi->peer_id = i;
+            mi->context.c2.tls_multi->rx_peer_id = i;
+            if (!mi->context.c2.tls_multi->use_asymmetric_peer_id)
+            {
+                mi->context.c2.tls_multi->tx_peer_id = i;
+            }
             m->instances[i] = mi;
             break;
         }
@@ -4243,7 +4253,7 @@
 
     /* should not really end up here, since multi_create_instance returns null
      * if amount of clients exceeds max_clients */
-    ASSERT(mi->context.c2.tls_multi->peer_id < m->max_clients);
+    ASSERT(mi->context.c2.tls_multi->rx_peer_id < m->max_clients);
 }
 
 /**************************************************************************/
diff --git a/src/openvpn/push.c b/src/openvpn/push.c
index ad8fa3d7..073e6b6 100644
--- a/src/openvpn/push.c
+++ b/src/openvpn/push.c
@@ -654,7 +654,7 @@
     if (tls_multi->use_peer_id)
     {
         push_option_fmt(gc, push_list, M_USAGE, "peer-id %d",
-                        tls_multi->peer_id);
+                        tls_multi->rx_peer_id);
     }
     /*
      * If server uses --auth-gen-token and we have an auth token
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 9c6616a..edac9aa 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -1181,7 +1181,9 @@
     /* get command line derived options */
     ret->opt = *tls_options;
     ret->dco_peer_id = -1;
-    ret->peer_id = MAX_PEER_ID;
+    ret->use_asymmetric_peer_id = false;
+    ret->rx_peer_id = MAX_PEER_ID;
+    ret->tx_peer_id = MAX_PEER_ID;
 
     return ret;
 }
@@ -1947,7 +1949,7 @@
  * @return          true if no error was encountered
  */
 static bool
-push_peer_info(struct buffer *buf, struct tls_session *session)
+push_peer_info(struct buffer *buf, struct tls_session *session, uint32_t peer_id)
 {
     struct gc_arena gc = gc_new();
     bool ret = false;
@@ -2038,6 +2040,7 @@
         iv_proto |= IV_PROTO_DYN_TLS_CRYPT;
 
         buf_printf(&out, "IV_PROTO=%d\n", iv_proto);
+        buf_printf(&out, "ID=%x\n", peer_id);
 
         if (session->opt->push_peer_info_detail > 1)
         {
@@ -2221,7 +2224,7 @@
         }
     }
 
-    if (!push_peer_info(buf, session))
+    if (!push_peer_info(buf, session, multi->rx_peer_id))
     {
         goto error;
     }
@@ -4143,9 +4146,8 @@
     msg(D_TLS_DEBUG, __func__);
 
     ASSERT(ks);
-
     peer = htonl(((P_DATA_V2 << P_OPCODE_SHIFT) | ks->key_id) << 24
-                 | (multi->peer_id & 0xFFFFFF));
+                 | (multi->tx_peer_id & 0xFFFFFF));
     ASSERT(buf_write_prepend(buf, &peer, 4));
 }
 
diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h
index e9e50da..1e2f534 100644
--- a/src/openvpn/ssl_common.h
+++ b/src/openvpn/ssl_common.h
@@ -689,8 +689,10 @@
 #define AUTH_TOKEN_VALID_EMPTYUSER (1 << 2)
 
     /* For P_DATA_V2 */
-    uint32_t peer_id;
+    uint32_t rx_peer_id;
+    uint32_t tx_peer_id;
     bool use_peer_id;
+    bool use_asymmetric_peer_id;
 
     char *remote_ciphername;    /**< cipher specified in peer's config file */
     bool remote_usescomp;       /**< remote announced comp-lzo in OCC string */
diff --git a/src/openvpn/ssl_ncp.c b/src/openvpn/ssl_ncp.c
index 74d7b43..5e0af03 100644
--- a/src/openvpn/ssl_ncp.c
+++ b/src/openvpn/ssl_ncp.c
@@ -426,7 +426,9 @@
     if (iv_proto_peer & IV_PROTO_DATA_V2)
     {
         multi->use_peer_id = true;
-        multi->peer_id = 0x76706e; /* 'v' 'p' 'n' */
+        multi->use_asymmetric_peer_id = true;
+        multi->rx_peer_id = 0x76706e; /* 'v' 'p' 'n' */
+        multi->tx_peer_id =  2033;
     }
 
     if (iv_proto_peer & IV_PROTO_CC_EXIT_NOTIFY)
@@ -469,7 +471,7 @@
             }
             else
             {
-                multi->peer_id = (peerid[0] << 16) + (peerid[1] << 8) + peerid[2];
+                multi->rx_peer_id = (peerid[0] << 16) + (peerid[1] << 8) + peerid[2];
             }
 
         }
@@ -513,10 +515,11 @@
     }
 
     msg(D_TLS_DEBUG_LOW, "P2P mode NCP negotiation result: "
-        "TLS_export=%d, DATA_v2=%d, peer-id %d, epoch=%d, cipher=%s",
+        "TLS_export=%d, DATA_v2=%d, rx-peer-id %d, tx-peer-id %d, epoch=%d, cipher=%s",
         (bool)(session->opt->crypto_flags & CO_USE_TLS_KEY_MATERIAL_EXPORT),
         multi->use_peer_id,
-        multi->peer_id,
+        multi->rx_peer_id,
+        multi->tx_peer_id,
         (bool)(session->opt->crypto_flags & CO_EPOCH_DATA_KEY_FORMAT),
         common_cipher);
 
diff --git a/src/openvpn/ssl_util.c b/src/openvpn/ssl_util.c
index d3d7b2c..fde6f7e 100644
--- a/src/openvpn/ssl_util.c
+++ b/src/openvpn/ssl_util.c
@@ -74,6 +74,24 @@
     return 0;
 }
 
+uint32_t
+extract_asymmetric_peer_id(const char *peer_info)
+{
+    const char *optstr = peer_info ? strstr(peer_info, "ID=") : NULL;
+    if (optstr)
+    {
+        uint32_t peer_id = 0;
+        int r = sscanf(optstr, "ID=%x", &peer_id);
+        {
+            if (r == 1 && peer_id >= 0)
+            {
+                return peer_id;
+            }
+        }
+    }
+    return 0;
+}
+
 const char *
 options_string_compat_lzo(const char *options, struct gc_arena *gc)
 {
diff --git a/src/openvpn/ssl_util.h b/src/openvpn/ssl_util.h
index e50f899..e9c7ef8 100644
--- a/src/openvpn/ssl_util.h
+++ b/src/openvpn/ssl_util.h
@@ -55,6 +55,8 @@
  */
 unsigned int extract_iv_proto(const char *peer_info);
 
+uint32_t extract_asymmetric_peer_id(const char *peer_info);
+
 /**
  * Takes a locally produced OCC string for TLS server mode and modifies as
  * if the option comp-lzo was enabled. This is to send a client in
diff --git a/tests/unit_tests/openvpn/test_crypto.c b/tests/unit_tests/openvpn/test_crypto.c
index 5b583c7..9d618ca 100644
--- a/tests/unit_tests/openvpn/test_crypto.c
+++ b/tests/unit_tests/openvpn/test_crypto.c
@@ -437,7 +437,7 @@
     o.authname = "SHA1";
     o.ciphername = "AES-256-GCM";
     o.tls_client = true;
-    o.peer_id = 77;
+    o.rx_peer_id = 77;
     o.use_peer_id = true;
     init_key_type(&kt, o.ciphername, o.authname, true, false);
 

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1089?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I0a13ee90b6706acf20eabcee3bab3f2dff639bf9
Gerrit-Change-Number: 1089
Gerrit-PatchSet: 1
Gerrit-Owner: its_Giaan <gia...@ma...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-MessageType: newchange


Thread: [Openvpn-devel] [M] Change in openvpn[master]: multipeer: introduce asymmetric peer-id

Robust and flexible VPN network tunnelling

openvpn-devel