Horton

Thread

Commits

  1. Mark ssl_passphrase_command as GUC_SUPERUSER_ONLY.

  1. Exposure related to GUC value of ssl_passphrase_command

    Moon, Insung <tsukiwamoon.pgsql@gmail.com> — 2019-11-05T08:14:41Z

    Deal Hackers.

The value of ssl_passphrase_command is set so that an external command
is called when the passphrase for decrypting an SSL file such as a
private key is obtained.
Therefore, easily set to work with echo "passphrase" or call to
another get of passphrase application.

I think that this GUC value doesn't contain very sensitive data,
but just in case, it's dangerous to be visible to all users.
I think do not possible these cases, but if a used echo external
commands or another external command,  know what application used to
get the password, maybe we can't be convinced that there's the safety
of using abuse by backtracking on applications.
So I think to the need only superusers or users with the default role
of pg_read_all_settings should see these values.

Patch is very simple.
How do you think about my thoughts like this?

Best regards.
Moon.

  2. Re: Exposure related to GUC value of ssl_passphrase_command

    Amit Langote <amitlangote09@gmail.com> — 2019-11-08T07:24:10Z

    Hello.

On Tue, Nov 5, 2019 at 5:15 PM Moon, Insung <tsukiwamoon.pgsql@gmail.com> wrote:
> Deal Hackers.
>
> The value of ssl_passphrase_command is set so that an external command
> is called when the passphrase for decrypting an SSL file such as a
> private key is obtained.
> Therefore, easily set to work with echo "passphrase" or call to
> another get of passphrase application.
>
> I think that this GUC value doesn't contain very sensitive data,
> but just in case, it's dangerous to be visible to all users.
> I think do not possible these cases, but if a used echo external
> commands or another external command,  know what application used to
> get the password, maybe we can't be convinced that there's the safety
> of using abuse by backtracking on applications.
> So I think to the need only superusers or users with the default role
> of pg_read_all_settings should see these values.
>
> Patch is very simple.
> How do you think about my thoughts like this?

I'm hardly an expert on this topic, but reading this blog post about
ssl_passphrase_command:

https://www.2ndquadrant.com/en/blog/postgresql-passphrase-protected-ssl-keys-systemd/

which mentions that some users might go with the very naive
configuration such as:

ssl_passphrase_command = 'echo "secret"'

maybe it makes sense to protect its value from everyone but superusers.

So +1.

Thanks,
Amit

  3. Re: Exposure related to GUC value of ssl_passphrase_command

    Fujii Masao <masao.fujii@gmail.com> — 2020-02-12T17:37:29Z

    On Fri, Nov 8, 2019 at 4:24 PM Amit Langote <amitlangote09@gmail.com> wrote:
>
> Hello.
>
> On Tue, Nov 5, 2019 at 5:15 PM Moon, Insung <tsukiwamoon.pgsql@gmail.com> wrote:
> > Deal Hackers.
> >
> > The value of ssl_passphrase_command is set so that an external command
> > is called when the passphrase for decrypting an SSL file such as a
> > private key is obtained.
> > Therefore, easily set to work with echo "passphrase" or call to
> > another get of passphrase application.
> >
> > I think that this GUC value doesn't contain very sensitive data,
> > but just in case, it's dangerous to be visible to all users.
> > I think do not possible these cases, but if a used echo external
> > commands or another external command,  know what application used to
> > get the password, maybe we can't be convinced that there's the safety
> > of using abuse by backtracking on applications.
> > So I think to the need only superusers or users with the default role
> > of pg_read_all_settings should see these values.
> >
> > Patch is very simple.
> > How do you think about my thoughts like this?
>
> I'm hardly an expert on this topic, but reading this blog post about
> ssl_passphrase_command:
>
> https://www.2ndquadrant.com/en/blog/postgresql-passphrase-protected-ssl-keys-systemd/
>
> which mentions that some users might go with the very naive
> configuration such as:
>
> ssl_passphrase_command = 'echo "secret"'
>
> maybe it makes sense to protect its value from everyone but superusers.
>
> So +1.

Seems this proposal is reasonable.

Regards,

-- 
Fujii Masao

  4. Re: Exposure related to GUC value of ssl_passphrase_command

    Kyotaro Horiguchi <horikyota.ntt@gmail.com> — 2020-02-13T02:28:05Z

    At Thu, 13 Feb 2020 02:37:29 +0900, Fujii Masao <masao.fujii@gmail.com> wrote in 
> On Fri, Nov 8, 2019 at 4:24 PM Amit Langote <amitlangote09@gmail.com> wrote:
> >
> > Hello.
> >
> > On Tue, Nov 5, 2019 at 5:15 PM Moon, Insung <tsukiwamoon.pgsql@gmail.com> wrote:
> > > Deal Hackers.
> > >
> > > The value of ssl_passphrase_command is set so that an external command
> > > is called when the passphrase for decrypting an SSL file such as a
> > > private key is obtained.
> > > Therefore, easily set to work with echo "passphrase" or call to
> > > another get of passphrase application.
> > >
> > > I think that this GUC value doesn't contain very sensitive data,
> > > but just in case, it's dangerous to be visible to all users.
> > > I think do not possible these cases, but if a used echo external
> > > commands or another external command,  know what application used to
> > > get the password, maybe we can't be convinced that there's the safety
> > > of using abuse by backtracking on applications.
> > > So I think to the need only superusers or users with the default role
> > > of pg_read_all_settings should see these values.
> > >
> > > Patch is very simple.
> > > How do you think about my thoughts like this?
> >
> > I'm hardly an expert on this topic, but reading this blog post about
> > ssl_passphrase_command:
> >
> > https://www.2ndquadrant.com/en/blog/postgresql-passphrase-protected-ssl-keys-systemd/
> >
> > which mentions that some users might go with the very naive
> > configuration such as:
> >
> > ssl_passphrase_command = 'echo "secret"'
> >
> > maybe it makes sense to protect its value from everyone but superusers.
> >
> > So +1.
> 
> Seems this proposal is reasonable.

I think it is reasonable.

By the way, I'm not sure the criteria of setting a GUC variable as
GUC_SUPERUSER_ONLY, but for example, ssl_max/min_protocol_version,
dynamic_library_path, log_directory, krb_server_keyfile,
data_directory and config_file are GUC_SUPERUSER_ONLY. So, it seems to
me very strange that ssl_*_file are not. Don't we need to mark them
maybe and some of the other ssl_* as the same?

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

  5. Re: Exposure related to GUC value of ssl_passphrase_command

    Michael Paquier <michael@paquier.xyz> — 2020-02-13T03:38:34Z

    On Thu, Feb 13, 2020 at 11:28:05AM +0900, Kyotaro Horiguchi wrote:
> I think it is reasonable.

Indeed, that makes sense to me as well.  I am adding Peter Eisentraut
in CC as the author/committer of 8a3d942 to comment on that.

> By the way, I'm not sure the criteria of setting a GUC variable as
> GUC_SUPERUSER_ONLY, but for example, ssl_max/min_protocol_version,
> dynamic_library_path, log_directory, krb_server_keyfile,
> data_directory and config_file are GUC_SUPERUSER_ONLY. So, it seems to
> me very strange that ssl_*_file are not. Don't we need to mark them
> maybe and some of the other ssl_* as the same?

This should be a separate discussion IMO.  Perhaps there is a point in
softening or hardening some of them.
--
Michael

  6. Re: Exposure related to GUC value of ssl_passphrase_command

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2020-02-13T09:11:07Z

    On 2020-02-13 04:38, Michael Paquier wrote:
> On Thu, Feb 13, 2020 at 11:28:05AM +0900, Kyotaro Horiguchi wrote:
>> I think it is reasonable.
> 
> Indeed, that makes sense to me as well.  I am adding Peter Eisentraut
> in CC as the author/committer of 8a3d942 to comment on that.

I'm OK with changing that.

>> By the way, I'm not sure the criteria of setting a GUC variable as
>> GUC_SUPERUSER_ONLY, but for example, ssl_max/min_protocol_version,
>> dynamic_library_path, log_directory, krb_server_keyfile,
>> data_directory and config_file are GUC_SUPERUSER_ONLY. So, it seems to
>> me very strange that ssl_*_file are not. Don't we need to mark them
>> maybe and some of the other ssl_* as the same?
> 
> This should be a separate discussion IMO.  Perhaps there is a point in
> softening or hardening some of them.

I think some of this makes sense, and we should have a discussion about it.

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

  7. Re: Exposure related to GUC value of ssl_passphrase_command

    Moon, Insung <tsukiwamoon.pgsql@gmail.com> — 2020-02-14T01:31:45Z

    Dear Hackers.

Thank you for an response.
I registered this entry in commifest of 2020-03.
# I registered in the security part, but if it is wrong, sincerely
apologize for this.

And I'd like to review show authority to ssl_ * later and discuss it
in a separate thread.

Best regards.
Moon.

On Thu, Feb 13, 2020 at 6:11 PM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
>
> On 2020-02-13 04:38, Michael Paquier wrote:
> > On Thu, Feb 13, 2020 at 11:28:05AM +0900, Kyotaro Horiguchi wrote:
> >> I think it is reasonable.
> >
> > Indeed, that makes sense to me as well.  I am adding Peter Eisentraut
> > in CC as the author/committer of 8a3d942 to comment on that.
>
> I'm OK with changing that.
>
> >> By the way, I'm not sure the criteria of setting a GUC variable as
> >> GUC_SUPERUSER_ONLY, but for example, ssl_max/min_protocol_version,
> >> dynamic_library_path, log_directory, krb_server_keyfile,
> >> data_directory and config_file are GUC_SUPERUSER_ONLY. So, it seems to
> >> me very strange that ssl_*_file are not. Don't we need to mark them
> >> maybe and some of the other ssl_* as the same?
> >
> > This should be a separate discussion IMO.  Perhaps there is a point in
> > softening or hardening some of them.
>
> I think some of this makes sense, and we should have a discussion about it.
>
> --
> Peter Eisentraut              http://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>
>

  8. Re: Exposure related to GUC value of ssl_passphrase_command

    Keisuke Kuroda <keisuke.kuroda.3862@gmail.com> — 2020-03-06T07:20:21Z

    The following review has been posted through the commitfest application:
make installcheck-world:  tested, passed
Implements feature:       tested, passed
Spec compliant:           tested, passed
Documentation:            not tested

I tested the patch on the master branch (addd034) and it works fine.

I think that test case which non-superuser can't see this parameter is unnecessary. 
There is a similar test for pg_read_all_settings role.

The new status of this patch is: Ready for Committer

  9. Re: Exposure related to GUC value of ssl_passphrase_command

    Fujii Masao <masao.fujii@oss.nttdata.com> — 2020-03-09T02:43:20Z

    
On 2020/02/14 10:31, Moon, Insung wrote:
> Dear Hackers.
> 
> Thank you for an response.
> I registered this entry in commifest of 2020-03.
> # I registered in the security part, but if it is wrong, sincerely
> apologize for this.
> 
> And I'd like to review show authority to ssl_ * later and discuss it
> in a separate thread.

So, you are planning to start new discussion about this?

Regards,

-- 
Fujii Masao
NTT DATA CORPORATION
Advanced Platform Technology Group
Research and Development Headquarters

  10. Re: Exposure related to GUC value of ssl_passphrase_command

    Fujii Masao <masao.fujii@oss.nttdata.com> — 2020-03-09T02:43:52Z

    
On 2020/03/06 16:20, keisuke kuroda wrote:
> The following review has been posted through the commitfest application:
> make installcheck-world:  tested, passed
> Implements feature:       tested, passed
> Spec compliant:           tested, passed
> Documentation:            not tested
> 
> I tested the patch on the master branch (addd034) and it works fine.
> 
> I think that test case which non-superuser can't see this parameter is unnecessary.
> There is a similar test for pg_read_all_settings role.
> 
> The new status of this patch is: Ready for Committer

Pushed! Thanks!

Regards,


-- 
Fujii Masao
NTT DATA CORPORATION
Advanced Platform Technology Group
Research and Development Headquarters

  11. Re: Exposure related to GUC value of ssl_passphrase_command

    Moon, Insung <tsukiwamoon.pgsql@gmail.com> — 2020-03-09T05:23:53Z

    Dear Kuroda-san, Fujii-san
Thank you for review and commit!
#Oops.. Sorry..This mail thread has been spammed in Gmail.

I'll go to submit a new discussion after found which case could leak
about the GUC parameters related to ssl_*.
Please wait a bit.

Best regards.
Moon.

On Mon, Mar 9, 2020 at 11:43 AM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
>
>
>
> On 2020/02/14 10:31, Moon, Insung wrote:
> > Dear Hackers.
> >
> > Thank you for an response.
> > I registered this entry in commifest of 2020-03.
> > # I registered in the security part, but if it is wrong, sincerely
> > apologize for this.
> >
> > And I'd like to review show authority to ssl_ * later and discuss it
> > in a separate thread.
>
> So, you are planning to start new discussion about this?
>
> Regards,
>
> --
> Fujii Masao
> NTT DATA CORPORATION
> Advanced Platform Technology Group
> Research and Development Headquarters