<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - [GTK] Use libgcrypt instead of GnuTLS for CryptoDigest and SubtleCrypto HMAC implementation"
href="https://bugs.webkit.org/show_bug.cgi?id=163125#c13">Comment # 13</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - [GTK] Use libgcrypt instead of GnuTLS for CryptoDigest and SubtleCrypto HMAC implementation"
href="https://bugs.webkit.org/show_bug.cgi?id=163125">bug 163125</a>
from <span class="vcard"><a class="email" href="mailto:olivier.blin@softathome.com" title="Olivier Blin <olivier.blin@softathome.com>"> <span class="fn">Olivier Blin</span></a>
</span></b>
<pre>(In reply to <a href="show_bug.cgi?id=163125#c11">comment #11</a>)
<span class="quote">> Comment on <span class=""><a href="attachment.cgi?id=290961&action=diff" name="attach_290961" title="Patch">attachment 290961</a> <a href="attachment.cgi?id=290961&action=edit" title="Patch">[details]</a></span>
> Patch
>
> View in context:
> <a href="https://bugs.webkit.org/attachment.cgi?id=290961&action=review">https://bugs.webkit.org/attachment.cgi?id=290961&action=review</a>
>
> > Source/WebCore/crypto/gcrypt/CryptoAlgorithmHMACGCrypt.cpp:40
> > +static int getGCryptDigestAlgorithm(CryptoAlgorithmIdentifier hashFunction)
>
> Don't use "get" here. You could use
> cryptoAlgorithmIdentifierToGCryptDigestAlgorithm or something similar, or
> gcryptDigestAlgorithmForCryptoAlgorithmIdentifier</span >
The same naming is using for the Mac and gnutls backends.
<span class="quote">> > Source/WebCore/crypto/gcrypt/CryptoAlgorithmHMACGCrypt.cpp:66
> > + gcry_mac_open(&hd, algorithm, 0, 0);
>
> 0, nullptr</span >
Ok
<span class="quote">> > Source/WebCore/crypto/gcrypt/CryptoAlgorithmHMACGCrypt.cpp:69
> > + gcry_mac_read(hd, result.data(), &digestLength);
>
> Don't you need to handle the return value of digestLength? Can it really be
> different to result.size()? I guess it can't be bigger than value returned
> by gcry_mac_get_algo_maclen, but can it be smaller? If it should be the same
> size I would add an assert, otherwise you would need to resize the vector
> here.</span >
It can be smaller according to the API:
<a href="https://gnupg.org/documentation/manuals/gcrypt/Working-with-MAC-algorithms.html#Working-with-MAC-algorithms">https://gnupg.org/documentation/manuals/gcrypt/Working-with-MAC-algorithms.html#Working-with-MAC-algorithms</a>
I will resize it.
<span class="quote">> > Source/WebCore/crypto/gcrypt/CryptoAlgorithmHMACGCrypt.cpp:72
> > + return result;
>
> Most of the gcry functions used above return a gcry_error_t that is not
> handled at all. We should call the failureCallback in platformSign if any of
> those fail.</span >
I did not find code that made use of the failureCallback, besides CryptoAlgorithmHMAC::generateKey
I'll upload a version which handles gcry failures, but it makes the layout tests hang on failure.
(In reply to <a href="show_bug.cgi?id=163125#c12">comment #12</a>)
<span class="quote">> Comment on <span class=""><a href="attachment.cgi?id=290961&action=diff" name="attach_290961" title="Patch">attachment 290961</a> <a href="attachment.cgi?id=290961&action=edit" title="Patch">[details]</a></span>
> Patch
>
> View in context:
> <a href="https://bugs.webkit.org/attachment.cgi?id=290961&action=review">https://bugs.webkit.org/attachment.cgi?id=290961&action=review</a>
>
> > Tools/gtk/install-dependencies:109
> > libfaad-dev \
> > + libgcrypt11-dev \
> > $(aptIfElse libgeoclue-2-dev libgeoclue-dev) \
>
> On Debian/Ubuntu libgcrypt11-dev is a transitional dummy package to ease the
> migration from the old libgcrypt11-dev to libgcrypt20-dev. So I would use
> here:
> $(aptIfElse libgcrypt20-dev libgcrypt11-dev) \</span >
Ok</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>