[PATCH] Reload SSL certificates on SIGHUP

On 5/30/15 10:14 PM, Andreas Karlsson wrote:

I have written a patch which makes it possible to change SSL
certificates (and other SSL parameters, including the CRL) without
restarting PostgreSQL. In fact this patch also makes it possible to turn
on or off ssl entirely without restart. It does so by initializing a new
SSL context when the postmaster receives a SIGHUP, and if the
initialization succeeded the old context is replaced by the new.

I think this would be a useful feature, and the implementation looks
sound. But I don't like how the reload is organized. Reinitializing
the context in the sighup handler, aside from questions about how much
work one should do in a signal handler, would cause SSL reinitialization
for unrelated reloads. We have the GUC assign hook mechanism for
handling this sort of thing. The trick would be that when multiple
SSL-related settings change, you only want to do one reinitialization.
You could either have the different assign hooks communicate with each
other somehow, or have them set a "need SSL init" flag that is checked
somewhere else.

There was some previous discussion[1] on the mailing list about what the
proper context should be for the SSL parameters, but as far as I can
tell the discussion never reached a conclusion. I have changed the SSL
GUCs to PGC_SIGUP since I felt that was the closest to the truth, but it
is not a perfect fit (the backends wont reload the SSL context). Should
we add a new context for the SSL GUCs?

I think PGC_SIGHUP is fine for this.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 07/02/2015 06:13 PM, Peter Eisentraut wrote:

I think this would be a useful feature, and the implementation looks
sound. But I don't like how the reload is organized. Reinitializing
the context in the sighup handler, aside from questions about how much
work one should do in a signal handler, would cause SSL reinitialization
for unrelated reloads. We have the GUC assign hook mechanism for
handling this sort of thing. The trick would be that when multiple
SSL-related settings change, you only want to do one reinitialization.
You could either have the different assign hooks communicate with each
other somehow, or have them set a "need SSL init" flag that is checked
somewhere else.

It is not enough to just add a hook to the GUCs since I would guess most
users would expect the certificate to be reloaded if just the file has
been replaced and no GUC was changed. To support this we would need to
also check the mtimes of the SSL files, would that complexity really be
worth it?

Andreas

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Wed, Jul 22, 2015 at 9:52 AM, Andreas Karlsson <andreas@proxel.se> wrote:

On 07/02/2015 06:13 PM, Peter Eisentraut wrote:

I think this would be a useful feature, and the implementation looks
sound. But I don't like how the reload is organized. Reinitializing
the context in the sighup handler, aside from questions about how much
work one should do in a signal handler, would cause SSL reinitialization
for unrelated reloads. We have the GUC assign hook mechanism for
handling this sort of thing. The trick would be that when multiple
SSL-related settings change, you only want to do one reinitialization.
You could either have the different assign hooks communicate with each
other somehow, or have them set a "need SSL init" flag that is checked
somewhere else.

It is not enough to just add a hook to the GUCs since I would guess most
users would expect the certificate to be reloaded if just the file has been
replaced and no GUC was changed.

Why? It seems to me that the assign hook gets called once per process
at reload for a SIGHUP parameter even if its value is not changed, no?

To support this we would need to also check
the mtimes of the SSL files, would that complexity really be worth it?

Or we could reload the SSL context unconditionally once per reload
loop. I am wondering how costly that may prove to be though.
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 7/21/15 8:52 PM, Andreas Karlsson wrote:

It is not enough to just add a hook to the GUCs since I would guess most
users would expect the certificate to be reloaded if just the file has
been replaced and no GUC was changed. To support this we would need to
also check the mtimes of the SSL files, would that complexity really be
worth it?

Actually, I misread your patch. I thought you only wanted to reload the
SSL files when the GUC settings change, but of course we also want to
reload them when the files are changed.

I don't have a problem with rebuilding the SSL context on every reload
cycle. We already do a lot of extra reloading every time, so a bit more
shouldn't hurt. But I'm not so sure whether we should do that in the
SIGHUP handler. I don't know how we got into the situation of doing all
the file reloads directly in the handler, but at least we can control
that code. Making a bunch of calls into an external library is a
different thing, though. Can we find a way to do this differently?

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Peter Eisentraut <peter_e@gmx.net> writes:

I don't have a problem with rebuilding the SSL context on every reload
cycle. We already do a lot of extra reloading every time, so a bit more
shouldn't hurt. But I'm not so sure whether we should do that in the
SIGHUP handler. I don't know how we got into the situation of doing all
the file reloads directly in the handler, but at least we can control
that code. Making a bunch of calls into an external library is a
different thing, though. Can we find a way to do this differently?

Do we have an idea how expensive it is to load that data?

A brute-force answer is to not have the postmaster load it at all,
but to have new backends do so (if needed) during their connection
acceptance/authentication phase. I'm not sure how much that would
add to the SSL connection startup time though. It would also mean
that problems with the SSL config files would only be reported during
subsequent connection starts, not at SIGHUP time, and indeed that
SIGHUP is more or less meaningless for SSL file changes: the instant
you change a file, it's live for later connections. On the plus side,
it would make Windows and Unix behavior closer, since (I suppose)
we're reloading that stuff anyway in EXEC_BACKEND builds.

I'm not entirely sure your concern is valid, though. We have always had
the principle that almost everything of interest in the postmaster happens
in signal handler functions. We could possibly change things so that
reloading config files is done in the "main loop" of ServerLoop, but
if we did, it would have to execute with all signals blocked, which seems
like just about as much of a risk for third-party code as executing that
code in a signal handler is.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 07/23/2015 07:19 AM, Michael Paquier wrote:

On Wed, Jul 22, 2015 at 9:52 AM, Andreas Karlsson <andreas@proxel.se> wrote:

On 07/02/2015 06:13 PM, Peter Eisentraut wrote:

I think this would be a useful feature, and the implementation looks
sound. But I don't like how the reload is organized. Reinitializing
the context in the sighup handler, aside from questions about how much
work one should do in a signal handler, would cause SSL reinitialization
for unrelated reloads. We have the GUC assign hook mechanism for
handling this sort of thing. The trick would be that when multiple
SSL-related settings change, you only want to do one reinitialization.
You could either have the different assign hooks communicate with each
other somehow, or have them set a "need SSL init" flag that is checked
somewhere else.

It is not enough to just add a hook to the GUCs since I would guess most
users would expect the certificate to be reloaded if just the file has been
replaced and no GUC was changed.

Why? It seems to me that the assign hook gets called once per process
at reload for a SIGHUP parameter even if its value is not changed, no?

My bad, I tested it and you are correct. But I am not convinced moving
the SSL initialization to a GUC assign hook would make anything clearer.
It would not move any work out of the signal handler either since the
assign hooks are ran inside it, and the hook will be ran in all backends
which is not any interesting property for SSL initialization.

Andreas

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Wed, Jul 29, 2015 at 10:24 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Peter Eisentraut <peter_e@gmx.net> writes:

I don't have a problem with rebuilding the SSL context on every reload
cycle. We already do a lot of extra reloading every time, so a bit more
shouldn't hurt. But I'm not so sure whether we should do that in the
SIGHUP handler. I don't know how we got into the situation of doing all
the file reloads directly in the handler, but at least we can control
that code. Making a bunch of calls into an external library is a
different thing, though. Can we find a way to do this differently?

Do we have an idea how expensive it is to load that data?

There are no numbers on this thread. And honestly I would be curious
as well to see a run of pgbench with -C doing or similar to check how
long it takes to establish a connection. I would expect it to be
measurable though, but here I'm just hand-waving ;)

A brute-force answer is to not have the postmaster load it at all,
but to have new backends do so (if needed) during their connection
acceptance/authentication phase. I'm not sure how much that would
add to the SSL connection startup time though. It would also mean
that problems with the SSL config files would only be reported during
subsequent connection starts, not at SIGHUP time, and indeed that
SIGHUP is more or less meaningless for SSL file changes: the instant
you change a file, it's live for later connections. On the plus side,
it would make Windows and Unix behavior closer, since (I suppose)
we're reloading that stuff anyway in EXEC_BACKEND builds.

Indeed.
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 07/29/2015 03:24 AM, Tom Lane wrote:

Peter Eisentraut <peter_e@gmx.net> writes:

I don't have a problem with rebuilding the SSL context on every reload
cycle. We already do a lot of extra reloading every time, so a bit more
shouldn't hurt. But I'm not so sure whether we should do that in the
SIGHUP handler. I don't know how we got into the situation of doing all
the file reloads directly in the handler, but at least we can control
that code. Making a bunch of calls into an external library is a
different thing, though. Can we find a way to do this differently?

Do we have an idea how expensive it is to load that data?

A brute-force answer is to not have the postmaster load it at all,
but to have new backends do so (if needed) during their connection
acceptance/authentication phase. I'm not sure how much that would
add to the SSL connection startup time though. It would also mean
that problems with the SSL config files would only be reported during
subsequent connection starts, not at SIGHUP time, and indeed that
SIGHUP is more or less meaningless for SSL file changes: the instant
you change a file, it's live for later connections. On the plus side,
it would make Windows and Unix behavior closer, since (I suppose)
we're reloading that stuff anyway in EXEC_BACKEND builds.

I measured it taking ~0.3ms to build the new SSL context in a simple
benchmark (certificate + CA + small crl).

Personally I do not think moving this to connection start would be worth
it since reporting errors that late is not nice for people who have
misconfigured their database, and even if my benchmarks indicates it is
relatively cheap to reload SSL adding more work to connection
establishing is something I would want to avoid unless it gives us a
clear benefit.

I'm not entirely sure your concern is valid, though. We have always had
the principle that almost everything of interest in the postmaster happens
in signal handler functions. We could possibly change things so that
reloading config files is done in the "main loop" of ServerLoop, but
if we did, it would have to execute with all signals blocked, which seems
like just about as much of a risk for third-party code as executing that
code in a signal handler is.

Agreed, I am not sure what moving it to the main loop would gain us.

--
Andreas

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[ moving this discussion back to the patch thread ]

Andreas Karlsson <andreas@proxel.se> writes:

On 08/25/2015 09:39 AM, Michael Paquier wrote:

-- Reload SSL certificates on SIGHUP: returned with feedback? I think
that this patch needs more work to be in a commitable state.

Maybe I am being dense here, but I do not feel like I have gotten any
clear feedback which gives me a way forward with the patch. I do not
really see what more I can do here other than resubmit it to the next CF
which I feel would be poor etiquette by me.

I think we pretty much rejected Peter's concern about doing the work
in the SIGHUP handler. There's been some other discussion about
refactoring the postmaster to not do all its work in signal handlers,
but that is material for a different patch. Absent hard evidence that
reloading SSL config in the handler actually fails, I don't think we
should ask this patch to do a half-baked job of refactoring that.

However ... a concern I've got is that there's a difference between how
the Unix and Windows builds work, and this patch will move that from a
back-burner issue to a significant concern. Namely, that on Unix we load
the SSL data once and that's what you use, while on Windows (or any
EXEC_BACKEND build) what you're going to get is whatever is in the files
right now when a connection starts, whether it's good or bad. What this
patch does, unless I missed something, is to persuade the Unix ports to
implement "reload SSL data at SIGHUP", which is good; but the Windows
behavior stays where it is.

It didn't matter so much as long as changing the SSL config files wasn't
considered a supported operation; but if that is supported, then people
are going to complain.

Is it unreasonable of me to ask for the Windows behavior to be fixed at
the same time? I dunno. It's perhaps less broken than the Unix behavior,
but that doesn't make it desirable. OTOH it might be a significantly
larger patch, and I confess I'm not even too sure what we'd have to do.

So I think the way to move this forward is to investigate how to hold
the SSL config constant until SIGHUP in an EXEC_BACKEND build. If we
find out that that's unreasonably difficult, maybe we'll decide that
we can live without it; but I'd like to see the question investigated
rather than ignored.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Wed, Aug 26, 2015 at 10:57 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

[...]
So I think the way to move this forward is to investigate how to hold
the SSL config constant until SIGHUP in an EXEC_BACKEND build. If we
find out that that's unreasonably difficult, maybe we'll decide that
we can live without it; but I'd like to see the question investigated
rather than ignored.

You have a point here.

In EXEC_BACKEND, parameter updated via SIGHUP are only taken into
account by newly-started backends, right? Hence, a way to do what we
want is to actually copy the data needed to initialize the SSL context
into alternate file(s). When postmaster starts up, or when SIGHUP
shows up those alternate files are upserted by the postmaster.
be-secure-openssl.c needs also to be changed such as with EXEC_BACKEND
the context needs to be loaded from those alternate files. At quick
glance this seems doable.

For now I am moving the patch to the next CF, more investigation is
surely needed.
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Wed, Aug 26, 2015 at 12:24 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:

On Wed, Aug 26, 2015 at 10:57 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

[...]
So I think the way to move this forward is to investigate how to hold
the SSL config constant until SIGHUP in an EXEC_BACKEND build. If we
find out that that's unreasonably difficult, maybe we'll decide that
we can live without it; but I'd like to see the question investigated
rather than ignored.

You have a point here.

In EXEC_BACKEND, parameter updated via SIGHUP are only taken into
account by newly-started backends, right?

Oops. I mistook with PGC_BACKEND here. Sorry for the noise.

Hence, a way to do what we
want is to actually copy the data needed to initialize the SSL context
into alternate file(s). When postmaster starts up, or when SIGHUP
shows up those alternate files are upserted by the postmaster.
be-secure-openssl.c needs also to be changed such as with EXEC_BACKEND
the context needs to be loaded from those alternate files. At quick
glance this seems doable.

Still, this idea would be to use a set of alternate files in global/
to set the context, basically something like
config_exec_ssl_cert_file, config_exec_ssl_key_file and
config_exec_ssl_ca_file. It does not seem to be necessary to
manipulate [read|write]_nondefault_variables() as the use of this
metadata should be made only when SSL context is initialized on
backend. Other thoughts welcome.
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 08/26/2015 03:57 AM, Tom Lane wrote:

Is it unreasonable of me to ask for the Windows behavior to be fixed at
the same time? I dunno. It's perhaps less broken than the Unix behavior,
but that doesn't make it desirable. OTOH it might be a significantly
larger patch, and I confess I'm not even too sure what we'd have to do.

So I think the way to move this forward is to investigate how to hold
the SSL config constant until SIGHUP in an EXEC_BACKEND build. If we
find out that that's unreasonably difficult, maybe we'll decide that
we can live without it; but I'd like to see the question investigated
rather than ignored.

I think this is a real concern and one that I will look into, to see if
it can be fixed with a reasonable amount of work.

Andreas

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 2015-08-27 01:12:54 +0200, Andreas Karlsson wrote:

I think this is a real concern and one that I will look into, to see if it
can be fixed with a reasonable amount of work.

This patch has been in waiting-for-author for a month. Marking it as
returned-with-feedback.

Greetings,

Andres Freund

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 08/26/2015 07:46 AM, Michael Paquier wrote:

On Wed, Aug 26, 2015 at 12:24 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:

On Wed, Aug 26, 2015 at 10:57 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

[...]
So I think the way to move this forward is to investigate how to hold
the SSL config constant until SIGHUP in an EXEC_BACKEND build. If we
find out that that's unreasonably difficult, maybe we'll decide that
we can live without it; but I'd like to see the question investigated
rather than ignored.

You have a point here.

In EXEC_BACKEND, parameter updated via SIGHUP are only taken into
account by newly-started backends, right?

Oops. I mistook with PGC_BACKEND here. Sorry for the noise.

Hence, a way to do what we
want is to actually copy the data needed to initialize the SSL context
into alternate file(s). When postmaster starts up, or when SIGHUP
shows up those alternate files are upserted by the postmaster.
be-secure-openssl.c needs also to be changed such as with EXEC_BACKEND
the context needs to be loaded from those alternate files. At quick
glance this seems doable.

Still, this idea would be to use a set of alternate files in global/
to set the context, basically something like
config_exec_ssl_cert_file, config_exec_ssl_key_file and
config_exec_ssl_ca_file. It does not seem to be necessary to
manipulate [read|write]_nondefault_variables() as the use of this
metadata should be made only when SSL context is initialized on
backend. Other thoughts welcome.

Sorry for dropping this patch, but now I have started looking at it again.

I started implementing your suggested solution, but realized that I do
not like copying of the private key file. The private key might have
been put by the DBA on another file system for security reasons and
having PostgreSQL copy potentially sensitive data to somewhere under
pg_data seems like a surprising behavior. Especially since this only
happens on some platforms.

I guess a possible solution would be to read the files into the
postmaster (where we already have the private key today) and have
OpenSSL read the keys from memory and re-implement something like
SSL_CTX_use_certificate_chain_file() in our code, and similar things for
the other functions which now take a path. This seems like a bit too
much work to burden this patch with (and not obviously something we
would want anyway) since the behavior is already different on Windows in
the current code.

Thoughts?

I have attached a rebased version of the original patch which applies on
current master.

Andreas

On Mon, Nov 23, 2015 at 12:29 PM, Andreas Karlsson <andreas@proxel.se> wrote:

On 08/26/2015 07:46 AM, Michael Paquier wrote:

On Wed, Aug 26, 2015 at 12:24 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:

On Wed, Aug 26, 2015 at 10:57 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

[...]
So I think the way to move this forward is to investigate how to hold
the SSL config constant until SIGHUP in an EXEC_BACKEND build. If we
find out that that's unreasonably difficult, maybe we'll decide that
we can live without it; but I'd like to see the question investigated
rather than ignored.

You have a point here.

In EXEC_BACKEND, parameter updated via SIGHUP are only taken into
account by newly-started backends, right?

Oops. I mistook with PGC_BACKEND here. Sorry for the noise.

Hence, a way to do what we
want is to actually copy the data needed to initialize the SSL context
into alternate file(s). When postmaster starts up, or when SIGHUP
shows up those alternate files are upserted by the postmaster.
be-secure-openssl.c needs also to be changed such as with EXEC_BACKEND
the context needs to be loaded from those alternate files. At quick
glance this seems doable.

Still, this idea would be to use a set of alternate files in global/
to set the context, basically something like
config_exec_ssl_cert_file, config_exec_ssl_key_file and
config_exec_ssl_ca_file. It does not seem to be necessary to
manipulate [read|write]_nondefault_variables() as the use of this
metadata should be made only when SSL context is initialized on
backend. Other thoughts welcome.

I started implementing your suggested solution, but realized that I do not
like copying of the private key file. The private key might have been put by
the DBA on another file system for security reasons and having PostgreSQL
copy potentially sensitive data to somewhere under pg_data seems like a
surprising behavior. Especially since this only happens on some platforms.

You are referring for example to Fedora/Ubuntu that use a symlink to
point to those SSL files, right? Yes this approach may be a security
concern in those cases... One idea may be that we actually encrypt
this data

I guess a possible solution would be to read the files into the postmaster
(where we already have the private key today) and have OpenSSL read the keys
from memory and re-implement something like
SSL_CTX_use_certificate_chain_file() in our code, and similar things for the
other functions which now take a path. This seems like a bit too much work
to burden this patch with (and not obviously something we would want anyway)
since the behavior is already different on Windows in the current code.

Thoughts?

Reimplementing a twin of SSL_CTX_use_certificate_chain_file() would
have benefits in this case, but that's really something to avoid. I
may say something stupid here, but what if as you say we store the
information of the certificate into a dedicated shared memory block
when postmaster starts up, except that when we need to reload the keys
we dump them into a temporary file with tmpfile or similar and then
read it using SSL_CTX_use_certificate_chain_file(). For EXEC_BACKEND,
the shared memory block will be reattached at startup using
PGSharedMemoryReAttach() so it should have the data. For
non-EXEC_BACKEND, the child processes will just inherit the shmem
block with fork(). When SIGHUP is issued, all the processes
unconditionally dump the data into a per-process tmp file and then
reload it in the SSL context.
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Sun, Nov 22, 2015 at 7:29 PM, Andreas Karlsson <andreas@proxel.se> wrote:

Sorry for dropping this patch, but now I have started looking at it again.

Any chance of picking this up again soon, Andreas? I think it's an
important project. I would like to review it.

--
Peter Geoghegan

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 08/31/2016 11:34 PM, Peter Geoghegan wrote:

On Sun, Nov 22, 2015 at 7:29 PM, Andreas Karlsson <andreas@proxel.se> wrote:

Sorry for dropping this patch, but now I have started looking at it again.

Any chance of picking this up again soon, Andreas? I think it's an
important project. I would like to review it.

I do not really have any good ideas for how to fix it for Windows, but
if anyone would like to discuss solutions I am interested in working on
this patch again.

The alternatives as I see them now:

1) Serialize the certificates, key, and CRL and write them to the
backend_var temp file and then deserialize everything in the backends.

Sounds like you would need to write some code for every SSL library to
support the serialization and deserialization, which I am not a fan of
doing just for one platform since I worry about little used code paths.
Additionally this would mean that we write a copy of the private key to
potentially another file system than the one where the private key is
stored, this sounds like a bad idea from a security point of view.

2) Copy all the SSL related files into the data directory at SIGHUP,
before loading them. While this does not require any serialization of
certificates it still has the problem of writing private keys to disk.

3) Leave my patch as it is now. This means the postmaster will reload
certificates on SIGHUP while the backends will also load them when
spawning. This means windows will continue to work the same as before my
patch.

Is there any other way to pass the current set of loaded certificates
and keys from the postmaster to the backends on Windows? I guess you
could use a pipe, but if so we should probably send all data on this
pipe, not just the SSL stuff.

I am leaning towards doing (3) but I know I am biased since it is less
work and I do not care much for Windows.

Andreas

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Sun, Sep 4, 2016 at 11:39 PM, Andreas Karlsson <andreas@proxel.se> wrote:

1) Serialize the certificates, key, and CRL and write them to the
backend_var temp file and then deserialize everything in the backends.

Sounds like you would need to write some code for every SSL library to
support the serialization and deserialization, which I am not a fan of doing
just for one platform since I worry about little used code paths.
Additionally this would mean that we write a copy of the private key to
potentially another file system than the one where the private key is
stored, this sounds like a bad idea from a security point of view.

Yeah... This would result in something that is heavily SSL-dependent,
which would be an additional maintenance pain when trying to support
future OpenSSL versions.

2) Copy all the SSL related files into the data directory at SIGHUP, before
loading them. While this does not require any serialization of certificates
it still has the problem of writing private keys to disk.

You expressed enough concern about that upthread, copying private keys
into PGDATA is a security concern.

3) Leave my patch as it is now. This means the postmaster will reload
certificates on SIGHUP while the backends will also load them when spawning.
This means windows will continue to work the same as before my patch.

Is there any other way to pass the current set of loaded certificates and
keys from the postmaster to the backends on Windows? I guess you could use a
pipe, but if so we should probably send all data on this pipe, not just the
SSL stuff.

I am leaning towards doing (3) but I know I am biased since it is less work
and I do not care much for Windows.

Seriously... The benefit of this feature is clear for a lot of people.
And the implementation dedicated only to Windows would just result in
a grotty thing anyway. So I'd say that at this point we could just
push for 3) and facilitate the life of most with SSL configuration.
The behavior across platforms needs to be properly documented for
sure.
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Wed, 7 Sep 2016 17:09:17 +0900
Michael Paquier <michael.paquier@gmail.com> wrote:

On Sun, Sep 4, 2016 at 11:39 PM, Andreas Karlsson <andreas@proxel.se>
wrote:

1) Serialize the certificates, key, and CRL and write them to the
backend_var temp file and then deserialize everything in the
backends.

Sounds like you would need to write some code for every SSL library
to support the serialization and deserialization, which I am not a
fan of doing just for one platform since I worry about little used
code paths. Additionally this would mean that we write a copy of
the private key to potentially another file system than the one
where the private key is stored, this sounds like a bad idea from a
security point of view.

Yeah... This would result in something that is heavily SSL-dependent,
which would be an additional maintenance pain when trying to support
future OpenSSL versions.

OpenSSL has documented API for serializing/deserializing each and every
cryptographic format it supports. And this API quite unlikely to change
in future OpenSSL versions. Moreover, LibreSSL is compatible with this
API as far as I know.

Really, Apache does simular thing for ages - it starts as root, loads
certificates and keys, serializes them in memory, then forks and drops
privileges, and then uses these keys and certificates.

There are two problems with this approach

1. You have to carefully design data structures to store serialized
keys. Apache made a mistake there and didn't allow for future invention
of new public key algorithms. So, in 2008 I had problems adding
russian GOST cryptography there.

2. You keys and certificates might not be stored in the filesystem at
all. They can live in some hardware cryptography module, which don't
let private keys out, just provide some handle.
(OpenSSL supports it via lodable engine modules, and Postgres allows to
use engine modules since 8.2, so people can use it with postgres).

Really OpenSSL/LibreSSL provide useful abstraction of memory BIO (Where
BIO stands for basic input/output) which can be used to store
serialized cryptographic data. And serializing/deserializing API is
designed to work with BIO anyway.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers