[PATCH] Enable SSL library detection via PQsslAttribute
Started by Jacob Championalmost 4 years ago12 messages
Hello,
As part of the NSS work it came up [1]/messages/by-id/a1798e46d6d801344ebc93672c6947ef5297c8a0.camel@vmware.com 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]: /messages/by-id/a1798e46d6d801344ebc93672c6947ef5297c8a0.camel@vmware.com
Attachments:
0001-Enable-SSL-library-detection-via-PQsslAttribute.patchtext/x-patch; name=0001-Enable-SSL-library-detection-via-PQsslAttribute.patchDownload
From a5e5549ccec9c70a510f031a678e9f0f32a35382 Mon Sep 17 00:00:00 2001
From: Jacob Champion <pchampion@vmware.com>
Date: Mon, 29 Nov 2021 14:36:38 -0800
Subject: [PATCH 1/2] Enable SSL library detection via PQsslAttribute()
Currently, libpq client code must have a connection handle before it can
query the "library" SSL attribute. This poses problems if the client
needs to know what SSL library is in use before constructing a
connection string. (For example, with the NSS proposal, a client would
have to decide whether to use the "ssldatabase" connection setting
rather than "sslcert" et al.)
Allow PQsslAttribute(NULL, "library") to return the library in use --
currently, just "OpenSSL" or NULL. The new behavior is announced with
the LIBPQ_HAS_SSL_LIBRARY_DETECTION feature macro, allowing clients to
differentiate between a libpq that was compiled without SSL support and
a libpq that's just too old to tell.
---
src/interfaces/libpq/fe-secure-openssl.c | 6 +++---
src/interfaces/libpq/libpq-fe.h | 2 ++
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
index f6e563a2e5..e095a0f538 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -1597,14 +1597,14 @@ PQsslAttributeNames(PGconn *conn)
const char *
PQsslAttribute(PGconn *conn, const char *attribute_name)
{
+ if (strcmp(attribute_name, "library") == 0)
+ return "OpenSSL";
+
if (!conn)
return NULL;
if (conn->ssl == NULL)
return NULL;
- if (strcmp(attribute_name, "library") == 0)
- return "OpenSSL";
-
if (strcmp(attribute_name, "key_bits") == 0)
{
static char sslbits_str[12];
diff --git a/src/interfaces/libpq/libpq-fe.h b/src/interfaces/libpq/libpq-fe.h
index 20eb855abc..7986445f1a 100644
--- a/src/interfaces/libpq/libpq-fe.h
+++ b/src/interfaces/libpq/libpq-fe.h
@@ -36,6 +36,8 @@ extern "C"
#define LIBPQ_HAS_PIPELINING 1
/* Indicates presence of PQsetTraceFlags; also new PQtrace output format */
#define LIBPQ_HAS_TRACE_FLAGS 1
+/* Indicates that PQsslAttribute(NULL, "library") is useful */
+#define LIBPQ_HAS_SSL_LIBRARY_DETECTION 1
/*
* Option flags for PQcopyResult
--
2.25.1
Re: [PATCH] Enable SSL library detection via PQsslAttribute
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
Re: [PATCH] Enable SSL library detection via PQsslAttribute
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]/messages/by-id/20220223203031.ezrd73ohvjgfksow@alap3.anarazel.de, so I'll need to watch that thread. :)
Thanks!
--Jacob
[1]: /messages/by-id/20220223203031.ezrd73ohvjgfksow@alap3.anarazel.de
Attachments:
v2-0001-Enable-SSL-library-detection-via-PQsslAttribute.patchtext/x-patch; name=v2-0001-Enable-SSL-library-detection-via-PQsslAttribute.patchDownload
From a5e5549ccec9c70a510f031a678e9f0f32a35382 Mon Sep 17 00:00:00 2001
From: Jacob Champion <pchampion@vmware.com>
Date: Mon, 29 Nov 2021 14:36:38 -0800
Subject: [PATCH v2 1/2] Enable SSL library detection via PQsslAttribute()
Currently, libpq client code must have a connection handle before it can
query the "library" SSL attribute. This poses problems if the client
needs to know what SSL library is in use before constructing a
connection string. (For example, with the NSS proposal, a client would
have to decide whether to use the "ssldatabase" connection setting
rather than "sslcert" et al.)
Allow PQsslAttribute(NULL, "library") to return the library in use --
currently, just "OpenSSL" or NULL. The new behavior is announced with
the LIBPQ_HAS_SSL_LIBRARY_DETECTION feature macro, allowing clients to
differentiate between a libpq that was compiled without SSL support and
a libpq that's just too old to tell.
---
src/interfaces/libpq/fe-secure-openssl.c | 6 +++---
src/interfaces/libpq/libpq-fe.h | 2 ++
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
index f6e563a2e5..e095a0f538 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -1597,14 +1597,14 @@ PQsslAttributeNames(PGconn *conn)
const char *
PQsslAttribute(PGconn *conn, const char *attribute_name)
{
+ if (strcmp(attribute_name, "library") == 0)
+ return "OpenSSL";
+
if (!conn)
return NULL;
if (conn->ssl == NULL)
return NULL;
- if (strcmp(attribute_name, "library") == 0)
- return "OpenSSL";
-
if (strcmp(attribute_name, "key_bits") == 0)
{
static char sslbits_str[12];
diff --git a/src/interfaces/libpq/libpq-fe.h b/src/interfaces/libpq/libpq-fe.h
index 20eb855abc..7986445f1a 100644
--- a/src/interfaces/libpq/libpq-fe.h
+++ b/src/interfaces/libpq/libpq-fe.h
@@ -36,6 +36,8 @@ extern "C"
#define LIBPQ_HAS_PIPELINING 1
/* Indicates presence of PQsetTraceFlags; also new PQtrace output format */
#define LIBPQ_HAS_TRACE_FLAGS 1
+/* Indicates that PQsslAttribute(NULL, "library") is useful */
+#define LIBPQ_HAS_SSL_LIBRARY_DETECTION 1
/*
* Option flags for PQcopyResult
--
2.25.1
v2-0002-WIP-add-regression-test-for-PQsslAttribute.patchtext/x-patch; name=v2-0002-WIP-add-regression-test-for-PQsslAttribute.patchDownload
From ff31c3275fbf57e1e5edef9c2926f2bd23ec8512 Mon Sep 17 00:00:00 2001
From: Jacob Champion <pchampion@vmware.com>
Date: Thu, 10 Feb 2022 14:41:04 -0800
Subject: [PATCH v2 2/2] WIP: add regression test for PQsslAttribute()
---
src/interfaces/libpq/test/.gitignore | 1 +
src/interfaces/libpq/test/Makefile | 5 +++-
src/interfaces/libpq/test/testclient.c | 37 ++++++++++++++++++++++++++
3 files changed, 42 insertions(+), 1 deletion(-)
create mode 100644 src/interfaces/libpq/test/testclient.c
diff --git a/src/interfaces/libpq/test/.gitignore b/src/interfaces/libpq/test/.gitignore
index 5387b3b6d9..a6be825f3c 100644
--- a/src/interfaces/libpq/test/.gitignore
+++ b/src/interfaces/libpq/test/.gitignore
@@ -1,3 +1,4 @@
+/testclient
/uri-regress
/regress.diff
/regress.out
diff --git a/src/interfaces/libpq/test/Makefile b/src/interfaces/libpq/test/Makefile
index 4832fab9d2..10cf3a34df 100644
--- a/src/interfaces/libpq/test/Makefile
+++ b/src/interfaces/libpq/test/Makefile
@@ -2,6 +2,8 @@ subdir = src/interfaces/libpq/test
top_builddir = ../../../..
include $(top_builddir)/src/Makefile.global
+export with_ssl
+
ifeq ($(PORTNAME), win32)
LDFLAGS += -lws2_32
endif
@@ -9,13 +11,14 @@ endif
override CPPFLAGS := -I$(libpq_srcdir) $(CPPFLAGS)
LDFLAGS_INTERNAL += $(libpq_pgport)
-PROGS = uri-regress
+PROGS = testclient uri-regress
all: $(PROGS)
installcheck: all
SRCDIR='$(top_srcdir)' SUBDIR='$(subdir)' \
$(PERL) $(top_srcdir)/$(subdir)/regress.pl
+ $(prove_installcheck)
clean distclean maintainer-clean:
rm -f $(PROGS) *.o
diff --git a/src/interfaces/libpq/test/testclient.c b/src/interfaces/libpq/test/testclient.c
new file mode 100644
index 0000000000..2c730d83fa
--- /dev/null
+++ b/src/interfaces/libpq/test/testclient.c
@@ -0,0 +1,37 @@
+/*
+ * testclient.c
+ * A test program for the libpq public API
+ *
+ * Copyright (c) 2022, PostgreSQL Global Development Group
+ *
+ * IDENTIFICATION
+ * src/interfaces/libpq/test/testclient.c
+ */
+
+#include "postgres_fe.h"
+
+#include "libpq-fe.h"
+
+static void
+print_ssl_library()
+{
+ const char *lib = PQsslAttribute(NULL, "library");
+
+ if (!lib)
+ fprintf(stderr, "SSL is not enabled\n");
+ else
+ printf("%s\n", lib);
+}
+
+int
+main(int argc, char *argv[])
+{
+ if ((argc > 1) && !strcmp(argv[1], "--ssl"))
+ {
+ print_ssl_library();
+ return 0;
+ }
+
+ printf("currently only --ssl is supported\n");
+ return 1;
+}
--
2.25.1
Re: [PATCH] Enable SSL library detection via PQsslAttribute
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
Attachments:
v3-0001-Enable-SSL-library-detection-via-PQsslAttribute.patchtext/x-patch; name=v3-0001-Enable-SSL-library-detection-via-PQsslAttribute.patchDownload
From f5f3d39dbcdfd0c53ab9dce6c7a9dbdd94b2584c Mon Sep 17 00:00:00 2001
From: Jacob Champion <pchampion@vmware.com>
Date: Mon, 29 Nov 2021 14:36:38 -0800
Subject: [PATCH v3] Enable SSL library detection via PQsslAttribute()
Currently, libpq client code must have a connection handle before it can
query the "library" SSL attribute. This poses problems if the client
needs to know what SSL library is in use before constructing a
connection string. (For example, with the NSS proposal, a client would
have to decide whether to use the "ssldatabase" connection setting
rather than "sslcert" et al.)
Allow PQsslAttribute(NULL, "library") to return the library in use --
currently, just "OpenSSL" or NULL. The new behavior is announced with
the LIBPQ_HAS_SSL_LIBRARY_DETECTION feature macro, allowing clients to
differentiate between a libpq that was compiled without SSL support and
a libpq that's just too old to tell.
---
src/interfaces/libpq/Makefile | 1 +
src/interfaces/libpq/fe-secure-openssl.c | 6 ++--
src/interfaces/libpq/libpq-fe.h | 2 ++
src/interfaces/libpq/t/002_api.pl | 23 +++++++++++++++
src/interfaces/libpq/test/.gitignore | 1 +
src/interfaces/libpq/test/Makefile | 2 +-
src/interfaces/libpq/test/testclient.c | 37 ++++++++++++++++++++++++
7 files changed, 68 insertions(+), 4 deletions(-)
create mode 100644 src/interfaces/libpq/t/002_api.pl
create mode 100644 src/interfaces/libpq/test/testclient.c
diff --git a/src/interfaces/libpq/Makefile b/src/interfaces/libpq/Makefile
index 3c53393fa4..89bf5e0126 100644
--- a/src/interfaces/libpq/Makefile
+++ b/src/interfaces/libpq/Makefile
@@ -13,6 +13,7 @@ subdir = src/interfaces/libpq
top_builddir = ../../..
include $(top_builddir)/src/Makefile.global
+export with_ssl
PGFILEDESC = "PostgreSQL Access Library"
diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
index f6e563a2e5..e095a0f538 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -1597,14 +1597,14 @@ PQsslAttributeNames(PGconn *conn)
const char *
PQsslAttribute(PGconn *conn, const char *attribute_name)
{
+ if (strcmp(attribute_name, "library") == 0)
+ return "OpenSSL";
+
if (!conn)
return NULL;
if (conn->ssl == NULL)
return NULL;
- if (strcmp(attribute_name, "library") == 0)
- return "OpenSSL";
-
if (strcmp(attribute_name, "key_bits") == 0)
{
static char sslbits_str[12];
diff --git a/src/interfaces/libpq/libpq-fe.h b/src/interfaces/libpq/libpq-fe.h
index 20eb855abc..7986445f1a 100644
--- a/src/interfaces/libpq/libpq-fe.h
+++ b/src/interfaces/libpq/libpq-fe.h
@@ -36,6 +36,8 @@ extern "C"
#define LIBPQ_HAS_PIPELINING 1
/* Indicates presence of PQsetTraceFlags; also new PQtrace output format */
#define LIBPQ_HAS_TRACE_FLAGS 1
+/* Indicates that PQsslAttribute(NULL, "library") is useful */
+#define LIBPQ_HAS_SSL_LIBRARY_DETECTION 1
/*
* Option flags for PQcopyResult
diff --git a/src/interfaces/libpq/t/002_api.pl b/src/interfaces/libpq/t/002_api.pl
new file mode 100644
index 0000000000..bcf91bc558
--- /dev/null
+++ b/src/interfaces/libpq/t/002_api.pl
@@ -0,0 +1,23 @@
+#!/usr/bin/perl
+
+# Copyright (c) 2022, PostgreSQL Global Development Group
+
+use strict;
+use warnings;
+
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+# Test PQsslAttribute(NULL, "library")
+my ($out, $err) = run_command(['testclient', '--ssl']);
+
+if ($ENV{with_ssl} eq 'openssl')
+{
+ is($out, 'OpenSSL', 'PQsslAttribute(NULL, "library") returns "OpenSSL"');
+}
+else
+{
+ is($err, 'SSL is not enabled', 'PQsslAttribute(NULL, "library") returns NULL');
+}
+
+done_testing();
diff --git a/src/interfaces/libpq/test/.gitignore b/src/interfaces/libpq/test/.gitignore
index 5e803d8816..4b17210483 100644
--- a/src/interfaces/libpq/test/.gitignore
+++ b/src/interfaces/libpq/test/.gitignore
@@ -1 +1,2 @@
+/testclient
/uri-regress
diff --git a/src/interfaces/libpq/test/Makefile b/src/interfaces/libpq/test/Makefile
index 5421215906..1d45be0c37 100644
--- a/src/interfaces/libpq/test/Makefile
+++ b/src/interfaces/libpq/test/Makefile
@@ -11,7 +11,7 @@ endif
override CPPFLAGS := -I$(libpq_srcdir) $(CPPFLAGS)
LDFLAGS_INTERNAL += $(libpq_pgport)
-PROGS = uri-regress
+PROGS = testclient uri-regress
all: $(PROGS)
diff --git a/src/interfaces/libpq/test/testclient.c b/src/interfaces/libpq/test/testclient.c
new file mode 100644
index 0000000000..2c730d83fa
--- /dev/null
+++ b/src/interfaces/libpq/test/testclient.c
@@ -0,0 +1,37 @@
+/*
+ * testclient.c
+ * A test program for the libpq public API
+ *
+ * Copyright (c) 2022, PostgreSQL Global Development Group
+ *
+ * IDENTIFICATION
+ * src/interfaces/libpq/test/testclient.c
+ */
+
+#include "postgres_fe.h"
+
+#include "libpq-fe.h"
+
+static void
+print_ssl_library()
+{
+ const char *lib = PQsslAttribute(NULL, "library");
+
+ if (!lib)
+ fprintf(stderr, "SSL is not enabled\n");
+ else
+ printf("%s\n", lib);
+}
+
+int
+main(int argc, char *argv[])
+{
+ if ((argc > 1) && !strcmp(argv[1], "--ssl"))
+ {
+ print_ssl_library();
+ return 0;
+ }
+
+ printf("currently only --ssl is supported\n");
+ return 1;
+}
--
2.25.1
Re: [PATCH] Enable SSL library detection via PQsslAttribute
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
Re: [PATCH] Enable SSL library detection via PQsslAttribute
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
Attachments:
since-v3.diff.txttext/plain; name=since-v3.diff.txtDownload
commit 53fca988682c80a99bbb19eeb3d7959533fc3b83
Author: Jacob Champion <pchampion@vmware.com>
Date: Fri Mar 25 14:16:47 2022 -0700
squash! Enable SSL library detection via PQsslAttribute()
Add docs.
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 3998b1781b..82f3092715 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -2538,6 +2538,17 @@ const char *PQsslAttribute(const PGconn *conn, const char *attribute_name);
Name of the SSL implementation in use. (Currently, only
<literal>"OpenSSL"</literal> is implemented)
</para>
+ <note>
+ <para>
+ As a special case, the <literal>library</literal> attribute may be
+ queried without an existing connection by passing NULL as the
+ <literal>conn</literal> argument. The historical behavior was to
+ return NULL for any attribute when a NULL <literal>conn</literal>
+ was provided; client programs needing to differentiate between the
+ newer and older implementations may check the
+ <literal>LIBPQ_HAS_SSL_LIBRARY_DETECTION</literal> feature macro.
+ </para>
+ </note>
</listitem>
</varlistentry>
<varlistentry>
v4-0001-Enable-SSL-library-detection-via-PQsslAttribute.patchtext/x-patch; name=v4-0001-Enable-SSL-library-detection-via-PQsslAttribute.patchDownload
From 1ec54121dc0eeae7e48b9e38b829980e6a11e31c Mon Sep 17 00:00:00 2001
From: Jacob Champion <pchampion@vmware.com>
Date: Mon, 29 Nov 2021 14:36:38 -0800
Subject: [PATCH v4] Enable SSL library detection via PQsslAttribute()
Currently, libpq client code must have a connection handle before it can
query the "library" SSL attribute. This poses problems if the client
needs to know what SSL library is in use before constructing a
connection string. (For example, with the NSS proposal, a client would
have to decide whether to use the "ssldatabase" connection setting
rather than "sslcert" et al.)
Allow PQsslAttribute(NULL, "library") to return the library in use --
currently, just "OpenSSL" or NULL. The new behavior is announced with
the LIBPQ_HAS_SSL_LIBRARY_DETECTION feature macro, allowing clients to
differentiate between a libpq that was compiled without SSL support and
a libpq that's just too old to tell.
---
doc/src/sgml/libpq.sgml | 11 +++++++
src/interfaces/libpq/Makefile | 1 +
src/interfaces/libpq/fe-secure-openssl.c | 6 ++--
src/interfaces/libpq/libpq-fe.h | 2 ++
src/interfaces/libpq/t/002_api.pl | 23 +++++++++++++++
src/interfaces/libpq/test/.gitignore | 1 +
src/interfaces/libpq/test/Makefile | 2 +-
src/interfaces/libpq/test/testclient.c | 37 ++++++++++++++++++++++++
8 files changed, 79 insertions(+), 4 deletions(-)
create mode 100644 src/interfaces/libpq/t/002_api.pl
create mode 100644 src/interfaces/libpq/test/testclient.c
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 3998b1781b..82f3092715 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -2538,6 +2538,17 @@ const char *PQsslAttribute(const PGconn *conn, const char *attribute_name);
Name of the SSL implementation in use. (Currently, only
<literal>"OpenSSL"</literal> is implemented)
</para>
+ <note>
+ <para>
+ As a special case, the <literal>library</literal> attribute may be
+ queried without an existing connection by passing NULL as the
+ <literal>conn</literal> argument. The historical behavior was to
+ return NULL for any attribute when a NULL <literal>conn</literal>
+ was provided; client programs needing to differentiate between the
+ newer and older implementations may check the
+ <literal>LIBPQ_HAS_SSL_LIBRARY_DETECTION</literal> feature macro.
+ </para>
+ </note>
</listitem>
</varlistentry>
<varlistentry>
diff --git a/src/interfaces/libpq/Makefile b/src/interfaces/libpq/Makefile
index 3c53393fa4..89bf5e0126 100644
--- a/src/interfaces/libpq/Makefile
+++ b/src/interfaces/libpq/Makefile
@@ -13,6 +13,7 @@ subdir = src/interfaces/libpq
top_builddir = ../../..
include $(top_builddir)/src/Makefile.global
+export with_ssl
PGFILEDESC = "PostgreSQL Access Library"
diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
index d81218a4cc..d3bf57b850 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -1631,14 +1631,14 @@ PQsslAttributeNames(PGconn *conn)
const char *
PQsslAttribute(PGconn *conn, const char *attribute_name)
{
+ if (strcmp(attribute_name, "library") == 0)
+ return "OpenSSL";
+
if (!conn)
return NULL;
if (conn->ssl == NULL)
return NULL;
- if (strcmp(attribute_name, "library") == 0)
- return "OpenSSL";
-
if (strcmp(attribute_name, "key_bits") == 0)
{
static char sslbits_str[12];
diff --git a/src/interfaces/libpq/libpq-fe.h b/src/interfaces/libpq/libpq-fe.h
index 20eb855abc..7986445f1a 100644
--- a/src/interfaces/libpq/libpq-fe.h
+++ b/src/interfaces/libpq/libpq-fe.h
@@ -36,6 +36,8 @@ extern "C"
#define LIBPQ_HAS_PIPELINING 1
/* Indicates presence of PQsetTraceFlags; also new PQtrace output format */
#define LIBPQ_HAS_TRACE_FLAGS 1
+/* Indicates that PQsslAttribute(NULL, "library") is useful */
+#define LIBPQ_HAS_SSL_LIBRARY_DETECTION 1
/*
* Option flags for PQcopyResult
diff --git a/src/interfaces/libpq/t/002_api.pl b/src/interfaces/libpq/t/002_api.pl
new file mode 100644
index 0000000000..bcf91bc558
--- /dev/null
+++ b/src/interfaces/libpq/t/002_api.pl
@@ -0,0 +1,23 @@
+#!/usr/bin/perl
+
+# Copyright (c) 2022, PostgreSQL Global Development Group
+
+use strict;
+use warnings;
+
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+# Test PQsslAttribute(NULL, "library")
+my ($out, $err) = run_command(['testclient', '--ssl']);
+
+if ($ENV{with_ssl} eq 'openssl')
+{
+ is($out, 'OpenSSL', 'PQsslAttribute(NULL, "library") returns "OpenSSL"');
+}
+else
+{
+ is($err, 'SSL is not enabled', 'PQsslAttribute(NULL, "library") returns NULL');
+}
+
+done_testing();
diff --git a/src/interfaces/libpq/test/.gitignore b/src/interfaces/libpq/test/.gitignore
index 5e803d8816..4b17210483 100644
--- a/src/interfaces/libpq/test/.gitignore
+++ b/src/interfaces/libpq/test/.gitignore
@@ -1 +1,2 @@
+/testclient
/uri-regress
diff --git a/src/interfaces/libpq/test/Makefile b/src/interfaces/libpq/test/Makefile
index 5421215906..1d45be0c37 100644
--- a/src/interfaces/libpq/test/Makefile
+++ b/src/interfaces/libpq/test/Makefile
@@ -11,7 +11,7 @@ endif
override CPPFLAGS := -I$(libpq_srcdir) $(CPPFLAGS)
LDFLAGS_INTERNAL += $(libpq_pgport)
-PROGS = uri-regress
+PROGS = testclient uri-regress
all: $(PROGS)
diff --git a/src/interfaces/libpq/test/testclient.c b/src/interfaces/libpq/test/testclient.c
new file mode 100644
index 0000000000..2c730d83fa
--- /dev/null
+++ b/src/interfaces/libpq/test/testclient.c
@@ -0,0 +1,37 @@
+/*
+ * testclient.c
+ * A test program for the libpq public API
+ *
+ * Copyright (c) 2022, PostgreSQL Global Development Group
+ *
+ * IDENTIFICATION
+ * src/interfaces/libpq/test/testclient.c
+ */
+
+#include "postgres_fe.h"
+
+#include "libpq-fe.h"
+
+static void
+print_ssl_library()
+{
+ const char *lib = PQsslAttribute(NULL, "library");
+
+ if (!lib)
+ fprintf(stderr, "SSL is not enabled\n");
+ else
+ printf("%s\n", lib);
+}
+
+int
+main(int argc, char *argv[])
+{
+ if ((argc > 1) && !strcmp(argv[1], "--ssl"))
+ {
+ print_ssl_library();
+ return 0;
+ }
+
+ printf("currently only --ssl is supported\n");
+ return 1;
+}
--
2.25.1
Re: [PATCH] Enable SSL library detection via PQsslAttribute
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/
Re: [PATCH] Enable SSL library detection via PQsslAttribute
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
Re: [PATCH] Enable SSL library detection via PQsslAttribute
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
Attachments:
since-v4.diff.txttext/plain; name=since-v4.diff.txtDownload
commit 585bf50bdd9ffc1b7c07b65e500a19bf50fbddff
Author: Jacob Champion <pchampion@vmware.com>
Date: Fri Mar 25 15:36:09 2022 -0700
squash! Enable SSL library detection via PQsslAttribute()
Move new docs out of a Note into the main body.
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 82f3092715..aa400d1b7c 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -2538,17 +2538,6 @@ const char *PQsslAttribute(const PGconn *conn, const char *attribute_name);
Name of the SSL implementation in use. (Currently, only
<literal>"OpenSSL"</literal> is implemented)
</para>
- <note>
- <para>
- As a special case, the <literal>library</literal> attribute may be
- queried without an existing connection by passing NULL as the
- <literal>conn</literal> argument. The historical behavior was to
- return NULL for any attribute when a NULL <literal>conn</literal>
- was provided; client programs needing to differentiate between the
- newer and older implementations may check the
- <literal>LIBPQ_HAS_SSL_LIBRARY_DETECTION</literal> feature macro.
- </para>
- </note>
</listitem>
</varlistentry>
<varlistentry>
@@ -2592,6 +2581,16 @@ const char *PQsslAttribute(const PGconn *conn, const char *attribute_name);
</varlistentry>
</variablelist>
</para>
+
+ <para>
+ As a special case, the <literal>library</literal> attribute may be
+ queried without an existing connection by passing NULL as the
+ <literal>conn</literal> argument. The historical behavior was to return
+ NULL for any attribute when a NULL <literal>conn</literal> was provided;
+ client programs needing to differentiate between the newer and older
+ implementations may check the
+ <literal>LIBPQ_HAS_SSL_LIBRARY_DETECTION</literal> feature macro.
+ </para>
</listitem>
</varlistentry>
v5-0001-Enable-SSL-library-detection-via-PQsslAttribute.patchtext/x-patch; name=v5-0001-Enable-SSL-library-detection-via-PQsslAttribute.patchDownload
From 6ea7a272a4cf316d4bf94da07c1d3538fcaa333e Mon Sep 17 00:00:00 2001
From: Jacob Champion <pchampion@vmware.com>
Date: Mon, 29 Nov 2021 14:36:38 -0800
Subject: [PATCH v5] Enable SSL library detection via PQsslAttribute()
Currently, libpq client code must have a connection handle before it can
query the "library" SSL attribute. This poses problems if the client
needs to know what SSL library is in use before constructing a
connection string. (For example, with the NSS proposal, a client would
have to decide whether to use the "ssldatabase" connection setting
rather than "sslcert" et al.)
Allow PQsslAttribute(NULL, "library") to return the library in use --
currently, just "OpenSSL" or NULL. The new behavior is announced with
the LIBPQ_HAS_SSL_LIBRARY_DETECTION feature macro, allowing clients to
differentiate between a libpq that was compiled without SSL support and
a libpq that's just too old to tell.
---
doc/src/sgml/libpq.sgml | 10 +++++++
src/interfaces/libpq/Makefile | 1 +
src/interfaces/libpq/fe-secure-openssl.c | 6 ++--
src/interfaces/libpq/libpq-fe.h | 2 ++
src/interfaces/libpq/t/002_api.pl | 23 +++++++++++++++
src/interfaces/libpq/test/.gitignore | 1 +
src/interfaces/libpq/test/Makefile | 2 +-
src/interfaces/libpq/test/testclient.c | 37 ++++++++++++++++++++++++
8 files changed, 78 insertions(+), 4 deletions(-)
create mode 100644 src/interfaces/libpq/t/002_api.pl
create mode 100644 src/interfaces/libpq/test/testclient.c
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 3998b1781b..aa400d1b7c 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -2581,6 +2581,16 @@ const char *PQsslAttribute(const PGconn *conn, const char *attribute_name);
</varlistentry>
</variablelist>
</para>
+
+ <para>
+ As a special case, the <literal>library</literal> attribute may be
+ queried without an existing connection by passing NULL as the
+ <literal>conn</literal> argument. The historical behavior was to return
+ NULL for any attribute when a NULL <literal>conn</literal> was provided;
+ client programs needing to differentiate between the newer and older
+ implementations may check the
+ <literal>LIBPQ_HAS_SSL_LIBRARY_DETECTION</literal> feature macro.
+ </para>
</listitem>
</varlistentry>
diff --git a/src/interfaces/libpq/Makefile b/src/interfaces/libpq/Makefile
index 3c53393fa4..89bf5e0126 100644
--- a/src/interfaces/libpq/Makefile
+++ b/src/interfaces/libpq/Makefile
@@ -13,6 +13,7 @@ subdir = src/interfaces/libpq
top_builddir = ../../..
include $(top_builddir)/src/Makefile.global
+export with_ssl
PGFILEDESC = "PostgreSQL Access Library"
diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
index d81218a4cc..d3bf57b850 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -1631,14 +1631,14 @@ PQsslAttributeNames(PGconn *conn)
const char *
PQsslAttribute(PGconn *conn, const char *attribute_name)
{
+ if (strcmp(attribute_name, "library") == 0)
+ return "OpenSSL";
+
if (!conn)
return NULL;
if (conn->ssl == NULL)
return NULL;
- if (strcmp(attribute_name, "library") == 0)
- return "OpenSSL";
-
if (strcmp(attribute_name, "key_bits") == 0)
{
static char sslbits_str[12];
diff --git a/src/interfaces/libpq/libpq-fe.h b/src/interfaces/libpq/libpq-fe.h
index 20eb855abc..7986445f1a 100644
--- a/src/interfaces/libpq/libpq-fe.h
+++ b/src/interfaces/libpq/libpq-fe.h
@@ -36,6 +36,8 @@ extern "C"
#define LIBPQ_HAS_PIPELINING 1
/* Indicates presence of PQsetTraceFlags; also new PQtrace output format */
#define LIBPQ_HAS_TRACE_FLAGS 1
+/* Indicates that PQsslAttribute(NULL, "library") is useful */
+#define LIBPQ_HAS_SSL_LIBRARY_DETECTION 1
/*
* Option flags for PQcopyResult
diff --git a/src/interfaces/libpq/t/002_api.pl b/src/interfaces/libpq/t/002_api.pl
new file mode 100644
index 0000000000..bcf91bc558
--- /dev/null
+++ b/src/interfaces/libpq/t/002_api.pl
@@ -0,0 +1,23 @@
+#!/usr/bin/perl
+
+# Copyright (c) 2022, PostgreSQL Global Development Group
+
+use strict;
+use warnings;
+
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+# Test PQsslAttribute(NULL, "library")
+my ($out, $err) = run_command(['testclient', '--ssl']);
+
+if ($ENV{with_ssl} eq 'openssl')
+{
+ is($out, 'OpenSSL', 'PQsslAttribute(NULL, "library") returns "OpenSSL"');
+}
+else
+{
+ is($err, 'SSL is not enabled', 'PQsslAttribute(NULL, "library") returns NULL');
+}
+
+done_testing();
diff --git a/src/interfaces/libpq/test/.gitignore b/src/interfaces/libpq/test/.gitignore
index 5e803d8816..4b17210483 100644
--- a/src/interfaces/libpq/test/.gitignore
+++ b/src/interfaces/libpq/test/.gitignore
@@ -1 +1,2 @@
+/testclient
/uri-regress
diff --git a/src/interfaces/libpq/test/Makefile b/src/interfaces/libpq/test/Makefile
index 5421215906..1d45be0c37 100644
--- a/src/interfaces/libpq/test/Makefile
+++ b/src/interfaces/libpq/test/Makefile
@@ -11,7 +11,7 @@ endif
override CPPFLAGS := -I$(libpq_srcdir) $(CPPFLAGS)
LDFLAGS_INTERNAL += $(libpq_pgport)
-PROGS = uri-regress
+PROGS = testclient uri-regress
all: $(PROGS)
diff --git a/src/interfaces/libpq/test/testclient.c b/src/interfaces/libpq/test/testclient.c
new file mode 100644
index 0000000000..2c730d83fa
--- /dev/null
+++ b/src/interfaces/libpq/test/testclient.c
@@ -0,0 +1,37 @@
+/*
+ * testclient.c
+ * A test program for the libpq public API
+ *
+ * Copyright (c) 2022, PostgreSQL Global Development Group
+ *
+ * IDENTIFICATION
+ * src/interfaces/libpq/test/testclient.c
+ */
+
+#include "postgres_fe.h"
+
+#include "libpq-fe.h"
+
+static void
+print_ssl_library()
+{
+ const char *lib = PQsslAttribute(NULL, "library");
+
+ if (!lib)
+ fprintf(stderr, "SSL is not enabled\n");
+ else
+ printf("%s\n", lib);
+}
+
+int
+main(int argc, char *argv[])
+{
+ if ((argc > 1) && !strcmp(argv[1], "--ssl"))
+ {
+ print_ssl_library();
+ return 0;
+ }
+
+ printf("currently only --ssl is supported\n");
+ return 1;
+}
--
2.25.1
Re: [PATCH] Enable SSL library detection via PQsslAttribute
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/
Re: [PATCH] Enable SSL library detection via PQsslAttribute
Pushed with a few small tweaks to make it match project style, thanks!
--
Daniel Gustafsson https://vmware.com/