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.
Christoph
--
cb@df7cb.de | http://www.df7cb.de/