Re: pgcrypto & strong ciphers limitation

From: Zdenek Kotala <Zdenek(dot)Kotala(at)Sun(dot)COM>
To: Marko Kreen <markokr(at)gmail(dot)com>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Stefan Kaltenbrunner <stefan(at)kaltenbrunner(dot)cc>
Subject: Re: pgcrypto & strong ciphers limitation
Date: 2007-07-24 15:02:23
Message-ID: 46A6147F.8030403@sun.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Marko Kreen wrote:
> On 7/24/07, Zdenek Kotala <Zdenek(dot)Kotala(at)sun(dot)com> wrote:

>> However, on default installation (which is commonly used) it is a
>> problem. Regression test cannot be fixed because it tests strong
>> ciphers, but there two very strange issue:
>>
>> 1) First issue is blowfish cipher. Because pgcrypto uses old interface
>> instead new "evp" it calls bf_set_key function which does not return any
>> output and cut key if it is too long. See
>> http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/common/openssl/crypto/bf/bf_skey.c
>>
>> line 84.
>>
>> If user installs strong crypto he will not be able decrypt data which
>> has been encrypted before.
>>
>> The fix of this issue is ugly, because there is not way how to verify
>> supported key length with old openssl API and only new API return err if
>> length is not supported.
>
> NAK. The fix is broken because it uses EVP interface. EVP is not
> a general-purpose interface because not all valid keys for cipher
> pass thru it. Only key-lengths used in SSL will work...

I'm not openssl expert, but if you look how to EVP call for setkey is
implemented you can see that finally is call BF_set_key. Only there is
one extra layer see
http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/common/openssl/crypto/evp/e_bf.c

> Could you rework the fix that it uses the BF_* interface,
> does a test-encoding with full-length key and compares it to
> expected result. And does it just once, not on each call.

OK. I can do, but it is not general solution. Because it will work only
in our case, because we know 128 is a restricted limit.

> That should be put into separate function probably.

yes

>> 2) AES ciphere crashes when key is longer. It happens because return
>> value from AES_set_encrypt_key is ignored and AES_encrypt is called with
>> uninitialized structure.
>
> ACK, error checking is good. But please return PXE_KEY_TOO_BIG
> directly from ossl_aes_key_init.

OK.

> I must admit the internal API for ciphers is clumsy and could
> need rework to something saner. This shows here.
>
>> I attach patch which fix both issues, but main problem is there that old
>> openssl API is used and supported key lengths are hardcoded. I think we
>> can add to TODO list rewrite pgcrypto to use evp openssl interface.
>
> pgcrypto _was_ written using EVP, but I needed to rewrite it
> when I found out EVP supports only key lengths used in SSL.

Is it still correct? It seems that blowfish accepts all key range, but
How I mention I'm not openssl guru and documentation is very bad :(.

Zdenek

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Stefan Kaltenbrunner 2007-07-24 15:24:36 Re: strange buildfarm failure on lionfish
Previous Message Stefan Kaltenbrunner 2007-07-24 14:51:11 strange buildfarm failure on lionfish