BUG #13755: pgwin32_is_service not checking if SECURITY_SERVICE_SID is disabled
The following bug has been logged on the website:
Bug reference: 13755
Logged by: Breen Hagan
Email address: breen@rtda.com
PostgreSQL version: 9.4.4
Operating system: Windows 8.1
Description:
Short version: pgwin32_is_service checks the process token for
SECURITY_SERVICE_RID by doing an EqualSid check. This will match against a
SECURITY_SERVICE_RID that has been disabled ("use_for_deny_only"), causing
PG to think it's a service when it is not. This causes it to attempt to log
to the event log, but this doesn't work, and so there is no logging at all.
Long version: We ship PG with our own product, which may or may not be
installed as a service. When running PG, we run postgres.exe directly via a
Tcl-based wrapper script so that we can monitor the output in real time.
This works as expected when our product is not being run as a service.
When our product is installed as a service, we use CreateRestrictedToken to
disable all admin rights as well as the SECURITY_SERVICE_RID, and use the
returned token with CreateProcessAsUser, for which we also specify
CREATE_NEW_CONSOLE. This process then calls our wrapper script. Inside
this wrapper, I can call GetStdHandle (via Twapi) and get valid handles for
all 3: in, out, and err. Yet when the script calls postgres.exe, nothing is
received on the output. As mentioned above, nothing is logged in the event
log, either.
If you look at
https://msdn.microsoft.com/en-us/library/windows/desktop/aa379554(v=vs.85).aspx,
this code is very similar to pgwin32_is_service (except that it looks for
Admins), but also checks the attributes on the SID to see if it is enabled,
or used for deny only. I believe this check needs to be added to
pgwin32_is_service.
Thanks!
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
On Wed, Nov 4, 2015 at 3:23 PM, <breen@rtda.com> wrote:
Short version: pgwin32_is_service checks the process token for
SECURITY_SERVICE_RID by doing an EqualSid check. This will match against a
SECURITY_SERVICE_RID that has been disabled ("use_for_deny_only"), causing
PG to think it's a service when it is not. This causes it to attempt to log
to the event log, but this doesn't work, and so there is no logging at all.
OK. So if I am following correctly... If Postgres process uses a
SECURITY_SERVICE_RID SID that has SE_GROUP_USE_FOR_DENY_ONLY enabled
it will try to access to the event logs but will be denied as all
accesses are denied with this attribute, right?
What do you think about the patch attached then?
--
Michael
Attachments:
20151105_windows_sid_deny.patchapplication/x-patch; name=20151105_windows_sid_deny.patchDownload+7-2
Michael,
I'm pretty sure your patch will fix my issue, but perhaps it should be a
positive check for SE_GROUP_ENABLED? I say "perhaps" because the last time
I did any serious Windows coding was 2005.
Thanks for the quick response!
Breen
On Thu, Nov 5, 2015 at 9:39 AM, Michael Paquier <michael.paquier@gmail.com>
wrote:
Show quoted text
On Wed, Nov 4, 2015 at 3:23 PM, <breen@rtda.com> wrote:
Short version: pgwin32_is_service checks the process token for
SECURITY_SERVICE_RID by doing an EqualSid check. This will matchagainst a
SECURITY_SERVICE_RID that has been disabled ("use_for_deny_only"),
causing
PG to think it's a service when it is not. This causes it to attempt to
log
to the event log, but this doesn't work, and so there is no logging at
all.
OK. So if I am following correctly... If Postgres process uses a
SECURITY_SERVICE_RID SID that has SE_GROUP_USE_FOR_DENY_ONLY enabled
it will try to access to the event logs but will be denied as all
accesses are denied with this attribute, right?What do you think about the patch attached then?
--
Michael
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.
Btw, I think that you would be interested in this patch as well:
/messages/by-id/CAB7nPqR=FsgqOsQL6qUC04XWbZ93Q9BC-qEmHu2Cvh8uMRNrNQ@mail.gmail.com
This makes pgwin32_is_service available for frontend applications as
well, hence you would not need to duplicate any upstream code and just
reuse it for your scripts. That's material for 9.6~ though. I am
actually planning to fix an old bug in pg_ctl handling of a service
using that.
I say "perhaps" because the last time
I did any serious Windows coding was 2005.
That's short considering these day's life average expectancy.
--
Michael
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
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
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
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.
Thanks,
Breen
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?--
MichaelSorry 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
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
Michael Paquier wrote:
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.
Bump.
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
On Tue, Apr 5, 2016 at 1:08 AM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
Michael Paquier wrote:
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.Bump.
Don't worry. This has not fallen from my radar yet..
--
Michael
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
On Tue, Apr 5, 2016 at 12:58 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
On Tue, Apr 5, 2016 at 1:08 AM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
Michael Paquier wrote:
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.Bump.
Don't worry. This has not fallen from my radar yet..
So I have been looking at this issue again and finished with the patch
attached. I think that it makes the most sense to browse the whole
list of groups, and choose if Postgres is running as a service if
service SID matches with one of the group SIDs listed, on top of which
this group SID should be enabled via SE_GROUP_ENABLED. Checking for
SE_GROUP_USE_FOR_DENY_ONLY would not make much sense, because it would
mean that SE_GROUP_ENABLED is not set, and that's what we are
interested in. That was in short the point of Breen, and it looks to
be the saner way to go.
What do others think?
--
Michael
Attachments:
win32-security-service-v2.patchinvalid/octet-stream; name=win32-security-service-v2.patchDownload+2-1
On 04/08/2016 09:48 AM, Michael Paquier wrote:
So I have been looking at this issue again and finished with the patch
attached. I think that it makes the most sense to browse the whole
list of groups, and choose if Postgres is running as a service if
service SID matches with one of the group SIDs listed, on top of which
this group SID should be enabled via SE_GROUP_ENABLED. Checking for
SE_GROUP_USE_FOR_DENY_ONLY would not make much sense, because it would
mean that SE_GROUP_ENABLED is not set, and that's what we are
interested in. That was in short the point of Breen, and it looks to
be the saner way to go.
Yeah, seems like the right way. pgwin32_is_admin() also checks for
SE_GROUP_ENABLED.
I think this is ready to be committed, except that I don't have an easy
way to reproduce the original problem to test this. I suppose I could
write a test program to call CreateRestrictedToken() and
CreateProcessAsUser(), but would rather avoid the work. Breen, if I push
a fix for this, can you build from sources and verify that it fixes your
original problem? Or alternatively, can you provide a test program that
I can use to verify it?
- Heikki
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
Hi,
Sorry for the delay in response. We don't presently build postgres for
Windows (we do for linux and macos), but I'm willing to give it a shot if
there is a solid doc on setting up the build. That would probably be
easier than doing a test program.
Breen
On Wed, Sep 21, 2016 at 7:50 AM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
Show quoted text
On 04/08/2016 09:48 AM, Michael Paquier wrote:
So I have been looking at this issue again and finished with the patch
attached. I think that it makes the most sense to browse the whole
list of groups, and choose if Postgres is running as a service if
service SID matches with one of the group SIDs listed, on top of which
this group SID should be enabled via SE_GROUP_ENABLED. Checking for
SE_GROUP_USE_FOR_DENY_ONLY would not make much sense, because it would
mean that SE_GROUP_ENABLED is not set, and that's what we are
interested in. That was in short the point of Breen, and it looks to
be the saner way to go.Yeah, seems like the right way. pgwin32_is_admin() also checks for
SE_GROUP_ENABLED.I think this is ready to be committed, except that I don't have an easy
way to reproduce the original problem to test this. I suppose I could write
a test program to call CreateRestrictedToken() and CreateProcessAsUser(),
but would rather avoid the work. Breen, if I push a fix for this, can you
build from sources and verify that it fixes your original problem? Or
alternatively, can you provide a test program that I can use to verify it?- Heikki
On Sat, Sep 24, 2016 at 2:55 AM, Breen Hagan <breen@rtda.com> wrote:
Sorry for the delay in response. We don't presently build postgres for
Windows (we do for linux and macos), but I'm willing to give it a shot if
there is a solid doc on setting up the build. That would probably be easier
than doing a test program.
There is a whole chapter in the docs in the matter:
https://www.postgresql.org/docs/9.0/static/install-windows.html
--
Michael
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
On Sat, Sep 24, 2016 at 8:52 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:
On Sat, Sep 24, 2016 at 2:55 AM, Breen Hagan <breen@rtda.com> wrote:
Sorry for the delay in response. We don't presently build postgres for
Windows (we do for linux and macos), but I'm willing to give it a shot if
there is a solid doc on setting up the build. That would probably be easier
than doing a test program.There is a whole chapter in the docs in the matter:
https://www.postgresql.org/docs/9.0/static/install-windows.html
(Moved to next CF, with same status "Ready for committer").
--
Michael
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
Hello,
From: Michael Paquier
(Moved to next CF, with same status "Ready for committer").
I reviewed and tested this patch after simplifying it like the
attached one. The file could be reduced by about 110 lines. Please
review and/or test it. Though I kept the status "ready for
committer", feel free to change it back based on the result.
I tested as follows. First, I confirmed that pg_is_admin() still
works by running postgres.exe from the Administrator command line:
--------------------------------------------------
G:\>postgres
Execution of PostgreSQL by a user with administrative permissions is
not
permitted.
The server must be started under an unprivileged user ID to prevent
possible system security compromises. See the documentation for
more information on how to properly start the server.
G:\>
--------------------------------------------------
Then, I added the following two elog() calls in postmaster.c so that
pg_is_admin() and pg_is_service() works fine.
--------------------------------------------------
maybe_start_bgworker();
elog(LOG, "pgwin32_is_admin = %d", pgwin32_is_admin());
elog(LOG, "pgwin32_is_service = %d", pgwin32_is_service());
status = ServerLoop();
--------------------------------------------------
To reproduce the OP's problem, I modified pg_ctl.c to disable
SECURITY_SERVICE_RID when spawning postgres.exe. Without the patch,
starting the Windows service emit the following log, showing that
pg_is_service() misjudged that postgres is running as a Windows
service:
LOG: pgwin32_is_admin = 0
LOG: pgwin32_is_service = 1
With the patch, the log became correct:
LOG: pgwin32_is_admin = 0
LOG: pgwin32_is_service = 0
Regards
Takayuki Tsunakawa
Attachments:
win32-security_service-v3.patchapplication/octet-stream; name=win32-security_service-v3.patchDownload
Hello,
Sorry, I may have had to send this to pgsql-hackers. I just replied
to all, which did not include pgsql-hackers but pgsql-bugs because
this discussion was on pgsql-bugs. CommitFest app doesn't seem to
reflect the mails on pgsql-bugs, so I'm re-submitting this here on
pgsql-hackers.
From: Michael Paquier
(Moved to next CF, with same status "Ready for committer").
I reviewed and tested this patch after simplifying it like the
attached one. The file could be reduced by about 110 lines. Please
review and/or test it. Though I kept the status "ready for
committer", feel free to change it back based on the result.
I tested as follows. First, I confirmed that pg_is_admin() still
works by running postgres.exe from the Administrator command line:
--------------------------------------------------
G:\>postgres
Execution of PostgreSQL by a user with administrative permissions is
not
permitted.
The server must be started under an unprivileged user ID to prevent
possible system security compromises. See the documentation for
more information on how to properly start the server.
G:\>
--------------------------------------------------
Then, I added the following two elog() calls in postmaster.c so that
pg_is_admin() and pg_is_service() works fine.
--------------------------------------------------
maybe_start_bgworker();
elog(LOG, "pgwin32_is_admin = %d", pgwin32_is_admin());
elog(LOG, "pgwin32_is_service = %d", pgwin32_is_service());
status = ServerLoop();
--------------------------------------------------
To reproduce the OP's problem, I modified pg_ctl.c to disable
SECURITY_SERVICE_RID when spawning postgres.exe. Without the patch,
starting the Windows service emit the following log, showing that
pg_is_service() misjudged that postgres is running as a Windows
service:
LOG: pgwin32_is_admin = 0
LOG: pgwin32_is_service = 1
With the patch, the log became correct:
LOG: pgwin32_is_admin = 0
LOG: pgwin32_is_service = 0
Regards
Takayuki Tsunakawa
Attachments:
win32-security_service-v3.patchapplication/octet-stream; name=win32-security_service-v3.patchDownload
On Sun, Nov 6, 2016 at 6:30 PM, MauMau <maumau307@gmail.com> wrote:
Sorry, I may have had to send this to pgsql-hackers. I just replied
to all, which did not include pgsql-hackers but pgsql-bugs because
this discussion was on pgsql-bugs. CommitFest app doesn't seem to
reflect the mails on pgsql-bugs, so I'm re-submitting this here on
pgsql-hackers.
No problem, I still see a unique thread so that's not an issue seen from here.
I reviewed and tested this patch after simplifying it like the
attached one. The file could be reduced by about 110 lines. Please
review and/or test it. Though I kept the status "ready for
committer", feel free to change it back based on the result.
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.
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?
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.
--
Michael
Attachments:
win32-disable-service-rid.patchtext/x-patch; charset=US-ASCII; name=win32-disable-service-rid.patchDownload+6-2
From: pgsql-hackers-owner@postgresql.org
[mailto:pgsql-hackers-owner@postgresql.org] On Behalf Of Michael Paquier
On Sun, Nov 6, 2016 at 6:30 PM, MauMau <maumau307@gmail.com> wrote:Sorry, I may have had to send this to pgsql-hackers. I just replied
to all, which did not include pgsql-hackers but pgsql-bugs because
this discussion was on pgsql-bugs. CommitFest app doesn't seem to
reflect the mails on pgsql-bugs, so I'm re-submitting this here on
pgsql-hackers.No problem, I still see a unique thread so that's not an issue seen from
here.
You are right. A while after I sent the second mail, I noticed the CommitFest app collected both of my mails. I was just impatient.
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.
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.
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.
Regards
Takayuki Tsunakawa
Attachments:
win32-security-service-v4.patchapplication/octet-stream; name=win32-security-service-v4.patchDownload+35-142
On Mon, Nov 7, 2016 at 9:49 AM, Tsunakawa, Takayuki
<tsunakawa.takay@jp.fujitsu.com> wrote:
From: pgsql-hackers-owner@postgresql.org
[mailto:pgsql-hackers-owner@postgresql.org] On Behalf Of Michael Paquier
On Sun, Nov 6, 2016 at 6:30 PM, MauMau <maumau307@gmail.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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
From: Michael Paquier
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.
Yes, I tested both your patch and mine. I used the attached pg_ctl.c.
It adds -z option which disables SECURITY_SERVICE_RID.
I registered the service with "pg_ctl register -N pg -D
datadir -w -z -S demand -U myuser -P mypass", then started the service
with "net start pg". The following messages were output in the server
log:
LOG: pgwin32_is_admin = 0
LOG: pgwin32_is_service = 0
LOG: database system was shut down at 2016-11-07 22:04:46 JST
LOG: MultiXact member wraparound protections are now enabled
LOG: database system is ready to accept connections
LOG: autovacuum launcher started
Without -z, the message becomes "pgwin32_is_service = 1". And without
the win32security.c patch, "pgwin32_is_service = 1" is output.
I guess you registered the service without specifying the service
account with -U. Then the service runs as the Local System account,
whence pgwin32_is_service() returns 1.
Regards
Takayuki Tsunakawa
