Обсуждение: Relaxing SSL key permission checks

Hi,

Currently the server insists on ssl_key_file's permissions to be 0600
or less, and be owned by the database user. Debian has been patching
be-secure.c since forever (the git history goes back to 8.2beta1) to
relax that to 0640 or less, and owned by root or the database user.

The reason for that is that we hooked the SSL certificate handling
into the system's /etc/ssl/ directory structure where private keys are
supposed to have permissions 0640 root:ssl-cert. The postgres user is
member of the ssl-cert group so it can read the key.

In the old days before 9.2 the server expected the SSL files in
PGDATA, and we created symlinks from there to /etc/ssl/. Since 9.2,
these certs are used in the ssl_*_file options.

Using symlinks in PGDATA to use system-wide certificates might have
been a hack, but with the "new" ssl_*_file options I think it might be
prudent to get the "allow group ssl-cert" patch upstreamed.

Comments? (There's no documentation yet, I'll add that if the feedback
is positive.)

Thanks,
Christoph

Christoph Berg <myon@debian.org> writes:
> Currently the server insists on ssl_key_file's permissions to be 0600
> or less, and be owned by the database user. Debian has been patching
> be-secure.c since forever (the git history goes back to 8.2beta1) to
> relax that to 0640 or less, and owned by root or the database user.

Debian can do that if they like, but it's entirely unacceptable as an
across-the-board patch.  Not all systems treat groups as being narrow
domains in which it's okay to assume that group-readable files are
secure enough to be keys.  As an example, on OS X user files tend to be
group "staff" or "admin" which'd be close enough to world readable.

We could allow group-readable if we had some way to know whether to
trust the specific group, but I don't think there's any practical
way to do that.  System conventions vary too much.
        regards, tom lane

On Thu, Feb 18, 2016 at 10:17:49AM -0500, Tom Lane wrote:
> Christoph Berg <myon@debian.org> writes:
> > Currently the server insists on ssl_key_file's permissions to be 0600
> > or less, and be owned by the database user. Debian has been patching
> > be-secure.c since forever (the git history goes back to 8.2beta1) to
> > relax that to 0640 or less, and owned by root or the database user.
> 
> Debian can do that if they like, but it's entirely unacceptable as an
> across-the-board patch.  Not all systems treat groups as being narrow
> domains in which it's okay to assume that group-readable files are
> secure enough to be keys.  As an example, on OS X user files tend to be
> group "staff" or "admin" which'd be close enough to world readable.
> 
> We could allow group-readable if we had some way to know whether to
> trust the specific group, but I don't think there's any practical
> way to do that.  System conventions vary too much.

Should we have a GUC to control the group permissions restriction?  I
can certainly see value in allowing for group access to the certificate.

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com

+ As you are, so once was I. As I am, so you will be. +
+ Roman grave inscription                             +

Bruce Momjian <bruce@momjian.us> writes:
> On Thu, Feb 18, 2016 at 10:17:49AM -0500, Tom Lane wrote:
>> We could allow group-readable if we had some way to know whether to
>> trust the specific group, but I don't think there's any practical
>> way to do that.  System conventions vary too much.

> Should we have a GUC to control the group permissions restriction?  I
> can certainly see value in allowing for group access to the certificate.

Meh ... I think such a GUC would mostly be a way to shoot yourself in
the foot.  (For example, imagine an OS X user who sets it to "staff"
instead of doing the right thing and adjusting the file's permissions.)

I did have a thought though: could we allow two distinct permissions
configurations?  That is, allow either:

* file is owned by us, mode 0600 or less

* file is owned by root, mode 0640 or less

The first case is what we allow today.  (We don't need an explicit
ownership check; if the mode is 0600 and we can read it, we must be
the owner.)  The second case is what Debian wants.  We already know
we are not root, so if we can read the file, we must be part of the
group that root has allowed to read the file, and at that point it's
on root's head whether or not that group is secure.  I don't have a
problem with trusting root's judgment on security matters --- if the
root admin is incompetent, there are probably holes everywhere anyway.

The problem with the proposed patch is that it's conflating these
distinct cases, but that's easily fixed.
        regards, tom lane

On 2016-02-18 10:17:49 -0500, Tom Lane wrote:
> Christoph Berg <myon@debian.org> writes:
> > Currently the server insists on ssl_key_file's permissions to be 0600
> > or less, and be owned by the database user. Debian has been patching
> > be-secure.c since forever (the git history goes back to 8.2beta1) to
> > relax that to 0640 or less, and owned by root or the database user.
> 
> Debian can do that if they like, but it's entirely unacceptable as an
> across-the-board patch.  Not all systems treat groups as being narrow
> domains in which it's okay to assume that group-readable files are
> secure enough to be keys.  As an example, on OS X user files tend to be
> group "staff" or "admin" which'd be close enough to world readable.
> 
> We could allow group-readable if we had some way to know whether to
> trust the specific group, but I don't think there's any practical
> way to do that.  System conventions vary too much.

Isn't that a bit overly restrictive? Asking users to patch out checks,
for perfectly reasonable configurations, strikes me as a bit unbalanced.
There's never reasons to make the file world read/writable; but there
seem to be plenty of scenarios where the file should be read/writable by
specific groups.  We don't prevent the user from making the
configuration file world-writable either, so there's not really that
much of a security benefit of being overly restrictive with other
parameters - you can just ocnfigure an archive command of your choosing
and such.

Andres

Andres Freund <andres@anarazel.de> writes:
> ... We don't prevent the user from making the
> configuration file world-writable either,

Maybe we should.  It wasn't an issue originally, because the config files
were necessarily inside $PGDATA which we restrict permissions on.  But
these days you can place the config files in places where untrustworthy
people could get at them.
        regards, tom lane

* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> Andres Freund <andres@anarazel.de> writes:
> > ... We don't prevent the user from making the
> > configuration file world-writable either,
>
> Maybe we should.  It wasn't an issue originally, because the config files
> were necessarily inside $PGDATA which we restrict permissions on.  But
> these days you can place the config files in places where untrustworthy
> people could get at them.

No, we should be improving our support of systems which provide more
specific groups, not destroying it.  Being able to run backups as a user
who is not able to modify the database would be great too, and that case
isn't covered by your approach to "allow group rights if the file is
owned by root."

Further, the notion that *this* is the footgun is completely off the
reservation- if the files have been changed to allow untrusted users to
have access to them, there isn't diddly we can do about it.  All we're
doing with this is imposing our own idea of what the system policy
should be, even though there are clear examples where that's just
blatently wrong.

If we really want to force these checks to happen (and I'm not
convinced that they're actually useful at all), then we need to provide
a way for users and distributions to control the specifics of the checks
as they chose.  Maybe that's a command-line switch instead of a GUC, or
it's something else, but there clearly isn't "one true way" here and we
should be flexible.

Thanks!

Stephen

Stephen Frost <sfrost@snowman.net> writes:
> Further, the notion that *this* is the footgun is completely off the
> reservation- if the files have been changed to allow untrusted users to
> have access to them, there isn't diddly we can do about it.

I completely disagree that those file-permissions checks are useless.
What they accomplish is, if you accidentally set up an insecure key file,
you'll get told about it fairly promptly, and have the opportunity to
either fix the permissions or generate a new key, depending on your
opinion of how likely it is that somebody stole the key already.  If we
made no checks then, more than likely, an insecure key file would just
sit there indefinitely, waiting for some passing blackhat to grab it.

We can certainly discuss whether we need more than one model of what
appropriate permissions are, but I do not believe that "rip out the
check" is a helpful answer.
        regards, tom lane

Tom,

* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> Stephen Frost <sfrost@snowman.net> writes:
> > Further, the notion that *this* is the footgun is completely off the
> > reservation- if the files have been changed to allow untrusted users to
> > have access to them, there isn't diddly we can do about it.
>
> I completely disagree that those file-permissions checks are useless.
> What they accomplish is, if you accidentally set up an insecure key file,
> you'll get told about it fairly promptly, and have the opportunity to
> either fix the permissions or generate a new key, depending on your
> opinion of how likely it is that somebody stole the key already.  If we
> made no checks then, more than likely, an insecure key file would just
> sit there indefinitely, waiting for some passing blackhat to grab it.

If that's something you're concerned with then the right answer is to
monitor the file permissions- and there are tools which do exactly that.

I disagree that it's PG's charter to do that and, frankly, you *won't*
be told, in most cases, promptly about such a change.  We don't check
the permissions on every file or even every directory on every query or
even every connection.  With uptimes of days, weeks and months quite
often seen, this idea that PG is protecting you with these checks in
any way is instead creating a false sense of security.

> We can certainly discuss whether we need more than one model of what
> appropriate permissions are, but I do not believe that "rip out the
> check" is a helpful answer.

We're over-claiming what these protections are actually providing.  At
best, they help a new user out when they're first setting up their
database, if they're doing it from source and fully understand how to
correctly set up pg_hba.conf and friends to secure *those*.  Users
installing packages from distributions will have the benefit that the
distribution will get the permissions correct initially and not have to
worry about them.  Permission changes to these files after the system is
set up and running indicate that it's already too late because PG won't
tell the user there's an issue until a DB restart a month or two later,
and by then, if there's an untrusted user who was able to gain access
thanks to such changes, you can be sure they will have.

Thanks!

Stephen

On 2/18/16 10:17 AM, Tom Lane wrote:
> Christoph Berg <myon@debian.org> writes:
>> Currently the server insists on ssl_key_file's permissions to be 0600
>> or less, and be owned by the database user. Debian has been patching
>> be-secure.c since forever (the git history goes back to 8.2beta1) to
>> relax that to 0640 or less, and owned by root or the database user.
> 
> Debian can do that if they like, but it's entirely unacceptable as an
> across-the-board patch.  Not all systems treat groups as being narrow
> domains in which it's okay to assume that group-readable files are
> secure enough to be keys.  As an example, on OS X user files tend to be
> group "staff" or "admin" which'd be close enough to world readable.
> 
> We could allow group-readable if we had some way to know whether to
> trust the specific group, but I don't think there's any practical
> way to do that.  System conventions vary too much.

Wouldn't POSIX ACLs bypass this anyway?

Stephen Frost <sfrost@snowman.net> writes:
> * Tom Lane (tgl@sss.pgh.pa.us) wrote:
>> I completely disagree that those file-permissions checks are useless.

> If that's something you're concerned with then the right answer is to
> monitor the file permissions- and there are tools which do exactly that.
> I disagree that it's PG's charter to do that and, frankly, you *won't*
> be told, in most cases, promptly about such a change.

I will just quote this bit from "man ssh":
    ~/.ssh/identity    ~/.ssh/id_dsa    ~/.ssh/id_ecdsa    ~/.ssh/id_rsa            Contains the private key for
authentication. These files contain            sensitive data and should be readable by the user but not acces-
  sible by others (read/write/execute).  ssh will simply ignore a            private key file if it is accessible by
others.

Now, I have heard it argued that the OpenSSH/L authors are a bunch of
idiots who know nothing about security.  But it's not like insisting
on restrictive permissions on key files is something we invented out
of the blue.  It's pretty standard practice, AFAICT.
        regards, tom lane

On 02/18/2016 08:22 PM, Tom Lane wrote:

> Now, I have heard it argued that the OpenSSH/L authors are a bunch of
> idiots who know nothing about security.  But it's not like insisting
> on restrictive permissions on key files is something we invented out
> of the blue.  It's pretty standard practice, AFAICT.
>
>             regards, tom lane

I think Tom has the right compromise. It must be 0600 for us, and 0640 
or less for root. That opens up the ability for other systems to have 
what it needs (although I am unsure of how Windows handles this) and 
allows us to keep a modicum of self respect in terms of what we allow.

Sincerely,

JD


-- 
Command Prompt, Inc.                  http://the.postgres.company/                        +1-503-667-4564
PostgreSQL Centered full stack support, consulting and development.
Everyone appreciates your honesty, until you are honest with them.

Re: Tom Lane 2016-02-18 <27423.1455809676@sss.pgh.pa.us>
> I did have a thought though: could we allow two distinct permissions
> configurations?  That is, allow either:
>
> * file is owned by us, mode 0600 or less
>
> * file is owned by root, mode 0640 or less
>
> The first case is what we allow today.  (We don't need an explicit
> ownership check; if the mode is 0600 and we can read it, we must be
> the owner.)  The second case is what Debian wants.  We already know
> we are not root, so if we can read the file, we must be part of the
> group that root has allowed to read the file, and at that point it's
> on root's head whether or not that group is secure.  I don't have a
> problem with trusting root's judgment on security matters --- if the
> root admin is incompetent, there are probably holes everywhere anyway.

Makes sense to me.

> The problem with the proposed patch is that it's conflating these
> distinct cases, but that's easily fixed.

Updated patch attached.

Christoph
--
cb@df7cb.de | http://www.df7cb.de/

Tom,

* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> Now, I have heard it argued that the OpenSSH/L authors are a bunch of
> idiots who know nothing about security.  But it's not like insisting
> on restrictive permissions on key files is something we invented out
> of the blue.  It's pretty standard practice, AFAICT.

I certainly was not intending to imply that anyone was an 'idiot'.

Nor am I arguing that we must remove all such checks from every part of
the system.

I do contend that relying on such checks that happen on a relativly
infrequent basis in daemon processes creates a false sense of security
and does not practically improve security, today.  As I mentioned
previously, perhaps before distributions were as cognizant about
security concerns or about considering how a particular daemon should
be set up, or when users were still frequently installing from source,
such checks were more valuable.  However, when they get in the way of
entirely reasonable system policies and require distributions to patch
the source code, they're a problem.  That doesn't mean we necessairly
have to remove them, but we should be flexible.

Similar checks in client utilities, such as the ssh example, or in psql,
are more useful and, from a practical standpoint, havn't been an issue
for system policies.

Further, we do more than check key files but also check permissions on
the data directory and don't provide any way for users to configure the
permissions on new files, which could be seen as akin to sshd requiring
user home directories to be 700 and forcing umask to 077.

Note that all of this only actually applies to OpenSSH, not to OpenSSL.
Certainly, as evidenced by the question which sparked this discussion,
the packages which are configured to use the local snakeoil cert on
Debian-based systems (which also includes postfix and Apache, offhand)
do not have a problem with the group read permissions that are being
asked for.  I don't find that to be offensive or unacceptable in the
least, nor do I feel that Debian is flawed for taking this approach, or
that OpenSSL is flawed for not having such a check.

Thanks!

Stephen

Re: To Tom Lane 2016-02-19 <20160219115334.GB26862@msg.df7cb.de>
> Updated patch attached.

*Blush* I though I had compile-tested the patch, but without
--enable-openssl it wasn't built :(.

The attached patch has successfully been included in the 9.6 Debian
package, passed the regression tests there, and I've also done some
chmod/chown tests on the filesystem to verify it indeed catches the
cases laid out by Tom.

Christoph
--
cb@df7cb.de | http://www.df7cb.de/

Christoph Berg <myon@debian.org> writes:
> The attached patch has successfully been included in the 9.6 Debian
> package, passed the regression tests there, and I've also done some
> chmod/chown tests on the filesystem to verify it indeed catches the
> cases laid out by Tom.

Please add this to the upcoming commitfest.  We don't seem to have
enough consensus to just apply it, but perhaps the review process
will produce some agreement.
        regards, tom lane

* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> Christoph Berg <myon@debian.org> writes:
> > The attached patch has successfully been included in the 9.6 Debian
> > package, passed the regression tests there, and I've also done some
> > chmod/chown tests on the filesystem to verify it indeed catches the
> > cases laid out by Tom.
>
> Please add this to the upcoming commitfest.  We don't seem to have
> enough consensus to just apply it, but perhaps the review process
> will produce some agreement.

Just to be clear, I'm not really against this patch as-is, but it
shouldn't be a precedent or limit us from supporting more permissive
permissions in other areas (or even here) if there are sensible
use-cases for more permissive permissions.

Thanks!

Stephen

Stephen Frost <sfrost@snowman.net> writes:
> Just to be clear, I'm not really against this patch as-is, but it
> shouldn't be a precedent or limit us from supporting more permissive
> permissions in other areas (or even here) if there are sensible
> use-cases for more permissive permissions.

OK, and to be clear, I'm not against considering other use-cases and
trying to do something appropriate for them.  I just reject the idea
that it's unnecessary or inappropriate for us to be concerned about
whether secret-holding files are secure.
        regards, tom lane

Re: Tom Lane 2016-02-22 <21507.1456099088@sss.pgh.pa.us>
> Stephen Frost <sfrost@snowman.net> writes:
> > Just to be clear, I'm not really against this patch as-is, but it
> > shouldn't be a precedent or limit us from supporting more permissive
> > permissions in other areas (or even here) if there are sensible
> > use-cases for more permissive permissions.
> 
> OK, and to be clear, I'm not against considering other use-cases and
> trying to do something appropriate for them.  I just reject the idea
> that it's unnecessary or inappropriate for us to be concerned about
> whether secret-holding files are secure.

I added the patch to the CF: https://commitfest.postgresql.org/9/532/

(I put it under "System administration" and not under "Security"
because it concerns operation.)

Christoph
-- 
cb@df7cb.de | http://www.df7cb.de/

Christoph Berg wrote:
> Re: To Tom Lane 2016-02-19 <20160219115334.GB26862@msg.df7cb.de>
> > Updated patch attached.
>
> *Blush* I though I had compile-tested the patch, but without
> --enable-openssl it wasn't built :(.
>
> The attached patch has successfully been included in the 9.6 Debian
> package, passed the regression tests there, and I've also done some
> chmod/chown tests on the filesystem to verify it indeed catches the
> cases laid out by Tom.

This looks like a pretty reasonable change to me.

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.)

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.

* 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?

--
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

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/

On 3/4/16 3:55 PM, Alvaro Herrera wrote:
> * 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?

I think we should not check for S_IXUSR.  There is no reason for doing that.

I can imagine that key files are sometimes copied around using USB
drives with FAT file systems or other means of that sort where
permissions can scrambled.  While I hate gratuitous executable bits as
much as the next person, insisting here would just create annoyances in
practice.

On 3/10/16 9:20 PM, Peter Eisentraut wrote:
> On 3/4/16 3:55 PM, Alvaro Herrera wrote:
>> * 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?
> 
> I think we should not check for S_IXUSR.  There is no reason for doing that.
> 
> I can imagine that key files are sometimes copied around using USB
> drives with FAT file systems or other means of that sort where
> permissions can scrambled.  While I hate gratuitous executable bits as
> much as the next person, insisting here would just create annoyances in
> practice.

I'm happy with this patch except this minor point.  Any final comments?

Peter Eisentraut wrote:
> On 3/10/16 9:20 PM, Peter Eisentraut wrote:
> > On 3/4/16 3:55 PM, Alvaro Herrera wrote:
> >> * 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?
> > 
> > I think we should not check for S_IXUSR.  There is no reason for doing that.
> > 
> > I can imagine that key files are sometimes copied around using USB
> > drives with FAT file systems or other means of that sort where
> > permissions can scrambled.  While I hate gratuitous executable bits as
> > much as the next person, insisting here would just create annoyances in
> > practice.
> 
> I'm happy with this patch except this minor point.  Any final comments?

No, I think you're right about that one.  Feel free to commit, or I can
do it if you don't want to.

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: Peter Eisentraut 2016-03-16 <56E8C221.1050206@gmx.net>
> >> * 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?
> > 
> > I think we should not check for S_IXUSR.  There is no reason for doing that.
> > 
> > I can imagine that key files are sometimes copied around using USB
> > drives with FAT file systems or other means of that sort where
> > permissions can scrambled.  While I hate gratuitous executable bits as
> > much as the next person, insisting here would just create annoyances in
> > practice.
> 
> I'm happy with this patch except this minor point.  Any final comments?

I'm fine with that change.

Do you want me to update the patch or do you already have a new
version, given it's marked as Ready for Committer?

Christoph
-- 
cb@df7cb.de | http://www.df7cb.de/

Committed with the discussed adjustment and documentation update.

On 3/18/16 2:26 PM, Christoph Berg wrote:
> Re: Peter Eisentraut 2016-03-16 <56E8C221.1050206@gmx.net>
>>>> * 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?
>>>
>>> I think we should not check for S_IXUSR.  There is no reason for doing that.
>>>
>>> I can imagine that key files are sometimes copied around using USB
>>> drives with FAT file systems or other means of that sort where
>>> permissions can scrambled.  While I hate gratuitous executable bits as
>>> much as the next person, insisting here would just create annoyances in
>>> practice.
>>
>> I'm happy with this patch except this minor point.  Any final comments?
> 
> I'm fine with that change.
> 
> Do you want me to update the patch or do you already have a new
> version, given it's marked as Ready for Committer?
> 
> Christoph
> 

Committed with the discussed adjustment and documentation update.

On 3/18/16 2:26 PM, Christoph Berg wrote:
> Re: Peter Eisentraut 2016-03-16 <56E8C221.1050206@gmx.net>
>>>> * 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?
>>>
>>> I think we should not check for S_IXUSR.  There is no reason for doing that.
>>>
>>> I can imagine that key files are sometimes copied around using USB
>>> drives with FAT file systems or other means of that sort where
>>> permissions can scrambled.  While I hate gratuitous executable bits as
>>> much as the next person, insisting here would just create annoyances in
>>> practice.
>>
>> I'm happy with this patch except this minor point.  Any final comments?
> 
> I'm fine with that change.
> 
> Do you want me to update the patch or do you already have a new
> version, given it's marked as Ready for Committer?
> 
> Christoph
>