From 63ff73fc4768b9225381b0f3f958d7d64654010a Mon Sep 17 00:00:00 2001 From: Selva Nair Date: Tue, 17 Apr 2018 22:39:12 -0400 Subject: [PATCH] Retry on management timeout instead of aborting In some cases the service may take a while to startup openvpn.exe, causing connection to the management interface to timeout. This could leave behind the OpenVPN process if/when it eventually starts up. (Trac 905, 1050). As errors in starting up the OpenVPN daemon are independently handled, its better to keep retrying the management interface connection until aborted due to errors or by the user. - On timeout, log a message on the status window and retry the management interface connection - Eliminate the timed-out state that is no longer used - Call StopOpenVPN() before abort so that OpenVPN daemon is not left running in case it starts up later. - In the unlikely event that OpenManagement() fails, show an error - User can abort by pressing disconnect A "retrying.." message is logged on to the status window every 15 seconds. See Trac: #905, #1050 Signed-off-by: Selva Nair --- main.c | 1 + manage.c | 5 ++--- manage.h | 1 + openvpn.c | 41 ++++++++++++++++++++++++++++++++--------- openvpn.h | 1 + options.h | 1 - 6 files changed, 37 insertions(+), 13 deletions(-) diff --git a/main.c b/main.c index 6b7f44e..c1ee3d5 100644 --- a/main.c +++ b/main.c @@ -188,6 +188,7 @@ int WINAPI _tWinMain (HINSTANCE hThisInstance, { echo_, OnEcho }, { bytecount_,OnByteCount }, { infomsg_, OnInfoMsg }, + { timeout_, OnTimeout }, { 0, NULL } }; InitManagement(handler); diff --git a/manage.c b/manage.c index cfd96f9..a567af7 100644 --- a/manage.c +++ b/manage.c @@ -209,10 +209,9 @@ OnManagement(SOCKET sk, LPARAM lParam) else { /* Connection to MI timed out. */ - if (c->state != disconnected) - c->state = timedout; CloseManagement (c); - rtmsg_handler[stop_](c, ""); + if (c->state != disconnected) + rtmsg_handler[timeout_](c, ""); } } else diff --git a/manage.h b/manage.h index 7b7f005..7c9dba4 100644 --- a/manage.h +++ b/manage.h @@ -38,6 +38,7 @@ typedef enum { needstr_, pkcs11_id_count_, infomsg_, + timeout_, mgmt_rtmsg_type_max } mgmt_rtmsg_type; diff --git a/openvpn.c b/openvpn.c index 81d457d..b5974d6 100644 --- a/openvpn.c +++ b/openvpn.c @@ -1231,6 +1231,32 @@ OnPassword(connection_t *c, char *msg) } } +/* + * Handle management connection timeout + */ +void +OnTimeout(connection_t *c, UNUSED char *msg) +{ + /* Connection to management timed out -- keep trying unless killed + * by a startup error from service or from openvpn daemon process. + * The user can terminate by pressing disconnect. + */ + if (o.silent_connection == 0) + { + SetForegroundWindow(c->hwndStatus); + ShowWindow(c->hwndStatus, SW_SHOW); + } + WriteStatusLog (c, L"GUI> ", LoadLocalizedString(IDS_NFO_CONN_TIMEOUT, c->log_path), false); + WriteStatusLog (c, L"GUI> ", L"Retrying. Press disconnect to abort", false); + c->state = connecting; + if (!OpenManagement(c)) + { + MessageBoxEx(NULL, L"Failed to open management", _T(PACKAGE_NAME), + MB_OK|MB_SETFOREGROUND|MB_ICONERROR, GetGUILanguage()); + StopOpenVPN(c); + } + return; +} /* * Handle exit of the OpenVPN process @@ -1239,7 +1265,6 @@ void OnStop(connection_t *c, UNUSED char *msg) { UINT txt_id, msg_id; - TCHAR *msg_xtra; SetMenuStatus(c, disconnected); switch (c->state) @@ -1267,13 +1292,9 @@ OnStop(connection_t *c, UNUSED char *msg) case resuming: case connecting: case reconnecting: - case timedout: /* We have failed to (re)connect */ txt_id = c->state == reconnecting ? IDS_NFO_STATE_FAILED_RECONN : IDS_NFO_STATE_FAILED; msg_id = c->state == reconnecting ? IDS_NFO_RECONN_FAILED : IDS_NFO_CONN_FAILED; - msg_xtra = c->state == timedout ? c->log_path : c->config_name; - if (c->state == timedout) - msg_id = IDS_NFO_CONN_TIMEOUT; c->state = disconnecting; CheckAndSetTrayIcon(); @@ -1287,7 +1308,7 @@ OnStop(connection_t *c, UNUSED char *msg) SetForegroundWindow(c->hwndStatus); ShowWindow(c->hwndStatus, SW_SHOW); } - MessageBox(c->hwndStatus, LoadLocalizedString(msg_id, msg_xtra), _T(PACKAGE_NAME), MB_OK); + MessageBox(c->hwndStatus, LoadLocalizedString(msg_id, c->config_name), _T(PACKAGE_NAME), MB_OK); SendMessage(c->hwndStatus, WM_CLOSE, 0, 0); break; @@ -1649,12 +1670,10 @@ OnService(connection_t *c, UNUSED char *msg) break; case ERROR_STARTUP_DATA: WriteStatusLog (c, prefix, L"OpenVPN not started due to previous errors", true); - c->state = timedout; /* Force the popup message to include the log file name */ OnStop (c, NULL); break; case ERROR_OPENVPN_STARTUP: WriteStatusLog (c, prefix, L"Check the log file for details", false); - c->state = timedout; /* Force the popup message to include the log file name */ OnStop(c, NULL); break; default: @@ -1992,7 +2011,11 @@ ThreadOpenVPNStatus(void *p) SetWindowText(c->hwndStatus, LoadLocalizedString(IDS_NFO_CONNECTION_XXX, conn_name)); if (!OpenManagement(c)) - PostMessage(c->hwndStatus, WM_CLOSE, 0, 0); + { + MessageBoxEx(NULL, L"Failed to open management", _T(PACKAGE_NAME), + MB_OK|MB_SETFOREGROUND|MB_ICONERROR, GetGUILanguage()); + StopOpenVPN(c); + } /* Start the async read loop for service and set it as the wait event */ if (c->iserv.hEvent) diff --git a/openvpn.h b/openvpn.h index 18ef250..8222a7e 100644 --- a/openvpn.h +++ b/openvpn.h @@ -43,6 +43,7 @@ void OnNeedStr(connection_t *, char *); void OnEcho(connection_t *, char *); void OnByteCount(connection_t *, char *); void OnInfoMsg(connection_t*, char*); +void OnTimeout(connection_t *, char *); void ResetSavePasswords(connection_t *); diff --git a/options.h b/options.h index 3a93ffb..b31ff4d 100644 --- a/options.h +++ b/options.h @@ -71,7 +71,6 @@ typedef enum { suspending, suspended, resuming, - timedout } conn_state_t; /* Interactive Service IO parameters */