Re: Unable to connect to PostgreSQL DB as root user when private key is owned by root with permission 640

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: "Suralkar, Yogendra (Yogendra)" <suralkary(at)avaya(dot)com>
Cc: "pgsql-bugs(at)lists(dot)postgresql(dot)org" <pgsql-bugs(at)lists(dot)postgresql(dot)org>, David Steele <david(at)pgmasters(dot)net>, "Porob, Dattaram (Datta)" <porobd(at)avaya(dot)com>, "Oswal, Prashant (Prashant) **CTR**" <poswal(at)avaya(dot)com>, "Patil, Parag (Parag)" <paragp(at)avaya(dot)com>, "Devaraj, Sankar (Sankar)" <devarajs(at)avaya(dot)com>, "Singh, Payal (Payal) **CTR**" <payals(at)avaya(dot)com>
Subject: Re: Unable to connect to PostgreSQL DB as root user when private key is owned by root with permission 640
Date: 2022-05-24 20:05:32
Message-ID: 1719047.1653422732@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

I wrote:
> TBH, my immediate reaction is "what are you doing running database
> accesses as root?". But given that you are, I see the problem: the test
> is coded like
> if ((buf.st_uid == geteuid() && buf.st_mode & (S_IRWXG | S_IRWXO)) ||
> (buf.st_uid == 0 && buf.st_mode & (S_IWGRP | S_IXGRP | S_IRWXO)))
> which was copied verbatim from the equivalent test in the backend.
> However, in the backend it's safe to assume that geteuid() != 0.
> libpq apparently shouldn't assume that, meaning that the two arms
> of the if aren't disjoint cases anymore, and it matters which one
> we check first.

After further poking at this, I see that we also have to drop the check of
file ownership. That was already dropped once long ago (3405f2b9253), on
the grounds that if the file has suitable permissions but its ownership
isn't what we expect then our read attempt will fail, so we needn't check
ownership explicitly. While I'd prefer a more explicit error than the
"Permission denied" that you get with this approach, the intent of this
patch was not to create any new failure modes, so I think we're stuck
with that.

Open questions:

* This puts us back into a situation where the frontend and server tests
are not in sync. Do we want to relax the server's checks to match this,
or just leave that side as it stands?

* I notice that while the code comments and error messages insist that
we're checking for 0600/0640 or less, the actual test is not that:
it doesn't look at S_IXUSR, so it will allow 0700 or 0740. Do we want
to do anything about that? TBH I'm content to leave it as-is.
Rejecting S_IXUSR doesn't seem like it buys any useful increment of
security, and I'm now afraid that somebody will complain that it
breaks their case that used to work. OTOH, updating the error messages
and documentation to match the code reality is not terribly attractive
either; it'll cause a lot of translation churn and probably add more
confusion than it removes.

regards, tom lane

Attachment Content-Type Size
undo-ssl-key-file-permissions-breakage.patch text/x-diff 3.5 KB

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Nathan Bossart 2022-05-24 20:11:45 Re: BUG #17496: to_char function resets if interval exceeds 23 hours 59 minutes
Previous Message Jeff Janes 2022-05-24 19:55:49 Re: BUG #17496: to_char function resets if interval exceeds 23 hours 59 minutes