From patchwork Thu Dec 30 06:21:36 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arne Schwabe X-Patchwork-Id: 2190 Return-Path: Delivered-To: patchwork@openvpn.net Delivered-To: patchwork@openvpn.net Received: from director8.mail.ord1d.rsapps.net ([172.31.255.6]) by backend41.mail.ord1d.rsapps.net with LMTP id iNobMeLqzWHdLgAAqwncew (envelope-from ) for ; Thu, 30 Dec 2021 12:22:42 -0500 Received: from proxy12.mail.iad3b.rsapps.net ([172.31.255.6]) by director8.mail.ord1d.rsapps.net with LMTP id MNqBAOPqzWGGEgAAfY0hYg (envelope-from ) for ; Thu, 30 Dec 2021 12:22:43 -0500 Received: from smtp40.gate.iad3b ([172.31.255.6]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by proxy12.mail.iad3b.rsapps.net with LMTPS id aEv/M+LqzWGPHgAAEsW3lA (envelope-from ) for ; Thu, 30 Dec 2021 12:22:42 -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: smtp40.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: 18063b78-6995-11ec-a3c4-5254000cc6d4-1-1 Received: from [216.105.38.7] ([216.105.38.7:58782] helo=lists.sourceforge.net) by smtp40.gate.iad3b.rsapps.net (envelope-from ) (ecelerity 4.2.38.62370 r(:)) with ESMTPS (cipher=DHE-RSA-AES256-GCM-SHA384) id 0D/61-29055-2EAEDC16; Thu, 30 Dec 2021 12:22:42 -0500 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.94.2) (envelope-from ) id 1n2z7e-0002YZ-DH; Thu, 30 Dec 2021 17:21:50 +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.94.2) (envelope-from ) id 1n2z7c-0002YT-Ow for openvpn-devel@lists.sourceforge.net; Thu, 30 Dec 2021 17:21:48 +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=L32AgJ2ePIz0SMMF9meTgpa17y9kf616Lq4VrzjRAPE=; b=VTSLvGwzR5bP+dxRzSDuUsssFz oXuUsnYB6tI9j0r9dQidaWvqFrZjJ19jgzphao5ntOWXtFY/9fTJsqtHnggUnq7LbHRj8sli7RmBP xWRa4TqYK9K5r5X39HUtoF6Y16kRqYG3DS8BnWpnESIPjwUGJPhZ5CxWkv65zQd6P9ZQ=; 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=L32AgJ2ePIz0SMMF9meTgpa17y9kf616Lq4VrzjRAPE=; b=cFQPuU4AkhNFK6S+R7zsIh3+4S vINTmi1eRAYcFMh8mxkMIHTL5Is0BAQVz3tngvvXzqh0NxX8VDBKACJtmCpxzI3PC0VbGi6I/o5Mw ic/NCG1IZIybW6c0N/S/B86RB0BDlQ9ZvkTLpfU1igNyZy5Wqlh7hOcqilhFm6d9qgIk=; Received: from mail.blinkt.de ([192.26.174.232]) by sfi-mx-1.v28.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.92.3) id 1n2z7a-00Dyh9-Eb for openvpn-devel@lists.sourceforge.net; Thu, 30 Dec 2021 17:21:48 +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 1n2z7Q-0005Sw-6w for openvpn-devel@lists.sourceforge.net; Thu, 30 Dec 2021 18:21:36 +0100 Received: (nullmailer pid 2017262 invoked by uid 10006); Thu, 30 Dec 2021 17:21:36 -0000 From: Arne Schwabe To: openvpn-devel@lists.sourceforge.net Date: Thu, 30 Dec 2021 18:21:36 +0100 Message-Id: <20211230172136.2017215-1-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. Patch V2: rebase on master Signed-off-by: Arne Schwabe --- src/openvpn/mtu.c | 43 +++++++++ src/openvpn/mtu.h | 11 +++ src/openvpn/options.c | 51 tests/unit_tests/openvpn/Makefile.am | 6 +- tests/u [...] 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: 1n2z7a-00Dyh9-Eb Subject: [Openvpn-devel] [PATCH v2 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. Patch V2: rebase on master Signed-off-by: Arne Schwabe Acked-by: Gert Doering --- src/openvpn/mtu.c | 43 +++++++++ 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, 186 insertions(+), 54 deletions(-) diff --git a/src/openvpn/mtu.c b/src/openvpn/mtu.c index 662984f6..dac0c1de 100644 --- a/src/openvpn/mtu.c +++ b/src/openvpn/mtu.c @@ -138,6 +138,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 ae83d3e7..f6013860 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 fb049621..2ca24685 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 44b77cc5..f681b353 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 51672f9b..19ce174e 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 70f726d0..867fa1bb 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)