public inbox for [email protected]  
help / color / mirror / Atom feed
From: Peter Eisentraut <[email protected]>
To: Jacob Champion <[email protected]>
To: [email protected] <[email protected]>
Subject: Re: pgcrypto: Remove internal padding implementation
Date: Mon, 21 Feb 2022 11:42:07 +0100
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<[email protected]>

On 15.02.22 00:07, Jacob Champion wrote:
> After this patch, 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. To see the difference you can try the
> following queries with and without the patch:
> 
>      select encrypt_iv('foo', '0123456', 'abcd', 'aes/pad:none');
>      select encode(decrypt_iv('\xa21a9c15231465964e3396d32095e67eb52bab05f556a581621dee1b85385789', '0123456', 'abcd', 'aes'), 'escape');

Interesting.  I have added test cases about this.  Could you describe 
how you arrived at the second test case?

> Both changes seem correct to me. I can imagine some system out there
> being somehow dependent on the prior decryption behavior to avoid a
> padding oracle -- but if that's a concern, hopefully you're not using
> unauthenticated encryption in the first place? It might be worth a note
> in the documentation.

Hmm.  I started reading up on "padding oracle attack".  I don't 
understand it well enough yet to know whether this is a concern here.

Is there any reasonable meaning of the previous behaviors?  Does bad 
padding even give correct answers on decryption?  What does encryption 
without padding even do on incorrect input sizes?
From 848289f7458d71742a327c1625c24a30981f9229 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
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



Attachments:

  [text/plain] v3-0001-pgcrypto-Remove-internal-padding-implementation.patch (10.5K, ../[email protected]/2-v3-0001-pgcrypto-Remove-internal-padding-implementation.patch)
  download | inline diff:
From 848289f7458d71742a327c1625c24a30981f9229 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
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



view thread (3+ messages)

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected]
  Subject: Re: pgcrypto: Remove internal padding implementation
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox