Code/test feature for sharing security descriptors#209
Code/test feature for sharing security descriptors#209DefaultRyan wants to merge 18 commits intomicrosoft:mainfrom
Conversation
Refactor to array of structs instead of two arrays.
ghost
added
the
|
Can you remove one level of directory structure so it's not "s.ac/s.ac/files" ? |
jonwis
left a comment
jonwis
left a comment
There was a problem hiding this comment.
Is the WinRT variant coming soon?
| THROW_WIN32(err); | ||
| } | ||
| tg.reset(static_cast<TOKEN_GROUPS*>(::HeapAlloc(::GetProcessHeap(), HEAP_ZERO_MEMORY, length))); | ||
| } |
There was a problem hiding this comment.
https://github.com/microsoft/wil/blob/master/include/wil/token_helpers.h could acquire this as a helper! (We need to eventually invert the lookup, so you can pass the token info type you want... there are several information classes that share the same output struct.)
| &length)); | ||
|
|
||
| auto sidLength = ::GetLengthSid(tg->Groups[0].Sid); | ||
| result.reset(::HeapAlloc(::GetProcessHeap(), 0, sidLength)); |
There was a problem hiding this comment.
Instead of allocating a new thing, maybe just return "tg" and let the caller root around in it? This is an internal method so nicety isn't necessarily required.
| _In_opt_ PSID principal, | ||
| uint32_t principalAccessMask, | ||
| _Outptr_ PSECURITY_DESCRIPTOR* securityDescriptor | ||
| ) |
There was a problem hiding this comment.
Use a function-level try/catch to reduce one level of indentation. This should probably be marked noexcept as well.
There was a problem hiding this comment.
This function is the implementation of a "Flat C" API. So we have a larger question of whether we are going to mark those APIs as noexcept in the headers.
Clearly, that keyword won't work for plain C consumers. It's only available for C++ consumers if they are running C++11 or greater. I'm in favor of this, but not sure if this decision has been made yet.
There was a problem hiding this comment.
I have moved the API into extern "C" which has a similar benefit to noexcept when compiling with /EHsc (which is the recommended setting).
| try | ||
| { | ||
| vector<EXPLICIT_ACCESS> ea; | ||
| if (accessRequestCount == UINT32_MAX) |
There was a problem hiding this comment.
Consider removing this "overflow' check; if the reserve fails then eventually the push-backs will fail later and that's OK.
| ea.push_back(entry); | ||
| } | ||
|
|
||
| wil::unique_any<PACL, decltype(&::LocalFree), ::LocalFree> acl; |
There was a problem hiding this comment.
Another thing to wil-ify as unique_hlocal_acl ? (No rush on any "put this in wil" comments.)
|
|
||
| auto result = wil::make_unique_hlocal<SECURITY_DESCRIPTOR>(); | ||
| THROW_IF_WIN32_BOOL_FALSE(::InitializeSecurityDescriptor(result.get(), SECURITY_DESCRIPTOR_REVISION)); | ||
| THROW_IF_WIN32_BOOL_FALSE(::SetSecurityDescriptorDacl(result.get(), TRUE, acl.get(), FALSE)); |
There was a problem hiding this comment.
Check out https://docs.microsoft.com/en-us/windows/win32/secauthz/absolute-and-self-relative-security-descriptors - this method needs to hand out a self-relative SD (and be documented as such) so the "binary bytes" thing and freeing the SD works correctly.
| { | ||
| STARTUPINFO si{}; | ||
| si.cb = sizeof(si); | ||
| PROCESS_INFORMATION pi{}; |
There was a problem hiding this comment.
See also:
using unique_process_information = unique_struct<PROCESS_INFORMATION, decltype(&details::CloseProcessInformation), details::CloseProcessInformation>;
…tPackagingOutputs target for referenced projects. (microsoft#260)
* updated FAQ * format edits * format edits
|
It's sitting idle for like almost one full year. Windows App SDK still only supports medium IL process. Dynamic Dependencies most likely won't support Elevation by this year. No mention of AppContainer scenarios either. @DefaultRyan Are we about to get sort of an update on this anytime sooner ? |
|
This PR got so old that it was easier to create a new one. See #2005 |
6 participants
This is the implementation and test code for #188