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

On Wed, Mar 9, 2016 at 11:44 PM, Breen Hagan <breen@rtda.com> wrote:
>
>
> On Sat, Nov 7, 2015 at 1:36 AM, Michael Paquier <michael.paquier@gmail.com>
> wrote:
>>
>> On Sat, Nov 7, 2015 at 4:09 PM, Michael Paquier
>> <michael.paquier@gmail.com> wrote:
>> > On Fri, Nov 6, 2015 at 1:00 AM, Breen Hagan <breen@rtda.com> wrote:
>> >> Michael,
>> >
>> > (You should avoid top-posting, this breaks the logic of a thread).
>> >
>> >> I'm pretty sure your patch will fix my issue, but perhaps it should be
>> >> a
>> >> positive check for SE_GROUP_ENABLED?
>> >
>> > If we want to be completely consistent with pgwin32_is_admin, that
>> > would be actually the opposite: Postgres should not start with an SID
>> > that has administrator's rights for security reasons.
>>
>> SECURITY_SERVICE_RID and SECURITY_BUILTIN_DOMAIN_RID are completely
>> separated concepts... Please ignore that. Still, yeah, it seems that
>> you are right, we would want SE_GROUP_ENABLED to be enabled to check
>> if process can access the event logs. Thoughts from any Windows ninja
>> in the surroundings?
>>
>> --
>> Michael
>
>
> Sorry to bring back a very old thread, but I was wondering if this was ever
> resolved? I saw
> an item in the 9.4.6 release notes that seemed similar, but upon checking
> the code, I see
> that pgwin32_is_service() still checks just for the existence of these RIDs
> without checking
> to see if they are enabled.

This is not resolved yet, this just fell from my radar and I recall
that I spent some time thinking about the consequences and whereabouts
of using either SE_GROUP_ENABLED or SE_GROUP_USE_FOR_DENY_ONLY,
without actually reaching a conclusion. I think that the patch would
be straight-forward. But it needs a bit of review from the author
(Hi!) and some extra input would be welcome. I guess I could try to
look at that again.. That won't be this week for sure though.
--
Michael