Re: Alvaro Herrera 2016-03-04 <20160304205521.GA735336@alvherre.pgsql>
> For the sake of cleanliness, I propose splitting out the checks for
> regular file and for owned-by-root-or-us from the actual chmod-level
> checks at the same time.  That way we can provide more specific error
> messages for each case.  (Furthermore, I'm pretty sure that the check
> for superuserness could be applied on Windows also -- in the attached
> patch it's still #ifdef'ed out, because I don't know how to write it.)

Thanks, this is a reasonable improvement. I've included your patch in
the 9.6 Debian package and also implemented some regression tests in
postgresql-common to make sure the bad permissions are indeed catched.

> After doing that change I started to look at the details of the check
> and found some mistakes:
> * it said "g=w" instead of "g=r", in contradiction with the numeric
> specification: g=w means 020 rather than 040.  We want the file to be
> group-readable, not group-writeable.

Typo on my side, sorry.

> * it failed to check for S_IXUSR, so permissions 0700 were okay, in
> contradiction with what the error message indicates.  This is a
> preexisting bug actually.  Do we want to fix it by preventing a
> user-executable file (possibly breaking compability with existing
> executable key files), or do we want to document what the restriction
> really is?

x on regular files doesn't do anything, so it wouldn't matter, but of
course syncing the code with the error message makes sense. I don't
think 0700 is a case that is seen in the wild (in contrast to 0777
files), and even if so, the error message clearly says what the
problem is.

-- |

Sent via pgsql-hackers mailing list (
To make changes to your subscription:

Reply via email to