From patchwork Tue Dec 7 06:01:59 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arne Schwabe X-Patchwork-Id: 2140 Return-Path: Delivered-To: patchwork@openvpn.net Delivered-To: patchwork@openvpn.net Received: from director9.mail.ord1d.rsapps.net ([172.31.255.6]) by backend41.mail.ord1d.rsapps.net with LMTP id QA/2D9aTr2FYUwAAqwncew (envelope-from ) for ; Tue, 07 Dec 2021 12:03:18 -0500 Received: from proxy20.mail.iad3b.rsapps.net ([172.31.255.6]) by director9.mail.ord1d.rsapps.net with LMTP id OKYwLdaTr2HKYAAAalYnBA (envelope-from ) for ; Tue, 07 Dec 2021 12:03:18 -0500 Received: from smtp6.gate.iad3b ([172.31.255.6]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by proxy20.mail.iad3b.rsapps.net with LMTPS id mPG/JNaTr2GRKgAAcDxLoQ (envelope-from ) for ; Tue, 07 Dec 2021 12:03:18 -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: smtp6.gate.iad3b.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: 92261414-577f-11ec-9e41-5254000d607e-1-1 Received: from [216.105.38.7] ([216.105.38.7:43520] helo=lists.sourceforge.net) by smtp6.gate.iad3b.rsapps.net (envelope-from ) (ecelerity 4.2.38.62370 r(:)) with ESMTPS (cipher=DHE-RSA-AES256-GCM-SHA384) id 7E/2D-29509-5D39FA16; Tue, 07 Dec 2021 12:03:17 -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.94.2) (envelope-from ) id 1mudrK-0002qb-1b; Tue, 07 Dec 2021 17:02:30 +0000 Received: from [172.30.20.202] (helo=mx.sourceforge.net) by sfs-ml-4.v29.lw.sourceforge.com with esmtps (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1mudrF-0002ou-MU for openvpn-devel@lists.sourceforge.net; Tue, 07 Dec 2021 17:02:25 +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=37nb5gAI16fQDPGyi0msTodefXWlOQ8Rn5Q0Ghudu08=; b=mkxnfE0mwCYNyjp0SI1HtfIGUc jOEW/WciWC1LRbkeaXIbuhvUCZ5XESJ1CRMwThFlTh5xZnAygV4cK2mOWvXUgT8yHdFbCj8YMHgtN WjQDdCflXwzznD0U+DGr2a88Bxu/rOtOHwTn+jTLBsvyBDkMkbhueb8DcwmcEG42smus=; 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=37nb5gAI16fQDPGyi0msTodefXWlOQ8Rn5Q0Ghudu08=; b=lrFCkpt09G01AtwgE217//NzDs Acvs3jbSvC2i3xd+ZOREngpq9J0AFTLgB2zWZgbt0NbjTwhkdpM5AmU9Jc8wPTIHVu5mWWlm0p89f FoCCPT2/e106/APfF6N2mWD4MkYXtDyuF30SBUkrkPj3JqApCtrJIVkC6pgifhfumQRY=; Received: from mail.blinkt.de ([192.26.174.232]) by sfi-mx-2.v28.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.92.3) id 1mudrD-0006Mr-I1 for openvpn-devel@lists.sourceforge.net; Tue, 07 Dec 2021 17:02:25 +0000 Received: from kamera.blinkt.de ([2001:638:502:390:20c:29ff:fec8:535c]) by mail.blinkt.de with smtp (Exim 4.94.2 (FreeBSD)) (envelope-from ) id 1mudr1-000Ie2-W1 for openvpn-devel@lists.sourceforge.net; Tue, 07 Dec 2021 18:02:11 +0100 Received: (nullmailer pid 3275912 invoked by uid 10006); Tue, 07 Dec 2021 17:02:12 -0000 From: Arne Schwabe To: openvpn-devel@lists.sourceforge.net Date: Tue, 7 Dec 2021 18:01:59 +0100 Message-Id: <20211207170211.3275837-10-arne@rfc2549.org> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20211207170211.3275837-1-arne@rfc2549.org> References: <20211207170211.3275837-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: Use the functions that directly compute the link mtu instead relying on the frame logic. Signed-off-by: Arne Schwabe --- src/openvpn/mtu.c | 50 +++++++++- src/openvpn/mtu.h | 11 +++ src/openvpn/options.c | 51 tests/unit_tests/openvpn/Makefile.am | 6 +- tests/ [...] Content analysis details: (0.3 points, 6.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- 0.2 HEADER_FROM_DIFFERENT_DOMAINS From and EnvelopeFrom 2nd level mail domains are different 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record 0.0 SPF_NONE SPF: sender does not publish an SPF Record X-Headers-End: 1mudrD-0006Mr-I1 Subject: [Openvpn-devel] [PATCH 09/21] Rework occ link-mtu calculation 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 Use the functions that directly compute the link mtu instead relying on the frame logic. Signed-off-by: Arne Schwabe --- src/openvpn/mtu.c | 50 +++++++++- src/openvpn/mtu.h | 11 +++ src/openvpn/options.c | 51 ---------- tests/unit_tests/openvpn/Makefile.am | 6 +- tests/unit_tests/openvpn/test_crypto.c | 128 ++++++++++++++++++++++++- tests/unit_tests/openvpn/test_misc.c | 1 + 6 files changed, 192 insertions(+), 55 deletions(-) diff --git a/src/openvpn/mtu.c b/src/openvpn/mtu.c index e7ff477cd..c7f69bb2a 100644 --- a/src/openvpn/mtu.c +++ b/src/openvpn/mtu.c @@ -61,6 +61,8 @@ frame_calculate_protocol_header_size(const struct key_type *kt, /* Sum of all the overhead that reduces the usable packet size */ size_t header_size = 0; + bool tlsmode = options->tls_server || options->tls_client; + /* A socks proxy adds 10 byte of extra header to each packet */ if (options->ce.socks_proxy_server && proto_is_udp(options->ce.proto)) { @@ -74,7 +76,10 @@ frame_calculate_protocol_header_size(const struct key_type *kt, } /* Add the opcode and peerid */ - header_size += options->use_peer_id ? 4 : 1; + if (tlsmode) + { + header_size += options->use_peer_id ? 4 : 1; + } /* Add the crypto overhead */ bool packet_id = options->replay; @@ -131,6 +136,49 @@ frame_calculate_payload_size(const struct frame *frame, const struct options *op return payload_size; } +size_t +calc_options_string_link_mtu(const struct options *o, const struct frame *frame) +{ + unsigned int payload = frame_calculate_payload_size(frame, o); + + /* neither --secret nor TLS mode */ + if (!o->tls_client && !o->tls_server && !o->shared_secret_file) + { + return payload; + } + + struct key_type occ_kt; + + /* o->ciphername might be BF-CBC even though the underlying SSL library + * does not support it. For this reason we workaround this corner case + * by pretending to have no encryption enabled and by manually adding + * the required packet overhead to the MTU computation. + */ + const char* ciphername = o->ciphername; + + unsigned int overhead = 0; + + if (strcmp(o->ciphername, "BF-CBC") == 0) + { + /* none has no overhead, so use this to later add only --auth + * overhead */ + + /* overhead of BF-CBC: 64 bit block size, 64 bit IV size */ + overhead += 64/8 + 64/8; + /* set ciphername to none, so its size does get added in the + * fake_kt and the cipher is not tried to be resolved */ + ciphername = "none"; + } + + /* We pass tlsmode always true here since as we do not need to check if + * the ciphers are actually valid for non tls in occ calucation */ + init_key_type(&occ_kt, ciphername, o->authname, true, false); + + overhead += frame_calculate_protocol_header_size(&occ_kt, o, 0, true); + + return payload + overhead; +} + void frame_finalize(struct frame *frame, bool link_mtu_defined, diff --git a/src/openvpn/mtu.h b/src/openvpn/mtu.h index ae83d3e7a..f60138607 100644 --- a/src/openvpn/mtu.h +++ b/src/openvpn/mtu.h @@ -280,6 +280,17 @@ frame_calculate_protocol_header_size(const struct key_type *kt, unsigned int payload_size, bool occ); +/** + * Calculate the link-mtu to advertise to our peer. The actual value is not + * relevant, because we will possibly perform data channel cipher negotiation + * after this, but older clients will log warnings if we do not supply them the + * value they expect. This assumes that the traditional cipher/auth directives + * in the config match the config of the peer. + */ +size_t +calc_options_string_link_mtu(const struct options *options, + const struct frame *frame); + /* * frame_set_mtu_dynamic and flags */ diff --git a/src/openvpn/options.c b/src/openvpn/options.c index c1663b264..441855c7d 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -3764,57 +3764,6 @@ pre_connect_restore(struct options *o, struct gc_arena *gc) o->data_channel_crypto_flags = 0; } -/** - * Calculate the link-mtu to advertise to our peer. The actual value is not - * relevant, because we will possibly perform data channel cipher negotiation - * after this, but older clients will log warnings if we do not supply them the - * value they expect. This assumes that the traditional cipher/auth directives - * in the config match the config of the peer. - */ -static size_t -calc_options_string_link_mtu(const struct options *o, const struct frame *frame) -{ - size_t link_mtu = EXPANDED_SIZE(frame); - - if (o->pull || o->mode == MODE_SERVER) - { - struct frame fake_frame = *frame; - struct key_type fake_kt; - - frame_remove_from_extra_frame(&fake_frame, crypto_max_overhead()); - - - /* o->ciphername might be BF-CBC even though the underlying SSL library - * does not support it. For this reason we workaround this corner case - * by pretending to have no encryption enabled and by manually adding - * the required packet overhead to the MTU computation. - */ - const char* ciphername = o->ciphername; - - if (strcmp(o->ciphername, "BF-CBC") == 0) - { - /* none has no overhead, so use this to later add only --auth - * overhead */ - - /* overhead of BF-CBC: 64 bit block size, 64 bit IV size */ - frame_add_to_extra_frame(&fake_frame, 64/8 + 64/8); - /* set ciphername to none, so its size does get added in the - * fake_kt and the cipher is not tried to be resolved */ - ciphername = "none"; - } - - init_key_type(&fake_kt, ciphername, o->authname, true, false); - - crypto_adjust_frame_parameters(&fake_frame, &fake_kt, o->replay, - cipher_kt_mode_ofb_cfb(fake_kt.cipher)); - frame_finalize(&fake_frame, o->ce.link_mtu_defined, o->ce.link_mtu, - o->ce.tun_mtu_defined, o->ce.tun_mtu); - msg(D_MTU_DEBUG, "%s: link-mtu %u -> %d", __func__, (unsigned int) link_mtu, - EXPANDED_SIZE(&fake_frame)); - link_mtu = EXPANDED_SIZE(&fake_frame); - } - return link_mtu; -} /* * Build an options string to represent data channel encryption options. * This string must match exactly between peers. The keysize is checked diff --git a/tests/unit_tests/openvpn/Makefile.am b/tests/unit_tests/openvpn/Makefile.am index 44b77cc5d..f681b353c 100644 --- a/tests/unit_tests/openvpn/Makefile.am +++ b/tests/unit_tests/openvpn/Makefile.am @@ -46,7 +46,9 @@ crypto_testdriver_SOURCES = test_crypto.c mock_msg.c mock_msg.h \ $(openvpn_srcdir)/crypto_openssl.c \ $(openvpn_srcdir)/otime.c \ $(openvpn_srcdir)/packet_id.c \ - $(openvpn_srcdir)/platform.c + $(openvpn_srcdir)/platform.c \ + $(openvpn_srcdir)/mtu.c \ + $(openvpn_srcdir)/mss.c packet_id_testdriver_CFLAGS = @TEST_CFLAGS@ \ -I$(openvpn_includedir) -I$(compat_srcdir) -I$(openvpn_srcdir) @@ -137,4 +139,4 @@ misc_testdriver_SOURCES = test_misc.c mock_msg.c \ mock_get_random.c \ $(openvpn_srcdir)/buffer.c \ $(openvpn_srcdir)/ssl_util.c \ - $(openvpn_srcdir)/platform.c \ No newline at end of file + $(openvpn_srcdir)/platform.c diff --git a/tests/unit_tests/openvpn/test_crypto.c b/tests/unit_tests/openvpn/test_crypto.c index 51672f9b2..19ce174ea 100644 --- a/tests/unit_tests/openvpn/test_crypto.c +++ b/tests/unit_tests/openvpn/test_crypto.c @@ -37,6 +37,7 @@ #include #include "crypto.h" +#include "options.h" #include "ssl_backend.h" #include "mock_msg.h" @@ -234,6 +235,130 @@ test_des_encrypt(void **state) free(src2); } +/* This test is in test_crypto as it calls into the functions that calculate + * the crypto overhead */ +static void +test_occ_mtu_calculation(void **state) +{ + struct gc_arena gc = gc_new(); + + struct frame f = { 0 }; + struct options o = { 0 }; + size_t linkmtu; + + /* common defaults */ + o.ce.tun_mtu = 1400; + o.replay = true; + o.ce.proto = PROTO_UDP; + + /* No crypto at all */ + o.ciphername = "none"; + o.authname = "none"; + linkmtu = calc_options_string_link_mtu(&o, &f); + assert_int_equal(linkmtu, 1400); + + /* Static key OCC examples */ + o.shared_secret_file = "not null"; + + /* secret, auth none, cipher none */ + o.ciphername = "none"; + o.authname = "none"; + linkmtu = calc_options_string_link_mtu(&o, &f); + assert_int_equal(linkmtu, 1408); + + /* secret, cipher AES-128-CBC, auth none */ + o.ciphername = "AES-128-CBC"; + o.authname = "none"; + linkmtu = calc_options_string_link_mtu(&o, &f); + assert_int_equal(linkmtu, 1440); + + /* secret, cipher none, auth SHA256 */ + o.ciphername = "none"; + o.authname = "SHA256"; + linkmtu = calc_options_string_link_mtu(&o, &f); + assert_int_equal(linkmtu, 1440); + + /* --secret, cipher BF-CBC, auth SHA1 */ + o.ciphername = "BF-CBC"; + o.authname = "SHA1"; + linkmtu = calc_options_string_link_mtu(&o, &f); + assert_int_equal(linkmtu, 1444); + + /* --secret, cipher BF-CBC, auth SHA1, tcp-client */ + o.ce.proto = PROTO_TCP_CLIENT; + linkmtu = calc_options_string_link_mtu(&o, &f); + assert_int_equal(linkmtu, 1446); + + o.ce.proto = PROTO_UDP; + + /* --secret, comp-lzo yes, cipher BF-CBC, auth SHA1 */ + o.comp.alg = COMP_ALG_LZO; + linkmtu = calc_options_string_link_mtu(&o, &f); + assert_int_equal(linkmtu, 1445); + + /* --secret, comp-lzo yes, cipher BF-CBC, auth SHA1, fragment 1200 */ + o.ce.fragment = 1200; + linkmtu = calc_options_string_link_mtu(&o, &f); + assert_int_equal(linkmtu, 1449); + + o.comp.alg = COMP_ALG_UNDEF; + o.ce.fragment = 0; + + /* TLS mode */ + o.shared_secret_file = NULL; + o.tls_client = true; + o.pull = true; + + /* tls client, cipher AES-128-CBC, auth SHA1, tls-auth*/ + o.authname = "SHA1"; + o.ciphername = "AES-128-CBC"; + o.tls_auth_file = "dummy"; + + linkmtu = calc_options_string_link_mtu(&o, &f); + assert_int_equal(linkmtu, 1457); + + /* tls client, cipher AES-128-CBC, auth SHA1 */ + o.tls_auth_file = NULL; + + linkmtu = calc_options_string_link_mtu(&o, &f); + assert_int_equal(linkmtu, 1457); + + /* tls client, cipher none, auth none */ + o.authname = "none"; + o.ciphername = "none"; + + linkmtu = calc_options_string_link_mtu(&o, &f); + assert_int_equal(linkmtu, 1405); + + /* tls client, auth none, cipher none, no-replay */ + o.replay = false; + + linkmtu = calc_options_string_link_mtu(&o, &f); + assert_int_equal(linkmtu, 1401); + + + o.replay = true; + + /* tls client, auth SHA1, cipher AES-256-GCM */ + o.authname = "SHA1"; + o.ciphername = "AES-256-GCM"; + linkmtu = calc_options_string_link_mtu(&o, &f); + assert_int_equal(linkmtu, 1449); + + + /* tls client, auth SHA1, cipher AES-256-GCM, fragment, comp-lzo yes */ + o.comp.alg = COMP_ALG_LZO; + o.ce.fragment = 1200; + linkmtu = calc_options_string_link_mtu(&o, &f); + assert_int_equal(linkmtu, 1454); + + /* tls client, auth SHA1, cipher AES-256-GCM, fragment, comp-lzo yes, socks */ + o.ce.socks_proxy_server = "socks.example.com"; + linkmtu = calc_options_string_link_mtu(&o, &f); + assert_int_equal(linkmtu, 1464); + + gc_free(&gc); +} int main(void) @@ -243,7 +368,8 @@ main(void) cmocka_unit_test(crypto_translate_cipher_names), cmocka_unit_test(crypto_test_tls_prf), cmocka_unit_test(crypto_test_hmac), - cmocka_unit_test(test_des_encrypt) + cmocka_unit_test(test_des_encrypt), + cmocka_unit_test(test_occ_mtu_calculation) }; #if defined(ENABLE_CRYPTO_OPENSSL) diff --git a/tests/unit_tests/openvpn/test_misc.c b/tests/unit_tests/openvpn/test_misc.c index 70f726d0f..867fa1bb5 100644 --- a/tests/unit_tests/openvpn/test_misc.c +++ b/tests/unit_tests/openvpn/test_misc.c @@ -37,6 +37,7 @@ #include #include "ssl_util.h" +#include "options.h" static void test_compat_lzo_string(void **state)