[webkit-reviews] review denied: [Bug 173589] [GCrypt] Properly initialize libgcrypt before using it : [Attachment 313387] Patch

[bugzilla-daemon at webkit.org]

Michael Catanzaro <mcatanzaro at igalia.com> has denied Zan Dobersek
<zan at falconsigh.net>'s request for review:
Bug 173589: [GCrypt] Properly initialize libgcrypt before using it
https://bugs.webkit.org/show_bug.cgi?id=173589

Attachment 313387: Patch

https://bugs.webkit.org/attachment.cgi?id=313387&action=review




--- Comment #4 from Michael Catanzaro <mcatanzaro at igalia.com> ---
Comment on attachment 313387
  --> https://bugs.webkit.org/attachment.cgi?id=313387
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=313387&action=review

Yeah, I agree with Carlos Garcia, there's got to be a better way to do this. If
it has to be done in WebKit2 entry points, that would be unfortunate layering,
but even that would seem preferable to adding such a huge number of
initialization calls.

There's another problem here: what about applications that use Libgcrypt
themselves? [1] recommends ensuring that applications have not already
initialized Libgcrypt before doing so. It also recommends enabling secure
memory before initialization, to ensure crypto keys don't get swapped to disk.
So I think PAL::GCrypt::initialize should do something like this:

// Inside std::call_once and !gcry_check_version
if (!gcry_control (GCRYCTL_INITIALIZATION_FINISHED_P)) {
    gcry_control (GCRYCTL_INIT_SECMEM, 16384, nullptr);
    gcry_control (GCRYCTL_INITIALIZATION_FINISHED, nullptr);
}

[1]
https://www.gnupg.org/documentation/manuals/gcrypt/Initializing-the-library.htm
l

> Source/WebCore/PAL/pal/crypto/gcrypt/Utilities.cpp:43
> +	   if (!gcry_check_version("1.7.0"))

This is going to be lost and forgotten whenever we update our min Libgcrypt
version. I first thought we should remove the runtime check, but apparently
Libgcrypt really wants this to happen first. So I would instead ensure it gets
defined somehow by CMake (probably in config.cmake) so that the min version can
be reused here instead of hardcoded.