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
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
|
From 145110101b70599cb9adcf4ed071856daac9c8af Mon Sep 17 00:00:00 2001
From: Arne Schwabe <arne@rfc2549.org>
Date: Sun, 28 Mar 2021 14:02:40 +0200
Subject: [PATCH] Move context_auth from context_2 to tls_multi and name it
multi_state
context_2 and tls_multi have the same life cycle for TLS connections
but so this move does not affect behaviour of the variable.
OpenVPN TLS multi code has a grown a lot more complex and code that
handles multi objects needs to know the state that the object is in.
Since not all code has access to the context_2 struct, the code that
does not have access is often not checking the state directly but
checks other parts of multi that have been affected from a state
change.
This patch also renames it to multi_state as this variable represents
the multi state machine status rather than just the state of the connect
authentication (more upcoming patches will move other states
into this variable).
Patch V2: also rename context_auth to multi_state, explain a bit why this
change is done.
Patch V3: Add comments for c2->multi NULL check forwarding. Fix compile
with ENABLE_ASYNC_PUSH.
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Antonio Quartulli <antonio@openvpn.net>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20210418160111.1494779-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg22155.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
(backported from commit 0767d5b447044e4cdcfd198058aef1f85f63bbe6)
---
src/openvpn/forward.c | 10 ++++++----
src/openvpn/multi.c | 28 ++++++++++++++--------------
src/openvpn/openvpn.h | 14 --------------
src/openvpn/push.c | 5 +++--
src/openvpn/ssl_common.h | 14 ++++++++++++++
5 files changed, 37 insertions(+), 34 deletions(-)
diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
index 7ed8d0d75..fd7412f73 100644
--- a/src/openvpn/forward.c
+++ b/src/openvpn/forward.c
@@ -526,9 +526,10 @@ encrypt_sign(struct context *c, bool comp_frag)
/*
* Drop non-TLS outgoing packet if client-connect script/plugin
- * has not yet succeeded.
+ * has not yet succeeded. In non-TLS mode tls_multi is not defined
+ * and we always pass packets.
*/
- if (c->c2.context_auth != CAS_SUCCEEDED)
+ if (c->c2.tls_multi && c->c2.tls_multi->multi_state != CAS_SUCCEEDED)
{
c->c2.buf.len = 0;
}
@@ -973,9 +974,10 @@ process_incoming_link_part1(struct context *c, struct link_socket_info *lsi, boo
/*
* Drop non-TLS packet if client-connect script/plugin and cipher selection
- * has not yet succeeded.
+ * has not yet succeeded. In non-TLS mode tls_multi is not defined
+ * and we always pass packets.
*/
- if (c->c2.context_auth != CAS_SUCCEEDED)
+ if (c->c2.tls_multi && c->c2.tls_multi->multi_state != CAS_SUCCEEDED)
{
c->c2.buf.len = 0;
}
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 137381805..599ffd86d 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -678,7 +678,7 @@ multi_close_instance(struct multi_context *m,
#ifdef MANAGEMENT_DEF_AUTH
set_cc_config(mi, NULL);
#endif
- if (mi->context.c2.context_auth == CAS_SUCCEEDED)
+ if (mi->context.c2.tls_multi->multi_state == CAS_SUCCEEDED)
{
multi_client_disconnect_script(mi);
}
@@ -788,7 +788,7 @@ multi_create_instance(struct multi_context *m, const struct mroute_addr *real)
goto err;
}
- mi->context.c2.context_auth = CAS_PENDING;
+ mi->context.c2.tls_multi->multi_state = CAS_PENDING;
if (hash_n_elements(m->hash) >= m->max_clients)
{
@@ -2436,18 +2436,18 @@ multi_client_connect_late_setup(struct multi_context *m,
mi->reporting_addr_ipv6 = mi->context.c2.push_ifconfig_ipv6_local;
/* set context-level authentication flag */
- mi->context.c2.context_auth = CAS_SUCCEEDED;
+ mi->context.c2.tls_multi->multi_state = CAS_SUCCEEDED;
/* authentication complete, calculate dynamic client specific options */
if (!multi_client_set_protocol_options(&mi->context))
{
- mi->context.c2.context_auth = CAS_FAILED;
+ mi->context.c2.tls_multi->multi_state = CAS_FAILED;
}
/* Generate data channel keys only if setting protocol options
* has not failed */
else if (!multi_client_generate_tls_keys(&mi->context))
{
- mi->context.c2.context_auth = CAS_FAILED;
+ mi->context.c2.tls_multi->multi_state = CAS_FAILED;
}
/* send push reply if ready */
@@ -2595,7 +2595,7 @@ multi_connection_established(struct multi_context *m, struct multi_instance *mi)
/* We are only called for the CAS_PENDING_x states, so we
* can ignore other states here */
- bool from_deferred = (mi->context.c2.context_auth != CAS_PENDING);
+ bool from_deferred = (mi->context.c2.tls_multi->multi_state != CAS_PENDING);
int *cur_handler_index = &mi->client_connect_defer_state.cur_handler_index;
unsigned int *option_types_found =
@@ -2607,7 +2607,7 @@ multi_connection_established(struct multi_context *m, struct multi_instance *mi)
*cur_handler_index = 0;
*option_types_found = 0;
/* Initially we have no handler that has returned a result */
- mi->context.c2.context_auth = CAS_PENDING_DEFERRED;
+ mi->context.c2.tls_multi->multi_state = CAS_PENDING_DEFERRED;
multi_client_connect_early_setup(m, mi);
}
@@ -2630,7 +2630,7 @@ multi_connection_established(struct multi_context *m, struct multi_instance *mi)
* Remember that we already had at least one handler
* returning a result should we go to into deferred state
*/
- mi->context.c2.context_auth = CAS_PENDING_DEFERRED_PARTIAL;
+ mi->context.c2.tls_multi->multi_state = CAS_PENDING_DEFERRED_PARTIAL;
break;
case CC_RET_SKIPPED:
@@ -2682,12 +2682,12 @@ multi_connection_established(struct multi_context *m, struct multi_instance *mi)
{
/* run the disconnect script if we had a connect script that
* did not fail */
- if (mi->context.c2.context_auth == CAS_PENDING_DEFERRED_PARTIAL)
+ if (mi->context.c2.tls_multi->multi_state == CAS_PENDING_DEFERRED_PARTIAL)
{
multi_client_disconnect_script(mi);
}
- mi->context.c2.context_auth = CAS_FAILED;
+ mi->context.c2.tls_multi->multi_state = CAS_FAILED;
}
/* increment number of current authenticated clients */
@@ -2990,13 +2990,13 @@ multi_process_post(struct multi_context *m, struct multi_instance *mi, const uns
{
/* connection is "established" when SSL/TLS key negotiation succeeds
* and (if specified) auth user/pass succeeds */
- if (is_cas_pending(mi->context.c2.context_auth)
+ if (is_cas_pending(mi->context.c2.tls_multi->multi_state)
&& CONNECTION_ESTABLISHED(&mi->context))
{
multi_connection_established(m, mi);
}
#if defined(ENABLE_ASYNC_PUSH) && defined(ENABLE_DEF_AUTH)
- if (is_cas_pending(mi->context.c2.context_auth)
+ if (is_cas_pending(mi->context.c2.tls_multi->multi_state)
&& mi->client_connect_defer_state.deferred_ret_file)
{
add_inotify_file_watch(m, mi, m->top.c2.inotify_fd,
@@ -3953,7 +3953,7 @@ management_client_auth(void *arg,
{
if (auth)
{
- if (is_cas_pending(mi->context.c2.context_auth))
+ if (is_cas_pending(mi->context.c2.tls_multi->multi_state))
{
set_cc_config(mi, cc_config);
cc_config_owned = false;
@@ -3965,7 +3965,7 @@ management_client_auth(void *arg,
{
msg(D_MULTI_LOW, "MULTI: connection rejected: %s, CLI:%s", reason, np(client_reason));
}
- if (!is_cas_pending(mi->context.c2.context_auth))
+ if (!is_cas_pending(mi->context.c2.tls_multi->multi_state))
{
send_auth_failed(&mi->context, client_reason); /* mid-session reauth failed */
multi_schedule_context_wakeup(m, mi);
diff --git a/src/openvpn/openvpn.h b/src/openvpn/openvpn.h
index a7b597749..d131ac59e 100644
--- a/src/openvpn/openvpn.h
+++ b/src/openvpn/openvpn.h
@@ -211,17 +211,6 @@ struct context_1
};
-/* client authentication state, CAS_SUCCEEDED must be 0 since
- * non multi code path still checks this variable but does not initialise it
- * so the code depends on zero initialisation */
-enum client_connect_status {
- CAS_SUCCEEDED=0,
- CAS_PENDING,
- CAS_PENDING_DEFERRED,
- CAS_PENDING_DEFERRED_PARTIAL, /**< at least handler succeeded, no result yet*/
- CAS_FAILED,
-};
-
static inline bool
is_cas_pending(enum client_connect_status cas)
{
@@ -458,9 +447,6 @@ struct context_2
int push_ifconfig_ipv6_netbits;
struct in6_addr push_ifconfig_ipv6_remote;
-
- enum client_connect_status context_auth;
-
struct event_timeout push_request_interval;
int n_sent_push_requests;
bool did_pre_pull_restore;
diff --git a/src/openvpn/push.c b/src/openvpn/push.c
index e0d2eeaf2..c47f4c8b6 100644
--- a/src/openvpn/push.c
+++ b/src/openvpn/push.c
@@ -733,13 +733,14 @@ process_incoming_push_request(struct context *c)
{
int ret = PUSH_MSG_ERROR;
- if (tls_authentication_status(c->c2.tls_multi, 0) == TLS_AUTHENTICATION_FAILED || c->c2.context_auth == CAS_FAILED)
+ if (tls_authentication_status(c->c2.tls_multi, 0) == TLS_AUTHENTICATION_FAILED
+ || c->c2.tls_multi->multi_state == CAS_FAILED)
{
const char *client_reason = tls_client_reason(c->c2.tls_multi);
send_auth_failed(c, client_reason);
ret = PUSH_MSG_AUTH_FAILURE;
}
- else if (c->c2.context_auth == CAS_SUCCEEDED)
+ else if (c->c2.tls_multi->multi_state == CAS_SUCCEEDED)
{
time_t now;
diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h
index a703f97cd..06c32ac1d 100644
--- a/src/openvpn/ssl_common.h
+++ b/src/openvpn/ssl_common.h
@@ -479,6 +479,19 @@ struct tls_session
*/
#define KEY_SCAN_SIZE 3
+
+/* client authentication state, CAS_SUCCEEDED must be 0 since
+ * non multi code path still checks this variable but does not initialise it
+ * so the code depends on zero initialisation */
+enum client_connect_status {
+ CAS_SUCCEEDED=0,
+ CAS_PENDING,
+ CAS_PENDING_DEFERRED,
+ CAS_PENDING_DEFERRED_PARTIAL, /**< at least handler succeeded, no result yet*/
+ CAS_FAILED,
+};
+
+
/**
* Security parameter state for a single VPN tunnel.
* @ingroup control_processor
@@ -519,6 +532,7 @@ struct tls_multi
int n_sessions; /**< Number of sessions negotiated thus
* far. */
+ enum client_connect_status multi_state;
/*
* Number of errors.
|
