pg.ddx.io  pgsql-committers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
pgsql: Fix and simplify check for whether we're running as Windows serv
14+ messages / 6 participants
[nested] [flat]

* pgsql: Fix and simplify check for whether we're running as Windows serv
@ 2017-03-17 09:18 Heikki Linnakangas <heikki.linnakangas@iki.fi>
  2017-03-17 17:57 ` Re: [COMMITTERS] pgsql: Fix and simplify check for whether we're running as Windows serv Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 14+ messages in thread

From: Heikki Linnakangas @ 2017-03-17 09:18 UTC (permalink / raw)
  To: pgsql-committers

Fix and simplify check for whether we're running as Windows service.

If the process token contains SECURITY_SERVICE_RID, but it has been
disabled by the SE_GROUP_USE_FOR_DENY_ONLY attribute, win32_is_service()
would incorrectly report that we're running as a service. That situation
arises, e.g. if postmaster is launched with a restricted security token,
with the "Log in as Service" privilege explicitly removed.

Replace the broken code with CheckProcessTokenMembership(), which does
this correctly. Also replace similar code in win32_is_admin(), even
though it got this right, for simplicity and consistency.

Per bug #13755, reported by Breen Hagan. Back-patch to all supported
versions. Patch by Takayuki Tsunakawa, reviewed by Michael Paquier.

Discussion: https://www.postgresql.org/message-id/20151104062315.2745.67143%40wrigleys.postgresql.org

Branch
------
master

Details
-------
http://git.postgresql.org/pg/commitdiff/ff30aec759bdc4de78912d91f650ec8fd95ff6bc

Modified Files
--------------
src/port/win32security.c | 180 ++++++++++-------------------------------------
1 file changed, 38 insertions(+), 142 deletions(-)


-- 
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers


^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: [COMMITTERS] pgsql: Fix and simplify check for whether we're running as Windows serv
  2017-03-17 09:18 pgsql: Fix and simplify check for whether we're running as Windows serv Heikki Linnakangas <heikki.linnakangas@iki.fi>
@ 2017-03-17 17:57 ` Tom Lane <tgl@sss.pgh.pa.us>
  2017-03-19 21:26   ` Re: [COMMITTERS] pgsql: Fix and simplify check for whether we're running as Windows serv Heikki Linnakangas <hlinnaka@iki.fi>
  0 siblings, 1 reply; 14+ messages in thread

From: Tom Lane @ 2017-03-17 17:57 UTC (permalink / raw)
  To: Heikki Linnakangas <heikki.linnakangas@iki.fi>; +Cc: pgsql-hackers@postgreSQL.org

Heikki Linnakangas <heikki.linnakangas@iki.fi> writes:
> Fix and simplify check for whether we're running as Windows service.

This seems to have broken narwhal:

Creating library file: libpostgres.a
../../src/port/libpgport_srv.a(win32security_srv.o)(.text+0x109): In function `pgwin32_is_admin':
C:/msys/1.0/local/pgbuildfarm/buildroot/HEAD/pgsql.build/src/port/win32security.c:77: undefined reference to `CheckTokenMembership'
../../src/port/libpgport_srv.a(win32security_srv.o)(.text+0x127):C:/msys/1.0/local/pgbuildfarm/buildroot/HEAD/pgsql.build/src/port/win32security.c:77: undefined reference to `CheckTokenMembership'
../../src/port/libpgport_srv.a(win32security_srv.o)(.text+0x264): In function `pgwin32_is_service':
C:/msys/1.0/local/pgbuildfarm/buildroot/HEAD/pgsql.build/src/port/win32security.c:138: undefined reference to `CheckTokenMembership'
../../src/port/libpgport_srv.a(win32security_srv.o)(.text+0x302):C:/msys/1.0/local/pgbuildfarm/buildroot/HEAD/pgsql.build/src/port/win32security.c:163: undefined reference to `CheckTokenMembership'
collect2: ld returned 1 exit status
make[2]: *** [postgres] Error 1

			regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers



^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: [COMMITTERS] pgsql: Fix and simplify check for whether we're running as Windows serv
  2017-03-17 09:18 pgsql: Fix and simplify check for whether we're running as Windows serv Heikki Linnakangas <heikki.linnakangas@iki.fi>
  2017-03-17 17:57 ` Re: [COMMITTERS] pgsql: Fix and simplify check for whether we're running as Windows serv Tom Lane <tgl@sss.pgh.pa.us>
@ 2017-03-19 21:26   ` Heikki Linnakangas <hlinnaka@iki.fi>
  2017-03-19 21:40     ` Re: [COMMITTERS] pgsql: Fix and simplify check for whether we're running as Windows serv Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 14+ messages in thread

From: Heikki Linnakangas @ 2017-03-19 21:26 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; Dave Page <dpage@postgresql.org>; +Cc: pgsql-hackers@postgreSQL.org

On 03/17/2017 07:57 PM, Tom Lane wrote:
> Heikki Linnakangas <heikki.linnakangas@iki.fi> writes:
>> Fix and simplify check for whether we're running as Windows service.
>
> This seems to have broken narwhal:
>
> Creating library file: libpostgres.a
> ../../src/port/libpgport_srv.a(win32security_srv.o)(.text+0x109): In function `pgwin32_is_admin':
> C:/msys/1.0/local/pgbuildfarm/buildroot/HEAD/pgsql.build/src/port/win32security.c:77: undefined reference to `CheckTokenMembership'
> ../../src/port/libpgport_srv.a(win32security_srv.o)(.text+0x127):C:/msys/1.0/local/pgbuildfarm/buildroot/HEAD/pgsql.build/src/port/win32security.c:77: undefined reference to `CheckTokenMembership'
> ../../src/port/libpgport_srv.a(win32security_srv.o)(.text+0x264): In function `pgwin32_is_service':
> C:/msys/1.0/local/pgbuildfarm/buildroot/HEAD/pgsql.build/src/port/win32security.c:138: undefined reference to `CheckTokenMembership'
> ../../src/port/libpgport_srv.a(win32security_srv.o)(.text+0x302):C:/msys/1.0/local/pgbuildfarm/buildroot/HEAD/pgsql.build/src/port/win32security.c:163: undefined reference to `CheckTokenMembership'
> collect2: ld returned 1 exit status
> make[2]: *** [postgres] Error 1

Hmm. The other MinGW animals are not having that problem.

According to the MSDN docs [1], CheckTokenMembership() is defined in the 
Advapi32 library. In pg_ctl.c and in 
src/include/common/restricted_token.c, we currently do this, for another 
function that's in Advapi32:

> /*
>  * Mingw headers are incomplete, and so are the libraries. So we have to load
>  * a whole lot of API functions dynamically. Since we have to do this anyway,
>  * also load the couple of functions that *do* exist in minwg headers but not
>  * on NT4. That way, we don't break on NT4.
>  */
> typedef BOOL (WINAPI * __CreateRestrictedToken) (HANDLE, DWORD, DWORD, PSID_AND_ATTRIBUTES, DWORD, PLUID_AND_ATTRIBUTES, DWORD, PSID_AND_ATTRIBUTES, PHANDLE);
> typedef BOOL (WINAPI * __IsProcessInJob) (HANDLE, HANDLE, PBOOL);
> typedef HANDLE (WINAPI * __CreateJobObject) (LPSECURITY_ATTRIBUTES, LPCTSTR);
> typedef BOOL (WINAPI * __SetInformationJobObject) (HANDLE, JOBOBJECTINFOCLASS, LPVOID, DWORD);
> typedef BOOL (WINAPI * __AssignProcessToJobObject) (HANDLE, HANDLE);
> typedef BOOL (WINAPI * __QueryInformationJobObject) (HANDLE, JOBOBJECTINFOCLASS, LPVOID, DWORD, LPDWORD);

CheckTokenMembership() is probably one of those missing functions, like 
CreateRestrictedToken.

I'd like to bump our minimum requirements,and stop supporting old MinGW 
versions that don't have those headers. I'm not sure what version they 
were added in, but frogmouth doesn't have this problem, and it uses gcc 
4.5.0. Once we do that, we can simplify pg_ctl.c and restricted_token.c 
to not open advapi32.dll dynamically, and call those functions the usual 
way.

It's not very nice to change the requirements in a minor version, but I 
don't think this would be a real problem for anyone. Not many people 
build PostgreSQL using MinGW, let alone with an ancient version of it. 
But if people don't agree, we could instead revert this patch and apply 
the smaller V2 patch [2] instead, in the back-branches.

Thoughts? Any objections to requiring a newer version of MinGW? Any 
objections to do so in the next minor release?

[1] 
https://msdn.microsoft.com/en-us/library/windows/desktop/aa376389%28v=vs.85%29.aspx

[2] 
https://www.postgresql.org/message-id/CAB7nPqSvfu%3DKpJ%3DNX%2BYAHmgAmQdzA7N5h31BjzXeMgczhGCC%2BQ%40...

- Heikki



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers



^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: [COMMITTERS] pgsql: Fix and simplify check for whether we're running as Windows serv
  2017-03-17 09:18 pgsql: Fix and simplify check for whether we're running as Windows serv Heikki Linnakangas <heikki.linnakangas@iki.fi>
  2017-03-17 17:57 ` Re: [COMMITTERS] pgsql: Fix and simplify check for whether we're running as Windows serv Tom Lane <tgl@sss.pgh.pa.us>
  2017-03-19 21:26   ` Re: [COMMITTERS] pgsql: Fix and simplify check for whether we're running as Windows serv Heikki Linnakangas <hlinnaka@iki.fi>
@ 2017-03-19 21:40     ` Tom Lane <tgl@sss.pgh.pa.us>
  2017-03-22 10:48       ` Re: [COMMITTERS] pgsql: Fix and simplify check for whether we're running as Windows serv Heikki Linnakangas <hlinnaka@iki.fi>
  0 siblings, 1 reply; 14+ messages in thread

From: Tom Lane @ 2017-03-19 21:40 UTC (permalink / raw)
  To: Heikki Linnakangas <hlinnaka@iki.fi>; +Cc: Dave Page <dpage@postgresql.org>; pgsql-hackers@postgresql.org

Heikki Linnakangas <hlinnaka@iki.fi> writes:
> On 03/17/2017 07:57 PM, Tom Lane wrote:
>> This seems to have broken narwhal:

> It's not very nice to change the requirements in a minor version, but I 
> don't think this would be a real problem for anyone. Not many people 
> build PostgreSQL using MinGW, let alone with an ancient version of it. 
> But if people don't agree, we could instead revert this patch and apply 
> the smaller V2 patch [2] instead, in the back-branches.

> Thoughts? Any objections to requiring a newer version of MinGW? Any 
> objections to do so in the next minor release?

Hm.  I'm +1 for doing that in HEAD, but less so for the back branches.

Can we get some fix on when the functions in question were added
to MinGW?  If we knew how new a toolchain we'd be requiring here,
that would help make this decision.

			regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers



^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: [COMMITTERS] pgsql: Fix and simplify check for whether we're running as Windows serv
  2017-03-17 09:18 pgsql: Fix and simplify check for whether we're running as Windows serv Heikki Linnakangas <heikki.linnakangas@iki.fi>
  2017-03-17 17:57 ` Re: [COMMITTERS] pgsql: Fix and simplify check for whether we're running as Windows serv Tom Lane <tgl@sss.pgh.pa.us>
  2017-03-19 21:26   ` Re: [COMMITTERS] pgsql: Fix and simplify check for whether we're running as Windows serv Heikki Linnakangas <hlinnaka@iki.fi>
  2017-03-19 21:40     ` Re: [COMMITTERS] pgsql: Fix and simplify check for whether we're running as Windows serv Tom Lane <tgl@sss.pgh.pa.us>
@ 2017-03-22 10:48       ` Heikki Linnakangas <hlinnaka@iki.fi>
  2017-03-22 14:13         ` Re: Re: [COMMITTERS] pgsql: Fix and simplify check for whether we're running as Windows serv Alvaro Herrera <alvherre@2ndquadrant.com>
  0 siblings, 1 reply; 14+ messages in thread

From: Heikki Linnakangas @ 2017-03-22 10:48 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Dave Page <dpage@postgresql.org>; pgsql-hackers@postgresql.org

On 03/19/2017 11:40 PM, Tom Lane wrote:
> Heikki Linnakangas <hlinnaka@iki.fi> writes:
>> On 03/17/2017 07:57 PM, Tom Lane wrote:
>>> This seems to have broken narwhal:
>
>> It's not very nice to change the requirements in a minor version, but I
>> don't think this would be a real problem for anyone. Not many people
>> build PostgreSQL using MinGW, let alone with an ancient version of it.
>> But if people don't agree, we could instead revert this patch and apply
>> the smaller V2 patch [2] instead, in the back-branches.
>
>> Thoughts? Any objections to requiring a newer version of MinGW? Any
>> objections to do so in the next minor release?
>
> Hm.  I'm +1 for doing that in HEAD, but less so for the back branches.
>
> Can we get some fix on when the functions in question were added
> to MinGW?  If we knew how new a toolchain we'd be requiring here,
> that would help make this decision.

I did some archeology, and found CheckTokenMembership() in MinGW's 
w32api packages version 3.14 
(https://sourceforge.net/projects/mingw/files/MinGW/Base/w32api/w32api-3.14/, 
in include/winbase.h). According to the timestamps on that download 
page, that was released in 2009. That was the oldest version I could 
find, so it might go even further back.

Dave, do you know exactly what version of MinGW narwhal is running? And 
how difficult is it to upgrade to something slightly more modern? Ease 
of upgrade is another good data point on how far we need to support old 
versions.

- Heikki



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers



^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: Re: [COMMITTERS] pgsql: Fix and simplify check for whether we're running as Windows serv
  2017-03-17 09:18 pgsql: Fix and simplify check for whether we're running as Windows serv Heikki Linnakangas <heikki.linnakangas@iki.fi>
  2017-03-17 17:57 ` Re: [COMMITTERS] pgsql: Fix and simplify check for whether we're running as Windows serv Tom Lane <tgl@sss.pgh.pa.us>
  2017-03-19 21:26   ` Re: [COMMITTERS] pgsql: Fix and simplify check for whether we're running as Windows serv Heikki Linnakangas <hlinnaka@iki.fi>
  2017-03-19 21:40     ` Re: [COMMITTERS] pgsql: Fix and simplify check for whether we're running as Windows serv Tom Lane <tgl@sss.pgh.pa.us>
  2017-03-22 10:48       ` Re: [COMMITTERS] pgsql: Fix and simplify check for whether we're running as Windows serv Heikki Linnakangas <hlinnaka@iki.fi>
@ 2017-03-22 14:13         ` Alvaro Herrera <alvherre@2ndquadrant.com>
  2017-03-22 17:44           ` Re: Re: [COMMITTERS] pgsql: Fix and simplify check for whether we're running as Windows serv Robert Haas <robertmhaas@gmail.com>
  0 siblings, 1 reply; 14+ messages in thread

From: Alvaro Herrera @ 2017-03-22 14:13 UTC (permalink / raw)
  To: Heikki Linnakangas <hlinnaka@iki.fi>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Dave Page <dpage@postgresql.org>; pgsql-hackers@postgresql.org, "Tsunakawa, Takayuki" <tsunakawa.takay@jp.fujitsu.com>

Heikki Linnakangas wrote:

> I did some archeology, and found CheckTokenMembership() in MinGW's w32api
> packages version 3.14
> (https://sourceforge.net/projects/mingw/files/MinGW/Base/w32api/w32api-3.14/,
> in include/winbase.h). According to the timestamps on that download page,
> that was released in 2009. That was the oldest version I could find, so it
> might go even further back.
> 
> Dave, do you know exactly what version of MinGW narwhal is running? And how
> difficult is it to upgrade to something slightly more modern? Ease of
> upgrade is another good data point on how far we need to support old
> versions.

Given that this was backpatched and that it broke narwhal in all
branches, I think the solution needs to make narwhal work again without
requiring it to upgrade; so we should acquire CheckTokenMembership via
dynloading just like we do the other functions.  If we want to require a
newer mingw version in pg10, that's acceptable, but it should be a
separate patch.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers



^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: Re: [COMMITTERS] pgsql: Fix and simplify check for whether we're running as Windows serv
  2017-03-17 09:18 pgsql: Fix and simplify check for whether we're running as Windows serv Heikki Linnakangas <heikki.linnakangas@iki.fi>
  2017-03-17 17:57 ` Re: [COMMITTERS] pgsql: Fix and simplify check for whether we're running as Windows serv Tom Lane <tgl@sss.pgh.pa.us>
  2017-03-19 21:26   ` Re: [COMMITTERS] pgsql: Fix and simplify check for whether we're running as Windows serv Heikki Linnakangas <hlinnaka@iki.fi>
  2017-03-19 21:40     ` Re: [COMMITTERS] pgsql: Fix and simplify check for whether we're running as Windows serv Tom Lane <tgl@sss.pgh.pa.us>
  2017-03-22 10:48       ` Re: [COMMITTERS] pgsql: Fix and simplify check for whether we're running as Windows serv Heikki Linnakangas <hlinnaka@iki.fi>
  2017-03-22 14:13         ` Re: Re: [COMMITTERS] pgsql: Fix and simplify check for whether we're running as Windows serv Alvaro Herrera <alvherre@2ndquadrant.com>
@ 2017-03-22 17:44           ` Robert Haas <robertmhaas@gmail.com>
  2017-03-24 10:44             ` Re: Re: [COMMITTERS] pgsql: Fix and simplify check for whether we're running as Windows serv Heikki Linnakangas <hlinnaka@iki.fi>
  0 siblings, 1 reply; 14+ messages in thread

From: Robert Haas @ 2017-03-22 17:44 UTC (permalink / raw)
  To: Alvaro Herrera <alvherre@2ndquadrant.com>; +Cc: Heikki Linnakangas <hlinnaka@iki.fi>; Tom Lane <tgl@sss.pgh.pa.us>; Dave Page <dpage@postgresql.org>; pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>; Tsunakawa, Takayuki <tsunakawa.takay@jp.fujitsu.com>

On Wed, Mar 22, 2017 at 10:13 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
> Heikki Linnakangas wrote:
>> I did some archeology, and found CheckTokenMembership() in MinGW's w32api
>> packages version 3.14
>> (https://sourceforge.net/projects/mingw/files/MinGW/Base/w32api/w32api-3.14/,
>> in include/winbase.h). According to the timestamps on that download page,
>> that was released in 2009. That was the oldest version I could find, so it
>> might go even further back.
>>
>> Dave, do you know exactly what version of MinGW narwhal is running? And how
>> difficult is it to upgrade to something slightly more modern? Ease of
>> upgrade is another good data point on how far we need to support old
>> versions.
>
> Given that this was backpatched and that it broke narwhal in all
> branches, I think the solution needs to make narwhal work again without
> requiring it to upgrade; so we should acquire CheckTokenMembership via
> dynloading just like we do the other functions.  If we want to require a
> newer mingw version in pg10, that's acceptable, but it should be a
> separate patch.

+1 for not moving the minimum system requirements in the back-branches.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers



^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: Re: [COMMITTERS] pgsql: Fix and simplify check for whether we're running as Windows serv
  2017-03-17 09:18 pgsql: Fix and simplify check for whether we're running as Windows serv Heikki Linnakangas <heikki.linnakangas@iki.fi>
  2017-03-17 17:57 ` Re: [COMMITTERS] pgsql: Fix and simplify check for whether we're running as Windows serv Tom Lane <tgl@sss.pgh.pa.us>
  2017-03-19 21:26   ` Re: [COMMITTERS] pgsql: Fix and simplify check for whether we're running as Windows serv Heikki Linnakangas <hlinnaka@iki.fi>
  2017-03-19 21:40     ` Re: [COMMITTERS] pgsql: Fix and simplify check for whether we're running as Windows serv Tom Lane <tgl@sss.pgh.pa.us>
  2017-03-22 10:48       ` Re: [COMMITTERS] pgsql: Fix and simplify check for whether we're running as Windows serv Heikki Linnakangas <hlinnaka@iki.fi>
  2017-03-22 14:13         ` Re: Re: [COMMITTERS] pgsql: Fix and simplify check for whether we're running as Windows serv Alvaro Herrera <alvherre@2ndquadrant.com>
  2017-03-22 17:44           ` Re: Re: [COMMITTERS] pgsql: Fix and simplify check for whether we're running as Windows serv Robert Haas <robertmhaas@gmail.com>
@ 2017-03-24 10:44             ` Heikki Linnakangas <hlinnaka@iki.fi>
  2017-03-24 11:35               ` Re: [COMMITTERS] pgsql: Fix and simplify check for whether we're running as Windows serv Dave Page <dpage@postgresql.org>
  0 siblings, 1 reply; 14+ messages in thread

From: Heikki Linnakangas @ 2017-03-24 10:44 UTC (permalink / raw)
  To: Robert Haas <robertmhaas@gmail.com>; Alvaro Herrera <alvherre@2ndquadrant.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Dave Page <dpage@postgresql.org>; pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>; Tsunakawa, Takayuki <tsunakawa.takay@jp.fujitsu.com>

On 03/22/2017 07:44 PM, Robert Haas wrote:
> On Wed, Mar 22, 2017 at 10:13 AM, Alvaro Herrera
> <alvherre@2ndquadrant.com> wrote:
>> Heikki Linnakangas wrote:
>>> I did some archeology, and found CheckTokenMembership() in MinGW's w32api
>>> packages version 3.14
>>> (https://sourceforge.net/projects/mingw/files/MinGW/Base/w32api/w32api-3.14/,
>>> in include/winbase.h). According to the timestamps on that download page,
>>> that was released in 2009. That was the oldest version I could find, so it
>>> might go even further back.
>>>
>>> Dave, do you know exactly what version of MinGW narwhal is running? And how
>>> difficult is it to upgrade to something slightly more modern? Ease of
>>> upgrade is another good data point on how far we need to support old
>>> versions.
>>
>> Given that this was backpatched and that it broke narwhal in all
>> branches, I think the solution needs to make narwhal work again without
>> requiring it to upgrade; so we should acquire CheckTokenMembership via
>> dynloading just like we do the other functions.  If we want to require a
>> newer mingw version in pg10, that's acceptable, but it should be a
>> separate patch.
>
> +1 for not moving the minimum system requirements in the back-branches.

Ok. I reverted this patch in the back-branches, and applied the much 
less invasive "V2" patch [1] instead. HEAD is unchanged, so narwhal 
still fails there.

Dave: the consensus is that we no longer support the old version of 
MinGW that narwhal is using, for PostgreSQL v 10. Can you modify the 
configuration of narwhal to not try building 'master' anymore, or 
upgrade the toolchain, please?

[1] 
https://www.postgresql.org/message-id/CAB7nPqSvfu%3DKpJ%3DNX%2BYAHmgAmQdzA7N5h31BjzXeMgczhGCC%2BQ%40...

- Heikki



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers



^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: [COMMITTERS] pgsql: Fix and simplify check for whether we're running as Windows serv
  2017-03-17 09:18 pgsql: Fix and simplify check for whether we're running as Windows serv Heikki Linnakangas <heikki.linnakangas@iki.fi>
  2017-03-17 17:57 ` Re: [COMMITTERS] pgsql: Fix and simplify check for whether we're running as Windows serv Tom Lane <tgl@sss.pgh.pa.us>
  2017-03-19 21:26   ` Re: [COMMITTERS] pgsql: Fix and simplify check for whether we're running as Windows serv Heikki Linnakangas <hlinnaka@iki.fi>
  2017-03-19 21:40     ` Re: [COMMITTERS] pgsql: Fix and simplify check for whether we're running as Windows serv Tom Lane <tgl@sss.pgh.pa.us>
  2017-03-22 10:48       ` Re: [COMMITTERS] pgsql: Fix and simplify check for whether we're running as Windows serv Heikki Linnakangas <hlinnaka@iki.fi>
  2017-03-22 14:13         ` Re: Re: [COMMITTERS] pgsql: Fix and simplify check for whether we're running as Windows serv Alvaro Herrera <alvherre@2ndquadrant.com>
  2017-03-22 17:44           ` Re: Re: [COMMITTERS] pgsql: Fix and simplify check for whether we're running as Windows serv Robert Haas <robertmhaas@gmail.com>
  2017-03-24 10:44             ` Re: Re: [COMMITTERS] pgsql: Fix and simplify check for whether we're running as Windows serv Heikki Linnakangas <hlinnaka@iki.fi>
@ 2017-03-24 11:35               ` Dave Page <dpage@postgresql.org>
  0 siblings, 0 replies; 14+ messages in thread

From: Dave Page @ 2017-03-24 11:35 UTC (permalink / raw)
  To: Heikki Linnakangas <hlinnaka@iki.fi>; +Cc: Robert Haas <robertmhaas@gmail.com>; Alvaro Herrera <alvherre@2ndquadrant.com>; Tom Lane <tgl@sss.pgh.pa.us>; pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>; Tsunakawa, Takayuki <tsunakawa.takay@jp.fujitsu.com>

On Friday, March 24, 2017, Heikki Linnakangas <hlinnaka@iki.fi> wrote:

> On 03/22/2017 07:44 PM, Robert Haas wrote:
>
>> On Wed, Mar 22, 2017 at 10:13 AM, Alvaro Herrera
>> <alvherre@2ndquadrant.com> wrote:
>>
>>> Heikki Linnakangas wrote:
>>>
>>>> I did some archeology, and found CheckTokenMembership() in MinGW's
>>>> w32api
>>>> packages version 3.14
>>>> (https://sourceforge.net/projects/mingw/files/MinGW/Base/
>>>> w32api/w32api-3.14/,
>>>> in include/winbase.h). According to the timestamps on that download
>>>> page,
>>>> that was released in 2009. That was the oldest version I could find, so
>>>> it
>>>> might go even further back.
>>>>
>>>> Dave, do you know exactly what version of MinGW narwhal is running? And
>>>> how
>>>> difficult is it to upgrade to something slightly more modern? Ease of
>>>> upgrade is another good data point on how far we need to support old
>>>> versions.
>>>>
>>>
>>> Given that this was backpatched and that it broke narwhal in all
>>> branches, I think the solution needs to make narwhal work again without
>>> requiring it to upgrade; so we should acquire CheckTokenMembership via
>>> dynloading just like we do the other functions.  If we want to require a
>>> newer mingw version in pg10, that's acceptable, but it should be a
>>> separate patch.
>>>
>>
>> +1 for not moving the minimum system requirements in the back-branches.
>>
>
> Ok. I reverted this patch in the back-branches, and applied the much less
> invasive "V2" patch [1] instead. HEAD is unchanged, so narwhal still fails
> there.
>
> Dave: the consensus is that we no longer support the old version of MinGW
> that narwhal is using, for PostgreSQL v 10. Can you modify the
> configuration of narwhal to not try building 'master' anymore, or upgrade
> the toolchain, please?
>

I've disabled it. Thanks.


-- 
Dave Page
PostgreSQL Core Team
http://www.postgresql.org/

^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* pgsql: Fix and simplify check for whether we're running as Windows serv
@ 2017-03-17 09:18 Heikki Linnakangas <heikki.linnakangas@iki.fi>
  0 siblings, 0 replies; 14+ messages in thread

From: Heikki Linnakangas @ 2017-03-17 09:18 UTC (permalink / raw)
  To: pgsql-committers

Fix and simplify check for whether we're running as Windows service.

If the process token contains SECURITY_SERVICE_RID, but it has been
disabled by the SE_GROUP_USE_FOR_DENY_ONLY attribute, win32_is_service()
would incorrectly report that we're running as a service. That situation
arises, e.g. if postmaster is launched with a restricted security token,
with the "Log in as Service" privilege explicitly removed.

Replace the broken code with CheckProcessTokenMembership(), which does
this correctly. Also replace similar code in win32_is_admin(), even
though it got this right, for simplicity and consistency.

Per bug #13755, reported by Breen Hagan. Back-patch to all supported
versions. Patch by Takayuki Tsunakawa, reviewed by Michael Paquier.

Discussion: https://www.postgresql.org/message-id/20151104062315.2745.67143%40wrigleys.postgresql.org

Branch
------
REL9_6_STABLE

Details
-------
http://git.postgresql.org/pg/commitdiff/38bdba54a64bacec78e3266f0848b0b4a824132a

Modified Files
--------------
src/port/win32security.c | 180 ++++++++++-------------------------------------
1 file changed, 38 insertions(+), 142 deletions(-)


-- 
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers


^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* pgsql: Fix and simplify check for whether we're running as Windows serv
@ 2017-03-17 09:18 Heikki Linnakangas <heikki.linnakangas@iki.fi>
  0 siblings, 0 replies; 14+ messages in thread

From: Heikki Linnakangas @ 2017-03-17 09:18 UTC (permalink / raw)
  To: pgsql-committers

Fix and simplify check for whether we're running as Windows service.

If the process token contains SECURITY_SERVICE_RID, but it has been
disabled by the SE_GROUP_USE_FOR_DENY_ONLY attribute, win32_is_service()
would incorrectly report that we're running as a service. That situation
arises, e.g. if postmaster is launched with a restricted security token,
with the "Log in as Service" privilege explicitly removed.

Replace the broken code with CheckProcessTokenMembership(), which does
this correctly. Also replace similar code in win32_is_admin(), even
though it got this right, for simplicity and consistency.

Per bug #13755, reported by Breen Hagan. Back-patch to all supported
versions. Patch by Takayuki Tsunakawa, reviewed by Michael Paquier.

Discussion: https://www.postgresql.org/message-id/20151104062315.2745.67143%40wrigleys.postgresql.org

Branch
------
REL9_5_STABLE

Details
-------
http://git.postgresql.org/pg/commitdiff/96fd76dd287593b3b444ebddc1f817bd08bc812a

Modified Files
--------------
src/backend/port/win32/security.c | 173 +++++++++-----------------------------
1 file changed, 38 insertions(+), 135 deletions(-)


-- 
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers


^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* pgsql: Fix and simplify check for whether we're running as Windows serv
@ 2017-03-17 09:18 Heikki Linnakangas <heikki.linnakangas@iki.fi>
  0 siblings, 0 replies; 14+ messages in thread

From: Heikki Linnakangas @ 2017-03-17 09:18 UTC (permalink / raw)
  To: pgsql-committers

Fix and simplify check for whether we're running as Windows service.

If the process token contains SECURITY_SERVICE_RID, but it has been
disabled by the SE_GROUP_USE_FOR_DENY_ONLY attribute, win32_is_service()
would incorrectly report that we're running as a service. That situation
arises, e.g. if postmaster is launched with a restricted security token,
with the "Log in as Service" privilege explicitly removed.

Replace the broken code with CheckProcessTokenMembership(), which does
this correctly. Also replace similar code in win32_is_admin(), even
though it got this right, for simplicity and consistency.

Per bug #13755, reported by Breen Hagan. Back-patch to all supported
versions. Patch by Takayuki Tsunakawa, reviewed by Michael Paquier.

Discussion: https://www.postgresql.org/message-id/20151104062315.2745.67143%40wrigleys.postgresql.org

Branch
------
REL9_4_STABLE

Details
-------
http://git.postgresql.org/pg/commitdiff/6b584c36a40ca68bd7f1943eb34ce1507b16bb2c

Modified Files
--------------
src/backend/port/win32/security.c | 173 +++++++++-----------------------------
1 file changed, 38 insertions(+), 135 deletions(-)


-- 
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers


^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* pgsql: Fix and simplify check for whether we're running as Windows serv
@ 2017-03-17 09:18 Heikki Linnakangas <heikki.linnakangas@iki.fi>
  0 siblings, 0 replies; 14+ messages in thread

From: Heikki Linnakangas @ 2017-03-17 09:18 UTC (permalink / raw)
  To: pgsql-committers

Fix and simplify check for whether we're running as Windows service.

If the process token contains SECURITY_SERVICE_RID, but it has been
disabled by the SE_GROUP_USE_FOR_DENY_ONLY attribute, win32_is_service()
would incorrectly report that we're running as a service. That situation
arises, e.g. if postmaster is launched with a restricted security token,
with the "Log in as Service" privilege explicitly removed.

Replace the broken code with CheckProcessTokenMembership(), which does
this correctly. Also replace similar code in win32_is_admin(), even
though it got this right, for simplicity and consistency.

Per bug #13755, reported by Breen Hagan. Back-patch to all supported
versions. Patch by Takayuki Tsunakawa, reviewed by Michael Paquier.

Discussion: https://www.postgresql.org/message-id/20151104062315.2745.67143%40wrigleys.postgresql.org

Branch
------
REL9_3_STABLE

Details
-------
http://git.postgresql.org/pg/commitdiff/3ebcc2498d2b011d42f7ad6a50728b9d7cfb1286

Modified Files
--------------
src/backend/port/win32/security.c | 173 +++++++++-----------------------------
1 file changed, 38 insertions(+), 135 deletions(-)


-- 
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers


^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* pgsql: Fix and simplify check for whether we're running as Windows serv
@ 2017-03-17 09:18 Heikki Linnakangas <heikki.linnakangas@iki.fi>
  0 siblings, 0 replies; 14+ messages in thread

From: Heikki Linnakangas @ 2017-03-17 09:18 UTC (permalink / raw)
  To: pgsql-committers

Fix and simplify check for whether we're running as Windows service.

If the process token contains SECURITY_SERVICE_RID, but it has been
disabled by the SE_GROUP_USE_FOR_DENY_ONLY attribute, win32_is_service()
would incorrectly report that we're running as a service. That situation
arises, e.g. if postmaster is launched with a restricted security token,
with the "Log in as Service" privilege explicitly removed.

Replace the broken code with CheckProcessTokenMembership(), which does
this correctly. Also replace similar code in win32_is_admin(), even
though it got this right, for simplicity and consistency.

Per bug #13755, reported by Breen Hagan. Back-patch to all supported
versions. Patch by Takayuki Tsunakawa, reviewed by Michael Paquier.

Discussion: https://www.postgresql.org/message-id/20151104062315.2745.67143%40wrigleys.postgresql.org

Branch
------
REL9_2_STABLE

Details
-------
http://git.postgresql.org/pg/commitdiff/9c52ddfcee067314a8db27ccd8c59dafe3059375

Modified Files
--------------
src/backend/port/win32/security.c | 173 +++++++++-----------------------------
1 file changed, 38 insertions(+), 135 deletions(-)


-- 
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers


^ permalink  raw  reply  [nested|flat] 14+ messages in thread


end of thread, other threads:[~2017-03-24 11:35 UTC | newest]

Thread overview: 14+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2017-03-17 09:18 pgsql: Fix and simplify check for whether we're running as Windows serv Heikki Linnakangas <heikki.linnakangas@iki.fi>
2017-03-17 17:57 ` Tom Lane <tgl@sss.pgh.pa.us>
2017-03-19 21:26   ` Heikki Linnakangas <hlinnaka@iki.fi>
2017-03-19 21:40     ` Tom Lane <tgl@sss.pgh.pa.us>
2017-03-22 10:48       ` Heikki Linnakangas <hlinnaka@iki.fi>
2017-03-22 14:13         ` Alvaro Herrera <alvherre@2ndquadrant.com>
2017-03-22 17:44           ` Robert Haas <robertmhaas@gmail.com>
2017-03-24 10:44             ` Heikki Linnakangas <hlinnaka@iki.fi>
2017-03-24 11:35               ` Dave Page <dpage@postgresql.org>
2017-03-17 09:18 pgsql: Fix and simplify check for whether we're running as Windows serv Heikki Linnakangas <heikki.linnakangas@iki.fi>
2017-03-17 09:18 pgsql: Fix and simplify check for whether we're running as Windows serv Heikki Linnakangas <heikki.linnakangas@iki.fi>
2017-03-17 09:18 pgsql: Fix and simplify check for whether we're running as Windows serv Heikki Linnakangas <heikki.linnakangas@iki.fi>
2017-03-17 09:18 pgsql: Fix and simplify check for whether we're running as Windows serv Heikki Linnakangas <heikki.linnakangas@iki.fi>
2017-03-17 09:18 pgsql: Fix and simplify check for whether we're running as Windows serv Heikki Linnakangas <heikki.linnakangas@iki.fi>

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox