From 0281a8e1db81695a9eb553a241529f3a0a9695d3 Mon Sep 17 00:00:00 2001 From: Daniel Lenski Date: Tue, 31 Jul 2018 19:35:59 -0700 Subject: [PATCH] Align naming and commenting of mechanism for receiving oversize packets across protocols We've now implemented mechanisms to tolerate larger-than-expected packets for: - Uncompressed CSTP packets ("Fixed regression with CSTP MTU handling" patch in July 2016) - Uncompressed oNCP packets ("Do not drop vpn connection if packet arrived is larger than MTU" patch in May 2017) - Uncompressed GPST packets (in original merge from March 2018; this is a virtual necessity for GlobalProtect because it has no functional mechanism for negotiating the MTU) - Uncompressed ESP packets ("check for oversize ESP packets, with 256 bytes of headroom above calculated" in March 2018; GlobalProtect requires this for the aforementioned reason) - Compressed CSTP packets (preceding patch in this series) Since this is a requiring issue across protocols, it's useful to align the naming, commenting, and packet sizing-tolerance across the source files. 1) Use receive_mtu everywhere as the name for the maximum tolerated size of an incoming packet. 2) Insert similar comments explaining its purpose everywhere it's used. 3) Use receive_mtu = MAX(16384, vpninfo->ip_info.mtu) for all TLS-based tunnels, because 16384 is the maximum TLS record size. 4) Use receive_mtu = MAX(2048, vpninfo->vpninfo->ip_info.mtu + 256) for all UDP-based tunnels, because the MTU of IP datagrams on the public internet is effectively ~1500. Signed-off-by: Daniel Lenski --- cstp.c | 11 +++++++---- esp.c | 6 +++++- gpst.c | 5 ++++- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/cstp.c b/cstp.c index c1311981..8cca637a 100644 --- a/cstp.c +++ b/cstp.c @@ -886,18 +886,21 @@ int cstp_mainloop(struct openconnect_info *vpninfo, int *timeout) and add POLLOUT. As it is, though, it'll just chew CPU time in that fairly unlikely situation, until the write backlog clears. */ while (1) { - int len = MAX(16384, vpninfo->deflate_pkt_size ? : vpninfo->ip_info.mtu); - int payload_len; + /* Some servers send us packets that are larger than + negotiated MTU. We reserve some extra space to + handle that */ + int receive_mtu = MAX(16384, vpninfo->deflate_pkt_size ? : vpninfo->ip_info.mtu); + int len, payload_len; if (!vpninfo->cstp_pkt) { - vpninfo->cstp_pkt = malloc(sizeof(struct pkt) + len); + vpninfo->cstp_pkt = malloc(sizeof(struct pkt) + receive_mtu); if (!vpninfo->cstp_pkt) { vpn_progress(vpninfo, PRG_ERR, _("Allocation failed\n")); break; } } - len = ssl_nonblock_read(vpninfo, vpninfo->cstp_pkt->cstp.hdr, len + 8); + len = ssl_nonblock_read(vpninfo, vpninfo->cstp_pkt->cstp.hdr, receive_mtu + 8); if (!len) break; if (len < 0) diff --git a/esp.c b/esp.c index 30de764d..e9760c43 100644 --- a/esp.c +++ b/esp.c @@ -106,10 +106,14 @@ int esp_mainloop(struct openconnect_info *vpninfo, int *timeout) struct esp *esp = &vpninfo->esp_in[vpninfo->current_esp_in]; struct esp *old_esp = &vpninfo->esp_in[vpninfo->current_esp_in ^ 1]; struct pkt *this; - int receive_mtu = MAX(2048, vpninfo->ip_info.mtu + 256); int work_done = 0; int ret; + /* Some servers send us packets that are larger than negotiated + MTU, or lack the ability to negotiate MTU (see gpst.c). We + reserve some extra space to handle that */ + int receive_mtu = MAX(2048, vpninfo->ip_info.mtu + 256); + if (vpninfo->dtls_state == DTLS_SLEEPING) { if (ka_check_deadline(timeout, time(NULL), vpninfo->new_dtls_started + vpninfo->dtls_attempt_period) || vpninfo->dtls_need_reconnect) { diff --git a/gpst.c b/gpst.c index 1cd98644..97207f9f 100644 --- a/gpst.c +++ b/gpst.c @@ -1014,7 +1014,10 @@ int gpst_mainloop(struct openconnect_info *vpninfo, int *timeout) goto do_reconnect; while (1) { - int receive_mtu = MAX(2048, vpninfo->ip_info.mtu + 256); + /* Some servers send us packets that are larger than + negotiated MTU. We reserve some extra space to + handle that */ + int receive_mtu = MAX(16384, vpninfo->ip_info.mtu); int len, payload_len; if (!vpninfo->cstp_pkt) {