This is the first of a number of patches to enhance SSL functionality, particularly w.r.t. passphrases. 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. cheers andrew -- Andrew Dunstan https://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services |
On Thu, Oct 31, 2019 at 11:37 AM Andrew Dunstan
<[hidden email]> 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 > <[hidden email]> 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
<[hidden email]> wrote: > On 11/1/19 11:01 AM, Robert Haas wrote: > > On Thu, Oct 31, 2019 at 11:37 AM Andrew Dunstan > > <[hidden email]> 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 |
In reply to this post by Andrew Dunstan-8
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 > > <[hidden email]> 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 <[hidden email]> 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 + |
In reply to this post by Thomas Munro-5
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 |
In reply to this post by Bruce Momjian
On Thu, 7 Nov 2019 at 10:24, Bruce Momjian <[hidden email]> wrote: What is the value of a shared library over a shell command? We had this 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 |
On Fri, Nov 08, 2019 at 11:12:08PM +0900, Simon Riggs wrote:
>On Thu, 7 Nov 2019 at 10:24, Bruce Momjian <[hidden email]> 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 |
In reply to this post by Bruce Momjian
On Wed, Nov 6, 2019 at 7:24 PM Bruce Momjian <[hidden email]> wrote: On Fri, Nov 1, 2019 at 01:57:29PM -0400, Andrew Dunstan wrote: For one, platforms where shell commands are a lot less convenient, such as Windows. We had this 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 |
In reply to this post by Andrew Dunstan-8
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 |
In reply to this post by Magnus Hagander-2
On Sun, Nov 10, 2019 at 01:01:17PM -0600, Magnus Hagander wrote:
> On Wed, Nov 6, 2019 at 7:24 PM Bruce Momjian <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> wrote: On Tue, Nov 12, 2019 at 09:51:33PM -0500, Bruce Momjian wrote: Or is it related to _securely_passing something? Yes > Also, why was this patch posted without any discussion of these issues? Very good idea |
In reply to this post by Bruce Momjian
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 |
In reply to this post by Simon Riggs
On Wed, Nov 13, 2019 at 01:20:43PM +0000, Simon Riggs wrote:
>On Wed, 13 Nov 2019 at 13:08, Bruce Momjian <[hidden email]> 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 <[hidden email]> 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 |
In reply to this post by Andrew Dunstan-8
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 |
In reply to this post by Tomas Vondra-4
On Wed, Nov 13, 2019 at 9:23 PM Tomas Vondra <[hidden email]> wrote: On Wed, Nov 13, 2019 at 01:20:43PM +0000, Simon Riggs wrote: 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.
There aren't exactly a large number of GUCs that take OS commands in total. Consistency itself certainly has some value. |
In reply to this post by Tomas Vondra-4
On Wed, Nov 13, 2019 at 3:23 PM Tomas Vondra
<[hidden email]> 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/ |
In reply to this post by Magnus Hagander-2
On Thu, Nov 14, 2019 at 11:42:05AM +0100, Magnus Hagander wrote:
> On Wed, Nov 13, 2019 at 9:23 PM Tomas Vondra <[hidden email]> > 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 <[hidden email]> 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 <[hidden email]> >> 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 |
Free forum by Nabble | Edit this page |