]> www.infradead.org Git - users/dwmw2/openconnect.git/commitdiff
Align naming and commenting of mechanism for receiving oversize packets across protocols
authorDaniel Lenski <dlenski@gmail.com>
Wed, 1 Aug 2018 02:35:59 +0000 (19:35 -0700)
committerDaniel Lenski <dlenski@gmail.com>
Thu, 2 Aug 2018 01:32:13 +0000 (18:32 -0700)
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 <dlenski@gmail.com>
cstp.c
esp.c
gpst.c

diff --git a/cstp.c b/cstp.c
index c1311981c39163bf16c25d115fe53dd327f59a23..8cca637a5d45e6c465b9c183861d6254d855d6e2 100644 (file)
--- 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 30de764d429170f2eccf499a422db49e7ada7cc7..e9760c43e51a488eb319b613611004281a6e4603 100644 (file)
--- 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 1cd98644f87a9276c9209630bc5283d7a5924999..97207f9f6c20ccbad13f3ae8829f314a9c70a818 100644 (file)
--- 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) {