Fix P2P epoch MTU accounting

P2P NCP stores the negotiated epoch flag in the live TLS session. Pass the active data-channel crypto flags through frame-overhead calculation so P2P uses the actual packet-ID format without writing session state back into the main options structure. Keep pre-negotiation calculations on their existing imported flags.

Add focused coverage for epoch and non-epoch P2P overhead.

Related: #1074

Signed-off-by: Darren Carreras <carrerasdarren@gmail.com>

Author: carrerasdarren-cell

CMakeLists.txt

@@ -842,6 +842,8 @@ if (BUILD_TESTING)
         src/openvpn/crypto_mbedtls.c
         src/openvpn/crypto_openssl.c
         src/openvpn/crypto.c
+        src/openvpn/mss.c
+        src/openvpn/mtu.c
         src/openvpn/otime.c
         src/openvpn/packet_id.c
         src/openvpn/ssl_util.c

src/openvpn/init.c

@@ -2856,7 +2856,8 @@ get_frame_mtu(struct context *c, const struct options *o)
         ASSERT(o->ce.link_mtu_defined);
         /* if we have a link mtu defined we calculate what the old code
          * would have come up with as tun-mtu */
-        size_t overhead = frame_calculate_protocol_header_size(&c->c1.ks.key_type, o, true);
+        size_t overhead = frame_calculate_protocol_header_size(
+            &c->c1.ks.key_type, o, o->imported_protocol_flags, true);
         mtu = o->ce.link_mtu - overhead;
     }
     else
@@ -3745,7 +3746,7 @@ do_init_fragment(struct context *c)
     c->c2.frame_fragment = c->c2.frame;
 
     frame_calculate_dynamic(&c->c2.frame_fragment, &c->c1.ks.key_type, &c->options,
-                            get_link_socket_info(c));
+                            c->options.imported_protocol_flags, get_link_socket_info(c));
     fragment_frame_init(c->c2.fragment, &c->c2.frame_fragment);
 }
 #endif
@@ -4656,6 +4657,7 @@ init_instance(struct context *c, const struct env_set *env, const unsigned int f
         for (int i = 0; i < c->c1.link_sockets_num; i++)
         {
             frame_calculate_dynamic(&c->c2.frame, &c->c1.ks.key_type, &c->options,
+                                    c->options.imported_protocol_flags,
                                     &c->c2.link_sockets[i]->info);
         }
     }

src/openvpn/mss.c

@@ -240,12 +240,12 @@ get_ip_encap_overhead(const struct options *options, const struct link_socket_in
 
 static void
 frame_calculate_fragment(struct frame *frame, struct key_type *kt, const struct options *options,
-                         struct link_socket_info *lsi)
+                         unsigned int crypto_flags, struct link_socket_info *lsi)
 {
 #if defined(ENABLE_FRAGMENT)
     size_t overhead;
 
-    overhead = frame_calculate_protocol_header_size(kt, options, false);
+    overhead = frame_calculate_protocol_header_size(kt, options, crypto_flags, false);
 
     if (options->ce.fragment_encap)
     {
@@ -263,14 +263,14 @@ frame_calculate_fragment(struct frame *frame, struct key_type *kt, const struct
     {
         /* The packet id gets added to *each* fragment in CBC mode, so we need
          * to account for it */
-        frame->max_fragment_size -= calc_packet_id_size_dc(options, kt);
+        frame->max_fragment_size -= calc_packet_id_size_dc(options, kt, crypto_flags);
     }
 #endif
 }
 
 static void
 frame_calculate_mssfix(struct frame *frame, struct key_type *kt, const struct options *options,
-                       struct link_socket_info *lsi)
+                       unsigned int crypto_flags, struct link_socket_info *lsi)
 {
     if (options->ce.mssfix_fixed)
     {
@@ -282,11 +282,12 @@ frame_calculate_mssfix(struct frame *frame, struct key_type *kt, const struct op
 
     size_t overhead, payload_overhead;
 
-    overhead = frame_calculate_protocol_header_size(kt, options, false);
+    overhead = frame_calculate_protocol_header_size(kt, options, crypto_flags, false);
 
     /* Calculate the number of bytes that the payload differs from the payload
      * MTU. This are fragment/compression/ethernet headers */
-    payload_overhead = frame_calculate_payload_overhead(frame->extra_tun, options, kt);
+    payload_overhead =
+        frame_calculate_payload_overhead(frame->extra_tun, options, kt, crypto_flags);
 
     /* We are in a "liberal" position with respect to MSS,
      * i.e. we assume that MSS can be calculated from MTU
@@ -315,16 +316,16 @@ frame_calculate_mssfix(struct frame *frame, struct key_type *kt, const struct op
 
 void
 frame_calculate_dynamic(struct frame *frame, struct key_type *kt, const struct options *options,
-                        struct link_socket_info *lsi)
+                        unsigned int crypto_flags, struct link_socket_info *lsi)
 {
     if (options->ce.fragment > 0)
     {
-        frame_calculate_fragment(frame, kt, options, lsi);
+        frame_calculate_fragment(frame, kt, options, crypto_flags, lsi);
     }
 
     if (options->ce.mssfix > 0)
     {
-        frame_calculate_mssfix(frame, kt, options, lsi);
+        frame_calculate_mssfix(frame, kt, options, crypto_flags, lsi);
     }
 }
 
@@ -354,7 +355,8 @@ frame_adjust_path_mtu(struct context *c)
             o->ce.mssfix, mtustr, pmtu);
         o->ce.mssfix = pmtu;
         o->ce.mssfix_encap = true;
-        frame_calculate_dynamic(&c->c2.frame, &c->c1.ks.key_type, o, lsi);
+        frame_calculate_dynamic(&c->c2.frame, &c->c1.ks.key_type, o,
+                                c->c2.crypto_options.flags, lsi);
     }
 
 #if defined(ENABLE_FRAGMENT)
@@ -367,7 +369,8 @@ frame_adjust_path_mtu(struct context *c)
             o->ce.fragment, mtustr, pmtu);
         o->ce.fragment = pmtu;
         o->ce.fragment_encap = true;
-        frame_calculate_dynamic(&c->c2.frame_fragment, &c->c1.ks.key_type, o, lsi);
+        frame_calculate_dynamic(&c->c2.frame_fragment, &c->c1.ks.key_type, o,
+                                c->c2.crypto_options.flags, lsi);
     }
 #endif
 }

src/openvpn/mss.h

@@ -35,9 +35,12 @@ void mss_fixup_ipv6(struct buffer *buf, uint16_t maxmss);
 
 void mss_fixup_dowork(struct buffer *buf, uint16_t maxmss);
 
-/** Set the --mssfix option. */
+/**
+ * Recalculate dynamic frame parameters using the active data-channel flags.
+ */
 void frame_calculate_dynamic(struct frame *frame, struct key_type *kt,
-                             const struct options *options, struct link_socket_info *lsi);
+                             const struct options *options, unsigned int crypto_flags,
+                             struct link_socket_info *lsi);
 
 /**
  * Checks and adjusts the fragment and mssfix value according to the

src/openvpn/mtu.c

@@ -49,10 +49,11 @@ alloc_buf_sock_tun(struct buffer *buf, const struct frame *frame)
 }
 
 unsigned int
-calc_packet_id_size_dc(const struct options *options, const struct key_type *kt)
+calc_packet_id_size_dc(const struct options *options, const struct key_type *kt,
+                       unsigned int crypto_flags)
 {
     bool tlsmode = options->tls_server || options->tls_client;
-    bool epoch = options->imported_protocol_flags & CO_EPOCH_DATA_KEY_FORMAT;
+    bool epoch = crypto_flags & CO_EPOCH_DATA_KEY_FORMAT;
 
     /* epoch format uses a 64-bit packet id: 16 bit epoch + 48 bit per-epoch counter */
     if (epoch)
@@ -67,7 +68,7 @@ calc_packet_id_size_dc(const struct options *options, const struct key_type *kt)
 
 size_t
 frame_calculate_protocol_header_size(const struct key_type *kt, const struct options *options,
-                                     bool occ)
+                                     unsigned int crypto_flags, bool occ)
 {
     /* Sum of all the overhead that reduces the usable packet size */
     size_t header_size = 0;
@@ -93,7 +94,7 @@ frame_calculate_protocol_header_size(const struct key_type *kt, const struct opt
         header_size += options->use_peer_id ? 4 : 1;
     }
 
-    unsigned int pkt_id_size = calc_packet_id_size_dc(options, kt);
+    unsigned int pkt_id_size = calc_packet_id_size_dc(options, kt, crypto_flags);
 
     /* For figuring out the crypto overhead, we need the size of the payload
      * including all headers that also get encrypted as part of the payload */
@@ -104,7 +105,7 @@ frame_calculate_protocol_header_size(const struct key_type *kt, const struct opt
 
 size_t
 frame_calculate_payload_overhead(size_t extra_tun, const struct options *options,
-                                 const struct key_type *kt)
+                                 const struct key_type *kt, unsigned int crypto_flags)
 {
     size_t overhead = 0;
 
@@ -136,7 +137,7 @@ frame_calculate_payload_overhead(size_t extra_tun, const struct options *options
         /* The packet id is part of the plain text payload instead of the
          * cleartext protocol header and needs to be included in the payload
          * overhead instead of the protocol header */
-        overhead += calc_packet_id_size_dc(options, kt);
+        overhead += calc_packet_id_size_dc(options, kt, crypto_flags);
     }
 
     return overhead;
@@ -147,7 +148,8 @@ frame_calculate_payload_size(const struct frame *frame, const struct options *op
                              const struct key_type *kt)
 {
     size_t payload_size = options->ce.tun_mtu;
-    payload_size += frame_calculate_payload_overhead(frame->extra_tun, options, kt);
+    payload_size += frame_calculate_payload_overhead(frame->extra_tun, options, kt,
+                                                     options->imported_protocol_flags);
     return payload_size;
 }
 
@@ -189,7 +191,7 @@ calc_options_string_link_mtu(const struct options *o, const struct frame *frame)
     init_key_type(&occ_kt, ciphername, o->authname, true, false);
 
     size_t payload = frame_calculate_payload_size(frame, o, &occ_kt);
-    overhead += frame_calculate_protocol_header_size(&occ_kt, o, true);
+    overhead += frame_calculate_protocol_header_size(&occ_kt, o, o->imported_protocol_flags, true);
 
     return payload + overhead;
 }

src/openvpn/mtu.h

@@ -229,7 +229,7 @@ size_t frame_calculate_payload_size(const struct frame *frame, const struct opti
  * *  [IP][UDP][OPENVPN PROTOCOL HEADER][ **PAYLOAD incl compression header** ]
  */
 size_t frame_calculate_payload_overhead(size_t extra_tun, const struct options *options,
-                                        const struct key_type *kt);
+                                        const struct key_type *kt, unsigned int crypto_flags);
 
 
 /**
@@ -244,11 +244,13 @@ size_t frame_calculate_payload_overhead(size_t extra_tun, const struct options *
  *
  * @param kt            the key_type to use to calculate the crypto overhead
  * @param options       the options struct to be used to calculate
+ * @param crypto_flags  the active data-channel crypto flags
  * @param occ           Use the calculation for the OCC link-mtu
  * @return              size of the overhead in bytes
  */
 size_t frame_calculate_protocol_header_size(const struct key_type *kt,
-                                            const struct options *options, bool occ);
+                                            const struct options *options,
+                                            unsigned int crypto_flags, bool occ);
 
 /**
  * Calculate the link-mtu to advertise to our peer.  The actual value is not
@@ -260,10 +262,11 @@ size_t frame_calculate_protocol_header_size(const struct key_type *kt,
 size_t calc_options_string_link_mtu(const struct options *options, const struct frame *frame);
 
 /**
- * Return the size of the packet ID size that is currently in use by cipher and
- * options for the data channel.
+ * Return the packet ID size currently in use by the cipher, options, and active
+ * data-channel crypto flags.
  */
-unsigned int calc_packet_id_size_dc(const struct options *options, const struct key_type *kt);
+unsigned int calc_packet_id_size_dc(const struct options *options, const struct key_type *kt,
+                                    unsigned int crypto_flags);
 
 /*
  * allocate a buffer for socket or tun layer

src/openvpn/occ.c

@@ -198,8 +198,8 @@ check_send_occ_load_test_dowork(struct context *c)
             c->c2.occ_op = entry->op;
             size_t payload_size =
                 frame_calculate_payload_size(&c->c2.frame, &c->options, &c->c1.ks.key_type);
-            size_t header_size =
-                frame_calculate_protocol_header_size(&c->c1.ks.key_type, &c->options, false);
+            size_t header_size = frame_calculate_protocol_header_size(
+                &c->c1.ks.key_type, &c->options, c->c2.crypto_options.flags, false);
 
             c->c2.occ_mtu_load_size = payload_size + header_size;
         }
@@ -303,10 +303,12 @@ check_send_occ_msg_dowork(struct context *c)
             const struct key_type *kt = &c->c1.ks.key_type;
 
             /* OCC message have comp/fragment headers but not ethernet headers */
-            payload_hdr = frame_calculate_payload_overhead(0, &c->options, kt);
+            payload_hdr =
+                frame_calculate_payload_overhead(0, &c->options, kt, c->c2.crypto_options.flags);
 
             /* Since we do not know the payload size we just pass 0 as size here */
-            proto_hdr = frame_calculate_protocol_header_size(kt, &c->options, false);
+            proto_hdr = frame_calculate_protocol_header_size(
+                kt, &c->options, c->c2.crypto_options.flags, false);
 
             need_to_add = min_int(c->c2.occ_mtu_load_size, c->c2.frame.buf.payload_size)
                           - OCC_STRING_SIZE - sizeof(uint8_t) /* occ opcode */

src/openvpn/ssl.c

@@ -1592,7 +1592,8 @@ tls_session_update_crypto_params_do_work(struct tls_multi *multi, struct tls_ses
         session->opt->crypto_flags |= CO_PACKET_ID_LONG_FORM;
     }
 
-    frame_calculate_dynamic(frame, &session->opt->key_type, options, lsi);
+    frame_calculate_dynamic(frame, &session->opt->key_type, options, session->opt->crypto_flags,
+                            lsi);
 
     frame_print(frame, D_MTU_INFO, "Data Channel MTU parms");
 
@@ -1605,7 +1606,8 @@ tls_session_update_crypto_params_do_work(struct tls_multi *multi, struct tls_ses
 
     if (frame_fragment)
     {
-        frame_calculate_dynamic(frame_fragment, &session->opt->key_type, options, lsi);
+        frame_calculate_dynamic(frame_fragment, &session->opt->key_type, options,
+                                session->opt->crypto_flags, lsi);
         frame_print(frame_fragment, D_MTU_INFO, "Fragmentation MTU parms");
     }
 

tests/unit_tests/openvpn/Makefile.am

@@ -341,6 +341,8 @@ ncp_testdriver_SOURCES = test_ncp.c \
 	$(top_srcdir)/src/openvpn/crypto_mbedtls.c \
 	$(top_srcdir)/src/openvpn/crypto_mbedtls_legacy.c \
 	$(top_srcdir)/src/openvpn/crypto_openssl.c \
+	$(top_srcdir)/src/openvpn/mss.c \
+	$(top_srcdir)/src/openvpn/mtu.c \
 	$(top_srcdir)/src/openvpn/otime.c \
 	$(top_srcdir)/src/openvpn/packet_id.c \
 	$(top_srcdir)/src/openvpn/platform.c \

tests/unit_tests/openvpn/test_crypto.c

@@ -363,7 +363,7 @@ test_mssfix_mtu_calculation(void **state)
     init_key_type(&kt, o.ciphername, o.authname, false, false);
 
     /* No encryption, just packet id (8) + TCP payload(20) + IP payload(20) */
-    frame_calculate_dynamic(&f, &kt, &o, NULL);
+    frame_calculate_dynamic(&f, &kt, &o, o.imported_protocol_flags, NULL);
     assert_int_equal(f.mss_fix, 952);
 
     /* Static key OCC examples */
@@ -373,7 +373,7 @@ test_mssfix_mtu_calculation(void **state)
     o.ciphername = "none";
     o.authname = "none";
     init_key_type(&kt, o.ciphername, o.authname, false, false);
-    frame_calculate_dynamic(&f, &kt, &o, NULL);
+    frame_calculate_dynamic(&f, &kt, &o, o.imported_protocol_flags, NULL);
     assert_int_equal(f.mss_fix, 952);
 
     /* secret, cipher AES-128-CBC, auth none */
@@ -387,7 +387,7 @@ test_mssfix_mtu_calculation(void **state)
          * all result in the same CBC block size/padding and <= 991 and >=1008
          * should be one block less and more respectively */
         o.ce.mssfix = i;
-        frame_calculate_dynamic(&f, &kt, &o, NULL);
+        frame_calculate_dynamic(&f, &kt, &o, o.imported_protocol_flags, NULL);
         if (i <= 991)
         {
             assert_int_equal(f.mss_fix, 911);
@@ -413,7 +413,7 @@ test_mssfix_mtu_calculation(void **state)
          * all result in the same CBC block size/padding and <= 991 and >=1008
          * should be one block less and more respectively */
         o.ce.mssfix = i;
-        frame_calculate_dynamic(&f, &kt, &o, NULL);
+        frame_calculate_dynamic(&f, &kt, &o, o.imported_protocol_flags, NULL);
         if (i <= 991)
         {
             assert_int_equal(f.mss_fix, 910);
@@ -443,7 +443,7 @@ test_mssfix_mtu_calculation(void **state)
         /* For stream ciphers, the value should not be influenced by block
          * sizes or similar but always have the same difference */
         o.ce.mssfix = i;
-        frame_calculate_dynamic(&f, &kt, &o, NULL);
+        frame_calculate_dynamic(&f, &kt, &o, o.imported_protocol_flags, NULL);
 
         /* 4 byte opcode/peerid, 4 byte pkt ID, 16 byte tag, 40 TCP+IP */
         assert_int_equal(f.mss_fix, i - 4 - 4 - 16 - 40);