On Tue, Nov 8, 2016 at 6:47 AM, MauMau <maumau307@gmail.com> wrote:
> As I guessed in the previous mail, both our patches cause
> pgwin32_is_service() to return 1 even when SECURITY_SERVICE_RID is
> disabled, if the service is running as a Local System. The existing
> logic of checking for Local System should be removed. The attached
> patch fixes this problem.
Meh. Local System accounts are used only by services (see comments of
pgwin32_is_service), so I'd expect pgwin32_is_service() to return true
in this case, contrary to what your v5 is doing. v4 is doing it better
I think at quick glance.
Here are the diffs between your v4 and v5 of this refactoring btw for
people wondering:/*
- * We consider ourselves running as a service if one of the following is
- * true:
- *
- * 1) We are running as Local System (only used by services)
- * 2) Our token contains SECURITY_SERVICE_RID (automatically added to the
+ * We consider ourselves running as a service if
+ * our token contains SECURITY_SERVICE_RID (automatically added to the * process token by the SCM when starting a
service)* * Return values:
@@ -115,39 +112,13 @@ pgwin32_is_service(void) static int _is_service = -1; BOOL IsMember; PSID
ServiceSid;
- PSID LocalSystemSid; SID_IDENTIFIER_AUTHORITY NtAuthority = {SECURITY_NT_AUTHORITY};
/* Only check the first time */ if (_is_service != -1) return _is_service;
- /* First check for Local System */
- if (!AllocateAndInitializeSid(&NtAuthority, 1,
- SECURITY_LOCAL_SYSTEM_RID, 0, 0, 0, 0, 0, 0, 0,
- &LocalSystemSid))
- {
- fprintf(stderr, "could not get SID for Local System account:
error code %lu\n",
- GetLastError());
- return -1;
- }
-
- if (!CheckTokenMembership(NULL, LocalSystemSid, &IsMember))
- {
- fprintf(stderr, "could not check access token membership:
error code %lu\n",
- GetLastError());
- FreeSid(LocalSystemSid);
- return -1;
- }
- FreeSid(LocalSystemSid);
-
- if (IsMember)
- {
- _is_service = 1;
- return _is_service;
- }
-
- /* Next check for service group */
+ /* Check for service group membership */
Not relying on the fact that local system accounts are only used by
services looks bad to me.
--
Michael
