psql: Make SSL info display more compact

Started by Peter Eisentrautalmost 4 years ago10 messages
#1Peter Eisentraut
peter.eisentraut@enterprisedb.com
1 attachment(s)

Currently, when you connect with psql over SSL, you get a display like
this:

psql (15devel)
SSL connection (protocol: TLSv1.3, cipher: TLS_AES_256_GCM_SHA384, bits: 256, compression: off)
Type "help" for help.

Since support for SSL compression has been removed from PostgreSQL, it
doesn't seem sensible to display it anymore. And while we're there, I
think the bits information is redundant, since it can be derived from
the cipher suite, either because it's part of the name (as in the
example) or by looking it up somewhere. So I propose that we make this
display a bit more compact like this:

psql (15devel)
SSL connection (protocol: TLSv1.3, cipher: TLS_AES_256_GCM_SHA384)
Type "help" for help.

See attached patch.

Attachments:

0001-psql-Make-SSL-info-display-more-compact.patchtext/plain; charset=UTF-8; name=0001-psql-Make-SSL-info-display-more-compact.patchDownload
From 65042b40c874ff9f3877e5bbb1915321f5759b68 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Mon, 28 Feb 2022 09:54:16 +0100
Subject: [PATCH] psql: Make SSL info display more compact

Don't display the status of SSL compression anymore, since it has been
removed from PostgreSQL.  Also, remove the bits display, since that
can be derived from the cipher suite.
---
 src/bin/psql/command.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 292cff5df9..20745717a8 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -3667,22 +3667,16 @@ printSSLInfo(void)
 {
 	const char *protocol;
 	const char *cipher;
-	const char *bits;
-	const char *compression;
 
 	if (!PQsslInUse(pset.db))
 		return;					/* no SSL */
 
 	protocol = PQsslAttribute(pset.db, "protocol");
 	cipher = PQsslAttribute(pset.db, "cipher");
-	bits = PQsslAttribute(pset.db, "key_bits");
-	compression = PQsslAttribute(pset.db, "compression");
 
-	printf(_("SSL connection (protocol: %s, cipher: %s, bits: %s, compression: %s)\n"),
+	printf(_("SSL connection (protocol: %s, cipher: %s)\n"),
 		   protocol ? protocol : _("unknown"),
-		   cipher ? cipher : _("unknown"),
-		   bits ? bits : _("unknown"),
-		   (compression && strcmp(compression, "off") != 0) ? _("on") : _("off"));
+		   cipher ? cipher : _("unknown"));
 }
 
 /*
-- 
2.35.1

#2Daniel Gustafsson
daniel@yesql.se
In reply to: Peter Eisentraut (#1)
Re: psql: Make SSL info display more compact

On 28 Feb 2022, at 10:02, Peter Eisentraut <peter.eisentraut@enterprisedb.com> wrote:

Since support for SSL compression has been removed from PostgreSQL, it
doesn't seem sensible to display it anymore.

This was originally done, but all client side changes reverted as there still
are server versions in production which allow compression.

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

In reply to: Daniel Gustafsson (#2)
Re: psql: Make SSL info display more compact

Daniel Gustafsson <daniel@yesql.se> writes:

On 28 Feb 2022, at 10:02, Peter Eisentraut <peter.eisentraut@enterprisedb.com> wrote:

Since support for SSL compression has been removed from PostgreSQL, it
doesn't seem sensible to display it anymore.

This was originally done, but all client side changes reverted as there still
are server versions in production which allow compression.

How about making it show "compression: on" if compression is on, but
nothing in the common "off" case?

- ilmari

#4Michael Paquier
michael@paquier.xyz
In reply to: Dagfinn Ilmari Mannsåker (#3)
Re: psql: Make SSL info display more compact

On Mon, Feb 28, 2022 at 10:50:01AM +0000, Dagfinn Ilmari Mannsåker wrote:

Daniel Gustafsson <daniel@yesql.se> writes:

On 28 Feb 2022, at 10:02, Peter Eisentraut <peter.eisentraut@enterprisedb.com> wrote:
This was originally done, but all client side changes reverted as there still
are server versions in production which allow compression.

How about making it show "compression: on" if compression is on, but
nothing in the common "off" case?

Hm, no, as it can be useful to know that compression is disabled when
connecting to an old server. What about that making the information
shown version-aware? I would choose to show the compression part only
for server versions where it is settable.
--
Michael

#5Peter Eisentraut
peter.eisentraut@enterprisedb.com
In reply to: Dagfinn Ilmari Mannsåker (#3)
Re: psql: Make SSL info display more compact

On 28.02.22 11:50, Dagfinn Ilmari Mannsåker wrote:

Daniel Gustafsson <daniel@yesql.se> writes:

On 28 Feb 2022, at 10:02, Peter Eisentraut <peter.eisentraut@enterprisedb.com> wrote:

Since support for SSL compression has been removed from PostgreSQL, it
doesn't seem sensible to display it anymore.

This was originally done, but all client side changes reverted as there still
are server versions in production which allow compression.

How about making it show "compression: on" if compression is on, but
nothing in the common "off" case?

That would work for me.

#6Peter Eisentraut
peter.eisentraut@enterprisedb.com
In reply to: Michael Paquier (#4)
Re: psql: Make SSL info display more compact

On 28.02.22 12:21, Michael Paquier wrote:

What about that making the information
shown version-aware? I would choose to show the compression part only
for server versions where it is settable.

That might lead to confusing results if you are not connecting to
something that is a stock PostgreSQL server.

#7Daniel Gustafsson
daniel@yesql.se
In reply to: Peter Eisentraut (#5)
Re: psql: Make SSL info display more compact

On 28 Feb 2022, at 12:56, Peter Eisentraut <peter.eisentraut@enterprisedb.com> wrote:
On 28.02.22 11:50, Dagfinn Ilmari Mannsåker wrote:

Daniel Gustafsson <daniel@yesql.se> writes:

This was originally done, but all client side changes reverted as there still
are server versions in production which allow compression.

How about making it show "compression: on" if compression is on, but
nothing in the common "off" case?

That would work for me.

On POLA grounds I would prefer to never to show it. If we ever get libpq
compression and want to show that, I'd prefer that we didn't end up
"compression" meaning one thing except when it means two things.

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

#8Tom Lane
tgl@sss.pgh.pa.us
In reply to: Daniel Gustafsson (#7)
Re: psql: Make SSL info display more compact

Daniel Gustafsson <daniel@yesql.se> writes:

On 28 Feb 2022, at 12:56, Peter Eisentraut <peter.eisentraut@enterprisedb.com> wrote:

On 28.02.22 11:50, Dagfinn Ilmari Mannsåker wrote:

How about making it show "compression: on" if compression is on, but
nothing in the common "off" case?

That would work for me.

On POLA grounds I would prefer to never to show it. If we ever get libpq
compression and want to show that, I'd prefer that we didn't end up
"compression" meaning one thing except when it means two things.

Well, any such output would presumably be on a different line and
thus distinguishable from the SSL property; plus, it'd be impossible
for both forms to show up in the same connection.

However, how about writing "SSL compression: on" versus writing
nothing? That avoids doubt about what it means. I don't buy
Michael's argument that this is ambiguous, either.

regards, tom lane

#9Peter Eisentraut
peter.eisentraut@enterprisedb.com
In reply to: Tom Lane (#8)
1 attachment(s)
Re: psql: Make SSL info display more compact

On 28.02.22 16:12, Tom Lane wrote:

Daniel Gustafsson <daniel@yesql.se> writes:

On 28 Feb 2022, at 12:56, Peter Eisentraut <peter.eisentraut@enterprisedb.com> wrote:

On 28.02.22 11:50, Dagfinn Ilmari Mannsåker wrote:

How about making it show "compression: on" if compression is on, but
nothing in the common "off" case?

That would work for me.

On POLA grounds I would prefer to never to show it. If we ever get libpq
compression and want to show that, I'd prefer that we didn't end up
"compression" meaning one thing except when it means two things.

Well, any such output would presumably be on a different line and
thus distinguishable from the SSL property; plus, it'd be impossible
for both forms to show up in the same connection.

However, how about writing "SSL compression: on" versus writing
nothing? That avoids doubt about what it means. I don't buy
Michael's argument that this is ambiguous, either.

I didn't mean to reopen the whole SSL compression can of worms here, I
was mistaken about the level of support left after PG14. I was merely
lightly annoyed that the psql startup display got quite long with not
very interesting information.

I propose a reduced patch that just removes the "bits" display, since
that is redundant with the "cipher" display.

Attachments:

v2-0001-psql-Make-SSL-info-display-more-compact.patchtext/plain; charset=UTF-8; name=v2-0001-psql-Make-SSL-info-display-more-compact.patchDownload
From 9768dc5ab9a01a60e899bfc5c9e1a51475ded183 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Tue, 1 Mar 2022 09:38:38 +0100
Subject: [PATCH v2] psql: Make SSL info display more compact

Remove the bits display, since that can be derived from the cipher
suite.
---
 src/bin/psql/command.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 292cff5df9..079f4a1a76 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -3667,7 +3667,6 @@ printSSLInfo(void)
 {
 	const char *protocol;
 	const char *cipher;
-	const char *bits;
 	const char *compression;
 
 	if (!PQsslInUse(pset.db))
@@ -3675,13 +3674,11 @@ printSSLInfo(void)
 
 	protocol = PQsslAttribute(pset.db, "protocol");
 	cipher = PQsslAttribute(pset.db, "cipher");
-	bits = PQsslAttribute(pset.db, "key_bits");
 	compression = PQsslAttribute(pset.db, "compression");
 
-	printf(_("SSL connection (protocol: %s, cipher: %s, bits: %s, compression: %s)\n"),
+	printf(_("SSL connection (protocol: %s, cipher: %s, compression: %s)\n"),
 		   protocol ? protocol : _("unknown"),
 		   cipher ? cipher : _("unknown"),
-		   bits ? bits : _("unknown"),
 		   (compression && strcmp(compression, "off") != 0) ? _("on") : _("off"));
 }
 

base-commit: a33e17f210547226ada52d2b8af851c3553bb4fa
-- 
2.35.1

#10Daniel Gustafsson
daniel@yesql.se
In reply to: Peter Eisentraut (#9)
Re: psql: Make SSL info display more compact

On 1 Mar 2022, at 09:44, Peter Eisentraut <peter.eisentraut@enterprisedb.com> wrote:

I propose a reduced patch that just removes the "bits" display, since that is
redundant with the "cipher"

No objections from me.

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