v3-0001-pgcrypto-Remove-internal-padding-implementation.patch

text/plain

Filename: v3-0001-pgcrypto-Remove-internal-padding-implementation.patch
Type: text/plain
Part: 0
Message: Re: pgcrypto: Remove internal padding implementation

Patch

Format: format-patch
Series: patch v3-0001
Subject: pgcrypto: Remove internal padding implementation
File+
contrib/pgcrypto/expected/rijndael.out 12 0
contrib/pgcrypto/openssl.c 14 8
contrib/pgcrypto/pgp-cfb.c 3 1
contrib/pgcrypto/px.c 2 118
contrib/pgcrypto/px.h 7 7
contrib/pgcrypto/sql/rijndael.sql 10 0
From 848289f7458d71742a327c1625c24a30981f9229 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Mon, 21 Feb 2022 11:24:27 +0100
Subject: [PATCH v3] 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 OpenSSL padding implementation is stricter than the previous
internal one: Bad padding is no longer ignored during decryption, and
encryption without padding now requires the input size to be a
multiple of the block size.  Add some test cases about this, too.

Discussion: https://www.postgresql.org/message-id/flat/ba94c26b-0c58-c97e-7a44-f44e08b4cca2%40enterprisedb.com
---
 contrib/pgcrypto/expected/rijndael.out |  12 +++
 contrib/pgcrypto/openssl.c             |  22 +++--
 contrib/pgcrypto/pgp-cfb.c             |   4 +-
 contrib/pgcrypto/px.c                  | 120 +------------------------
 contrib/pgcrypto/px.h                  |  14 +--
 contrib/pgcrypto/sql/rijndael.sql      |  10 +++
 6 files changed, 48 insertions(+), 134 deletions(-)

diff --git a/contrib/pgcrypto/expected/rijndael.out b/contrib/pgcrypto/expected/rijndael.out
index ad69cdba49..8dc103dd91 100644
--- 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,12 @@ 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
+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 e236b0d79c..68fd61b716 100644
--- 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 dafa562daa..de41e825b0 100644
--- 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 4205e9c3ef..914effa81f 100644
--- 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"},
@@ -223,129 +222,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 17d6f22498..c51655e352 100644
--- 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
@@ -143,8 +143,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;
@@ -207,10 +207,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 b162fc61f5..31e61990c6 100644
--- a/contrib/pgcrypto/sql/rijndael.sql
+++ b/contrib/pgcrypto/sql/rijndael.sql
@@ -24,6 +24,12 @@
 '\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,10 @@
 
 -- 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
+select encode(decrypt_iv('\xa21a9c15231465964e3396d32095e67eb52bab05f556a581621dee1b85385789', '0123456', 'abcd', 'aes'), 'escape');
 
 -- iv
 select encrypt_iv('foo', '0123456', 'abcd', 'aes');

base-commit: bf4ed12b58205d8527053d53c8f473074e191b8d
-- 
2.35.1