Thread: pgcrypto: openssl digest fix
Some builds (depends on crypto engine support?) of OpenSSL 0.9.7x have EVP_DigestFinal function which which clears all of EVP_MD_CTX. This makes pgcrypto crash in functions which re-use one digest context several times: hmac() and crypt() with md5 algorithm. Following patch fixes it by carring the digest info around EVP_DigestFinal and re-initializing cipher. Please apply this also to stable branches (8.0 / 7.4). Note that this can be blamed on OpenSSL 0.9.7x backwards- compatibility functions: 0.9.6x and new 0.9.7x API (EVP_DigestFinal_ex) do clear the "secret data" but keep the general algorithm info. But still, the fact is that pgcrypto was relying on undocumented beheviour. -- marko
Attachment
Marko Kreen wrote: > Some builds (depends on crypto engine support?) of OpenSSL > 0.9.7x have EVP_DigestFinal function which which clears all of > EVP_MD_CTX. This makes pgcrypto crash in functions which > re-use one digest context several times: hmac() and crypt() > with md5 algorithm. > > Following patch fixes it by carring the digest info around > EVP_DigestFinal and re-initializing cipher. Applied to HEAD, REL8_0_STABLE and REL7_4_STABLE. Thanks for the patch. > Please apply this also to stable branches (8.0 / 7.4). Should it be backpatched to 7.3 and 7.2 as well? -Neil
On Sat, Mar 12, 2005 at 05:59:24PM +1100, Neil Conway wrote: > Marko Kreen wrote: > >Please apply this also to stable branches (8.0 / 7.4). > > Should it be backpatched to 7.3 and 7.2 as well? It would be nice. I didn't know there are releases of those planned as well. Now looking into it, 7.3 and 7.2 branch are missing the OpenSSL EVP cipher functions removal patch - which is even more nasty as it does not crash but silently corrupts data. 'make installcheck' detects it, but if somebody forgets to run it... (Thankfully encrypt()/decrypt() are not used much.) Would you apply this one aswell? I see that the original patch (openssl.c r1.11) applies to both branches without problems. It is a bit larger than this one tho'. -- marko
Marko Kreen wrote: > Would you apply this one aswell? I see that the original > patch (openssl.c r1.11) applies to both branches without problems. > It is a bit larger than this one tho'. Should there have been a patch attached to this mail? -Neil
On Sun, Mar 13, 2005 at 11:12:42AM +1100, Neil Conway wrote: > Marko Kreen wrote: > >Would you apply this one aswell? I see that the original > >patch (openssl.c r1.11) applies to both branches without problems. > >It is a bit larger than this one tho'. > > Should there have been a patch attached to this mail? Ah, ofcourse. -- marko
Attachment
Marko Kreen wrote: > Ah, ofcourse. The patch seems rather large to be applying to 7.3 and 7.2 -- but it's your contrib/ module, so I'll assume you're pretty confident this doesn't cause any regressions... I'll apply the patch to 7.3 and 7.2 stable branches tomorrow. -Neil
On Sun, Mar 13, 2005 at 09:43:02PM +1100, Neil Conway wrote: > Marko Kreen wrote: > >Ah, ofcourse. > > The patch seems rather large to be applying to 7.3 and 7.2 -- but it's > your contrib/ module, so I'll assume you're pretty confident this > doesn't cause any regressions... The patch itself is simply "cvs diff -r1.10 -r1.11 openssl.c", so there should not be any recent typos in it. Now I also tested it with both REL7_3_STABLE and REL7_2_STABLE and found no problems. So I think its fine. > I'll apply the patch to 7.3 and 7.2 stable branches tomorrow. Cool. -- marko
Marko Kreen wrote: > The patch itself is simply "cvs diff -r1.10 -r1.11 openssl.c", > so there should not be any recent typos in it. Now I also tested > it with both REL7_3_STABLE and REL7_2_STABLE and found no problems. > So I think its fine. I've applied both this patch and the original patch (fix-openssl.diff) to REL7_3_STABLE and REL7_2_STABLE. Thanks for the patches. -Neil