Attention is currently required from: flichtenheld, its_Giaan, plaisthos. Hello flichtenheld, plaisthos,
I'd like you to reexamine a change. Please visit http://gerrit.openvpn.net/c/openvpn/+/587?usp=email to look at the new patch set (#3). The following approvals got outdated and were removed: Code-Review-1 by flichtenheld Change subject: Ensures all params are ready before invoking dco_set_peer() ...................................................................... Ensures all params are ready before invoking dco_set_peer() In UDP case the dco_set_peer() is currently perfomed at the wrong time since the mssfix param is calculated later on in tls_session_update_crypto_params_do_work(). By moving the dco_set_peer() inside the tls_session_update_crypto_params_do_work() we will 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 <gianma...@mandelbit.com> --- M src/openvpn/init.c M src/openvpn/multi.c M src/openvpn/ssl.c M src/openvpn/ssl.h 4 files changed, 35 insertions(+), 22 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/87/587/3 diff --git a/src/openvpn/init.c b/src/openvpn/init.c index a49e563..0999f4d 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -2359,7 +2359,8 @@ 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; @@ -2578,7 +2579,8 @@ 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 03177bb..0509911 100644 --- a/src/openvpn/multi.c +++ b/src/openvpn/multi.c @@ -2364,21 +2364,6 @@ 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 @@ 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 e0e9591..50689e9 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -1584,7 +1584,8 @@ 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) { @@ -1631,6 +1632,26 @@ return false; } } + + if (dco_enabled(options)) + { + /* dco_set_peer() must be called only when both + * keepalive and mss_fix are properly setted. */ + 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); } @@ -1639,7 +1660,8 @@ 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)) { @@ -1650,7 +1672,7 @@ 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 1a45048..0e43961 100644 --- a/src/openvpn/ssl.h +++ b/src/openvpn/ssl.h @@ -457,6 +457,8 @@ * @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. */ @@ -465,7 +467,8 @@ 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 -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/587?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: Ic8538e734dba53cd43fead3961e4401c8037e079 Gerrit-Change-Number: 587 Gerrit-PatchSet: 3 Gerrit-Owner: its_Giaan <gianma...@mandelbit.com> Gerrit-Reviewer: flichtenheld <fr...@lichtenheld.com> Gerrit-Reviewer: plaisthos <arne-open...@rfc2549.org> Gerrit-CC: openvpn-devel <openvpn-devel@lists.sourceforge.net> Gerrit-Attention: plaisthos <arne-open...@rfc2549.org> Gerrit-Attention: its_Giaan <gianma...@mandelbit.com> Gerrit-Attention: flichtenheld <fr...@lichtenheld.com> Gerrit-MessageType: newpatchset
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel