On Tue, Nov 8, 2016 at 1:36 PM, Tsunakawa, Takayuki
<tsunakawa.ta...@jp.fujitsu.com> wrote:
> From: pgsql-hackers-ow...@postgresql.org
>> [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Michael Paquier
>> Hm... See here:
>> http://stackoverflow.com/questions/6084547/how-to-check-whether-a-proc
>> ess-is-running-as-a-windows-service
>> And particularly this quote:
>> "No, that is not reliable because if a service is started from command line
>> for example it will not have this token. "
>
> Is there any Microsoft document that states this?  I don't think the above 
> comment is correct, because SECURITY_SERVICE_RID was present when I started 
> the service from command line with "net start".

Not that I can see of... So maybe I'm just confused by this comment as
it is added by the SCM itself, right?

Things are this way since b15f9b08 that introduced
pgwin32_is_service(). Still, by considering what you say, you
definitely have a point that if postgres is started by another service
running as Local System logs are going where they should not. Let's
remove the check for LocalSystem but still check for SE_GROUP_ENABLED.
So, without any refactoring work, isn't the attached patch just but
fine? That seems to work properly for me.
-- 
Michael
diff --git a/src/port/win32security.c b/src/port/win32security.c
index 2c9ca15..e329eb0 100644
--- a/src/port/win32security.c
+++ b/src/port/win32security.c
@@ -122,12 +122,9 @@ pgwin32_is_admin(void)
 }
 
 /*
- * 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
- *	  process token by the SCM when starting a service)
+ * We consider ourselves running as a service if our token contains
+ * SECURITY_SERVICE_RID, which is automatically added to the process token
+ * by the SCM when starting a service.
  *
  * Return values:
  *	 0 = Not service
@@ -147,9 +144,7 @@ pgwin32_is_service(void)
 	char	   *InfoBuffer = NULL;
 	char		errbuf[256];
 	PTOKEN_GROUPS Groups;
-	PTOKEN_USER User;
 	PSID		ServiceSid;
-	PSID		LocalSystemSid;
 	SID_IDENTIFIER_AUTHORITY NtAuthority = {SECURITY_NT_AUTHORITY};
 	UINT		x;
 
@@ -164,37 +159,6 @@ pgwin32_is_service(void)
 		return -1;
 	}
 
-	/* First check for local system */
-	if (!pgwin32_get_dynamic_tokeninfo(AccessToken, TokenUser, &InfoBuffer,
-									   errbuf, sizeof(errbuf)))
-	{
-		fprintf(stderr, "%s", errbuf);
-		return -1;
-	}
-
-	User = (PTOKEN_USER) InfoBuffer;
-
-	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\n");
-		CloseHandle(AccessToken);
-		return -1;
-	}
-
-	if (EqualSid(LocalSystemSid, User->User.Sid))
-	{
-		FreeSid(LocalSystemSid);
-		free(InfoBuffer);
-		CloseHandle(AccessToken);
-		_is_service = 1;
-		return _is_service;
-	}
-
-	FreeSid(LocalSystemSid);
-	free(InfoBuffer);
-
 	/* Now check for group SID */
 	if (!pgwin32_get_dynamic_tokeninfo(AccessToken, TokenGroups, &InfoBuffer,
 									   errbuf, sizeof(errbuf)))
@@ -218,7 +182,8 @@ pgwin32_is_service(void)
 	_is_service = 0;
 	for (x = 0; x < Groups->GroupCount; x++)
 	{
-		if (EqualSid(ServiceSid, Groups->Groups[x].Sid))
+		if (EqualSid(ServiceSid, Groups->Groups[x].Sid) &&
+			(Groups->Groups[x].Attributes & SE_GROUP_ENABLED))
 		{
 			_is_service = 1;
 			break;
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to