Reduce fragmentation when using ncp-ciphers
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg18975.html
This commit is contained in:
parent
31c498d59b
commit
2ba01738a7
Notes:
svn2git
2021-03-31 03:12:20 +00:00
svn path=/head/; revision=524180
@ -3,7 +3,7 @@
|
||||
|
||||
PORTNAME= openvpn
|
||||
DISTVERSION= 2.4.8
|
||||
PORTREVISION?= 0
|
||||
PORTREVISION?= 1
|
||||
CATEGORIES= security net net-vpn
|
||||
MASTER_SITES= https://swupdate.openvpn.org/community/releases/ \
|
||||
https://build.openvpn.net/downloads/releases/
|
||||
|
@ -0,0 +1,195 @@
|
||||
From 3bd91cd0e68762b861c57cf37f144d8a11704e9d Mon Sep 17 00:00:00 2001
|
||||
From: Lev Stipakov <lev@openvpn.net>
|
||||
Date: Wed, 30 Oct 2019 14:44:59 +0200
|
||||
Subject: [PATCH] Fix broken fragmentation logic when using NCP
|
||||
|
||||
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 <lev@openvpn.net>
|
||||
Acked-by: Gert Doering <gert@greenie.muc.de>
|
||||
Message-Id: <1572439499-16276-1-git-send-email-lstipakov@gmail.com>
|
||||
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg18975.html
|
||||
Signed-off-by: Gert Doering <gert@greenie.muc.de>
|
||||
---
|
||||
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 65f790fda..84bb58447 100644
|
||||
--- ./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 d3785cabd..37b832ab0 100644
|
||||
--- ./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 77361833d..ed7975c35 100644
|
||||
--- ./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 dd5bd4163..ba2fbe404 100644
|
||||
--- ./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 9696e9bab..7dcd9622f 100644
|
||||
--- ./src/openvpn/ssl.c
|
||||
+++ b/src/openvpn/ssl.c
|
||||
@@ -1962,7 +1962,8 @@ tls_session_generate_data_channel_keys(struct tls_session *session)
|
||||
|
||||
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 8066789b6..6672d43fb 100644
|
||||
--- ./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.
|
Loading…
Reference in New Issue
Block a user