Skip to content

Commit

Permalink
Align naming and commenting of mechanism for receiving oversize packe…
Browse files Browse the repository at this point in the history
…ts 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 <dlenski@gmail.com>
  • Loading branch information
dlenski committed Aug 2, 2018
1 parent 87a66e1 commit 0281a8e
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 6 deletions.
11 changes: 7 additions & 4 deletions cstp.c
Expand Up @@ -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)
Expand Down
6 changes: 5 additions & 1 deletion esp.c
Expand Up @@ -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) {
Expand Down
5 changes: 4 additions & 1 deletion gpst.c
Expand Up @@ -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) {
Expand Down

0 comments on commit 0281a8e

Please sign in to comment.