Re: BUG #16476: pgp_sym_encrypt_bytea with compress-level=6 :Wrong key or corrupt data - Mailing list pgsql-bugs

At Thu, 11 Jun 2020 21:57:25 -0400, Tom Lane <tgl@sss.pgh.pa.us> wrote in
> Kyotaro Horiguchi <horikyota.ntt@gmail.com> writes:
> >> I put an assertion in the patch, but that is not appropriare. It shoud be
> >> an ereport instead. I¢ll fix that later.
> > Fixed.
>
> Hm, should we add a test case exercising this code?

Agrred.  As far as I found, there is another point where a stream ends
with an empty-packet at 16318 bytes, but he stream is not a compressed
data stream.

In the attached, generates a source bytes using random(). To make sure
to stabilize the result, it does setseed(0) just before.  One concern
of the test is there's not means to find if the test gets stale.
Concretely if somehow the data length to cause the issue moved, the
check would silently get no longer effective.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center
From bfbe165af79dbe90e6152f738921311a483ff663 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyoga.ntt@gmail.com>
Date: Thu, 11 Jun 2020 20:29:23 +0900
Subject: [PATCH v3] Make sure to consume stream-terminating packet

A compressed stream may end with an empty packet.  In this case
decompression finishes before reading the empty packet and the
remaining stream packet causes a failure in reading the following
data. Make sure to consume such packets after compression finishes.
---
 contrib/pgcrypto/expected/pgp-compression.out | 21 ++++++++++++++++++
 contrib/pgcrypto/pgp-compress.c               | 22 +++++++++++++++++++
 contrib/pgcrypto/sql/pgp-compression.sql      | 12 ++++++++++
 3 files changed, 55 insertions(+)

diff --git a/contrib/pgcrypto/expected/pgp-compression.out b/contrib/pgcrypto/expected/pgp-compression.out
index 32b350b8fe..84d0956070 100644
--- a/contrib/pgcrypto/expected/pgp-compression.out
+++ b/contrib/pgcrypto/expected/pgp-compression.out
@@ -48,3 +48,24 @@ select pgp_sym_decrypt(
  Secret message
 (1 row)
 
+-- check if trailing empty-packet of compressed stream is correctly read
+select setseed(0);
+ setseed 
+---------
+ 
+(1 row)
+
+select bytes =
+        pgp_sym_decrypt_bytea(
+            pgp_sym_encrypt_bytea(bytes, E'\\x12345678',
+                                    'compress-algo=1,compress-level=1'),
+            E'\\x12345678') from
+        (select -- generate incompressible source data with 16385 bytes 
+         string_agg(decode(lpad(to_hex((random()*256)::int),2,'0'), 'hex'), '')
+         as bytes
+        from generate_series(0, 16365)) t;
+ ?column? 
+----------
+ t
+(1 row)
+
diff --git a/contrib/pgcrypto/pgp-compress.c b/contrib/pgcrypto/pgp-compress.c
index 0505bdee92..296afb3324 100644
--- a/contrib/pgcrypto/pgp-compress.c
+++ b/contrib/pgcrypto/pgp-compress.c
@@ -286,7 +286,29 @@ restart:
 
     dec->buf_data = dec->buf_len - dec->stream.avail_out;
     if (res == Z_STREAM_END)
+    {
+        uint8 *tmp;
+
+        /*
+         * A stream must be terminated by a normal packet. If the last stream
+         * packet in the source stream is a full packet, a normal empty packet
+         * must follow. Since the underlying packet reader doesn't know that
+         * the compressed stream has been ended, we need to to consume the
+         * terminating packet here. This read doesn't harm even if the stream
+         * has already ended.
+         */
+        res = pullf_read(src, 1, &tmp);
+
+        if (res < 0)
+            return res;
+        else if (res > 0)
+        {
+            px_debug("decompress_read: extra bytes after end of stream");
+            return PXE_PGP_CORRUPT_DATA;
+        }
+        
         dec->eof = 1;
+    }
     goto restart;
 }
 
diff --git a/contrib/pgcrypto/sql/pgp-compression.sql b/contrib/pgcrypto/sql/pgp-compression.sql
index ca9ee1fc00..7eb65048dd 100644
--- a/contrib/pgcrypto/sql/pgp-compression.sql
+++ b/contrib/pgcrypto/sql/pgp-compression.sql
@@ -28,3 +28,15 @@ select pgp_sym_decrypt(
     pgp_sym_encrypt('Secret message', 'key',
             'compress-algo=2, compress-level=0'),
     'key', 'expect-compress-algo=0');
+
+-- check if trailing empty-packet of compressed stream is correctly read
+select setseed(0);
+select bytes =
+        pgp_sym_decrypt_bytea(
+            pgp_sym_encrypt_bytea(bytes, E'\\x12345678',
+                                    'compress-algo=1,compress-level=1'),
+            E'\\x12345678') from
+        (select -- generate incompressible source data with 16385 bytes 
+         string_agg(decode(lpad(to_hex((random()*256)::int),2,'0'), 'hex'), '')
+         as bytes
+        from generate_series(0, 16365)) t;
-- 
2.18.2