Add warning about vulnerabilities of using compression

Author: knizhnik

doc/src/sgml/protocol.sgml

@@ -275,12 +275,13 @@
       <term>CompressionAck</term>
       <listitem>
        <para>
-         Server acknowledge using compression for client-server communication protocol.
+         Server acknowledges using compression for client-server communication protocol.
          Compression can be requested by client by including "compression" option in connection string.
-         Client sends to the server list of compression algorithms, supported by client library.
-         If server supports one of this algorithms, then it acknowledges use of this algorithm and then all libpq messages send both from client to server and
+         Client sends to the server list of compression algorithms, supported by client library
+         (compression algorithm is identified by one letter: <literal>'f'</literal> - Facebook zstd, <literal>'z'</literal> - zlib,...).
+         If server supports one of this algorithms, then it acknowledges use of this algorithm and all subsequent libpq messages send both from client to server and
          visa versa will be compressed. If server is not supporting any of the suggested algorithms, then it replies with 'n' (no compression)
-         message and it is up to the client whether to continue work without compression or report error.
+         algorithm identifier and it is up to the client whether to continue work without compression or report error.
        </para>
       </listitem>
      </varlistentry>
@@ -3460,7 +3461,7 @@ CompressionAck (B)
 </term>
 <listitem>
 <para>
-        Used compression algorithm. Right now the following streaming compression algorithms are supported: 'f' - Facebook zstd, 'z' - zlib, 'n' - no compresion.
+        Used compression algorithm. Right now the following streaming compression algorithms are supported: 'f' - Facebook zstd, 'z' - zlib, 'n' - no compression.
 </para>
 </listitem>
 </varlistentry>
@@ -5895,8 +5896,9 @@ StartupMessage (F)
 <listitem>
 <para>
                         Request compression of libpq traffic. Value is list of compression algorithms supported by client:
-                        <literal>'f'</literal> - Facebook zstd, <literal>'z'</literal> - zlib, <literal>'n'</literal> - no compresion.
-                        By default compression is disabled.
+                        <literal>'f'</literal> - Facebook zstd, <literal>'z'</literal> - zlib, <literal>'n'</literal> - no compression.
+                        By default compression is disabled. Please notice that using compression together with SSL may add extra vulnerabilities:
+                        <ulink url="https://en.wikipedia.org/wiki/CRIME">CRIME</ulink>.
 </para>
 </listitem>
 </varlistentry>

src/backend/libpq/pqcomm.c

@@ -199,16 +199,22 @@ WaitEventSet *FeBeWaitSet;
 int
 pq_configure(Port* port)
 {
-	char server_compression_algorithms[ZPQ_MAX_ALGORITHMS];
 	char* client_compression_algorithms = port->compression_algorithms;
-	char compression_algorithm = ZPQ_NO_COMPRESSION;
-	char compression[6] = {'z',0,0,0,5,0}; /* message length = 5 */
-	int rc;
-
-	zpq_get_supported_algorithms(server_compression_algorithms);
-
+	/*
+	 * If client request compression, it sends list of supported compression algorithms.
+	 * Each compression algorirthm is idetified by one letter ('f' - Facebook zsts, 'z' - xlib)
+	 */
 	if (client_compression_algorithms)
 	{
+		char server_compression_algorithms[ZPQ_MAX_ALGORITHMS];
+		char compression_algorithm = ZPQ_NO_COMPRESSION;
+		char compression[6] = {'z',0,0,0,5,0}; /* message length = 5 */
+		int rc;
+
+		/* Get list of compression algorithms, supported by server */
+		zpq_get_supported_algorithms(server_compression_algorithms);
+
+		/* Intersect lists */
 		while (*client_compression_algorithms != '\0')
 		{
 			if (strchr(server_compression_algorithms, *client_compression_algorithms))
@@ -218,19 +224,19 @@ pq_configure(Port* port)
 			}
 			client_compression_algorithms += 1;
 		}
-	}
 
-	compression[5] = compression_algorithm;
-	/* Switch on compression at client side */
-	socket_set_nonblocking(false);
-	while ((rc = secure_write(MyProcPort, compression, sizeof(compression))) < 0
-		   && errno == EINTR);
-	if ((size_t)rc != sizeof(compression))
-		return -1;
-
-	/* initialize compression */
-	if (zpq_set_algorithm(compression_algorithm))
-		PqStream = zpq_create((zpq_tx_func)secure_write, (zpq_rx_func)secure_read, MyProcPort);
+		compression[5] = compression_algorithm;
+		/* Send 'z' message to the client with selectde comression algorithm ('n' if match is ont found) */
+		socket_set_nonblocking(false);
+		while ((rc = secure_write(MyProcPort, compression, sizeof(compression))) < 0
+			   && errno == EINTR);
+		if ((size_t)rc != sizeof(compression))
+			return -1;
+
+		/* initialize compression */
+		if (zpq_set_algorithm(compression_algorithm))
+			PqStream = zpq_create((zpq_tx_func)secure_write, (zpq_rx_func)secure_read, MyProcPort);
+	}
 	return 0;
 }
 
@@ -983,6 +989,7 @@ socket_set_nonblocking(bool nonblocking)
 /* --------------------------------
  *		pq_recvbuf - load some bytes into the input buffer
  *
+ *      nowait parameter toggles non-blocking mode. 
  *		returns number of read bytes, EOF if trouble
  * --------------------------------
  */
@@ -1012,6 +1019,7 @@ pq_recvbuf(bool nowait)
 	for (;;)
 	{
 		size_t processed = 0;
+		/* If srteaming compression is enabled then use correpondent comression read function. */
 		r = PqStream
 			? zpq_read(PqStream, PqRecvBuffer + PqRecvLength,
 					   PQ_RECV_BUFFER_SIZE - PqRecvLength, &processed)

src/common/zpq_stream.c

@@ -3,16 +3,48 @@
 #include "c.h"
 #include "pg_config.h"
 
-static int zpq_algorithm_impl;
-
+/*
+ * Functions implementing streaming compression algorithm
+ */
 typedef struct
 {
+	/*
+	 * Returns letter identifying compression algorithm.
+	 */
 	char    (*name)(void);
+
+	/*
+	 * Create compression stream with using rx/tx function for fetching/sending compressed data
+	 */
 	ZpqStream* (*create)(zpq_tx_func tx_func, zpq_rx_func rx_func, void *arg);
+
+	/*
+	 * Read up to "size" raw (decompressed) bytes.
+	 * Returns number of decompressed bytes or error code. In the last case amount of decompressed bytes is stored in *processed.
+	 * Error code is either ZPQ_DECOMPRESS_ERROR either error code returned by the rx function.
+	 */
 	ssize_t (*read)(ZpqStream *zs, void *buf, size_t size, size_t *processed);
+
+	/*
+	 * Write up to "size" raw (decompressed) bytes.
+	 * Returns number of written raw bytes or error code returned by tx function.
+	 * In the last case amount of written raw bytes is stored in *processed.
+	 */
 	ssize_t (*write)(ZpqStream *zs, void const *buf, size_t size, size_t *processed);
+
+	/*
+	 * Free stream created by create function.
+	 */
 	void    (*free)(ZpqStream *zs);
+
+	/*
+	 * Get error message.
+	 */
 	char const* (*error)(ZpqStream *zs);
+
+	/*
+	 * Returns amount of data in internal compression buffer.
+	 */
 	size_t  (*buffered)(ZpqStream *zs);
 } ZpqAlgorithm;
 
@@ -31,8 +63,8 @@ typedef struct ZstdStream
 	ZSTD_DStream*  rx_stream;
 	ZSTD_outBuffer tx;
 	ZSTD_inBuffer  rx;
-	size_t         tx_not_flushed; /* Amount of datas in internal zstd buffer */
-	size_t         tx_buffered;    /* Data which is consumed by zpq_read but not yet sent */
+	size_t         tx_not_flushed; /* Amount of data in internal zstd buffer */
+	size_t         tx_buffered;    /* Data which is consumed by ztd_read but not yet sent */
 	zpq_tx_func    tx_func;
 	zpq_rx_func    rx_func;
 	void*          arg;
@@ -367,7 +399,10 @@ zlib_name(void)
 
 #endif
 
-static ZpqAlgorithm const zpq_algorithms[] = 
+/*
+ * Array with all supported compression algorithms.
+ */
+static ZpqAlgorithm const zpq_algorithms[] =
 {
 #if HAVE_LIBZSTD
 	{zstd_name, zstd_create, zstd_read, zstd_write, zstd_free, zstd_error, zstd_buffered},
@@ -378,6 +413,12 @@ static ZpqAlgorithm const zpq_algorithms[] =
 	{NULL}
 };
 
+/*
+ * Index of used compression algorithm in zpq_algorithms array.
+ */
+static int zpq_algorithm_impl;
+
+
 ZpqStream*
 zpq_create(zpq_tx_func tx_func, zpq_rx_func rx_func, void *arg)
 {
@@ -415,6 +456,11 @@ zpq_buffered(ZpqStream *zs)
 	return zpq_algorithms[zpq_algorithm_impl].buffered(zs);
 }
 
+/*
+ * Get list of the supported algorithms.
+ * Each algorithm is identified by one letter: 'f' - Facebook zstd, 'z' - zlib.
+ * Algorithm identifies are appended to the provided buffer and terminated by '\0'.
+ */
 void
 zpq_get_supported_algorithms(char algorithms[ZPQ_MAX_ALGORITHMS])
 {
@@ -428,7 +474,11 @@ zpq_get_supported_algorithms(char algorithms[ZPQ_MAX_ALGORITHMS])
 	algorithms[i] = '\0';
 }
 
-
+/*
+ * Choose current algorithm implementation.
+ * Returns true if algorithm identifier is located in the list of the supported algorithms,
+ * false otherwise
+ */
 bool
 zpq_set_algorithm(char name)
 {

src/interfaces/libpq/fe-protocol3.c

@@ -2135,6 +2135,10 @@ pqBuildStartupPacket3(PGconn *conn, int *packetlen,
 	return startpacket;
 }
 
+/*
+ * Parse boolean value. This code is copied from backend/utils/atd/bool.c
+ * because it is not available at frontend.
+ */
 static bool
 parse_bool(const char *value, bool *result)
 {
@@ -2257,6 +2261,10 @@ build_startup_packet(const PGconn *conn, char *packet,
 	if (conn->compression && conn->compression[0])
 	{
 		bool enabled;
+		/*
+		 * If compressoin is enabled, then send to the server list of compression algorithms
+		 * supported by client
+		 */
 		if (parse_bool(conn->compression, &enabled))
 		{
 			char compression_algorithms[ZPQ_MAX_ALGORITHMS];