Re: pgcrypto: Remove internal padding implementation

On Mon, 2022-02-14 at 10:42 +0100, Peter Eisentraut wrote:
> This is a rebase of the patch from [0]. It removes the internal padding
> implementation in pgcrypto and lets OpenSSL do it. The internal
> implementation was once applicable to the non-OpenSSL code paths, but
> those have since been removed.

These removed parts looked interesting to me:

> - else if (bpos % bs)
> - {
> - /* ERROR? */
> - pad = bs - (bpos % bs);
> - for (i = 0; i < pad; i++)
> - bbuf[bpos++] = 0;
> - }

> - /* 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;
> - }

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');

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.

--Jacob