Re: Relaxing SSL key permission checks

From: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
To: Christoph Berg <myon(at)debian(dot)org>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Bruce Momjian <bruce(at)momjian(dot)us>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Relaxing SSL key permission checks
Date: 2016-03-04 20:55:21
Message-ID: 20160304205521.GA735336@alvherre.pgsql
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Christoph Berg wrote:
> Re: To Tom Lane 2016-02-19 <20160219115334(dot)GB26862(at)msg(dot)df7cb(dot)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

Attachment Content-Type Size
ssl_key_permissions-4.patch text/x-diff 2.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2016-03-04 20:58:05 Re: postgres_fdw vs. force_parallel_mode on ppc
Previous Message Robert Haas 2016-03-04 20:48:29 Re: Typo in comment