diff --git a/windows/winpgnt.c b/windows/winpgnt.c index 3451feca..2e7154a0 100644 --- a/windows/winpgnt.c +++ b/windows/winpgnt.c @@ -19,6 +19,10 @@ #ifndef NO_SECURITY #include +#ifdef DEBUG_IPC +#define _WIN32_WINNT 0x0500 /* for ConvertSidToStringSid */ +#include +#endif #endif #define IDI_MAINICON 200 @@ -113,7 +117,7 @@ static tree234 *rsakeys, *ssh2keys; static int has_security; #ifndef NO_SECURITY -DECL_WINDOWS_FUNCTION(static, DWORD, GetSecurityInfo, +DECL_WINDOWS_FUNCTION(extern, DWORD, GetSecurityInfo, (HANDLE, SE_OBJECT_TYPE, SECURITY_INFORMATION, PSID *, PSID *, PACL *, PACL *, PSECURITY_DESCRIPTOR *)); @@ -1817,8 +1821,7 @@ static LRESULT CALLBACK WndProc(HWND hwnd, UINT message, void *p; HANDLE filemap; #ifndef NO_SECURITY - HANDLE proc; - PSID mapowner, procowner; + PSID mapowner, ourself; PSECURITY_DESCRIPTOR psd1 = NULL, psd2 = NULL; #endif int ret = 0; @@ -1840,40 +1843,35 @@ static LRESULT CALLBACK WndProc(HWND hwnd, UINT message, #ifndef NO_SECURITY int rc; if (has_security) { - if ((proc = OpenProcess(MAXIMUM_ALLOWED, FALSE, - GetCurrentProcessId())) == - NULL) { + if ((ourself = get_user_sid()) == NULL) { #ifdef DEBUG_IPC - debug(("couldn't get handle for process\n")); + debug(("couldn't get user SID\n")); #endif return 0; - } - if (p_GetSecurityInfo(proc, SE_KERNEL_OBJECT, - OWNER_SECURITY_INFORMATION, - &procowner, NULL, NULL, NULL, - &psd2) != ERROR_SUCCESS) { -#ifdef DEBUG_IPC - debug(("couldn't get owner info for process\n")); -#endif - CloseHandle(proc); - return 0; /* unable to get security info */ - } - CloseHandle(proc); + } + if ((rc = p_GetSecurityInfo(filemap, SE_KERNEL_OBJECT, OWNER_SECURITY_INFORMATION, &mapowner, NULL, NULL, NULL, &psd1) != ERROR_SUCCESS)) { #ifdef DEBUG_IPC - debug( - ("couldn't get owner info for filemap: %d\n", - rc)); + debug(("couldn't get owner info for filemap: %d\n", + rc)); #endif return 0; } #ifdef DEBUG_IPC - debug(("got security stuff\n")); + { + LPTSTR ours, theirs; + ConvertSidToStringSid(mapowner, &theirs); + ConvertSidToStringSid(ourself, &ours); + debug(("got both sids: ours=%s theirs=%s\n", + ours, theirs)); + LocalFree(ours); + LocalFree(theirs); + } #endif - if (!EqualSid(mapowner, procowner)) + if (!EqualSid(mapowner, ourself)) return 0; /* security ID mismatch! */ #ifdef DEBUG_IPC debug(("security stuff matched\n")); @@ -1892,9 +1890,9 @@ static LRESULT CALLBACK WndProc(HWND hwnd, UINT message, { int i; for (i = 0; i < 5; i++) - debug( - ("p[%d]=%02x\n", i, - ((unsigned char *) p)[i]));} + debug(("p[%d]=%02x\n", i, + ((unsigned char *) p)[i])); + } #endif answer_msg(p); ret = 1; @@ -1972,9 +1970,7 @@ int WINAPI WinMain(HINSTANCE inst, HINSTANCE prev, LPSTR cmdline, int show) /* * Attempt to get the security API we need. */ - advapi = load_system32_dll("advapi32.dll"); - GET_WINDOWS_FUNCTION(advapi, GetSecurityInfo); - if (!p_GetSecurityInfo) { + if (!init_advapi()) { MessageBox(NULL, "Unable to access security APIs. Pageant will\n" "not run, in case it causes a security breach.", diff --git a/windows/winpgntc.c b/windows/winpgntc.c index 5407f2d8..de2b7009 100644 --- a/windows/winpgntc.c +++ b/windows/winpgntc.c @@ -86,15 +86,70 @@ DECL_WINDOWS_FUNCTION(static, BOOL, InitializeSecurityDescriptor, (PSECURITY_DESCRIPTOR, DWORD)); DECL_WINDOWS_FUNCTION(static, BOOL, SetSecurityDescriptorOwner, (PSECURITY_DESCRIPTOR, PSID, BOOL)); -static int init_advapi(void) +DECL_WINDOWS_FUNCTION(, DWORD, GetSecurityInfo, + (HANDLE, SE_OBJECT_TYPE, SECURITY_INFORMATION, + PSID *, PSID *, PACL *, PACL *, + PSECURITY_DESCRIPTOR *)); +int init_advapi(void) { advapi = load_system32_dll("advapi32.dll"); return advapi && + GET_WINDOWS_FUNCTION(advapi, GetSecurityInfo) && GET_WINDOWS_FUNCTION(advapi, OpenProcessToken) && GET_WINDOWS_FUNCTION(advapi, GetTokenInformation) && GET_WINDOWS_FUNCTION(advapi, InitializeSecurityDescriptor) && GET_WINDOWS_FUNCTION(advapi, SetSecurityDescriptorOwner); } + +PSID get_user_sid(void) +{ + HANDLE proc = NULL, tok = NULL; + TOKEN_USER *user = NULL; + DWORD toklen, sidlen; + PSID sid = NULL, ret = NULL; + + if ((proc = OpenProcess(MAXIMUM_ALLOWED, FALSE, + GetCurrentProcessId())) == NULL) + goto cleanup; + + if (!p_OpenProcessToken(proc, TOKEN_QUERY, &tok)) + goto cleanup; + + if (!p_GetTokenInformation(tok, TokenUser, NULL, 0, &toklen) && + GetLastError() != ERROR_INSUFFICIENT_BUFFER) + goto cleanup; + + if ((user = (TOKEN_USER *)LocalAlloc(LPTR, toklen)) == NULL) + goto cleanup; + + if (!p_GetTokenInformation(tok, TokenUser, user, toklen, &toklen)) + goto cleanup; + + sidlen = GetLengthSid(user->User.Sid); + + sid = (PSID)smalloc(sidlen); + + if (!CopySid(sidlen, sid, user->User.Sid)) + goto cleanup; + + /* Success. Move sid into the return value slot, and null it out + * to stop the cleanup code freeing it. */ + ret = sid; + sid = NULL; + + cleanup: + if (proc != NULL) + CloseHandle(proc); + if (tok != NULL) + CloseHandle(tok); + if (user != NULL) + LocalFree(user); + if (sid != NULL) + sfree(sid); + + return ret; +} + #endif int agent_query(void *in, int inlen, void **out, int *outlen, @@ -108,8 +163,7 @@ int agent_query(void *in, int inlen, void **out, int *outlen, COPYDATASTRUCT cds; SECURITY_ATTRIBUTES sa, *psa; PSECURITY_DESCRIPTOR psd = NULL; - HANDLE proc, tok; - TOKEN_USER *user = NULL; + PSID usersid = NULL; *out = NULL; *outlen = 0; @@ -130,31 +184,16 @@ int agent_query(void *in, int inlen, void **out, int *outlen, * run PSFTPs which refer back to the owning user's * unprivileged Pageant. */ - - if ((proc = OpenProcess(MAXIMUM_ALLOWED, FALSE, - GetCurrentProcessId())) != NULL) { - if (p_OpenProcessToken(proc, TOKEN_QUERY, &tok)) { - DWORD retlen; - p_GetTokenInformation(tok, TokenUser, NULL, 0, &retlen); - user = (TOKEN_USER *)LocalAlloc(LPTR, retlen); - if (!p_GetTokenInformation(tok, TokenUser, - user, retlen, &retlen)) { - LocalFree(user); - user = NULL; - } - CloseHandle(tok); - } - CloseHandle(proc); - } + usersid = get_user_sid(); psa = NULL; - if (user) { + if (usersid) { psd = (PSECURITY_DESCRIPTOR) LocalAlloc(LPTR, SECURITY_DESCRIPTOR_MIN_LENGTH); if (psd) { if (p_InitializeSecurityDescriptor (psd, SECURITY_DESCRIPTOR_REVISION) && - p_SetSecurityDescriptorOwner(psd, user->User.Sid, FALSE)) { + p_SetSecurityDescriptorOwner(psd, usersid, FALSE)) { sa.nLength = sizeof(sa); sa.bInheritHandle = TRUE; sa.lpSecurityDescriptor = psd; @@ -221,7 +260,6 @@ int agent_query(void *in, int inlen, void **out, int *outlen, CloseHandle(filemap); if (psd) LocalFree(psd); - if (user) - LocalFree(user); + sfree(usersid); return 1; } diff --git a/windows/winstuff.h b/windows/winstuff.h index 6d5175d5..feaf9e9d 100644 --- a/windows/winstuff.h +++ b/windows/winstuff.h @@ -497,7 +497,7 @@ int handle_backlog(struct handle *h); void *handle_get_privdata(struct handle *h); /* - * pageantc.c needs to schedule callbacks for asynchronous agent + * winpgntc.c needs to schedule callbacks for asynchronous agent * requests. This has to be done differently in GUI and console, so * there's an exported function used for the purpose. * @@ -508,6 +508,14 @@ void agent_schedule_callback(void (*callback)(void *, void *, int), void *callback_ctx, void *data, int len); #define FLAG_SYNCAGENT 0x1000 +/* + * winpgntc.c also exports these two functions which are used by the + * server side of Pageant as well, to get the user SID for comparing + * with clients'. + */ +int init_advapi(void); /* initialises everything needed by get_user_sid */ +PSID get_user_sid(void); + /* * Exports from winser.c. */