Disallow SSL compression?

Started by Daniel Gustafssonalmost 5 years ago18 messages
#1Daniel Gustafsson
daniel@yesql.se
1 attachment(s)

A few years ago we discussed whether to disable SSL compression [0]/messages/by-id/595cf3b1-4ffe-7f05-6f72-f72b7afa7993@2ndquadrant.com which ended
up with it being off by default combined with a recommendation against it in
the docs.

OpenSSL themselves disabled SSL compression by default in 2016 in 1.1.0 with
distros often having had it disabled for a long while before then. Further,
TLSv1.3 removes compression entirely on the protocol level mandating that only
NULL compression is allowed in the ClientHello. NSS, which is discussed in
another thread, removed SSL compression entirely in version 3.33 in 2017.

It seems about time to revisit this since it's unlikely to work anywhere but in
a very small subset of system setups (being disabled by default everywhere) and
is thus likely to be very untested at best. There is also the security aspect
which is less clear-cut for us compared to HTTP client/servers, but not refuted
(the linked thread has a good discussion on this).

The attached removes sslcompression to see what it would look like. The server
actively disallows it and the parameter is removed, but the sslcompression
column in the stat view is retained. An alternative could be to retain the
parameter but not act on it in order to not break scripts etc, but that just
postpones the pain until when we inevitably do remove it.

Thoughts? Any reason to keep supporting SSL compression or is it time for v14
to remove it? Are there still users leveraging this for protocol compression
without security making it worthwhile to keep?

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

[0]: /messages/by-id/595cf3b1-4ffe-7f05-6f72-f72b7afa7993@2ndquadrant.com

Attachments:

openssl_disallow_compression.patchapplication/octet-stream; name=openssl_disallow_compression.patch; x-unix-mode=0644Download
From f5a2ba03675725adcd95faa48b1ece1bb5b4c0dd Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <daniel@yesql.se>
Date: Wed, 17 Feb 2021 11:15:40 +0100
Subject: [PATCH] Disallow SSL compression

SSL compression has been disabled in OpenSSL since version 1.1.0, and
was disabled in many distros long before that.  The most recent TLS
version, TLSv1.3, also disallows compression at the protocol level.
PostgreSQL disabled compression by default years ago and recommended
against using it, this finally removes the feaure

Discussion:  https://www.postgresql.org/message-id/flat/595cf3b1-4ffe-7f05-6f72-f72b7afa7993%402ndquadrant.com
---
 .../postgres_fdw/expected/postgres_fdw.out    |  3 +-
 contrib/postgres_fdw/sql/postgres_fdw.sql     |  1 -
 doc/src/sgml/libpq.sgml                       | 36 -------------------
 src/backend/libpq/be-secure-openssl.c         |  3 ++
 src/interfaces/libpq/fe-connect.c             |  6 ----
 src/interfaces/libpq/fe-secure-openssl.c      |  7 ++--
 src/interfaces/libpq/libpq-int.h              |  1 -
 7 files changed, 6 insertions(+), 51 deletions(-)

diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 0649b6b81c..c2565dfc70 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -163,7 +163,6 @@ ALTER SERVER testserver1 OPTIONS (
 	keepalives_interval 'value',
 	tcp_user_timeout 'value',
 	-- requiressl 'value',
-	sslcompression 'value',
 	sslmode 'value',
 	sslcert 'value',
 	sslkey 'value',
@@ -8946,7 +8945,7 @@ DO $d$
     END;
 $d$;
 ERROR:  invalid option "password"
-HINT:  Valid options in this context are: service, passfile, channel_binding, connect_timeout, dbname, host, hostaddr, port, options, application_name, keepalives, keepalives_idle, keepalives_interval, keepalives_count, tcp_user_timeout, sslmode, sslcompression, sslcert, sslkey, sslrootcert, sslcrl, sslcrldir, requirepeer, ssl_min_protocol_version, ssl_max_protocol_version, gssencmode, krbsrvname, gsslib, target_session_attrs, use_remote_estimate, fdw_startup_cost, fdw_tuple_cost, extensions, updatable, fetch_size, batch_size
+HINT:  Valid options in this context are: service, passfile, channel_binding, connect_timeout, dbname, host, hostaddr, port, options, application_name, keepalives, keepalives_idle, keepalives_interval, keepalives_count, tcp_user_timeout, sslmode, sslcert, sslkey, sslrootcert, sslcrl, sslcrldir, requirepeer, ssl_min_protocol_version, ssl_max_protocol_version, gssencmode, krbsrvname, gsslib, target_session_attrs, use_remote_estimate, fdw_startup_cost, fdw_tuple_cost, extensions, updatable, fetch_size, batch_size
 CONTEXT:  SQL statement "ALTER SERVER loopback_nopw OPTIONS (ADD password 'dummypw')"
 PL/pgSQL function inline_code_block line 3 at EXECUTE
 -- If we add a password for our user mapping instead, we should get a different
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index 2b525ea44a..a143a70406 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -177,7 +177,6 @@ ALTER SERVER testserver1 OPTIONS (
 	keepalives_interval 'value',
 	tcp_user_timeout 'value',
 	-- requiressl 'value',
-	sslcompression 'value',
 	sslmode 'value',
 	sslcert 'value',
 	sslkey 'value',
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 5e25f20843..594100fa4d 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -1616,32 +1616,6 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
       </listitem>
      </varlistentry>
 
-     <varlistentry id="libpq-connect-sslcompression" xreflabel="sslcompression">
-      <term><literal>sslcompression</literal></term>
-      <listitem>
-       <para>
-        If set to 1, data sent over SSL connections will be compressed.  If
-        set to 0, compression will be disabled.  The default is 0.  This
-        parameter is ignored if a connection without SSL is made.
-       </para>
-
-       <para>
-        SSL compression is nowadays considered insecure and its use is no
-        longer recommended.  <productname>OpenSSL</productname> 1.1.0 disables
-        compression by default, and many operating system distributions
-        disable it in prior versions as well, so setting this parameter to on
-        will not have any effect if the server does not accept compression.
-       </para>
-
-       <para>
-        If security is not a primary concern, compression can improve
-        throughput if the network is the bottleneck.  Disabling compression
-        can improve response time and throughput if CPU performance is the
-        limiting factor.
-       </para>
-      </listitem>
-     </varlistentry>
-
      <varlistentry id="libpq-connect-sslcert" xreflabel="sslcert">
       <term><literal>sslcert</literal></term>
       <listitem>
@@ -7155,16 +7129,6 @@ myEventProc(PGEventId evtId, void *evtInfo, void *passThrough)
      </para>
     </listitem>
 
-    <listitem>
-     <para>
-      <indexterm>
-       <primary><envar>PGSSLCOMPRESSION</envar></primary>
-      </indexterm>
-      <envar>PGSSLCOMPRESSION</envar> behaves the same as the <xref
-      linkend="libpq-connect-sslcompression"/> connection parameter.
-     </para>
-    </listitem>
-
     <listitem>
      <para>
       <indexterm>
diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c
index 4c4f025eb1..b4800bb147 100644
--- a/src/backend/libpq/be-secure-openssl.c
+++ b/src/backend/libpq/be-secure-openssl.c
@@ -245,6 +245,9 @@ be_tls_init(bool isServerStart)
 	/* disallow SSL session caching, too */
 	SSL_CTX_set_session_cache_mode(context, SSL_SESS_CACHE_OFF);
 
+	/* disallow SSL compression */
+	SSL_CTX_set_options(context, SSL_OP_NO_COMPRESSION);
+
 	/* set up ephemeral DH and ECDH keys */
 	if (!initialize_dh(context, isServerStart))
 		goto error;
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index db71fea169..3c9862d7c4 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -293,10 +293,6 @@ static const internalPQconninfoOption PQconninfoOptions[] = {
 		"SSL-Mode", "", 12,		/* sizeof("verify-full") == 12 */
 	offsetof(struct pg_conn, sslmode)},
 
-	{"sslcompression", "PGSSLCOMPRESSION", "0", NULL,
-		"SSL-Compression", "", 1,
-	offsetof(struct pg_conn, sslcompression)},
-
 	{"sslcert", "PGSSLCERT", NULL, NULL,
 		"SSL-Client-Cert", "", 64,
 	offsetof(struct pg_conn, sslcert)},
@@ -4004,8 +4000,6 @@ freePGconn(PGconn *conn)
 		free(conn->sslcrl);
 	if (conn->sslcrldir)
 		free(conn->sslcrldir);
-	if (conn->sslcompression)
-		free(conn->sslcompression);
 	if (conn->requirepeer)
 		free(conn->requirepeer);
 	if (conn->ssl_min_protocol_version)
diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
index 0fa10a23b4..bee0226552 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -1258,12 +1258,9 @@ initialize_SSL(PGconn *conn)
 		SSL_set_verify(conn->ssl, SSL_VERIFY_PEER, verify_cb);
 
 	/*
-	 * Set compression option if necessary.
+	 * Disable SSL compression
 	 */
-	if (conn->sslcompression && conn->sslcompression[0] == '0')
-		SSL_set_options(conn->ssl, SSL_OP_NO_COMPRESSION);
-	else
-		SSL_clear_options(conn->ssl, SSL_OP_NO_COMPRESSION);
+	SSL_set_options(conn->ssl, SSL_OP_NO_COMPRESSION);
 
 	return 0;
 }
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index ce36aabd25..78cd5bc647 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -356,7 +356,6 @@ struct pg_conn
 	char	   *keepalives_count;	/* maximum number of TCP keepalive
 									 * retransmits */
 	char	   *sslmode;		/* SSL mode (require,prefer,allow,disable) */
-	char	   *sslcompression; /* SSL compression (0 or 1) */
 	char	   *sslkey;			/* client key filename */
 	char	   *sslcert;		/* client certificate filename */
 	char	   *sslpassword;	/* client key file password */
-- 
2.21.1 (Apple Git-122.3)

#2Magnus Hagander
magnus@hagander.net
In reply to: Daniel Gustafsson (#1)
Re: Disallow SSL compression?

On Thu, Feb 18, 2021 at 1:51 PM Daniel Gustafsson <daniel@yesql.se> wrote:

A few years ago we discussed whether to disable SSL compression [0] which ended
up with it being off by default combined with a recommendation against it in
the docs.

OpenSSL themselves disabled SSL compression by default in 2016 in 1.1.0 with
distros often having had it disabled for a long while before then. Further,
TLSv1.3 removes compression entirely on the protocol level mandating that only
NULL compression is allowed in the ClientHello. NSS, which is discussed in
another thread, removed SSL compression entirely in version 3.33 in 2017.

It seems about time to revisit this since it's unlikely to work anywhere but in
a very small subset of system setups (being disabled by default everywhere) and
is thus likely to be very untested at best. There is also the security aspect
which is less clear-cut for us compared to HTTP client/servers, but not refuted
(the linked thread has a good discussion on this).

Agreed. It will also help with not having to implement it in new SSL
implementations I'm sure :)

The attached removes sslcompression to see what it would look like. The server
actively disallows it and the parameter is removed, but the sslcompression
column in the stat view is retained. An alternative could be to retain the
parameter but not act on it in order to not break scripts etc, but that just
postpones the pain until when we inevitably do remove it.

Thoughts? Any reason to keep supporting SSL compression or is it time for v14
to remove it? Are there still users leveraging this for protocol compression
without security making it worthwhile to keep?

When the last round of discussion happened, I had multiple customers
who did exactly that. None of them do that anymore, due to the pain of
making it work...

I think for libpq we want to keep the option for a while but making it
a no-op, to not unnecessarily break systems where people just upgrade
libpq, though. And document it as such having no effect and "will
eventually be removed".

--
Magnus Hagander
Me: https://www.hagander.net/
Work: https://www.redpill-linpro.com/

#3Daniel Gustafsson
daniel@yesql.se
In reply to: Magnus Hagander (#2)
Re: Disallow SSL compression?

On 22 Feb 2021, at 11:52, Magnus Hagander <magnus@hagander.net> wrote:

On Thu, Feb 18, 2021 at 1:51 PM Daniel Gustafsson <daniel@yesql.se> wrote:

A few years ago we discussed whether to disable SSL compression [0] which ended
up with it being off by default combined with a recommendation against it in
the docs.

OpenSSL themselves disabled SSL compression by default in 2016 in 1.1.0 with
distros often having had it disabled for a long while before then. Further,
TLSv1.3 removes compression entirely on the protocol level mandating that only
NULL compression is allowed in the ClientHello. NSS, which is discussed in
another thread, removed SSL compression entirely in version 3.33 in 2017.

It seems about time to revisit this since it's unlikely to work anywhere but in
a very small subset of system setups (being disabled by default everywhere) and
is thus likely to be very untested at best. There is also the security aspect
which is less clear-cut for us compared to HTTP client/servers, but not refuted
(the linked thread has a good discussion on this).

Agreed. It will also help with not having to implement it in new SSL
implementations I'm sure :)

Not really, no other TLS library I would consider using actually has
compression (except maybe wolfSSL?). GnuTLS and NSS both removed it, and
Secure Transport and Schannel never had it AFAIK.

The attached removes sslcompression to see what it would look like. The server
actively disallows it and the parameter is removed, but the sslcompression
column in the stat view is retained. An alternative could be to retain the
parameter but not act on it in order to not break scripts etc, but that just
postpones the pain until when we inevitably do remove it.

Thoughts? Any reason to keep supporting SSL compression or is it time for v14
to remove it? Are there still users leveraging this for protocol compression
without security making it worthwhile to keep?

When the last round of discussion happened, I had multiple customers
who did exactly that. None of them do that anymore, due to the pain of
making it work...

Unsurprisingly.

I think for libpq we want to keep the option for a while but making it
a no-op, to not unnecessarily break systems where people just upgrade
libpq, though. And document it as such having no effect and "will
eventually be removed".

Agreed, that's better.

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

#4Michael Paquier
michael@paquier.xyz
In reply to: Daniel Gustafsson (#3)
Re: Disallow SSL compression?

On Mon, Feb 22, 2021 at 12:27:38PM +0100, Daniel Gustafsson wrote:

On 22 Feb 2021, at 11:52, Magnus Hagander <magnus@hagander.net> wrote:

I think for libpq we want to keep the option for a while but making it
a no-op, to not unnecessarily break systems where people just upgrade
libpq, though. And document it as such having no effect and "will
eventually be removed".

Agreed, that's better.

+1.  There is just pain waiting ahead when breaking connection strings
that used to work previously.  A "while" could take a long time
though, see the case of "tty" that's still around (cb7fb3c).  Could
you update the patch to do that?  This requires an update of
fe-connect.c and libpq.sgml.
--
Michael
#5Magnus Hagander
magnus@hagander.net
In reply to: Daniel Gustafsson (#3)
Re: Disallow SSL compression?

On Mon, Feb 22, 2021 at 12:28 PM Daniel Gustafsson <daniel@yesql.se> wrote:

On 22 Feb 2021, at 11:52, Magnus Hagander <magnus@hagander.net> wrote:

On Thu, Feb 18, 2021 at 1:51 PM Daniel Gustafsson <daniel@yesql.se> wrote:

A few years ago we discussed whether to disable SSL compression [0] which ended
up with it being off by default combined with a recommendation against it in
the docs.

OpenSSL themselves disabled SSL compression by default in 2016 in 1.1.0 with
distros often having had it disabled for a long while before then. Further,
TLSv1.3 removes compression entirely on the protocol level mandating that only
NULL compression is allowed in the ClientHello. NSS, which is discussed in
another thread, removed SSL compression entirely in version 3.33 in 2017.

It seems about time to revisit this since it's unlikely to work anywhere but in
a very small subset of system setups (being disabled by default everywhere) and
is thus likely to be very untested at best. There is also the security aspect
which is less clear-cut for us compared to HTTP client/servers, but not refuted
(the linked thread has a good discussion on this).

Agreed. It will also help with not having to implement it in new SSL
implementations I'm sure :)

Not really, no other TLS library I would consider using actually has
compression (except maybe wolfSSL?). GnuTLS and NSS both removed it, and
Secure Transport and Schannel never had it AFAIK.

Ah, well, you'd still have to implement some empty placeholder :)

The attached removes sslcompression to see what it would look like. The server
actively disallows it and the parameter is removed, but the sslcompression
column in the stat view is retained. An alternative could be to retain the
parameter but not act on it in order to not break scripts etc, but that just
postpones the pain until when we inevitably do remove it.

Thoughts? Any reason to keep supporting SSL compression or is it time for v14
to remove it? Are there still users leveraging this for protocol compression
without security making it worthwhile to keep?

When the last round of discussion happened, I had multiple customers
who did exactly that. None of them do that anymore, due to the pain of
making it work...

Unsurprisingly.

I think for libpq we want to keep the option for a while but making it
a no-op, to not unnecessarily break systems where people just upgrade
libpq, though. And document it as such having no effect and "will
eventually be removed".

Agreed, that's better.

In fact, pg_basebackup with -R will generate a connection string that
includes sslcompression=0 when used today (unless you jump through the
hoops to make it work), so not accepting that noe would definitely
break a lot of things needlessly.

--
Magnus Hagander
Me: https://www.hagander.net/
Work: https://www.redpill-linpro.com/

#6Daniel Gustafsson
daniel@yesql.se
In reply to: Magnus Hagander (#5)
1 attachment(s)
Re: Disallow SSL compression?

On 26 Feb 2021, at 11:02, Magnus Hagander <magnus@hagander.net> wrote:
On Mon, Feb 22, 2021 at 12:28 PM Daniel Gustafsson <daniel@yesql.se> wrote:

On 22 Feb 2021, at 11:52, Magnus Hagander <magnus@hagander.net> wrote:

Agreed. It will also help with not having to implement it in new SSL
implementations I'm sure :)

Not really, no other TLS library I would consider using actually has
compression (except maybe wolfSSL?). GnuTLS and NSS both removed it, and
Secure Transport and Schannel never had it AFAIK.

Ah, well, you'd still have to implement some empty placeholder :)

Correct.

The attached removes sslcompression to see what it would look like. The server
actively disallows it and the parameter is removed, but the sslcompression
column in the stat view is retained. An alternative could be to retain the
parameter but not act on it in order to not break scripts etc, but that just
postpones the pain until when we inevitably do remove it.

Thoughts? Any reason to keep supporting SSL compression or is it time for v14
to remove it? Are there still users leveraging this for protocol compression
without security making it worthwhile to keep?

When the last round of discussion happened, I had multiple customers
who did exactly that. None of them do that anymore, due to the pain of
making it work...

Unsurprisingly.

I think for libpq we want to keep the option for a while but making it
a no-op, to not unnecessarily break systems where people just upgrade
libpq, though. And document it as such having no effect and "will
eventually be removed".

Agreed, that's better.

In fact, pg_basebackup with -R will generate a connection string that
includes sslcompression=0 when used today (unless you jump through the
hoops to make it work), so not accepting that noe would definitely
break a lot of things needlessly.

Yup, and as mentioned elsewhere in the thread the standard way of doing it is
to leave the param behind and just document it as not in use. Attached is a v2
which retains the sslcompression parameter for backwards compatibility.

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

Attachments:

v2-0001-Disallow-SSL-compression.patchapplication/octet-stream; name=v2-0001-Disallow-SSL-compression.patch; x-unix-mode=0644Download
From 12f326a496de53b4c162e7fbf21975ae00289774 Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <daniel@yesql.se>
Date: Wed, 17 Feb 2021 11:15:40 +0100
Subject: [PATCH v2] Disallow SSL compression

SSL compression has been disabled in OpenSSL since version 1.1.0, and
was disabled in many distros long before that.  The most recent TLS
version, TLSv1.3, also disallows compression at the protocol level.
PostgreSQL disabled compression by default years ago and recommended
against using it, this finally removes the feaure while leaving the
parameter to keep applications from breaking.

Discussion:  https://postgr.es/m/595cf3b1-4ffe-7f05-6f72-f72b7afa7993%402ndquadrant.com
Discussion:  https://postgr.es/m/7E384D48-11C5-441B-9EC3-F7DB1F8518F6@yesql.se
---
 doc/src/sgml/libpq.sgml                  | 33 ++----------------------
 src/backend/libpq/be-secure-openssl.c    |  8 +++---
 src/interfaces/libpq/fe-connect.c        |  6 ++++-
 src/interfaces/libpq/fe-secure-openssl.c | 13 +++++-----
 src/interfaces/libpq/libpq-int.h         |  2 +-
 5 files changed, 19 insertions(+), 43 deletions(-)

diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 5e25f20843..4e5de7a75c 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -1620,24 +1620,7 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
       <term><literal>sslcompression</literal></term>
       <listitem>
        <para>
-        If set to 1, data sent over SSL connections will be compressed.  If
-        set to 0, compression will be disabled.  The default is 0.  This
-        parameter is ignored if a connection without SSL is made.
-       </para>
-
-       <para>
-        SSL compression is nowadays considered insecure and its use is no
-        longer recommended.  <productname>OpenSSL</productname> 1.1.0 disables
-        compression by default, and many operating system distributions
-        disable it in prior versions as well, so setting this parameter to on
-        will not have any effect if the server does not accept compression.
-       </para>
-
-       <para>
-        If security is not a primary concern, compression can improve
-        throughput if the network is the bottleneck.  Disabling compression
-        can improve response time and throughput if CPU performance is the
-        limiting factor.
+        Ignored (formerly, this specified whether to attempt SSL compression).
        </para>
       </listitem>
      </varlistentry>
@@ -2488,9 +2471,7 @@ const char *PQsslAttribute(const PGconn *conn, const char *attribute_name);
          <term><literal>compression</literal></term>
           <listitem>
            <para>
-            If SSL compression is in use, returns the name of the compression
-            algorithm, or "on" if compression is used but the algorithm is
-            not known. If compression is not in use, returns "off".
+            Compression is no longer supported, always returns "off".
            </para>
           </listitem>
          </varlistentry>
@@ -7155,16 +7136,6 @@ myEventProc(PGEventId evtId, void *evtInfo, void *passThrough)
      </para>
     </listitem>
 
-    <listitem>
-     <para>
-      <indexterm>
-       <primary><envar>PGSSLCOMPRESSION</envar></primary>
-      </indexterm>
-      <envar>PGSSLCOMPRESSION</envar> behaves the same as the <xref
-      linkend="libpq-connect-sslcompression"/> connection parameter.
-     </para>
-    </listitem>
-
     <listitem>
      <para>
       <indexterm>
diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c
index 4c4f025eb1..118ec8dfa2 100644
--- a/src/backend/libpq/be-secure-openssl.c
+++ b/src/backend/libpq/be-secure-openssl.c
@@ -245,6 +245,9 @@ be_tls_init(bool isServerStart)
 	/* disallow SSL session caching, too */
 	SSL_CTX_set_session_cache_mode(context, SSL_SESS_CACHE_OFF);
 
+	/* disallow SSL compression */
+	SSL_CTX_set_options(context, SSL_OP_NO_COMPRESSION);
+
 	/* set up ephemeral DH and ECDH keys */
 	if (!initialize_dh(context, isServerStart))
 		goto error;
@@ -1185,10 +1188,7 @@ be_tls_get_cipher_bits(Port *port)
 bool
 be_tls_get_compression(Port *port)
 {
-	if (port->ssl)
-		return (SSL_get_current_compression(port->ssl) != NULL);
-	else
-		return false;
+	return false;
 }
 
 const char *
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index db71fea169..a89be0d9d3 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -293,8 +293,12 @@ static const internalPQconninfoOption PQconninfoOptions[] = {
 		"SSL-Mode", "", 12,		/* sizeof("verify-full") == 12 */
 	offsetof(struct pg_conn, sslmode)},
 
+	/*
+	 * "sslcompression" is no longer used, but keep it present for backwards
+	 * compatibility.
+	 */
 	{"sslcompression", "PGSSLCOMPRESSION", "0", NULL,
-		"SSL-Compression", "", 1,
+		"SSL-Compression", "D", 1,
 	offsetof(struct pg_conn, sslcompression)},
 
 	{"sslcert", "PGSSLCERT", NULL, NULL,
diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
index 0fa10a23b4..bc58cdcd47 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -1258,12 +1258,9 @@ initialize_SSL(PGconn *conn)
 		SSL_set_verify(conn->ssl, SSL_VERIFY_PEER, verify_cb);
 
 	/*
-	 * Set compression option if necessary.
+	 * Disable SSL compression
 	 */
-	if (conn->sslcompression && conn->sslcompression[0] == '0')
-		SSL_set_options(conn->ssl, SSL_OP_NO_COMPRESSION);
-	else
-		SSL_clear_options(conn->ssl, SSL_OP_NO_COMPRESSION);
+	SSL_set_options(conn->ssl, SSL_OP_NO_COMPRESSION);
 
 	return 0;
 }
@@ -1553,8 +1550,12 @@ PQsslAttribute(PGconn *conn, const char *attribute_name)
 	if (strcmp(attribute_name, "cipher") == 0)
 		return SSL_get_cipher(conn->ssl);
 
+	/*
+	 * SSL compression is disabled, so even if connecting to an older server
+	 * which still supports it, it wont be active.
+	 */
 	if (strcmp(attribute_name, "compression") == 0)
-		return SSL_get_current_compression(conn->ssl) ? "on" : "off";
+		return "off";
 
 	if (strcmp(attribute_name, "protocol") == 0)
 		return SSL_get_version(conn->ssl);
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index ce36aabd25..66cb24b001 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -356,7 +356,7 @@ struct pg_conn
 	char	   *keepalives_count;	/* maximum number of TCP keepalive
 									 * retransmits */
 	char	   *sslmode;		/* SSL mode (require,prefer,allow,disable) */
-	char	   *sslcompression; /* SSL compression (0 or 1) */
+	char	   *sslcompression; /* SSL compression (OBSOLETE, NOT USED) */
 	char	   *sslkey;			/* client key filename */
 	char	   *sslcert;		/* client certificate filename */
 	char	   *sslpassword;	/* client key file password */
-- 
2.21.1 (Apple Git-122.3)

#7Daniel Gustafsson
daniel@yesql.se
In reply to: Michael Paquier (#4)
Re: Disallow SSL compression?

On 26 Feb 2021, at 03:34, Michael Paquier <michael@paquier.xyz> wrote:

There is just pain waiting ahead when breaking connection strings
that used to work previously. A "while" could take a long time
though, see the case of "tty" that's still around (cb7fb3c).

I see your tty removal from 2003, and raise you one "authtype" which was axed
on January 26 1998 in commit d5bbe2aca55bc833e38c768d7f82, but which is still
around. More on that in a separate thread though.

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

#8Daniel Gustafsson
daniel@yesql.se
In reply to: Daniel Gustafsson (#6)
1 attachment(s)
Re: Disallow SSL compression?

On 26 Feb 2021, at 20:34, Daniel Gustafsson <daniel@yesql.se> wrote:

Attached is a v2 which retains the sslcompression parameter for backwards
compatibility.

And now a v3 which fixes an oversight in postgres_fdw as well as adds an SSL
TAP test to cover deprecated parameters.

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

Attachments:

v3-0001-Disallow-SSL-compression.patchapplication/octet-stream; name=v3-0001-Disallow-SSL-compression.patch; x-unix-mode=0644Download
From 981a55f11d68e1ff4c461e8e1d6184c064852cd6 Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <daniel@yesql.se>
Date: Wed, 17 Feb 2021 11:15:40 +0100
Subject: [PATCH v3] Disallow SSL compression

SSL compression has been disabled in OpenSSL since version 1.1.0, and
was disabled in many distros long before that.  The most recent TLS
version, TLSv1.3, also disallows compression at the protocol level.
PostgreSQL disabled compression by default years ago and recommended
against using it, this finally removes the feaure while leaving the
parameter to keep applications from breaking.

Also add a test for deprecated SSL parameters to ensure backwards
compatibility.

Discussion:  https://postgr.es/m/595cf3b1-4ffe-7f05-6f72-f72b7afa7993%402ndquadrant.com
Discussion:  https://postgr.es/m/7E384D48-11C5-441B-9EC3-F7DB1F8518F6@yesql.se
---
 .../postgres_fdw/expected/postgres_fdw.out    |  4 +--
 contrib/postgres_fdw/sql/postgres_fdw.sql     |  2 +-
 doc/src/sgml/libpq.sgml                       | 33 ++-----------------
 src/backend/libpq/be-secure-openssl.c         |  3 ++
 src/interfaces/libpq/fe-connect.c             |  6 +++-
 src/interfaces/libpq/fe-secure-openssl.c      | 13 ++++----
 src/interfaces/libpq/libpq-int.h              |  2 +-
 src/test/ssl/t/001_ssltests.pl                |  9 ++++-
 8 files changed, 29 insertions(+), 43 deletions(-)

diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 0649b6b81c..01420d93b6 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -163,7 +163,7 @@ ALTER SERVER testserver1 OPTIONS (
 	keepalives_interval 'value',
 	tcp_user_timeout 'value',
 	-- requiressl 'value',
-	sslcompression 'value',
+	-- sslcompression 'value',
 	sslmode 'value',
 	sslcert 'value',
 	sslkey 'value',
@@ -8946,7 +8946,7 @@ DO $d$
     END;
 $d$;
 ERROR:  invalid option "password"
-HINT:  Valid options in this context are: service, passfile, channel_binding, connect_timeout, dbname, host, hostaddr, port, options, application_name, keepalives, keepalives_idle, keepalives_interval, keepalives_count, tcp_user_timeout, sslmode, sslcompression, sslcert, sslkey, sslrootcert, sslcrl, sslcrldir, requirepeer, ssl_min_protocol_version, ssl_max_protocol_version, gssencmode, krbsrvname, gsslib, target_session_attrs, use_remote_estimate, fdw_startup_cost, fdw_tuple_cost, extensions, updatable, fetch_size, batch_size
+HINT:  Valid options in this context are: service, passfile, channel_binding, connect_timeout, dbname, host, hostaddr, port, options, application_name, keepalives, keepalives_idle, keepalives_interval, keepalives_count, tcp_user_timeout, sslmode, sslcert, sslkey, sslrootcert, sslcrl, sslcrldir, requirepeer, ssl_min_protocol_version, ssl_max_protocol_version, gssencmode, krbsrvname, gsslib, target_session_attrs, use_remote_estimate, fdw_startup_cost, fdw_tuple_cost, extensions, updatable, fetch_size, batch_size
 CONTEXT:  SQL statement "ALTER SERVER loopback_nopw OPTIONS (ADD password 'dummypw')"
 PL/pgSQL function inline_code_block line 3 at EXECUTE
 -- If we add a password for our user mapping instead, we should get a different
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index 2b525ea44a..f343162cef 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -177,7 +177,7 @@ ALTER SERVER testserver1 OPTIONS (
 	keepalives_interval 'value',
 	tcp_user_timeout 'value',
 	-- requiressl 'value',
-	sslcompression 'value',
+	-- sslcompression 'value',
 	sslmode 'value',
 	sslcert 'value',
 	sslkey 'value',
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index cc20b0f234..124c53e7b1 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -1635,24 +1635,7 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
       <term><literal>sslcompression</literal></term>
       <listitem>
        <para>
-        If set to 1, data sent over SSL connections will be compressed.  If
-        set to 0, compression will be disabled.  The default is 0.  This
-        parameter is ignored if a connection without SSL is made.
-       </para>
-
-       <para>
-        SSL compression is nowadays considered insecure and its use is no
-        longer recommended.  <productname>OpenSSL</productname> 1.1.0 disables
-        compression by default, and many operating system distributions
-        disable it in prior versions as well, so setting this parameter to on
-        will not have any effect if the server does not accept compression.
-       </para>
-
-       <para>
-        If security is not a primary concern, compression can improve
-        throughput if the network is the bottleneck.  Disabling compression
-        can improve response time and throughput if CPU performance is the
-        limiting factor.
+        Ignored (formerly, this specified whether to attempt SSL compression).
        </para>
       </listitem>
      </varlistentry>
@@ -2562,9 +2545,7 @@ const char *PQsslAttribute(const PGconn *conn, const char *attribute_name);
          <term><literal>compression</literal></term>
           <listitem>
            <para>
-            If SSL compression is in use, returns the name of the compression
-            algorithm, or "on" if compression is used but the algorithm is
-            not known. If compression is not in use, returns "off".
+            Compression is no longer supported, always returns "off".
            </para>
           </listitem>
          </varlistentry>
@@ -7229,16 +7210,6 @@ myEventProc(PGEventId evtId, void *evtInfo, void *passThrough)
      </para>
     </listitem>
 
-    <listitem>
-     <para>
-      <indexterm>
-       <primary><envar>PGSSLCOMPRESSION</envar></primary>
-      </indexterm>
-      <envar>PGSSLCOMPRESSION</envar> behaves the same as the <xref
-      linkend="libpq-connect-sslcompression"/> connection parameter.
-     </para>
-    </listitem>
-
     <listitem>
      <para>
       <indexterm>
diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c
index 4c4f025eb1..b4800bb147 100644
--- a/src/backend/libpq/be-secure-openssl.c
+++ b/src/backend/libpq/be-secure-openssl.c
@@ -245,6 +245,9 @@ be_tls_init(bool isServerStart)
 	/* disallow SSL session caching, too */
 	SSL_CTX_set_session_cache_mode(context, SSL_SESS_CACHE_OFF);
 
+	/* disallow SSL compression */
+	SSL_CTX_set_options(context, SSL_OP_NO_COMPRESSION);
+
 	/* set up ephemeral DH and ECDH keys */
 	if (!initialize_dh(context, isServerStart))
 		goto error;
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 9812a14662..3ff6d4a9ea 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -293,8 +293,12 @@ static const internalPQconninfoOption PQconninfoOptions[] = {
 		"SSL-Mode", "", 12,		/* sizeof("verify-full") == 12 */
 	offsetof(struct pg_conn, sslmode)},
 
+	/*
+	 * "sslcompression" is no longer used, but keep it present for backwards
+	 * compatibility.
+	 */
 	{"sslcompression", "PGSSLCOMPRESSION", "0", NULL,
-		"SSL-Compression", "", 1,
+		"SSL-Compression", "D", 1,
 	offsetof(struct pg_conn, sslcompression)},
 
 	{"sslcert", "PGSSLCERT", NULL, NULL,
diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
index 0fa10a23b4..bc58cdcd47 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -1258,12 +1258,9 @@ initialize_SSL(PGconn *conn)
 		SSL_set_verify(conn->ssl, SSL_VERIFY_PEER, verify_cb);
 
 	/*
-	 * Set compression option if necessary.
+	 * Disable SSL compression
 	 */
-	if (conn->sslcompression && conn->sslcompression[0] == '0')
-		SSL_set_options(conn->ssl, SSL_OP_NO_COMPRESSION);
-	else
-		SSL_clear_options(conn->ssl, SSL_OP_NO_COMPRESSION);
+	SSL_set_options(conn->ssl, SSL_OP_NO_COMPRESSION);
 
 	return 0;
 }
@@ -1553,8 +1550,12 @@ PQsslAttribute(PGconn *conn, const char *attribute_name)
 	if (strcmp(attribute_name, "cipher") == 0)
 		return SSL_get_cipher(conn->ssl);
 
+	/*
+	 * SSL compression is disabled, so even if connecting to an older server
+	 * which still supports it, it wont be active.
+	 */
 	if (strcmp(attribute_name, "compression") == 0)
-		return SSL_get_current_compression(conn->ssl) ? "on" : "off";
+		return "off";
 
 	if (strcmp(attribute_name, "protocol") == 0)
 		return SSL_get_version(conn->ssl);
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index 0c9e95f1a7..d15a447324 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -376,7 +376,7 @@ struct pg_conn
 	char	   *keepalives_count;	/* maximum number of TCP keepalive
 									 * retransmits */
 	char	   *sslmode;		/* SSL mode (require,prefer,allow,disable) */
-	char	   *sslcompression; /* SSL compression (0 or 1) */
+	char	   *sslcompression; /* SSL compression (OBSOLETE, NOT USED) */
 	char	   *sslkey;			/* client key filename */
 	char	   *sslcert;		/* client certificate filename */
 	char	   *sslpassword;	/* client key file password */
diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl
index 864f6e209f..f15b3bf9db 100644
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -17,7 +17,7 @@ if ($ENV{with_ssl} ne 'openssl')
 }
 else
 {
-	plan tests => 100;
+	plan tests => 101;
 }
 
 #### Some configuration
@@ -157,6 +157,13 @@ test_connect_fails(
 	qr/root certificate file "invalid" does not exist/,
 	"connect without server root cert sslmode=verify-full");
 
+# Test deprecated SSL parameters which should be accepted for backwards
+# compatibility
+test_connect_ok(
+	$common_connstr,
+	"sslrootcert=invalid sslmode=require sslcompression=1 requiressl=1",
+	"connect with deprecated connection parameters");
+
 # Try with wrong root cert, should fail. (We're using the client CA as the
 # root, but the server's key is signed by the server CA.)
 test_connect_fails($common_connstr,
-- 
2.21.1 (Apple Git-122.3)

#9Peter Eisentraut
peter.eisentraut@enterprisedb.com
In reply to: Daniel Gustafsson (#8)
Re: Disallow SSL compression?

On 03.03.21 11:31, Daniel Gustafsson wrote:

On 26 Feb 2021, at 20:34, Daniel Gustafsson <daniel@yesql.se> wrote:

Attached is a v2 which retains the sslcompression parameter for backwards
compatibility.

And now a v3 which fixes an oversight in postgres_fdw as well as adds an SSL
TAP test to cover deprecated parameters.

Per your other thread, you should also remove the environment variable.

In postgres_fdw, I think commenting it out is not the right change. The
other commented out values are still valid settings but are omitted from
the test for other reasons. It's not entirely all clear, but we don't
have to keep obsolete stuff in there forever.

#10Michael Paquier
michael@paquier.xyz
In reply to: Peter Eisentraut (#9)
Re: Disallow SSL compression?

On Wed, Mar 03, 2021 at 03:14:01PM +0100, Peter Eisentraut wrote:

Per your other thread, you should also remove the environment variable.

In postgres_fdw, I think commenting it out is not the right change. The
other commented out values are still valid settings but are omitted from the
test for other reasons. It's not entirely all clear, but we don't have to
keep obsolete stuff in there forever.

Agreed on both points. Also, it seems a bit weird to keep around
pg_stat_ssl.compression while we know that it will always be false.
Wouldn't it be better to get rid of that in PgBackendSSLStatus and
pg_stat_ssl then?
--
Michael

#11Daniel Gustafsson
daniel@yesql.se
In reply to: Peter Eisentraut (#9)
1 attachment(s)
Re: Disallow SSL compression?

On 3 Mar 2021, at 15:14, Peter Eisentraut <peter.eisentraut@enterprisedb.com> wrote:

On 03.03.21 11:31, Daniel Gustafsson wrote:

On 26 Feb 2021, at 20:34, Daniel Gustafsson <daniel@yesql.se> wrote:
Attached is a v2 which retains the sslcompression parameter for backwards
compatibility.

And now a v3 which fixes an oversight in postgres_fdw as well as adds an SSL
TAP test to cover deprecated parameters.

Per your other thread, you should also remove the environment variable.

Fixed.

In postgres_fdw, I think commenting it out is not the right change. The other commented out values are still valid settings but are omitted from the test for other reasons. It's not entirely all clear, but we don't have to keep obsolete stuff in there forever.

Ah, I didn't get that distinction but that makes sense. Fixed.

The attached version takes a step further and removes sslcompression from
pg_conn and just eats the value as there is no use in setting a dummy alue. It
also removes compression from PgBackendSSLStatus and be_tls_get_compression as
raised by Michael downthread. I opted for keeping the column in pg_stat_ssl
with a note in the documentation that it will be removed, for the same
backwards compatibility reason of eating the connection param without acting on
it. This might be overthinking it however.

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

Attachments:

v4-0001-Disallow-SSL-compression.patchapplication/octet-stream; name=v4-0001-Disallow-SSL-compression.patch; x-unix-mode=0644Download
From 733ba28a13fd3baf2959061dc660dc6b7e5afd37 Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <daniel@yesql.se>
Date: Thu, 4 Mar 2021 23:10:22 +0100
Subject: [PATCH v4] Disallow SSL compression

SSL compression has been disabled in OpenSSL since version 1.1.0, and
was disabled in many distros long before that.  The most recent TLS
version, TLSv1.3, also disallows compression at the protocol level.
PostgreSQL disabled compression by default years ago and recommended
against using it, this finally removes the feaure while leaving the
parameter to keep applications from breaking. The compression field
in pg_stat_ssl is kept for a set number of releases to avoid breaking
applications consuming it. On top of removing the ability to activate
compression by configuration, compression is actively disabled in
both frontend and backend to avoid overrides from local openssl
configurations.

Also add a test for deprecated SSL parameters to ensure backwards
compatibility.

Discussion:  https://postgr.es/m/595cf3b1-4ffe-7f05-6f72-f72b7afa7993%402ndquadrant.com
Discussion:  https://postgr.es/m/7E384D48-11C5-441B-9EC3-F7DB1F8518F6@yesql.se
---
 .../postgres_fdw/expected/postgres_fdw.out    |  3 +-
 contrib/postgres_fdw/sql/postgres_fdw.sql     |  1 -
 doc/src/sgml/libpq.sgml                       | 33 ++-----------------
 doc/src/sgml/monitoring.sgml                  |  5 +--
 src/backend/libpq/be-secure-openssl.c         | 12 ++-----
 src/backend/postmaster/pgstat.c               |  1 -
 src/backend/utils/adt/pgstatfuncs.c           |  2 +-
 src/backend/utils/init/postinit.c             |  5 ++-
 src/include/libpq/libpq-be.h                  |  1 -
 src/include/pgstat.h                          |  1 -
 src/interfaces/libpq/fe-connect.c             | 11 ++++---
 src/interfaces/libpq/fe-secure-openssl.c      | 13 ++++----
 src/interfaces/libpq/libpq-int.h              |  1 -
 src/test/ssl/t/001_ssltests.pl                |  9 ++++-
 14 files changed, 33 insertions(+), 65 deletions(-)

diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 0649b6b81c..c2565dfc70 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -163,7 +163,6 @@ ALTER SERVER testserver1 OPTIONS (
 	keepalives_interval 'value',
 	tcp_user_timeout 'value',
 	-- requiressl 'value',
-	sslcompression 'value',
 	sslmode 'value',
 	sslcert 'value',
 	sslkey 'value',
@@ -8946,7 +8945,7 @@ DO $d$
     END;
 $d$;
 ERROR:  invalid option "password"
-HINT:  Valid options in this context are: service, passfile, channel_binding, connect_timeout, dbname, host, hostaddr, port, options, application_name, keepalives, keepalives_idle, keepalives_interval, keepalives_count, tcp_user_timeout, sslmode, sslcompression, sslcert, sslkey, sslrootcert, sslcrl, sslcrldir, requirepeer, ssl_min_protocol_version, ssl_max_protocol_version, gssencmode, krbsrvname, gsslib, target_session_attrs, use_remote_estimate, fdw_startup_cost, fdw_tuple_cost, extensions, updatable, fetch_size, batch_size
+HINT:  Valid options in this context are: service, passfile, channel_binding, connect_timeout, dbname, host, hostaddr, port, options, application_name, keepalives, keepalives_idle, keepalives_interval, keepalives_count, tcp_user_timeout, sslmode, sslcert, sslkey, sslrootcert, sslcrl, sslcrldir, requirepeer, ssl_min_protocol_version, ssl_max_protocol_version, gssencmode, krbsrvname, gsslib, target_session_attrs, use_remote_estimate, fdw_startup_cost, fdw_tuple_cost, extensions, updatable, fetch_size, batch_size
 CONTEXT:  SQL statement "ALTER SERVER loopback_nopw OPTIONS (ADD password 'dummypw')"
 PL/pgSQL function inline_code_block line 3 at EXECUTE
 -- If we add a password for our user mapping instead, we should get a different
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index 2b525ea44a..a143a70406 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -177,7 +177,6 @@ ALTER SERVER testserver1 OPTIONS (
 	keepalives_interval 'value',
 	tcp_user_timeout 'value',
 	-- requiressl 'value',
-	sslcompression 'value',
 	sslmode 'value',
 	sslcert 'value',
 	sslkey 'value',
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 0553279314..eb9bc7084d 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -1635,24 +1635,7 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
       <term><literal>sslcompression</literal></term>
       <listitem>
        <para>
-        If set to 1, data sent over SSL connections will be compressed.  If
-        set to 0, compression will be disabled.  The default is 0.  This
-        parameter is ignored if a connection without SSL is made.
-       </para>
-
-       <para>
-        SSL compression is nowadays considered insecure and its use is no
-        longer recommended.  <productname>OpenSSL</productname> 1.1.0 disables
-        compression by default, and many operating system distributions
-        disable it in prior versions as well, so setting this parameter to on
-        will not have any effect if the server does not accept compression.
-       </para>
-
-       <para>
-        If security is not a primary concern, compression can improve
-        throughput if the network is the bottleneck.  Disabling compression
-        can improve response time and throughput if CPU performance is the
-        limiting factor.
+        Ignored (formerly, this specified whether to attempt SSL compression).
        </para>
       </listitem>
      </varlistentry>
@@ -2545,9 +2528,7 @@ const char *PQsslAttribute(const PGconn *conn, const char *attribute_name);
          <term><literal>compression</literal></term>
           <listitem>
            <para>
-            If SSL compression is in use, returns the name of the compression
-            algorithm, or "on" if compression is used but the algorithm is
-            not known. If compression is not in use, returns "off".
+            Compression is no longer supported, always returns "off".
            </para>
           </listitem>
          </varlistentry>
@@ -7182,16 +7163,6 @@ myEventProc(PGEventId evtId, void *evtInfo, void *passThrough)
      </para>
     </listitem>
 
-    <listitem>
-     <para>
-      <indexterm>
-       <primary><envar>PGSSLCOMPRESSION</envar></primary>
-      </indexterm>
-      <envar>PGSSLCOMPRESSION</envar> behaves the same as the <xref
-      linkend="libpq-connect-sslcompression"/> connection parameter.
-     </para>
-    </listitem>
-
     <listitem>
      <para>
       <indexterm>
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 3513e127b7..419a101697 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -3070,8 +3070,9 @@ SELECT pid, wait_event_type, wait_event FROM pg_stat_activity WHERE wait_event i
        <structfield>compression</structfield> <type>boolean</type>
       </para>
       <para>
-       True if SSL compression is in use, false if not,
-       or NULL if SSL is not in use on this connection
+       SSL compression is deprecated, this field is always false. This field
+       will be removed in a future version of
+       <productname>PostgreSQL</productname>
       </para></entry>
      </row>
 
diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c
index 4c4f025eb1..8c37381add 100644
--- a/src/backend/libpq/be-secure-openssl.c
+++ b/src/backend/libpq/be-secure-openssl.c
@@ -245,6 +245,9 @@ be_tls_init(bool isServerStart)
 	/* disallow SSL session caching, too */
 	SSL_CTX_set_session_cache_mode(context, SSL_SESS_CACHE_OFF);
 
+	/* disallow SSL compression */
+	SSL_CTX_set_options(context, SSL_OP_NO_COMPRESSION);
+
 	/* set up ephemeral DH and ECDH keys */
 	if (!initialize_dh(context, isServerStart))
 		goto error;
@@ -1182,15 +1185,6 @@ be_tls_get_cipher_bits(Port *port)
 		return 0;
 }
 
-bool
-be_tls_get_compression(Port *port)
-{
-	if (port->ssl)
-		return (SSL_get_current_compression(port->ssl) != NULL);
-	else
-		return false;
-}
-
 const char *
 be_tls_get_version(Port *port)
 {
diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index f75b52719d..9259dc9d3e 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -3215,7 +3215,6 @@ pgstat_bestart(void)
 	{
 		lbeentry.st_ssl = true;
 		lsslstatus.ssl_bits = be_tls_get_cipher_bits(MyProcPort);
-		lsslstatus.ssl_compression = be_tls_get_compression(MyProcPort);
 		strlcpy(lsslstatus.ssl_version, be_tls_get_version(MyProcPort), NAMEDATALEN);
 		strlcpy(lsslstatus.ssl_cipher, be_tls_get_cipher(MyProcPort), NAMEDATALEN);
 		be_tls_get_peer_subject_name(MyProcPort, lsslstatus.ssl_client_dn, NAMEDATALEN);
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 62bff52638..75f7b33410 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -875,7 +875,7 @@ pg_stat_get_activity(PG_FUNCTION_ARGS)
 				values[19] = CStringGetTextDatum(beentry->st_sslstatus->ssl_version);
 				values[20] = CStringGetTextDatum(beentry->st_sslstatus->ssl_cipher);
 				values[21] = Int32GetDatum(beentry->st_sslstatus->ssl_bits);
-				values[22] = BoolGetDatum(beentry->st_sslstatus->ssl_compression);
+				values[22] = BoolGetDatum(false);	/* compression deprecated */
 
 				if (beentry->st_sslstatus->ssl_client_dn[0])
 					values[23] = CStringGetTextDatum(beentry->st_sslstatus->ssl_client_dn);
diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c
index e5965bc517..7abeccb536 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -264,11 +264,10 @@ PerformAuthentication(Port *port)
 
 #ifdef USE_SSL
 		if (port->ssl_in_use)
-			appendStringInfo(&logmsg, _(" SSL enabled (protocol=%s, cipher=%s, bits=%d, compression=%s)"),
+			appendStringInfo(&logmsg, _(" SSL enabled (protocol=%s, cipher=%s, bits=%d)"),
 							 be_tls_get_version(port),
 							 be_tls_get_cipher(port),
-							 be_tls_get_cipher_bits(port),
-							 be_tls_get_compression(port) ? _("on") : _("off"));
+							 be_tls_get_cipher_bits(port));
 #endif
 #ifdef ENABLE_GSS
 		if (port->gss)
diff --git a/src/include/libpq/libpq-be.h b/src/include/libpq/libpq-be.h
index 7be1a67d69..30fb4e613d 100644
--- a/src/include/libpq/libpq-be.h
+++ b/src/include/libpq/libpq-be.h
@@ -266,7 +266,6 @@ extern ssize_t be_tls_write(Port *port, void *ptr, size_t len, int *waitfor);
  * Return information about the SSL connection.
  */
 extern int	be_tls_get_cipher_bits(Port *port);
-extern bool be_tls_get_compression(Port *port);
 extern const char *be_tls_get_version(Port *port);
 extern const char *be_tls_get_cipher(Port *port);
 extern void be_tls_get_peer_subject_name(Port *port, char *ptr, size_t len);
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index 724068cf87..e0c70d221b 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -1135,7 +1135,6 @@ typedef struct PgBackendSSLStatus
 {
 	/* Information about SSL connection */
 	int			ssl_bits;
-	bool		ssl_compression;
 	char		ssl_version[NAMEDATALEN];
 	char		ssl_cipher[NAMEDATALEN];
 	char		ssl_client_dn[NAMEDATALEN];
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index f83af03d0a..459c40bfac 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -293,9 +293,12 @@ static const internalPQconninfoOption PQconninfoOptions[] = {
 		"SSL-Mode", "", 12,		/* sizeof("verify-full") == 12 */
 	offsetof(struct pg_conn, sslmode)},
 
-	{"sslcompression", "PGSSLCOMPRESSION", "0", NULL,
-		"SSL-Compression", "", 1,
-	offsetof(struct pg_conn, sslcompression)},
+	/*
+	 * "sslcompression" is no longer used, but keep it present for backwards
+	 * compatibility.
+	 */
+	{"sslcompression", NULL, NULL, NULL,
+		"SSL-Compression", "D", 1, -1},
 
 	{"sslcert", "PGSSLCERT", NULL, NULL,
 		"SSL-Client-Cert", "", 64,
@@ -4080,8 +4083,6 @@ freePGconn(PGconn *conn)
 		free(conn->sslcrl);
 	if (conn->sslcrldir)
 		free(conn->sslcrldir);
-	if (conn->sslcompression)
-		free(conn->sslcompression);
 	if (conn->requirepeer)
 		free(conn->requirepeer);
 	if (conn->ssl_min_protocol_version)
diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
index 0fa10a23b4..bc58cdcd47 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -1258,12 +1258,9 @@ initialize_SSL(PGconn *conn)
 		SSL_set_verify(conn->ssl, SSL_VERIFY_PEER, verify_cb);
 
 	/*
-	 * Set compression option if necessary.
+	 * Disable SSL compression
 	 */
-	if (conn->sslcompression && conn->sslcompression[0] == '0')
-		SSL_set_options(conn->ssl, SSL_OP_NO_COMPRESSION);
-	else
-		SSL_clear_options(conn->ssl, SSL_OP_NO_COMPRESSION);
+	SSL_set_options(conn->ssl, SSL_OP_NO_COMPRESSION);
 
 	return 0;
 }
@@ -1553,8 +1550,12 @@ PQsslAttribute(PGconn *conn, const char *attribute_name)
 	if (strcmp(attribute_name, "cipher") == 0)
 		return SSL_get_cipher(conn->ssl);
 
+	/*
+	 * SSL compression is disabled, so even if connecting to an older server
+	 * which still supports it, it wont be active.
+	 */
 	if (strcmp(attribute_name, "compression") == 0)
-		return SSL_get_current_compression(conn->ssl) ? "on" : "off";
+		return "off";
 
 	if (strcmp(attribute_name, "protocol") == 0)
 		return SSL_get_version(conn->ssl);
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index 8d51e6ed9f..cca98c14bf 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -360,7 +360,6 @@ struct pg_conn
 	char	   *keepalives_count;	/* maximum number of TCP keepalive
 									 * retransmits */
 	char	   *sslmode;		/* SSL mode (require,prefer,allow,disable) */
-	char	   *sslcompression; /* SSL compression (0 or 1) */
 	char	   *sslkey;			/* client key filename */
 	char	   *sslcert;		/* client certificate filename */
 	char	   *sslpassword;	/* client key file password */
diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl
index 864f6e209f..f15b3bf9db 100644
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -17,7 +17,7 @@ if ($ENV{with_ssl} ne 'openssl')
 }
 else
 {
-	plan tests => 100;
+	plan tests => 101;
 }
 
 #### Some configuration
@@ -157,6 +157,13 @@ test_connect_fails(
 	qr/root certificate file "invalid" does not exist/,
 	"connect without server root cert sslmode=verify-full");
 
+# Test deprecated SSL parameters which should be accepted for backwards
+# compatibility
+test_connect_ok(
+	$common_connstr,
+	"sslrootcert=invalid sslmode=require sslcompression=1 requiressl=1",
+	"connect with deprecated connection parameters");
+
 # Try with wrong root cert, should fail. (We're using the client CA as the
 # root, but the server's key is signed by the server CA.)
 test_connect_fails($common_connstr,
-- 
2.21.1 (Apple Git-122.3)

#12Daniel Gustafsson
daniel@yesql.se
In reply to: Michael Paquier (#10)
Re: Disallow SSL compression?

On 4 Mar 2021, at 11:59, Michael Paquier <michael@paquier.xyz> wrote:

On Wed, Mar 03, 2021 at 03:14:01PM +0100, Peter Eisentraut wrote:

Per your other thread, you should also remove the environment variable.

In postgres_fdw, I think commenting it out is not the right change. The
other commented out values are still valid settings but are omitted from the
test for other reasons. It's not entirely all clear, but we don't have to
keep obsolete stuff in there forever.

Agreed on both points. Also, it seems a bit weird to keep around
pg_stat_ssl.compression while we know that it will always be false.
Wouldn't it be better to get rid of that in PgBackendSSLStatus and
pg_stat_ssl then?

Fixed in the v4 posted just now, although I opted for keeping the column in
pg_stat_ssl for backwards compatibility with a doc note.

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

#13Michael Paquier
michael@paquier.xyz
In reply to: Daniel Gustafsson (#11)
Re: Disallow SSL compression?

On Thu, Mar 04, 2021 at 11:52:56PM +0100, Daniel Gustafsson wrote:

The attached version takes a step further and removes sslcompression from
pg_conn and just eats the value as there is no use in setting a dummy alue. It
also removes compression from PgBackendSSLStatus and be_tls_get_compression as
raised by Michael downthread. I opted for keeping the column in pg_stat_ssl
with a note in the documentation that it will be removed, for the same
backwards compatibility reason of eating the connection param without acting on
it. This might be overthinking it however.

FWIW, I would vote to nuke it from all those places, reducing a bit
pg_stat_get_activity() while on it. Keeping it around in the system
catalogs may cause confusion IMHO, by making people think that it is
still possible to get into configurations where sslcompression could
be really enabled. The rest of the patch looks fine to me.
--
Michael

#14Daniel Gustafsson
daniel@yesql.se
In reply to: Michael Paquier (#13)
1 attachment(s)
Re: Disallow SSL compression?

On 5 Mar 2021, at 08:04, Michael Paquier <michael@paquier.xyz> wrote:

On Thu, Mar 04, 2021 at 11:52:56PM +0100, Daniel Gustafsson wrote:

The attached version takes a step further and removes sslcompression from
pg_conn and just eats the value as there is no use in setting a dummy alue. It
also removes compression from PgBackendSSLStatus and be_tls_get_compression as
raised by Michael downthread. I opted for keeping the column in pg_stat_ssl
with a note in the documentation that it will be removed, for the same
backwards compatibility reason of eating the connection param without acting on
it. This might be overthinking it however.

FWIW, I would vote to nuke it from all those places, reducing a bit
pg_stat_get_activity() while on it. Keeping it around in the system
catalogs may cause confusion IMHO, by making people think that it is
still possible to get into configurations where sslcompression could
be really enabled. The rest of the patch looks fine to me.

Attached is a version which removes that as well. I left the compression
keyword in PQsslAttribute on purpose, not really for backwards compatibility
(PQsslAttributeNames takes care of that) but rather since it's a more generic
connection-info function.

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

Attachments:

v5-0001-Disallow-SSL-compression.patchapplication/octet-stream; name=v5-0001-Disallow-SSL-compression.patch; x-unix-mode=0644Download
From 01e00ce024b2885902b516a5052c9601c73db074 Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <daniel@yesql.se>
Date: Fri, 5 Mar 2021 13:20:00 +0100
Subject: [PATCH v5] Disallow SSL compression

SSL compression has been disabled in OpenSSL since version 1.1.0, and
was disabled in many distros long before that.  The most recent TLS
version, TLSv1.3, also disallows compression at the protocol level.
PostgreSQL disabled compression by default years ago and recommended
against using it, this finally removes the feaure while leaving the
parameter to keep applications from breaking.  On top of removing the
ability to activate compression by configuration, compression is
actively disabled in both frontend and backend to avoid overrides from
local openssl configurations.

Also add a test for deprecated SSL parameters to ensure backwards
compatibility.

Discussion:  https://postgr.es/m/595cf3b1-4ffe-7f05-6f72-f72b7afa7993%402ndquadrant.com
Discussion:  https://postgr.es/m/7E384D48-11C5-441B-9EC3-F7DB1F8518F6@yesql.se
---
 .../postgres_fdw/expected/postgres_fdw.out    |  3 +-
 contrib/postgres_fdw/sql/postgres_fdw.sql     |  1 -
 doc/src/sgml/libpq.sgml                       | 33 ++---------------
 doc/src/sgml/monitoring.sgml                  |  5 +--
 src/backend/catalog/system_views.sql          |  1 -
 src/backend/libpq/be-secure-openssl.c         | 12 ++-----
 src/backend/postmaster/pgstat.c               |  1 -
 src/backend/utils/adt/pgstatfuncs.c           | 36 +++++++++----------
 src/backend/utils/init/postinit.c             |  5 ++-
 src/bin/psql/command.c                        |  7 ++--
 src/include/catalog/pg_proc.dat               |  6 ++--
 src/include/libpq/libpq-be.h                  |  1 -
 src/include/pgstat.h                          |  1 -
 src/interfaces/libpq/fe-connect.c             | 11 +++---
 src/interfaces/libpq/fe-secure-openssl.c      | 13 +++----
 src/interfaces/libpq/libpq-int.h              |  1 -
 src/test/regress/expected/rules.out           |  9 +++--
 src/test/ssl/t/001_ssltests.pl                | 17 ++++++---
 18 files changed, 62 insertions(+), 101 deletions(-)

diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 0649b6b81c..c2565dfc70 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -163,7 +163,6 @@ ALTER SERVER testserver1 OPTIONS (
 	keepalives_interval 'value',
 	tcp_user_timeout 'value',
 	-- requiressl 'value',
-	sslcompression 'value',
 	sslmode 'value',
 	sslcert 'value',
 	sslkey 'value',
@@ -8946,7 +8945,7 @@ DO $d$
     END;
 $d$;
 ERROR:  invalid option "password"
-HINT:  Valid options in this context are: service, passfile, channel_binding, connect_timeout, dbname, host, hostaddr, port, options, application_name, keepalives, keepalives_idle, keepalives_interval, keepalives_count, tcp_user_timeout, sslmode, sslcompression, sslcert, sslkey, sslrootcert, sslcrl, sslcrldir, requirepeer, ssl_min_protocol_version, ssl_max_protocol_version, gssencmode, krbsrvname, gsslib, target_session_attrs, use_remote_estimate, fdw_startup_cost, fdw_tuple_cost, extensions, updatable, fetch_size, batch_size
+HINT:  Valid options in this context are: service, passfile, channel_binding, connect_timeout, dbname, host, hostaddr, port, options, application_name, keepalives, keepalives_idle, keepalives_interval, keepalives_count, tcp_user_timeout, sslmode, sslcert, sslkey, sslrootcert, sslcrl, sslcrldir, requirepeer, ssl_min_protocol_version, ssl_max_protocol_version, gssencmode, krbsrvname, gsslib, target_session_attrs, use_remote_estimate, fdw_startup_cost, fdw_tuple_cost, extensions, updatable, fetch_size, batch_size
 CONTEXT:  SQL statement "ALTER SERVER loopback_nopw OPTIONS (ADD password 'dummypw')"
 PL/pgSQL function inline_code_block line 3 at EXECUTE
 -- If we add a password for our user mapping instead, we should get a different
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index 2b525ea44a..a143a70406 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -177,7 +177,6 @@ ALTER SERVER testserver1 OPTIONS (
 	keepalives_interval 'value',
 	tcp_user_timeout 'value',
 	-- requiressl 'value',
-	sslcompression 'value',
 	sslmode 'value',
 	sslcert 'value',
 	sslkey 'value',
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 0553279314..eb9bc7084d 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -1635,24 +1635,7 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
       <term><literal>sslcompression</literal></term>
       <listitem>
        <para>
-        If set to 1, data sent over SSL connections will be compressed.  If
-        set to 0, compression will be disabled.  The default is 0.  This
-        parameter is ignored if a connection without SSL is made.
-       </para>
-
-       <para>
-        SSL compression is nowadays considered insecure and its use is no
-        longer recommended.  <productname>OpenSSL</productname> 1.1.0 disables
-        compression by default, and many operating system distributions
-        disable it in prior versions as well, so setting this parameter to on
-        will not have any effect if the server does not accept compression.
-       </para>
-
-       <para>
-        If security is not a primary concern, compression can improve
-        throughput if the network is the bottleneck.  Disabling compression
-        can improve response time and throughput if CPU performance is the
-        limiting factor.
+        Ignored (formerly, this specified whether to attempt SSL compression).
        </para>
       </listitem>
      </varlistentry>
@@ -2545,9 +2528,7 @@ const char *PQsslAttribute(const PGconn *conn, const char *attribute_name);
          <term><literal>compression</literal></term>
           <listitem>
            <para>
-            If SSL compression is in use, returns the name of the compression
-            algorithm, or "on" if compression is used but the algorithm is
-            not known. If compression is not in use, returns "off".
+            Compression is no longer supported, always returns "off".
            </para>
           </listitem>
          </varlistentry>
@@ -7182,16 +7163,6 @@ myEventProc(PGEventId evtId, void *evtInfo, void *passThrough)
      </para>
     </listitem>
 
-    <listitem>
-     <para>
-      <indexterm>
-       <primary><envar>PGSSLCOMPRESSION</envar></primary>
-      </indexterm>
-      <envar>PGSSLCOMPRESSION</envar> behaves the same as the <xref
-      linkend="libpq-connect-sslcompression"/> connection parameter.
-     </para>
-    </listitem>
-
     <listitem>
      <para>
       <indexterm>
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 3513e127b7..419a101697 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -3070,8 +3070,9 @@ SELECT pid, wait_event_type, wait_event FROM pg_stat_activity WHERE wait_event i
        <structfield>compression</structfield> <type>boolean</type>
       </para>
       <para>
-       True if SSL compression is in use, false if not,
-       or NULL if SSL is not in use on this connection
+       SSL compression is deprecated, this field is always false. This field
+       will be removed in a future version of
+       <productname>PostgreSQL</productname>
       </para></entry>
      </row>
 
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index fc94a73a54..fb1116d09a 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -863,7 +863,6 @@ CREATE VIEW pg_stat_ssl AS
             S.sslversion AS version,
             S.sslcipher AS cipher,
             S.sslbits AS bits,
-            S.sslcompression AS compression,
             S.ssl_client_dn AS client_dn,
             S.ssl_client_serial AS client_serial,
             S.ssl_issuer_dn AS issuer_dn
diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c
index 4c4f025eb1..8c37381add 100644
--- a/src/backend/libpq/be-secure-openssl.c
+++ b/src/backend/libpq/be-secure-openssl.c
@@ -245,6 +245,9 @@ be_tls_init(bool isServerStart)
 	/* disallow SSL session caching, too */
 	SSL_CTX_set_session_cache_mode(context, SSL_SESS_CACHE_OFF);
 
+	/* disallow SSL compression */
+	SSL_CTX_set_options(context, SSL_OP_NO_COMPRESSION);
+
 	/* set up ephemeral DH and ECDH keys */
 	if (!initialize_dh(context, isServerStart))
 		goto error;
@@ -1182,15 +1185,6 @@ be_tls_get_cipher_bits(Port *port)
 		return 0;
 }
 
-bool
-be_tls_get_compression(Port *port)
-{
-	if (port->ssl)
-		return (SSL_get_current_compression(port->ssl) != NULL);
-	else
-		return false;
-}
-
 const char *
 be_tls_get_version(Port *port)
 {
diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index f75b52719d..9259dc9d3e 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -3215,7 +3215,6 @@ pgstat_bestart(void)
 	{
 		lbeentry.st_ssl = true;
 		lsslstatus.ssl_bits = be_tls_get_cipher_bits(MyProcPort);
-		lsslstatus.ssl_compression = be_tls_get_compression(MyProcPort);
 		strlcpy(lsslstatus.ssl_version, be_tls_get_version(MyProcPort), NAMEDATALEN);
 		strlcpy(lsslstatus.ssl_cipher, be_tls_get_cipher(MyProcPort), NAMEDATALEN);
 		be_tls_get_peer_subject_name(MyProcPort, lsslstatus.ssl_client_dn, NAMEDATALEN);
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 62bff52638..318ce154fd 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -569,7 +569,7 @@ pg_stat_get_progress_info(PG_FUNCTION_ARGS)
 Datum
 pg_stat_get_activity(PG_FUNCTION_ARGS)
 {
-#define PG_STAT_GET_ACTIVITY_COLS	30
+#define PG_STAT_GET_ACTIVITY_COLS	29
 	int			num_backends = pgstat_fetch_stat_numbackends();
 	int			curr_backend;
 	int			pid = PG_ARGISNULL(0) ? -1 : PG_GETARG_INT32(0);
@@ -708,7 +708,7 @@ pg_stat_get_activity(PG_FUNCTION_ARGS)
 			pfree(clipped_activity);
 
 			/* leader_pid */
-			nulls[29] = true;
+			nulls[28] = true;
 
 			proc = BackendPidGetProc(beentry->st_procpid);
 
@@ -745,8 +745,8 @@ pg_stat_get_activity(PG_FUNCTION_ARGS)
 				 */
 				if (leader && leader->pid != beentry->st_procpid)
 				{
-					values[29] = Int32GetDatum(leader->pid);
-					nulls[29] = false;
+					values[28] = Int32GetDatum(leader->pid);
+					nulls[28] = false;
 				}
 			}
 
@@ -875,44 +875,43 @@ pg_stat_get_activity(PG_FUNCTION_ARGS)
 				values[19] = CStringGetTextDatum(beentry->st_sslstatus->ssl_version);
 				values[20] = CStringGetTextDatum(beentry->st_sslstatus->ssl_cipher);
 				values[21] = Int32GetDatum(beentry->st_sslstatus->ssl_bits);
-				values[22] = BoolGetDatum(beentry->st_sslstatus->ssl_compression);
 
 				if (beentry->st_sslstatus->ssl_client_dn[0])
-					values[23] = CStringGetTextDatum(beentry->st_sslstatus->ssl_client_dn);
+					values[22] = CStringGetTextDatum(beentry->st_sslstatus->ssl_client_dn);
 				else
-					nulls[23] = true;
+					nulls[22] = true;
 
 				if (beentry->st_sslstatus->ssl_client_serial[0])
-					values[24] = DirectFunctionCall3(numeric_in,
+					values[23] = DirectFunctionCall3(numeric_in,
 													 CStringGetDatum(beentry->st_sslstatus->ssl_client_serial),
 													 ObjectIdGetDatum(InvalidOid),
 													 Int32GetDatum(-1));
 				else
-					nulls[24] = true;
+					nulls[23] = true;
 
 				if (beentry->st_sslstatus->ssl_issuer_dn[0])
-					values[25] = CStringGetTextDatum(beentry->st_sslstatus->ssl_issuer_dn);
+					values[24] = CStringGetTextDatum(beentry->st_sslstatus->ssl_issuer_dn);
 				else
-					nulls[25] = true;
+					nulls[24] = true;
 			}
 			else
 			{
 				values[18] = BoolGetDatum(false);	/* ssl */
-				nulls[19] = nulls[20] = nulls[21] = nulls[22] = nulls[23] = nulls[24] = nulls[25] = true;
+				nulls[19] = nulls[20] = nulls[21] = nulls[22] = nulls[23] = nulls[24] = true;
 			}
 
 			/* GSSAPI information */
 			if (beentry->st_gss)
 			{
-				values[26] = BoolGetDatum(beentry->st_gssstatus->gss_auth); /* gss_auth */
-				values[27] = CStringGetTextDatum(beentry->st_gssstatus->gss_princ);
-				values[28] = BoolGetDatum(beentry->st_gssstatus->gss_enc);	/* GSS Encryption in use */
+				values[25] = BoolGetDatum(beentry->st_gssstatus->gss_auth); /* gss_auth */
+				values[26] = CStringGetTextDatum(beentry->st_gssstatus->gss_princ);
+				values[27] = BoolGetDatum(beentry->st_gssstatus->gss_enc);	/* GSS Encryption in use */
 			}
 			else
 			{
-				values[26] = BoolGetDatum(false);	/* gss_auth */
-				nulls[27] = true;	/* No GSS principal */
-				values[28] = BoolGetDatum(false);	/* GSS Encryption not in
+				values[25] = BoolGetDatum(false);	/* gss_auth */
+				nulls[26] = true;	/* No GSS principal */
+				values[27] = BoolGetDatum(false);	/* GSS Encryption not in
 													 * use */
 			}
 		}
@@ -942,7 +941,6 @@ pg_stat_get_activity(PG_FUNCTION_ARGS)
 			nulls[26] = true;
 			nulls[27] = true;
 			nulls[28] = true;
-			nulls[29] = true;
 		}
 
 		tuplestore_putvalues(tupstore, tupdesc, values, nulls);
diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c
index e5965bc517..7abeccb536 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -264,11 +264,10 @@ PerformAuthentication(Port *port)
 
 #ifdef USE_SSL
 		if (port->ssl_in_use)
-			appendStringInfo(&logmsg, _(" SSL enabled (protocol=%s, cipher=%s, bits=%d, compression=%s)"),
+			appendStringInfo(&logmsg, _(" SSL enabled (protocol=%s, cipher=%s, bits=%d)"),
 							 be_tls_get_version(port),
 							 be_tls_get_cipher(port),
-							 be_tls_get_cipher_bits(port),
-							 be_tls_get_compression(port) ? _("on") : _("off"));
+							 be_tls_get_cipher_bits(port));
 #endif
 #ifdef ENABLE_GSS
 		if (port->gss)
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index c98e3d31d0..8d6970a4f3 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -3509,7 +3509,6 @@ printSSLInfo(void)
 	const char *protocol;
 	const char *cipher;
 	const char *bits;
-	const char *compression;
 
 	if (!PQsslInUse(pset.db))
 		return;					/* no SSL */
@@ -3517,13 +3516,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, bits: %s)\n"),
 		   protocol ? protocol : _("unknown"),
 		   cipher ? cipher : _("unknown"),
-		   bits ? bits : _("unknown"),
-		   (compression && strcmp(compression, "off") != 0) ? _("on") : _("off"));
+		   bits ? bits : _("unknown"));
 }
 
 /*
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 59d2b71ca9..bf46c6bf65 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -5249,9 +5249,9 @@
   proname => 'pg_stat_get_activity', prorows => '100', proisstrict => 'f',
   proretset => 't', provolatile => 's', proparallel => 'r',
   prorettype => 'record', proargtypes => 'int4',
-  proallargtypes => '{int4,oid,int4,oid,text,text,text,text,text,timestamptz,timestamptz,timestamptz,timestamptz,inet,text,int4,xid,xid,text,bool,text,text,int4,bool,text,numeric,text,bool,text,bool,int4}',
-  proargmodes => '{i,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o}',
-  proargnames => '{pid,datid,pid,usesysid,application_name,state,query,wait_event_type,wait_event,xact_start,query_start,backend_start,state_change,client_addr,client_hostname,client_port,backend_xid,backend_xmin,backend_type,ssl,sslversion,sslcipher,sslbits,sslcompression,ssl_client_dn,ssl_client_serial,ssl_issuer_dn,gss_auth,gss_princ,gss_enc,leader_pid}',
+  proallargtypes => '{int4,oid,int4,oid,text,text,text,text,text,timestamptz,timestamptz,timestamptz,timestamptz,inet,text,int4,xid,xid,text,bool,text,text,int4,text,numeric,text,bool,text,bool,int4}',
+  proargmodes => '{i,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o}',
+  proargnames => '{pid,datid,pid,usesysid,application_name,state,query,wait_event_type,wait_event,xact_start,query_start,backend_start,state_change,client_addr,client_hostname,client_port,backend_xid,backend_xmin,backend_type,ssl,sslversion,sslcipher,sslbits,ssl_client_dn,ssl_client_serial,ssl_issuer_dn,gss_auth,gss_princ,gss_enc,leader_pid}',
   prosrc => 'pg_stat_get_activity' },
 { oid => '3318',
   descr => 'statistics: information about progress of backends running maintenance command',
diff --git a/src/include/libpq/libpq-be.h b/src/include/libpq/libpq-be.h
index 7be1a67d69..30fb4e613d 100644
--- a/src/include/libpq/libpq-be.h
+++ b/src/include/libpq/libpq-be.h
@@ -266,7 +266,6 @@ extern ssize_t be_tls_write(Port *port, void *ptr, size_t len, int *waitfor);
  * Return information about the SSL connection.
  */
 extern int	be_tls_get_cipher_bits(Port *port);
-extern bool be_tls_get_compression(Port *port);
 extern const char *be_tls_get_version(Port *port);
 extern const char *be_tls_get_cipher(Port *port);
 extern void be_tls_get_peer_subject_name(Port *port, char *ptr, size_t len);
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index 724068cf87..e0c70d221b 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -1135,7 +1135,6 @@ typedef struct PgBackendSSLStatus
 {
 	/* Information about SSL connection */
 	int			ssl_bits;
-	bool		ssl_compression;
 	char		ssl_version[NAMEDATALEN];
 	char		ssl_cipher[NAMEDATALEN];
 	char		ssl_client_dn[NAMEDATALEN];
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index f83af03d0a..459c40bfac 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -293,9 +293,12 @@ static const internalPQconninfoOption PQconninfoOptions[] = {
 		"SSL-Mode", "", 12,		/* sizeof("verify-full") == 12 */
 	offsetof(struct pg_conn, sslmode)},
 
-	{"sslcompression", "PGSSLCOMPRESSION", "0", NULL,
-		"SSL-Compression", "", 1,
-	offsetof(struct pg_conn, sslcompression)},
+	/*
+	 * "sslcompression" is no longer used, but keep it present for backwards
+	 * compatibility.
+	 */
+	{"sslcompression", NULL, NULL, NULL,
+		"SSL-Compression", "D", 1, -1},
 
 	{"sslcert", "PGSSLCERT", NULL, NULL,
 		"SSL-Client-Cert", "", 64,
@@ -4080,8 +4083,6 @@ freePGconn(PGconn *conn)
 		free(conn->sslcrl);
 	if (conn->sslcrldir)
 		free(conn->sslcrldir);
-	if (conn->sslcompression)
-		free(conn->sslcompression);
 	if (conn->requirepeer)
 		free(conn->requirepeer);
 	if (conn->ssl_min_protocol_version)
diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
index 0fa10a23b4..bc58cdcd47 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -1258,12 +1258,9 @@ initialize_SSL(PGconn *conn)
 		SSL_set_verify(conn->ssl, SSL_VERIFY_PEER, verify_cb);
 
 	/*
-	 * Set compression option if necessary.
+	 * Disable SSL compression
 	 */
-	if (conn->sslcompression && conn->sslcompression[0] == '0')
-		SSL_set_options(conn->ssl, SSL_OP_NO_COMPRESSION);
-	else
-		SSL_clear_options(conn->ssl, SSL_OP_NO_COMPRESSION);
+	SSL_set_options(conn->ssl, SSL_OP_NO_COMPRESSION);
 
 	return 0;
 }
@@ -1553,8 +1550,12 @@ PQsslAttribute(PGconn *conn, const char *attribute_name)
 	if (strcmp(attribute_name, "cipher") == 0)
 		return SSL_get_cipher(conn->ssl);
 
+	/*
+	 * SSL compression is disabled, so even if connecting to an older server
+	 * which still supports it, it wont be active.
+	 */
 	if (strcmp(attribute_name, "compression") == 0)
-		return SSL_get_current_compression(conn->ssl) ? "on" : "off";
+		return "off";
 
 	if (strcmp(attribute_name, "protocol") == 0)
 		return SSL_get_version(conn->ssl);
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index 8d51e6ed9f..cca98c14bf 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -360,7 +360,6 @@ struct pg_conn
 	char	   *keepalives_count;	/* maximum number of TCP keepalive
 									 * retransmits */
 	char	   *sslmode;		/* SSL mode (require,prefer,allow,disable) */
-	char	   *sslcompression; /* SSL compression (0 or 1) */
 	char	   *sslkey;			/* client key filename */
 	char	   *sslcert;		/* client certificate filename */
 	char	   *sslpassword;	/* client key file password */
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index b1c9b7bdfe..dd5cc9c221 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1764,7 +1764,7 @@ pg_stat_activity| SELECT s.datid,
     s.backend_xmin,
     s.query,
     s.backend_type
-   FROM ((pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, sslcompression, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid)
+   FROM ((pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid)
      LEFT JOIN pg_database d ON ((s.datid = d.oid)))
      LEFT JOIN pg_authid u ON ((s.usesysid = u.oid)));
 pg_stat_all_indexes| SELECT c.oid AS relid,
@@ -1876,7 +1876,7 @@ pg_stat_gssapi| SELECT s.pid,
     s.gss_auth AS gss_authenticated,
     s.gss_princ AS principal,
     s.gss_enc AS encrypted
-   FROM pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, sslcompression, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid)
+   FROM pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid)
   WHERE (s.client_port IS NOT NULL);
 pg_stat_progress_analyze| SELECT s.pid,
     s.datid,
@@ -2033,7 +2033,7 @@ pg_stat_replication| SELECT s.pid,
     w.sync_priority,
     w.sync_state,
     w.reply_time
-   FROM ((pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, sslcompression, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid)
+   FROM ((pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid)
      JOIN pg_stat_get_wal_senders() w(pid, state, sent_lsn, write_lsn, flush_lsn, replay_lsn, write_lag, flush_lag, replay_lag, sync_priority, sync_state, reply_time) ON ((s.pid = w.pid)))
      LEFT JOIN pg_authid u ON ((s.usesysid = u.oid)));
 pg_stat_replication_slots| SELECT s.slot_name,
@@ -2060,11 +2060,10 @@ pg_stat_ssl| SELECT s.pid,
     s.sslversion AS version,
     s.sslcipher AS cipher,
     s.sslbits AS bits,
-    s.sslcompression AS compression,
     s.ssl_client_dn AS client_dn,
     s.ssl_client_serial AS client_serial,
     s.ssl_issuer_dn AS issuer_dn
-   FROM pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, sslcompression, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid)
+   FROM pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid)
   WHERE (s.client_port IS NOT NULL);
 pg_stat_subscription| SELECT su.oid AS subid,
     su.subname,
diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl
index 864f6e209f..8b719a624c 100644
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -17,7 +17,7 @@ if ($ENV{with_ssl} ne 'openssl')
 }
 else
 {
-	plan tests => 100;
+	plan tests => 101;
 }
 
 #### Some configuration
@@ -157,6 +157,13 @@ test_connect_fails(
 	qr/root certificate file "invalid" does not exist/,
 	"connect without server root cert sslmode=verify-full");
 
+# Test deprecated SSL parameters which should be accepted for backwards
+# compatibility
+test_connect_ok(
+	$common_connstr,
+	"sslrootcert=invalid sslmode=require sslcompression=1 requiressl=1",
+	"connect with deprecated connection parameters");
+
 # Try with wrong root cert, should fail. (We're using the client CA as the
 # root, but the server's key is signed by the server CA.)
 test_connect_fails($common_connstr,
@@ -376,8 +383,8 @@ command_like(
 		"$common_connstr sslrootcert=invalid", '-c',
 		"SELECT * FROM pg_stat_ssl WHERE pid = pg_backend_pid()"
 	],
-	qr{^pid,ssl,version,cipher,bits,compression,client_dn,client_serial,issuer_dn\r?\n
-				^\d+,t,TLSv[\d.]+,[\w-]+,\d+,f,_null_,_null_,_null_\r?$}mx,
+	qr{^pid,ssl,version,cipher,bits,client_dn,client_serial,issuer_dn\r?\n
+				^\d+,t,TLSv[\d.]+,[\w-]+,\d+,_null_,_null_,_null_\r?$}mx,
 	'pg_stat_ssl view without client certificate');
 
 # Test min/max SSL protocol versions.
@@ -493,8 +500,8 @@ command_like(
 		'-c',
 		"SELECT * FROM pg_stat_ssl WHERE pid = pg_backend_pid()"
 	],
-	qr{^pid,ssl,version,cipher,bits,compression,client_dn,client_serial,issuer_dn\r?\n
-				^\d+,t,TLSv[\d.]+,[\w-]+,\d+,f,/CN=ssltestuser,1,\Q/CN=Test CA for PostgreSQL SSL regression test client certs\E\r?$}mx,
+	qr{^pid,ssl,version,cipher,bits,client_dn,client_serial,issuer_dn\r?\n
+				^\d+,t,TLSv[\d.]+,[\w-]+,\d+,/CN=ssltestuser,1,\Q/CN=Test CA for PostgreSQL SSL regression test client certs\E\r?$}mx,
 	'pg_stat_ssl with client certificate');
 
 # client key with wrong permissions
-- 
2.21.1 (Apple Git-122.3)

#15Michael Paquier
michael@paquier.xyz
In reply to: Daniel Gustafsson (#14)
Re: Disallow SSL compression?

On Fri, Mar 05, 2021 at 01:21:01PM +0100, Daniel Gustafsson wrote:

On 5 Mar 2021, at 08:04, Michael Paquier <michael@paquier.xyz> wrote:

FWIW, I would vote to nuke it from all those places, reducing a bit
pg_stat_get_activity() while on it. Keeping it around in the system
catalogs may cause confusion IMHO, by making people think that it is
still possible to get into configurations where sslcompression could
be really enabled. The rest of the patch looks fine to me.

Attached is a version which removes that as well.

Peter, Magnus, any comments about this point?

I left the compression
keyword in PQsslAttribute on purpose, not really for backwards compatibility
(PQsslAttributeNames takes care of that) but rather since it's a more generic
connection-info function.

Makes sense.
--
Michael

#16Magnus Hagander
magnus@hagander.net
In reply to: Michael Paquier (#15)
Re: Disallow SSL compression?

On Fri, Mar 5, 2021 at 1:37 PM Michael Paquier <michael@paquier.xyz> wrote:

On Fri, Mar 05, 2021 at 01:21:01PM +0100, Daniel Gustafsson wrote:

On 5 Mar 2021, at 08:04, Michael Paquier <michael@paquier.xyz> wrote:

FWIW, I would vote to nuke it from all those places, reducing a bit
pg_stat_get_activity() while on it. Keeping it around in the system
catalogs may cause confusion IMHO, by making people think that it is
still possible to get into configurations where sslcompression could
be really enabled. The rest of the patch looks fine to me.

Attached is a version which removes that as well.

Peter, Magnus, any comments about this point?

We've broken stats views before. While it'd be nice if we could group
multiple breakages at the same time, I don't think it's that
important. Better to get rid of it once and for all from as many
places as possible.

--
Magnus Hagander
Me: https://www.hagander.net/
Work: https://www.redpill-linpro.com/

#17Michael Paquier
michael@paquier.xyz
In reply to: Magnus Hagander (#16)
Re: Disallow SSL compression?

On Fri, Mar 05, 2021 at 05:44:20PM +0100, Magnus Hagander wrote:

We've broken stats views before. While it'd be nice if we could group
multiple breakages at the same time, I don't think it's that
important. Better to get rid of it once and for all from as many
places as possible.

Okay, cool. I'd rather wait more for Peter before doing anything, so
if there are no objections, I'll look at that stuff again at the
beginning of next week and perhaps apply it. If you wish to take care
of that yourself, please feel free to do so, of course.
--
Michael

#18Michael Paquier
michael@paquier.xyz
In reply to: Michael Paquier (#17)
Re: Disallow SSL compression?

On Sat, Mar 06, 2021 at 10:39:52AM +0900, Michael Paquier wrote:

Okay, cool. I'd rather wait more for Peter before doing anything, so
if there are no objections, I'll look at that stuff again at the
beginning of next week and perhaps apply it. If you wish to take care
of that yourself, please feel free to do so, of course.

So, I have looked at the proposed patch in details, fixed the
documentation of pg_stat_ssl where compression was still listed,
checked a couple of things with and without OpenSSL, across past major
PG versions with OpenSSL 1.0.2 to see if compression was getting
disabled correctly. And things look all good, so applied.
--
Michael