Exposure related to GUC value of ssl_passphrase_command

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

Exposure related to GUC value of ssl_passphrase_command

Moon, Insung-2
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.

Change-show-authority-of-ssl_passphrase_command.diff (628 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Exposure related to GUC value of ssl_passphrase_command

Amit Langote
Hello.

On Tue, Nov 5, 2019 at 5:15 PM Moon, Insung <[hidden email]> 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


Reply | Threaded
Open this post in threaded view
|

Re: Exposure related to GUC value of ssl_passphrase_command

Fujii Masao-2
On Fri, Nov 8, 2019 at 4:24 PM Amit Langote <[hidden email]> wrote:

>
> Hello.
>
> On Tue, Nov 5, 2019 at 5:15 PM Moon, Insung <[hidden email]> 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


Reply | Threaded
Open this post in threaded view
|

Re: Exposure related to GUC value of ssl_passphrase_command

Kyotaro Horiguchi-4
At Thu, 13 Feb 2020 02:37:29 +0900, Fujii Masao <[hidden email]> wrote in

> On Fri, Nov 8, 2019 at 4:24 PM Amit Langote <[hidden email]> wrote:
> >
> > Hello.
> >
> > On Tue, Nov 5, 2019 at 5:15 PM Moon, Insung <[hidden email]> 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


Reply | Threaded
Open this post in threaded view
|

Re: Exposure related to GUC value of ssl_passphrase_command

Michael Paquier-2
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

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Exposure related to GUC value of ssl_passphrase_command

Peter Eisentraut-6
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


Reply | Threaded
Open this post in threaded view
|

Re: Exposure related to GUC value of ssl_passphrase_command

Moon, Insung-2
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
<[hidden email]> 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
>
>


Reply | Threaded
Open this post in threaded view
|

Re: Exposure related to GUC value of ssl_passphrase_command

keisuke kuroda
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
Reply | Threaded
Open this post in threaded view
|

Re: Exposure related to GUC value of ssl_passphrase_command

Fujii Masao-4
In reply to this post by Moon, Insung-2


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


Reply | Threaded
Open this post in threaded view
|

Re: Exposure related to GUC value of ssl_passphrase_command

Fujii Masao-4
In reply to this post by keisuke kuroda


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


Reply | Threaded
Open this post in threaded view
|

Re: Exposure related to GUC value of ssl_passphrase_command

Moon, Insung-2
In reply to this post by Fujii Masao-4
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 <[hidden email]> 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