Ticket #1140: 0001-Fix-broken-fragment-mssfix-with-NCP.patch

File 0001-Fix-broken-fragment-mssfix-with-NCP.patch, 4.9 KB (added by stipa, 5 years ago)

  • src/openvpn/init.c 

    From b1cd3682a8699eff2e31ea026479a5877e67a3e1 Mon Sep 17 00:00:00 2001
From: Lev Stipakov <lev@openvpn.net>
Date: Fri, 9 Nov 2018 16:17:23 +0200
Subject: [PATCH] Fix broken fragment/mssfix with NCP

The problem seems to be caused by fragment_frame holding
incorrect "extra_frame" value. Initially it is based on
max crypto overhead. After NCP, we adjust extra_frame
value in frame and use it in mss_fixup_dowork().

However resulting packet size exceeds fragment size,
which is calculated with incorrect fragment_frame->extra_frame.
Because fragment size is exceeded, and we perform
unneeded fragmentation.

As a fix, fragment_frame->frame_extra should be
also updated on NCP negotiation.

Signed-off-by: Lev Stipakov <lstipakov@gmail.com>
---
 src/openvpn/init.c | 11 ++++++++++-
 src/openvpn/push.c |  9 ++++++++-
 src/openvpn/ssl.c  | 12 +++++++++++-
 src/openvpn/ssl.h  |  4 +++-
 4 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 39e8ca5f..a5b27589 100644
    a b do_deferred_options(struct context *c, const unsigned int found) 
    23002300        {
    23012301            tls_poor_mans_ncp(&c->options, c->c2.tls_multi->remote_ciphername);
    23022302        }
     2303        struct frame *frame_fragment = NULL;
     2304#ifdef ENABLE_FRAGMENT
     2305        if (c->options.ce.fragment)
     2306        {
     2307            frame_fragment = &c->c2.frame_fragment;
     2308        }
     2309#endif
     2310
    23032311        /* Do not regenerate keys if server sends an extra push reply */
    23042312        if (!session->key[KS_PRIMARY].crypto_options.key_ctx_bi.initialized
    2305             && !tls_session_update_crypto_params(session, &c->options, &c->c2.frame))
     2313            && !tls_session_update_crypto_params(session, &c->options, &c->c2.frame,
     2314                                                 frame_fragment))
    23062315        {
    23072316            msg(D_TLS_ERRORS, "OPTIONS ERROR: failed to import crypto options");
    23082317            return false;

  • src/openvpn/push.c 

    diff --git a/src/openvpn/push.c b/src/openvpn/push.c
index 72f09962..a44646a5 100644
    a b incoming_push_message(struct context *c, const struct buffer *buffer) 
    274274    {
    275275        if (c->options.mode == MODE_SERVER)
    276276        {
     277            struct frame *frame_fragment = NULL;
     278#ifdef ENABLE_FRAGMENT
     279            if (c->options.ce.fragment)
     280            {
     281                frame_fragment = &c->c2.frame_fragment;
     282            }
     283#endif
    277284            struct tls_session *session = &c->c2.tls_multi->session[TM_ACTIVE];
    278285            /* Do not regenerate keys if client send a second push request */
    279286            if (!session->key[KS_PRIMARY].crypto_options.key_ctx_bi.initialized
    280287                && !tls_session_update_crypto_params(session, &c->options,
    281                                                      &c->c2.frame))
     288                                                     &c->c2.frame, frame_fragment))
    282289            {
    283290                msg(D_TLS_ERRORS, "TLS Error: initializing data channel failed");
    284291                goto error;

  • src/openvpn/ssl.c 

    diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 74b88ce6..4b912977 100644
    a b cleanup: 
    19861986
    19871987bool
    19881988tls_session_update_crypto_params(struct tls_session *session,
    1989                                  struct options *options, struct frame *frame)
     1989                                 struct options *options, struct frame *frame,
     1990                                 struct frame *frame_fragment)
    19901991{
    19911992    if (!session->opt->server
    19921993        && 0 != strcmp(options->ciphername, session->opt->config_ciphername)
    … … tls_session_update_crypto_params(struct tls_session *session, 
    20302031    frame_init_mssfix(frame, options);
    20312032    frame_print(frame, D_MTU_INFO, "Data Channel MTU parms");
    20322033
     2034    if (frame_fragment)
     2035    {
     2036        frame_remove_from_extra_frame(frame_fragment, crypto_max_overhead());
     2037        crypto_adjust_frame_parameters(frame_fragment, &session->opt->key_type,
     2038                                       options->replay, packet_id_long_form);
     2039        frame_set_mtu_dynamic(frame_fragment, options->ce.fragment, SET_MTU_UPPER_BOUND);
     2040        frame_print(frame_fragment, D_MTU_INFO, "Fragmentation MTU parms");
     2041    }
     2042
    20332043    return tls_session_generate_data_channel_keys(session);
    20342044}
    20352045

  • src/openvpn/ssl.h 

    diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h
index ac6fef75..db9ea610 100644
    a b void tls_update_remote_addr(struct tls_multi *multi, 
    482482 * @return true if updating succeeded, false otherwise.
    483483 */
    484484bool tls_session_update_crypto_params(struct tls_session *session,
    485                                       struct options *options, struct frame *frame);
     485                                      struct options *options,
     486                                      struct frame *frame,
     487                                      struct frame *frame_fragment);
    486488
    487489/**
    488490 * "Poor man's NCP": Use peer cipher if it is an allowed (NCP) cipher.