From d22ba6b2c551fa83d23b5cf668e08a08fde446bc Mon Sep 17 00:00:00 2001 From: Lev Stipakov Date: Mon, 21 Jan 2019 22:04:54 +0200 Subject: Fix broken fragment/mssfix with NCP NCP negotiation replaces worst cast crypto overhead with actual one in data channel frame. That frame params are used by mssfix. Fragment frame still contains worst case overhead. Because of that TCP packets are fragmented, since MSS value exceeds max fragment size. Fix by replacing worst case crypto overhead with actual one for fragment frame, as it is done for data channel frame. Trac #1140 Signed-off-by: Lev Stipakov Acked-by: Arne Schwabe Acked-by: Steffan Karger Message-Id: <1548101094-4449-1-git-send-email-lstipakov@gmail.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg18135.html Signed-off-by: Gert Doering --- 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(-) (limited to 'src') diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c index 35df089..c2dcb53 100644 --- a/src/openvpn/forward.c +++ b/src/openvpn/forward.c @@ -1093,6 +1093,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 5ccf4d9..b5a034d 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -2333,9 +2333,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; @@ -3118,6 +3127,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 fca33f2..29d21f0 100644 --- a/src/openvpn/openvpn.h +++ b/src/openvpn/openvpn.h @@ -265,6 +265,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 8befc6f..248c2e4 100644 --- a/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 640808f..abc3c53 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -1990,7 +1990,8 @@ cleanup: 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) @@ -2034,6 +2035,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->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 660e9eb..e98c54c 100644 --- a/src/openvpn/ssl.h +++ b/src/openvpn/ssl.h @@ -476,15 +476,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. -- cgit v1.1