From 16cf7f6da951325e6b452549c471cc97f06b1d11 Mon Sep 17 00:00:00 2001 From: Selva Nair Date: Thu, 21 Jun 2018 23:38:13 -0400 Subject: [PATCH] Force script-security to SSEC_BUILT_IN by default The setting is persisted in the registry as HKCU\OpenVPN-GUI\config-name\script_security (DWORD) with possible values 0 to 3 that has the same meaning as described in OpenVPN man page or -1 to mean do not override the option (i.e, any value in the config file will be effective). The setting is reverted to default if the file changes (tracked by a digest of the file). This feature is not effective until the next commit where the digest function is implemented. The config file is locked against writes when a connection is active. This ensures that if/when the file is re-read by OpenVPN for a SIGHUP restart the contents have not changed. If no value is found in the registry the setting defaults to 1 (SSEC_BUILT_IN). TODO: add a dialog to edit the setting Signed-off-by: Selva Nair --- Makefile.am | 1 + openvpn-gui-res.h | 1 + openvpn.c | 27 ++++++++ options.h | 2 + registry.c | 36 +++++++++++ registry.h | 3 + script_security.c | 160 ++++++++++++++++++++++++++++++++++++++++++++++ script_security.h | 56 ++++++++++++++++ 8 files changed, 286 insertions(+) create mode 100644 script_security.c create mode 100644 script_security.h diff --git a/Makefile.am b/Makefile.am index 8301087..f90556a 100644 --- a/Makefile.am +++ b/Makefile.am @@ -92,6 +92,7 @@ openvpn_gui_SOURCES = \ chartable.h \ save_pass.c save_pass.h \ env_set.c env_set.h \ + script_security.c script_security.h \ openvpn-gui-res.h openvpn_gui_LDFLAGS = -mwindows diff --git a/openvpn-gui-res.h b/openvpn-gui-res.h index ab20f73..6b1d3d9 100644 --- a/openvpn-gui-res.h +++ b/openvpn-gui-res.h @@ -118,6 +118,7 @@ #define ID_EDT_PRECONNECT_TIMEOUT 282 #define ID_EDT_CONNECT_TIMEOUT 283 #define ID_EDT_DISCONNECT_TIMEOUT 284 +#define ID_CHK_SSEC_OVERRIDE 285 /* Connections dialog */ #define ID_DLG_CONNECTIONS 290 diff --git a/openvpn.c b/openvpn.c index 9debf24..df01879 100644 --- a/openvpn.c +++ b/openvpn.c @@ -50,6 +50,7 @@ #include "access.h" #include "save_pass.h" #include "env_set.h" +#include "script_security.h" extern options_t o; @@ -1554,6 +1555,8 @@ OnNeedStr (connection_t *c, UNUSED char *msg) static void Cleanup (connection_t *c) { + lock_config_file(c, false); /* unlock */ + CloseManagement (c); free_dynamic_cr (c); @@ -1775,7 +1778,10 @@ ThreadOpenVPNStatus(void *p) /* Create and Show Status Dialog */ c->hwndStatus = CreateLocalizedDialogParam(ID_DLG_STATUS, StatusDialogFunc, (LPARAM) c); if (!c->hwndStatus) + { + Cleanup(c); return 1; + } CheckAndSetTrayIcon(); SetMenuStatus(c, connecting); @@ -1916,6 +1922,19 @@ StartOpenVPN(connection_t *c) inet_ntoa(c->manage.skaddr.sin_addr), ntohs(c->manage.skaddr.sin_port), (o.proxy_source != config ? _T("--management-query-proxy ") : _T(""))); + /* append --script-security n to command line */ + if (!o.disable_ssec_override) + { + lock_config_file(c, true); + int ssec_flag = get_script_security(c); + if (ssec_flag != SSEC_UNDEF) + { + TCHAR ssec[32]; + _sntprintf_0(ssec, _T(" --script-security %d"), ssec_flag); + _tcsncat(cmdline, ssec, _countof(cmdline) - _tcslen(cmdline) - 1); + } + } + /* Try to open the service pipe */ if (!IsUserAdmin() && InitServiceIO (&c->iserv)) { @@ -1967,12 +1986,14 @@ StartOpenVPN(connection_t *c) { ShowLocalizedMsg(IDS_ERR_INIT_SEC_DESC); CloseHandle(c->exit_event); + lock_config_file(c, false); /* unlock */ return FALSE; } if (!SetSecurityDescriptorDacl(&sd, TRUE, NULL, FALSE)) { ShowLocalizedMsg(IDS_ERR_SET_SEC_DESC_ACL); CloseHandle(c->exit_event); + lock_config_file(c, false); /* unlock */ return FALSE; } @@ -1980,6 +2001,7 @@ StartOpenVPN(connection_t *c) if (!SetProcessPriority(&priority)) { CloseHandle(c->exit_event); + lock_config_file(c, false); /* unlock */ return FALSE; } @@ -1988,6 +2010,7 @@ StartOpenVPN(connection_t *c) if (hNul == INVALID_HANDLE_VALUE) { CloseHandle(c->exit_event); + lock_config_file(c, false); /* unlock */ return FALSE; } @@ -2044,6 +2067,10 @@ out: CloseHandle(hStdInRead); if (hNul && hNul != INVALID_HANDLE_VALUE) CloseHandle(hNul); + + if (!retval) + lock_config_file(c, false); /* unlock */ + return retval; } diff --git a/options.h b/options.h index c523f6f..49b3755 100644 --- a/options.h +++ b/options.h @@ -155,6 +155,7 @@ struct connection { unsigned long long int bytes_in; unsigned long long int bytes_out; struct env_item *es; /* Pointer to the head of config-specific env variables list */ + HANDLE hfile; /* Config file handle used for locking */ }; /* All options used within OpenVPN GUI */ @@ -203,6 +204,7 @@ typedef struct { DWORD disconnectscript_timeout; /* Disconnect Script execution timeout (sec) */ DWORD preconnectscript_timeout; /* Preconnect Script execution timeout (sec) */ DWORD config_menu_view; /* 0 for auto, 1 for original flat menu, 2 for hierarchical */ + DWORD disable_ssec_override; /* Disable script-security setting override */ #ifdef DEBUG FILE *debug_fp; diff --git a/registry.c b/registry.c index 11470e2..9d4123f 100644 --- a/registry.c +++ b/registry.c @@ -60,6 +60,7 @@ struct regkey_int { {L"connectscript_timeout", &o.connectscript_timeout, 30}, {L"disconnectscript_timeout", &o.disconnectscript_timeout, 10}, {L"show_script_window", &o.show_script_window, 0}, + {L"disable_ssec_override", &o.disable_ssec_override, 0}, {L"service_only", &o.service_only, 0}, {L"config_menu_view", &o.config_menu_view, CONFIG_VIEW_AUTO} }; @@ -487,3 +488,38 @@ DeleteConfigRegistryValue(const WCHAR *config_name, const WCHAR *name) return (status == ERROR_SUCCESS); } + +int +GetConfigRegistryValueNumeric(const WCHAR *config_name, const TCHAR *name, DWORD *data) +{ + int ret = 0; + HKEY regkey; + + if (!OpenConfigRegistryKey(config_name, ®key, FALSE)) return 0; + + ret = GetRegistryValueNumeric(regkey, name, data); + + RegCloseKey(regkey); + + return ret; +} + +int +SetConfigRegistryValueNumeric(const WCHAR *config_name, const TCHAR *name, DWORD data) +{ + int ret = 0; + HKEY regkey; + + if (!OpenConfigRegistryKey(config_name, ®key, FALSE)) return 0; + + if (RegSetValueEx(regkey, name, 0, REG_DWORD, (PBYTE) &data, sizeof(data)) != ERROR_SUCCESS) + { + MsgToEventLog(EVENTLOG_ERROR_TYPE, L"Error writing registry value <%s> for config <%s>", + config_name, name); + ret = 1; + } + + RegCloseKey(regkey); + + return ret; +} diff --git a/registry.h b/registry.h index 272e7c0..004f6a9 100644 --- a/registry.h +++ b/registry.h @@ -34,4 +34,7 @@ int SetConfigRegistryValueBinary(const WCHAR *config_name, const WCHAR *name, co DWORD GetConfigRegistryValue(const WCHAR *config_name, const WCHAR *name, BYTE *data, DWORD len); int DeleteConfigRegistryValue(const WCHAR *config_name, const WCHAR *name); +int GetConfigRegistryValueNumeric(const WCHAR *config_name, const TCHAR *name, DWORD *data); +int SetConfigRegistryValueNumeric(const WCHAR *config_name, const TCHAR *name, DWORD data); + #endif diff --git a/script_security.c b/script_security.c new file mode 100644 index 0000000..d897830 --- /dev/null +++ b/script_security.c @@ -0,0 +1,160 @@ +/* + * This file is a part of OpenVPN-GUI -- A Windows GUI for OpenVPN. + * + * Copyright (C) 2018 Selva Nair + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program (see the file COPYING included with this + * distribution); if not, write to the Free Software Foundation, Inc., + * 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#ifdef HAVE_CONFIG_H +#include +#endif + +#include +#include +#include "script_security.h" +#include "registry.h" +#include "misc.h" +#include "main.h" + +#define MAX_HASHLEN 64 +#define HASHALG CALG_SHA1 + +/* mock hash routine - to be replaced */ +DWORD +md_file(UNUSED ALG_ID alg, UNUSED const wchar_t *fname, BYTE *digest, DWORD *digest_len) +{ + /* not implemented */ + memset(digest, 0, *digest_len); + return 0; +} + +/* Return the script_security setting that should override the + * value in the config file. + * If the config file has been modified since last time the + * setting was confirmed by the user, its reset to the default. + */ +int +get_script_security(const connection_t *c) +{ + DWORD err; + wchar_t filename[MAX_PATH]; + BYTE saved_digest[MAX_HASHLEN], current_digest[MAX_HASHLEN]; + DWORD digest_len = _countof(saved_digest); + + int ssec_flag = ssec_default(); + + wcs_concat2(filename, _countof(filename), c->config_dir, c->config_file, L"\\"); + + err = md_file(HASHALG, filename, current_digest, &digest_len); + + if (err) + { + MsgToEventLog(EVENTLOG_ERROR_TYPE, L"Failed to compute file digest of <%s>: error = %lu", + filename, err); + return ssec_flag; + } + + /* If there is a digest is in registry and matches that of the file, + * read and use the script security setting in the registry. + */ + digest_len = GetConfigRegistryValue(c->config_name, L"config_digest", saved_digest, + _countof(saved_digest)); + if (digest_len) + { + DWORD flag; + if (memcmp(current_digest, saved_digest, digest_len)) + { + /* if digest does not match clear script-security flag from registry + * to force the default. Its ok to call delete even if the key + * doesn't exist. */ + DeleteConfigRegistryValue(c->config_name, L"script_security"); + } + else if (GetConfigRegistryValueNumeric(c->config_name, L"script_security", &flag)) + { + /* ignore any out of range value in registry */ + if (flag <= SSEC_PW_ENV || (int) flag == -1) + ssec_flag = (int) flag; + } + } + + return ssec_flag; +} + +/* Update the script security flag for a connection profile and save + * it in registry + */ +void +set_script_security(connection_t *c, int ssec_flag) +{ + DWORD err; + wchar_t filename[MAX_PATH]; + BYTE digest[MAX_HASHLEN]; + DWORD digest_len = _countof(digest); + + wcs_concat2(filename, _countof(filename), c->config_dir, c->config_file, L"\\"); + + err = md_file(HASHALG, filename, digest, &digest_len); + if (err) + { + MsgToEventLog(EVENTLOG_ERROR_TYPE, L"Failed to compute file digest of <%s>: error = %lu", + filename, err); + return; + } + SetConfigRegistryValueBinary(c->config_name, L"config_digest", digest, digest_len); + SetConfigRegistryValueNumeric(c->config_name, L"script_security", (DWORD) ssec_flag); +} + +/* Lock/unlock the config file to protect against changes when a connection + * is active. Call with |lock| == true to lock, as false to unlock. + * The lock will still permit shared read required for OpenVPN to parse + * the config file. + */ +void +lock_config_file(connection_t *c, BOOL lock) +{ + wchar_t fname[MAX_PATH]; + OVERLAPPED o = {.Offset = 0, .hEvent = NULL}; + DWORD bytes_low = (DWORD) -1; + DWORD bytes_hi = 0; + + wcs_concat2(fname, _countof(fname), c->config_dir, c->config_file, L"\\"); + + if (lock) + { + c->hfile = CreateFile(fname, GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING, + FILE_ATTRIBUTE_NORMAL, NULL); + + if (c->hfile == NULL || c->hfile == INVALID_HANDLE_VALUE) + { + MsgToEventLog(EVENTLOG_ERROR_TYPE, L"Failed to open the config file <%s> for reading", + fname); + return; + } + + if (!LockFileEx(c->hfile, LOCKFILE_FAIL_IMMEDIATELY, 0, bytes_low, bytes_hi, &o)) + MsgToEventLog(EVENTLOG_ERROR_TYPE, L"Failed to write-lock the config file <%s>: err = %lu", + fname, GetLastError()); + } + else + { + if (c->hfile == NULL || c->hfile == INVALID_HANDLE_VALUE) return; + + UnlockFileEx(c->hfile, 0, bytes_low, bytes_hi, &o); + CloseHandle(c->hfile); + c->hfile = NULL; + } +} diff --git a/script_security.h b/script_security.h new file mode 100644 index 0000000..ab3fad1 --- /dev/null +++ b/script_security.h @@ -0,0 +1,56 @@ +/* + * This file is a part of OpenVPN-GUI -- A Windows GUI for OpenVPN. + * + * Copyright (C) 2018 Selva Nair + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program (see the file COPYING included with this + * distribution); if not, write to the Free Software Foundation, Inc., + * 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#ifndef SCRIPT_SECURITY_H +#define SCRIPT_SECURITY_H + +#include "options.h" + +/* script-security flags for OpenVPN */ +#define SSEC_UNDEF -1 /* Do not override the setting in the config file */ +#define SSEC_NONE 0 /* No external command execution permitted */ +#define SSEC_BUILT_IN 1 /* Permit only built-in commands such as ipconfig, netsh */ +#define SSEC_SCRIPTS 2 /* Permit running of user speciifed scripts and executables */ +#define SSEC_PW_ENV 3 /* Permit running of scripts/executables that may receive + * sensitive data such as passwords through the environment */ + +static inline int +ssec_default(void) +{ + return SSEC_BUILT_IN; +} + +/* Return the permitted script security setting for a connection profile. + * If no setting is found returns ssec_default() + */ +int get_script_security(const connection_t *c); + +/* Update script security setting for a connection profile */ +void set_script_security(connection_t *c, int ssec_flag); + +/* Lock or unlock the config file for connection profile c + * to avoid modification when active. A shared lock is created + * so that other processes can read the file. + * Lock if lock is true, unlock otherwise. + */ +void lock_config_file(connection_t *c, BOOL lock); + +#endif /* SCRIPT_SECURITY_H */