Horton

Thread

Commits

  1. Enable SSL library detection via PQsslAttribute()

  1. [PATCH] Enable SSL library detection via PQsslAttribute

    Jacob Champion <pchampion@vmware.com> — 2022-02-23T18:38:08Z

    Hello,

As part of the NSS work it came up [1] that clients don't have a good
way to ask libpq what SSL library it was compiled with, unless they
already have a connection pointer so that they can call
PQsslAttribute(conn, "library"). This poses a chicken-and-egg problem:
with the NSS proposal, the client may have to know which library is in
use before it can construct a proper connection string. For example, I
have a test suite that needs to set up an NSS database with
certificates before telling libpq to connect using that database.

The simplest proposal was to just allow PQsslAttribute() to take NULL
as the connection parameter when querying the "library" attribute, and
that's what I've done in this patch. In current versions of libpq, the
"library" attribute will always be NULL if you pass NULL as the
connection; a client that needs to know whether this new behavior is
present can look at the LIBPQ_HAS_SSL_LIBRARY_DETECTION feature macro.

If this looks good, I'm not sure how best to test it in the regression
suite. I see that libpq has an installcheck recipe that compiles a test
executable for URI parsing; should I add a simple test alongside that?

Thanks,
--Jacob

[1] https://www.postgresql.org/message-id/a1798e46d6d801344ebc93672c6947ef5297c8a0.camel%40vmware.com

  2. Re: [PATCH] Enable SSL library detection via PQsslAttribute

    Andrew Dunstan <andrew@dunslane.net> — 2022-02-23T19:11:34Z

    On 2/23/22 13:38, Jacob Champion wrote:
> Hello,
>
> As part of the NSS work it came up [1] that clients don't have a good
> way to ask libpq what SSL library it was compiled with, unless they
> already have a connection pointer so that they can call
> PQsslAttribute(conn, "library"). This poses a chicken-and-egg problem:
> with the NSS proposal, the client may have to know which library is in
> use before it can construct a proper connection string. For example, I
> have a test suite that needs to set up an NSS database with
> certificates before telling libpq to connect using that database.
>
> The simplest proposal was to just allow PQsslAttribute() to take NULL
> as the connection parameter when querying the "library" attribute, and
> that's what I've done in this patch. In current versions of libpq, the
> "library" attribute will always be NULL if you pass NULL as the
> connection; a client that needs to know whether this new behavior is
> present can look at the LIBPQ_HAS_SSL_LIBRARY_DETECTION feature macro.
>
> If this looks good, I'm not sure how best to test it in the regression
> suite. I see that libpq has an installcheck recipe that compiles a test
> executable for URI parsing; should I add a simple test alongside that?


Create a TAP tests that calls a small client?


cheers


andrew


--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  3. Re: [PATCH] Enable SSL library detection via PQsslAttribute

    Jacob Champion <pchampion@vmware.com> — 2022-02-23T23:20:52Z

    On Wed, 2022-02-23 at 14:11 -0500, Andrew Dunstan wrote:
> On 2/23/22 13:38, Jacob Champion wrote:
> > 
> > If this looks good, I'm not sure how best to test it in the regression
> > suite. I see that libpq has an installcheck recipe that compiles a test
> > executable for URI parsing; should I add a simple test alongside that?
> 
> Create a TAP tests that calls a small client?

First stab in v2-0002. Though I see that Andres is overhauling the
tests in this folder today [1], so I'll need to watch that thread. :)

Thanks!
--Jacob

[1] https://www.postgresql.org/message-id/20220223203031.ezrd73ohvjgfksow%40alap3.anarazel.de

  4. Re: [PATCH] Enable SSL library detection via PQsslAttribute

    Jacob Champion <pchampion@vmware.com> — 2022-02-28T20:21:45Z

    On Wed, 2022-02-23 at 23:20 +0000, Jacob Champion wrote:
> First stab in v2-0002. Though I see that Andres is overhauling the
> tests in this folder today [1], so I'll need to watch that thread. :)

v3 rebases over Andres' changes and actually adds the Perl driver that
I missed the git-add for.

--Jacob

  5. Re: [PATCH] Enable SSL library detection via PQsslAttribute

    Robert Haas <robertmhaas@gmail.com> — 2022-03-25T19:32:23Z

    On Mon, Feb 28, 2022 at 3:21 PM Jacob Champion <pchampion@vmware.com> wrote:
> v3 rebases over Andres' changes and actually adds the Perl driver that
> I missed the git-add for.

This seems totally reasonable. However, I think it should update the
documentation somehow.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

  6. Re: [PATCH] Enable SSL library detection via PQsslAttribute

    Jacob Champion <pchampion@vmware.com> — 2022-03-25T21:25:37Z

    On Fri, 2022-03-25 at 15:32 -0400, Robert Haas wrote:
> On Mon, Feb 28, 2022 at 3:21 PM Jacob Champion <pchampion@vmware.com> wrote:
> > v3 rebases over Andres' changes and actually adds the Perl driver that
> > I missed the git-add for.
> 
> This seems totally reasonable. However, I think it should update the
> documentation somehow.

Done in v4.

Do I need to merge my tiny test program into the libpq_pipeline tests?
I'm not sure what the roadmap is for those.

Thanks!
--Jacob

  7. Re: [PATCH] Enable SSL library detection via PQsslAttribute

    Daniel Gustafsson <daniel@yesql.se> — 2022-03-25T21:48:33Z

    > On 25 Mar 2022, at 22:25, Jacob Champion <pchampion@vmware.com> wrote:
> On Fri, 2022-03-25 at 15:32 -0400, Robert Haas wrote:

>> This seems totally reasonable. However, I think it should update the
>> documentation somehow.
> 
> Done in v4.

I would prefer to not introduce a <note> for this, I think adding it as a
<para> under PQsslAttribute is better given the rest of the libpq API
documentation. The proposed text reads fine to me.

--
Daniel Gustafsson		https://vmware.com/

  8. Re: [PATCH] Enable SSL library detection via PQsslAttribute

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-03-25T22:00:22Z

    Jacob Champion <pchampion@vmware.com> writes:
> Do I need to merge my tiny test program into the libpq_pipeline tests?

Doesn't seem worth the trouble to me, notably because you'd
then have to cope with non-SSL builds too.

			regards, tom lane

  9. Re: [PATCH] Enable SSL library detection via PQsslAttribute

    Jacob Champion <pchampion@vmware.com> — 2022-03-25T22:45:33Z

    On Fri, 2022-03-25 at 18:00 -0400, Tom Lane wrote:
> Jacob Champion <pchampion@vmware.com> writes:
> > Do I need to merge my tiny test program into the libpq_pipeline tests?
> 
> Doesn't seem worth the trouble to me, notably because you'd
> then have to cope with non-SSL builds too.

Fine by me.

v5 moves the docs out of the Note, as requested by Daniel.

Thanks,
--Jacob

  10. Re: [PATCH] Enable SSL library detection via PQsslAttribute

    Daniel Gustafsson <daniel@yesql.se> — 2022-03-28T20:33:57Z

    > On 25 Mar 2022, at 23:45, Jacob Champion <pchampion@vmware.com> wrote:
> 
> On Fri, 2022-03-25 at 18:00 -0400, Tom Lane wrote:
>> Jacob Champion <pchampion@vmware.com> writes:
>>> Do I need to merge my tiny test program into the libpq_pipeline tests?
>> 
>> Doesn't seem worth the trouble to me, notably because you'd
>> then have to cope with non-SSL builds too.
> 
> Fine by me.
> 
> v5 moves the docs out of the Note, as requested by Daniel.

I went over this again and I think this version is ready for committer.  Having
tried to add implement a new TLS library I would add a small comment on
PQsslAttributeNames() for this, reverse-engineering what to implement is hard
as it is without special cases easily identifiable.  That can easily be done
when pushed, no need for a new version IMHO.

--
Daniel Gustafsson		https://vmware.com/

  11. Re: [PATCH] Enable SSL library detection via PQsslAttribute

    Daniel Gustafsson <daniel@yesql.se> — 2022-03-29T12:08:26Z

    Pushed with a few small tweaks to make it match project style, thanks!

--
Daniel Gustafsson		https://vmware.com/

  12. Re: [PATCH] Enable SSL library detection via PQsslAttribute

    Jacob Champion <pchampion@vmware.com> — 2022-03-29T15:29:32Z

    On Tue, 2022-03-29 at 14:08 +0200, Daniel Gustafsson wrote:
> Pushed with a few small tweaks to make it match project style, thanks!

Thank you!

--Jacob