From patchwork Thu Oct 20 10:05:46 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arne Schwabe X-Patchwork-Id: 2826 Return-Path: Delivered-To: patchwork@openvpn.net Delivered-To: patchwork@openvpn.net Received: from director14.mail.ord1d.rsapps.net ([172.27.255.1]) by backend30.mail.ord1d.rsapps.net with LMTP id EOM0AKodUWPUYQAAIUCqbw (envelope-from ) for ; Thu, 20 Oct 2022 06:06:34 -0400 Received: from proxy4.mail.iad3a.rsapps.net ([172.27.255.1]) by director14.mail.ord1d.rsapps.net with LMTP id AO4CAKodUWNqdgAAeJ7fFg (envelope-from ) for ; Thu, 20 Oct 2022 06:06:34 -0400 Received: from smtp51.gate.iad3a ([172.27.255.1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by proxy4.mail.iad3a.rsapps.net with LMTPS id OCw4NqkdUWM4VgAA8Zvu4w (envelope-from ) for ; Thu, 20 Oct 2022 06:06:33 -0400 X-Spam-Threshold: 95 X-Spam-Score: 0 X-Spam-Flag: NO X-Virus-Scanned: OK X-Orig-To: openvpnslackdevel@openvpn.net X-Originating-Ip: [216.105.38.7] Authentication-Results: smtp51.gate.iad3a.rsapps.net; iprev=pass policy.iprev="216.105.38.7"; spf=pass smtp.mailfrom="openvpn-devel-bounces@lists.sourceforge.net" smtp.helo="lists.sourceforge.net"; dkim=fail (signature verification failed) header.d=sourceforge.net; dkim=fail (signature verification failed) header.d=sf.net; dmarc=none (p=nil; dis=none) header.from=rfc2549.org X-Suspicious-Flag: YES X-Classification-ID: df33f680-505e-11ed-90d9-525400aaff7b-1-1 Received: from [216.105.38.7] ([216.105.38.7:60242] helo=lists.sourceforge.net) by smtp51.gate.iad3a.rsapps.net (envelope-from ) (ecelerity 4.2.38.62370 r(:)) with ESMTPS (cipher=DHE-RSA-AES256-GCM-SHA384) id D5/95-14745-8AD11536; Thu, 20 Oct 2022 06:06:33 -0400 Received: from [127.0.0.1] (helo=sfs-ml-2.v29.lw.sourceforge.com) by sfs-ml-2.v29.lw.sourceforge.com with esmtp (Exim 4.95) (envelope-from ) id 1olSR6-0005fm-CA; Thu, 20 Oct 2022 10:06:00 +0000 Received: from [172.30.20.202] (helo=mx.sourceforge.net) by sfs-ml-2.v29.lw.sourceforge.com with esmtps (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1olSR4-0005ff-Ne for openvpn-devel@lists.sourceforge.net; Thu, 20 Oct 2022 10:05:58 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=Content-Transfer-Encoding:MIME-Version:References: In-Reply-To:Message-Id:Date:Subject:To:From:Sender:Reply-To:Cc:Content-Type: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=qvIGibjKTwMvD5nKlaO6IOGmU45YCfRqYxAd7hUjrc8=; b=S2IoWmHGkhfA5q9M4YLHyxmxj3 lEfBijpTqh8f/GxSAR8Zy4/66Vl8tviwuQhnzz0Rh3jhDZ6Om/rxRORnecNGSlKrcIl3dsiXyPJkC 2GUJNjjRkm97ICqdCIsHFPoy2uOs8S3YXWtJj7zMHb+hxjG1zXT5P/jkS+/Q0u9Uwu2s=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To:Message-Id: Date:Subject:To:From:Sender:Reply-To:Cc:Content-Type:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=qvIGibjKTwMvD5nKlaO6IOGmU45YCfRqYxAd7hUjrc8=; b=Qnryxk5RkbIqHseEi5JbaiU/0Z R/7gMB4qneFE7ZSi9gA24y1pOJ0HBPMvzUG8JoHnoGAP7yRaR2fGFbJMwazA2WEPd3I14MT7iXNSH H1S/PFfeSyBdVwG0l71NRPu1Dl+08RDLgNluFIi+WPybNOKeX/ETlLkCd6L73vNnlSds=; Received: from mail.blinkt.de ([192.26.174.232]) by sfi-mx-2.v28.lw.sourceforge.com with esmtps (TLS1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.95) id 1olSQz-0003Gs-Gd for openvpn-devel@lists.sourceforge.net; Thu, 20 Oct 2022 10:05:58 +0000 Received: from kamera.blinkt.de ([2001:638:502:390:20c:29ff:fec8:535c]) by mail.blinkt.de with smtp (Exim 4.95 (FreeBSD)) (envelope-from ) id 1olSQs-000MwU-2t for openvpn-devel@lists.sourceforge.net; Thu, 20 Oct 2022 12:05:46 +0200 Received: (nullmailer pid 3023073 invoked by uid 10006); Thu, 20 Oct 2022 10:05:46 -0000 From: Arne Schwabe To: openvpn-devel@lists.sourceforge.net Date: Thu, 20 Oct 2022 12:05:46 +0200 Message-Id: <20221020100546.3023025-1-arne@rfc2549.org> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20220921104930.3452270-1-arne@rfc2549.org> References: <20220921104930.3452270-1-arne@rfc2549.org> MIME-Version: 1.0 X-Spam-Report: Spam detection software, running on the system "util-spamd-1.v13.lw.sourceforge.com", has NOT identified this incoming email as spam. The original message has been attached to this so you can view it or label similar future email. If you have any questions, see the administrator of that system for details. Content preview: Currently control packet size is controlled by tun-mtu in a very non-obvious way since the control overhead is not taken into account and control channel packet will end up with a different size than [...] Content analysis details: (0.3 points, 6.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record 0.2 HEADER_FROM_DIFFERENT_DOMAINS From and EnvelopeFrom 2nd level mail domains are different 0.0 SPF_NONE SPF: sender does not publish an SPF Record X-Headers-End: 1olSQz-0003Gs-Gd Subject: [Openvpn-devel] [PATCH v5 2/3] Allow setting control channel packet size with max-packet-size X-BeenThere: openvpn-devel@lists.sourceforge.net X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox Currently control packet size is controlled by tun-mtu in a very non-obvious way since the control overhead is not taken into account and control channel packet will end up with a different size than data channel packet. Instead we decouple this and introduce max-packet-size. Control packet size defaults to 1250 if max-packet-size is not set. Patch v2: rebase on latest patch set Patch v3: Introduce TLS_CHANNEL_MTU_MIN define and give explaination of its value. Patch v4: introduce max-packet-size instead of tls-mtu Patch v5: improve documentation Signed-off-by: Arne Schwabe Acked-By: Frank Lichtenheld --- Changes.rst | 11 +++++++++- doc/man-sections/link-options.rst | 34 ++++++++++++++++++++++++++++--- src/openvpn/common.h | 13 ++++++++++++ src/openvpn/init.c | 8 ++++++-- src/openvpn/mtu.h | 5 +++++ src/openvpn/options.c | 21 +++++++++++++++++++ src/openvpn/options.h | 1 + src/openvpn/ssl.c | 26 ++++++++++++----------- src/openvpn/ssl.h | 8 +++----- 9 files changed, 104 insertions(+), 23 deletions(-) diff --git a/Changes.rst b/Changes.rst index fc5a1a853..61466c854 100644 --- a/Changes.rst +++ b/Changes.rst @@ -100,6 +100,13 @@ Inline auth username and password http-proxy-user-pass too. +Improved control channel packet size control (``max-packet-size``) + The size of control channel is no longer tied to + ``--link-mtu``/``--tun-mtu`` and can be set using ``--max-packet-size``. + Setting the size to small sizes no longer breaks the OpenVPN protocol in + certain situations. ``max-packet-size`` will also set ``mssfix`` to try + to limit data-channel packets as well. + Deprecated features ------------------- ``inetd`` has been removed @@ -162,7 +169,9 @@ User-visible Changes - Option ``--nobind`` is default when ``--client`` or ``--pull`` is used in the configuration - :code:`link_mtu` parameter is removed from environment or replaced with 0 when scripts are called with parameters. This parameter is unreliable and no longer internally calculated. - +- control channel packet maximum size is no longer influenced by + ``--link-mtu``/``--tun-mtu`` and must be set by ``--max-packet-size`` now. + The default is 1250 for the control channel size. - In point-to-point OpenVPN setups (no ``--server``), using ``--explict-exit-notiy`` on one end would terminate the other side at session end. This is considered a no longer useful default and has diff --git a/doc/man-sections/link-options.rst b/doc/man-sections/link-options.rst index eb098a0f8..6a7f48a59 100644 --- a/doc/man-sections/link-options.rst +++ b/doc/man-sections/link-options.rst @@ -172,9 +172,9 @@ the local and the remote host. the first place, and if big packets come through anyhow (from protocols other than TCP), ``--fragment`` will internally fragment them. - Both ``--fragment`` and ``--mssfix`` are designed to work around cases - where Path MTU discovery is broken on the network path between OpenVPN - peers. + ``max-packet-size``, ``--fragment`` and ``--mssfix`` are designed to work + around cases where Path MTU discovery is broken on the network path + between OpenVPN peers. The usual symptom of such a breakdown is an OpenVPN connection which successfully starts, but then stalls during active usage. @@ -189,6 +189,10 @@ the local and the remote host. --tun-mtu 1500 --fragment 1300 --mssfix + If the ``max-packet-size size`` option is used in the configuration + it will also act as if ``mssfix size mtu`` was specified in the + configuration. + --mtu-disc type Should we do Path MTU discovery on TCP/UDP channel? Only supported on OSes such as Linux that supports the necessary system call to set. @@ -465,3 +469,27 @@ the local and the remote host. if mode server: socket-flags TCP_NODELAY push "socket-flags TCP_NODELAY" + +--max-packet-size size + This option will instruct OpenVPN to try to limit the maximum on-write packet + size by restricting the control channel packet size and setting ``--mssfix``. + + OpenVPN will try to keep its control channel messages below this size but + due to some constraints in the protocol this is not always possible. If the + option is not set, the control packet maximum size defaults to 1250. + The control channel packet size will be restricted to values between + 512 and 2048. The maximum packet size includes encapsulation overhead like + UDP and IP. + + In terms of ``--mssfix`` it will expand to: + :: + + mssfix size mtu + + If you need to set ``--mssfix`` for data channel and control channel maximum + packet size independently, use ``--max-packet-size`` first, followed by a + ``--mssfix`` in the configuration. + + In general the default size of 1250 should work almost universally apart + from specific corner cases, especially since IPv6 requires a MTU of 1280 + or larger. diff --git a/src/openvpn/common.h b/src/openvpn/common.h index b94680885..dce6fd01d 100644 --- a/src/openvpn/common.h +++ b/src/openvpn/common.h @@ -68,6 +68,19 @@ typedef unsigned long ptr_type; */ #define TLS_CHANNEL_BUF_SIZE 2048 +/* TLS control buffer minimum size. This size is not actually inherent to + * the OpenVPN protocol. But with our current sending window being 6 and the + * receive window being 8 or 12 depending on the OpenVPN version, the biggest + * payload we can send is 6 * min_size. And we need to support to send payloads + * of TLS_CHANNEL_BUF_SIZE. Splitting this into more than + * 6 packets (with overhead) would complicate our sending logic a lot more. + * Diving TLS_CHANNEL_BUF_SIZE (2048) by 6 gets us ~342 byte. Allowing for + * ~100 bytes of overhead (in OpenVPN headers + IP headers) and rounding + * up to the next "nice" number gives use 512. + * + * */ +#define TLS_CHANNEL_MTU_MIN 512 + /* * This parameter controls the maximum size of a bundle * of pushed options. diff --git a/src/openvpn/init.c b/src/openvpn/init.c index c48048a1b..f1e9d31cc 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -2651,6 +2651,10 @@ frame_finalize_options(struct context *c, const struct options *o) * space */ size_t payload_size = max_int(1500, frame->tun_mtu); + /* we need to be also large enough to hold larger control channel packets + * if configured */ + payload_size = max_int(payload_size, o->ce.tls_mtu); + /* The extra tun needs to be added to the payload size */ if (o->ce.tun_mtu_defined) { @@ -3198,7 +3202,7 @@ do_init_frame_tls(struct context *c) { if (c->c2.tls_multi) { - tls_multi_init_finalize(c->c2.tls_multi, &c->c2.frame); + tls_multi_init_finalize(c->c2.tls_multi, c->options.ce.tls_mtu); ASSERT(c->c2.tls_multi->opt.frame.buf.payload_size <= c->c2.frame.buf.payload_size); frame_print(&c->c2.tls_multi->opt.frame, D_MTU_INFO, @@ -3206,7 +3210,7 @@ do_init_frame_tls(struct context *c) } if (c->c2.tls_auth_standalone) { - tls_init_control_channel_frame_parameters(&c->c2.frame, &c->c2.tls_auth_standalone->frame); + tls_init_control_channel_frame_parameters(&c->c2.tls_auth_standalone->frame, c->options.ce.tls_mtu); frame_print(&c->c2.tls_auth_standalone->frame, D_MTU_INFO, "TLS-Auth MTU parms"); c->c2.tls_auth_standalone->tls_wrap.work = alloc_buf_gc(BUF_SIZE(&c->c2.frame), &c->c2.gc); diff --git a/src/openvpn/mtu.h b/src/openvpn/mtu.h index d4856f166..370806fb5 100644 --- a/src/openvpn/mtu.h +++ b/src/openvpn/mtu.h @@ -79,6 +79,11 @@ */ #define MSSFIX_DEFAULT 1492 +/* + * Default maximum size of control channel packets + */ +#define TLS_MTU_DEFAULT 1250 + /* * Alignment of payload data such as IP packet or * ethernet frame. diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 9f5e4b35d..a86cc5cf1 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -826,6 +826,7 @@ init_options(struct options *o, const bool init_gc) o->ce.bind_local = true; o->ce.tun_mtu = TUN_MTU_DEFAULT; o->ce.link_mtu = LINK_MTU_DEFAULT; + o->ce.tls_mtu = TLS_MTU_DEFAULT; o->ce.mtu_discover_type = -1; o->ce.mssfix = 0; o->ce.mssfix_default = true; @@ -1712,6 +1713,7 @@ show_connection_entry(const struct connection_entry *o) SHOW_BOOL(link_mtu_defined); SHOW_INT(tun_mtu_extra); SHOW_BOOL(tun_mtu_extra_defined); + SHOW_INT(tls_mtu); SHOW_INT(mtu_discover_type); @@ -6457,6 +6459,25 @@ add_option(struct options *options, options->ce.tun_mtu_extra = positive_atoi(p[1]); options->ce.tun_mtu_extra_defined = true; } + else if (streq(p[0], "max-packet-size") && p[1] && !p[2]) + { + VERIFY_PERMISSION(OPT_P_MTU|OPT_P_CONNECTION); + int maxmtu = positive_atoi(p[1]); + options->ce.tls_mtu = constrain_int(maxmtu, TLS_CHANNEL_MTU_MIN, TLS_CHANNEL_BUF_SIZE); + + if (maxmtu < TLS_CHANNEL_MTU_MIN || maxmtu > TLS_CHANNEL_BUF_SIZE) + { + msg(M_WARN, "Note: max-packet-size value outside of allowed " + "control channel packet size (%d to %d), will use %d " + "instead.", TLS_CHANNEL_MTU_MIN, TLS_CHANNEL_BUF_SIZE, + options->ce.tls_mtu); + } + + /* also set mssfix maxmtu mtu */ + options->ce.mssfix = maxmtu; + options->ce.mssfix_default = false; + options->ce.mssfix_encap = true; + } #ifdef ENABLE_FRAGMENT else if (streq(p[0], "mtu-dynamic")) { diff --git a/src/openvpn/options.h b/src/openvpn/options.h index 55322baa0..b165ee5b7 100644 --- a/src/openvpn/options.h +++ b/src/openvpn/options.h @@ -123,6 +123,7 @@ struct connection_entry bool tun_mtu_extra_defined; int link_mtu; /* MTU of device over which tunnel packets pass via TCP/UDP */ bool link_mtu_defined; /* true if user overriding parm with command line option */ + int tls_mtu; /* Maximum MTU for the control channel messages */ /* Advanced MTU negotiation and datagram fragmentation options */ int mtu_discover_type; /* used if OS supports setting Path MTU discovery options on socket */ diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index 552a20846..f4a2f6566 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -297,8 +297,7 @@ tls_limit_reneg_bytes(const char *ciphername, int *reneg_bytes) } void -tls_init_control_channel_frame_parameters(const struct frame *data_channel_frame, - struct frame *frame) +tls_init_control_channel_frame_parameters(struct frame *frame, int tls_mtu) { /* * frame->extra_frame is already initialized with tls_auth buffer requirements, @@ -323,18 +322,21 @@ tls_init_control_channel_frame_parameters(const struct frame *data_channel_frame /* Previous OpenVPN version calculated the maximum size and buffer of a * control frame depending on the overhead of the data channel frame - * overhead and limited its maximum size to 1250. We always allocate the - * TLS_CHANNEL_BUF_SIZE buffer size since a lot of code blindly assumes - * a large buffer (e.g. PUSH_BUNDLE_SIZE) and also our peer might have - * a higher size configured and we still want to be able to receive the - * packets. frame->mtu_mtu is set as suggestion for the maximum packet - * size */ - frame->buf.payload_size = 1250 + overhead; + * overhead and limited its maximum size to 1250. Since control frames + * also need to fit into data channel buffer we have the same + * default of 1500 + 100 as data channel buffers have. Increasing + * control channel mtu beyond this limit also increases the data channel + * buffers */ + frame->buf.payload_size = max_int(1500, tls_mtu) + 100; frame->buf.headroom = overhead; frame->buf.tailroom = overhead; - frame->tun_mtu = min_int(data_channel_frame->tun_mtu, 1250); + frame->tun_mtu = tls_mtu; + + /* Ensure the tun-mtu stays in a valid range */ + frame->tun_mtu = min_int(frame->tun_mtu, TLS_CHANNEL_BUF_SIZE); + frame->tun_mtu = max_int(frame->tun_mtu, TLS_CHANNEL_MTU_MIN); } /** @@ -1314,9 +1316,9 @@ tls_multi_init(struct tls_options *tls_options) } void -tls_multi_init_finalize(struct tls_multi *multi, const struct frame *frame) +tls_multi_init_finalize(struct tls_multi *multi, int tls_mtu) { - tls_init_control_channel_frame_parameters(frame, &multi->opt.frame); + tls_init_control_channel_frame_parameters(&multi->opt.frame, tls_mtu); /* initialize the active and untrusted sessions */ tls_session_init(multi, &multi->session[TM_ACTIVE]); diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h index 9ae6ae8fc..118bd860a 100644 --- a/src/openvpn/ssl.h +++ b/src/openvpn/ssl.h @@ -166,10 +166,9 @@ struct tls_multi *tls_multi_init(struct tls_options *tls_options); * * @param multi - The \c tls_multi structure of which to finalize * initialization. - * @param frame - The data channel's \c frame structure. + * @param tls_mtu - maximum allowed size for control channel packets */ -void tls_multi_init_finalize(struct tls_multi *multi, - const struct frame *frame); +void tls_multi_init_finalize(struct tls_multi *multi, int tls_mtu); /* * Initialize a standalone tls-auth verification object. @@ -181,8 +180,7 @@ struct tls_auth_standalone *tls_auth_standalone_init(struct tls_options *tls_opt * Setups the control channel frame size parameters from the data channel * parameters */ -void tls_init_control_channel_frame_parameters(const struct frame *data_channel_frame, - struct frame *frame); +void tls_init_control_channel_frame_parameters(struct frame *frame, int tls_mtu); /* * Set local and remote option compatibility strings.