[Webkit-unassigned] [Bug 163125] [GTK] Use libgcrypt instead of GnuTLS for CryptoDigest and SubtleCrypto HMAC implementation

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=163125

--- Comment #13 from Olivier Blin <olivier.blin at softathome.com> ---
(In reply to comment #11)
> Comment on attachment 290961 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=290961&action=review
> 
> > 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

The same naming is using for the Mac and gnutls backends.

> > Source/WebCore/crypto/gcrypt/CryptoAlgorithmHMACGCrypt.cpp:66
> > +    gcry_mac_open(&hd, algorithm, 0, 0);
> 
> 0, nullptr

Ok

> > 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.

It can be smaller according to the API:
https://gnupg.org/documentation/manuals/gcrypt/Working-with-MAC-algorithms.html#Working-with-MAC-algorithms

I will resize it.

> > 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.

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 comment #12)
> Comment on attachment 290961 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=290961&action=review
> 
> > 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) \

Ok

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20161102/70b35280/attachment.html>