From: Simon Tatham Date: Mon, 25 Nov 2013 18:35:14 +0000 (+0000) Subject: Switch to using SIDs in make_private_security_descriptor(). X-Git-Url: https://asedeno.scripts.mit.edu/gitweb/?a=commitdiff_plain;h=c097e177aee4d113d7a227de9aec70021e8751ea;p=PuTTY_svn.git 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 9cc8176f..a172628f 100644 --- 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 27f04dd5..d04e88a9 100644 --- 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 57de5d1d..bd649827 100644 --- 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 1ad20ba1..17bad46d 100644 --- 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);