Acked-by: Gert Doering <gert@greenie.muc.de>
Change makes sense, to take IPv4/IPv6 encap into account.
Code looks reasonable.
Tested with my previous mssfix test setup (and looking very carefully
at tcpdump-reported "length" values, which one is "packet" and which
one is "UDP encap" length :-) - that got me confused last time...).
- BF-CBC, LZ4 (conf), --mssfix 1000 mtu, over IPv4
v4 TCP -> MSS 890, resulting UDP payload 968, IP size 996 bytes
v6 TCP -> MSS 870, resulting UDP payload 968, IP size 996 bytes
(= OK, CBC rounding on 8 byte boundaries)
- BF-CBC, LZ4 (conf), --mssfix 1000 mtu, over IPv6
v4 TCP -> MSS 874, resulting UDP payload 952, IP size 1000 bytes
v6 TCP -> MSS 854, resulting UDP payload 952, IP size 1000 bytes
- AES256GCM, LZ4, --mssfix 1000 mtu, over IPv4
v4 TCP -> MSS 907, resulting UDP payload 972, IP size 1000 bytes
v6 TCP -> MSS 887, resulting UDP payload 972, IP size 1000 bytes
- AES256GCM, LZ4, --mssfix 1000 mtu, over IPv6
v4 TCP -> MSS 887, resulting UDP payload 952, IP size 1000 bytes
v6 TCP -> MSS 867, resulting UDP payload 952, IP size 1000 bytes
"just for completeness" I've re-tested "without 'mtu'" for a few cases
- BF-CBC, LZ4 (conf), --mssfix 1000 mtu, over IPv4
v4 TCP -> MSS 922, resulting UDP payload 1000, IP size 1028 bytes
v6 TCP -> MSS 902, resulting UDP payload 1000, IP size 1028 bytes
- AES256GCM, LZ4, --mssfix 1000, over IPv4
v4 TCP -> MSS 935, resulting UDP payload 1000, IP size 1028 bytes
v6 TCP -> MSS 915, resulting UDP payload 1000, IP size 1028 bytes
- AES256GCM, LZ4, --mssfix 1000, over IPv6
v4 TCP -> MSS 935, resulting UDP payload 1000, IP size 1048 bytes
v6 TCP -> MSS 915, resulting UDP payload 1000, IP size 1048 bytes
(this is "as before", so the previously documented behaviour is not
broken)
The "if (p[2]..." part of options.c is not perfect (it will check this
even if p[1] was already found to be NULL) but given the calling
environment, it's well defined as all of p[] is CLEAR()ed on every
line.
As discussed on IRC, I have adjusted the man page entry somewhat
(1473 was just plain wrong, 1478 is correct math, but I found the
"pass IPv4 packets" text still misleading wrt inside/outside IPv4)
For reference, this was 13/21 in the v1 and v2 series, got feedback
from Frank Lichtenheld. I have adjusted the commit message according
to this (make the sentence a bit clearer). The RST formatting comments
have been addressed in v2.
Your patch has been applied to the master branch.
commit 0fcb7cadb225d7f43e29c3ad6a1e9c34abbb8a63
Author: Arne Schwabe
Date: Sat Jan 1 17:25:23 2022 +0100
Implement optional mtu parameter for mssfix
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20220101162532.2251835-6-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg23495.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
--
kind regards,
Gert Doering
@@ -62,6 +62,12 @@ Optional ciphers in ``--data-ciphers``
Ciphers in ``--data-ciphers`` can now be prefixed with a ``?`` to mark
those as optional and only use them if the SSL library supports them.
+
+Improved ``--mssfix`` calculation
+ The ``--mssfix`` option now allows an optional :code:`mtu` parameter to specify
+ that different overhead for IPv4/IPv6 should taken into account and the resulting
+ size is specified as the total size of the VPN packets including IP and UDP headers.
+
Deprecated features
-------------------
``inetd`` has been removed
@@ -110,19 +110,37 @@ the local and the remote host.
(:code:`p2p`). OpenVPN 2.0 introduces a new mode (:code:`server`) which
implements a multi-client server capability.
---mssfix max
+--mssfix args
+
+ Valid syntax:
+ ::
+
+ mssfix max [mtu]
+
+ mssfix
+
Announce to TCP sessions running over the tunnel that they should limit
their send packet sizes such that after OpenVPN has encapsulated them,
the resulting UDP packet size that OpenVPN sends to its peer will not
- exceed ``max`` bytes. The default value is :code:`1450`.
-
- The ``max`` parameter is interpreted in the same way as the
- ``--link-mtu`` parameter, i.e. the UDP packet size after encapsulation
- overhead has been added in, but not including the UDP header itself.
- Resulting packet would be at most 28 bytes larger for IPv4 and 48 bytes
- for IPv6 (20/40 bytes for IP header and 8 bytes for UDP header). Default
- value of 1450 allows IPv4 packets to be transmitted over a link with MTU
- 1473 or higher without IP level fragmentation.
+ exceed ``max`` bytes. The default value is :code:`1450`. Use :code:`0`
+ as max to disable mssfix.
+
+ If the :code:`mtu` parameter is specified the ``max`` value is interpreted
+ as the resulting packet size of VPN packets including the IP and UDP header.
+ Support for the :code:`mtu` parameter was added with OpenVPN version 2.6.0.
+
+ If the :code:`mtu` parameter is not specified, the ``max`` parameter
+ is interpreted in the same way as the ``--link-mtu`` parameter, i.e.
+ the UDP packet size after encapsulation overhead has been added in, but
+ not including the UDP header itself. Resulting packet would be at most 28
+ bytes larger for IPv4 and 48 bytes for IPv6 (20/40 bytes for IP header and
+ 8 bytes for UDP header). Default value of 1450 allows IPv4 packets to be
+ transmitted over a link with MTU 1473 or higher without IP level
+ fragmentation.
+
+ if ``--mssfix`` is specified is specified without any parameter it
+ inherits the parameters of ``--fragment`` if specified or uses the
+ default for ``--mssfix`` otherwise.
The ``--mssfix`` option only makes sense when you are using the UDP
protocol for OpenVPN peer-to-peer communication, i.e. ``--proto udp``.
@@ -2271,7 +2271,7 @@ do_deferred_p2p_ncp(struct context *c)
#endif
if (!tls_session_update_crypto_params(session, &c->options, &c->c2.frame,
- frame_fragment))
+ frame_fragment, get_link_socket_info(c)))
{
msg(D_TLS_ERRORS, "ERROR: failed to set crypto cipher");
return false;
@@ -2386,7 +2386,7 @@ do_deferred_options(struct context *c, const unsigned int found)
struct tls_session *session = &c->c2.tls_multi->session[TM_ACTIVE];
if (!tls_session_update_crypto_params(session, &c->options, &c->c2.frame,
- frame_fragment))
+ frame_fragment, get_link_socket_info(c)))
{
msg(D_TLS_ERRORS, "OPTIONS ERROR: failed to import crypto options");
return false;
@@ -4302,7 +4302,8 @@ init_instance(struct context *c, const struct env_set *env, const unsigned int f
#endif
/* initialize dynamic MTU variable */
- frame_calculate_mssfix(&c->c2.frame, &c->c1.ks.key_type, &c->options);
+ frame_calculate_mssfix(&c->c2.frame, &c->c1.ks.key_type, &c->options,
+ get_link_socket_info(c));
/* bind the TCP/UDP socket */
if (c->mode == CM_P2P || c->mode == CM_TOP || c->mode == CM_CHILD_TCP)
@@ -227,9 +227,30 @@ adjust_payload_max_cbc(const struct key_type *kt, unsigned int target)
}
}
+static unsigned int
+get_ip_encap_overhead(const struct options *options,
+ const struct link_socket_info *lsi)
+{
+ /* Add the overhead of the encapsulating IP packets */
+ sa_family_t af;
+ if (lsi->lsa)
+ {
+ af = lsi->lsa->actual.dest.addr.sa.sa_family;
+ }
+ else
+ {
+ /* In the early init before the connection is established or we
+ * are in listen mode we can only make an educated guess
+ * from the af of the connection entry */
+ af = options->ce.af;
+ }
+ return datagram_overhead(af, lsi->proto);
+}
+
void
frame_calculate_mssfix(struct frame *frame, struct key_type *kt,
- const struct options *options)
+ const struct options *options,
+ struct link_socket_info *lsi)
{
if (options->ce.mssfix == 0)
{
@@ -251,6 +272,12 @@ frame_calculate_mssfix(struct frame *frame, struct key_type *kt,
*
* (RFC 879, section 7). */
+ if (options->ce.mssfix_encap)
+ {
+ /* Add the overhead of the encapsulating IP packets */
+ overhead += get_ip_encap_overhead(options, lsi);
+ }
+
/* Add 20 bytes for the IPv4 header and 20 byte for the TCP header of the
* payload, the mssfix method will add 20 extra if payload is IPv6 */
payload_overhead += 20 + 20;
@@ -37,6 +37,7 @@ void mss_fixup_dowork(struct buffer *buf, uint16_t maxmss);
/** Set the --mssfix option. */
void frame_calculate_mssfix(struct frame *frame, struct key_type *kt,
- const struct options *options);
+ const struct options *options,
+ struct link_socket_info *lsi);
#endif
@@ -2286,7 +2286,8 @@ multi_client_generate_tls_keys(struct context *c)
#endif
struct tls_session *session = &c->c2.tls_multi->session[TM_ACTIVE];
if (!tls_session_update_crypto_params(session, &c->options,
- &c->c2.frame, frame_fragment))
+ &c->c2.frame, frame_fragment,
+ get_link_socket_info(c)))
{
msg(D_TLS_ERRORS, "TLS Error: initializing data channel failed");
register_signal(c, SIGUSR1, "process-push-msg-failed");
@@ -6771,18 +6771,27 @@ add_option(struct options *options,
VERIFY_PERMISSION(OPT_P_GENERAL);
script_security_set(atoi(p[1]));
}
- else if (streq(p[0], "mssfix") && !p[2])
+ else if (streq(p[0], "mssfix") && !p[3])
{
VERIFY_PERMISSION(OPT_P_GENERAL|OPT_P_CONNECTION);
if (p[1])
{
options->ce.mssfix = positive_atoi(p[1]);
}
- else
+
+ if (!p[1])
{
options->ce.mssfix_default = true;
}
+ if (p[2] && streq(p[2], "mtu"))
+ {
+ options->ce.mssfix_encap = true;
+ }
+ else if (p[2])
+ {
+ msg(msglevel, "Unknown parameter to --mssfix: %s", p[2]);
+ }
}
else if (streq(p[0], "disable-occ") && !p[1])
{
@@ -127,6 +127,8 @@ struct connection_entry
int fragment; /* internal fragmentation size */
int mssfix; /* Upper bound on TCP MSS */
bool mssfix_default; /* true if --mssfix was supplied without a parameter */
+ bool mssfix_encap; /* true if --mssfix had the "mtu" parameter to include
+ * overhead from IP and TCP/UDP encapsulation */
int explicit_exit_notification; /* Explicitly tell peer when we are exiting via OCC_EXIT or [RESTART] message */
@@ -1883,7 +1883,8 @@ cleanup:
bool
tls_session_update_crypto_params_do_work(struct tls_session *session,
struct options* options, struct frame *frame,
- struct frame *frame_fragment)
+ struct frame *frame_fragment,
+ struct link_socket_info *lsi)
{
if (session->key[KS_PRIMARY].crypto_options.key_ctx_bi.initialized)
{
@@ -1913,7 +1914,7 @@ tls_session_update_crypto_params_do_work(struct tls_session *session,
options->replay, packet_id_long_form);
frame_finalize(frame, options->ce.link_mtu_defined, options->ce.link_mtu,
options->ce.tun_mtu_defined, options->ce.tun_mtu);
- frame_calculate_mssfix(frame, &session->opt->key_type, options);
+ frame_calculate_mssfix(frame, &session->opt->key_type, options, lsi);
frame_print(frame, D_MTU_INFO, "Data Channel MTU parms");
/*
@@ -1938,7 +1939,8 @@ tls_session_update_crypto_params_do_work(struct tls_session *session,
bool
tls_session_update_crypto_params(struct tls_session *session,
struct options *options, struct frame *frame,
- struct frame *frame_fragment)
+ struct frame *frame_fragment,
+ struct link_socket_info *lsi)
{
bool cipher_allowed_as_fallback = options->enable_ncp_fallback
@@ -1957,7 +1959,8 @@ tls_session_update_crypto_params(struct tls_session *session,
/* Import crypto settings that might be set by pull/push */
session->opt->crypto_flags |= options->data_channel_crypto_flags;
- return tls_session_update_crypto_params_do_work(session, options, frame, frame_fragment);
+ return tls_session_update_crypto_params_do_work(session, options, frame,
+ frame_fragment, lsi);
}
@@ -508,13 +508,16 @@ void tls_update_remote_addr(struct tls_multi *multi,
* @param frame The frame options for this session (frame overhead is
* adjusted based on the selected cipher/auth).
* @param frame_fragment The fragment frame options.
+ * @param lsi link socket info to adjust MTU related options
+ * depending on the current protocol
*
* @return true if updating succeeded or keys are already generated, false otherwise.
*/
bool tls_session_update_crypto_params(struct tls_session *session,
struct options *options,
struct frame *frame,
- struct frame *frame_fragment);
+ struct frame *frame_fragment,
+ struct link_socket_info *lsi);
/*
* inline functions
@@ -382,7 +382,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_mssfix(&f, &kt, &o);
+ frame_calculate_mssfix(&f, &kt, &o, NULL);
assert_int_equal(f.mss_fix, 952);
/* Static key OCC examples */
@@ -392,7 +392,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_mssfix(&f, &kt, &o);
+ frame_calculate_mssfix(&f, &kt, &o, NULL);
assert_int_equal(f.mss_fix, 952);
/* secret, cipher AES-128-CBC, auth none */
@@ -406,7 +406,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_mssfix(&f, &kt, &o);
+ frame_calculate_mssfix(&f, &kt, &o, NULL);
if (i <= 991)
{
assert_int_equal(f.mss_fix, 911);
@@ -434,7 +434,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_mssfix(&f, &kt, &o);
+ frame_calculate_mssfix(&f, &kt, &o, 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);