From df6653229ad6397ae602444fa1da2718b3058ba3 Mon Sep 17 00:00:00 2001 From: Nikos Mavrogiannopoulos Date: Sun, 30 Mar 2014 15:02:55 +0200 Subject: [PATCH] more cleanups in MTU calculation --- src/worker-vpn.c | 107 +++++++++++++++++++++++------------------------ src/worker.h | 3 +- 2 files changed, 54 insertions(+), 56 deletions(-) diff --git a/src/worker-vpn.c b/src/worker-vpn.c index fffad01f..8635ac4a 100644 --- a/src/worker-vpn.c +++ b/src/worker-vpn.c @@ -70,6 +70,9 @@ /* HTTP requests prior to disconnection */ #define MAX_HTTP_REQUESTS 16 +#define CSTP_DTLS_OVERHEAD 1 +#define CSTP_OVERHEAD 8 + static int terminate = 0; static int parse_cstp_data(struct worker_st *ws, uint8_t * buf, size_t buf_size, time_t); @@ -821,7 +824,7 @@ void mtu_set(worker_st * ws, unsigned mtu) ws->conn_mtu = mtu; if (ws->dtls_session) - gnutls_dtls_set_data_mtu(ws->dtls_session, ws->conn_mtu); + gnutls_dtls_set_data_mtu(ws->dtls_session, ws->conn_mtu+CSTP_DTLS_OVERHEAD); mtu_send(ws, ws->conn_mtu); } @@ -998,9 +1001,6 @@ static void set_net_priority(worker_st * ws, int fd, int priority) return; } -#define CSTP_DTLS_OVERHEAD 1 -#define CSTP_OVERHEAD 8 - #define SEND_ERR(x) if (x<0) goto send_error /* connect_handler: @@ -1019,8 +1019,8 @@ static int connect_handler(worker_st * ws) { struct http_req_st *req = &ws->req; fd_set rfds; - int l, e, max, ret, crypto_overhead, t; - unsigned tls_retry, dtls_mtu, cstp_mtu; + int l, e, max, ret, crypto_overhead = 0, t; + unsigned tls_retry; char *p; #ifdef HAVE_PSELECT struct timespec tv; @@ -1307,6 +1307,7 @@ static int connect_handler(worker_st * ws) SEND_ERR(ret); } + /* calculate base MTU */ if (ws->config->default_mtu > 0) { ws->vinfo.mtu = ws->config->default_mtu; } @@ -1316,11 +1317,6 @@ static int connect_handler(worker_st * ws) ws->vinfo.mtu = MIN(ws->vinfo.mtu, req->base_mtu); } - proto_overhead = CSTP_OVERHEAD; - /* plaintext MTU is the device MTU minus the overhead - * of the CSTP protocol. */ - ws->conn_mtu = ws->vinfo.mtu - proto_overhead; - sl = sizeof(max); ret = getsockopt(ws->conn_fd, IPPROTO_TCP, TCP_MAXSEG, &max, &sl); if (ret == -1) { @@ -1330,14 +1326,28 @@ static int connect_handler(worker_st * ws) } else { max -= 13; oclog(ws, LOG_DEBUG, "TCP MSS is %u", max); - if (max > 0 && max - proto_overhead < ws->conn_mtu) { + if (max > 0 && max < ws->vinfo.mtu) { oclog(ws, LOG_DEBUG, - "reducing MTU due to TCP MSS to %u", - max - proto_overhead); + "reducing MTU due to TCP MSS to %u", max); + ws->vinfo.mtu = max; } - ws->conn_mtu = MIN(ws->conn_mtu, max - proto_overhead); } + ret = tls_printf(ws->session, "X-CSTP-Base-MTU: %u\r\n", ws->vinfo.mtu); + SEND_ERR(ret); + oclog(ws, LOG_DEBUG, "CSTP Base MTU is %u bytes", ws->vinfo.mtu); + + /* calculate TLS channel MTU */ + crypto_overhead = CSTP_OVERHEAD + + tls_get_overhead(gnutls_protocol_get_version(ws->session), + gnutls_cipher_get(ws->session), + gnutls_mac_get(ws->session)); + + /* plaintext MTU is the device MTU minus the overhead + * of the CSTP protocol. */ + ws->conn_mtu = ws->vinfo.mtu - crypto_overhead; + + /* set TCP socket options */ if (ws->config->output_buffer > 0) { t = ws->conn_mtu * ws->config->output_buffer; @@ -1401,16 +1411,16 @@ static int connect_handler(worker_st * ws) /* assume that if IPv6 is used over TCP then the same would be used over UDP */ if (ws->proto == AF_INET) - proto_overhead = 20 + CSTP_DTLS_OVERHEAD; /* ip */ + proto_overhead = 20; /* ip */ else - proto_overhead = 40 + CSTP_DTLS_OVERHEAD; /* ipv6 */ + proto_overhead = 40; /* ipv6 */ proto_overhead += 8; /* udp */ - proto_overhead += CSTP_DTLS_OVERHEAD; /* crypto overhead for DTLS */ crypto_overhead = tls_get_overhead(ws->req.selected_ciphersuite->gnutls_version, ws->req.selected_ciphersuite->gnutls_cipher, ws->req.selected_ciphersuite->gnutls_mac); + crypto_overhead += CSTP_DTLS_OVERHEAD; oclog(ws, LOG_DEBUG, "DTLS overhead is %u", proto_overhead+crypto_overhead); @@ -1418,15 +1428,13 @@ static int connect_handler(worker_st * ws) /* plaintext MTU is the device MTU minus the overhead * of the DTLS (+AnyConnect header) protocol. */ - ws->conn_mtu = MIN(ws->conn_mtu, ws->vinfo.mtu - proto_overhead); + ws->conn_mtu = MIN(ws->conn_mtu, ws->vinfo.mtu - proto_overhead - crypto_overhead); - dtls_mtu = ws->conn_mtu - crypto_overhead; - - tls_printf(ws->session, "X-DTLS-MTU: %u\r\n", dtls_mtu); - oclog(ws, LOG_DEBUG, "suggesting DTLS MTU %u", dtls_mtu); + tls_printf(ws->session, "X-DTLS-MTU: %u\r\n", ws->conn_mtu); + oclog(ws, LOG_DEBUG, "suggesting DTLS MTU %u", ws->conn_mtu); if (ws->config->output_buffer > 0) { - t = MIN(2048, dtls_mtu * ws->config->output_buffer); + t = MIN(2048, ws->conn_mtu * ws->config->output_buffer); setsockopt(ws->udp_fd, SOL_SOCKET, SO_SNDBUF, &t, sizeof(t)); if (ret == -1) @@ -1435,36 +1443,22 @@ static int connect_handler(worker_st * ws) } set_net_priority(ws, ws->udp_fd, ws->config->net_priority); - } else - dtls_mtu = 0; + } - if (ws->buffer_size <= ws->conn_mtu + proto_overhead) { + /* hack for openconnect. It uses only a single MTU value */ + ret = tls_printf(ws->session, "X-CSTP-MTU: %u\r\n", ws->conn_mtu); + SEND_ERR(ret); + + if (ws->buffer_size <= ws->conn_mtu + CSTP_OVERHEAD) { oclog(ws, LOG_WARNING, "buffer size is smaller than MTU (%u < %u); adjusting", ws->buffer_size, ws->conn_mtu); - ws->buffer_size = ws->conn_mtu + proto_overhead; + ws->buffer_size = ws->conn_mtu + CSTP_OVERHEAD; ws->buffer = safe_realloc(ws->buffer, ws->buffer_size); if (ws->buffer == NULL) goto exit; } - /* crypto overhead for TLS */ - crypto_overhead = - tls_get_overhead(gnutls_protocol_get_version(ws->session), - gnutls_cipher_get(ws->session), - gnutls_mac_get(ws->session)); - cstp_mtu = ws->conn_mtu - crypto_overhead; - if (dtls_mtu > 0) /* this is a hack for openconnect which reads a single MTU value */ - cstp_mtu = MIN(cstp_mtu, dtls_mtu); - - ret = tls_printf(ws->session, "X-CSTP-MTU: %u\r\n", cstp_mtu); - SEND_ERR(ret); - - ret = tls_printf(ws->session, "X-CSTP-Base-MTU: %u\r\n", ws->conn_mtu); - SEND_ERR(ret); - - oclog(ws, LOG_DEBUG, "CSTP Base MTU is %u bytes", ws->conn_mtu); - mtu_send(ws, ws->conn_mtu); if (ws->config->banner) { @@ -1547,11 +1541,12 @@ static int connect_handler(worker_st * ws) gettime(&tnow); now = tnow.tv_sec; - if (periodic_check(ws, proto_overhead, now, ws->config->dpd) < 0) + if (periodic_check(ws, proto_overhead+crypto_overhead, now, ws->config->dpd) < 0) goto exit; + /* send pending data from tun device */ if (FD_ISSET(ws->tun_fd, &rfds)) { - l = read(ws->tun_fd, ws->buffer + 8, ws->conn_mtu - 1); + l = read(ws->tun_fd, ws->buffer + 8, ws->conn_mtu); if (l < 0) { e = errno; @@ -1570,7 +1565,7 @@ static int connect_handler(worker_st * ws) } /* only transmit if allowed */ - if (bandwidth_update(&b_tx, l - 1, ws->conn_mtu, &tnow) + if (bandwidth_update(&b_tx, l, ws->conn_mtu, &tnow) != 0) { tls_retry = 0; oclog(ws, LOG_TRANSFER_DEBUG, "sending %d byte(s)\n", l); @@ -1590,9 +1585,8 @@ static int connect_handler(worker_st * ws) oclog(ws, LOG_TRANSFER_DEBUG, "retrying (TLS) %d\n", l); tls_retry = 1; - } else if (ret >= ws->conn_mtu - && ws->config->try_mtu != - 0) { + } else if (ret >= ws->conn_mtu && + ws->config->try_mtu != 0) { mtu_ok(ws); } } @@ -1617,6 +1611,7 @@ static int connect_handler(worker_st * ws) } } + /* read pending data from network */ if (FD_ISSET(ws->conn_fd, &rfds) || tls_pending != 0) { ret = tls_recv_nb(ws->session, ws->buffer, @@ -1740,7 +1735,7 @@ static int connect_handler(worker_st * ws) goto exit; gnutls_dtls_set_mtu(ws->dtls_session, - ws->conn_mtu); + ws->conn_mtu + crypto_overhead); mtu_discovery_init(ws, ws->conn_mtu); break; @@ -1773,9 +1768,11 @@ static int connect_handler(worker_st * ws) } if (ret == 0) { - unsigned mtu = - gnutls_dtls_get_data_mtu(ws-> - dtls_session) - proto_overhead; + unsigned mtu; + + /* gnutls_dtls_get_data_mtu() already subtracts the crypto overhead */ + mtu = gnutls_dtls_get_data_mtu(ws-> + dtls_session) - CSTP_DTLS_OVERHEAD; /* openconnect doesn't like if we send more bytes * than the initially agreed MTU */ diff --git a/src/worker.h b/src/worker.h index d4ed9fdc..da261a26 100644 --- a/src/worker.h +++ b/src/worker.h @@ -173,7 +173,8 @@ typedef struct worker_st { /* ws->conn_mtu: The MTU of the plaintext data we can send to the client. * It also matches the MTU of the TUN device. Note that this is - * the same as the 'real' MTU of the connection, minus the IP+UDP+CSTP headers. */ + * the same as the 'real' MTU of the connection, minus the IP+UDP+CSTP headers + * and the DTLS crypto overhead. */ unsigned conn_mtu; /* Indicates whether the new IPv6 headers will