Skip to contentSkip to conversation

Handle localized Administrators group name in windows

PATCH In review Selva Nair 2 participants 2 messages Mar 5, 2016 – Mar 6, 2016

lore
Share & tools
lore

Selva Nair

author
#0 lore raw

PATCH 1/1 Handle localized Administrators group name in windows

Interactive service allows all configs and options if the user is in "Administrators" group. This patch makes it work even if the admin group is renamed or localized.

While at it, also remove two unused variables in validate.c.

Thanks to Leonardo Basilio <[email protected]...> for testing the patch on a localized version of windows and Samuli Seppänen <[email protected]...> for pointing out this issue.

---
 src/openvpnserv/validate.c |   45 +++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 42 insertions(+), 3 deletions(-)
src/openvpnserv/validate.c +42 -3
index b5809b3..7458d75 100644
@@ -57,8 +57,6 @@ static BOOL
 CheckConfigPath (const WCHAR *workdir, const WCHAR *fname, const settings_t *s)
 {
     WCHAR tmp[MAX_PATH];
-    WCHAR widepath[MAX_PATH];
-    WCHAR relpath[MAX_PATH];
     const WCHAR *config_file = NULL;
     const WCHAR *config_dir = NULL;
 
@@ -112,6 +110,36 @@ OptionLookup (const WCHAR *name, const WCHAR *white_list[])
 }
 
 /*
+ * The Administrators group may be localized or renamed by admins.
+ * Get the local name of the group using the SID.
+ */
+static BOOL
+GetBuiltinAdminGroupName (WCHAR *name, DWORD nlen)
+{
+    BOOL b = FALSE;
+    PSID admin_sid = NULL;
+    DWORD sid_size = SECURITY_MAX_SID_SIZE;
+    SID_NAME_USE snu;
+
+    WCHAR domain[MAX_NAME];
+    DWORD dlen = _countof(domain);
+
+    admin_sid = malloc(sid_size);
+    if (!admin_sid)
+        return FALSE;
+
+    b = CreateWellKnownSid(WinBuiltinAdministratorsSid, NULL, admin_sid,  &sid_size);
+    if(b)
+    {
+        b = LookupAccountSidW(NULL, admin_sid, name, &nlen, domain, &dlen, &snu);
+    }
+
+    free (admin_sid);
+
+    return b;
+}
+
+/*
  * Check whether user is a member of Administrators group or
  * the group specified in s->ovpn_admin_group
  */
@@ -125,6 +153,7 @@ IsAuthorizedUser (SID *sid, settings_t *s)
     const WCHAR *admin_group[2];
     WCHAR username[MAX_NAME];
     WCHAR domain[MAX_NAME];
+    WCHAR sysadmin_group[MAX_NAME];
     DWORD err, len = MAX_NAME;
     int i;
     BOOL ret = FALSE;
@@ -147,7 +176,17 @@ IsAuthorizedUser (SID *sid, settings_t *s)
         goto out;
     }
 
-    admin_group[0] = SYSTEM_ADMIN_GROUP;
+    if (GetBuiltinAdminGroupName(sysadmin_group, _countof(sysadmin_group)))
+    {
+        admin_group[0] = sysadmin_group;
+    }
+    else
+    {
+        MsgToEventLog (M_SYSERR, TEXT("Failed to get the name of Administrators group. Using the default."));
+        /* use the default value */
+        admin_group[0] = SYSTEM_ADMIN_GROUP;
+    }
+
 #ifdef UNICODE
     admin_group[1] = s->ovpn_admin_group;
 #else
Signature · 1 line
1.7.10.4
#1 lore raw

ACK, thanks (based on "stare at code and MSDN docs" and Leonardo's testing).

Your patch has been applied to the master branch.

commit 6370f703573c6284e0b3c5935ab204285cdda8e6

Author: Selva Nair
Date:   Sat Mar 5 14:39:56 2016 -0500
     Handle localized Administrators group name in windows

     Signed-off-by: Selva Nair <[email protected]...>
     Acked-by: Gert Doering <[email protected]...>
     Message-Id: <[email protected]...>
     URL: http://article.gmane.org/gmane.network.openvpn.devel/11316
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering