1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
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.
|
