Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit f5576a2

Browse files
committed
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 <pchampion@vmware.com> Reviewed-by: Nathan Bossart <nathandbossart@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/ba94c26b-0c58-c97e-7a44-f44e08b4cca2%40enterprisedb.com
1 parent 9ca234b commit f5576a2

File tree

6 files changed

+52
-134
lines changed

6 files changed

+52
-134
lines changed

contrib/pgcrypto/expected/rijndael.out

+14
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,12 @@ SELECT encrypt(
3939
\x8ea2b7ca516745bfeafc49904b496089
4040
(1 row)
4141

42+
-- without padding, input not multiple of block size
43+
SELECT encrypt(
44+
'\x00112233445566778899aabbccddeeff00',
45+
'\x000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f',
46+
'aes-cbc/pad:none');
47+
ERROR: encrypt error: Encryption failed
4248
-- key padding
4349
SELECT encrypt(
4450
'\x0011223344',
@@ -95,6 +101,14 @@ select encode(decrypt(encrypt('foo', '0123456', 'aes'), '0123456', 'aes'), 'esca
95101
foo
96102
(1 row)
97103

104+
-- data not multiple of block size
105+
select encode(decrypt(encrypt('foo', '0123456', 'aes') || '\x00'::bytea, '0123456', 'aes'), 'escape');
106+
ERROR: decrypt error: Decryption failed
107+
-- bad padding
108+
-- (The input value is the result of encrypt_iv('abcdefghijklmnopqrstuvwxyz', '0123456', 'abcd', 'aes')
109+
-- with the 16th byte changed (s/db/eb/) to corrupt the padding of the last block.)
110+
select encode(decrypt_iv('\xa21a9c15231465964e3396d32095e67eb52bab05f556a581621dee1b85385789', '0123456', 'abcd', 'aes'), 'escape');
111+
ERROR: decrypt_iv error: Decryption failed
98112
-- iv
99113
select encrypt_iv('foo', '0123456', 'abcd', 'aes');
100114
encrypt_iv

contrib/pgcrypto/openssl.c

+14-8
Original file line numberDiff line numberDiff line change
@@ -369,17 +369,17 @@ gen_ossl_free(PX_Cipher *c)
369369
}
370370

371371
static int
372-
gen_ossl_decrypt(PX_Cipher *c, const uint8 *data, unsigned dlen,
373-
uint8 *res)
372+
gen_ossl_decrypt(PX_Cipher *c, int padding, const uint8 *data, unsigned dlen,
373+
uint8 *res, unsigned *rlen)
374374
{
375375
OSSLCipher *od = c->ptr;
376-
int outlen;
376+
int outlen, outlen2;
377377

378378
if (!od->init)
379379
{
380380
if (!EVP_DecryptInit_ex(od->evp_ctx, od->evp_ciph, NULL, NULL, NULL))
381381
return PXE_CIPHER_INIT;
382-
if (!EVP_CIPHER_CTX_set_padding(od->evp_ctx, 0))
382+
if (!EVP_CIPHER_CTX_set_padding(od->evp_ctx, padding))
383383
return PXE_CIPHER_INIT;
384384
if (!EVP_CIPHER_CTX_set_key_length(od->evp_ctx, od->klen))
385385
return PXE_CIPHER_INIT;
@@ -390,22 +390,25 @@ gen_ossl_decrypt(PX_Cipher *c, const uint8 *data, unsigned dlen,
390390

391391
if (!EVP_DecryptUpdate(od->evp_ctx, res, &outlen, data, dlen))
392392
return PXE_DECRYPT_FAILED;
393+
if (!EVP_DecryptFinal_ex(od->evp_ctx, res + outlen, &outlen2))
394+
return PXE_DECRYPT_FAILED;
395+
*rlen = outlen + outlen2;
393396

394397
return 0;
395398
}
396399

397400
static int
398-
gen_ossl_encrypt(PX_Cipher *c, const uint8 *data, unsigned dlen,
399-
uint8 *res)
401+
gen_ossl_encrypt(PX_Cipher *c, int padding, const uint8 *data, unsigned dlen,
402+
uint8 *res, unsigned *rlen)
400403
{
401404
OSSLCipher *od = c->ptr;
402-
int outlen;
405+
int outlen, outlen2;
403406

404407
if (!od->init)
405408
{
406409
if (!EVP_EncryptInit_ex(od->evp_ctx, od->evp_ciph, NULL, NULL, NULL))
407410
return PXE_CIPHER_INIT;
408-
if (!EVP_CIPHER_CTX_set_padding(od->evp_ctx, 0))
411+
if (!EVP_CIPHER_CTX_set_padding(od->evp_ctx, padding))
409412
return PXE_CIPHER_INIT;
410413
if (!EVP_CIPHER_CTX_set_key_length(od->evp_ctx, od->klen))
411414
return PXE_CIPHER_INIT;
@@ -416,6 +419,9 @@ gen_ossl_encrypt(PX_Cipher *c, const uint8 *data, unsigned dlen,
416419

417420
if (!EVP_EncryptUpdate(od->evp_ctx, res, &outlen, data, dlen))
418421
return PXE_ENCRYPT_FAILED;
422+
if (!EVP_EncryptFinal_ex(od->evp_ctx, res + outlen, &outlen2))
423+
return PXE_ENCRYPT_FAILED;
424+
*rlen = outlen + outlen2;
419425

420426
return 0;
421427
}

contrib/pgcrypto/pgp-cfb.c

+3-1
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,9 @@ cfb_process(PGP_CFB *ctx, const uint8 *data, int len, uint8 *dst,
220220

221221
while (len > 0)
222222
{
223-
px_cipher_encrypt(ctx->ciph, ctx->fr, ctx->block_size, ctx->fre);
223+
unsigned rlen;
224+
225+
px_cipher_encrypt(ctx->ciph, 0, ctx->fr, ctx->block_size, ctx->fre, &rlen);
224226
if (ctx->block_no < 5)
225227
ctx->block_no++;
226228

contrib/pgcrypto/px.c

+2-118
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@ static const struct error_desc px_err_list[] = {
4444
{PXE_ERR_GENERIC, "Some PX error (not specified)"},
4545
{PXE_NO_HASH, "No such hash algorithm"},
4646
{PXE_NO_CIPHER, "No such cipher algorithm"},
47-
{PXE_NOTBLOCKSIZE, "Data not a multiple of block size"},
4847
{PXE_BAD_OPTION, "Unknown option"},
4948
{PXE_BAD_FORMAT, "Badly formatted type"},
5049
{PXE_KEY_TOO_BIG, "Key was too big"},
@@ -221,129 +220,14 @@ static int
221220
combo_encrypt(PX_Combo *cx, const uint8 *data, unsigned dlen,
222221
uint8 *res, unsigned *rlen)
223222
{
224-
int err = 0;
225-
uint8 *bbuf;
226-
unsigned bs,
227-
bpos,
228-
i,
229-
pad;
230-
231-
PX_Cipher *c = cx->cipher;
232-
233-
bbuf = NULL;
234-
bs = px_cipher_block_size(c);
235-
236-
/* encrypt */
237-
if (bs > 1)
238-
{
239-
bbuf = palloc(bs * 4);
240-
bpos = dlen % bs;
241-
*rlen = dlen - bpos;
242-
memcpy(bbuf, data + *rlen, bpos);
243-
244-
/* encrypt full-block data */
245-
if (*rlen)
246-
{
247-
err = px_cipher_encrypt(c, data, *rlen, res);
248-
if (err)
249-
goto out;
250-
}
251-
252-
/* bbuf has now bpos bytes of stuff */
253-
if (cx->padding)
254-
{
255-
pad = bs - (bpos % bs);
256-
for (i = 0; i < pad; i++)
257-
bbuf[bpos++] = pad;
258-
}
259-
else if (bpos % bs)
260-
{
261-
/* ERROR? */
262-
pad = bs - (bpos % bs);
263-
for (i = 0; i < pad; i++)
264-
bbuf[bpos++] = 0;
265-
}
266-
267-
/* encrypt the rest - pad */
268-
if (bpos)
269-
{
270-
err = px_cipher_encrypt(c, bbuf, bpos, res + *rlen);
271-
*rlen += bpos;
272-
}
273-
}
274-
else
275-
{
276-
/* stream cipher/mode - no pad needed */
277-
err = px_cipher_encrypt(c, data, dlen, res);
278-
if (err)
279-
goto out;
280-
*rlen = dlen;
281-
}
282-
out:
283-
if (bbuf)
284-
pfree(bbuf);
285-
286-
return err;
223+
return px_cipher_encrypt(cx->cipher, cx->padding, data, dlen, res, rlen);
287224
}
288225

289226
static int
290227
combo_decrypt(PX_Combo *cx, const uint8 *data, unsigned dlen,
291228
uint8 *res, unsigned *rlen)
292229
{
293-
int err = 0;
294-
unsigned bs,
295-
i,
296-
pad;
297-
unsigned pad_ok;
298-
299-
PX_Cipher *c = cx->cipher;
300-
301-
/* decide whether zero-length input is allowed */
302-
if (dlen == 0)
303-
{
304-
/* with padding, empty ciphertext is not allowed */
305-
if (cx->padding)
306-
return PXE_DECRYPT_FAILED;
307-
308-
/* without padding, report empty result */
309-
*rlen = 0;
310-
return 0;
311-
}
312-
313-
bs = px_cipher_block_size(c);
314-
if (bs > 1 && (dlen % bs) != 0)
315-
goto block_error;
316-
317-
/* decrypt */
318-
*rlen = dlen;
319-
err = px_cipher_decrypt(c, data, dlen, res);
320-
if (err)
321-
return err;
322-
323-
/* unpad */
324-
if (bs > 1 && cx->padding)
325-
{
326-
pad = res[*rlen - 1];
327-
pad_ok = 0;
328-
if (pad > 0 && pad <= bs && pad <= *rlen)
329-
{
330-
pad_ok = 1;
331-
for (i = *rlen - pad; i < *rlen; i++)
332-
if (res[i] != pad)
333-
{
334-
pad_ok = 0;
335-
break;
336-
}
337-
}
338-
339-
if (pad_ok)
340-
*rlen -= pad;
341-
}
342-
343-
return 0;
344-
345-
block_error:
346-
return PXE_NOTBLOCKSIZE;
230+
return px_cipher_decrypt(cx->cipher, cx->padding, data, dlen, res, rlen);
347231
}
348232

349233
static void

contrib/pgcrypto/px.h

+7-7
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@
4747
#define PXE_ERR_GENERIC -1
4848
#define PXE_NO_HASH -2
4949
#define PXE_NO_CIPHER -3
50-
#define PXE_NOTBLOCKSIZE -4
50+
/* -4 is unused */
5151
#define PXE_BAD_OPTION -5
5252
#define PXE_BAD_FORMAT -6
5353
#define PXE_KEY_TOO_BIG -7
@@ -144,8 +144,8 @@ struct px_cipher
144144
unsigned (*iv_size) (PX_Cipher *c);
145145

146146
int (*init) (PX_Cipher *c, const uint8 *key, unsigned klen, const uint8 *iv);
147-
int (*encrypt) (PX_Cipher *c, const uint8 *data, unsigned dlen, uint8 *res);
148-
int (*decrypt) (PX_Cipher *c, const uint8 *data, unsigned dlen, uint8 *res);
147+
int (*encrypt) (PX_Cipher *c, int padding, const uint8 *data, unsigned dlen, uint8 *res, unsigned *rlen);
148+
int (*decrypt) (PX_Cipher *c, int padding, const uint8 *data, unsigned dlen, uint8 *res, unsigned *rlen);
149149
void (*free) (PX_Cipher *c);
150150
/* private */
151151
void *ptr;
@@ -208,10 +208,10 @@ void px_debug(const char *fmt,...) pg_attribute_printf(1, 2);
208208
#define px_cipher_block_size(c) (c)->block_size(c)
209209
#define px_cipher_iv_size(c) (c)->iv_size(c)
210210
#define px_cipher_init(c, k, klen, iv) (c)->init(c, k, klen, iv)
211-
#define px_cipher_encrypt(c, data, dlen, res) \
212-
(c)->encrypt(c, data, dlen, res)
213-
#define px_cipher_decrypt(c, data, dlen, res) \
214-
(c)->decrypt(c, data, dlen, res)
211+
#define px_cipher_encrypt(c, padding, data, dlen, res, rlen) \
212+
(c)->encrypt(c, padding, data, dlen, res, rlen)
213+
#define px_cipher_decrypt(c, padding, data, dlen, res, rlen) \
214+
(c)->decrypt(c, padding, data, dlen, res, rlen)
215215
#define px_cipher_free(c) (c)->free(c)
216216

217217

contrib/pgcrypto/sql/rijndael.sql

+12
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,12 @@ SELECT encrypt(
2424
'\x000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f',
2525
'aes-cbc/pad:none');
2626

27+
-- without padding, input not multiple of block size
28+
SELECT encrypt(
29+
'\x00112233445566778899aabbccddeeff00',
30+
'\x000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f',
31+
'aes-cbc/pad:none');
32+
2733
-- key padding
2834

2935
SELECT encrypt(
@@ -50,6 +56,12 @@ select encrypt('foo', '0123456789012345678901', 'aes');
5056

5157
-- decrypt
5258
select encode(decrypt(encrypt('foo', '0123456', 'aes'), '0123456', 'aes'), 'escape');
59+
-- data not multiple of block size
60+
select encode(decrypt(encrypt('foo', '0123456', 'aes') || '\x00'::bytea, '0123456', 'aes'), 'escape');
61+
-- bad padding
62+
-- (The input value is the result of encrypt_iv('abcdefghijklmnopqrstuvwxyz', '0123456', 'abcd', 'aes')
63+
-- with the 16th byte changed (s/db/eb/) to corrupt the padding of the last block.)
64+
select encode(decrypt_iv('\xa21a9c15231465964e3396d32095e67eb52bab05f556a581621dee1b85385789', '0123456', 'abcd', 'aes'), 'escape');
5365

5466
-- iv
5567
select encrypt_iv('foo', '0123456', 'abcd', 'aes');

0 commit comments

Comments
 (0)