[Openvpn-devel,v3] Use P_DATA_V2 for server->client packets too

Message ID 1511531903-19349-1-git-send-email-steffan.karger@fox-it.com
State Accepted
Headers show
Series
  • [Openvpn-devel,v3] Use P_DATA_V2 for server->client packets too
Related show

Commit Message

Steffan Karger Nov. 24, 2017, 1:58 p.m.
From: Steffan Karger <steffan@karger.me>

P_DATA_V2 introduced the peer-id.  This allows clients to float, but as a
side-effect 32-bit aligns the encrypted data.  That alignment improves
performance particularly on cheaper/older CPUs.  So although servers don't
actually have a peer-id, still use the V2 packet format (with a zero-id)
for server->client traffic too.

Signed-off-by: Steffan Karger <steffan@karger.me>
---
v2: actually enable P_DATA_V2... Now tested with:
    2.4<>2.4 (V2), 2.4-srv<>2.3-clt (V2), 2.3-srv<>2.4-clt (V1),
    2.4-srv<>2.2-clt (V1)
v3: move "use_peer_id = true" inside "if IV_PROTO >= 2" (thanks Gert)

 src/openvpn/forward.c | 4 ++--
 src/openvpn/push.c    | 1 +
 2 files changed, 3 insertions(+), 2 deletions(-)

Comments

Gert Doering Nov. 24, 2017, 2:28 p.m. | #1
ACK.  Patch looks good and passes t_server tests (2.2, 2.3, 2.4 clients).

(Also registering Antonios ACK, as the v2 code was technically working,
and this is just getting it fully right)

Since the patch is fairly trivial I've let myself be convinced that it's
a long-term support thing to have this in 2.4 (namely: get rid of P_DATA_V1
as soon as everyone has migrated to 2.4, not having to wait until after
2.5).

Your patch has been applied to the master and release/2.4 branch.

commit 3b9cce657b0ba876c56ee6f14664a8a77f5b82d5 (master)
commit 6232c7b3827f90d9b25135c523703204bc2095c0 (release/2.4)
Author: Steffan Karger
Date:   Fri Nov 24 14:58:23 2017 +0100

     Use P_DATA_V2 for server->client packets too

     Signed-off-by: Steffan Karger <steffan@karger.me>
     Acked-by: Antonio Quartulli <antonio@openvpn.net>
     Acked-by: Gert Doering <gert@greenie.muc.de>
     Message-Id: <1511531903-19349-1-git-send-email-steffan.karger@fox-it.com>
     URL: https://www.mail-archive.com/search?l=mid&q=1511531903-19349-1-git-send-email-steffan.karger@fox-it.com
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Patch

diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
index 1b7455b..a868a8f 100644
--- a/src/openvpn/forward.c
+++ b/src/openvpn/forward.c
@@ -496,7 +496,7 @@  encrypt_sign(struct context *c, bool comp_frag)
         /* If using P_DATA_V2, prepend the 1-byte opcode and 3-byte peer-id to the
          * packet before openvpn_encrypt(), so we can authenticate the opcode too.
          */
-        if (c->c2.buf.len > 0 && !c->c2.tls_multi->opt.server && c->c2.tls_multi->use_peer_id)
+        if (c->c2.buf.len > 0 && c->c2.tls_multi->use_peer_id)
         {
             tls_prepend_opcode_v2(c->c2.tls_multi, &b->encrypt_buf);
         }
@@ -512,7 +512,7 @@  encrypt_sign(struct context *c, bool comp_frag)
     /* Do packet administration */
     if (c->c2.tls_multi)
     {
-        if (c->c2.buf.len > 0 && (c->c2.tls_multi->opt.server || !c->c2.tls_multi->use_peer_id))
+        if (c->c2.buf.len > 0 && !c->c2.tls_multi->use_peer_id)
         {
             tls_prepend_opcode_v1(c->c2.tls_multi, &c->c2.buf);
         }
diff --git a/src/openvpn/push.c b/src/openvpn/push.c
index 5947a31..e7aecbb 100644
--- a/src/openvpn/push.c
+++ b/src/openvpn/push.c
@@ -365,6 +365,7 @@  prepare_push_reply(struct context *c, struct gc_arena *gc,
         {
             push_option_fmt(gc, push_list, M_USAGE, "peer-id %d",
                             tls_multi->peer_id);
+            tls_multi->use_peer_id = true;
         }
     }