Thread: controlling the location of server-side SSL files

I think I would like to have a set of GUC parameters to control the
location of the server-side SSL files.  In a setup that has all the
other configuration files under /etc, the SSL files ought to go there as
well.  (For comparison, most email and web servers keep them there.)
Having them in the data directory keeps requiring a bunch of special
treatment in the configuration management set up, and during backup and
recovery.  Any concerns about that?

<p><br /> On Jan 2, 2012 5:33 AM, "Peter Eisentraut" <<a href="mailto:peter_e@gmx.net">peter_e@gmx.net</a>>
wrote:<br/> ><br /> > I think I would like to have a set of GUC parameters to control the<br /> > location of
theserver-side SSL files.  In a setup that has all the<br /> > other configuration files under /etc, the SSL files
oughtto go there as<br /> > well.  (For comparison, most email and web servers keep them there.)<br /> > Having
themin the data directory keeps requiring a bunch of special<br /> > treatment in the configuration management set
up,and during backup and<br /> > recovery.  Any concerns about that?<br /> ><p>While I personally prefer keeping
themin the data directory for backup reasons (clearly different backup reasons than yours though), i agree we should
havesuch an option - for consistency if nothing else. <p>Were you thinking one option pointing to a directory or one
optionper file? <p>/Magnus <br /> 

On mån, 2012-01-02 at 15:47 +0100, Magnus Hagander wrote:
> Were you thinking one option pointing to a directory or one option per
> file?

One option per file:

ssl_cert_file
ssl_key_file
ssl_ca_file
ssl_crl_file

This is very similar to the configuration of, for example, Apache,
Dovecot, Postfix, so it should be quite familiar to administrators.  It
also mirrors that we have libpq options to set these things on the
client side.  (We use the term "root certificate" in libpq, but I think
"CA" is more commonly used in these situations.  Not sure.)

Peter Eisentraut <peter_e@gmx.net> writes:
> On mån, 2012-01-02 at 15:47 +0100, Magnus Hagander wrote:
>> Were you thinking one option pointing to a directory or one option per
>> file?

> One option per file:

That seems like serious overkill.  Why not one option specifying the
directory?  What use-case is there for letting them be in different
directories, let alone letting the DBA choose random names for each one?
        regards, tom lane

On mån, 2012-01-02 at 23:42 -0500, Tom Lane wrote:
> Peter Eisentraut <peter_e@gmx.net> writes:
> > On mån, 2012-01-02 at 15:47 +0100, Magnus Hagander wrote:
> >> Were you thinking one option pointing to a directory or one option per
> >> file?
> 
> > One option per file:
> 
> That seems like serious overkill.  Why not one option specifying the
> directory?  What use-case is there for letting them be in different
> directories, let alone letting the DBA choose random names for each one?

Several consistency reasons:

- We provide the same per-file options in libpq.

- Indeed, if you use something like dblink or plproxy, these might even
point to the same files.

- We also have settings for hba_file and ident_file that point to a
file.

- All other daemons with SSL support known to me, such as mail and web
servers, have per-file options.

Also some practical reasons:

- Yes, choosing random names is important.  We have local naming
schemes.  And I would rather have a descriptive file name for the CA
than having them all named root.crt, and if I want to know which one it
is I have to look inside the file.

- You might not want all of the files in the same directory.  CA and CRL
might live elsewhere, for example.

On Tue, Jan 3, 2012 at 6:25 PM, Peter Eisentraut <peter_e@gmx.net> wrote:
> On mån, 2012-01-02 at 23:42 -0500, Tom Lane wrote:
>> Peter Eisentraut <peter_e@gmx.net> writes:
>> > On mån, 2012-01-02 at 15:47 +0100, Magnus Hagander wrote:
>> >> Were you thinking one option pointing to a directory or one option per
>> >> file?
>>
>> > One option per file:
>>
>> That seems like serious overkill.  Why not one option specifying the
>> directory?  What use-case is there for letting them be in different
>> directories, let alone letting the DBA choose random names for each one?
>
> [ reasons ]

I agree with these reasons.  We don't get charged $0.50 per GUC, so
there's no particular reason to contort things to have fewer of them.
It's nice where it's possible, of course, but not when it makes people
contort things to support the way we think our users should lay out
their filesystem.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert Haas <robertmhaas@gmail.com> writes:
> On Tue, Jan 3, 2012 at 6:25 PM, Peter Eisentraut <peter_e@gmx.net> wrote:
>> [ reasons ]

> I agree with these reasons.  We don't get charged $0.50 per GUC, so
> there's no particular reason to contort things to have fewer of them.

Well, there definitely is a distributed cost to each additional GUC.
Peter's given what are probably adequate reasons to add several of them
here, but that doesn't mean we should not ask the question whether each
new GUC is really necessary.
        regards, tom lane

On Tue, Jan 3, 2012 at 9:38 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> On Tue, Jan 3, 2012 at 6:25 PM, Peter Eisentraut <peter_e@gmx.net> wrote:
>>> [ reasons ]
>
>> I agree with these reasons.  We don't get charged $0.50 per GUC, so
>> there's no particular reason to contort things to have fewer of them.
>
> Well, there definitely is a distributed cost to each additional GUC.
> Peter's given what are probably adequate reasons to add several of them
> here, but that doesn't mean we should not ask the question whether each
> new GUC is really necessary.

No argument.  I'm merely saying that I think the rationale for these
GUCs is solid enough to justify their existence.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On mån, 2012-01-02 at 06:32 +0200, Peter Eisentraut wrote:
> I think I would like to have a set of GUC parameters to control the
> location of the server-side SSL files.

Here is the patch for this.

One thing that is perhaps worth thinking about:  Currently, we just
ignore missing root.crt and root.crl files.  With this patch, we still
do this, even if the user has given a specific nondefault location.
That seems a bit odd, but I can't think of a simple way to do it better.

On Sat, Jan 14, 2012 at 8:40 AM, Peter Eisentraut <peter_e@gmx.net> wrote:
> On mån, 2012-01-02 at 06:32 +0200, Peter Eisentraut wrote:
>> I think I would like to have a set of GUC parameters to control the
>> location of the server-side SSL files.
>
> Here is the patch for this.
>
> One thing that is perhaps worth thinking about:  Currently, we just
> ignore missing root.crt and root.crl files.  With this patch, we still
> do this, even if the user has given a specific nondefault location.
> That seems a bit odd, but I can't think of a simple way to do it better.

There's a review in the CF app for this finding only minor issues, so
I'm marking this patch therein as "Ready for Committer".

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On tor, 2012-01-19 at 15:44 -0500, Robert Haas wrote:
> On Sat, Jan 14, 2012 at 8:40 AM, Peter Eisentraut <peter_e@gmx.net> wrote:
> > On mån, 2012-01-02 at 06:32 +0200, Peter Eisentraut wrote:
> >> I think I would like to have a set of GUC parameters to control the
> >> location of the server-side SSL files.
> >
> > Here is the patch for this.
> >
> > One thing that is perhaps worth thinking about:  Currently, we just
> > ignore missing root.crt and root.crl files.  With this patch, we still
> > do this, even if the user has given a specific nondefault location.
> > That seems a bit odd, but I can't think of a simple way to do it better.
> 
> There's a review in the CF app for this finding only minor issues, so
> I'm marking this patch therein as "Ready for Committer".

OK, no one had any concerns about the missing file behavior I described
above?  If not, then I'll commit it soon.

On tis, 2012-01-24 at 22:05 +0200, Peter Eisentraut wrote:
> > > One thing that is perhaps worth thinking about:  Currently, we just
> > > ignore missing root.crt and root.crl files.  With this patch, we still
> > > do this, even if the user has given a specific nondefault location.
> > > That seems a bit odd, but I can't think of a simple way to do it better.
> > 
> > There's a review in the CF app for this finding only minor issues, so
> > I'm marking this patch therein as "Ready for Committer".
> 
> OK, no one had any concerns about the missing file behavior I
> described above?  If not, then I'll commit it soon.

I'm still worried about this.  If we ignore a missing root.crt, then the
effect is that authentication and certificate verification might fail,
which would be annoying, but you'd notice it soon enough.  But if we
ignore a missing root.crl, we are creating a security hole.

My best idea at the moment is that we should set these parameters to
empty by default, and make users point them to existing files if they
want to use that functionality.  Comments?

On tis, 2012-01-24 at 22:05 +0200, Peter Eisentraut wrote:
> > > One thing that is perhaps worth thinking about:  Currently, we just
> > > ignore missing root.crt and root.crl files.  With this patch, we still
> > > do this, even if the user has given a specific nondefault location.
> > > That seems a bit odd, but I can't think of a simple way to do it better.
> >
> > There's a review in the CF app for this finding only minor issues, so
> > I'm marking this patch therein as "Ready for Committer".
>
> OK, no one had any concerns about the missing file behavior I
> described above?  If not, then I'll commit it soon.

I'm still worried about this.  If we ignore a missing root.crt, then the
effect is that authentication and certificate verification might fail,
which would be annoying, but you'd notice it soon enough.  But if we
ignore a missing root.crl, we are creating a security hole.

My best idea at the moment is that we should set these parameters to
empty by default, and make users point them to existing files if they
want to use that functionality.  Comments?

On ons, 2012-02-08 at 09:16 +0100, Magnus Hagander wrote:
> > My best idea at the moment is that we should set these parameters to
> > empty by default, and make users point them to existing files if they
> > want to use that functionality.  Comments?
> >
>
> +1. Anybody who actually cares about setting up security is likely not
> going to rely on defaults anyway - and is certainly going to review
> whatever they are. So there should be no big problem there.

Updated patch to reflect this.

On ons, 2012-02-08 at 09:16 +0100, Magnus Hagander wrote:
> > I'm still worried about this.  If we ignore a missing root.crt, then the
> > effect is that authentication and certificate verification might fail,
> > which would be annoying, but you'd notice it soon enough.  But if we
> > ignore a missing root.crl, we are creating a security hole.
> >
> 
> Yes, ignoring a missing file in a security context is definitely not good.
> It should throw an error.
> 
> We have a few bad defaults from the old days around SSL for this, but if it
> requires breaking backwards compatibility to get it right, I think we
> should still do it. 

Btw., should we also consider making similar changes on the libpq side?

Peter Eisentraut <peter_e@gmx.net> writes:
> On ons, 2012-02-08 at 09:16 +0100, Magnus Hagander wrote:
>> Yes, ignoring a missing file in a security context is definitely not good.
>> It should throw an error.
>> 
>> We have a few bad defaults from the old days around SSL for this, but if it
>> requires breaking backwards compatibility to get it right, I think we
>> should still do it. 

> Btw., should we also consider making similar changes on the libpq side?

I think that breaking compatibility of libpq's behavior is a whole lot
harder sell than changing things in a way that only affects what people
have to put into postgresql.conf.  We've always treated the latter as
something that can change across major versions.

In particular, I observe that we get pushback anytime we break something
in a way that makes SSL config files be required on the client side;
see bug #6302 for most recent example.
        regards, tom lane

On ons, 2012-02-29 at 14:20 -0500, Tom Lane wrote:
> In particular, I observe that we get pushback anytime we break something
> in a way that makes SSL config files be required on the client side;
> see bug #6302 for most recent example.

*If* we were to make a change in libpq analogous to the server side, the
effect would be to make the files less required, which could actually
help the case of bug #6302.

Peter Eisentraut <peter_e@gmx.net> writes:
> On ons, 2012-02-29 at 14:20 -0500, Tom Lane wrote:
>> In particular, I observe that we get pushback anytime we break something
>> in a way that makes SSL config files be required on the client side;
>> see bug #6302 for most recent example.

> *If* we were to make a change in libpq analogous to the server side, the
> effect would be to make the files less required, which could actually
> help the case of bug #6302.

Hm?  Obviously I misunderstood what changes you were proposing to make,
so would you mind spelling it out?
        regards, tom lane

On ons, 2012-02-29 at 14:27 -0500, Tom Lane wrote:
> Peter Eisentraut <peter_e@gmx.net> writes:
> > On ons, 2012-02-29 at 14:20 -0500, Tom Lane wrote:
> >> In particular, I observe that we get pushback anytime we break something
> >> in a way that makes SSL config files be required on the client side;
> >> see bug #6302 for most recent example.
> 
> > *If* we were to make a change in libpq analogous to the server side, the
> > effect would be to make the files less required, which could actually
> > help the case of bug #6302.
> 
> Hm?  Obviously I misunderstood what changes you were proposing to make,
> so would you mind spelling it out?

The details are to be determined, but a possible change would likely be
that instead of looking for a file and using it if and only if found,
there would be some kind of connection parameter saying "use this file
for this functionality", and otherwise it's not used.  The particular
example would be the CRL file.

Peter Eisentraut <peter_e@gmx.net> writes:
> On ons, 2012-02-29 at 14:27 -0500, Tom Lane wrote:
>> Hm?  Obviously I misunderstood what changes you were proposing to make,
>> so would you mind spelling it out?

> The details are to be determined, but a possible change would likely be
> that instead of looking for a file and using it if and only if found,
> there would be some kind of connection parameter saying "use this file
> for this functionality", and otherwise it's not used.  The particular
> example would be the CRL file.

Mph.  That seems unlikely to be a net win to me.  The scenario I'm
imagining is that you ("you" being DBA for some group of people) didn't
have a CRL file before, and now you need one.  Your administration
problem is to get that CRL file into place for all your users.
If we change as above, then you still have that admin problem, plus now
you have another: getting all your users to use the new connection
parameter.  Which, as a rule, is going to be tough (for example, psql
has no easy way to make that happen).  The new admin problem offers you
no leverage at all on the old one, either, since a user who's not
acquired the CRL file more than likely hasn't changed his connection
habits either.

There may or may not be some value in a connection parameter that allows
specifying a location besides ~/.postgresql/ for the SSL support files.
But I don't find any attraction in changing the default behavior.
        regards, tom lane