pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedBUG #16476: pgp_sym_encrypt_bytea with compress-level=6 : Wrong key or corrupt data
10+ messages / 6 participants
[nested] [flat]
Commits from this thread:
2020-07-27 a3ab7a707d 0caf1fc6e8 5bd087eb5d 202fc4ca53 9729f99798 8a60f541f3 aaa132a65d Fix corner case with 16kB-long decompression in pgcrypto, take 2
2020-07-23 7dab4569d1 b9b610577d 3d4a778152 d8ec3b1263 475c69c976 ccf964a801 d0519e9fea Fix ancient violation of zlib's API spec.
2020-07-22 9e108984fb 35e142202b bba2e66aec eceb415c6f 5bfd0936b4 e0cb8282d6 dbf17ca1bf Fix corner case with PGP decompression in pgcrypto
* BUG #16476: pgp_sym_encrypt_bytea with compress-level=6 : Wrong key or corrupt data
@ 2020-06-03 07:41 PG Bug reporting form <noreply@postgresql.org>
0 siblings, 1 reply; 10+ messages in thread
From: PG Bug reporting form @ 2020-06-03 07:41 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org; +Cc: frank.gagnepain@intm.fr
The following bug has been logged on the website:
Bug reference: 16476
Logged by: Frank Gagnepain
Email address: frank.gagnepain@intm.fr
PostgreSQL version: 10.13
Operating system: Debian 10
Description:
Hello to the support team,
I already sent a bug report for this issue, but PostgreSQL version was
9.4.21 which isnt supported anymore
So we tested this bug with a 10.13 PostgreSQL version this time and we got
the exact same issue.
I get "ERROR: Wrong key or corrupt data" when using successively function
pgp_sym_encrypt_bytea and pgp_sym_decrypt_bytea on only some bytea data in
db with those options :
compress-algo=1 (ZIP algo)
cipher-algo=aes256
compress-level=6 (which is the default compress-level)
With any other value for compress-level (0,1,2,3,4,5,7,8,9) for
pgp_sym_encrypt_bytea, i get no error with pgp_sym_decrypt_bytea...
Here is what i do to test this error :
create or replace function bytea_import(p_path text, p_result out bytea)
language plpgsql as $$
declare
l_oid oid;
r record;
begin
p_result := '';
select lo_import(p_path) into l_oid;
for r in ( select data
from pg_largeobject
where loid = l_oid
order by pageno ) loop
p_result = p_result || r.data;
end loop;
perform lo_unlink(l_oid);
end;$$;
select
pgp_sym_decrypt_bytea(pgp_sym_encrypt_bytea(bytea_import(DATA),'password','compress-algo=1,
cipher-algo=aes256, compress-level=6'),'password','compress-algo=1,
cipher-algo=aes256');
ERROR: Wrong key or corrupt data
Unfortunately i cant post any example of DATA since those are supposed to
be
sensitive data.
Nevertheless, does this kind of error rings a bell to anyone ?
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: BUG #16476: pgp_sym_encrypt_bytea with compress-level=6 : Wrong key or corrupt data
@ 2020-06-03 12:35 Jeff Janes <jeff.janes@gmail.com>
parent: PG Bug reporting form <noreply@postgresql.org>
0 siblings, 2 replies; 10+ messages in thread
From: Jeff Janes @ 2020-06-03 12:35 UTC (permalink / raw)
To: frank.gagnepain@intm.fr, pgsql-bugs <pgsql-bugs@lists.postgresql.org>
> select
>
> pgp_sym_decrypt_bytea(pgp_sym_encrypt_bytea(bytea_import(DATA),'password','compress-algo=1,
> cipher-algo=aes256, compress-level=6'),'password','compress-algo=1,
> cipher-algo=aes256');
>
>
decryption reads the settings from the encrypted message header, there is
no need to specify them again.
I can reproduce this at any compression level if the data is random (not
compressible) and exactly 16365 bytes long. If the data is compressible,
then you need a longer length of message to reproduce it and it depends on
the random content and compression level.
I'm attaching the reproducer as a Perl script. I have not investigated the
C code of pgcrypto itself.
Cheers,
Jeff
Attachments:
pgcrypto_zip.pl (application/octet-stream, 824B) download
^ permalink raw reply [nested|flat] 10+ messages in thread
* RE: BUG #16476: pgp_sym_encrypt_bytea with compress-level=6 : Wrong key or corrupt data
@ 2020-06-10 15:17 Frank Gagnepain <frank.gagnepain@intm.fr>
parent: Jeff Janes <jeff.janes@gmail.com>
1 sibling, 0 replies; 10+ messages in thread
From: Frank Gagnepain @ 2020-06-10 15:17 UTC (permalink / raw)
To: Jeff Janes <jeff.janes@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
Hello again,
Thank you for this script ,
We managed to get you an example of data that triggers the error message (with compress-level=6) in attachments.
You would have to unzip first and then test it (I mean it hasnt been zipped by pgcrypto).
Cheers,
Frank GAGNEPAIN
________________________________
De : Jeff Janes <jeff.janes@gmail.com>
Envoyé : mercredi 3 juin 2020 14:35
À : Frank Gagnepain <frank.gagnepain@intm.fr>; pgsql-bugs <pgsql-bugs@lists.postgresql.org>
Objet : Re: BUG #16476: pgp_sym_encrypt_bytea with compress-level=6 : Wrong key or corrupt data
select
pgp_sym_decrypt_bytea(pgp_sym_encrypt_bytea(bytea_import(DATA),'password','compress-algo=1,
cipher-algo=aes256, compress-level=6'),'password','compress-algo=1,
cipher-algo=aes256');
decryption reads the settings from the encrypted message header, there is no need to specify them again.
I can reproduce this at any compression level if the data is random (not compressible) and exactly 16365 bytes long. If the data is compressible, then you need a longer length of message to reproduce it and it depends on the random content and compression level.
I'm attaching the reproducer as a Perl script. I have not investigated the C code of pgcrypto itself.
Cheers,
Jeff
Attachments:
CP04003072_PART2_SANS_ENTETE.zip (application/zip, 676.2K) download
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: BUG #16476: pgp_sym_encrypt_bytea with compress-level=6 : Wrong key or corrupt data
@ 2020-06-11 11:58 Kyotaro Horiguchi <horikyota.ntt@gmail.com>
parent: Jeff Janes <jeff.janes@gmail.com>
1 sibling, 1 reply; 10+ messages in thread
From: Kyotaro Horiguchi @ 2020-06-11 11:58 UTC (permalink / raw)
To: jeff.janes@gmail.com; +Cc: frank.gagnepain@intm.fr; pgsql-bugs@lists.postgresql.org
At Wed, 3 Jun 2020 08:35:02 -0400, Jeff Janes <jeff.janes@gmail.com> wrote in
> I can reproduce this at any compression level if the data is random (not
> compressible) and exactly 16365 bytes long. If the data is compressible,
> then you need a longer length of message to reproduce it and it depends on
> the random content and compression level.
>
> I'm attaching the reproducer as a Perl script. I have not investigated the
> C code of pgcrypto itself.
Thanks for the reproducer.
Compressed stream must end with a normal packet. If a stream ends with
a complete stream packet, deflator adds a zero-length normal packet at
the end. decompress_read forgets to read such a terminating packet
when EOF comes at just at the end of the last stream packet. An extra
call to pullf_read at EOF correctly consumes such an extra packet.
The extra call doesn't harm if a stream ends with partial normal
packet.
The reproducer becomes not to fail with the attached patch.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
Attachments:
0001-Make-sure-to-consume-stream-terminating-packet.patch (text/x-patch, 0B) download
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: BUG #16476: pgp_sym_encrypt_bytea with compress-level=6 : Wrong key or corrupt data
@ 2020-06-11 13:17 Kyotaro Horiguchi <horikyota.ntt@gmail.com>
parent: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
0 siblings, 1 reply; 10+ messages in thread
From: Kyotaro Horiguchi @ 2020-06-11 13:17 UTC (permalink / raw)
To: Jeff Janes <jeff.janes@gmail.com>; +Cc: frank.gagnepain@intm.fr; pgsql-bugs@lists.postgresql.org
>
> The reproducer becomes not to fail with the attached patch.
I put an assertion in the patch, but that is not appropriare. It shoud be
an ereport instead. I’ll fix that later.
regards.
--
Kyotaro Horiguchi
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: BUG #16476: pgp_sym_encrypt_bytea with compress-level=6 : Wrong key or corrupt data
@ 2020-06-12 01:51 Kyotaro Horiguchi <horikyota.ntt@gmail.com>
parent: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
0 siblings, 1 reply; 10+ messages in thread
From: Kyotaro Horiguchi @ 2020-06-12 01:51 UTC (permalink / raw)
To: jeff.janes@gmail.com; +Cc: frank.gagnepain@intm.fr; pgsql-bugs@lists.postgresql.org
At Thu, 11 Jun 2020 22:17:26 +0900, Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote in
> >
> > The reproducer becomes not to fail with the attached patch.
>
>
> I put an assertion in the patch, but that is not appropriare. It shoud be
> an ereport instead. I¢ll fix that later.
Fixed.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
Attachments:
v2-0001-Make-sure-to-consume-stream-terminating-packet.patch (text/x-patch, 0B) download
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: BUG #16476: pgp_sym_encrypt_bytea with compress-level=6 : Wrong key or corrupt data
@ 2020-06-12 01:57 Tom Lane <tgl@sss.pgh.pa.us>
parent: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
0 siblings, 1 reply; 10+ messages in thread
From: Tom Lane @ 2020-06-12 01:57 UTC (permalink / raw)
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>; +Cc: jeff.janes@gmail.com; frank.gagnepain@intm.fr; pgsql-bugs@lists.postgresql.org
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?
regards, tom lane
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: BUG #16476: pgp_sym_encrypt_bytea with compress-level=6 : Wrong key or corrupt data
@ 2020-06-12 05:54 Kyotaro Horiguchi <horikyota.ntt@gmail.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 10+ messages in thread
From: Kyotaro Horiguchi @ 2020-06-12 05:54 UTC (permalink / raw)
To: tgl@sss.pgh.pa.us; +Cc: jeff.janes@gmail.com; frank.gagnepain@intm.fr; pgsql-bugs@lists.postgresql.org
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
Attachments:
v3-0001-Make-sure-to-consume-stream-terminating-packet.patch (text/x-patch, 0B) download
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: BUG #16476: pgp_sym_encrypt_bytea with compress-level=6 : Wrong key or corrupt data
@ 2020-06-24 07:59 Michael Paquier <michael@paquier.xyz>
parent: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
0 siblings, 1 reply; 10+ messages in thread
From: Michael Paquier @ 2020-06-24 07:59 UTC (permalink / raw)
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>; +Cc: tgl@sss.pgh.pa.us; jeff.janes@gmail.com; frank.gagnepain@intm.fr; pgsql-bugs@lists.postgresql.org
On Fri, Jun 12, 2020 at 02:54:12PM +0900, Kyotaro Horiguchi wrote:
> 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.
FYI, I have begun looking at this report, and I am reviewing the
proposed patch.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: BUG #16476: pgp_sym_encrypt_bytea with compress-level=6 : Wrong key or corrupt data
@ 2020-06-25 06:45 Michael Paquier <michael@paquier.xyz>
parent: Michael Paquier <michael@paquier.xyz>
0 siblings, 0 replies; 10+ messages in thread
From: Michael Paquier @ 2020-06-25 06:45 UTC (permalink / raw)
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>; +Cc: tgl@sss.pgh.pa.us; jeff.janes@gmail.com; frank.gagnepain@intm.fr; pgsql-bugs@lists.postgresql.org
On Wed, Jun 24, 2020 at 04:59:21PM +0900, Michael Paquier wrote:
> FYI, I have begun looking at this report, and I am reviewing the
> proposed patch.
Okay. First I found the test case you added a bit hard to parse, so I
have refactored it with a CTE in charge of building the random string,
with the main query doing the compression/decompression and a check
making sure that the original string and the result match. I quite
liked the logic with lpad() to append zeros if the computation with
random()*256 returned a result less than 16, as well as the use of
string_agg() for that purpose to build the string. I have also
switched the second arguments of the functions to just use 'key', for
readability. Using a random string sounds good to me here. It could
always be possible that we finish with something less random, causing
it to to become compressed but I'll believe in the rule of chaos
here.
Then, the fix you are proposing is to simply make sure that all the
input from the source stream is properly consumed even after the zlib
stream has ended in this corner case thanks to pullf_read(), and that
sounds good to me. However, I had an idea slightly different than
yours, consisting of simply reading the contents of the source before
checking if there is any available in the decompressed buffer (the
check on buf_data before the goto restart step). That makes the fix a
bit simpler, without changing the logic.
I am also attaching an extra script I used to validate this stuff
based on the regression test of the patch, that has allowed me to
check the logic for random strings up to 33kB, like that for example
(this one took 8 mins on my laptop):
select count(test_crypto(len)) from generate_series(0, 33000) as len;
The inputs and outputs perfectly matched in all my tests, with the
16kB string being the only one failing in the range I have tested on
HEAD, test passing with the patch of course. One or more extra pairs
of eyes is welcome, so please feel free to look at the version
attached.
Thanks,
--
Michael
Attachments:
crypto.sql (application/x-sql, 854B) download
pgcrypto-zlib-v4.patch (text/x-diff, 3.5K) download
diff --git a/contrib/pgcrypto/expected/pgp-compression.out b/contrib/pgcrypto/expected/pgp-compression.out
index 32b350b8fe..d4c57feba3 100644
--- a/contrib/pgcrypto/expected/pgp-compression.out
+++ b/contrib/pgcrypto/expected/pgp-compression.out
@@ -48,3 +48,33 @@ select pgp_sym_decrypt(
Secret message
(1 row)
+-- check corner case involving an input string of 16kB, as per bug #16476.
+SELECT setseed(0);
+ setseed
+---------
+
+(1 row)
+
+WITH random_string AS
+(
+ -- This generates a random string of 16366 bytes. This is chosen
+ -- as random so that it does not get compressed, and the decompression
+ -- would work on a string with the same length as the origin, making the
+ -- test behavior more predictible. lpad() ensures that the generated
+ -- hexadecimal value is completed by extra zero characters if random()
+ -- has generated a value strictly lower than 16.
+ SELECT string_agg(decode(lpad(to_hex((random()*256)::int), 2, '0'), 'hex'), '') as bytes
+ FROM generate_series(0, 16365)
+)
+SELECT bytes =
+ pgp_sym_decrypt_bytea(
+ pgp_sym_encrypt_bytea(bytes, 'key',
+ 'compress-algo=1,compress-level=1'),
+ 'key', 'expect-compress-algo=1')
+ AS is_same
+ FROM random_string;
+ is_same
+---------
+ t
+(1 row)
+
diff --git a/contrib/pgcrypto/pgp-compress.c b/contrib/pgcrypto/pgp-compress.c
index 0505bdee92..a37595dcc9 100644
--- a/contrib/pgcrypto/pgp-compress.c
+++ b/contrib/pgcrypto/pgp-compress.c
@@ -243,6 +243,20 @@ decompress_read(void *priv, PullFilter *src, int len,
struct DecomprData *dec = priv;
restart:
+ if (dec->stream.avail_in == 0)
+ {
+ uint8 *tmp;
+
+ res = pullf_read(src, 8192, &tmp);
+ if (res < 0)
+ return res;
+
+ Assert(res != Z_STREAM_END || dec->buf_data > 0);
+
+ dec->stream.next_in = tmp;
+ dec->stream.avail_in = res;
+ }
+
if (dec->buf_data > 0)
{
if (len > dec->buf_data)
@@ -256,17 +270,6 @@ restart:
if (dec->eof)
return 0;
- if (dec->stream.avail_in == 0)
- {
- uint8 *tmp;
-
- res = pullf_read(src, 8192, &tmp);
- if (res < 0)
- return res;
- dec->stream.next_in = tmp;
- dec->stream.avail_in = res;
- }
-
dec->stream.next_out = dec->buf;
dec->stream.avail_out = dec->buf_len;
dec->pos = dec->buf;
diff --git a/contrib/pgcrypto/sql/pgp-compression.sql b/contrib/pgcrypto/sql/pgp-compression.sql
index ca9ee1fc00..87c59c6cab 100644
--- a/contrib/pgcrypto/sql/pgp-compression.sql
+++ b/contrib/pgcrypto/sql/pgp-compression.sql
@@ -28,3 +28,24 @@ select pgp_sym_decrypt(
pgp_sym_encrypt('Secret message', 'key',
'compress-algo=2, compress-level=0'),
'key', 'expect-compress-algo=0');
+
+-- check corner case involving an input string of 16kB, as per bug #16476.
+SELECT setseed(0);
+WITH random_string AS
+(
+ -- This generates a random string of 16366 bytes. This is chosen
+ -- as random so that it does not get compressed, and the decompression
+ -- would work on a string with the same length as the origin, making the
+ -- test behavior more predictible. lpad() ensures that the generated
+ -- hexadecimal value is completed by extra zero characters if random()
+ -- has generated a value strictly lower than 16.
+ SELECT string_agg(decode(lpad(to_hex((random()*256)::int), 2, '0'), 'hex'), '') as bytes
+ FROM generate_series(0, 16365)
+)
+SELECT bytes =
+ pgp_sym_decrypt_bytea(
+ pgp_sym_encrypt_bytea(bytes, 'key',
+ 'compress-algo=1,compress-level=1'),
+ 'key', 'expect-compress-algo=1')
+ AS is_same
+ FROM random_string;
end of thread, other threads:[~2020-06-25 06:45 UTC | newest]
Thread overview: 10+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-06-03 07:41 BUG #16476: pgp_sym_encrypt_bytea with compress-level=6 : Wrong key or corrupt data PG Bug reporting form <noreply@postgresql.org>
2020-06-03 12:35 ` Jeff Janes <jeff.janes@gmail.com>
2020-06-10 15:17 ` Frank Gagnepain <frank.gagnepain@intm.fr>
2020-06-11 11:58 ` Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2020-06-11 13:17 ` Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2020-06-12 01:51 ` Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2020-06-12 01:57 ` Tom Lane <tgl@sss.pgh.pa.us>
2020-06-12 05:54 ` Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2020-06-24 07:59 ` Michael Paquier <michael@paquier.xyz>
2020-06-25 06:45 ` Michael Paquier <michael@paquier.xyz>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox