From e2ee619fa2b805fb0bd205e4d1965767b324ad8c Mon Sep 17 00:00:00 2001 From: David Woodhouse Date: Sat, 30 Jun 2012 01:41:59 +0100 Subject: [PATCH] Separate requested from received MTU settings This fixes a bug where an MTU requested with the --mtu option will actually be set as the interface MTU even if the server replies with a smaller value. It also fixes reconnect behaviour, by not treating the MTU response from the server on the original connection into an override for the reconnect. Signed-off-by: David Woodhouse --- cstp.c | 26 +++++++++++++++++--------- dtls.c | 6 +++--- library.c | 1 - main.c | 10 +++++----- openconnect-internal.h | 3 ++- tun.c | 6 +++--- www/changelog.xml | 1 + 7 files changed, 31 insertions(+), 22 deletions(-) diff --git a/cstp.c b/cstp.c index 4da46988..bb15e9c9 100644 --- a/cstp.c +++ b/cstp.c @@ -106,7 +106,7 @@ static int __attribute__ ((format (printf, 3, 4))) */ static void calculate_mtu(struct openconnect_info *vpninfo, int *base_mtu, int *mtu) { - *mtu = vpninfo->mtu; + *mtu = vpninfo->reqmtu; *base_mtu = vpninfo->basemtu; #if defined(__linux__) && defined(TCP_INFO) @@ -269,6 +269,7 @@ static int start_cstp_connection(struct openconnect_info *vpninfo) /* We may have advertised it, but we only do it if the server agrees */ vpninfo->deflate = 0; + mtu = 0; while ((i = openconnect_SSL_gets(vpninfo, buf, sizeof(buf)))) { struct vpn_option *new_option; @@ -311,9 +312,9 @@ static int start_cstp_connection(struct openconnect_info *vpninfo) next_dtls_option = &new_option->next; if (!strcmp(buf + 7, "MTU")) { - int mtu = atol(colon); - if (mtu > vpninfo->mtu) - vpninfo->mtu = mtu; + int dtlsmtu = atol(colon); + if (dtlsmtu > mtu) + mtu = dtlsmtu; } else if (!strcmp(buf + 7, "Session-ID")) { if (strlen(colon) != 64) { vpn_progress(vpninfo, PRG_ERR, @@ -352,9 +353,9 @@ static int start_cstp_connection(struct openconnect_info *vpninfo) return -EINVAL; } } else if (!strcmp(buf + 7, "MTU")) { - int mtu = atol(colon); - if (mtu > vpninfo->mtu) - vpninfo->mtu = mtu; + int cstpmtu = atol(colon); + if (cstpmtu > mtu) + mtu = cstpmtu; } else if (!strcmp(buf + 7, "Address")) { if (strchr(new_option->value, ':')) { if (!vpninfo->disable_ipv6) @@ -413,6 +414,13 @@ static int start_cstp_connection(struct openconnect_info *vpninfo) } } + if (!mtu) { + vpn_progress(vpninfo, PRG_ERR, + _("No MTU received. Aborting\n")); + return -EINVAL; + } + vpninfo->actual_mtu = mtu; + if (!vpninfo->vpn_addr && !vpninfo->vpn_addr6) { vpn_progress(vpninfo, PRG_ERR, _("No IP address received. Aborting\n")); @@ -563,7 +571,7 @@ int cstp_reconnect(struct openconnect_info *vpninfo) static int inflate_and_queue_packet(struct openconnect_info *vpninfo, unsigned char *buf, int len) { - struct pkt *new = malloc(sizeof(struct pkt) + vpninfo->mtu); + struct pkt *new = malloc(sizeof(struct pkt) + vpninfo->actual_mtu); uint32_t pkt_sum; if (!new) @@ -575,7 +583,7 @@ static int inflate_and_queue_packet(struct openconnect_info *vpninfo, vpninfo->inflate_strm.avail_in = len - 4; vpninfo->inflate_strm.next_out = new->data; - vpninfo->inflate_strm.avail_out = vpninfo->mtu; + vpninfo->inflate_strm.avail_out = vpninfo->actual_mtu; vpninfo->inflate_strm.total_out = 0; if (inflate(&vpninfo->inflate_strm, Z_SYNC_FLUSH)) { diff --git a/dtls.c b/dtls.c index 5916daa2..46413a41 100644 --- a/dtls.c +++ b/dtls.c @@ -410,7 +410,7 @@ int dtls_try_handshake(struct openconnect_info *vpninfo) #ifdef HAVE_GNUTLS_DTLS_SET_DATA_MTU /* Make sure GnuTLS's idea of the MTU is sufficient to take a full VPN MTU (with 1-byte header) in a data record. */ - err = gnutls_dtls_set_data_mtu(vpninfo->new_dtls_ssl, vpninfo->mtu + 1); + err = gnutls_dtls_set_data_mtu(vpninfo->new_dtls_ssl, vpninfo->actual_mtu + 1); if (err) { vpn_progress(vpninfo, PRG_ERR, _("Failed to set DTLS MTU: %s\n"), @@ -423,7 +423,7 @@ int dtls_try_handshake(struct openconnect_info *vpninfo) We only support AES128-CBC and DES-CBC3-SHA anyway, so working out the worst case isn't hard. */ gnutls_dtls_set_mtu(vpninfo->new_dtls_ssl, - vpninfo->mtu + 1 /* packet + header */ + vpninfo->actual_mtu + 1 /* packet + header */ + 13 /* DTLS header */ + 20 /* biggest supported MAC (SHA1) */ + 16 /* biggest supported IV (AES-128) */ @@ -676,7 +676,7 @@ int dtls_mainloop(struct openconnect_info *vpninfo, int *timeout) char magic_pkt; while (1) { - int len = vpninfo->mtu; + int len = vpninfo->actual_mtu; unsigned char *buf; if (!dtls_pkt) { diff --git a/library.c b/library.c index 121d4ec6..0a3c87d5 100644 --- a/library.c +++ b/library.c @@ -37,7 +37,6 @@ struct openconnect_info *openconnect_vpninfo_new (char *useragent, { struct openconnect_info *vpninfo = calloc (sizeof(*vpninfo), 1); - vpninfo->mtu = 1406; vpninfo->ssl_fd = -1; vpninfo->cert_expire_warning = 60 * 86400; vpninfo->useragent = openconnect_create_useragent (useragent); diff --git a/main.c b/main.c index 4495c9e7..27388663 100644 --- a/main.c +++ b/main.c @@ -456,7 +456,7 @@ int main(int argc, char **argv) /* Set up some defaults */ vpninfo->tun_fd = vpninfo->ssl_fd = vpninfo->dtls_fd = vpninfo->new_dtls_fd = -1; vpninfo->useragent = openconnect_create_useragent("Open AnyConnect VPN Agent"); - vpninfo->mtu = 0; + vpninfo->reqmtu = 0; vpninfo->deflate = 1; vpninfo->dtls_attempt_period = 60; vpninfo->max_qlen = 10; @@ -576,10 +576,10 @@ int main(int argc, char **argv) use_syslog = 1; break; case 'm': - vpninfo->mtu = atol(config_arg); - if (vpninfo->mtu < 576) { - fprintf(stderr, _("MTU %d too small\n"), vpninfo->mtu); - vpninfo->mtu = 576; + vpninfo->reqmtu = atol(config_arg); + if (vpninfo->reqmtu < 576) { + fprintf(stderr, _("MTU %d too small\n"), vpninfo->reqmtu); + vpninfo->reqmtu = 576; } break; case OPT_BASEMTU: diff --git a/openconnect-internal.h b/openconnect-internal.h index 83419cb0..51a3e260 100644 --- a/openconnect-internal.h +++ b/openconnect-internal.h @@ -235,7 +235,8 @@ struct openconnect_info { int script_tun; char *ifname; - int mtu, basemtu; + int actual_mtu; + int reqmtu, basemtu; const char *banner; const char *vpn_addr; const char *vpn_netmask; diff --git a/tun.c b/tun.c index a9e18ac4..fdde7de6 100644 --- a/tun.c +++ b/tun.c @@ -91,7 +91,7 @@ static int set_tun_mtu(struct openconnect_info *vpninfo) memset(&ifr, 0, sizeof(ifr)); strncpy(ifr.ifr_name, vpninfo->ifname, sizeof(ifr.ifr_name) - 1); - ifr.ifr_mtu = vpninfo->mtu; + ifr.ifr_mtu = vpninfo->actual_mtu; if (ioctl(net_fd, SIOCSIFMTU, &ifr) < 0) perror(_("SIOCSIFMTU")); @@ -259,7 +259,7 @@ static void set_script_env(struct openconnect_info *vpninfo) unsetenv("CISCO_SPLIT_INC"); unsetenv("CISCO_SPLIT_EXC"); - setenv_int("INTERNAL_IP4_MTU", vpninfo->mtu); + setenv_int("INTERNAL_IP4_MTU", vpninfo->actual_mtu); if (vpninfo->vpn_addr) { setenv("INTERNAL_IP4_ADDRESS", vpninfo->vpn_addr, 1); @@ -686,7 +686,7 @@ int tun_mainloop(struct openconnect_info *vpninfo, int *timeout) if (FD_ISSET(vpninfo->tun_fd, &vpninfo->select_rfds)) { while (1) { - int len = vpninfo->mtu; + int len = vpninfo->actual_mtu; if (!out_pkt) { out_pkt = malloc(sizeof(struct pkt) + len); diff --git a/www/changelog.xml b/www/changelog.xml index 97c26372..12057292 100644 --- a/www/changelog.xml +++ b/www/changelog.xml @@ -17,6 +17,7 @@