> On 19 Sep 2020, at 04:11, Michael Paquier <mich...@paquier.xyz> wrote: > On Fri, Sep 18, 2020 at 04:11:13PM +0200, Daniel Gustafsson wrote:
>> The other problem was that the cipher context >> padding must be explicitly set, whereas in previous versions relying on the >> default worked fine. EVP_CIPHER_CTX_set_padding always returns 1 so thats >> why >> it isn't checking the returnvalue as the other nearby initialization calls. > > It seems to me that it would be a good idea to still check for the > return value of EVP_CIPHER_CTX_set_padding() and just return with > a PXE_CIPHER_INIT. By looking at the upstream code, it is true that > it always returns true for <= 1.1.1, but that's not the case for > 3.0.0. Some code paths of upstream also check after it. Thats a good point, it's now provider dependent and can thus fail in case the provider isn't supplying the functionality. We've already loaded a provider where we know the call is supported, but it's of course better to check. Fixed in the attached v2. I was only reading the docs and not the code, and they haven't been updated to reflect this. I'll open a PR to the OpenSSL devs to fix that. > Also, what's the impact with disabling the padding for <= 1.1.1? This > part of the upstream code is still a bit obscure to me.. I'm far from fluent in OpenSSL, but AFAICT it has to do with the new provider API. The default value for padding is unchanged, but it needs to be propaged into the provider to be set in the context where the old code picked it up automatically. The relevant OpenSSL commit (83f68df32f0067ee7b0) which changes the docs to say the function should be called doesn't contain enough information to explain why. >> To avoid problems with the by LibreSSL overloaded OPENSSL_VERSION_NUMBER >> macro >> (which too is deprecated in 3.0.0), I used the new macro which is only set in >> 3.0.0. Not sure if that's considered acceptable or if we should invent our >> own >> version macro in autoconf. > > OSSL_PROVIDER_load() is new as of 3.0.0, so using a configure switch > similarly as what we do for the other functions should be more > consistent and enough, no? Good point, fixed in the attached. >> For the main SSL tests, the incorrect password test has a new errormessage >> which is added in 0002. > > Hmm. I am linking to a build of alpha6 here, but I still see the > error being reported as a bad decrypt for this test. Interesting. Turns out it was coming from when I tested against OpenSSL git HEAD, so it may come in alpha7 (or whatever the next version will be). Let's disregard this for now until dust settles, I've dropped the patch from the series. cheers ./daniel
0001-Fix-OpenSSL-3-support-in-pgcrypto-v2.patch
Description: Binary data
