From 352e44f03d28eaf99359710f4f213bb1ef8a1299 Mon Sep 17 00:00:00 2001 From: Selva Nair Date: Sat, 13 Feb 2016 22:44:52 -0500 Subject: [PATCH 1/2] Better error reporting when connection fails to come up - Handle early errors (openvpn exits before management connection is up) with a helpful error message that points the user to view log. - Include only readable config files in the connection list - Warn if no connection profiles found TODO: handle startup errors from interactive service --- manage.c | 5 ++++- openvpn-gui-res.h | 4 ++++ openvpn.c | 11 ++++++++--- openvpn_config.c | 27 ++++++++++++++++++++++++++- options.h | 3 ++- res/openvpn-gui-res-en.rc | 5 +++++ 6 files changed, 49 insertions(+), 6 deletions(-) diff --git a/manage.c b/manage.c index 4ec6891..d6c3c18 100644 --- a/manage.c +++ b/manage.c @@ -54,7 +54,6 @@ InitManagement(const mgmt_rtmsg_handler *handler) } } - /* * Connect to the OpenVPN management interface and register * asynchronous socket event notification for it @@ -205,7 +204,11 @@ OnManagement(SOCKET sk, LPARAM lParam) if (time(NULL) < c->manage.timeout) connect(c->manage.sk, (SOCKADDR *)&c->manage.skaddr, sizeof(c->manage.skaddr)); else + { + /* Connection to MI timed out. */ + c->state = timedout; rtmsg_handler[stop](c, ""); + } } break; diff --git a/openvpn-gui-res.h b/openvpn-gui-res.h index 8377ce3..2bf9d23 100644 --- a/openvpn-gui-res.h +++ b/openvpn-gui-res.h @@ -156,6 +156,8 @@ #define IDS_ERR_CONN_SCRIPT_FAILED 1248 #define IDS_ERR_RUN_CONN_SCRIPT_TIMEOUT 1249 #define IDS_ERR_CONFIG_EXIST 1251 +#define IDS_NFO_CONN_TIMEOUT 1252 +#define IDS_NFO_NO_CONFIGS 1253 /* Program Startup Related */ #define IDS_ERR_OPEN_DEBUG_FILE 1301 @@ -225,6 +227,8 @@ #define IDS_ERR_OPEN_SCMGR 1706 #define IDS_ERR_STOP_SERVICE 1707 #define IDS_NFO_RESTARTED 1708 +#define IDS_ERR_ACCESS_SERVICE_PIPE 1709 +#define IDS_ERR_WRITE_SERVICE_PIPE 1710 /* Registry Related */ #define IDS_ERR_GET_WINDOWS_DIR 1801 diff --git a/openvpn.c b/openvpn.c index 213a189..c30f589 100644 --- a/openvpn.c +++ b/openvpn.c @@ -341,6 +341,7 @@ void OnStop(connection_t *c, UNUSED char *msg) { UINT txt_id, msg_id; + TCHAR *msg_xtra; SetMenuStatus(c, disconnected); switch (c->state) @@ -366,9 +367,13 @@ 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(); @@ -382,7 +387,7 @@ OnStop(connection_t *c, UNUSED char *msg) SetForegroundWindow(c->hwndStatus); ShowWindow(c->hwndStatus, SW_SHOW); } - ShowLocalizedMsg(msg_id, c->config_name); + ShowLocalizedMsg(msg_id, msg_xtra); SendMessage(c->hwndStatus, WM_CLOSE, 0, 0); break; @@ -703,7 +708,7 @@ StartOpenVPN(connection_t *c) DWORD dwMode = PIPE_READMODE_MESSAGE; if (!SetNamedPipeHandleState(service, &dwMode, NULL, NULL)) { - // TODO: add error message + ShowLocalizedMsg (IDS_ERR_ACCESS_SERVICE_PIPE); CloseHandle(c->exit_event); goto out; } @@ -715,7 +720,7 @@ StartOpenVPN(connection_t *c) if (!WriteFile(service, startup_info, size * sizeof (TCHAR), &written, NULL)) { - // TODO: add error message + ShowLocalizedMsg (IDS_ERR_WRITE_SERVICE_PIPE); CloseHandle(c->exit_event); goto out; } diff --git a/openvpn_config.c b/openvpn_config.c index f40a110..1adb579 100644 --- a/openvpn_config.c +++ b/openvpn_config.c @@ -61,6 +61,22 @@ match(const WIN32_FIND_DATA *find, const TCHAR *ext) return match_false; } +static bool +CheckReadAccess (const TCHAR *path) +{ + HANDLE h; + bool ret = FALSE; + + h = CreateFile (path, GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING, + FILE_ATTRIBUTE_NORMAL, NULL); + if ( h != INVALID_HANDLE_VALUE ) + { + ret = TRUE; + CloseHandle (h); + } + + return ret; +} static int ConfigAlreadyExists(TCHAR *newconfig) @@ -111,6 +127,8 @@ BuildFileList() HANDLE find_handle; TCHAR find_string[MAX_PATH]; TCHAR subdir_table[MAX_CONFIG_SUBDIRS][MAX_PATH]; + TCHAR fullpath[MAX_PATH]; + static int warn_no_configs = 1; int subdirs = 0; int i; @@ -134,7 +152,9 @@ BuildFileList() match_t match_type = match(&find_obj, o.ext_string); if (match_type == match_file) { - AddConfigFileToList(o.num_configs++, find_obj.cFileName, o.config_dir); + _sntprintf_0(fullpath, _T("%s\\%s"), o.config_dir, find_obj.cFileName); + if (CheckReadAccess (fullpath)) + AddConfigFileToList(o.num_configs++, find_obj.cFileName, o.config_dir); } else if (match_type == match_dir) { @@ -184,4 +204,9 @@ BuildFileList() FindClose(find_handle); } + if (o.num_configs == 0 && warn_no_configs) + { + ShowLocalizedMsg(IDS_NFO_NO_CONFIGS, o.config_dir); + warn_no_configs = 0; + } } diff --git a/options.h b/options.h index 6165d67..d581c49 100644 --- a/options.h +++ b/options.h @@ -67,7 +67,8 @@ typedef enum { disconnecting, suspending, suspended, - resuming + resuming, + timedout } conn_state_t; /* Connections parameters */ diff --git a/res/openvpn-gui-res-en.rc b/res/openvpn-gui-res-en.rc index 431f4bd..1b6ebb0 100644 --- a/res/openvpn-gui-res-en.rc +++ b/res/openvpn-gui-res-en.rc @@ -182,6 +182,7 @@ BEGIN /* OpenVPN */ IDS_ERR_MANY_CONFIGS "OpenVPN GUI does not support more than %d configs. Please contact the author if you have the need for more." + IDS_NFO_NO_CONFIGS "No readable connection profiles (config files) found at %s" IDS_ERR_ONE_CONN_OLD_VER "You can only have one connection running at the same time when using an older version on OpenVPN than 2.0-beta6." IDS_ERR_STOP_SERV_OLD_VER "You cannot use OpenVPN GUI to start a connection while the OpenVPN Service is running (with OpenVPN 1.5/1.6). Stop OpenVPN Service first if you want to use OpenVPN GUI." IDS_ERR_CREATE_EVENT "CreateEvent failed on exit event: %s" @@ -226,6 +227,8 @@ BEGIN IDS_ERR_CONFIG_EXIST "There already exist a config file named '%s'. You cannot " \ "have multiple config files with the same name, even if " \ "they reside in diffrent folders." + IDS_NFO_CONN_TIMEOUT "Connecting to management interface failed.\n" \ + "View log file (%s) for more details." /* main - Resources */ IDS_ERR_OPEN_DEBUG_FILE "Error opening debug file (%s) for output." IDS_ERR_CREATE_PATH "Could not create %s path:\n%s" @@ -322,6 +325,8 @@ BEGIN IDS_ERR_OPEN_SCMGR "OpenSCManager failed (%d)" IDS_ERR_STOP_SERVICE "Failed to stop OpenVPN Service" IDS_NFO_RESTARTED "OpenVPN Service Restarted." + IDS_ERR_ACCESS_SERVICE_PIPE "Access to service pipe failed." + IDS_ERR_WRITE_SERVICE_PIPE "Writing to service pipe failed." /* registry */ IDS_ERR_GET_WINDOWS_DIR "Error getting Windows Directory." From 5ce1298452181eb6b569766a91fba30f1aff89e8 Mon Sep 17 00:00:00 2001 From: Selva Nair Date: Tue, 16 Feb 2016 15:49:13 -0500 Subject: [PATCH 2/2] Warn if interative service is not installed or not running Signed-off-by: Selva Nair --- main.c | 3 +++ openvpn-gui-res.h | 2 ++ res/openvpn-gui-res-en.rc | 4 ++++ service.c | 34 ++++++++++++++++++++++++++++++++++ 4 files changed, 43 insertions(+) diff --git a/main.c b/main.c index 20e81ab..b6b97be 100644 --- a/main.c +++ b/main.c @@ -178,6 +178,9 @@ int WINAPI _tWinMain (HINSTANCE hThisInstance, exit(1); } + if (!IsUserAdmin()) + CheckIServiceStatus(); + BuildFileList(); if (!VerifyAutoConnections()) { exit(1); diff --git a/openvpn-gui-res.h b/openvpn-gui-res.h index 2bf9d23..8a949ef 100644 --- a/openvpn-gui-res.h +++ b/openvpn-gui-res.h @@ -229,6 +229,8 @@ #define IDS_NFO_RESTARTED 1708 #define IDS_ERR_ACCESS_SERVICE_PIPE 1709 #define IDS_ERR_WRITE_SERVICE_PIPE 1710 +#define IDS_ERR_NOTSTARTED_ISERVICE 1711 +#define IDS_ERR_INSTALL_ISERVICE 1712 /* Registry Related */ #define IDS_ERR_GET_WINDOWS_DIR 1801 diff --git a/res/openvpn-gui-res-en.rc b/res/openvpn-gui-res-en.rc index 1b6ebb0..c994810 100644 --- a/res/openvpn-gui-res-en.rc +++ b/res/openvpn-gui-res-en.rc @@ -327,6 +327,10 @@ BEGIN IDS_NFO_RESTARTED "OpenVPN Service Restarted." IDS_ERR_ACCESS_SERVICE_PIPE "Access to service pipe failed." IDS_ERR_WRITE_SERVICE_PIPE "Writing to service pipe failed." + IDS_ERR_INSTALL_ISERVICE """OpenVPNServiceInteractive"" is not installed.\n" + "Tasks requiring administrative access may not work." + IDS_ERR_NOTSTARTED_ISERVICE """OpenVPNServiceInteractive"" is not started.\n" + "Tasks requiring administrative access may not work." /* registry */ IDS_ERR_GET_WINDOWS_DIR "Error getting Windows Directory." diff --git a/service.c b/service.c index 21cab44..fc7367f 100644 --- a/service.c +++ b/service.c @@ -241,6 +241,40 @@ int MyReStartService() return(false); } +bool +CheckIServiceStatus() +{ + SC_HANDLE schSCManager; + SC_HANDLE schService; + SERVICE_STATUS ssStatus; + + // Open a handle to the SC Manager database. + schSCManager = OpenSCManager(NULL, NULL, SC_MANAGER_CONNECT); + + if (NULL == schSCManager) + return(false); + + schService = OpenService(schSCManager, _T("OpenVPNServiceInteractive"), + SERVICE_QUERY_STATUS); + if (schService == NULL && + GetLastError() == ERROR_SERVICE_DOES_NOT_EXIST) + { + /* warn that iservice is not installed */ + ShowLocalizedMsg(IDS_ERR_INSTALL_ISERVICE); + return(false); + } + + if (!QueryServiceStatus(schService, &ssStatus)) + return(false); + + if (ssStatus.dwCurrentState != SERVICE_RUNNING) + { + /* warn that iservice is not started */ + ShowLocalizedMsg(IDS_ERR_NOTSTARTED_ISERVICE); + return(false); + } + return true; +} int CheckServiceStatus() {