Horton

Re: OpenSSL 3.0.0 compatibility

Peter Eisentraut <peter.eisentraut@enterprisedb.com>

View thread View on postgr.es
From: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
To: Daniel Gustafsson <daniel@yesql.se>
Cc: Michael Paquier <michael@paquier.xyz>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2021-07-03T15:00:42Z
Lists: pgsql-hackers

Attachments

On 12.03.21 08:51, Peter Eisentraut wrote:
> 
> On 11.03.21 11:41, Daniel Gustafsson wrote:
>> .. and apply the padding changes as proposed in a patch upthread like 
>> this (these
>> work for all OpenSSL versions I've tested, and I'm rather more puzzled 
>> as to
>> why we got away with not having them in the past):
> 
> Yes, before proceeding with this, we should probably understand why 
> these changes are effective and why they haven't been required in the past.

I took another look at this with openssl-3.0.0-beta1.  The issue with 
the garbled padding output is still there.  What I found is that 
pgcrypto has been using the encryption and decryption API slightly 
incorrectly.  You are supposed to call EVP_DecryptUpdate() followed by 
EVP_DecryptFinal_ex() (and similarly for encryption), but pgcrypto 
doesn't do the second one.  (To be fair, this API was added to OpenSSL 
after pgcrypto first appeared.)  The "final" functions take care of the 
padding.  We have been getting away with it like this because we do the 
padding manually elsewhere.  But apparently, something has changed in 
OpenSSL 3.0.0 in that if padding is enabled in OpenSSL, 
EVP_DecryptUpdate() doesn't flush the last normal block until the 
"final" function is called.

Your proposed fix was to explicitly disable padding, and then this 
problem goes away.  You can still call the "final" functions, but they 
won't do anything, except check that there is no more data left, but we 
already check that elsewhere.

Another option is to throw out our own padding code and let OpenSSL 
handle it.  See attached demo patch.  But that breaks the non-OpenSSL 
code in internal.c, so we'd have to re-add the padding code there.  So 
this isn't quite as straightforward an option.  (At least, with the 
patch we can confirm that the OpenSSL padding works consistently with 
our own implementation.)

So I think your proposed patch is sound and a good short-term and 
low-risk solution.

Commits

  1. Define OPENSSL_API_COMPAT

  2. Add alternative output for OpenSSL 3 without legacy loaded

  3. Disable OpenSSL EVP digest padding in pgcrypto

  4. pgcrypto: Check for error return of px_cipher_decrypt()

  5. OpenSSL 3.0.0 compatibility in tests

  6. Make ssl certificate for ssl_passphrase_callback test via Makefile

  7. Provide a TLS init hook