From: Gianmarco De Gregori Date: Fri, 6 Sep 2024 14:57:45 +0000 (+0200) Subject: Ensures all params are ready before invoking dco_set_peer() X-Git-Tag: v2.7_alpha1~210 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=32e748bd3320cd07b9e7671ee0cec95f4fd85935;p=thirdparty%2Fopenvpn.git Ensures all params are ready before invoking dco_set_peer() In UDP case, on a p2mp server, dco_set_peer() is currently called at the wrong time since the mssfix param is calculated later on in tls_session_update_crypto_params_do_work(). Move the dco_set_peer() inside tls_session_update_crypto_params_do_work(), and remove p2p_set_dco_keepalive() to avoid calling dco_set_peer() twice on the client side. This way, we'll ensure that all crypto and frame params are properly initialized and if an update occurs DCO will be notified. Change-Id: Ic8538e734dba53cd43fead3961e4401c8037e079 Signed-off-by: Gianmarco De Gregori Acked-by: Lev Stipakov Message-Id: <20240906145745.67596-1-frank@lichtenheld.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg29086.html Signed-off-by: Gert Doering --- diff --git a/src/openvpn/init.c b/src/openvpn/init.c index 4f6334680..dd56961c1 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -2178,27 +2178,6 @@ options_hash_changed_or_zero(const struct sha256_digest *a, || !memcmp(a, &zero, sizeof(struct sha256_digest)); } -static bool -p2p_set_dco_keepalive(struct context *c) -{ - 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->dco_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 parameters for DCO peer (id=%u): %s", - c->c2.tls_multi->dco_peer_id, strerror(-ret)); - return false; - } - } - return true; -} - /** * Helper function for tls_print_deferred_options_results * Adds the ", " delimitor if there already some data in the @@ -2363,7 +2342,8 @@ do_deferred_options_part2(struct context *c) if (!tls_session_update_crypto_params(c->c2.tls_multi, session, &c->options, &c->c2.frame, frame_fragment, - get_link_socket_info(c))) + get_link_socket_info(c), + &c->c1.tuntap->dco)) { msg(D_TLS_ERRORS, "OPTIONS ERROR: failed to import crypto options"); return false; @@ -2472,12 +2452,6 @@ do_up(struct context *c, bool pulled_options, unsigned int option_types_found) } } - if (c->mode == MODE_POINT_TO_POINT && !p2p_set_dco_keepalive(c)) - { - msg(D_TLS_ERRORS, "ERROR: Failed to apply DCO keepalive or MSS fix parameters"); - return false; - } - if (c->c2.did_open_tun) { c->c1.pulled_options_digest_save = c->c2.pulled_options_digest; @@ -2582,7 +2556,8 @@ do_deferred_p2p_ncp(struct context *c) if (!tls_session_update_crypto_params(c->c2.tls_multi, session, &c->options, &c->c2.frame, frame_fragment, - get_link_socket_info(c))) + get_link_socket_info(c), + &c->c1.tuntap->dco)) { msg(D_TLS_ERRORS, "ERROR: failed to set crypto cipher"); return false; diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c index 03177bb8b..0509911d6 100644 --- a/src/openvpn/multi.c +++ b/src/openvpn/multi.c @@ -2364,21 +2364,6 @@ multi_client_setup_dco_initial(struct multi_context *m, return false; } - if (mi->context.options.ping_send_timeout || mi->context.c2.frame.mss_fix) - { - ret = dco_set_peer(&mi->context.c1.tuntap->dco, - mi->context.c2.tls_multi->dco_peer_id, - mi->context.options.ping_send_timeout, - mi->context.options.ping_rec_timeout, - mi->context.c2.frame.mss_fix); - if (ret < 0) - { - msg(D_DCO, "Cannot set DCO peer parameters for %s (id=%u): %s", - multi_instance_string(mi, false, gc), - mi->context.c2.tls_multi->dco_peer_id, strerror(-ret)); - return false; - } - } return true; } @@ -2398,7 +2383,8 @@ multi_client_generate_tls_keys(struct context *c) 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))) + get_link_socket_info(c), + &c->c1.tuntap->dco)) { msg(D_TLS_ERRORS, "TLS Error: initializing data channel failed"); register_signal(c->sig, SIGUSR1, "process-push-msg-failed"); diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index 8c72d0b32..e2be61400 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -1591,7 +1591,8 @@ tls_session_update_crypto_params_do_work(struct tls_multi *multi, struct options *options, struct frame *frame, struct frame *frame_fragment, - struct link_socket_info *lsi) + struct link_socket_info *lsi, + dco_context_t *dco) { if (session->key[KS_PRIMARY].crypto_options.key_ctx_bi.initialized) { @@ -1638,6 +1639,25 @@ tls_session_update_crypto_params_do_work(struct tls_multi *multi, return false; } } + + if (dco_enabled(options)) + { + /* dco_set_peer() must be called if either keepalive or + * mssfix are set to update in-kernel config */ + if (options->ping_send_timeout || frame->mss_fix) + { + int ret = dco_set_peer(dco, multi->dco_peer_id, + options->ping_send_timeout, + options->ping_rec_timeout, + frame->mss_fix); + if (ret < 0) + { + msg(D_DCO, "Cannot set DCO peer parameters for peer (id=%u): %s", + multi->dco_peer_id, strerror(-ret)); + return false; + } + } + } return tls_session_generate_data_channel_keys(multi, session); } @@ -1646,7 +1666,8 @@ tls_session_update_crypto_params(struct tls_multi *multi, struct tls_session *session, struct options *options, struct frame *frame, struct frame *frame_fragment, - struct link_socket_info *lsi) + struct link_socket_info *lsi, + dco_context_t *dco) { if (!check_session_cipher(session, options)) { @@ -1657,7 +1678,7 @@ tls_session_update_crypto_params(struct tls_multi *multi, session->opt->crypto_flags |= options->imported_protocol_flags; return tls_session_update_crypto_params_do_work(multi, session, options, - frame, frame_fragment, lsi); + frame, frame_fragment, lsi, dco); } diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h index 6c2bfc322..eea13234b 100644 --- a/src/openvpn/ssl.h +++ b/src/openvpn/ssl.h @@ -460,6 +460,8 @@ void tls_update_remote_addr(struct tls_multi *multi, * @param frame_fragment The fragment frame options. * @param lsi link socket info to adjust MTU related options * depending on the current protocol + * @param dco The dco context to perform dco_set_peer() + * whenever a crypto param update occurs. * * @return true if updating succeeded or keys are already generated, false otherwise. */ @@ -468,7 +470,8 @@ bool tls_session_update_crypto_params(struct tls_multi *multi, struct options *options, struct frame *frame, struct frame *frame_fragment, - struct link_socket_info *lsi); + struct link_socket_info *lsi, + dco_context_t *dco); /* * inline functions