Re: Re: BUG #13755: pgwin32_is_service not checking if SECURITY_SERVICE_SID is disabled

On Mon, Nov 7, 2016 at 9:49 AM, Tsunakawa, Takayuki
<tsunakawa(dot)takay(at)jp(dot)fujitsu(dot)com> wrote:
> From: pgsql-hackers-owner(at)postgresql(dot)org
>> [mailto:pgsql-hackers-owner(at)postgresql(dot)org] On Behalf Of Michael Paquier
>> On Sun, Nov 6, 2016 at 6:30 PM, MauMau <maumau307(at)gmail(dot)com> wrote:
>> So you see the same behavior with the patch I sent and your refactoring,
>> right? If yes, backpatching the one-liner is the safest bet to me. We could
>> keep the refactoring for HEAD if it makes sense.
>
> Yes. And It's fine to me that your patch will be applied to previous releases and my patch to HEAD only. This is a good (rare?) chance to reduce the Windows-specific code, so I want to take advantage of it.

Yes, I can follow that argument.

>> Something is wrong with the format of your patch by the way. My Windows
>> and even OSX environments recognize it as a binary file, though I can read
>> it in any editor and I cannot apply it cleanly with a simple patch command.
>> Could you send it again and double-check?
>
> Ouch, the Git shell included in GitHub Desktop for Windows produced the diff in UTF-16 and CR/LF line terminators. I haven't found how to fix it, so I generated the attached patch on Linux. Please check it.

And the patch got twice smaller in size. Thanks.

>> > To reproduce the OP's problem, I modified pg_ctl.c to disable
>> > SECURITY_SERVICE_RID when spawning postgres.exe.
>>
>> So basically you allocated a SID to drop via AllocateAndInitializeSid,
>> called _CreateRestrictedToken and let the process being spawned? I think
>> that this is the patch attached (win32-disable-service-rid.patch). Could
>> you confirm? I want to be sure that we are testing the same things.
>
> Yes, I did the same.

Hm.. I have just tested HEAD, my patch and your patch using my patch
test on pg_ctl.c, but I am always getting pgwin32_is_service set to 0
when running pg_ctl start from a terminal, and set it to 1 when
running pg_ctl service to register the service startup. Could you
precise in which ways you started the Postgres instance and could you
post the patch of pg_ctl you used? I am afraid that I am taking it
incorrectly because I am not able to see any differences.

Also, did you test the patch I posted and were you able to see the
same differences as with your patch? I still think that my short patch
is logically correct but if the tests are not we are in a no-go
position for any fix posted on this thread.
--
Michael