Attention is currently required from: plaisthos.

Hello plaisthos,

I'd like you to do a code review.
Please visit

    http://gerrit.openvpn.net/c/openvpn/+/1714?usp=email

to review the following change.


Change subject: Whitelist configs in autostart_config_dir
......................................................................

Whitelist configs in autostart_config_dir

We currently whitelist these configs by checking that the user is
NT Service\<ovpn_service_user>, which defaults to OpenVPNService.
In some setups one may need to change this service user to, say,
NT Authority\Local Service, in which case the validation fails.

Make it easier to change the service user by whitelisting all
configs in autostart_config_dir.

See also: Github OpenVPN/openvpn#1056

Change-Id: I52609b71deb8810a3fea639494b5fa13d9e5a493
---
M src/openvpnserv/common.c
M src/openvpnserv/service.h
M src/openvpnserv/validate.c
3 files changed, 10 insertions(+), 2 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/14/1714/1

diff --git a/src/openvpnserv/common.c b/src/openvpnserv/common.c
index 7fc8c14..5db5891 100644
--- a/src/openvpnserv/common.c
+++ b/src/openvpnserv/common.c
@@ -115,6 +115,13 @@
         goto out;
     }

+    swprintf(default_value, _countof(default_value), L"%ls\\config-auto\\", 
install_path);
+    error = GetRegString(key, L"autostart_config_dir", 
s->autostart_config_dir, sizeof(s->autostart_config_dir), default_value);
+    if (error != ERROR_SUCCESS || 
!ensure_trailing_backslash(s->autostart_config_dir, 
_countof(s->autostart_config_dir)))
+    {
+        goto out;
+    }
+
     swprintf(default_value, _countof(default_value), L"%ls\\bin\\", 
install_path);
     error = GetRegString(key, L"bin_dir", s->bin_dir, sizeof(s->bin_dir), 
default_value);
     if (error != ERROR_SUCCESS || !ensure_trailing_backslash(s->bin_dir, 
_countof(s->bin_dir)))
diff --git a/src/openvpnserv/service.h b/src/openvpnserv/service.h
index c67d67f..9ef9744 100644
--- a/src/openvpnserv/service.h
+++ b/src/openvpnserv/service.h
@@ -65,6 +65,7 @@
 {
     WCHAR exe_path[MAX_PATH];
     WCHAR config_dir[MAX_PATH];
+    WCHAR autostart_config_dir[MAX_PATH];
     WCHAR bin_dir[MAX_PATH];
     WCHAR ext_string[16];
     WCHAR log_dir[MAX_PATH];
diff --git a/src/openvpnserv/validate.c b/src/openvpnserv/validate.c
index 5cf8325..8f7adcd 100644
--- a/src/openvpnserv/validate.c
+++ b/src/openvpnserv/validate.c
@@ -54,7 +54,7 @@
 static PTOKEN_GROUPS GetTokenGroups(const HANDLE token);

 /*
- * Check that config path is inside config_dir
+ * Check that config path is inside config_dir or autostart_config_dir
  * The logic here is simple: if the path isn't prefixed with config_dir it's 
rejected
  */
 static BOOL
@@ -78,7 +78,7 @@
         res = PathCchCanonicalize(config_path, _countof(config_path), fname);
     }

-    return res == S_OK && wcsnicmp(config_path, s->config_dir, 
wcslen(s->config_dir)) == 0;
+    return res == S_OK && (wcsnicmp(config_path, s->config_dir, 
wcslen(s->config_dir)) == 0 || wcsnicmp(config_path, s->autostart_config_dir, 
wcslen(s->autostart_config_dir)) == 0);
 }



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1714?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings?usp=email

Gerrit-MessageType: newchange
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I52609b71deb8810a3fea639494b5fa13d9e5a493
Gerrit-Change-Number: 1714
Gerrit-PatchSet: 1
Gerrit-Owner: selvanair <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: plaisthos <[email protected]>
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Reply via email to