Thread: BUG #11905: "Error: Wrong key or corrupt data" with pgp_sym_decrypt when bytea size is 2^x - 6 where x >=14

The following bug has been logged on the website:

Bug reference:      11905
Logged by:          Connor Penhale
Email address:      connor.penhale@openlogic.com
PostgreSQL version: 9.3.1
Operating system:   OS X Yosemitie
Description:

Hello Postgres Community,

When I attempt to decrypt a file I successfully encrypted with a size of
65530, I get the “Error: Wrong key or corrupt data” message.

Here are the steps to reproduce in plain text:


$ head -c 65536 </dev/urandom > can_decode.txt
$ head -c 65530 </dev/urandom > can_not_decode.txt
$ psql

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;$$;


create table enc (file_name text PRIMARY KEY, blob bytea, file_size bigint)


insert into enc values ( 'can_decode.txt', (select
bytea_import(‘~/can_decode.txt')), 65536);


insert into enc values ( 'can_not_decode.txt', (select
bytea_import(‘~/can_not_decode.txt')), 65530);


select file_name,md5(blob),file_size from enc;
file_name | md5 | file_size
------------------------------+----------------------------------+-----------
can_decode.txt | a4a66bd8f16aab703857085e474a5873 | 65536
can_not_decode.txt | c04549e976852a555070c3a6ba9d64b4 | 65530


insert into enc values ( 'can_decode.txt enc', NULL , 65536);


insert into enc values ( 'can_not_decode.txt enc', NULL , 65530);


update enc set blob = (select pgp_sym_encrypt_bytea(blob, 'I AM THE
PASSWORD') from enc where file_name = 'can_decode.txt') where file_name =
'can_decode.txt enc';


update enc set blob = (select pgp_sym_encrypt_bytea(blob, 'I AM THE
PASSWORD') from enc where file_name = 'can_not_decode.txt') where file_name
= 'can_not_decode.txt enc';


insert into enc values ( 'can_decode.txt de-enc', NULL , 65536);


insert into enc values ( 'can_not_decode.txt de-enc', NULL , 65530);



update enc set blob = (select pgp_sym_decrypt_bytea(blob, 'I AM THE
PASSWORD') from enc where file_name = 'can_decode.txt enc') where file_name
= 'can_decode.txt de-enc';


update enc set blob = (select pgp_sym_decrypt_bytea(blob, 'I AM THE
PASSWORD') from enc where file_name = 'can_not_decode.txt enc') where
file_name = 'can_not_decode.txt de-enc';

At this point, I get the error.

I’ve been able to reproduce this on Postgres 9.1 and 9.3.

Please let me know if I can provide more information.

Thanks!
Connor

--
Connor Penhale | Open Source Solutions Architect
OpenLogic, a Rogue Wave Company
Accelerating Great Code
5500 Flatiron Parkway, Suite 200, Boulder, CO 80301
P 1 866-399-6736 | Toll Free
On Fri, Nov 7, 2014 at 1:56 PM, <connor.penhale@openlogic.com> wrote:

> The following bug has been logged on the website:
>
> Bug reference:      11905
> Logged by:          Connor Penhale
> Email address:      connor.penhale@openlogic.com
> PostgreSQL version: 9.3.1
> Operating system:   OS X Yosemitie
> Description:
>
> Hello Postgres Community,
>
> When I attempt to decrypt a file I successfully encrypted with a size of
> 65530, I get the =E2=80=9CError: Wrong key or corrupt data=E2=80=9D messa=
ge.
>
...


> I=E2=80=99ve been able to reproduce this on Postgres 9.1 and 9.3.
>

I can reproduce this error, and first get it at a size of 16378 (i.e.
2^14-6).

I can reproduce it also against the development head code, including after
applying the recently proposed patch
http://www.postgresql.org/message-id/5454F3C2.608@joh.to

I can reproduce it easily with the single query:

select pgp_sym_decrypt(pgp_sym_encrypt(?,'I am the password'),'I am the
password','debug=3D1')


The problem is that mdc_read is getting invoked with an len argument of 0.
When it successfully reads 0 bytes, it interprets that as an error.  I
can't figure out why it is getting invoked with a length of 0, too many
pointer indirections for me to follow at this time.

Cheers,

Jeff
On 2014-11-11 18:52, Jeff Janes wrote:
> The problem is that mdc_read is getting invoked with an len argument of 0.
> When it successfully reads 0 bytes, it interprets that as an error.  I
> can't figure out why it is getting invoked with a length of 0, too many
> pointer indirections for me to follow at this time.

This sounds a lot like another bug I've seen.  Can you try this patch?

*** a/contrib/pgcrypto/pgp-decrypt.c
--- b/contrib/pgcrypto/pgp-decrypt.c
***************
*** 182,188 **** pktreader_pull(void *priv, PullFilter *src, int len,
       if (pkt->type == PKT_CONTEXT)
           return pullf_read(src, len, data_p);

!     if (pkt->len == 0)
       {
           /* this was last chunk in stream */
           if (pkt->type == PKT_NORMAL)
--- 182,188 ----
       if (pkt->type == PKT_CONTEXT)
           return pullf_read(src, len, data_p);

!     while (pkt->len == 0)
       {
           /* this was last chunk in stream */
           if (pkt->type == PKT_NORMAL)



.marko
On Tue, Nov 11, 2014 at 10:11 AM, Marko Tiikkaja <marko@joh.to> wrote:
On 2014-11-11 18:52, Jeff Janes wrote:
The problem is that mdc_read is getting invoked with an len argument of 0.
When it successfully reads 0 bytes, it interprets that as an error.  I
can't figure out why it is getting invoked with a length of 0, too many
pointer indirections for me to follow at this time.

This sounds a lot like another bug I've seen.  Can you try this patch?

That fixes it.  I've tested all lengths from 1 to 133,000

I've added a regression test to it, in the attached.
 
How do we go about incorporating it, as far as bumping the extension version-number and such?  Or does it not get bumped at all because there is no change to the interface?

Cheers,

Jeff
Attachment
Jeff Janes <jeff.janes@gmail.com> writes:
> How do we go about incorporating it, as far as bumping the extension
> version-number and such?  Or does it not get bumped at all because there is
> no change to the interface?

It's only a C code change, so no problem.  If we had to change the SQL
code for pgcrypto, we'd have to worry about version numbers.

            regards, tom lane
Jeff Janes <jeff.janes@gmail.com> writes:
> On Tue, Nov 11, 2014 at 10:11 AM, Marko Tiikkaja <marko@joh.to> wrote:
>> This sounds a lot like another bug I've seen.  Can you try this patch?

> That fixes it.  I've tested all lengths from 1 to 133,000
> I've added a regression test to it, in the attached.

Pushed.

            regards, tom lane