ssl passphrase callback

On Thu, Oct 31, 2019 at 11:37 AM Andrew Dunstan
<andrew.dunstan@2ndquadrant.com> wrote:

This patch provides a hook for a function that can supply an SSL
passphrase. The hook can be filled in by a shared preloadable module. In
order for that to be effective, the startup order is modified slightly.
There is a test attached that builds and uses one trivial
implementation, which just takes a configuration setting and rot13's it
before supplying the result as the passphrase.

It seems to me that it would be a lot better to have an example in
contrib that does something which might be of actual use to users,
such as running a shell command and reading the passphrase from
stdout.

Features that are only accessible by writing C code are, in general,
not as desirable as features which can be accessed via SQL or
configuration.

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

On 11/1/19 11:01 AM, Robert Haas wrote:

On Thu, Oct 31, 2019 at 11:37 AM Andrew Dunstan
<andrew.dunstan@2ndquadrant.com> wrote:

This patch provides a hook for a function that can supply an SSL
passphrase. The hook can be filled in by a shared preloadable module. In
order for that to be effective, the startup order is modified slightly.
There is a test attached that builds and uses one trivial
implementation, which just takes a configuration setting and rot13's it
before supplying the result as the passphrase.

It seems to me that it would be a lot better to have an example in
contrib that does something which might be of actual use to users,
such as running a shell command and reading the passphrase from
stdout.

Features that are only accessible by writing C code are, in general,
not as desirable as features which can be accessed via SQL or
configuration.

Well, I tried to provide the most trivial and simple test I could come
up with. Running a shell command can already be accomplished via the
ssl_passphrase_command setting.

cheers

andrew

--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Sat, Nov 2, 2019 at 6:57 AM Andrew Dunstan
<andrew.dunstan@2ndquadrant.com> wrote:

On 11/1/19 11:01 AM, Robert Haas wrote:

On Thu, Oct 31, 2019 at 11:37 AM Andrew Dunstan
<andrew.dunstan@2ndquadrant.com> wrote:

This patch provides a hook for a function that can supply an SSL
passphrase. The hook can be filled in by a shared preloadable module. In
order for that to be effective, the startup order is modified slightly.
There is a test attached that builds and uses one trivial
implementation, which just takes a configuration setting and rot13's it
before supplying the result as the passphrase.

It seems to me that it would be a lot better to have an example in
contrib that does something which might be of actual use to users,
such as running a shell command and reading the passphrase from
stdout.

Features that are only accessible by writing C code are, in general,
not as desirable as features which can be accessed via SQL or
configuration.

Well, I tried to provide the most trivial and simple test I could come
up with. Running a shell command can already be accomplished via the
ssl_passphrase_command setting.

It looks like the new declarations in libpq-be.h are ifdef'd out in a
non-USE_SSL build, but then we still try to build the new test module
and it fails:

https://ci.appveyor.com/project/postgresql-cfbot/postgresql/build/1.0.64071

On Fri, Nov 1, 2019 at 01:57:29PM -0400, Andrew Dunstan wrote:

On 11/1/19 11:01 AM, Robert Haas wrote:

On Thu, Oct 31, 2019 at 11:37 AM Andrew Dunstan
<andrew.dunstan@2ndquadrant.com> wrote:

This patch provides a hook for a function that can supply an SSL
passphrase. The hook can be filled in by a shared preloadable module. In
order for that to be effective, the startup order is modified slightly.
There is a test attached that builds and uses one trivial
implementation, which just takes a configuration setting and rot13's it
before supplying the result as the passphrase.

It seems to me that it would be a lot better to have an example in
contrib that does something which might be of actual use to users,
such as running a shell command and reading the passphrase from
stdout.

Features that are only accessible by writing C code are, in general,
not as desirable as features which can be accessed via SQL or
configuration.

Well, I tried to provide the most trivial and simple test I could come
up with. Running a shell command can already be accomplished via the
ssl_passphrase_command setting.

What is the value of a shared library over a shell command? We had this
discussion in relation to archive_command years ago, and decided on a
shell command as the best API.

--
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. +
+                      Ancient Roman grave inscription +

On 11/4/19 4:43 PM, Thomas Munro wrote:

It looks like the new declarations in libpq-be.h are ifdef'd out in a
non-USE_SSL build, but then we still try to build the new test module
and it fails:

https://ci.appveyor.com/project/postgresql-cfbot/postgresql/build/1.0.64071

I think this updated patch should fix things.

cheers

andrew

--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Thu, 7 Nov 2019 at 10:24, Bruce Momjian <bruce@momjian.us> wrote:

What is the value of a shared library over a shell command? We had this
discussion in relation to archive_command years ago, and decided on a
shell command as the best API.

I don't recall such a discussion, but I can give perspective:

* shell command offered the widest and simplest API for integration, which
was the most important consideration for a backup API. That choice caused
difficulty with the need to pass information to the external command, e.g.
%f %p

* shared library is more appropriate for a security-related module, so
users can't see how it is configured, as well as being more
tightly integrated so it can be better tailored to various uses

Summary is that the choice is not random, nor mere preference

--
Simon Riggs http://www.2ndQuadrant.com/
<http://www.2ndquadrant.com/&gt;
PostgreSQL Solutions for the Enterprise

On Fri, Nov 08, 2019 at 11:12:08PM +0900, Simon Riggs wrote:

On Thu, 7 Nov 2019 at 10:24, Bruce Momjian <bruce@momjian.us> wrote:

What is the value of a shared library over a shell command? We had
this discussion in relation to archive_command years ago, and decided
on a shell command as the best API.

I don't recall such a discussion, but I can give perspective:

* shell command offered the widest and simplest API for integration,
which was the most important consideration for a backup API. That
choice caused difficulty with the need to pass information to the
external command, e.g. %f %p

It's not clear to me why simple API for integration would be less
valuable for this feature. Also, I'm sure passing data to/from shell
command may be tricky, but presumably we have figured how to do that.

* shared library is more appropriate for a security-related module, so
users can't see how it is configured, as well as being more
tightly integrated so it can be better tailored to various uses

I don't follow. Why would there be a significant difference between a
shell command/script and shared library in this respect? If you don't
want the users to see the config, just store it in a separate file and
it's about the same as storing it in the .so library.

Is there something that can be done with a .so library but can't be done
with a shell command (which may just call a binary, with all the config
included, making it equal to the .so solution)?

regards

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

On Wed, Nov 6, 2019 at 7:24 PM Bruce Momjian <bruce@momjian.us> wrote:

On Fri, Nov 1, 2019 at 01:57:29PM -0400, Andrew Dunstan wrote:

On 11/1/19 11:01 AM, Robert Haas wrote:

On Thu, Oct 31, 2019 at 11:37 AM Andrew Dunstan
<andrew.dunstan@2ndquadrant.com> wrote:

This patch provides a hook for a function that can supply an SSL
passphrase. The hook can be filled in by a shared preloadable module.

In

order for that to be effective, the startup order is modified

slightly.

There is a test attached that builds and uses one trivial
implementation, which just takes a configuration setting and rot13's

it

before supplying the result as the passphrase.

It seems to me that it would be a lot better to have an example in
contrib that does something which might be of actual use to users,
such as running a shell command and reading the passphrase from
stdout.

Features that are only accessible by writing C code are, in general,
not as desirable as features which can be accessed via SQL or
configuration.

Well, I tried to provide the most trivial and simple test I could come
up with. Running a shell command can already be accomplished via the
ssl_passphrase_command setting.

What is the value of a shared library over a shell command?

For one, platforms where shell commands are a lot less convenient, such as
Windows.

We had this
discussion in relation to archive_command years ago, and decided on a
shell command as the best API.

I don't recall that from back then, but that was a long time ago.

But it's interesting that you mention it, given the number of people I have
been discussing that with recently, under the topic of changing it from a
shell command into a shared library API (with there being a shell command
as one possible implementation of course).

One of the main reasons there being to be easily able to transfer more
state and give results other than just an exit code, no need to deal with
parameter escaping etc. Which probably wouldn't matter as much to an SSL
passphrase command, but still.

//Magnus

On 11/7/19 12:30 PM, Andrew Dunstan wrote:

On 11/4/19 4:43 PM, Thomas Munro wrote:

It looks like the new declarations in libpq-be.h are ifdef'd out in a
non-USE_SSL build, but then we still try to build the new test module
and it fails:

https://ci.appveyor.com/project/postgresql-cfbot/postgresql/build/1.0.64071

I think this updated patch should fix things.

This time with a typo fixed to keep the cfbot happy.

cheers

andrew

--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Sun, Nov 10, 2019 at 01:01:17PM -0600, Magnus Hagander wrote:

On Wed, Nov 6, 2019 at 7:24 PM Bruce Momjian <bruce@momjian.us> wrote:
� We had this
discussion in relation to archive_command years ago, and decided on a
shell command as the best API.

I don't recall that from back then, but that was a long time ago.

But it's interesting that you mention it, given the number of people I have
been discussing that with recently, under the topic of changing it from a shell
command into a shared library API (with there being a shell command as one
possible implementation of course).�

One of the main reasons there being to be easily able to transfer more state
and give results other than just an exit code, no need to deal with parameter
escaping etc. Which probably wouldn't matter as much to an SSL passphrase
command, but still.

I get the callback-is-easier issue with shared objects, but are we
expecting to pass in more information here than we do for
archive_command? I would think not. What I am saying is that if we
don't think passing things in works, we should fix all these external
commands, or something. I don't see why ssl_passphrase_command is
different, except that it is new. Or is it related to _securely_
passing something?

Also, why was this patch posted without any discussion of these issues?
Shouldn't we ideally discuss the API first?

--
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. +
+                      Ancient Roman grave inscription +

On Tue, Nov 12, 2019 at 09:51:33PM -0500, Bruce Momjian wrote:

On Sun, Nov 10, 2019 at 01:01:17PM -0600, Magnus Hagander wrote:

On Wed, Nov 6, 2019 at 7:24 PM Bruce Momjian <bruce@momjian.us> wrote:
� We had this
discussion in relation to archive_command years ago, and decided on a
shell command as the best API.

I don't recall that from back then, but that was a long time ago.

But it's interesting that you mention it, given the number of people I have
been discussing that with recently, under the topic of changing it from a shell
command into a shared library API (with there being a shell command as one
possible implementation of course).�

One of the main reasons there being to be easily able to transfer more state
and give results other than just an exit code, no need to deal with parameter
escaping etc. Which probably wouldn't matter as much to an SSL passphrase
command, but still.

I get the callback-is-easier issue with shared objects, but are we
expecting to pass in more information here than we do for
archive_command? I would think not. What I am saying is that if we
don't think passing things in works, we should fix all these external
commands, or something. I don't see why ssl_passphrase_command is
different, except that it is new. Or is it related to _securely_
passing something?

Also, why was this patch posted without any discussion of these issues?
Shouldn't we ideally discuss the API first?

I wonder if every GUC that takes an OS command should allow a shared
object to be specified --- maybe control that if the command string
starts with a # or something.

--
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. +
+                      Ancient Roman grave inscription +

On Wed, 13 Nov 2019 at 13:08, Bruce Momjian <bruce@momjian.us> wrote:

On Tue, Nov 12, 2019 at 09:51:33PM -0500, Bruce Momjian wrote:

On Sun, Nov 10, 2019 at 01:01:17PM -0600, Magnus Hagander wrote:

On Wed, Nov 6, 2019 at 7:24 PM Bruce Momjian <bruce@momjian.us> wrote:

One of the main reasons there being to be easily able to transfer more

state

and give results other than just an exit code, no need to deal with

parameter

escaping etc. Which probably wouldn't matter as much to an SSL

passphrase

command, but still.

I get the callback-is-easier issue with shared objects, but are we
expecting to pass in more information here than we do for
archive_command? I would think not. What I am saying is that if we
don't think passing things in works, we should fix all these external
commands, or something. I don't see why ssl_passphrase_command is
different, except that it is new.

Or is it related to _securely_passing something?

Yes

Also, why was this patch posted without any discussion of these issues?
Shouldn't we ideally discuss the API first?

I wonder if every GUC that takes an OS command should allow a shared
object to be specified --- maybe control that if the command string
starts with a # or something.

Very good idea

--
Simon Riggs http://www.2ndQuadrant.com/
<http://www.2ndquadrant.com/&gt;
PostgreSQL Solutions for the Enterprise

On 11/13/19 8:08 AM, Bruce Momjian wrote:

Also, why was this patch posted without any discussion of these issues?
Shouldn't we ideally discuss the API first?

This is a very tiny patch. I don't think it's unusual to post such
things without prior discussion. I would agree with your point if it
were thousands of lines instead of 20 or so lines of core code.

cheers

andrew

--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Wed, Nov 13, 2019 at 01:20:43PM +0000, Simon Riggs wrote:

On Wed, 13 Nov 2019 at 13:08, Bruce Momjian <bruce@momjian.us> wrote:

On Tue, Nov 12, 2019 at 09:51:33PM -0500, Bruce Momjian wrote:

On Sun, Nov 10, 2019 at 01:01:17PM -0600, Magnus Hagander wrote:

On Wed, Nov 6, 2019 at 7:24 PM Bruce Momjian <bruce@momjian.us> wrote:

One of the main reasons there being to be easily able to transfer more

state

and give results other than just an exit code, no need to deal with

parameter

escaping etc. Which probably wouldn't matter as much to an SSL

passphrase

command, but still.

I get the callback-is-easier issue with shared objects, but are we
expecting to pass in more information here than we do for
archive_command? I would think not. What I am saying is that if we
don't think passing things in works, we should fix all these external
commands, or something. I don't see why ssl_passphrase_command is
different, except that it is new.

Or is it related to _securely_passing something?

Yes

I think it would be beneficial to explain why shared object is more
secure than an OS command. Perhaps it's common knowledge, but it's not
quite obvious to me.

Also, why was this patch posted without any discussion of these issues?
Shouldn't we ideally discuss the API first?

I wonder if every GUC that takes an OS command should allow a shared
object to be specified --- maybe control that if the command string
starts with a # or something.

Very good idea

If it's about securely passing sensitive information (i.e. passphrase)
as was suggested, then I think that only applies to fairly small number
of GUCs.

regards

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

On Wed, Nov 13, 2019 at 02:48:01PM -0500, Andrew Dunstan wrote:

On 11/13/19 8:08 AM, Bruce Momjian wrote:

Also, why was this patch posted without any discussion of these issues?
Shouldn't we ideally discuss the API first?

This is a very tiny patch. I don't think it's unusual to post such
things without prior discussion. I would agree with your point if it
were thousands of lines instead of 20 or so lines of core code.

Not sure that's really true. I think patches should provide some basic
explanation why the feature is desirable, no matter the number of lines.

Also, there were vague references to issues with passing parameters to
archive_command. A link to details, past discussion, or brief
explanation would be nice.

regards

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

On Wed, Nov 13, 2019 at 9:23 PM Tomas Vondra <tomas.vondra@2ndquadrant.com>
wrote:

On Wed, Nov 13, 2019 at 01:20:43PM +0000, Simon Riggs wrote:

On Wed, 13 Nov 2019 at 13:08, Bruce Momjian <bruce@momjian.us> wrote:

On Tue, Nov 12, 2019 at 09:51:33PM -0500, Bruce Momjian wrote:

On Sun, Nov 10, 2019 at 01:01:17PM -0600, Magnus Hagander wrote:

On Wed, Nov 6, 2019 at 7:24 PM Bruce Momjian <bruce@momjian.us>

wrote:

One of the main reasons there being to be easily able to transfer

more

state

and give results other than just an exit code, no need to deal with

parameter

escaping etc. Which probably wouldn't matter as much to an SSL

passphrase

command, but still.

I get the callback-is-easier issue with shared objects, but are we
expecting to pass in more information here than we do for
archive_command? I would think not. What I am saying is that if we
don't think passing things in works, we should fix all these external
commands, or something. I don't see why ssl_passphrase_command is
different, except that it is new.

Or is it related to _securely_passing something?

Yes

I think it would be beneficial to explain why shared object is more
secure than an OS command. Perhaps it's common knowledge, but it's not
quite obvious to me.

Yeah, that probably wouldn't hurt. It's also securely passing from more
than one perspective -- both from the "cannot be eavesdropped" (like
putting the password on the commandline for example) and the requirement
for escaping.

Also, why was this patch posted without any discussion of these

issues?

Shouldn't we ideally discuss the API first?

I wonder if every GUC that takes an OS command should allow a shared
object to be specified --- maybe control that if the command string
starts with a # or something.

Very good idea

If it's about securely passing sensitive information (i.e. passphrase)
as was suggested, then I think that only applies to fairly small number
of GUCs.

There aren't exactly a large number of GUCs that take OS commands in total.
Consistency itself certainly has some value.

--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/&gt;
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/&gt;

On Wed, Nov 13, 2019 at 3:23 PM Tomas Vondra
<tomas.vondra@2ndquadrant.com> wrote:

I think it would be beneficial to explain why shared object is more
secure than an OS command. Perhaps it's common knowledge, but it's not
quite obvious to me.

External command args can be viewed by other OS users (not just the
postgres user). For non-sensitive arguments (ex: WAL filename?) that's
not an issue but if you plan on passing in something potentially
secret value from the backend to the external OS command, that value
would be exposed:

Terminal 1 (run a command as some other user):
$ sudo -u nobody sleep 5

Terminal 2 (view command args as a different non-super user):
$ ps -u nobody -o args
COMMAND
sleep 5

A shared library would not have this problem as the backend directly
executes the library in the same process.

Has the idea of using environment variables (rather than command line
args) for external commands been brought up before? I couldn't find
anything in the mailing list archives.

Environment variables have the advantage of only being readable by the
process owner and super user. They also naturally have a "name" and do
not have escaping or quoting issues.

For example, archive_command %p could be exposed as
PG_ARG_ARCHIVE_PATH and %f could be exposed as
PG_ARG_ARCHIVE_FILENAME. Then you could have a script use them via:

#!/usr/bin/env bash
set -euo pipefail
main () {
local archive_dir="/path/to/archive_dir/"
local archive_file="${archive_dir}${PG_ARG_ARCHIVE_FILENAME}"
test ! -f "${archive_file}" && cp -- "${PG_ARG_ARCHIVE_PATH}"
"${archive_file}"
}
main "$@"

It's not particularly useful for that basic archive case but if
there's something like PG_ARG_SUPER_SECRET then the executed command
could receive that value without it being exposed. That'd be useful
for something like a callout to an external KMS (key management
system).

Nothing stops something like this with being used in tandem with
string substitution to create the full commands. That'd give backwards
compatibility too. The main limitation compared to a shared library is
that you'd still have to explicitly pick and name the exposed argument
environment variables (i.e. like picking the set of %? substitutions).
If there's a generic shared-library-for-external-commands approach
then there could be a built-in library that provides this type of
"expose as env vars" functionality.

Regards,
-- Sehrope Sarkuni
Founder & CEO | JackDB, Inc. | https://www.jackdb.com/

On Thu, Nov 14, 2019 at 11:42:05AM +0100, Magnus Hagander wrote:

On Wed, Nov 13, 2019 at 9:23 PM Tomas Vondra <tomas.vondra@2ndquadrant.com>
I think it would be beneficial to explain why shared object is more
secure than an OS command. Perhaps it's common knowledge, but it's not
quite obvious to me.

Yeah, that probably wouldn't hurt. It's also securely passing from more than
one perspective -- both from the "cannot be eavesdropped" (like putting the
password on the commandline for example) and the requirement for escaping.

I think a bigger issue is that if you want to give people the option of
using a shell command or a shared object, and if you use two commands to
control it, it isn't clear what happens if both are defined. By using
some character prefix to control if a shared object is used, you can use
a single variable and there is no confusion over having two variables
and their conflicting behavior.

--
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. +
+                      Ancient Roman grave inscription +

On 11/14/19 11:07 AM, Bruce Momjian wrote:

On Thu, Nov 14, 2019 at 11:42:05AM +0100, Magnus Hagander wrote:

On Wed, Nov 13, 2019 at 9:23 PM Tomas Vondra <tomas.vondra@2ndquadrant.com>
I think it would be beneficial to explain why shared object is more
secure than an OS command. Perhaps it's common knowledge, but it's not
quite obvious to me.

Yeah, that probably wouldn't hurt. It's also securely passing from more than
one perspective -- both from the "cannot be eavesdropped" (like putting the
password on the commandline for example) and the requirement for escaping.

I think a bigger issue is that if you want to give people the option of
using a shell command or a shared object, and if you use two commands to
control it, it isn't clear what happens if both are defined. By using
some character prefix to control if a shared object is used, you can use
a single variable and there is no confusion over having two variables
and their conflicting behavior.

I'm  not sure how that would work in the present instance. The shared
preloaded module installs a function and defines the params it wants. If
we somehow unify the params with ssl_passphrase_command that could look
icky, and the module would have to parse the settings string. That's not
a problem for the sample module which only needs one param, but it will
be for other more complex implementations.

I'm quite open to suggestions, but I want things to be tolerably clean.

cheers

andrew

--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services