From fcc9deaac886fce1a542be765f9d5fcb76d1a9a9 Mon Sep 17 00:00:00 2001 From: Daniel Lenski Date: Sat, 16 May 2020 19:31:22 -0700 Subject: [PATCH] add delay_close and use it for clean PPP termination on cancel, pause, or server-sent termination Whatever delay_close is set to decrements on each mainloop iteration, and if delay_close == 1, we don't set work_done. This allows us to set delay_close = 2 for the case where we need to send a termination request immediately, and then wait briefly for an acknowledgment. Signed-off-by: Daniel Lenski --- f5.c | 3 --- mainloop.c | 40 +++++++++++++++++++++++++++------------- openconnect-internal.h | 1 + ppp.c | 32 +++++++++++++++++++++++++++----- 4 files changed, 55 insertions(+), 21 deletions(-) diff --git a/f5.c b/f5.c index b23b960e..a9a307e2 100644 --- a/f5.c +++ b/f5.c @@ -443,9 +443,6 @@ int f5_bye(struct openconnect_info *vpninfo, const char *reason) char *res_buf = NULL; int ret; - /* XX: handle clean PPP termination? - ppp_bye(vpninfo); */ - /* We need to close and reopen the HTTPS connection (to kill * the f5 tunnel) and submit a new HTTPS request to logout. */ diff --git a/mainloop.c b/mainloop.c index f78c0c2a..b9c8ce29 100644 --- a/mainloop.c +++ b/mainloop.c @@ -243,28 +243,42 @@ int openconnect_mainloop(struct openconnect_info *vpninfo, poll_cmd_fd(vpninfo, 0); if (vpninfo->got_cancel_cmd) { - if (vpninfo->cancel_type == OC_CMD_CANCEL) { + if (vpninfo->delay_close > 0) { + vpn_progress(vpninfo, PRG_DEBUG, _("Delaying cancel.\n")); + /* XX: don't let this spin forever */ + if (--vpninfo->delay_close > 0) + did_work++; + } else if (vpninfo->cancel_type == OC_CMD_CANCEL) { vpninfo->quit_reason = "Aborted by caller"; + vpninfo->got_cancel_cmd = 0; ret = -EINTR; + break; } else { + vpninfo->got_cancel_cmd = 0; ret = -ECONNABORTED; + break; } - vpninfo->got_cancel_cmd = 0; - break; } if (vpninfo->got_pause_cmd) { - /* close all connections and wait for the user to call - openconnect_mainloop() again */ - openconnect_close_https(vpninfo, 0); - if (vpninfo->dtls_state > DTLS_DISABLED) { - vpninfo->proto->udp_close(vpninfo); - vpninfo->new_dtls_started = 0; - } + if (vpninfo->delay_close > 0) { + vpn_progress(vpninfo, PRG_DEBUG, _("Delaying pause.\n")); + /* XX: don't let this spin forever */ + if (--vpninfo->delay_close > 0) + did_work++; + } else { + /* close all connections and wait for the user to call + openconnect_mainloop() again */ + openconnect_close_https(vpninfo, 0); + if (vpninfo->dtls_state > DTLS_DISABLED) { + vpninfo->proto->udp_close(vpninfo); + vpninfo->new_dtls_started = 0; + } - vpninfo->got_pause_cmd = 0; - vpn_progress(vpninfo, PRG_INFO, _("Caller paused the connection\n")); - return 0; + vpninfo->got_pause_cmd = 0; + vpn_progress(vpninfo, PRG_INFO, _("Caller paused the connection\n")); + return 0; + } } if (did_work) diff --git a/openconnect-internal.h b/openconnect-internal.h index 18de249b..890c264a 100644 --- a/openconnect-internal.h +++ b/openconnect-internal.h @@ -649,6 +649,7 @@ struct openconnect_info { int ssl_fd; int dtls_fd; int delay_tunnel; /* Delay tunnel setup */ + int delay_close; /* Delay close of mainloop */ int dtls_tos_current; int dtls_pass_tos; diff --git a/ppp.c b/ppp.c index 207b87a9..fad9476f 100644 --- a/ppp.c +++ b/ppp.c @@ -698,6 +698,7 @@ static int handle_config_packet(struct openconnect_info *vpninfo, vpninfo->quit_reason); } ppp->ppp_state = PPPS_TERMINATE; + vpninfo->delay_close = 0; break; case ECHOREP: @@ -771,15 +772,36 @@ static int handle_state_transition(struct openconnect_info *vpninfo, int *timeou break; ppp->ppp_state = PPPS_NETWORK; - vpninfo->delay_tunnel = 0; + vpninfo->delay_tunnel = 0; /* tunnel can start now */ + vpninfo->delay_close = 2; /* need two mainloop iterations on close (send TERMREQ; receive TERMACK) */ /* fall through */ case PPPS_NETWORK: - break; + if (vpninfo->got_pause_cmd || vpninfo->got_cancel_cmd) + ppp->ppp_state = PPPS_TERMINATE; + else + break; + /* fall through */ + case PPPS_TERMINATE: - if (!vpninfo->quit_reason) - vpninfo->quit_reason = "Unknown"; - return -EPIPE; + /* XX: If server terminated, we already ACK'ed it */ + 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. */ + if (!(ppp->lcp.state & NCP_TERM_REQ_SENT)) { + ppp->lcp.state |= NCP_TERM_REQ_SENT; + ppp->lcp.last_req = now; + (void) queue_config_packet(vpninfo, PPP_LCP, ++ppp->lcp.id, TERMREQ, 0, NULL); + } + if (!ka_check_deadline(timeout, now, ppp->lcp.last_req + 3)) + vpninfo->delay_close = 1; + else + (void) queue_config_packet(vpninfo, PPP_LCP, ++ppp->lcp.id, TERMREQ, 0, NULL); + } + break; case PPPS_AUTHENTICATE: /* XX: should never */ default: vpninfo->quit_reason = "Unexpected state"; -- 2.50.1