From patchwork Mon Nov 12 03:16:15 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Lev Stipakov X-Patchwork-Id: 606 X-Patchwork-Delegate: steffan@karger.me Return-Path: Delivered-To: patchwork@openvpn.net Delivered-To: patchwork@openvpn.net Received: from director11.mail.ord1d.rsapps.net ([172.30.191.6]) by backend30.mail.ord1d.rsapps.net with LMTP id WNwWMlaf6VsKUAAAIUCqbw for ; Mon, 12 Nov 2018 10:42:14 -0500 Received: from proxy3.mail.ord1d.rsapps.net ([172.30.191.6]) by director11.mail.ord1d.rsapps.net with LMTP id OB0UMlaf6VtyeAAAvGGmqA ; Mon, 12 Nov 2018 10:42:14 -0500 Received: from smtp34.gate.ord1c ([172.30.191.6]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by proxy3.mail.ord1d.rsapps.net with LMTP id uLLHMVaf6VsZHAAA7WKfLA ; Mon, 12 Nov 2018 10:42:14 -0500 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: smtp34.gate.ord1c.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: 861ea94a-e691-11e8-b5de-545200247500-1-1 Received: from [216.105.38.7] ([216.105.38.7:30846] helo=lists.sourceforge.net) by smtp34.gate.ord1c.rsapps.net (envelope-from ) (ecelerity 4.2.38.62370 r(:)) with ESMTPS (cipher=DHE-RSA-AES256-GCM-SHA384) id 8F/E8-03805-55F99EB5; Mon, 12 Nov 2018 10:42:13 -0500 Received: from [127.0.0.1] (helo=sfs-ml-4.v29.lw.sourceforge.com) by sfs-ml-4.v29.lw.sourceforge.com with esmtp (Exim 4.90_1) (envelope-from ) id 1gMD5J-0001ZP-Cr; Mon, 12 Nov 2018 14:21:01 +0000 Received: from [172.30.20.202] (helo=mx.sourceforge.net) by sfs-ml-4.v29.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.90_1) (envelope-from ) id 1gMD5H-0001ZF-JZ for openvpn-devel@lists.sourceforge.net; Mon, 12 Nov 2018 14:20:59 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=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:In-Reply-To:References:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=zWJ3g/x2xkdogrGhhWQZ5ZtrH8FiHQQ/JP2npA4PaEA=; b=lxZvzu+2K2zxXPDnZuX3r0vmlQ 6bizxw1QzAEyPCbLR3D1WQr/2fCpw1KJjOBJFuqndvGYgD3ejpRneVnkwE8gGRpEHukSl2IY92lFV Hd+CS7kKy3HPYyOqGxJjTnrk1GZ129A2N/UXR3pMEcPVary2eMomkYuoSLrphLPjsIm4=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=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: In-Reply-To:References:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=zWJ3g/x2xkdogrGhhWQZ5ZtrH8FiHQQ/JP2npA4PaEA=; b=bmDW8kFkMgVau/uBH71lqDwhdx 6mWH+ucxuOliQNiY3wGu1e+uEAlaKL65qAxBIPZWLD46eIuIGaxzi9JHTRcLzZiHFrf2HYGuG3TH6 u85YUM6dir4lAdUjSmSpHEct6n1b0Lfg46yuhsX63zYltqIq8m+evp0ZAvwujEkUYZJo=; Received: from mail-ed1-f68.google.com ([209.85.208.68]) by sfi-mx-3.v28.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES128-GCM-SHA256:128) (Exim 4.90_1) id 1gMD4z-006jby-W6 for openvpn-devel@lists.sourceforge.net; Mon, 12 Nov 2018 14:20:59 +0000 Received: by mail-ed1-f68.google.com with SMTP id j6so2518847edp.9 for ; Mon, 12 Nov 2018 06:20:41 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:subject:date:message-id; bh=zWJ3g/x2xkdogrGhhWQZ5ZtrH8FiHQQ/JP2npA4PaEA=; b=Rc2bkjuasz0wPIAIzZwUrqMVZzD18htTzMacWplxAimbqM4mcwsHGqrOmXswRXRidx 1RkMsmfJ8m5jUKTh/klxFM1aNU/GadHrIdrXnr2F8V5Qx9gX/4+lfRwFe6lMmXdYtpQ5 jUpwFfWP97/+Ecyq8ODZoBbiffjMTe5DYSQZjUQuv9jGswdYOHnBu4FAUZ7vESoUTn8O xgI21SE0yOPixF0kj1jUJuQ2hJJ6cgK8V6XTmT4LCPe6eoV+3BoeNk7InVZB/e9pA4n9 8FSA34hmi3pW8baVe2TO0eORLaQuJEkwAFuboDZbThlPoxT7x/3r1bYgJzQuk66lTRvJ e0gA== 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; bh=zWJ3g/x2xkdogrGhhWQZ5ZtrH8FiHQQ/JP2npA4PaEA=; b=RClK0iU5Xi8O+k/yWqEhq4j/pbZvzmacBXXoNU3gljiNYY7N9XwE2W+b7HBY3VrKpa dXDRoxVCyghhSqSsSNTnW96iadYXqHxRnJ+GlQd7gBlGxVuxDgDEWBf9K0jaAlMgBd73 iICDT70wCPiZcdK/McoFQXnOGcOUF11oiPl1KW9KsJQmzsH14ZrGzQ4Y86ZDBldccCLj cJ1pnkxjKy7z5ddSYxuuuR8FQ5H+pyyRxP/cXVLnjKmSVHkDDSd75BcJDKkON62ElfRf ohNnVuMpOKA/OwiJrp5PYLKyBkcf0fZkgK4DkSIia+HVyiE+CUTLbNOxBL60lCFd051T oWBA== X-Gm-Message-State: AGRZ1gIpf6BYlQx03k+ScXGTtZppcZmE7uTAFesx/s1KRQMxUpEi7sOJ MU4f8R5pQMVVaNta9Begizb07mkSlRE= X-Google-Smtp-Source: AJdET5efmyRVwYeaAC4T+fUMychU8zI/w+HTeOU+Jj45iwtkuKQwY83dIqqn4ctfm47WfUwWIEFBPw== X-Received: by 2002:a17:906:8314:: with SMTP id j20-v6mr9743096ejx.60.1542032434746; Mon, 12 Nov 2018 06:20:34 -0800 (PST) Received: from stipakov.fi (stipakov.fi. [128.199.52.117]) by smtp.gmail.com with ESMTPSA id c11-v6sm194681ejz.70.2018.11.12.06.20.33 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Mon, 12 Nov 2018 06:20:34 -0800 (PST) From: Lev Stipakov To: openvpn-devel@lists.sourceforge.net Date: Mon, 12 Nov 2018 16:16:15 +0200 Message-Id: <1542032175-5716-1-git-send-email-lstipakov@gmail.com> X-Mailer: git-send-email 2.7.4 X-Spam-Report: Spam Filtering performed by mx.sourceforge.net. See http://spamassassin.org/tag/ for more details. 0.0 FREEMAIL_FROM Sender email is commonly abused enduser mail provider (lstipakov[at]gmail.com) -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 0.0 TIME_LIMIT_EXCEEDED Exceeded time limit / deadline X-Headers-End: 1gMD4z-006jby-W6 Subject: [Openvpn-devel] [PATCH] Fix broken fragment/mssfix with 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 NCP negotiation replaces worst cast crypto overhead with actual one in data channel frame. That frame params are used by mssfix. Fragment frame still contains worst case overhead. Because of that TCP packets are fragmented, since MSS value exceeds max fragment size. Fix by replacing worst case crypto overhead with actual one for fragment frame, as it is done for data channel frame. Trac #1140 Signed-off-by: Lev Stipakov --- src/openvpn/forward.c | 1 + 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, 47 insertions(+), 8 deletions(-) diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c index f8faa81..773f5d7 100644 --- a/src/openvpn/forward.c +++ b/src/openvpn/forward.c @@ -1090,6 +1090,7 @@ 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; + c->c2.frame_fragment = c->c2.frame_fragment_initial; } interval_action(&c->c2.tmp_int); diff --git a/src/openvpn/init.c b/src/openvpn/init.c index 39e8ca5..db96353 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -2300,9 +2300,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; @@ -3082,6 +3091,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 d11f61d..a5d250f 100644 --- a/src/openvpn/openvpn.h +++ b/src/openvpn/openvpn.h @@ -265,6 +265,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 72f0996..a44646a 100644 --- a/src/openvpn/push.c +++ b/src/openvpn/push.c @@ -274,11 +274,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 74b88ce..8942f71 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -1986,7 +1986,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) @@ -2030,6 +2031,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->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 ac6fef7..4802e62 100644 --- a/src/openvpn/ssl.h +++ b/src/openvpn/ssl.h @@ -474,15 +474,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.