Re: pgcrypto (v02) - Mailing list pgsql-patches

There is updated version of patch. See comments bellow:

Marko Kreen wrote:
> On 7/27/07, Zdenek Kotala <Zdenek.Kotala@sun.com> wrote:
>> I attach pgcrypto patch which fix two problems on system without strong
>> crypto support (e.g. default Solaris 10 installation):
>>
>> 1) postgres crashes when AES cipher uses long key
>> 2) Blowfish silently cut longer keys. It could bring problem when
>> crypted data are transfered from one server to another with strong keys
>> support.
>
> Couple of style nitpicks:
> * please use hex arrays, instead octal-quoted strings.  easier on the eye.

fixed

> * use memcmp() instead of for() loop.

fixed

> * 16 byte bufs for 8 bytes is confusing.

I think it must be 16 because block size is 16 bytes. I'm not sure if 8
bytes could not cause buffer overflow.


>> This patch was discussed there:
>> http://archives.postgresql.org/pgsql-hackers/2007-07/msg00762.php
>>
>> This patch is applicable also on 8.2, 8.1 (and maybe older) version of
>> postgresql.
>
> OpenSSL autoconfiguration was added in 8.1, so patching older
> versions is not that critical.

        Zdenek


Index: contrib/pgcrypto/openssl.c
===================================================================
RCS file: /zfs_data/cvs_pgsql/cvsroot/pgsql/contrib/pgcrypto/openssl.c,v
retrieving revision 1.30
diff -c -r1.30 openssl.c
*** contrib/pgcrypto/openssl.c    2006/10/04 00:29:46    1.30
--- contrib/pgcrypto/openssl.c    2007/08/07 10:35:24
***************
*** 375,385 ****
--- 375,424 ----

  /* Blowfish */

+ /* Check if strong crypto is supported. Some openssl installation could support only short keys
+   and unfortunatelly BF_sef_key does not return any error value. This function tests if is possible
+   use strong key.
+ */
+ static
+ int bf_check_supported_key_len()
+ {
+     static unsigned char key[] = {0xf0,0xe1,0xd2,0xc3,0xb4,0xa5,0x96,0x87,0x78,0x69,0x5a,0x4b,
+                                     0x3c,0x2d,0x1e,0x0f,0x00,0x11,0x22,0x33,0x44,0x55,0x66,0x77,
+                                     0x04,0x68,0x91,0x04,0xc2,0xfd,0x3b,0x2f,0x58,0x40,0x23,0x64,
+                                     0x1a,0xba,0x61,0x76,0x1f,0x1f,0x1f,0x1f,0x0e,0x0e,0x0e,0x0e,
+                                     0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff};
+
+     static unsigned char data[16] = {0xfe,0xdc,0xba,0x98,0x76,0x54,0x32,0x10};
+     static unsigned char res[] = {0xc0,0x45,0x04,0x01,0x2e,0x4e,0x1f,0x53};
+     static unsigned char out[16];
+     BF_KEY bf_key;
+     int n;
+
+     /* encrypt and th 448bits key and verify output */
+     BF_set_key(&bf_key,56, key);
+     BF_ecb_encrypt(    data, out, &bf_key, BF_ENCRYPT);
+
+     if( memcmp(out, res, 8) != 0)
+         return 0;    /* Output does not match -> strong cipher is not supported */
+     return 1;
+ }
+
  static int
  bf_init(PX_Cipher * c, const uint8 *key, unsigned klen, const uint8 *iv)
  {
      ossldata   *od = c->ptr;
+     static int bf_is_strong = -1;
+
+     /* Test if key len is supported. BF_set_key silently cut large keys and it could be
+      be a problem when user transfer crypted data from one server to another. */
+
+     if( bf_is_strong == -1)
+         bf_is_strong = bf_check_supported_key_len();
+
+     if( !bf_is_strong && klen>16 )
+         return PXE_KEY_TOO_BIG;

+     /* Key len is supported. We can use it. */
      BF_set_key(&od->u.bf.key, klen, key);
      if (iv)
          memcpy(od->iv, iv, BF_BLOCK);
***************
*** 692,705 ****
      return 0;
  }

! static void
  ossl_aes_key_init(ossldata * od, int type)
  {
      if (type == AES_ENCRYPT)
!         AES_set_encrypt_key(od->key, od->klen * 8, &od->u.aes_key);
      else
!         AES_set_decrypt_key(od->key, od->klen * 8, &od->u.aes_key);
!     od->init = 1;
  }

  static int
--- 731,755 ----
      return 0;
  }

! static int
  ossl_aes_key_init(ossldata * od, int type)
  {
+     int err;
+     /* Strong key support could miss on some openssl installation, we must
+         check return value, from set key function.
+     */
      if (type == AES_ENCRYPT)
!         err = AES_set_encrypt_key(od->key, od->klen * 8, &od->u.aes_key);
      else
!         err = AES_set_decrypt_key(od->key, od->klen * 8, &od->u.aes_key);
!
!     if (err == 0)
!     {
!         od->init = 1;
!         return 0;
!     }
!     od->init = 0;
!     return PXE_KEY_TOO_BIG;
  }

  static int
***************
*** 709,717 ****
      unsigned    bs = gen_ossl_block_size(c);
      ossldata   *od = c->ptr;
      const uint8 *end = data + dlen - bs;

      if (!od->init)
!         ossl_aes_key_init(od, AES_ENCRYPT);

      for (; data <= end; data += bs, res += bs)
          AES_ecb_encrypt(data, res, &od->u.aes_key, AES_ENCRYPT);
--- 759,769 ----
      unsigned    bs = gen_ossl_block_size(c);
      ossldata   *od = c->ptr;
      const uint8 *end = data + dlen - bs;
+     int err;

      if (!od->init)
!         if( 0 != (err = ossl_aes_key_init(od, AES_ENCRYPT) ))
!             return err;

      for (; data <= end; data += bs, res += bs)
          AES_ecb_encrypt(data, res, &od->u.aes_key, AES_ENCRYPT);
***************
*** 725,733 ****
      unsigned    bs = gen_ossl_block_size(c);
      ossldata   *od = c->ptr;
      const uint8 *end = data + dlen - bs;

      if (!od->init)
!         ossl_aes_key_init(od, AES_DECRYPT);

      for (; data <= end; data += bs, res += bs)
          AES_ecb_encrypt(data, res, &od->u.aes_key, AES_DECRYPT);
--- 777,787 ----
      unsigned    bs = gen_ossl_block_size(c);
      ossldata   *od = c->ptr;
      const uint8 *end = data + dlen - bs;
+     int err;

      if (!od->init)
!         if( 0 != (err = ossl_aes_key_init(od, AES_DECRYPT) ))
!             return err;

      for (; data <= end; data += bs, res += bs)
          AES_ecb_encrypt(data, res, &od->u.aes_key, AES_DECRYPT);
***************
*** 739,748 ****
                       uint8 *res)
  {
      ossldata   *od = c->ptr;

      if (!od->init)
!         ossl_aes_key_init(od, AES_ENCRYPT);
!
      AES_cbc_encrypt(data, res, dlen, &od->u.aes_key, od->iv, AES_ENCRYPT);
      return 0;
  }
--- 793,804 ----
                       uint8 *res)
  {
      ossldata   *od = c->ptr;
+     int err;

      if (!od->init)
!         if( 0 != (err = ossl_aes_key_init(od, AES_ENCRYPT) ))
!             return err;
!
      AES_cbc_encrypt(data, res, dlen, &od->u.aes_key, od->iv, AES_ENCRYPT);
      return 0;
  }
***************
*** 752,760 ****
                       uint8 *res)
  {
      ossldata   *od = c->ptr;

      if (!od->init)
!         ossl_aes_key_init(od, AES_DECRYPT);

      AES_cbc_encrypt(data, res, dlen, &od->u.aes_key, od->iv, AES_DECRYPT);
      return 0;
--- 808,818 ----
                       uint8 *res)
  {
      ossldata   *od = c->ptr;
+     int err;

      if (!od->init)
!         if( 0 != (err = ossl_aes_key_init(od, AES_DECRYPT) ))
!             return err;

      AES_cbc_encrypt(data, res, dlen, &od->u.aes_key, od->iv, AES_DECRYPT);
      return 0;