From f7adbaba9252ccf5bc300134cbe2e04ac32b187f Mon Sep 17 00:00:00 2001 From: Bill Zissimopoulos Date: Wed, 18 May 2016 10:05:33 -0700 Subject: [PATCH] dll: FspCallNamedPipeSecurely replaces CallNamedPipeW --- build/VStudio/launcher/launchctl.vcxproj | 5 + inc/winfsp/winfsp.h | 4 + src/dll/np.c | 115 ++++++++++++++++++++++- src/launcher/launchctl.c | 8 +- src/launcher/launcher.h | 15 ++- 5 files changed, 131 insertions(+), 16 deletions(-) diff --git a/build/VStudio/launcher/launchctl.vcxproj b/build/VStudio/launcher/launchctl.vcxproj index 7745a77a..cfc460ca 100644 --- a/build/VStudio/launcher/launchctl.vcxproj +++ b/build/VStudio/launcher/launchctl.vcxproj @@ -183,6 +183,11 @@ _UNICODE;UNICODE;%(PreprocessorDefinitions);MyProductName=$(MyProductName);MyDescription=$(MyDescription);MyCompanyName=$(MyCompanyName);MyCopyright=$(MyCopyright);MyVersion=$(MyVersion);MyVersionWithCommas=$(MyVersionWithCommas) + + + {4a7c0b21-9e10-4c81-92de-1493efcf24eb} + + diff --git a/inc/winfsp/winfsp.h b/inc/winfsp/winfsp.h index c09e7c10..802589ee 100644 --- a/inc/winfsp/winfsp.h +++ b/inc/winfsp/winfsp.h @@ -1034,6 +1034,10 @@ FSP_API VOID FspEventLogV(ULONG Type, PWSTR Format, va_list ap); FSP_API VOID FspDebugLog(const char *format, ...); FSP_API VOID FspDebugLogSD(const char *format, PSECURITY_DESCRIPTOR SecurityDescriptor); FSP_API VOID FspDebugLogFT(const char *format, PFILETIME FileTime); +FSP_API NTSTATUS FspCallNamedPipeSecurely(PWSTR PipeName, + PVOID InBuffer, ULONG InBufferSize, PVOID OutBuffer, ULONG OutBufferSize, + PULONG PBytesTransferred, ULONG Timeout, + PSID Sid); #ifdef __cplusplus } diff --git a/src/dll/np.c b/src/dll/np.c index 48c25ffd..464e2eef 100644 --- a/src/dll/np.c +++ b/src/dll/np.c @@ -17,11 +17,117 @@ #include #include +#include #include #define FSP_NP_NAME LIBRARY_NAME ".Np" #define FSP_NP_TYPE ' spF' /* pick a value hopefully not in use */ +FSP_API NTSTATUS FspCallNamedPipeSecurely(PWSTR PipeName, + PVOID InBuffer, ULONG InBufferSize, PVOID OutBuffer, ULONG OutBufferSize, + PULONG PBytesTransferred, ULONG Timeout, + PSID Sid) +{ + NTSTATUS Result; + HANDLE Pipe = INVALID_HANDLE_VALUE; + DWORD PipeMode; + + Pipe = CreateFileW(PipeName, + GENERIC_READ | FILE_WRITE_DATA | FILE_WRITE_ATTRIBUTES, + FILE_SHARE_READ | FILE_SHARE_WRITE, 0, OPEN_EXISTING, + SECURITY_SQOS_PRESENT | SECURITY_IDENTIFICATION, 0); + if (INVALID_HANDLE_VALUE == Pipe) + { + if (ERROR_PIPE_BUSY != GetLastError()) + { + Result = FspNtStatusFromWin32(GetLastError()); + goto exit; + } + + WaitNamedPipeW(PipeName, Timeout); + + Pipe = CreateFileW(PipeName, + GENERIC_READ | FILE_WRITE_DATA | FILE_WRITE_ATTRIBUTES, + FILE_SHARE_READ | FILE_SHARE_WRITE, 0, OPEN_EXISTING, + SECURITY_SQOS_PRESENT | SECURITY_IDENTIFICATION, 0); + if (INVALID_HANDLE_VALUE == Pipe) + { + Result = FspNtStatusFromWin32(GetLastError()); + goto exit; + } + } + + if (0 != Sid) + { + PSECURITY_DESCRIPTOR SecurityDescriptor = 0; + PSID OwnerSid, WellKnownSid = 0; + DWORD SidSize, LastError; + + /* if it is a small number treat it like a well known SID */ + if (1024 > (INT_PTR)Sid) + { + SidSize = SECURITY_MAX_SID_SIZE; + WellKnownSid = MemAlloc(SidSize); + if (0 == WellKnownSid) + { + Result = STATUS_INSUFFICIENT_RESOURCES; + goto sid_exit; + } + + if (!CreateWellKnownSid((INT_PTR)Sid, 0, WellKnownSid, &SidSize)) + { + Result = FspNtStatusFromWin32(GetLastError()); + goto sid_exit; + } + } + + LastError = GetSecurityInfo(Pipe, SE_FILE_OBJECT, + OWNER_SECURITY_INFORMATION, &OwnerSid, 0, 0, 0, &SecurityDescriptor); + if (0 != LastError) + { + Result = FspNtStatusFromWin32(GetLastError()); + goto sid_exit; + } + + if (!EqualSid(OwnerSid, WellKnownSid ? WellKnownSid : Sid)) + { + Result = STATUS_ACCESS_DENIED; + goto sid_exit; + } + + Result = STATUS_SUCCESS; + + sid_exit: + MemFree(WellKnownSid); + LocalFree(SecurityDescriptor); + + if (!NT_SUCCESS(Result)) + goto exit; + } + + PipeMode = PIPE_READMODE_MESSAGE | PIPE_WAIT; + if (!SetNamedPipeHandleState(Pipe, &PipeMode, 0, 0)) + { + Result = FspNtStatusFromWin32(GetLastError()); + goto exit; + } + + if (!TransactNamedPipe(Pipe, InBuffer, InBufferSize, OutBuffer, OutBufferSize, + PBytesTransferred, 0)) + { + Result = FspNtStatusFromWin32(GetLastError()); + goto exit; + } + + Result = STATUS_SUCCESS; + +exit: + if (INVALID_HANDLE_VALUE != Pipe) + CloseHandle(Pipe); + + return Result; +} + DWORD APIENTRY NPGetCaps(DWORD Index) { switch (Index) @@ -132,12 +238,13 @@ static inline BOOLEAN FspNpParseRemoteName(PWSTR RemoteName, static inline DWORD FspNpCallLauncherPipe(PWSTR PipeBuf, ULONG SendSize, ULONG RecvSize) { DWORD NpResult; - DWORD LastError, BytesTransferred; + NTSTATUS Result; + DWORD BytesTransferred; - LastError = CallNamedPipeW(L"" LAUNCHER_PIPE_NAME, PipeBuf, SendSize, PipeBuf, RecvSize, - &BytesTransferred, NMPWAIT_USE_DEFAULT_WAIT) ? 0 : GetLastError(); + Result = FspCallNamedPipeSecurely(L"" LAUNCHER_PIPE_NAME, PipeBuf, SendSize, PipeBuf, RecvSize, + &BytesTransferred, NMPWAIT_USE_DEFAULT_WAIT, LAUNCHER_PIPE_OWNER); - if (0 != LastError) + if (!NT_SUCCESS(Result)) NpResult = WN_NO_NETWORK; else if (sizeof(WCHAR) > BytesTransferred) NpResult = WN_NO_NETWORK; diff --git a/src/launcher/launchctl.c b/src/launcher/launchctl.c index f97047fc..0c3a0f9c 100644 --- a/src/launcher/launchctl.c +++ b/src/launcher/launchctl.c @@ -63,13 +63,15 @@ static void usage(void) static int call_pipe_and_report(PWSTR PipeBuf, ULONG SendSize, ULONG RecvSize) { + NTSTATUS Result; DWORD LastError, BytesTransferred; - LastError = CallNamedPipeW(L"" LAUNCHER_PIPE_NAME, PipeBuf, SendSize, PipeBuf, RecvSize, - &BytesTransferred, NMPWAIT_USE_DEFAULT_WAIT) ? 0 : GetLastError(); + Result = FspCallNamedPipeSecurely(L"" LAUNCHER_PIPE_NAME, PipeBuf, SendSize, PipeBuf, RecvSize, + &BytesTransferred, NMPWAIT_USE_DEFAULT_WAIT, LAUNCHER_PIPE_OWNER); + LastError = FspWin32FromNtStatus(Result); if (0 != LastError) - warn("KO CallNamedPipeW = %ld", LastError); + warn("KO CallNamedPipe = %ld", LastError); else if (sizeof(WCHAR) > BytesTransferred) warn("KO launcher: empty buffer"); else if (LauncherSuccess == PipeBuf[0]) diff --git a/src/launcher/launcher.h b/src/launcher/launcher.h index 7f92e275..d71f73b0 100644 --- a/src/launcher/launcher.h +++ b/src/launcher/launcher.h @@ -29,16 +29,13 @@ #define LAUNCHER_PIPE_DEFAULT_TIMEOUT 3000 /* - * The launcher named pipe SDDL gives full access to LocalSystem and Administrators. - * It also gives GENERIC_READ and GENERIC_WRITE access to Everyone. This includes the - * FILE_CREATE_PIPE_INSTANCE right which should not normally be granted to any process - * that is not the pipe server. The reason that the GENERIC_WRITE is required is to allow - * clients to use CallNamedPipeW which opens the pipe handle using CreateFileW and the - * GENERIC_READ | GENERIC_WRITE access right. The reason that it should be safe to grant - * the FILE_CREATE_PIPE_INSTANCE right is that the server creates the named pipe with - * MaxInstances == 1 (and therefore no client can create additional instances). + * The launcher named pipe SDDL gives full access to LocalSystem and Administrators and + * GENERIC_READ and FILE_WRITE_DATA access to Everyone. We are careful not to give the + * FILE_CREATE_PIPE_INSTANCE right to Everyone to disallow the creation of additional + * pipe instances. */ -#define LAUNCHER_PIPE_SDDL "D:P(A;;GA;;;SY)(A;;GA;;;BA)(A;;GRGW;;;WD)" +#define LAUNCHER_PIPE_SDDL "O:SYG:SYD:P(A;;GA;;;SY)(A;;GA;;;BA)(A;;GRDCCR;;;WD)" +#define LAUNCHER_PIPE_OWNER ((PSID)WinLocalSystemSid) /* * The default service instance SDDL gives full access to LocalSystem and Administrators.