Fix handling of lost TERMACK

The comment said we'd send a TERMREQ "once more" after 3 seconds if we
didn't get a TERMACK in response to the first one. It transpires this
claim was false. If we didn't get a response to the first one, we would
send a *flood* of TERMREQs until we filled the socket sndbuf, with no
delay between them at all because we weren't updating ->lcp.last_req.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>

diff --git a/ppp.c b/ppp.c
index 88b2d62a75ec0ba61256894b466fed53bc71636b..57e1b2877444696b71be1b3f2c24191cfe3695eb 100644 (file)
--- a/ppp.c
+++ b/ppp.c
@@ -987,19 +987,30 @@ static int handle_state_transition(struct openconnect_info *vpninfo, int dtls,
                if (ppp->lcp.state & NCP_TERM_REQ_RECEIVED)
                        return -EPIPE;
                else if (!(ppp->lcp.state & NCP_TERM_ACK_RECEIVED)) {
-                       /* We need to send a TERMREQ and wait for a TERMACK, but not keep retrying if it
-                        * fails. We attempt to send TERMREQ once, then wait for 3 seconds, and
-                        * send once more TERMREQ if that fails. */
+                       /* We need to send a TERMREQ and wait for a TERMACK, but not keep
+                        * retrying for ever if it fails. For TLS, which ought to be
+                        * lossless, we send TERMREQ just once, wait for a second, and
+                        * give up. For DTLS we try three times (with a second between
+                        * them and give up if none of them elicit a TERMACK. */
                        if (!(ppp->lcp.state & NCP_TERM_REQ_SENT)) {
                                ppp->lcp.state |= NCP_TERM_REQ_SENT;
+                               ppp->lcp.termreqs_sent = 1;
                                ppp->lcp.last_req = now;
                                (void) queue_config_packet(vpninfo, PPP_LCP, ++ppp->lcp.id, TERMREQ, 0, NULL);
-                               vpninfo->delay_close = DELAY_CLOSE_WAIT; /* need to wait until we receive TERMACK */
-                       }
-                       if (!ka_check_deadline(timeout, now, ppp->lcp.last_req + 3))
-                               vpninfo->delay_close = DELAY_CLOSE_WAIT; /* still waiting to receive TERMACK */
-                       else
+                               vpninfo->delay_close = DELAY_CLOSE_WAIT; /* Wait 1s for TERMACK */
+
+                       } else if (!ka_check_deadline(timeout, now, ppp->lcp.last_req + 1)) {
+                               vpninfo->delay_close = DELAY_CLOSE_WAIT; /* Still waiting */
+
+                       } else if (!dtls || ppp->lcp.termreqs_sent >= 3) {
+                               ppp->ppp_state = PPPS_TERMINATE;
+
+                       } else {
+                               ppp->lcp.termreqs_sent++;
+                               ppp->lcp.last_req = now;
                                (void) queue_config_packet(vpninfo, PPP_LCP, ++ppp->lcp.id, TERMREQ, 0, NULL);
+                               vpninfo->delay_close = DELAY_CLOSE_WAIT; /* Wait 1s more */
+                       }
                }
                break;
        case PPPS_AUTHENTICATE: /* XX: should never */

diff --git a/ppp.h b/ppp.h
index c425677820b7fc8d7a200bbea24fd7f551029330..e9176ab1a996b23f6ead7069d92e3d862ff83c2a 100644 (file)
--- a/ppp.h
+++ b/ppp.h
@@ -94,6 +94,7 @@
 struct oc_ncp {
        int state;
        int id;
+       int termreqs_sent;
        time_t last_req;
 };