Re: Exposure related to GUC value of ssl_passphrase_command
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 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
Re: Exposure related to GUC value of ssl_passphrase_command
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
Re: Exposure related to GUC value of ssl_passphrase_command
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
Re: Exposure related to GUC value of ssl_passphrase_command
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
Re: Exposure related to GUC value of ssl_passphrase_command
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 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 > >
Re: Exposure related to GUC value of ssl_passphrase_command
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
Re: Exposure related to GUC value of ssl_passphrase_command
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 Description: PGP signature
Re: Exposure related to GUC value of ssl_passphrase_command
At Thu, 13 Feb 2020 02:37:29 +0900, Fujii Masao wrote in > On Fri, Nov 8, 2019 at 4:24 PM Amit Langote wrote: > > > > Hello. > > > > On Tue, Nov 5, 2019 at 5:15 PM Moon, Insung > > 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
Re: Exposure related to GUC value of ssl_passphrase_command
On Fri, Nov 8, 2019 at 4:24 PM Amit Langote wrote: > > Hello. > > On Tue, Nov 5, 2019 at 5:15 PM Moon, Insung > 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
Re: Exposure related to GUC value of ssl_passphrase_command
Hello. On Tue, Nov 5, 2019 at 5:15 PM Moon, Insung 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
