Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Delta Between Two Patch Sets: src/shared/Communication.cpp

Issue 6308231068516352: Fix issues with security tokens (Enhanced Protected Mode, Protected Mode etc) (Closed)
Left Patch Set: Created Nov. 15, 2013, 7:22 p.m.
Right Patch Set: Slaying the comments Created March 4, 2014, 10:38 a.m.
Left:
Right:
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
Left: Side by side diff | Download
Right: Side by side diff | Download
LEFTRIGHT
1 #include <Windows.h> 1 #include <Windows.h>
2 #include <Lmcons.h> 2 #include <Lmcons.h>
3 #include <Sddl.h> 3 #include <Sddl.h>
4 #include <aclapi.h> 4 #include <aclapi.h>
5 #include <strsafe.h> 5 #include <strsafe.h>
6 6
7 #include "AutoHandle.h" 7 #include "AutoHandle.h"
8 #include "Communication.h" 8 #include "Communication.h"
9 #include "Utils.h" 9 #include "Utils.h"
10 10
11 11
12 //
13 // Application Package Authority.
14 //
15
16 #define SECURITY_APP_PACKAGE_AUTHORITY {0,0,0,0,0,15}
17
18 #define SECURITY_APP_PACKAGE_BASE_RID (0x00000002L)
19 #define SECURITY_BUILTIN_APP_PACKAGE_RID_COUNT (2L)
20 #define SECURITY_APP_PACKAGE_RID_COUNT (8L)
21 #define SECURITY_CAPABILITY_BASE_RID (0x00000003L)
22 #define SECURITY_BUILTIN_CAPABILITY_RID_COUNT (2L)
23 #define SECURITY_CAPABILITY_RID_COUNT (5L)
24
25 //
26 // Built-in Packages.
27 //
28
29 #define SECURITY_BUILTIN_PACKAGE_ANY_PACKAGE (0x00000001L)
30 12
31 namespace 13 namespace
32 { 14 {
33 const int bufferSize = 1024; 15 const int bufferSize = 1024;
34 16
35 std::string AppendErrorCode(const std::string& message) 17 std::string AppendErrorCode(const std::string& message)
36 { 18 {
37 std::stringstream stream; 19 std::stringstream stream;
38 stream << message << " (Error code: " << GetLastError() << ")"; 20 stream << message << " (Error code: " << GetLastError() << ")";
39 return stream.str(); 21 return stream.str();
(...skipping 26 matching lines...) Expand all
66 DWORD sidLength = GetLengthSid(tokenGroups->Groups[0].Sid); 48 DWORD sidLength = GetLengthSid(tokenGroups->Groups[0].Sid);
67 std::auto_ptr<SID> sid(new SID[sidLength]); 49 std::auto_ptr<SID> sid(new SID[sidLength]);
68 if (!CopySid(sidLength, sid.get(), tokenGroups->Groups[0].Sid)) 50 if (!CopySid(sidLength, sid.get(), tokenGroups->Groups[0].Sid))
69 throw std::runtime_error("CopySid failed"); 51 throw std::runtime_error("CopySid failed");
70 return sid; 52 return sid;
71 } 53 }
72 54
73 // Creates a security descriptor: 55 // Creates a security descriptor:
74 // Allows ALL access to Logon SID and to all app containers in DACL. 56 // Allows ALL access to Logon SID and to all app containers in DACL.
75 // Sets Low Integrity in SACL. 57 // Sets Low Integrity in SACL.
76 std::auto_ptr<SECURITY_DESCRIPTOR> CreateSecurityDescriptorLowAndAppContainers (PSID logonSid) 58 std::auto_ptr<SECURITY_DESCRIPTOR> CreateSecurityDescriptor(PSID logonSid)
Felix Dahlke 2013/12/10 16:46:36 I don't think we need to put the specifics of the
77 { 59 {
78 EXPLICIT_ACCESSW explicitAccess[2] = {}; 60 EXPLICIT_ACCESSW explicitAccess[2] = {};
79 61
80 explicitAccess[0].grfAccessPermissions = STANDARD_RIGHTS_ALL | SPECIFIC_RIGH TS_ALL; 62 explicitAccess[0].grfAccessPermissions = STANDARD_RIGHTS_ALL | SPECIFIC_RIGH TS_ALL;
81 explicitAccess[0].grfAccessMode = SET_ACCESS; 63 explicitAccess[0].grfAccessMode = SET_ACCESS;
82 explicitAccess[0].grfInheritance= NO_INHERITANCE; 64 explicitAccess[0].grfInheritance= NO_INHERITANCE;
83 explicitAccess[0].Trustee.TrusteeForm = TRUSTEE_IS_SID; 65 explicitAccess[0].Trustee.TrusteeForm = TRUSTEE_IS_SID;
84 explicitAccess[0].Trustee.TrusteeType = TRUSTEE_IS_USER; 66 explicitAccess[0].Trustee.TrusteeType = TRUSTEE_IS_USER;
85 explicitAccess[0].Trustee.ptstrName = static_cast<LPWSTR>(logonSid); 67 explicitAccess[0].Trustee.ptstrName = static_cast<LPWSTR>(logonSid);
86 68
87 std::tr1::shared_ptr<SID> sharedAllAppContainersSid; 69 std::tr1::shared_ptr<SID> sharedAllAppContainersSid;
88 // TODO: Would be better to detect if AppContainers are supported instead of checking the Windows version 70 // TODO: Would be better to detect if AppContainers are supported instead of checking the Windows version
Felix Dahlke 2014/06/24 15:30:39 You can also slay the TODO comment here now!
89 bool isWindows8 = IsWindows8(); 71 bool isAppContainersSupported = IsAppContainersSupported();
Felix Dahlke 2013/12/10 16:46:36 Agreed, would be better to check for app container
90 if (isWindows8) 72 if (isAppContainersSupported)
91 { 73 {
92 PSID allAppContainersSid = 0;
93 SID_IDENTIFIER_AUTHORITY ApplicationAuthority = SECURITY_APP_PACKAGE_AUTHO RITY;
Felix Dahlke 2013/12/10 16:46:36 Variables should start with a lower case letter.
94
95 // Create a well-known SID for the all appcontainers group. 74 // Create a well-known SID for the all appcontainers group.
Felix Dahlke 2013/12/10 16:46:36 I think it'd make sense to move this comment in fr
96 // We need to allow access to all AppContainers, since, apparently, 75 // We need to allow access to all AppContainers, since, apparently,
97 // giving access to specific AppContainer (for example AppContainer of IE) 76 // giving access to specific AppContainer (for example AppContainer of IE)
98 // tricks Windows into thinking that token is IN AppContainer. 77 // tricks Windows into thinking that token is IN AppContainer.
99 // Which blocks all the calls from outside, making it impossible to commun icate 78 // Which blocks all the calls from outside, making it impossible to commun icate
100 // with engine when IE is launched with different security settings. 79 // with the engine when IE is launched with different security settings.
Felix Dahlke 2013/12/10 16:46:36 with "the" engine
101 AllocateAndInitializeSid(&ApplicationAuthority, 80 PSID allAppContainersSid = 0;
81 SID_IDENTIFIER_AUTHORITY applicationAuthority = SECURITY_APP_PACKAGE_AUTHO RITY;
82
83 AllocateAndInitializeSid(&applicationAuthority,
102 SECURITY_BUILTIN_APP_PACKAGE_RID_COUNT, 84 SECURITY_BUILTIN_APP_PACKAGE_RID_COUNT,
103 SECURITY_APP_PACKAGE_BASE_RID, 85 SECURITY_APP_PACKAGE_BASE_RID,
104 SECURITY_BUILTIN_PACKAGE_ANY_PACKAGE, 86 SECURITY_BUILTIN_PACKAGE_ANY_PACKAGE,
105 0, 0, 0, 0, 0, 0, 87 0, 0, 0, 0, 0, 0,
106 &allAppContainersSid); 88 &allAppContainersSid);
107 sharedAllAppContainersSid.reset(static_cast<SID*>(allAppContainersSid), Fr eeSid); // Just to simplify cleanup 89 sharedAllAppContainersSid.reset(static_cast<SID*>(allAppContainersSid), Fr eeSid); // Just to simplify cleanup
108 90
109 explicitAccess[1].grfAccessPermissions = STANDARD_RIGHTS_ALL | SPECIFIC_RI GHTS_ALL; 91 explicitAccess[1].grfAccessPermissions = STANDARD_RIGHTS_ALL | SPECIFIC_RI GHTS_ALL;
110 explicitAccess[1].grfAccessMode = SET_ACCESS; 92 explicitAccess[1].grfAccessMode = SET_ACCESS;
111 explicitAccess[1].grfInheritance= NO_INHERITANCE; 93 explicitAccess[1].grfInheritance= NO_INHERITANCE;
112 explicitAccess[1].Trustee.TrusteeForm = TRUSTEE_IS_SID; 94 explicitAccess[1].Trustee.TrusteeForm = TRUSTEE_IS_SID;
113 explicitAccess[1].Trustee.TrusteeType = TRUSTEE_IS_GROUP; 95 explicitAccess[1].Trustee.TrusteeType = TRUSTEE_IS_GROUP;
114 explicitAccess[1].Trustee.ptstrName = static_cast<LPWSTR>(allAppContainers Sid); 96 explicitAccess[1].Trustee.ptstrName = static_cast<LPWSTR>(allAppContainers Sid);
115 } 97 }
116 PACL acl = 0; 98 PACL acl = 0;
117 if (SetEntriesInAcl(isWindows8 ? 2 : 1, explicitAccess, 0, &acl) != ERROR_SU CCESS) 99 if (SetEntriesInAcl(isAppContainersSupported ? 2 : 1, explicitAccess, 0, &ac l) != ERROR_SUCCESS)
118 return std::auto_ptr<SECURITY_DESCRIPTOR>(0); 100 return std::auto_ptr<SECURITY_DESCRIPTOR>(0);
119 std::tr1::shared_ptr<ACL> sharedAcl(static_cast<ACL*>(acl), LocalFree); // J ust to simplify cleanup 101 std::tr1::shared_ptr<ACL> sharedAcl(static_cast<ACL*>(acl), LocalFree); // J ust to simplify cleanup
120 102
121 std::auto_ptr<SECURITY_DESCRIPTOR> securityDescriptor((SECURITY_DESCRIPTOR*) new char[SECURITY_DESCRIPTOR_MIN_LENGTH]); 103 std::auto_ptr<SECURITY_DESCRIPTOR> securityDescriptor((SECURITY_DESCRIPTOR*) new char[SECURITY_DESCRIPTOR_MIN_LENGTH]);
122 if (!InitializeSecurityDescriptor(securityDescriptor.get(), SECURITY_DESCRIP TOR_REVISION)) 104 if (!InitializeSecurityDescriptor(securityDescriptor.get(), SECURITY_DESCRIP TOR_REVISION))
123 return std::auto_ptr<SECURITY_DESCRIPTOR>(0); 105 return std::auto_ptr<SECURITY_DESCRIPTOR>(0);
124 106
125 if (!SetSecurityDescriptorDacl(securityDescriptor.get(), TRUE, acl, FALSE)) 107 if (!SetSecurityDescriptorDacl(securityDescriptor.get(), TRUE, acl, FALSE))
126 return std::auto_ptr<SECURITY_DESCRIPTOR>(0); 108 return std::auto_ptr<SECURITY_DESCRIPTOR>(0);
127 109
128 // Create a dummy security descriptor with low integrirty preset and copy it s SACL into ours 110 // Create a dummy security descriptor with low integrirty preset and copy it s SACL into ours
129 LPCWSTR accessControlEntry = L"S:(ML;;NW;;;LW)"; 111 LPCWSTR accessControlEntry = L"S:(ML;;NW;;;LW)";
130 PSECURITY_DESCRIPTOR pDummySecurityDescriptorLow; 112 PSECURITY_DESCRIPTOR dummySecurityDescriptorLow;
Felix Dahlke 2013/12/10 16:46:36 Also quite hungarian :)
131 ConvertStringSecurityDescriptorToSecurityDescriptorW(accessControlEntry, SDD L_REVISION_1, &pDummySecurityDescriptorLow, 0); 113 ConvertStringSecurityDescriptorToSecurityDescriptorW(accessControlEntry, SDD L_REVISION_1, &dummySecurityDescriptorLow, 0);
132 std::tr1::shared_ptr<SECURITY_DESCRIPTOR> sharedDummySecurityDescriptor(stat ic_cast<SECURITY_DESCRIPTOR*>(pDummySecurityDescriptorLow), LocalFree); // Just to simplify cleanup 114 std::tr1::shared_ptr<SECURITY_DESCRIPTOR> sharedDummySecurityDescriptor(stat ic_cast<SECURITY_DESCRIPTOR*>(dummySecurityDescriptorLow), LocalFree); // Just t o simplify cleanup
133 BOOL saclPresent = FALSE; 115 BOOL saclPresent = FALSE;
134 BOOL saclDefaulted = FALSE; 116 BOOL saclDefaulted = FALSE;
135 PACL sacl; 117 PACL sacl;
Felix Dahlke 2014/06/24 15:30:39 Hm, don't we leak sacl now that you removed shared
Eric 2014/06/25 17:43:55 Evidently not. The security descriptor structure h
Felix Dahlke 2014/06/27 14:35:56 Yeah, I think you're right Eric. Seems like Oleksa
136 GetSecurityDescriptorSacl(pDummySecurityDescriptorLow, &saclPresent, &sacl, &saclDefaulted); 118 GetSecurityDescriptorSacl(dummySecurityDescriptorLow, &saclPresent, &sacl, & saclDefaulted);
137 std::tr1::shared_ptr<ACL> sharedSacl(static_cast<ACL*>(sacl), LocalFree); // Just to simplify cleanup
138 if (saclPresent) 119 if (saclPresent)
139 { 120 {
140 if (!SetSecurityDescriptorSacl(securityDescriptor.get(), TRUE, sacl, FALSE )) 121 if (!SetSecurityDescriptorSacl(securityDescriptor.get(), TRUE, sacl, FALSE ))
141 { 122 {
142 DWORD err = GetLastError(); 123 DWORD err = GetLastError();
143 return std::auto_ptr<SECURITY_DESCRIPTOR>(0); 124 return std::auto_ptr<SECURITY_DESCRIPTOR>(0);
144 } 125 }
145 } 126 }
146 127
147 return securityDescriptor; 128 return securityDescriptor;
(...skipping 50 matching lines...) Expand 10 before | Expand all | Expand 10 after
198 securityAttributes.nLength = sizeof(SECURITY_ATTRIBUTES); 179 securityAttributes.nLength = sizeof(SECURITY_ATTRIBUTES);
199 securityAttributes.bInheritHandle = TRUE; 180 securityAttributes.bInheritHandle = TRUE;
200 181
201 std::tr1::shared_ptr<SECURITY_DESCRIPTOR> sharedSecurityDescriptor; // Just to simplify cleanup 182 std::tr1::shared_ptr<SECURITY_DESCRIPTOR> sharedSecurityDescriptor; // Just to simplify cleanup
202 183
203 AutoHandle token; 184 AutoHandle token;
204 OpenProcessToken(GetCurrentProcess(), TOKEN_READ, token); 185 OpenProcessToken(GetCurrentProcess(), TOKEN_READ, token);
205 std::auto_ptr<SID> logonSid = GetLogonSid(token); 186 std::auto_ptr<SID> logonSid = GetLogonSid(token);
206 // Create a SECURITY_DESCRIPTOR that has both Low Integrity and allows acces s to all AppContainers 187 // Create a SECURITY_DESCRIPTOR that has both Low Integrity and allows acces s to all AppContainers
207 // This is needed since IE likes to jump out of Enhanced Protected Mode for specific pages (bing.com) 188 // This is needed since IE likes to jump out of Enhanced Protected Mode for specific pages (bing.com)
208 std::auto_ptr<SECURITY_DESCRIPTOR> securityDescriptor = CreateSecurityDescri ptorLowAndAppContainers(logonSid.get()); 189 std::auto_ptr<SECURITY_DESCRIPTOR> securityDescriptor = CreateSecurityDescri ptor(logonSid.get());
209 securityAttributes.lpSecurityDescriptor = securityDescriptor.release(); 190 securityAttributes.lpSecurityDescriptor = securityDescriptor.release();
210 sharedSecurityDescriptor.reset(static_cast<SECURITY_DESCRIPTOR*>(securityAtt ributes.lpSecurityDescriptor)); 191 sharedSecurityDescriptor.reset(static_cast<SECURITY_DESCRIPTOR*>(securityAtt ributes.lpSecurityDescriptor));
211 192
212 pipe = CreateNamedPipeW(pipeName.c_str(), PIPE_ACCESS_DUPLEX, PIPE_TYPE_MES SAGE | PIPE_READMODE_MESSAGE | PIPE_WAIT, 193 pipe = CreateNamedPipeW(pipeName.c_str(), PIPE_ACCESS_DUPLEX, PIPE_TYPE_MES SAGE | PIPE_READMODE_MESSAGE | PIPE_WAIT,
213 PIPE_UNLIMITED_INSTANCES, bufferSize, bufferSize, 0, &securityAttributes); 194 PIPE_UNLIMITED_INSTANCES, bufferSize, bufferSize, 0, &securityAttributes);
214 195
215 } 196 }
216 else 197 else
217 { 198 {
218 pipe = CreateFileW(pipeName.c_str(), GENERIC_READ | GENERIC_WRITE, 0, 0, OPE N_EXISTING, 0, 0); 199 pipe = CreateFileW(pipeName.c_str(), GENERIC_READ | GENERIC_WRITE, 0, 0, OPE N_EXISTING, 0, 0);
(...skipping 52 matching lines...) Expand 10 before | Expand all | Expand 10 after
271 return Communication::InputBuffer(stream.str()); 252 return Communication::InputBuffer(stream.str());
272 } 253 }
273 254
274 void Communication::Pipe::WriteMessage(Communication::OutputBuffer& message) 255 void Communication::Pipe::WriteMessage(Communication::OutputBuffer& message)
275 { 256 {
276 DWORD bytesWritten; 257 DWORD bytesWritten;
277 std::string data = message.Get(); 258 std::string data = message.Get();
278 if (!WriteFile(pipe, data.c_str(), static_cast<DWORD>(data.length()), &bytesWr itten, 0)) 259 if (!WriteFile(pipe, data.c_str(), static_cast<DWORD>(data.length()), &bytesWr itten, 0))
279 throw std::runtime_error("Failed to write to pipe"); 260 throw std::runtime_error("Failed to write to pipe");
280 } 261 }
LEFTRIGHT

Powered by Google App Engine
This is Rietveld