Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtp (Exim 4.84_2) (envelope-from ) id 1c3s0j-0005SN-Fo for pgsql-hackers@arkaria.postgresql.org; Mon, 07 Nov 2016 22:03:25 +0000 Received: from localhost ([127.0.0.1] helo=postgresql.org) by malur.postgresql.org with smtp (Exim 4.84_2) (envelope-from ) id 1c3s0i-0001tc-US for pgsql-hackers@arkaria.postgresql.org; Mon, 07 Nov 2016 22:03:24 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1c3s0h-0001tT-8s for pgsql-hackers@postgresql.org; Mon, 07 Nov 2016 22:03:23 +0000 Received: from mail-pf0-x242.google.com ([2607:f8b0:400e:c00::242]) by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.84_2) (envelope-from ) id 1c3s0d-0004Jq-8l for pgsql-hackers@postgresql.org; Mon, 07 Nov 2016 22:03:22 +0000 Received: by mail-pf0-x242.google.com with SMTP id i88so17153042pfk.2 for ; Mon, 07 Nov 2016 14:03:18 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=message-id:from:to:cc:references:in-reply-to:subject:date :mime-version:importance; bh=K0o51eerWY6/Oj/dAxqxKDLlBKdUDmu4ch9IDdQCl18=; b=dznN75KxkktR25yOlFvlPPi0sXSmUqV4Rlrms81jMUrfnXHPMWPPZXrhdM4AYyUjtH UnotS6qcuSG8b/5p1w22VaWRk60zxMQSzjOK7k/1k/lStFTcOPhUHp0Hm5Ol22HQie5o 8bE/9xyMbvaDi2AMVAwIxsdRw6Lp8j+NuoIKJlY4VfoGjgtnt0Pr195FT73ERbyn8VkO /Oem5bELY7SDNioCiFaI1LlDRDDjz0x0UZu0yAeS45GY13IFIRK54okmfOiwHmGEzNcG Y+a5DgFVhfDuCQB+OSsA1e1gmBucVtJ/NEDqgUISxijkA/HXS/0+FSO5iBsmv+bWFWls 8iIQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:message-id:from:to:cc:references:in-reply-to :subject:date:mime-version:importance; bh=K0o51eerWY6/Oj/dAxqxKDLlBKdUDmu4ch9IDdQCl18=; b=U3NnxL6BnTaf/It7ZDGZoJy8a61KccxVKXhicJSMvBxCY6bJdu2PdyvlQaw5yqmxR/ vZcRjM4A+AYL3m9X4wLfBfdKaIzl9shgQqPJsyU3pgLpDX1ZrZgu/IoCbghMDtMhL/ZX zyK49cJGRDNTlyd9LYzCy2eoyOLpHa1vmoKMsj7ugab8kIzIYRw44rZr1Bawwmdsl+SC TEwH78O5BRBEz+vyJh+emTBIhlwCK2168dBRyTLoDHKlhjVOta7gmZhR+CioGGSueAVM ez2Jlxx5ztMjqQgiS/GAoAY1Q+/rn1b62oXONhjcrELDU+AUhTMOfwXCAabMxD+Lwf08 lNUQ== X-Gm-Message-State: ABUngvf36GJTOY3lPg/V18WCOgmh7+rrwb6atRIZa7h+xPSsZbbA44Eulso/SXLbVzyNjg== X-Received: by 10.99.120.75 with SMTP id t72mr13413053pgc.17.1478555276006; Mon, 07 Nov 2016 13:47:56 -0800 (PST) Received: from tunaPC (p293207-ipngn200405sizuokaden.shizuoka.ocn.ne.jp. [123.223.203.207]) by smtp.gmail.com with ESMTPSA id q12sm4134819pfj.18.2016.11.07.13.47.53 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 07 Nov 2016 13:47:55 -0800 (PST) Message-ID: <8230C8E316B544FC92813D96FA139F2E@tunaPC> From: "MauMau" To: "Michael Paquier" , "Tsunakawa, Takayuki" Cc: "Breen Hagan" , "Heikki Linnakangas" , "Alvaro Herrera" , "PostgreSQL Hackers" References: <20160404160801.GA186687@alvherre.pgsql> <64a0ee81-2e30-c9b1-97b6-312772f89a2e@iki.fi> <952A88FFCBDA419EB6D6556B6A8F29C1@tunaPC> <0A3221C70F24FB45833433255569204D1F63BD65@G01JPEXMBYT05> In-Reply-To: Subject: Re: Re: BUG #13755: pgwin32_is_service not checking if SECURITY_SERVICE_SID is disabled Date: Tue, 8 Nov 2016 06:47:52 +0900 MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----=_NextPart_000_006A_01D2398C.06DE1CB0" X-Priority: 3 X-MSMail-Priority: Normal Importance: Normal X-Mailer: Microsoft Windows Live Mail 16.4.3528.331 X-MimeOLE: Produced By Microsoft MimeOLE V16.4.3528.331 X-Pg-Spam-Score: -2.5 (--) List-Archive: List-Help: List-ID: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: X-Mailing-List: pgsql-hackers Precedence: bulk Sender: pgsql-hackers-owner@postgresql.org This is a multi-part message in MIME format. ------=_NextPart_000_006A_01D2398C.06DE1CB0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Hi, Michael 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. Regards Takayuki Tsunakawa ------=_NextPart_000_006A_01D2398C.06DE1CB0 Content-Type: application/octet-stream; name="win32-security-service-v5.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="win32-security-service-v5.patch" diff --git a/src/port/win32security.c b/src/port/win32security.c=0A= index 2c9ca15..9e65e60 100644=0A= --- a/src/port/win32security.c=0A= +++ b/src/port/win32security.c=0A= @@ -18,11 +18,6 @@=0A= #endif=0A= =20=0A= =20=0A= -static BOOL pgwin32_get_dynamic_tokeninfo(HANDLE token,=0A= - TOKEN_INFORMATION_CLASS class,=0A= - char **InfoBuffer, char *errbuf, int errsize);=0A= -=0A= -=0A= /*=0A= * Utility wrapper for frontend and backend when reporting an error=0A= * message.=0A= @@ -53,33 +48,11 @@ log_error(const char *fmt,...)=0A= int=0A= pgwin32_is_admin(void)=0A= {=0A= - HANDLE AccessToken;=0A= - char *InfoBuffer =3D NULL;=0A= - char errbuf[256];=0A= - PTOKEN_GROUPS Groups;=0A= PSID AdministratorsSid;=0A= PSID PowerUsersSid;=0A= SID_IDENTIFIER_AUTHORITY NtAuthority =3D {SECURITY_NT_AUTHORITY};=0A= - UINT x;=0A= - BOOL success;=0A= -=0A= - if (!OpenProcessToken(GetCurrentProcess(), TOKEN_READ, &AccessToken))=0A= - {=0A= - log_error(_("could not open process token: error code %lu\n"),=0A= - GetLastError());=0A= - exit(1);=0A= - }=0A= -=0A= - if (!pgwin32_get_dynamic_tokeninfo(AccessToken, TokenGroups,=0A= - &InfoBuffer, errbuf, sizeof(errbuf)))=0A= - {=0A= - log_error("%s", errbuf);=0A= - exit(1);=0A= - }=0A= -=0A= - Groups =3D (PTOKEN_GROUPS) InfoBuffer;=0A= -=0A= - CloseHandle(AccessToken);=0A= + BOOL IsAdministrators;=0A= + BOOL IsPowerUsers;=0A= =20=0A= if (!AllocateAndInitializeSid(&NtAuthority, 2,=0A= SECURITY_BUILTIN_DOMAIN_RID,=0A= @@ -101,32 +74,26 @@ pgwin32_is_admin(void)=0A= exit(1);=0A= }=0A= =20=0A= - success =3D FALSE;=0A= -=0A= - for (x =3D 0; x < Groups->GroupCount; x++)=0A= + if (!CheckTokenMembership(NULL, AdministratorsSid, &IsAdministrators) ||= =0A= + !CheckTokenMembership(NULL, PowerUsersSid, &IsPowerUsers))=0A= {=0A= - if ((EqualSid(AdministratorsSid, Groups->Groups[x].Sid) &&=0A= - (Groups->Groups[x].Attributes & SE_GROUP_ENABLED)) ||=0A= - (EqualSid(PowerUsersSid, Groups->Groups[x].Sid) &&=0A= - (Groups->Groups[x].Attributes & SE_GROUP_ENABLED)))=0A= - {=0A= - success =3D TRUE;=0A= - break;=0A= - }=0A= + log_error("could not check access token membership: error code %lu\n",= =0A= + GetLastError());=0A= + exit(1);=0A= }=0A= =20=0A= - free(InfoBuffer);=0A= FreeSid(AdministratorsSid);=0A= FreeSid(PowerUsersSid);=0A= - return success;=0A= +=0A= + if (IsAdministrators || IsPowerUsers)=0A= + return 1;=0A= + else=0A= + return 0;=0A= }=0A= =20=0A= /*=0A= - * We consider ourselves running as a service if one of the following is= =0A= - * true:=0A= - *=0A= - * 1) We are running as Local System (only used by services)=0A= - * 2) Our token contains SECURITY_SERVICE_RID (automatically added to the= =0A= + * We consider ourselves running as a service if=20=0A= + * our token contains SECURITY_SERVICE_RID (automatically added to the=0A= * process token by the SCM when starting a service)=0A= *=0A= * Return values:=0A= @@ -143,140 +110,37 @@ int=0A= pgwin32_is_service(void)=0A= {=0A= static int _is_service =3D -1;=0A= - HANDLE AccessToken;=0A= - char *InfoBuffer =3D NULL;=0A= - char errbuf[256];=0A= - PTOKEN_GROUPS Groups;=0A= - PTOKEN_USER User;=0A= + BOOL IsMember;=0A= PSID ServiceSid;=0A= - PSID LocalSystemSid;=0A= SID_IDENTIFIER_AUTHORITY NtAuthority =3D {SECURITY_NT_AUTHORITY};=0A= - UINT x;=0A= =20=0A= /* Only check the first time */=0A= if (_is_service !=3D -1)=0A= return _is_service;=0A= =20=0A= - if (!OpenProcessToken(GetCurrentProcess(), TOKEN_READ, &AccessToken))=0A= - {=0A= - fprintf(stderr, "could not open process token: error code %lu\n",=0A= - GetLastError());=0A= - return -1;=0A= - }=0A= -=0A= - /* First check for local system */=0A= - if (!pgwin32_get_dynamic_tokeninfo(AccessToken, TokenUser, &InfoBuffer,= =0A= - errbuf, sizeof(errbuf)))=0A= - {=0A= - fprintf(stderr, "%s", errbuf);=0A= - return -1;=0A= - }=0A= -=0A= - User =3D (PTOKEN_USER) InfoBuffer;=0A= -=0A= - if (!AllocateAndInitializeSid(&NtAuthority, 1,=0A= - SECURITY_LOCAL_SYSTEM_RID, 0, 0, 0, 0, 0, 0, 0,=0A= - &LocalSystemSid))=0A= - {=0A= - fprintf(stderr, "could not get SID for local system account\n");=0A= - CloseHandle(AccessToken);=0A= - return -1;=0A= - }=0A= -=0A= - if (EqualSid(LocalSystemSid, User->User.Sid))=0A= - {=0A= - FreeSid(LocalSystemSid);=0A= - free(InfoBuffer);=0A= - CloseHandle(AccessToken);=0A= - _is_service =3D 1;=0A= - return _is_service;=0A= - }=0A= -=0A= - FreeSid(LocalSystemSid);=0A= - free(InfoBuffer);=0A= -=0A= - /* Now check for group SID */=0A= - if (!pgwin32_get_dynamic_tokeninfo(AccessToken, TokenGroups, &InfoBuffer,= =0A= - errbuf, sizeof(errbuf)))=0A= - {=0A= - fprintf(stderr, "%s", errbuf);=0A= - return -1;=0A= - }=0A= -=0A= - Groups =3D (PTOKEN_GROUPS) InfoBuffer;=0A= -=0A= + /* Check for service group membership */=0A= if (!AllocateAndInitializeSid(&NtAuthority, 1,=0A= SECURITY_SERVICE_RID, 0, 0, 0, 0, 0, 0, 0,=0A= &ServiceSid))=0A= {=0A= - fprintf(stderr, "could not get SID for service group\n");=0A= - free(InfoBuffer);=0A= - CloseHandle(AccessToken);=0A= + fprintf(stderr, "could not get SID for service group: error code %lu\n",= =0A= + GetLastError());=0A= return -1;=0A= }=0A= =20=0A= - _is_service =3D 0;=0A= - for (x =3D 0; x < Groups->GroupCount; x++)=0A= + if (!CheckTokenMembership(NULL, ServiceSid, &IsMember))=0A= {=0A= - if (EqualSid(ServiceSid, Groups->Groups[x].Sid))=0A= - {=0A= - _is_service =3D 1;=0A= - break;=0A= - }=0A= + fprintf(stderr, "could not check access token membership: error code %lu= \n",=0A= + GetLastError());=0A= + FreeSid(ServiceSid);=0A= + return -1;=0A= }=0A= -=0A= - free(InfoBuffer);=0A= FreeSid(ServiceSid);=0A= =20=0A= - CloseHandle(AccessToken);=0A= + if (IsMember)=0A= + _is_service =3D 1;=0A= + else=0A= + _is_service =3D 0;=0A= =20=0A= return _is_service;=0A= }=0A= -=0A= -=0A= -/*=0A= - * Call GetTokenInformation() on a token and return a dynamically sized=0A= - * buffer with the information in it. This buffer must be free():d by=0A= - * the calling function!=0A= - */=0A= -static BOOL=0A= -pgwin32_get_dynamic_tokeninfo(HANDLE token, TOKEN_INFORMATION_CLASS class,= =0A= - char **InfoBuffer, char *errbuf, int errsize)=0A= -{=0A= - DWORD InfoBufferSize;=0A= -=0A= - if (GetTokenInformation(token, class, NULL, 0, &InfoBufferSize))=0A= - {=0A= - snprintf(errbuf, errsize,=0A= - "could not get token information buffer size: got zero size\n");=0A= - return FALSE;=0A= - }=0A= -=0A= - if (GetLastError() !=3D ERROR_INSUFFICIENT_BUFFER)=0A= - {=0A= - snprintf(errbuf, errsize,=0A= - "could not get token information buffer size: error code %lu\n",=0A= - GetLastError());=0A= - return FALSE;=0A= - }=0A= -=0A= - *InfoBuffer =3D malloc(InfoBufferSize);=0A= - if (*InfoBuffer =3D=3D NULL)=0A= - {=0A= - snprintf(errbuf, errsize,=0A= - "could not allocate %d bytes for token information\n",=0A= - (int) InfoBufferSize);=0A= - return FALSE;=0A= - }=0A= -=0A= - if (!GetTokenInformation(token, class, *InfoBuffer,=0A= - InfoBufferSize, &InfoBufferSize))=0A= - {=0A= - snprintf(errbuf, errsize,=0A= - "could not get token information: error code %lu\n",=0A= - GetLastError());=0A= - return FALSE;=0A= - }=0A= -=0A= - return TRUE;=0A= -}=0A= ------=_NextPart_000_006A_01D2398C.06DE1CB0 Content-Type: text/plain Content-Disposition: inline Content-Transfer-Encoding: 8bit MIME-Version: 1.0 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers ------=_NextPart_000_006A_01D2398C.06DE1CB0--