Re: pgcrypto: Remove internal padding implementation - Mailing list pgsql-hackers

From Jacob Champion
Subject Re: pgcrypto: Remove internal padding implementation
Date
Msg-id adc64e21677b8b896203d06f231762ff5d8e4652.camel@vmware.com
Whole thread Raw
In response to pgcrypto: Remove internal padding implementation  (Peter Eisentraut <peter.eisentraut@enterprisedb.com>)
Responses Re: pgcrypto: Remove internal padding implementation  (Peter Eisentraut <peter.eisentraut@enterprisedb.com>)
List pgsql-hackers
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

pgsql-hackers by date:

Previous
From: Andres Freund
Date:
Subject: Re: fixing bookindex.html bloat
Next
From: Swaha Miller
Date:
Subject: Re: support for CREATE MODULE