Skip to content

Commit

Permalink
Remove support for SSL compression
Browse files Browse the repository at this point in the history
PostgreSQL disabled compression as of e3bdb2d and the documentation
recommends against using it since.  Additionally, SSL compression has
been disabled in OpenSSL since version 1.1.0, and was disabled in many
distributions long before that.  The most recent TLS version, TLSv1.3,
disallows compression at the protocol level.

This commit removes the feature itself, removing support for the libpq
parameter sslcompression (parameter still listed for compatibility
reasons with existing connection strings, just ignored), and removes
the equivalent field in pg_stat_ssl and de facto PgBackendSSLStatus.

Note that, 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 configurations.

A TAP test is added for deprecated SSL parameters to check after
backwards compatibility.

Bump catalog version.

Author: Daniel Gustafsson
Reviewed-by: Peter Eisentraut, Magnus Hagander, Michael Paquier
Discussion:  https://postgr.es/m/7E384D48-11C5-441B-9EC3-F7DB1F8518F6@yesql.se
  • Loading branch information
michaelpq committed Mar 9, 2021
1 parent d4545dc commit f9264d1
Show file tree
Hide file tree
Showing 19 changed files with 60 additions and 112 deletions.
3 changes: 1 addition & 2 deletions contrib/postgres_fdw/expected/postgres_fdw.out
Expand Up @@ -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',
Expand Down Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion contrib/postgres_fdw/sql/postgres_fdw.sql
Expand Up @@ -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',
Expand Down
33 changes: 2 additions & 31 deletions doc/src/sgml/libpq.sgml
Expand Up @@ -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>
Expand Down Expand Up @@ -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".
SSL compression is no longer supported, always returns "off".
</para>
</listitem>
</varlistentry>
Expand Down Expand Up @@ -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>
Expand Down
10 changes: 0 additions & 10 deletions doc/src/sgml/monitoring.sgml
Expand Up @@ -3065,16 +3065,6 @@ SELECT pid, wait_event_type, wait_event FROM pg_stat_activity WHERE wait_event i
</para></entry>
</row>

<row>
<entry role="catalog_table_entry"><para role="column_definition">
<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
</para></entry>
</row>

<row>
<entry role="catalog_table_entry"><para role="column_definition">
<structfield>client_dn</structfield> <type>text</type>
Expand Down
1 change: 0 additions & 1 deletion src/backend/catalog/system_views.sql
Expand Up @@ -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
Expand Down
12 changes: 3 additions & 9 deletions src/backend/libpq/be-secure-openssl.c
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
{
Expand Down
1 change: 0 additions & 1 deletion src/backend/postmaster/pgstat.c
Expand Up @@ -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);
Expand Down
36 changes: 17 additions & 19 deletions src/backend/utils/adt/pgstatfuncs.c
Expand Up @@ -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);
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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;
}
}

Expand Down Expand Up @@ -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 */
}
}
Expand Down Expand Up @@ -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);
Expand Down
5 changes: 2 additions & 3 deletions src/backend/utils/init/postinit.c
Expand Up @@ -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)
Expand Down
7 changes: 2 additions & 5 deletions src/bin/psql/command.c
Expand Up @@ -3509,21 +3509,18 @@ 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, bits: %s)\n"),
protocol ? protocol : _("unknown"),
cipher ? cipher : _("unknown"),
bits ? bits : _("unknown"),
(compression && strcmp(compression, "off") != 0) ? _("on") : _("off"));
bits ? bits : _("unknown"));
}

/*
Expand Down
2 changes: 1 addition & 1 deletion src/include/catalog/catversion.h
Expand Up @@ -53,6 +53,6 @@
*/

/* yyyymmddN */
#define CATALOG_VERSION_NO 202103081
#define CATALOG_VERSION_NO 202103091

#endif
6 changes: 3 additions & 3 deletions src/include/catalog/pg_proc.dat
Expand Up @@ -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',
Expand Down
1 change: 0 additions & 1 deletion src/include/libpq/libpq-be.h
Expand Up @@ -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);
Expand Down
1 change: 0 additions & 1 deletion src/include/pgstat.h
Expand Up @@ -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];
Expand Down
11 changes: 6 additions & 5 deletions src/interfaces/libpq/fe-connect.c
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down
15 changes: 7 additions & 8 deletions src/interfaces/libpq/fe-secure-openssl.c
Expand Up @@ -1257,13 +1257,8 @@ initialize_SSL(PGconn *conn)
if (have_rootcert)
SSL_set_verify(conn->ssl, SSL_VERIFY_PEER, verify_cb);

/*
* Set compression option if necessary.
*/
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);
/* disable SSL compression */
SSL_set_options(conn->ssl, SSL_OP_NO_COMPRESSION);

return 0;
}
Expand Down Expand Up @@ -1553,8 +1548,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 will not 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);
Expand Down
1 change: 0 additions & 1 deletion src/interfaces/libpq/libpq-int.h
Expand Up @@ -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 */
Expand Down

0 comments on commit f9264d1

Please sign in to comment.