Thread: [bug fix] pg_ctl fails with config-only directory
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
On Wed, Dec 4, 2013 at 7:57 PM, MauMau <maumau307@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-4658@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
From: "Amit Kapila" <amit.kapila16@gmail.com> > On Wed, Dec 4, 2013 at 7:57 PM, MauMau <maumau307@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-4658@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
Attachment
On Thu, Dec 5, 2013 at 6:30 PM, MauMau <maumau307@gmail.com> wrote: > From: "Amit Kapila" <amit.kapila16@gmail.com> >> >> On Wed, Dec 4, 2013 at 7:57 PM, MauMau <maumau307@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-4658@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.6070703@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
From: "Amit Kapila" <amit.kapila16@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.6070703@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
On Sat, Dec 7, 2013 at 6:02 PM, MauMau <maumau307@gmail.com> wrote: > From: "Amit Kapila" <amit.kapila16@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.6070703@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 havelink 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
Hi, personally I really dislike constructs like you used: #ifdef WIN32if (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
From: "Christian Kruse" <christian@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
Attachment
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
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
From: "Heikki Linnakangas" <hlinnakangas@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
"MauMau" <maumau307@gmail.com> writes:
> From: "Heikki Linnakangas" <hlinnakangas@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
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