pgcrypto: Remove internal padding implementation

Use the padding provided by OpenSSL instead of doing it ourselves.
The internal implementation was once applicable to the non-OpenSSL
code paths, but those have since been removed.  The padding algorithm
is still the same.

The OpenSSL padding implementation is stricter than the previous
internal one: Bad padding during decryption is now an error, and
encryption without padding now requires the input size to be a
multiple of the block size, otherwise it is also an error.
Previously, these cases silently proceeded, in spite of the
documentation saying otherwise.

Add some test cases about this, too.  (The test cases are in
rijndael.sql, but they apply to all encryption algorithms.)

Reviewed-by: Jacob Champion 
Reviewed-by: Nathan Bossart 
Discussion: https://www.postgresql.org/message-id/flat/ba94c26b-0c58-c97e-7a44-f44e08b4cca2%40enterprisedb.com

diff --git a/contrib/pgcrypto/expected/rijndael.out b/contrib/pgcrypto/expected/rijndael.out
index ad69cdba4934ce4d67755abdbb8fd038c2efc4a3..015ba4430d9626f3045037c7998f674f1a47c064 100644 (file)
--- a/contrib/pgcrypto/expected/rijndael.out
+++ b/contrib/pgcrypto/expected/rijndael.out
@@ -39,6 +39,12 @@ SELECT encrypt(
  \x8ea2b7ca516745bfeafc49904b496089
 (1 row)
 
+-- without padding, input not multiple of block size
+SELECT encrypt(
+'\x00112233445566778899aabbccddeeff00',
+'\x000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f',
+'aes-cbc/pad:none');
+ERROR:  encrypt error: Encryption failed
 -- key padding
 SELECT encrypt(
 '\x0011223344',
@@ -95,6 +101,14 @@ select encode(decrypt(encrypt('foo', '0123456', 'aes'), '0123456', 'aes'), 'esca
  foo
 (1 row)
 
+-- data not multiple of block size
+select encode(decrypt(encrypt('foo', '0123456', 'aes') || '\x00'::bytea, '0123456', 'aes'), 'escape');
+ERROR:  decrypt error: Decryption failed
+-- bad padding
+-- (The input value is the result of encrypt_iv('abcdefghijklmnopqrstuvwxyz', '0123456', 'abcd', 'aes')
+-- with the 16th byte changed (s/db/eb/) to corrupt the padding of the last block.)
+select encode(decrypt_iv('\xa21a9c15231465964e3396d32095e67eb52bab05f556a581621dee1b85385789', '0123456', 'abcd', 'aes'), 'escape');
+ERROR:  decrypt_iv error: Decryption failed
 -- iv
 select encrypt_iv('foo', '0123456', 'abcd', 'aes');
              encrypt_iv             

diff --git a/contrib/pgcrypto/openssl.c b/contrib/pgcrypto/openssl.c
index e236b0d79c7cf93bb6a98c2877f9bf841be63cca..68fd61b716f2b18e9dd292443bd980b8e89b8084 100644 (file)
--- a/contrib/pgcrypto/openssl.c
+++ b/contrib/pgcrypto/openssl.c
@@ -369,17 +369,17 @@ gen_ossl_free(PX_Cipher *c)
 }
 
 static int
-gen_ossl_decrypt(PX_Cipher *c, const uint8 *data, unsigned dlen,
-                uint8 *res)
+gen_ossl_decrypt(PX_Cipher *c, int padding, const uint8 *data, unsigned dlen,
+                uint8 *res, unsigned *rlen)
 {
    OSSLCipher *od = c->ptr;
-   int         outlen;
+   int         outlen, outlen2;
 
    if (!od->init)
    {
        if (!EVP_DecryptInit_ex(od->evp_ctx, od->evp_ciph, NULL, NULL, NULL))
            return PXE_CIPHER_INIT;
-       if (!EVP_CIPHER_CTX_set_padding(od->evp_ctx, 0))
+       if (!EVP_CIPHER_CTX_set_padding(od->evp_ctx, padding))
            return PXE_CIPHER_INIT;
        if (!EVP_CIPHER_CTX_set_key_length(od->evp_ctx, od->klen))
            return PXE_CIPHER_INIT;
@@ -390,22 +390,25 @@ gen_ossl_decrypt(PX_Cipher *c, const uint8 *data, unsigned dlen,
 
    if (!EVP_DecryptUpdate(od->evp_ctx, res, &outlen, data, dlen))
        return PXE_DECRYPT_FAILED;
+   if (!EVP_DecryptFinal_ex(od->evp_ctx, res + outlen, &outlen2))
+       return PXE_DECRYPT_FAILED;
+   *rlen = outlen + outlen2;
 
    return 0;
 }
 
 static int
-gen_ossl_encrypt(PX_Cipher *c, const uint8 *data, unsigned dlen,
-                uint8 *res)
+gen_ossl_encrypt(PX_Cipher *c, int padding, const uint8 *data, unsigned dlen,
+                uint8 *res, unsigned *rlen)
 {
    OSSLCipher *od = c->ptr;
-   int         outlen;
+   int         outlen, outlen2;
 
    if (!od->init)
    {
        if (!EVP_EncryptInit_ex(od->evp_ctx, od->evp_ciph, NULL, NULL, NULL))
            return PXE_CIPHER_INIT;
-       if (!EVP_CIPHER_CTX_set_padding(od->evp_ctx, 0))
+       if (!EVP_CIPHER_CTX_set_padding(od->evp_ctx, padding))
            return PXE_CIPHER_INIT;
        if (!EVP_CIPHER_CTX_set_key_length(od->evp_ctx, od->klen))
            return PXE_CIPHER_INIT;
@@ -416,6 +419,9 @@ gen_ossl_encrypt(PX_Cipher *c, const uint8 *data, unsigned dlen,
 
    if (!EVP_EncryptUpdate(od->evp_ctx, res, &outlen, data, dlen))
        return PXE_ENCRYPT_FAILED;
+   if (!EVP_EncryptFinal_ex(od->evp_ctx, res + outlen, &outlen2))
+       return PXE_ENCRYPT_FAILED;
+   *rlen = outlen + outlen2;
 
    return 0;
 }

diff --git a/contrib/pgcrypto/pgp-cfb.c b/contrib/pgcrypto/pgp-cfb.c
index dafa562daa128e7b60844c7e8137809afc6b1f3e..de41e825b0ce85289b9de1763ca8900c3f1e182f 100644 (file)
--- a/contrib/pgcrypto/pgp-cfb.c
+++ b/contrib/pgcrypto/pgp-cfb.c
@@ -220,7 +220,9 @@ cfb_process(PGP_CFB *ctx, const uint8 *data, int len, uint8 *dst,
 
    while (len > 0)
    {
-       px_cipher_encrypt(ctx->ciph, ctx->fr, ctx->block_size, ctx->fre);
+       unsigned    rlen;
+
+       px_cipher_encrypt(ctx->ciph, 0, ctx->fr, ctx->block_size, ctx->fre, &rlen);
        if (ctx->block_no < 5)
            ctx->block_no++;
 

diff --git a/contrib/pgcrypto/px.c b/contrib/pgcrypto/px.c
index 0010addaf7d0371f6bc7449745dba45fefe9c325..c139798f3b2a32d240087a100aaa2ba37d562195 100644 (file)
--- a/contrib/pgcrypto/px.c
+++ b/contrib/pgcrypto/px.c
@@ -44,7 +44,6 @@ static const struct error_desc px_err_list[] = {
    {PXE_ERR_GENERIC, "Some PX error (not specified)"},
    {PXE_NO_HASH, "No such hash algorithm"},
    {PXE_NO_CIPHER, "No such cipher algorithm"},
-   {PXE_NOTBLOCKSIZE, "Data not a multiple of block size"},
    {PXE_BAD_OPTION, "Unknown option"},
    {PXE_BAD_FORMAT, "Badly formatted type"},
    {PXE_KEY_TOO_BIG, "Key was too big"},
@@ -221,129 +220,14 @@ static int
 combo_encrypt(PX_Combo *cx, const uint8 *data, unsigned dlen,
              uint8 *res, unsigned *rlen)
 {
-   int         err = 0;
-   uint8      *bbuf;
-   unsigned    bs,
-               bpos,
-               i,
-               pad;
-
-   PX_Cipher  *c = cx->cipher;
-
-   bbuf = NULL;
-   bs = px_cipher_block_size(c);
-
-   /* encrypt */
-   if (bs > 1)
-   {
-       bbuf = palloc(bs * 4);
-       bpos = dlen % bs;
-       *rlen = dlen - bpos;
-       memcpy(bbuf, data + *rlen, bpos);
-
-       /* encrypt full-block data */
-       if (*rlen)
-       {
-           err = px_cipher_encrypt(c, data, *rlen, res);
-           if (err)
-               goto out;
-       }
-
-       /* bbuf has now bpos bytes of stuff */
-       if (cx->padding)
-       {
-           pad = bs - (bpos % bs);
-           for (i = 0; i < pad; i++)
-               bbuf[bpos++] = pad;
-       }
-       else if (bpos % bs)
-       {
-           /* ERROR? */
-           pad = bs - (bpos % bs);
-           for (i = 0; i < pad; i++)
-               bbuf[bpos++] = 0;
-       }
-
-       /* encrypt the rest - pad */
-       if (bpos)
-       {
-           err = px_cipher_encrypt(c, bbuf, bpos, res + *rlen);
-           *rlen += bpos;
-       }
-   }
-   else
-   {
-       /* stream cipher/mode - no pad needed */
-       err = px_cipher_encrypt(c, data, dlen, res);
-       if (err)
-           goto out;
-       *rlen = dlen;
-   }
-out:
-   if (bbuf)
-       pfree(bbuf);
-
-   return err;
+   return px_cipher_encrypt(cx->cipher, cx->padding, data, dlen, res, rlen);
 }
 
 static int
 combo_decrypt(PX_Combo *cx, const uint8 *data, unsigned dlen,
              uint8 *res, unsigned *rlen)
 {
-   int         err = 0;
-   unsigned    bs,
-               i,
-               pad;
-   unsigned    pad_ok;
-
-   PX_Cipher  *c = cx->cipher;
-
-   /* decide whether zero-length input is allowed */
-   if (dlen == 0)
-   {
-       /* with padding, empty ciphertext is not allowed */
-       if (cx->padding)
-           return PXE_DECRYPT_FAILED;
-
-       /* without padding, report empty result */
-       *rlen = 0;
-       return 0;
-   }
-
-   bs = px_cipher_block_size(c);
-   if (bs > 1 && (dlen % bs) != 0)
-       goto block_error;
-
-   /* decrypt */
-   *rlen = dlen;
-   err = px_cipher_decrypt(c, data, dlen, res);
-   if (err)
-       return err;
-
-   /* unpad */
-   if (bs > 1 && cx->padding)
-   {
-       pad = res[*rlen - 1];
-       pad_ok = 0;
-       if (pad > 0 && pad <= bs && pad <= *rlen)
-       {
-           pad_ok = 1;
-           for (i = *rlen - pad; i < *rlen; i++)
-               if (res[i] != pad)
-               {
-                   pad_ok = 0;
-                   break;
-               }
-       }
-
-       if (pad_ok)
-           *rlen -= pad;
-   }
-
-   return 0;
-
-block_error:
-   return PXE_NOTBLOCKSIZE;
+   return px_cipher_decrypt(cx->cipher, cx->padding, data, dlen, res, rlen);
 }
 
 static void

diff --git a/contrib/pgcrypto/px.h b/contrib/pgcrypto/px.h
index eef49a8b766b88f98ec0426c8f4db0c974d6a607..f175862f8e0a5d7583959b443c5a0fe7cc4c1575 100644 (file)
--- a/contrib/pgcrypto/px.h
+++ b/contrib/pgcrypto/px.h
@@ -47,7 +47,7 @@
 #define PXE_ERR_GENERIC                -1
 #define PXE_NO_HASH                    -2
 #define PXE_NO_CIPHER              -3
-#define PXE_NOTBLOCKSIZE           -4
+/* -4 is unused */
 #define PXE_BAD_OPTION             -5
 #define PXE_BAD_FORMAT             -6
 #define PXE_KEY_TOO_BIG                -7
@@ -144,8 +144,8 @@ struct px_cipher
    unsigned    (*iv_size) (PX_Cipher *c);
 
    int         (*init) (PX_Cipher *c, const uint8 *key, unsigned klen, const uint8 *iv);
-   int         (*encrypt) (PX_Cipher *c, const uint8 *data, unsigned dlen, uint8 *res);
-   int         (*decrypt) (PX_Cipher *c, const uint8 *data, unsigned dlen, uint8 *res);
+   int         (*encrypt) (PX_Cipher *c, int padding, const uint8 *data, unsigned dlen, uint8 *res, unsigned *rlen);
+   int         (*decrypt) (PX_Cipher *c, int padding, const uint8 *data, unsigned dlen, uint8 *res, unsigned *rlen);
    void        (*free) (PX_Cipher *c);
    /* private */
    void       *ptr;
@@ -208,10 +208,10 @@ void      px_debug(const char *fmt,...) pg_attribute_printf(1, 2);
 #define px_cipher_block_size(c)        (c)->block_size(c)
 #define px_cipher_iv_size(c)       (c)->iv_size(c)
 #define px_cipher_init(c, k, klen, iv) (c)->init(c, k, klen, iv)
-#define px_cipher_encrypt(c, data, dlen, res) \
-                   (c)->encrypt(c, data, dlen, res)
-#define px_cipher_decrypt(c, data, dlen, res) \
-                   (c)->decrypt(c, data, dlen, res)
+#define px_cipher_encrypt(c, padding, data, dlen, res, rlen) \
+                   (c)->encrypt(c, padding, data, dlen, res, rlen)
+#define px_cipher_decrypt(c, padding, data, dlen, res, rlen) \
+                   (c)->decrypt(c, padding, data, dlen, res, rlen)
 #define px_cipher_free(c)      (c)->free(c)
 
 

diff --git a/contrib/pgcrypto/sql/rijndael.sql b/contrib/pgcrypto/sql/rijndael.sql
index b162fc61f5f845f39fc1810b9605ef990741c768..a27664199804e591c7539b07f1fd267316c05eb2 100644 (file)
--- a/contrib/pgcrypto/sql/rijndael.sql
+++ b/contrib/pgcrypto/sql/rijndael.sql
@@ -24,6 +24,12 @@ SELECT encrypt(
 '\x000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f',
 'aes-cbc/pad:none');
 
+-- without padding, input not multiple of block size
+SELECT encrypt(
+'\x00112233445566778899aabbccddeeff00',
+'\x000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f',
+'aes-cbc/pad:none');
+
 -- key padding
 
 SELECT encrypt(
@@ -50,6 +56,12 @@ select encrypt('foo', '0123456789012345678901', 'aes');
 
 -- decrypt
 select encode(decrypt(encrypt('foo', '0123456', 'aes'), '0123456', 'aes'), 'escape');
+-- data not multiple of block size
+select encode(decrypt(encrypt('foo', '0123456', 'aes') || '\x00'::bytea, '0123456', 'aes'), 'escape');
+-- bad padding
+-- (The input value is the result of encrypt_iv('abcdefghijklmnopqrstuvwxyz', '0123456', 'abcd', 'aes')
+-- with the 16th byte changed (s/db/eb/) to corrupt the padding of the last block.)
+select encode(decrypt_iv('\xa21a9c15231465964e3396d32095e67eb52bab05f556a581621dee1b85385789', '0123456', 'abcd', 'aes'), 'escape');
 
 -- iv
 select encrypt_iv('foo', '0123456', 'abcd', 'aes');