Re: [HACKERS] [bug fix] pg_ctl fails with config-only directory

2014-04-04 Thread Tom Lane
MauMau maumau...@gmail.com writes:
 From: Heikki Linnakangas hlinnakan...@vmware.com
 Hmm, why do this only on Windows? If postgres -C is safe enough to run 
 as Administrator on Windows, why not allow running it as root on Unix as 
 well? Even if there's no particular need to allow it as root on Unix, 
 fewer #ifdefs is good.

 Yes, I limited the change only to Windows, because I thought I might get get 
 objection against unnecessary change.  Plus, --single should not be allowed 
 for root, because root cannot be the PostgreSQL user account.

Indeed, and why would that not apply to Windows as well?  AFAIK, inclusion
of --single in this list is just plain nuts.  The most likely result of
running that way is creation of root-owned files inside $PGDATA, which
would cause havoc for later normally-privileged postmasters.

I will go and commit this, without the #ifdefs and without the --single
exclusion.

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


Re: [HACKERS] [bug fix] pg_ctl fails with config-only directory

2014-04-04 Thread Tom Lane
I wrote:
 I will go and commit this, without the #ifdefs and without the --single
 exclusion.

On closer inspection I realized that the switch parsing was still far too
risky, because it would treat -C in any word of the process command line
as a reason not to check for root.  Quite aside from the fact that some of
those words might be switch arguments not switches, main.c is also the
front end for other operating modes that have switches unrelated to the
postmaster's switches.  --boot mode doesn't have any -C switch today, but
it might do so tomorrow, and that would result in a hard-to-notice hole in
our root protections.

However, there is a reasonably simple way around that objection, which is
to only skip the root check if -C is the first switch.  pg_ctl can easily
be changed to call it that way, and we're not really here to make -C easy
for root users to call manually, so I'm not too concerned about that
aspect of it.  --describe-config is only accepted as the first switch
anyway, so there's no issue there either.

Committed with appropriate changes.

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


Re: [HACKERS] [bug fix] pg_ctl fails with config-only directory

2014-02-20 Thread Heikki Linnakangas

On 02/01/2014 12:28 PM, Christian Kruse wrote:

On 31/01/14 22:17, MauMau wrote:

Thanks for reviewing the patch.  Fixed.  I'll add this revised patch to the
CommitFest entry soon.


Looks fine for me. Set it to „waiting for commit.“


Hmm, why do this only on Windows? If postgres -C is safe enough to run 
as Administrator on Windows, why not allow running it as root on Unix as 
well? Even if there's no particular need to allow it as root on Unix, 
fewer #ifdefs is good.


- Heikki


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


Re: [HACKERS] [bug fix] pg_ctl fails with config-only directory

2014-02-20 Thread MauMau

From: Heikki Linnakangas hlinnakan...@vmware.com
Hmm, why do this only on Windows? If postgres -C is safe enough to run 
as Administrator on Windows, why not allow running it as root on Unix as 
well? Even if there's no particular need to allow it as root on Unix, 
fewer #ifdefs is good.


Yes, I limited the change only to Windows, because I thought I might get get 
objection against unnecessary change.  Plus, --single should not be allowed 
for root, because root cannot be the PostgreSQL user account.


Regards
MauMau



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


Re: [HACKERS] [bug fix] pg_ctl fails with config-only directory

2014-02-01 Thread Christian Kruse
Hi,

On 31/01/14 22:17, MauMau wrote:
 Thanks for reviewing the patch.  Fixed.  I'll add this revised patch to the
 CommitFest entry soon.

Looks fine for me. Set it to „waiting for commit.“

Best regards,

-- 
 Christian Kruse   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services



pgpLWrb04lc6J.pgp
Description: PGP signature


Re: [HACKERS] [bug fix] pg_ctl fails with config-only directory

2014-01-31 Thread MauMau

From: Christian Kruse christ...@2ndquadrant.com

personally I really dislike constructs like you used:


Thanks for reviewing the patch.  Fixed.  I'll add this revised patch to the 
CommitFest entry soon.


Regards
MauMau


config_dir_win_v2.patch
Description: Binary data

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


Re: [HACKERS] [bug fix] pg_ctl fails with config-only directory

2014-01-30 Thread Christian Kruse
Hi,

personally I really dislike constructs like you used:

#ifdef WIN32
if (check_if_admin)
#endif
check_root(progname);

It is hard to read and may confuse editors. Can you rewrite it?

The rest looks fine to me.

Best regards,

-- 
 Christian Kruse   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services



pgp56dqGkFa4q.pgp
Description: PGP signature


Re: [HACKERS] [bug fix] pg_ctl fails with config-only directory

2013-12-07 Thread MauMau

From: Amit Kapila amit.kapil...@gmail.com

Today, I had again gone through all the discussion that happened at
that time related to this problem
and I found that later in discussion it was discussed something on
lines as your Approach-2,
please see the link
http://www.postgresql.org/message-id/503a879c.6070...@dunslane.net


Thank you again.  I'm a bit relieved to see similar discussion.



Wouldn't the other way to resolve this problem be reinvoke pg_ctl in
non-restricted mode for the case in question?


It's effectively the same as not checking admin privileges.  And direct 
invocation of postgres -C/--describe-config still cannot be helped.

I think it is important to resolve this problem, so please godhead and
upload this patch to next CF.


Thanks.

Regards
MauMau



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


Re: [HACKERS] [bug fix] pg_ctl fails with config-only directory

2013-12-07 Thread Amit Kapila
On Sat, Dec 7, 2013 at 6:02 PM, MauMau maumau...@gmail.com wrote:
 From: Amit Kapila amit.kapil...@gmail.com

 Today, I had again gone through all the discussion that happened at
 that time related to this problem
 and I found that later in discussion it was discussed something on
 lines as your Approach-2,
 please see the link
 http://www.postgresql.org/message-id/503a879c.6070...@dunslane.net


 Thank you again.  I'm a bit relieved to see similar discussion.



 Wouldn't the other way to resolve this problem be reinvoke pg_ctl in
 non-restricted mode for the case in question?


 It's effectively the same as not checking admin privileges.  And direct
 invocation of postgres -C/--describe-config still cannot be helped.

   Yeah, that is the only point of reinvoke pg_ctl in non-restricted
mode, that it should be only allowed through
   pg_ctl, but not directly with postgres -C.
   In anycase, I think now we have link for the discussion and patch
as well for other Approach, so if the consensus
   is to modify postgres code such that we don't need to check for
admin privs for -C/--describe-config option, then the
   patch proposed by you can be taken else the other patch can be used.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


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


Re: [HACKERS] [bug fix] pg_ctl fails with config-only directory

2013-12-06 Thread Amit Kapila
On Thu, Dec 5, 2013 at 6:30 PM, MauMau maumau...@gmail.com wrote:
 From: Amit Kapila amit.kapil...@gmail.com

 On Wed, Dec 4, 2013 at 7:57 PM, MauMau maumau...@gmail.com wrote:


 Approach-2 has been discussed previously to resolve it and it doesn't seem
 to be
 a good way to handle it. Please refer link:
 http://www.postgresql.org/message-id/1339601668-sup-4...@alvh.no-ip.org

 You can go through that mail chain and see if there can be a better
 solution than Approach-2.


 Thanks for the info.  I understand your feeling, but we need to be
 practical.  I believe we should not leave a bug and inconvenience by
 worrying about theory too much.  In addition to the config-only directory,
 the DBA with admin privs will naturally want to run postgres -C and
 postgres --describe-config, because they are useful and so described in
 the manual.  I don't see any (at least big) risk in allowing postgres
 -C/--describe-config to run with admin privs.

Today, I had again gone through all the discussion that happened at
that time related to this problem
and I found that later in discussion it was discussed something on
lines as your Approach-2,
please see the link
http://www.postgresql.org/message-id/503a879c.6070...@dunslane.net

 In addition, recent Windows
 versions help to secure the system by revoking admin privs with UAC, don't
 they?  Disabling UAC is not recommended.

 I couldn't find a way to let postgres delete its token groups from its own
 primary access token.  There doesn't seem to be a reasonably clean and good
 way.

Wouldn't the other way to resolve this problem be reinvoke pg_ctl in
non-restricted mode for the case in question?

 So I had to choose approach 2.  Please find attached the patch.  This simple
 and not-complex change worked well.  I'd like to add this to 2014-1
 commitfest this weekend unless a better approach is proposed.

I think it is important to resolve this problem, so please godhead and
upload this patch to next CF.


With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


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


Re: [HACKERS] [bug fix] pg_ctl fails with config-only directory

2013-12-05 Thread MauMau

From: Amit Kapila amit.kapil...@gmail.com

On Wed, Dec 4, 2013 at 7:57 PM, MauMau maumau...@gmail.com wrote:

* Approach 1
When postgres starts, it removes Administrator privileges from its own
process.  But is this possible at all?  Windows security API is complex 
and
provides many functions.  It seems difficult to understand them.  I'm 
afraid
it would take a long time to figure out the solution.  Is there any good 
web

page to look at?

* Approach 2
Do not call check_root() on Windows when -C, --describe-config, 
or --single
is specified when running postgres.  This would be easy, and should not 
be
dangerous in terms of security because attackers cannot get into the 
server

process via network.


Approach-2 has been discussed previously to resolve it and it doesn't seem 
to be

a good way to handle it. Please refer link:
http://www.postgresql.org/message-id/1339601668-sup-4...@alvh.no-ip.org

You can go through that mail chain and see if there can be a better
solution than Approach-2.


Thanks for the info.  I understand your feeling, but we need to be 
practical.  I believe we should not leave a bug and inconvenience by 
worrying about theory too much.  In addition to the config-only directory, 
the DBA with admin privs will naturally want to run postgres -C and 
postgres --describe-config, because they are useful and so described in 
the manual.  I don't see any (at least big) risk in allowing 
postgres -C/--describe-config to run with admin privs.  In addition, recent 
Windows versions help to secure the system by revoking admin privs with UAC, 
don't they?  Disabling UAC is not recommended.


I couldn't find a way to let postgres delete its token groups from its own 
primary access token.  There doesn't seem to be a reasonably clean and good 
way.


So I had to choose approach 2.  Please find attached the patch.  This simple 
and not-complex change worked well.  I'd like to add this to 2014-1 
commitfest this weekend unless a better approach is proposed.


Regards
MauMau



config_dir_win.patch
Description: Binary data

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


[HACKERS] [bug fix] pg_ctl fails with config-only directory

2013-12-04 Thread MauMau

Hello,

I've found a bug and would like to fix it, but I cannot figure out how to do 
that well.  Could you give me any advice?  I encountered this on PG 9.2, but 
it will probably exist in later versions.


[Problem]
On Windows, a user with Administrator privileges can start the database 
server.  However, when he uses config-only directory, the database server 
cannot be started.  pg_ctl start fails with the following messages:


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.


[Cause]
pg_ctl runs postgres -C data_directory to know the data directory.  But 
postgres cannot be run by a user with Administrator privileges, and displays 
the above messages.



[Fix]
It is ideal that users with administrative privileges can start postgres, 
with the Administrator privileges removed.


Currently, initdb and pg_ctl take trouble to invoke postgres in a process 
with restricted privileges.  I understand this improvement was done in 8.2 
or 8.3 for convenience.  The same convenience should be available when 
running postgres directly, at least postgres -C, 
postgres --describe-config, and postgres --single.


Then, how can we do this?  Which approach should we take?

* Approach 1
When postgres starts, it removes Administrator privileges from its own 
process.  But is this possible at all?  Windows security API is complex and 
provides many functions.  It seems difficult to understand them.  I'm afraid 
it would take a long time to figure out the solution.  Is there any good web 
page to look at?


* Approach 2
Do not call check_root() on Windows when -C, --describe-config, or --single 
is specified when running postgres.  This would be easy, and should not be 
dangerous in terms of security because attackers cannot get into the server 
process via network.


I'll try to find a solution based on approach 1, but I doubt there's one. 
If okay, I want to take approach 2.  Could you give me your thoughts?


Regards
MauMau



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


Re: [HACKERS] [bug fix] pg_ctl fails with config-only directory

2013-12-04 Thread Amit Kapila
On Wed, Dec 4, 2013 at 7:57 PM, MauMau maumau...@gmail.com wrote:
 Hello,

 I've found a bug and would like to fix it, but I cannot figure out how to do
 that well.  Could you give me any advice?  I encountered this on PG 9.2, but
 it will probably exist in later versions.

 [Problem]
 On Windows, a user with Administrator privileges can start the database
 server.  However, when he uses config-only directory, the database server
 cannot be started.  pg_ctl start fails with the following messages:

 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.


 [Cause]
 pg_ctl runs postgres -C data_directory to know the data directory.  But
 postgres cannot be run by a user with Administrator privileges, and displays
 the above messages.


 [Fix]
 It is ideal that users with administrative privileges can start postgres,
 with the Administrator privileges removed.

 Currently, initdb and pg_ctl take trouble to invoke postgres in a process
 with restricted privileges.  I understand this improvement was done in 8.2
 or 8.3 for convenience.  The same convenience should be available when
 running postgres directly, at least postgres -C, postgres
 --describe-config, and postgres --single.

 Then, how can we do this?  Which approach should we take?

 * Approach 1
 When postgres starts, it removes Administrator privileges from its own
 process.  But is this possible at all?  Windows security API is complex and
 provides many functions.  It seems difficult to understand them.  I'm afraid
 it would take a long time to figure out the solution.  Is there any good web
 page to look at?

 * Approach 2
 Do not call check_root() on Windows when -C, --describe-config, or --single
 is specified when running postgres.  This would be easy, and should not be
 dangerous in terms of security because attackers cannot get into the server
 process via network.

Approach-2 has been discussed previously to resolve it and it doesn't seem to be
a good way to handle it. Please refer link:
http://www.postgresql.org/message-id/1339601668-sup-4...@alvh.no-ip.org

You can go through that mail chain and see if there can be a better
solution than Approach-2.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


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