On 29 Apr 2022, at 19:02, Kristof Provost wrote:
From: Kristof Provost <k...@freebsd.org>
We must create the peer before we can dco_set_peer or dco_new_key.
On the other hand, we must first process options, because those may
change our peer id and we should create the peer with the correct id.
Split up do_deferred_options() in do_deferred_options() and
finish_options(). Call any DCO configuration operations (i.e.
dco_set_peer()/dco_new_key()) after we've created the peer (i.e.
dco_new_peer()).
Signed-off-by: Kristof Provost <kprov...@netgate.com>
---
src/openvpn/init.c | 112
+++++++++++++++++++++++++-------------------
src/openvpn/init.h | 2 +
src/openvpn/multi.c | 2 +
3 files changed, 68 insertions(+), 48 deletions(-)
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 8496e21f..5d53cf7e 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -2100,26 +2100,26 @@ do_up(struct context *c, bool pulled_options,
unsigned int option_types_found)
}
}
- if ((c->mode == MODE_POINT_TO_POINT) && c->c2.did_open_tun)
+ if (pulled_options)
{
- /* If we are in DCO mode we need to set the new peer
options now */
- int ret = dco_p2p_add_new_peer(c);
- if (ret < 0)
+ if (!do_deferred_options(c, option_types_found))
{
- msg(D_DCO, "Cannot add peer to DCO: %s",
strerror(-ret));
+ msg(D_PUSH_ERRORS, "ERROR: Failed to apply push
options");
return false;
}
}
-
- if (pulled_options)
+ if (c->mode == MODE_POINT_TO_POINT)
{
- if (!do_deferred_options(c, option_types_found))
+ /* If we are in DCO mode we need to set the new peer
options now */
+ int ret = dco_p2p_add_new_peer(c);
+ if (ret < 0)
{
- msg(D_PUSH_ERRORS, "ERROR: Failed to apply push
options");
+ msg(D_DCO, "Cannot add peer to DCO: %s",
strerror(-ret));
return false;
}
}
- else if (c->mode == MODE_POINT_TO_POINT)
+
+ if (!pulled_options && c->mode == MODE_POINT_TO_POINT)
{
if (!do_deferred_p2p_ncp(c))
{
@@ -2128,6 +2128,13 @@ do_up(struct context *c, bool pulled_options,
unsigned int option_types_found)
}
}
+
+ if (!finish_options(c))
+ {
+ msg(D_TLS_ERRORS, "ERROR: Failed to finish option
processing");
+ return false;
+ }
+
if (c->c2.did_open_tun)
{
c->c1.pulled_options_digest_save =
c->c2.pulled_options_digest;
@@ -2344,49 +2351,58 @@ do_deferred_options(struct context *c, const
unsigned int found)
{
return false;
}
- struct frame *frame_fragment = NULL;
+ }
+
+ return true;
+}
+
+bool
+finish_options(struct context *c)
+{
+ if (!c->options.pull || !dco_enabled(&c->options))
+ {
+ return true;
+ }
+
+ struct frame *frame_fragment = NULL;
#ifdef ENABLE_FRAGMENT
- if (c->options.ce.fragment)
- {
- frame_fragment = &c->c2.frame_fragment;
- }
+ if (c->options.ce.fragment)
+ {
+ frame_fragment = &c->c2.frame_fragment;
+ }
#endif
- struct tls_session *session =
&c->c2.tls_multi->session[TM_ACTIVE];
- if (!tls_session_update_crypto_params(c->c2.tls_multi,
session,
- &c->options,
&c->c2.frame,
- frame_fragment,
-
get_link_socket_info(c)))
- {
- msg(D_TLS_ERRORS, "OPTIONS ERROR: failed to import crypto
options");
- return false;
- }
+ struct tls_session *session =
&c->c2.tls_multi->session[TM_ACTIVE];
+ if (!tls_session_update_crypto_params(c->c2.tls_multi, session,
+ &c->options, &c->c2.frame,
+ frame_fragment,
+ get_link_socket_info(c)))
+ {
+ msg(D_TLS_ERRORS, "OPTIONS ERROR: failed to import crypto
options");
+ return false;
+ }
- if (dco_enabled(&c->options))
- {
- /* Check if the pushed options are compatible with DCO if
we have
- * DCO enabled */
- if (!check_dco_pull_options(&c->options))
- {
- msg(D_TLS_ERRORS, "OPTIONS ERROR: pushed options are
incompatible with "
- "data channel offload. Use --disable-dco to
connect"
- "to this server");
- return false;
- }
+ /* Check if the pushed options are compatible with DCO if we have
+ * DCO enabled */
+ if (!check_dco_pull_options(&c->options))
+ {
+ msg(D_TLS_ERRORS, "OPTIONS ERROR: pushed options are
incompatible with "
+ "data channel offload. Use --disable-dco to connect"
+ "to this server");
+ return false;
+ }
- if (c->options.ping_send_timeout || c->c2.frame.mss_fix)
- {
- int ret = dco_set_peer(&c->c1.tuntap->dco,
- c->c2.tls_multi->peer_id,
- c->options.ping_send_timeout,
- c->options.ping_rec_timeout,
- c->c2.frame.mss_fix);
- if (ret < 0)
- {
- msg(D_DCO, "Cannot set DCO peer: %s",
strerror(-ret));
- return false;
- }
- }
+ if (c->options.ping_send_timeout || c->c2.frame.mss_fix)
+ {
+ int ret = dco_set_peer(&c->c1.tuntap->dco,
+ c->c2.tls_multi->peer_id,
+ c->options.ping_send_timeout,
+ c->options.ping_rec_timeout,
+ c->c2.frame.mss_fix);
+ if (ret < 0)
+ {
+ msg(D_DCO, "Cannot set DCO peer: %s", strerror(-ret));
+ return false;
}
}
return true;
diff --git a/src/openvpn/init.h b/src/openvpn/init.h
index 1c341da3..5cc2a990 100644
--- a/src/openvpn/init.h
+++ b/src/openvpn/init.h
@@ -97,6 +97,8 @@ void reset_coarse_timers(struct context *c);
bool do_deferred_options(struct context *c, const unsigned int
found);
+bool finish_options(struct context *c);
+
void inherit_context_child(struct context *dest,
const struct context *src);
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 958712f1..47e1c6cc 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -2452,6 +2452,8 @@ multi_client_connect_late_setup(struct
multi_context *m,
mi->context.c2.tls_multi->multi_state = CAS_FAILED;
}
+ finish_options(&mi->context);
+
/* send push reply if ready */
if (mi->context.c2.push_request_received)
{
--
This patch isn’t quite right. It works well for DCO, but breaks
non-DCO. It fails to call tls_session_update_crypto_params() if DCO is
disabled. It needs this on top of it:
commit ee846144a46b20b889ab1966246f789f3266c2f9 (HEAD -> tmp_v10)
Author: Kristof Provost <k...@freebsd.org>
Date: Thu May 5 19:36:13 2022 +0200
Fix finish_options() to perform the required work for non-DCO modes
We still need to run tls_session_update_crypto_params(), even when
DCO
is not enabled.
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 7083b803..ab59cbd7 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -2374,7 +2374,7 @@ do_deferred_options(struct context *c, const
unsigned int found)
bool
finish_options(struct context *c)
{
- if (!c->options.pull || !dco_enabled(&c->options))
+ if (!c->options.pull)
{
return true;
}
@@ -2399,7 +2399,7 @@ finish_options(struct context *c)
/* Check if the pushed options are compatible with DCO if we have
* DCO enabled */
- if (!check_dco_pull_options(&c->options))
+ if (dco_enabled(&c->options) &&
!check_dco_pull_options(&c->options))
{
msg(D_TLS_ERRORS, "OPTIONS ERROR: pushed options are
incompatible with "
"data channel offload. Use --disable-dco to connect"
@@ -2407,7 +2407,7 @@ finish_options(struct context *c)
return false;
}
- if (c->options.ping_send_timeout || c->c2.frame.mss_fix)
+ if (dco_enabled(&c->options) && (c->options.ping_send_timeout ||
c->c2.frame.mss_fix))
{
int ret = dco_set_peer(&c->c1.tuntap->dco,
c->c2.tls_multi->peer_id,
I’ll post an updated series in due course, but wanted to point this
issue out already.
Kristof
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
