Thread: libpq sslpassword parameter and callback function
This patch provides for an sslpassword parameter for libpq, and a hook that a client can fill in for a callback function to set the password. This provides similar facilities to those already available in the JDBC driver. There is also a function to fetch the sslpassword from the connection parameters, in the same way that other settings can be fetched. This is mostly the excellent work of my colleague Craig Ringer, with a few embellishments from me. Here are his notes: Allow libpq to non-interactively decrypt client certificates that are stored encrypted by adding a new "sslpassword" connection option. The sslpassword option offers a middle ground between a cleartext key and setting up advanced key mangement via openssl engines, PKCS#11, USB crypto offload and key escrow, etc. Previously use of encrypted client certificate keys only worked if the user could enter the key's password interactively on stdin, in response to openssl's default prompt callback: Enter PEM passhprase: That's infesible in many situations, especially things like use from postgres_fdw. This change also allows admins to prevent libpq from ever prompting for a password by calling: PQsetSSLKeyPassHook(PQdefaultSSLKeyPassHook); which is useful since OpenSSL likes to open /dev/tty to prompt for a password, so even closing stdin won't stop it blocking if there's no user input available. Applications may also override or extend SSL password fetching with their own callback. There is deliberately no environment variable equivalent for the sslpassword option. cheers andrew -- Andrew Dunstan https://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
This time with attachment. On 10/31/19 6:33 PM, Andrew Dunstan wrote: > This patch provides for an sslpassword parameter for libpq, and a hook > that a client can fill in for a callback function to set the password. > > > This provides similar facilities to those already available in the JDBC > driver. > > > There is also a function to fetch the sslpassword from the connection > parameters, in the same way that other settings can be fetched. > > > This is mostly the excellent work of my colleague Craig Ringer, with a > few embellishments from me. > > > Here are his notes: > > > Allow libpq to non-interactively decrypt client certificates that > are stored > encrypted by adding a new "sslpassword" connection option. > > The sslpassword option offers a middle ground between a cleartext > key and > setting up advanced key mangement via openssl engines, PKCS#11, USB > crypto > offload and key escrow, etc. > > Previously use of encrypted client certificate keys only worked if > the user > could enter the key's password interactively on stdin, in response > to openssl's > default prompt callback: > > Enter PEM passhprase: > > That's infesible in many situations, especially things like use from > postgres_fdw. > > This change also allows admins to prevent libpq from ever prompting > for a > password by calling: > > PQsetSSLKeyPassHook(PQdefaultSSLKeyPassHook); > > which is useful since OpenSSL likes to open /dev/tty to prompt for a > password, > so even closing stdin won't stop it blocking if there's no user > input available. > Applications may also override or extend SSL password fetching with > their own > callback. > > There is deliberately no environment variable equivalent for the > sslpassword > option. > > > cheers > > > andrew > > -- Andrew Dunstan https://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachment
On 10/31/19 6:34 PM, Andrew Dunstan wrote: > This time with attachment. > > > On 10/31/19 6:33 PM, Andrew Dunstan wrote: >> This patch provides for an sslpassword parameter for libpq, and a hook >> that a client can fill in for a callback function to set the password. >> >> >> This provides similar facilities to those already available in the JDBC >> driver. >> >> >> There is also a function to fetch the sslpassword from the connection >> parameters, in the same way that other settings can be fetched. >> >> >> This is mostly the excellent work of my colleague Craig Ringer, with a >> few embellishments from me. >> >> >> Here are his notes: >> >> >> Allow libpq to non-interactively decrypt client certificates that >> are stored >> encrypted by adding a new "sslpassword" connection option. >> >> The sslpassword option offers a middle ground between a cleartext >> key and >> setting up advanced key mangement via openssl engines, PKCS#11, USB >> crypto >> offload and key escrow, etc. >> >> Previously use of encrypted client certificate keys only worked if >> the user >> could enter the key's password interactively on stdin, in response >> to openssl's >> default prompt callback: >> >> Enter PEM passhprase: >> >> That's infesible in many situations, especially things like use from >> postgres_fdw. >> >> This change also allows admins to prevent libpq from ever prompting >> for a >> password by calling: >> >> PQsetSSLKeyPassHook(PQdefaultSSLKeyPassHook); >> >> which is useful since OpenSSL likes to open /dev/tty to prompt for a >> password, >> so even closing stdin won't stop it blocking if there's no user >> input available. >> Applications may also override or extend SSL password fetching with >> their own >> callback. >> >> There is deliberately no environment variable equivalent for the >> sslpassword >> option. >> >> I should also mention that this patch provides for support for DER format certificates and keys. cheers andrew -- Andrew Dunstan https://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Fri, 1 Nov 2019 at 07:27, Andrew Dunstan <andrew.dunstan@2ndquadrant.com> wrote:
On 10/31/19 6:34 PM, Andrew Dunstan wrote:
> This time with attachment.
>
>
> On 10/31/19 6:33 PM, Andrew Dunstan wrote:
>> This patch provides for an sslpassword parameter for libpq, and a hook
>> that a client can fill in for a callback function to set the password.
>>
>>
>> This provides similar facilities to those already available in the JDBC
>> driver.
>>
>>
>> There is also a function to fetch the sslpassword from the connection
>> parameters, in the same way that other settings can be fetched.
>>
>>
>> This is mostly the excellent work of my colleague Craig Ringer, with a
>> few embellishments from me.
>>
>>
>> Here are his notes:
>>
>>
>> Allow libpq to non-interactively decrypt client certificates that
>> are stored
>> encrypted by adding a new "sslpassword" connection option.
>>
>> The sslpassword option offers a middle ground between a cleartext
>> key and
>> setting up advanced key mangement via openssl engines, PKCS#11, USB
>> crypto
>> offload and key escrow, etc.
>>
>> Previously use of encrypted client certificate keys only worked if
>> the user
>> could enter the key's password interactively on stdin, in response
>> to openssl's
>> default prompt callback:
>>
>> Enter PEM passhprase:
>>
>> That's infesible in many situations, especially things like use from
>> postgres_fdw.
>>
>> This change also allows admins to prevent libpq from ever prompting
>> for a
>> password by calling:
>>
>> PQsetSSLKeyPassHook(PQdefaultSSLKeyPassHook);
>>
>> which is useful since OpenSSL likes to open /dev/tty to prompt for a
>> password,
>> so even closing stdin won't stop it blocking if there's no user
>> input available.
>> Applications may also override or extend SSL password fetching with
>> their own
>> callback.
>>
>> There is deliberately no environment variable equivalent for the
>> sslpassword
>> option.
>>
>>
I should also mention that this patch provides for support for DER
format certificates and keys.
Yep, that was a trivial change I rolled into it.
FWIW, this is related to two other patches: the patch to allow passwordless fdw connections with explicit superuser approval, and the patch to allow sslkey/sslpassword to be set as user mapping options in postgres_fdw . Together all three patches make it possible to use SSL client certificates to manage authentication in postgres_fdw user mappings.
On Sun, Nov 10, 2019 at 05:47:24PM +0800, Craig Ringer wrote: > Yep, that was a trivial change I rolled into it. > > FWIW, this is related to two other patches: the patch to allow passwordless fdw > connections with explicit superuser approval, and the patch to allow sslkey/ > sslpassword to be set as user mapping options in postgres_fdw . Together all > three patches make it possible to use SSL client certificates to manage > authentication in postgres_fdw user mappings. Oh, nice, greatly needed! -- 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 10/31/19 7:27 PM, Andrew Dunstan wrote: > On 10/31/19 6:34 PM, Andrew Dunstan wrote: >> This time with attachment. >> >> >> On 10/31/19 6:33 PM, Andrew Dunstan wrote: >>> This patch provides for an sslpassword parameter for libpq, and a hook >>> that a client can fill in for a callback function to set the password. >>> >>> >>> This provides similar facilities to those already available in the JDBC >>> driver. >>> >>> >>> There is also a function to fetch the sslpassword from the connection >>> parameters, in the same way that other settings can be fetched. >>> >>> >>> This is mostly the excellent work of my colleague Craig Ringer, with a >>> few embellishments from me. >>> >>> >>> Here are his notes: >>> >>> >>> Allow libpq to non-interactively decrypt client certificates that >>> are stored >>> encrypted by adding a new "sslpassword" connection option. >>> >>> The sslpassword option offers a middle ground between a cleartext >>> key and >>> setting up advanced key mangement via openssl engines, PKCS#11, USB >>> crypto >>> offload and key escrow, etc. >>> >>> Previously use of encrypted client certificate keys only worked if >>> the user >>> could enter the key's password interactively on stdin, in response >>> to openssl's >>> default prompt callback: >>> >>> Enter PEM passhprase: >>> >>> That's infesible in many situations, especially things like use from >>> postgres_fdw. >>> >>> This change also allows admins to prevent libpq from ever prompting >>> for a >>> password by calling: >>> >>> PQsetSSLKeyPassHook(PQdefaultSSLKeyPassHook); >>> >>> which is useful since OpenSSL likes to open /dev/tty to prompt for a >>> password, >>> so even closing stdin won't stop it blocking if there's no user >>> input available. >>> Applications may also override or extend SSL password fetching with >>> their own >>> callback. >>> >>> There is deliberately no environment variable equivalent for the >>> sslpassword >>> option. >>> >>> > I should also mention that this patch provides for support for DER > format certificates and keys. > > Here's an updated version of the patch, adjusted to the now committed changes to TestLib.pm. cheers andrew -- Andrew Dunstan https://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachment
On 11/25/19 4:09 PM, Andrew Dunstan wrote: > On 10/31/19 7:27 PM, Andrew Dunstan wrote: >> On 10/31/19 6:34 PM, Andrew Dunstan wrote: >>> This time with attachment. >>> >>> >>> On 10/31/19 6:33 PM, Andrew Dunstan wrote: >>>> This patch provides for an sslpassword parameter for libpq, and a hook >>>> that a client can fill in for a callback function to set the password. >>>> >>>> >>>> This provides similar facilities to those already available in the JDBC >>>> driver. >>>> >>>> >>>> There is also a function to fetch the sslpassword from the connection >>>> parameters, in the same way that other settings can be fetched. >>>> >>>> >>>> This is mostly the excellent work of my colleague Craig Ringer, with a >>>> few embellishments from me. >>>> >>>> >>>> Here are his notes: >>>> >>>> >>>> Allow libpq to non-interactively decrypt client certificates that >>>> are stored >>>> encrypted by adding a new "sslpassword" connection option. >>>> >>>> The sslpassword option offers a middle ground between a cleartext >>>> key and >>>> setting up advanced key mangement via openssl engines, PKCS#11, USB >>>> crypto >>>> offload and key escrow, etc. >>>> >>>> Previously use of encrypted client certificate keys only worked if >>>> the user >>>> could enter the key's password interactively on stdin, in response >>>> to openssl's >>>> default prompt callback: >>>> >>>> Enter PEM passhprase: >>>> >>>> That's infesible in many situations, especially things like use from >>>> postgres_fdw. >>>> >>>> This change also allows admins to prevent libpq from ever prompting >>>> for a >>>> password by calling: >>>> >>>> PQsetSSLKeyPassHook(PQdefaultSSLKeyPassHook); >>>> >>>> which is useful since OpenSSL likes to open /dev/tty to prompt for a >>>> password, >>>> so even closing stdin won't stop it blocking if there's no user >>>> input available. >>>> Applications may also override or extend SSL password fetching with >>>> their own >>>> callback. >>>> >>>> There is deliberately no environment variable equivalent for the >>>> sslpassword >>>> option. >>>> >>>> >> I should also mention that this patch provides for support for DER >> format certificates and keys. >> >> > > Here's an updated version of the patch, adjusted to the now committed > changes to TestLib.pm. > > Here's an update now we have backed out the TestLib changes. The tests that need a pty are skipped. cheers andrew -- Andrew Dunstan https://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachment
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: tested, passed
Documentation: tested, failed
Hi Andrew,
I've reviewed your "libpq sslpassword parameter and callback function" patch
(0001-libpq-sslpassword-der-support.patch),and only found a few minor things (otherwise it looked good to me):
1) There's a few trailing white-space warnings on patch application (from where you modified to skip 2 of the tests):
git apply 0001-libpq-sslpassword-der-support.patch
0001-libpq-sslpassword-der-support.patch:649: trailing whitespace.
# so they don't hang. For now they are not performed.
0001-libpq-sslpassword-der-support.patch:659: trailing whitespace.
warning: 2 lines add whitespace errors.
2) src/interfaces/libpq/libpq-fe.h
The following portion of the comment should be removed.
+ * 2ndQPostgres extension. If you need to be compatible with unpatched libpq
+ * you must dlsym() these.
3) Documentation for the "PQsslpassword" function should be added to the libpq "33.2 Connection Status Functions"
section.
I made the following notes about how/what I reviewed and tested:
- Applied patch and built Postgres (--with-openssl --enable-tap-tests), checked build output
- Checked patch code modifications (format, logic, memory usage, efficiency, corner cases etc.)
- Built documentation and checked updated portions (format, grammar, details, completeness etc.)
- Checked test updates
- Ran updated contrib/dblink tests - confirmed all passed
- Ran updated SSL (TAP) tests - confirmed all passed
- Ran "make installcheck-world", as per review requirements
- Wrote small libpq-based app to test:
- new APIs (PQsslpassword, PQsetSSLKeyPassHook, PQgetSSLKeyPassHook, PQdefaultSSLKeyPassHook)
- passphrase-protected key with/without patch
- patch with/without new key password callack
- patch and certificate with/without pass phrase protection on key
- default callback, callback delegation
- PEM/DER keys
Regards,
Greg
On 11/28/19 10:25 PM, Greg Nancarrow 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: tested, failed > > Hi Andrew, > > I've reviewed your "libpq sslpassword parameter and callback function" patch (0001-libpq-sslpassword-der-support.patch),and only found a few minor things (otherwise it looked good to me): > > 1) There's a few trailing white-space warnings on patch application (from where you modified to skip 2 of the tests): > git apply 0001-libpq-sslpassword-der-support.patch > 0001-libpq-sslpassword-der-support.patch:649: trailing whitespace. > # so they don't hang. For now they are not performed. > 0001-libpq-sslpassword-der-support.patch:659: trailing whitespace. > > warning: 2 lines add whitespace errors. > > > 2) src/interfaces/libpq/libpq-fe.h > The following portion of the comment should be removed. > > + * 2ndQPostgres extension. If you need to be compatible with unpatched libpq > + * you must dlsym() these. > > 3) Documentation for the "PQsslpassword" function should be added to the libpq "33.2 Connection Status Functions" section. > > > I made the following notes about how/what I reviewed and tested: > > - Applied patch and built Postgres (--with-openssl --enable-tap-tests), checked build output > - Checked patch code modifications (format, logic, memory usage, efficiency, corner cases etc.) > - Built documentation and checked updated portions (format, grammar, details, completeness etc.) > - Checked test updates > - Ran updated contrib/dblink tests - confirmed all passed > - Ran updated SSL (TAP) tests - confirmed all passed > - Ran "make installcheck-world", as per review requirements > - Wrote small libpq-based app to test: > - new APIs (PQsslpassword, PQsetSSLKeyPassHook, PQgetSSLKeyPassHook, PQdefaultSSLKeyPassHook) > - passphrase-protected key with/without patch > - patch with/without new key password callack > - patch and certificate with/without pass phrase protection on key > - default callback, callback delegation > - PEM/DER keys > > Thanks, nice thorough review. Here's an updated patch that I think fixes all the things you mentioned. I plan to commit this tomorrow. cheers andrew -- Andrew Dunstan https://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachment
On Fri, Nov 29, 2019 at 09:27:02AM -0500, Andrew Dunstan wrote: > On 11/28/19 10:25 PM, Greg Nancarrow wrote: > > 3) Documentation for the "PQsslpassword" function should be added to the libpq "33.2 Connection Status Functions" section. > I plan to commit this tomorrow. The PQsslpassword() function retrieves a connection parameter value, which one can retrieve with PQconninfo(). Since introducing PQconninfo(), we have not added any per-parameter accessor functions. Would you remove PQsslpassword()?
On 12/6/19 2:57 AM, Noah Misch wrote: > On Fri, Nov 29, 2019 at 09:27:02AM -0500, Andrew Dunstan wrote: >> On 11/28/19 10:25 PM, Greg Nancarrow wrote: >>> 3) Documentation for the "PQsslpassword" function should be added to the libpq "33.2 Connection Status Functions" section. >> I plan to commit this tomorrow. > The PQsslpassword() function retrieves a connection parameter value, which one > can retrieve with PQconninfo(). Since introducing PQconninfo(), we have not > added any per-parameter accessor functions. Would you remove PQsslpassword()? *sigh* ok cheers andrew -- Andrew Dunstan https://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services