> -----Original Message-----
> From: libssh2-devel [mailto:libssh2-devel-bounces_at_cool.haxx.se] On Behalf Of
> Peter Stuge
> Sent: woensdag 25 februari 2015 03:40
> To: libssh2-devel_at_cool.haxx.se
> Subject: Re: Ok, let's talk release again. For real.
>
> Bert Huijben wrote:
> > I currently use a patch in my own build (that version is attached).
>
> Is the new code backwards compatible - ie. works also with older
> versions of pageant?
The relevant change in putty's own code is in r9043 (svn://svn.tartarus.org/sgt/putty)
[[
------------------------------------------------------------------------
r9043 | simon | 2010-12-23 16:22:50 +0100 (do, 23 dec 2010) | 5 lines
Changed paths:
M /putty/windows/winpgntc.c
M /putty/windows/winstuff.h
More careful owner SID selection in the Pageant client code. This
should solve some of the SID-mismatch issues we've occasionally had
reported. Because it's a modification on the client side, it doesn't
affect the security of Pageant itself.
Index: putty/windows/winstuff.h
===================================================================
--- putty/windows/winstuff.h (revision 9042)
+++ putty/windows/winstuff.h (revision 9043)
@@ -96,9 +96,9 @@
#define STR1(x) #x
#define STR(x) STR1(x)
#define GET_WINDOWS_FUNCTION_PP(module, name) \
- p_##name = module ? (t_##name) GetProcAddress(module, STR(name)) : NULL
+ (p_##name = module ? (t_##name) GetProcAddress(module, STR(name)) : NULL)
#define GET_WINDOWS_FUNCTION(module, name) \
- p_##name = module ? (t_##name) GetProcAddress(module, #name) : NULL
+ (p_##name = module ? (t_##name) GetProcAddress(module, #name) : NULL)
/*
* Global variables. Most modules declare these `extern', but
Index: putty/windows/winpgntc.c
===================================================================
--- putty/windows/winpgntc.c (revision 9042)
+++ putty/windows/winpgntc.c (revision 9043)
@@ -7,6 +7,10 @@
#include "putty.h"
+#ifndef NO_SECURITY
+#include <aclapi.h>
+#endif
+
#define AGENT_COPYDATA_ID 0x804e50ba /* random goop */
#define AGENT_MAX_MSGLEN 8192
@@ -66,6 +70,33 @@
#endif
+/*
+ * Dynamically load advapi32.dll for SID manipulation. In its absence,
+ * we degrade gracefully.
+ */
+#ifndef NO_SECURITY
+int advapi_initialised = FALSE;
+static HMODULE advapi;
+DECL_WINDOWS_FUNCTION(static, BOOL, OpenProcessToken,
+ (HANDLE, DWORD, PHANDLE));
+DECL_WINDOWS_FUNCTION(static, BOOL, GetTokenInformation,
+ (HANDLE, TOKEN_INFORMATION_CLASS,
+ LPVOID, DWORD, PDWORD));
+DECL_WINDOWS_FUNCTION(static, BOOL, InitializeSecurityDescriptor,
+ (PSECURITY_DESCRIPTOR, DWORD));
+DECL_WINDOWS_FUNCTION(static, BOOL, SetSecurityDescriptorOwner,
+ (PSECURITY_DESCRIPTOR, PSID, BOOL));
+static int init_advapi(void)
+{
+ advapi = load_system32_dll("advapi32.dll");
+ return advapi &&
+ GET_WINDOWS_FUNCTION(advapi, OpenProcessToken) &&
+ GET_WINDOWS_FUNCTION(advapi, GetTokenInformation) &&
+ GET_WINDOWS_FUNCTION(advapi, InitializeSecurityDescriptor) &&
+ GET_WINDOWS_FUNCTION(advapi, SetSecurityDescriptorOwner);
+}
+#endif
+
int agent_query(void *in, int inlen, void **out, int *outlen,
void (*callback)(void *, void *, int), void *callback_ctx)
{
@@ -75,6 +106,10 @@
unsigned char *p, *ret;
int id, retlen;
COPYDATASTRUCT cds;
+ SECURITY_ATTRIBUTES sa, *psa;
+ PSECURITY_DESCRIPTOR psd = NULL;
+ HANDLE proc, tok;
+ TOKEN_USER *user = NULL;
*out = NULL;
*outlen = 0;
@@ -83,7 +118,57 @@
if (!hwnd)
return 1; /* *out == NULL, so failure */
mapname = dupprintf("PageantRequest%08x", (unsigned)GetCurrentThreadId());
- filemap = CreateFileMapping(INVALID_HANDLE_VALUE, NULL, PAGE_READWRITE,
+
+#ifndef NO_SECURITY
+ if (advapi_initialised || init_advapi()) {
+ /*
+ * Make the file mapping we create for communication with
+ * Pageant owned by the user SID rather than the default. This
+ * should make communication between processes with slightly
+ * different contexts more reliable: in particular, command
+ * prompts launched as administrator should still be able to
+ * 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);
+ }
+
+ psa = NULL;
+ if (user) {
+ 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)) {
+ sa.nLength = sizeof(sa);
+ sa.bInheritHandle = TRUE;
+ sa.lpSecurityDescriptor = psd;
+ psa = &sa;
+ } else {
+ LocalFree(psd);
+ psd = NULL;
+ }
+ }
+ }
+ }
+#endif /* NO_SECURITY */
+
+ filemap = CreateFileMapping(INVALID_HANDLE_VALUE, psa, PAGE_READWRITE,
0, AGENT_MAX_MSGLEN, mapname);
if (filemap == NULL || filemap == INVALID_HANDLE_VALUE)
return 1; /* *out == NULL, so failure */
@@ -134,5 +219,9 @@
}
UnmapViewOfFile(p);
CloseHandle(filemap);
+ if (psd)
+ LocalFree(psd);
+ if (user)
+ LocalFree(user);
return 1;
}
]]
After that pageant was updated to allow both this extended value as the previous value in r9178, r9264, to extend the number of supported use cases even more.
The change itself is not relevant, but let me quote r9264's log message:
[[
r9264 | simon | 2011-08-13 16:48:36 +0200 (za, 13 aug 2011) | 8 lines
Readjust Pageant's SID check _again_, to make it the union of the
policies before and after r9178, and hence able to talk to both
0.60-like and 0.61-like clients.
I had failed to consider that many pieces of code derived from PuTTY
would have imported the Pageant client code, so we shouldn't randomly
stop supporting things just because _we_ aren't using them anymore.
]]
The code in libssh2 clearly falls in this category.
Bert
_______________________________________________
libssh2-devel http://cool.haxx.se/cgi-bin/mailman/listinfo/libssh2-devel
Received on 2015-02-28