Switch to using SIDs in make_private_security_descriptor().

Daniel Meidlinger reports that at least one Windows machine which is
not obviously otherwise misconfigured will respond to our
SetEntriesInAcl call with odd errors like ERROR_NONE_MAPPED or
ERROR_TRUSTED_RELATIONSHIP_FAILURE. This is apparently to do with
failure to convert the names "EVERYONE" and "CURRENT_USER" used in the
ACL specification to SIDs. (Or perhaps only one of them is the problem
- I didn't investigate in that direction.)

If we instead construct a fully SID-based ACL, using the well-known
world SID in place of EVERYONE and calling our existing get_user_sid
routine in place of CURRENT_USER, he reports that the problem goes
away, so let's do that instead.

While I'm here, I've slightly simplified the function prototype of
make_private_security_descriptor(), by turning 'networksid' into an
internal static that we can reuse in subsequent calls once we've set
it up. (Mostly because I didn't fancy adding another two pointless
parameters at every call site for the two new SIDs.)

git-svn-id: http://svn.tartarus.org/sgt/putty@10096 cda61777-01e9-0310-a592-d414129be87e

diff --git a/windows/winnps.c b/windows/winnps.c
index 9cc8176fdefdc63091add8123af4886d3836127e..a172628f86c3366bec2c7d514897a81b142cf6ed 100644 (file)
--- a/windows/winnps.c
+++ b/windows/winnps.c
@@ -26,7 +26,6 @@ struct Socket_named_pipe_server_tag {
 
     /* Parameters for (repeated) creation of named pipe objects */
     PSECURITY_DESCRIPTOR psd;
-    PSID networksid;
     PACL acl;
     char *pipename;
 
@@ -56,8 +55,6 @@ static void sk_namedpipeserver_close(Socket s)
     CloseHandle(ps->connect_ovl.hEvent);
     sfree(ps->error);
     sfree(ps->pipename);
-    if (ps->networksid)
-        LocalFree(ps->networksid);
     if (ps->acl)
         LocalFree(ps->acl);
     if (ps->psd)
@@ -222,15 +219,13 @@ Socket new_named_pipe_listener(const char *pipename, Plug plug)
     ret->error = NULL;
     ret->psd = NULL;
     ret->pipename = dupstr(pipename);
-    ret->networksid = NULL;
     ret->acl = NULL;
 
     assert(strncmp(pipename, "\\\\.\\pipe\\", 9) == 0);
     assert(strchr(pipename + 9, '\\') == NULL);
 
     if (!make_private_security_descriptor(GENERIC_READ | GENERIC_WRITE,
-                                          &ret->psd, &ret->networksid,
-                                          &ret->acl, &ret->error)) {
+                                          &ret->psd, &ret->acl, &ret->error)) {
         goto cleanup;
     }
 

diff --git a/windows/winsecur.c b/windows/winsecur.c
index 27f04dd59e1c4da05796428205141c6c2257aa02..d04e88a92370bbb57b0dff65e3dcbabba7d4e1eb 100644 (file)
--- a/windows/winsecur.c
+++ b/windows/winsecur.c
@@ -101,17 +101,19 @@ PSID get_user_sid(void)
 
 int make_private_security_descriptor(DWORD permissions,
                                      PSECURITY_DESCRIPTOR *psd,
-                                     PSID *networksid,
                                      PACL *acl,
                                      char **error)
 {
+    SID_IDENTIFIER_AUTHORITY world_auth = SECURITY_WORLD_SID_AUTHORITY;
     SID_IDENTIFIER_AUTHORITY nt_auth = SECURITY_NT_AUTHORITY;
     EXPLICIT_ACCESS ea[3];
     int acl_err;
     int ret = FALSE;
 
+    /* Initialised once, then kept around to reuse forever */
+    static PSID worldsid, networksid, usersid;
+
     *psd = NULL;
-    *networksid = NULL;
     *acl = NULL;
     *error = NULL;
 
@@ -120,30 +122,49 @@ int make_private_security_descriptor(DWORD permissions,
         goto cleanup;
     }
 
-    if (!AllocateAndInitializeSid(&nt_auth, 1, SECURITY_NETWORK_RID,
-                                  0, 0, 0, 0, 0, 0, 0, networksid)) {
-        *error = dupprintf("unable to construct SID for "
-                           "local same-user access only: %s",
-                           win_strerror(GetLastError()));
-        goto cleanup;
+    if (!usersid) {
+        if ((usersid = get_user_sid()) == NULL) {
+            *error = dupprintf("unable to construct SID for current user: %s",
+                               win_strerror(GetLastError()));
+            goto cleanup;
+        }
+    }
+
+    if (!worldsid) {
+        if (!AllocateAndInitializeSid(&world_auth, 1, SECURITY_WORLD_RID,
+                                      0, 0, 0, 0, 0, 0, 0, &worldsid)) {
+            *error = dupprintf("unable to construct SID for world: %s",
+                               win_strerror(GetLastError()));
+            goto cleanup;
+        }
+    }
+
+    if (!networksid) {
+        if (!AllocateAndInitializeSid(&nt_auth, 1, SECURITY_NETWORK_RID,
+                                      0, 0, 0, 0, 0, 0, 0, &networksid)) {
+            *error = dupprintf("unable to construct SID for "
+                               "local same-user access only: %s",
+                               win_strerror(GetLastError()));
+            goto cleanup;
+        }
     }
 
     memset(ea, 0, sizeof(ea));
     ea[0].grfAccessPermissions = permissions;
     ea[0].grfAccessMode = REVOKE_ACCESS;
     ea[0].grfInheritance = NO_INHERITANCE;
-    ea[0].Trustee.TrusteeForm = TRUSTEE_IS_NAME;
-    ea[0].Trustee.ptstrName = "EVERYONE";
+    ea[0].Trustee.TrusteeForm = TRUSTEE_IS_SID;
+    ea[0].Trustee.ptstrName = (LPTSTR)worldsid;
     ea[1].grfAccessPermissions = permissions;
     ea[1].grfAccessMode = GRANT_ACCESS;
     ea[1].grfInheritance = NO_INHERITANCE;
-    ea[1].Trustee.TrusteeForm = TRUSTEE_IS_NAME;
-    ea[1].Trustee.ptstrName = "CURRENT_USER";
+    ea[1].Trustee.TrusteeForm = TRUSTEE_IS_SID;
+    ea[1].Trustee.ptstrName = (LPTSTR)usersid;
     ea[2].grfAccessPermissions = permissions;
     ea[2].grfAccessMode = REVOKE_ACCESS;
     ea[2].grfInheritance = NO_INHERITANCE;
     ea[2].Trustee.TrusteeForm = TRUSTEE_IS_SID;
-    ea[2].Trustee.ptstrName = (LPTSTR)*networksid;
+    ea[2].Trustee.ptstrName = (LPTSTR)networksid;
 
     acl_err = p_SetEntriesInAclA(3, ea, NULL, acl);
     if (acl_err != ERROR_SUCCESS || *acl == NULL) {
@@ -180,10 +201,6 @@ int make_private_security_descriptor(DWORD permissions,
             LocalFree(*psd);
             *psd = NULL;
         }
-        if (*networksid) {
-            LocalFree(*networksid);
-            *networksid = NULL;
-        }
         if (*acl) {
             LocalFree(*acl);
             *acl = NULL;

diff --git a/windows/winsecur.h b/windows/winsecur.h
index 57de5d1de8c5f7c7c44aee94b67363d6b9097156..bd64982768aaf3d18f4add9c15c88730928f08cf 100644 (file)
--- a/windows/winsecur.h
+++ b/windows/winsecur.h
@@ -50,15 +50,13 @@ PSID get_user_sid(void);
  * servers, i.e. allowing access only to the current user id and also
  * only local (i.e. not over SMB) connections.
  *
- * If this function returns TRUE, then 'psd', 'networksid' and 'acl'
- * will all have been filled in with memory allocated using LocalAlloc
- * (and hence must be freed later using LocalFree). If it returns
- * FALSE, then instead 'error' has been filled with a dynamically
- * allocated error message.
+ * If this function returns TRUE, then 'psd' and 'acl' will have been
+ * filled in with memory allocated using LocalAlloc (and hence must be
+ * freed later using LocalFree). If it returns FALSE, then instead
+ * 'error' has been filled with a dynamically allocated error message.
  */
 int make_private_security_descriptor(DWORD permissions,
                                      PSECURITY_DESCRIPTOR *psd,
-                                     PSID *networksid,
                                      PACL *acl,
                                      char **error);
 

diff --git a/windows/winshare.c b/windows/winshare.c
index 1ad20ba1148150afbaffb81083106cb42092206c..17bad46d913e548c4355894509b5af271ead0b07 100644 (file)
--- a/windows/winshare.c
+++ b/windows/winshare.c
@@ -115,7 +115,6 @@ int platform_ssh_share(const char *pi_name, Conf *conf,
     Socket retsock;
     PSECURITY_DESCRIPTOR psd;
     PACL acl;
-    PSID networksid;
 
     /*
      * Transform the platform-independent version of the connection
@@ -140,8 +139,7 @@ int platform_ssh_share(const char *pi_name, Conf *conf,
 
         mutexname = make_name(CONNSHARE_MUTEX_PREFIX, name);
         if (!make_private_security_descriptor(MUTEX_ALL_ACCESS,
-                                              &psd, &networksid,
-                                              &acl, logtext)) {
+                                              &psd, &acl, logtext)) {
             sfree(mutexname);
             return SHARE_NONE;
         }
@@ -158,14 +156,12 @@ int platform_ssh_share(const char *pi_name, Conf *conf,
                                  mutexname, win_strerror(GetLastError()));
             sfree(mutexname);
             LocalFree(psd);
-            LocalFree(networksid);
             LocalFree(acl);
             return SHARE_NONE;
         }
 
         sfree(mutexname);
         LocalFree(psd);
-        LocalFree(networksid);
         LocalFree(acl);
 
         WaitForSingleObject(mutex, INFINITE);