Thread

Commits

  1. libq support for sslpassword connection param, DER format keys

  1. libpq sslpassword parameter and callback function

    Andrew Dunstan <andrew.dunstan@2ndquadrant.com> — 2019-10-31T22:33:12Z

    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
    
    
    
    
    
  2. Re: libpq sslpassword parameter and callback function

    Andrew Dunstan <andrew.dunstan@2ndquadrant.com> — 2019-10-31T22:34:32Z

    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
    
    
  3. Re: libpq sslpassword parameter and callback function

    Andrew Dunstan <andrew.dunstan@2ndquadrant.com> — 2019-10-31T23:27:52Z

    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
    
    
    
    
    
  4. Re: libpq sslpassword parameter and callback function

    Craig Ringer <craig@2ndquadrant.com> — 2019-11-10T09:47:24Z

    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.
    
    
    -- 
     Craig Ringer                   http://www.2ndQuadrant.com/
     2ndQuadrant - PostgreSQL Solutions for the Enterprise
    
  5. Re: libpq sslpassword parameter and callback function

    Bruce Momjian <bruce@momjian.us> — 2019-11-13T03:10:38Z

    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 +
    
    
    
    
  6. Re: libpq sslpassword parameter and callback function

    Andrew Dunstan <andrew.dunstan@2ndquadrant.com> — 2019-11-25T21:09:08Z

    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
    
    
  7. Re: libpq sslpassword parameter and callback function

    Andrew Dunstan <andrew.dunstan@2ndquadrant.com> — 2019-11-28T00:06:10Z

    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
    
    
  8. Re: libpq sslpassword parameter and callback function

    Greg Nancarrow <gregn4422@gmail.com> — 2019-11-29T03:25:45Z

    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
  9. Re: libpq sslpassword parameter and callback function

    Andrew Dunstan <andrew.dunstan@2ndquadrant.com> — 2019-11-29T14:27:02Z

    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
    
    
  10. Re: libpq sslpassword parameter and callback function

    Noah Misch <noah@leadboat.com> — 2019-12-06T07:57:15Z

    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()?
    
    
    
    
  11. Re: libpq sslpassword parameter and callback function

    Andrew Dunstan <andrew.dunstan@2ndquadrant.com> — 2019-12-07T13:51:20Z

    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