Fix frame size calculation for non-CBC modes.

CBC mode is the only mode that OpenVPN supports that needs padding. So, only include the worst case padding size in the frame size calculation when using CBC mode.

While doing so, rewrite crypto_adjust_frame_parameters() to be better readable, and provide debug output (for high debug levels).

---
 src/openvpn/crypto.c | 26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

index ef2bde1..2ba8487 100644
@@ -403,11 +403,27 @@ crypto_adjust_frame_parameters(struct frame *frame,
 			       bool packet_id,
 			       bool packet_id_long_form)
 {
-  frame_add_to_extra_frame (frame,
-			    (packet_id ? packet_id_size (packet_id_long_form) : 0) +
-			    ((cipher_defined && use_iv) ? cipher_kt_iv_size (kt->cipher) : 0) +
-			    (cipher_defined ? cipher_kt_block_size (kt->cipher) : 0) + /* worst case padding expansion */
-			    kt->hmac_length);
+  size_t crypto_overhead = 0;
+
+  if (packet_id)
+    crypto_overhead += packet_id_size (packet_id_long_form);
+
+  if (cipher_defined)
+    {
+      if (use_iv)
+	crypto_overhead += cipher_kt_iv_size (kt->cipher);
+
+      if (cipher_kt_mode_cbc (kt->cipher))
+	/* worst case padding expansion */
+	crypto_overhead += cipher_kt_block_size (kt->cipher);
+    }
+
+  crypto_overhead += kt->hmac_length;
+
+  frame_add_to_extra_frame (frame, crypto_overhead);
+
+  msg(D_MTU_DEBUG, "%s: Adjusting frame parameters for crypto by %zu bytes",
+      __func__, crypto_overhead);
 }
 
 /*

1.9.1

Hi,

Is this a bugfix or performance/packet size optimization? I assume "master only"?

gert

USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             [email protected]...
fax: +49-89-35655025                        [email protected]...

Hi,

More a bug than a packet size optimization, but neither really. It's just something that was not correct.

Too-large frame parameters don't really seem to have an influence on the actual connection. At least, for as far as I grasp the networking code... (Please correct me if I'm wrong.) The calculated values are reported to the user and peer. By default the tun-mtu is set to 1500, and the link-mtu to 1500 + calculated overhead. If that calculated overhead is too large, too large buffers are allocated, but the packets actually sent are still limited by their real payload+overhead. If peers don't agree on their link-mtu's they will issue a warning, but - as long as the buffers are large enough - will still happily connect. Since OFB/CFB has been fixed (very recently) in both master and release/2.3, I'd say this fix should go into both.

-Steffan

ACK, after discussion on #openvpn-devel. For CBC (which are the default modes so far) this is only refactoring, so no compatibility issues are to be expected.

Your patch has been applied to the master and release/2.3 branches.

commit 669f898b8fcaf7a8d43825fa0255c2791cc0ef89 (master) commit 6f0ab30d7f034d4f8d7c2ca872cfef066b16c7f0 (release/2.3)

Author: Steffan Karger
Date:   Tue Jul 29 22:52:24 2014 +0200

     Fix frame size calculation for non-CBC modes.

     Signed-off-by: Steffan Karger <[email protected]...>
     Acked-by: Gert Doering <[email protected]...>
     Message-Id: <[email protected]...>
     URL: http://article.gmane.org/gmane.network.openvpn.devel/8952
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering