Re: pgcrypto: Remove internal padding implementation

On Mon, 2022-02-21 at 11:42 +0100, Peter Eisentraut wrote:
> 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?

Sure -- that second ciphertext is the result of running

    SELECT encrypt_iv('abcdefghijklmnopqrstuvwxyz', '0123456', 'abcd', 'aes');

and then incrementing the last byte of the first block (i.e. the 16th
byte) to corrupt the padding in the last block.

> > 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.

I *think* the only reasonable way to prevent that attack is by
authenticating with a MAC or an authenticated cipher, to avoid running
decryption on corrupted ciphertext in the first place. But I'm out of
my expertise here.

> Is there any reasonable meaning of the previous behaviors?

I definitely don't think the previous behavior was reasonable. It's
possible that someone built a strange system on top of that
unreasonable behavior, but I hope not.

> Does bad padding even give correct answers on decryption?

No -- or rather, I'm not really sure how to define "correct answer" for
garbage input. I especially don't like that two different ciphertexts
can silently decrypt to the same plaintext.

> What does encryption without padding even do on incorrect input sizes?

Looks like it's being silently zero-padded by the previous code, which
doesn't seem very helpful to me. The documentation says "data must be
multiple of cipher block size" for the pad:none case, though I suppose
it doesn't say what happens if you ignore the "must".

--Jacob