From patchwork Wed Oct 30 01:44:59 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Lev Stipakov X-Patchwork-Id: 874 Return-Path: Delivered-To: patchwork@openvpn.net Delivered-To: patchwork@openvpn.net Received: from director7.mail.ord1d.rsapps.net ([172.27.255.7]) by backend30.mail.ord1d.rsapps.net with LMTP id eDmiEMuGuV34GQAAIUCqbw for ; Wed, 30 Oct 2019 08:49:15 -0400 Received: from proxy8.mail.iad3a.rsapps.net ([172.27.255.7]) by director7.mail.ord1d.rsapps.net with LMTP id wHLnDcuGuV2XfgAAovjBpQ ; Wed, 30 Oct 2019 08:49:15 -0400 Received: from smtp31.gate.iad3a ([172.27.255.7]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by proxy8.mail.iad3a.rsapps.net with LMTP id ILyUB8uGuV1WHgAAsBr/qg ; Wed, 30 Oct 2019 08:49:15 -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: smtp31.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; dkim=fail (signature verification failed) header.d=gmail.com; dmarc=fail (p=none; dis=none) header.from=gmail.com X-Suspicious-Flag: YES X-Classification-ID: ad004de2-fb13-11e9-94d4-5254003d9392-1-1 Received: from [216.105.38.7] ([216.105.38.7:45858] helo=lists.sourceforge.net) by smtp31.gate.iad3a.rsapps.net (envelope-from ) (ecelerity 4.2.38.62370 r(:)) with ESMTPS (cipher=DHE-RSA-AES256-GCM-SHA384) id B6/2D-20094-AC689BD5; Wed, 30 Oct 2019 08:49:14 -0400 Received: from [127.0.0.1] (helo=sfs-ml-1.v29.lw.sourceforge.com) by sfs-ml-1.v29.lw.sourceforge.com with esmtp (Exim 4.90_1) (envelope-from ) id 1iPnOL-0003lT-Gd; Wed, 30 Oct 2019 12:48:01 +0000 Received: from [172.30.20.202] (helo=mx.sourceforge.net) by sfs-ml-1.v29.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.90_1) (envelope-from ) id 1iPnOK-0003lG-RC for openvpn-devel@lists.sourceforge.net; Wed, 30 Oct 2019 12:48:00 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=References:In-Reply-To:Message-Id:Date:Subject:To: From:Sender:Reply-To:Cc:MIME-Version:Content-Type:Content-Transfer-Encoding: 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=EGQk3d+28kMSi7j4lXG05RyvpLtW8rVYtTPDfQbZ3MQ=; b=BUXUvLaBh/7yaAm9Uk5uhXzkoP BzKXLF1hsX1EQ8txPOMU2bznedzIESw+DHI0PEnhh+6jaA5L8wwiTAbI3y8XFVd1/jCUxb61B89Nf JWHUXIHoWFTA+g5b6/xtRi+XPbdjiKOFalI6KwrSRwNG0Z+ex696tt+TaR2k3cZ4Fw3s=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=References:In-Reply-To:Message-Id:Date:Subject:To:From:Sender:Reply-To:Cc :MIME-Version:Content-Type:Content-Transfer-Encoding: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=EGQk3d+28kMSi7j4lXG05RyvpLtW8rVYtTPDfQbZ3MQ=; b=kJ8YCS4ZpdjpHoYu5o/Jr/suxM ClhJZYIVHJlGi+axzt9mOMFkoHC2YdEXlQuuCLJMJpwxz3tcLU/XRG3qYMzMz5mFNb6EXpnCchVQV /zJhwQNfy2sk1IFVmibD8SfmacGkRZakTm+dysBe9j5jmjOLpUSCkf8EtAniY8Ln9wk8=; Received: from mail-wr1-f68.google.com ([209.85.221.68]) by sfi-mx-1.v28.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES128-GCM-SHA256:128) (Exim 4.92.2) id 1iPnOJ-008hPV-9v for openvpn-devel@lists.sourceforge.net; Wed, 30 Oct 2019 12:48:00 +0000 Received: by mail-wr1-f68.google.com with SMTP id n15so2123408wrw.13 for ; Wed, 30 Oct 2019 05:47:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:subject:date:message-id:in-reply-to:references; bh=EGQk3d+28kMSi7j4lXG05RyvpLtW8rVYtTPDfQbZ3MQ=; b=vFehpjMxR63HsM3k/LYXSTwOoLK2buweJ2s6zPvOxamjfA8kcl697tXn+WEcutUSD4 lPKWm4zD9/UObHN+08Q+YFPQ0I03Zza1vX8u9yiUvcujE7mwue7vpg7WbEhEJZiTJDWH /s4M5915XHWM82+qTfWwuoxi1a/MKRU8c8mUToQAv4a2kyB5YfIwpFFLbgwFmuHyjRPE bJ9ChVi3xktnWH7regNwOxOMVcvSZ6R1kP8N+UP5TkJs87oOVNizWTELY+/zw6lnaRqG /4dgOXOe+uINEXt7wC2t5C05wGCch72OTEGekHn8cj5g4ynAyWbSlfNnEEaEs53BhYAT ILNg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references; bh=EGQk3d+28kMSi7j4lXG05RyvpLtW8rVYtTPDfQbZ3MQ=; b=KP/yivNWdG/JKLfvsUuhdQ+dWxXnY/k/vBC6FeXPby8JwWIxW3gdq//CYxtxjKDqhY AT4+NUxjiTGXlvAQnDcXwSvCOz2E6jU+QmDudYSdYo/hoLNqzwqqBs6h7Xu+GJZ7iB5J CyglK8z2K9An5yKApG6DroqpMl/RN3V8u00rCD+L/hlxLHCasTa10JWAtiqqV3UVzgUw D37rhtfjU8REAmC5ju4/TsrB7P38zXeduycs92EnjK5YCoHTIQ+DucMppcGee/9o755L VzFfipbZbV7w4eF6HTdOHMhnpUnR34reizb6PfT6qh8xeuDFUBP0nB23OYkqp+cpoQmq CEBQ== X-Gm-Message-State: APjAAAU60wM8aps+kvHErMxo2qqRkB0VuauGMFF5yzkyhqH9s5Ky5qxD ZMYd0yrVyhuojwizXPcmHTSaa6WgRBc4sQ== X-Google-Smtp-Source: APXvYqwVETUgn3iAV+5QmLHDRF/7KtSr+C2wMWST7pC8637afMwM8gtBsDPMColADaOCz3jaDVryxQ== X-Received: by 2002:adf:f08d:: with SMTP id n13mr23364008wro.324.1572439672209; Wed, 30 Oct 2019 05:47:52 -0700 (PDT) Received: from stipakov.fi (stipakov.fi. [128.199.52.117]) by smtp.gmail.com with ESMTPSA id c8sm1876177wml.44.2019.10.30.05.47.51 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Wed, 30 Oct 2019 05:47:51 -0700 (PDT) From: Lev Stipakov To: openvpn-devel@lists.sourceforge.net Date: Wed, 30 Oct 2019 14:44:59 +0200 Message-Id: <1572439499-16276-1-git-send-email-lstipakov@gmail.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: References: X-Spam-Report: Spam Filtering performed by mx.sourceforge.net. See http://spamassassin.org/tag/ for more details. -0.0 RCVD_IN_DNSWL_NONE RBL: Sender listed at https://www.dnswl.org/, no trust [209.85.221.68 listed in list.dnswl.org] 0.0 FREEMAIL_FROM Sender email is commonly abused enduser mail provider (lstipakov[at]gmail.com) 0.0 URIBL_BLOCKED ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [URIs: openvpn.net] -0.0 RCVD_IN_MSPIKE_H2 RBL: Average reputation (+2) [209.85.221.68 listed in wl.mailspike.net] -0.0 SPF_PASS SPF: sender matches SPF record 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid X-Headers-End: 1iPnOJ-008hPV-9v Subject: [Openvpn-devel] [PATCH v3] Fix broken fragmentation logic when using NCP 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: , MIME-Version: 1.0 Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox From: Lev Stipakov This is the 2.4 backport of master patch (commit d22ba6b). NCP negotiation replaces worst case crypto overhead with actual one in data channel frame. That frame params are used by mssfix. Fragment frame still contains worst case overhead. Without this patch, fragmentation logic incorrectly uses max crypto overhead when calculating packet size. It exceeds fragment size and openvpn peforms fragmentation: > sudo tcpdump port 1194 13:59:06.956394 IP server.fi.openvpn > nat2.panoulu.net.openvpn: UDP, length 652 13:59:06.956489 IP server.fi.openvpn > nat2.panoulu.net.openvpn: UDP, length 648 This patch fixes fragmentation calculation by setting actual crypto overhead, and no unnecessary fragmentation is performed: > sudo tcpdump port 1194 13:58:08.685915 IP server.fi.openvpn > nat2.panoulu.net.openvpn: UDP, length 1272 13:58:08.686007 IP server.fi.openvpn > nat2.panoulu.net.openvpn: UDP, length 1272 Trac #1140 Signed-off-by: Lev Stipakov Acked-by: Gert Doering --- v3: Refine a bit misleading commit message. Before one might think that problem is in incorrect MSS value being set - it is not. The problem is in fragment calculation. v2: Fix --disable-fragment build src/openvpn/forward.c | 3 +++ src/openvpn/init.c | 12 +++++++++++- src/openvpn/openvpn.h | 1 + src/openvpn/push.c | 9 ++++++++- src/openvpn/ssl.c | 19 ++++++++++++++++++- src/openvpn/ssl.h | 13 ++++++++----- 6 files changed, 49 insertions(+), 8 deletions(-) diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c index 65f790f..84bb584 100644 --- a/src/openvpn/forward.c +++ b/src/openvpn/forward.c @@ -873,6 +873,9 @@ process_incoming_link_part1(struct context *c, struct link_socket_info *lsi, boo if (is_hard_reset(opcode, c->options.key_method)) { c->c2.frame = c->c2.frame_initial; +#ifdef ENABLE_FRAGMENT + c->c2.frame_fragment = c->c2.frame_fragment_initial; +#endif } interval_action(&c->c2.tmp_int); diff --git a/src/openvpn/init.c b/src/openvpn/init.c index d3785ca..37b832a 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -2294,9 +2294,18 @@ do_deferred_options(struct context *c, const unsigned int found) { tls_poor_mans_ncp(&c->options, c->c2.tls_multi->remote_ciphername); } + struct frame *frame_fragment = NULL; +#ifdef ENABLE_FRAGMENT + if (c->options.ce.fragment) + { + frame_fragment = &c->c2.frame_fragment; + } +#endif + /* Do not regenerate keys if server sends an extra push reply */ if (!session->key[KS_PRIMARY].crypto_options.key_ctx_bi.initialized - && !tls_session_update_crypto_params(session, &c->options, &c->c2.frame)) + && !tls_session_update_crypto_params(session, &c->options, &c->c2.frame, + frame_fragment)) { msg(D_TLS_ERRORS, "OPTIONS ERROR: failed to import crypto options"); return false; @@ -3035,6 +3044,7 @@ do_init_frame(struct context *c) */ c->c2.frame_fragment = c->c2.frame; frame_subtract_extra(&c->c2.frame_fragment, &c->c2.frame_fragment_omit); + c->c2.frame_fragment_initial = c->c2.frame_fragment; #endif #if defined(ENABLE_FRAGMENT) && defined(ENABLE_OCC) diff --git a/src/openvpn/openvpn.h b/src/openvpn/openvpn.h index 7736183..ed7975c 100644 --- a/src/openvpn/openvpn.h +++ b/src/openvpn/openvpn.h @@ -269,6 +269,7 @@ struct context_2 /* Object to handle advanced MTU negotiation and datagram fragmentation */ struct fragment_master *fragment; struct frame frame_fragment; + struct frame frame_fragment_initial; struct frame frame_fragment_omit; #endif diff --git a/src/openvpn/push.c b/src/openvpn/push.c index dd5bd41..ba2fbe4 100644 --- a/src/openvpn/push.c +++ b/src/openvpn/push.c @@ -287,11 +287,18 @@ incoming_push_message(struct context *c, const struct buffer *buffer) { if (c->options.mode == MODE_SERVER) { + struct frame *frame_fragment = NULL; +#ifdef ENABLE_FRAGMENT + if (c->options.ce.fragment) + { + frame_fragment = &c->c2.frame_fragment; + } +#endif struct tls_session *session = &c->c2.tls_multi->session[TM_ACTIVE]; /* Do not regenerate keys if client send a second push request */ if (!session->key[KS_PRIMARY].crypto_options.key_ctx_bi.initialized && !tls_session_update_crypto_params(session, &c->options, - &c->c2.frame)) + &c->c2.frame, frame_fragment)) { msg(D_TLS_ERRORS, "TLS Error: initializing data channel failed"); goto error; diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index 9696e9b..7dcd962 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -1962,7 +1962,8 @@ cleanup: bool tls_session_update_crypto_params(struct tls_session *session, - struct options *options, struct frame *frame) + struct options *options, struct frame *frame, + struct frame *frame_fragment) { if (!session->opt->server && 0 != strcmp(options->ciphername, session->opt->config_ciphername) @@ -2006,6 +2007,22 @@ tls_session_update_crypto_params(struct tls_session *session, frame_init_mssfix(frame, options); frame_print(frame, D_MTU_INFO, "Data Channel MTU parms"); + /* + * mssfix uses data channel framing, which at this point contains + * actual overhead. Fragmentation logic uses frame_fragment, which + * still contains worst case overhead. Replace it with actual overhead + * to prevent unneeded fragmentation. + */ + + if (frame_fragment) + { + frame_remove_from_extra_frame(frame_fragment, crypto_max_overhead()); + crypto_adjust_frame_parameters(frame_fragment, &session->opt->key_type, + options->use_iv, options->replay, packet_id_long_form); + frame_set_mtu_dynamic(frame_fragment, options->ce.fragment, SET_MTU_UPPER_BOUND); + frame_print(frame_fragment, D_MTU_INFO, "Fragmentation MTU parms"); + } + return tls_session_generate_data_channel_keys(session); } diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h index 8066789..6672d43 100644 --- a/src/openvpn/ssl.h +++ b/src/openvpn/ssl.h @@ -475,15 +475,18 @@ void tls_update_remote_addr(struct tls_multi *multi, * Update TLS session crypto parameters (cipher and auth) and derive data * channel keys based on the supplied options. * - * @param session The TLS session to update. - * @param options The options to use when updating session. - * @param frame The frame options for this session (frame overhead is - * adjusted based on the selected cipher/auth). + * @param session The TLS session to update. + * @param options The options to use when updating session. + * @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. * * @return true if updating succeeded, false otherwise. */ bool tls_session_update_crypto_params(struct tls_session *session, - struct options *options, struct frame *frame); + struct options *options, + struct frame *frame, + struct frame *frame_fragment); /** * "Poor man's NCP": Use peer cipher if it is an allowed (NCP) cipher.