Bug 138612 - [EFL] Add subtle crypto to the build system
Summary: [EFL] Add subtle crypto to the build system
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-11-11 08:18 PST by Éva Balázsfalvi
Modified: 2014-12-04 05:23 PST (History)
8 users (show)

See Also:


Attachments
Patch (16.15 KB, patch)
2014-11-11 08:48 PST, Éva Balázsfalvi
no flags Details | Formatted Diff | Diff
Patch (17.06 KB, patch)
2014-11-14 06:58 PST, Éva Balázsfalvi
no flags Details | Formatted Diff | Diff
Patch (16.38 KB, patch)
2014-11-17 07:54 PST, Éva Balázsfalvi
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Éva Balázsfalvi 2014-11-11 08:18:35 PST
Add subtle crypto to the build system.

It is obvious to make Efl use GnuTLS as well, and since there seems no reason why to separate Efl and Gtk implementations from each other, I also propose renaming the gtk directory and file suffixes to gnutls.
Comment 1 Éva Balázsfalvi 2014-11-11 08:48:37 PST
Created attachment 241352 [details]
Patch
Comment 2 WebKit Commit Bot 2014-11-11 08:52:03 PST
Attachment 241352 [details] did not pass style-queue:


ERROR: Source/WebCore/crypto/gnutls/CryptoDigestGnuTLS.cpp:31:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSA_OAEPGnuTLS.cpp:38:  CryptoAlgorithmRSA_OAEP::platformEncrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSA_OAEPGnuTLS.cpp:50:  CryptoAlgorithmRSA_OAEP::platformDecrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmHMACGnuTLS.cpp:34:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmAES_KWGnuTLS.cpp:37:  CryptoAlgorithmAES_KW::platformEncrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmAES_KWGnuTLS.cpp:48:  CryptoAlgorithmAES_KW::platformDecrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSASSA_PKCS1_v1_5GnuTLS.cpp:38:  CryptoAlgorithmRSASSA_PKCS1_v1_5::platformSign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSASSA_PKCS1_v1_5GnuTLS.cpp:51:  CryptoAlgorithmRSASSA_PKCS1_v1_5::platformVerify is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSAES_PKCS1_v1_5GnuTLS.cpp:37:  CryptoAlgorithmRSAES_PKCS1_v1_5::platformEncrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSAES_PKCS1_v1_5GnuTLS.cpp:48:  CryptoAlgorithmRSAES_PKCS1_v1_5::platformDecrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmAES_CBCGnuTLS.cpp:38:  CryptoAlgorithmAES_CBC::platformEncrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmAES_CBCGnuTLS.cpp:50:  CryptoAlgorithmAES_CBC::platformDecrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 12 in 20 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Gyuyoung Kim 2014-11-12 20:36:59 PST
Comment on attachment 241352 [details]
Patch

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

Looks fine for EFL port. However I think we need to get agreement from GTK port folks.

> Source/WebCore/PlatformEfl.cmake:503
> +        crypto/gnutls/CryptoAlgorithmRegistryGnuTLS.cpp

Wrong alphabet sorting. These files should be placed in front of crypto/keys/
Comment 4 Gyuyoung Kim 2014-11-12 20:37:40 PST
Comment on attachment 241352 [details]
Patch

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

> Source/cmake/OptionsEfl.cmake:107
> +WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_SUBTLE_CRYPTO OFF)

BTW, is there any reason SUBTLE_CRYPTO is disabled for cmake build ?
Comment 5 Éva Balázsfalvi 2014-11-14 04:34:34 PST
(In reply to comment #4)
> Comment on attachment 241352 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=241352&action=review
> 
> > Source/cmake/OptionsEfl.cmake:107
> > +WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_SUBTLE_CRYPTO OFF)
> 
> BTW, is there any reason SUBTLE_CRYPTO is disabled for cmake build ?

When it came to enabling SUBTLE_CRYPTO, I based my patch on bug 135798. There is a good reasoning why to disable it by default, and I thought we need this kind of solution in case of EFL, too.
Comment 6 Éva Balázsfalvi 2014-11-14 06:58:21 PST
Created attachment 241586 [details]
Patch
Comment 7 WebKit Commit Bot 2014-11-14 06:59:54 PST
Attachment 241586 [details] did not pass style-queue:


ERROR: Source/WebCore/crypto/gnutls/CryptoDigestGnuTLS.cpp:31:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSA_OAEPGnuTLS.cpp:38:  CryptoAlgorithmRSA_OAEP::platformEncrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSA_OAEPGnuTLS.cpp:50:  CryptoAlgorithmRSA_OAEP::platformDecrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmHMACGnuTLS.cpp:34:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmAES_KWGnuTLS.cpp:37:  CryptoAlgorithmAES_KW::platformEncrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmAES_KWGnuTLS.cpp:48:  CryptoAlgorithmAES_KW::platformDecrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSASSA_PKCS1_v1_5GnuTLS.cpp:38:  CryptoAlgorithmRSASSA_PKCS1_v1_5::platformSign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSASSA_PKCS1_v1_5GnuTLS.cpp:51:  CryptoAlgorithmRSASSA_PKCS1_v1_5::platformVerify is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSAES_PKCS1_v1_5GnuTLS.cpp:37:  CryptoAlgorithmRSAES_PKCS1_v1_5::platformEncrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSAES_PKCS1_v1_5GnuTLS.cpp:48:  CryptoAlgorithmRSAES_PKCS1_v1_5::platformDecrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmAES_CBCGnuTLS.cpp:38:  CryptoAlgorithmAES_CBC::platformEncrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmAES_CBCGnuTLS.cpp:50:  CryptoAlgorithmAES_CBC::platformDecrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 12 in 20 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 8 Csaba Osztrogonác 2014-11-17 02:07:38 PST
(In reply to comment #5)
> (In reply to comment #4)
> > Comment on attachment 241352 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=241352&action=review
> > 
> > > Source/cmake/OptionsEfl.cmake:107
> > > +WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_SUBTLE_CRYPTO OFF)
> > 
> > BTW, is there any reason SUBTLE_CRYPTO is disabled for cmake build ?
> 
> When it came to enabling SUBTLE_CRYPTO, I based my patch on bug 135798.
> There is a good reasoning why to disable it by default, and I thought we
> need this kind of solution in case of EFL, too.

Disabling in Options<Efl|Gtk>.cmake and enabling it in Features.pm
is the common way for experimental / not completed features.

Features.pm is only used by build-webkit, which is for developers and bots
to be able test experimental features too. But production builds don't use
build-webkit at all, they call cmake directly, so experimental features are
disabled for production builds. Crypto seems to be in a very early stage now,
so it shouldn't be enabled for production build yet.
Comment 9 Csaba Osztrogonác 2014-11-17 02:12:51 PST
(In reply to comment #7)
> Attachment 241586 [details] did not pass style-queue:
> 
> 
> ERROR: Source/WebCore/crypto/gnutls/CryptoDigestGnuTLS.cpp:31:  Alphabetical
> sorting problem.  [build/include_order] [4]
> ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSA_OAEPGnuTLS.cpp:38: 
> CryptoAlgorithmRSA_OAEP::platformEncrypt is incorrectly named. Don't use
> underscores in your identifier names.  [readability/naming/underscores] [4]
> ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSA_OAEPGnuTLS.cpp:50: 
> CryptoAlgorithmRSA_OAEP::platformDecrypt is incorrectly named. Don't use
> underscores in your identifier names.  [readability/naming/underscores] [4]
> ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmHMACGnuTLS.cpp:34: 
> Alphabetical sorting problem.  [build/include_order] [4]
> ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmAES_KWGnuTLS.cpp:37: 
> CryptoAlgorithmAES_KW::platformEncrypt is incorrectly named. Don't use
> underscores in your identifier names.  [readability/naming/underscores] [4]
> ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmAES_KWGnuTLS.cpp:48: 
> CryptoAlgorithmAES_KW::platformDecrypt is incorrectly named. Don't use
> underscores in your identifier names.  [readability/naming/underscores] [4]
> ERROR:
> Source/WebCore/crypto/gnutls/CryptoAlgorithmRSASSA_PKCS1_v1_5GnuTLS.cpp:38: 
> CryptoAlgorithmRSASSA_PKCS1_v1_5::platformSign is incorrectly named. Don't
> use underscores in your identifier names.  [readability/naming/underscores]
> [4]
> ERROR:
> Source/WebCore/crypto/gnutls/CryptoAlgorithmRSASSA_PKCS1_v1_5GnuTLS.cpp:51: 
> CryptoAlgorithmRSASSA_PKCS1_v1_5::platformVerify is incorrectly named. Don't
> use underscores in your identifier names.  [readability/naming/underscores]
> [4]
> ERROR:
> Source/WebCore/crypto/gnutls/CryptoAlgorithmRSAES_PKCS1_v1_5GnuTLS.cpp:37: 
> CryptoAlgorithmRSAES_PKCS1_v1_5::platformEncrypt is incorrectly named. Don't
> use underscores in your identifier names.  [readability/naming/underscores]
> [4]
> ERROR:
> Source/WebCore/crypto/gnutls/CryptoAlgorithmRSAES_PKCS1_v1_5GnuTLS.cpp:48: 
> CryptoAlgorithmRSAES_PKCS1_v1_5::platformDecrypt is incorrectly named. Don't
> use underscores in your identifier names.  [readability/naming/underscores]
> [4]
> ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmAES_CBCGnuTLS.cpp:38: 
> CryptoAlgorithmAES_CBC::platformEncrypt is incorrectly named. Don't use
> underscores in your identifier names.  [readability/naming/underscores] [4]
> ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmAES_CBCGnuTLS.cpp:50: 
> CryptoAlgorithmAES_CBC::platformDecrypt is incorrectly named. Don't use
> underscores in your identifier names.  [readability/naming/underscores] [4]
> Total errors found: 12 in 20 files
> 
> 
> If any of these errors are false positives, please file a bug against
> check-webkit-style.

We can ignore these style issues, the class names is come from the original
design: Source/WebCore/crypto/algorithms/*.h . We shouldn't change them now.
Comment 10 Éva Balázsfalvi 2014-11-17 07:54:00 PST
Created attachment 241712 [details]
Patch

Had to undo two, previously corrected 'alphabetical order' style issue, because we can't switch the order of #include <gnutls/crypto.h> and #include <gnutls/gnutls.h>, or else the build will fail.
Comment 11 WebKit Commit Bot 2014-11-17 07:56:58 PST
Attachment 241712 [details] did not pass style-queue:


ERROR: Source/WebCore/crypto/gnutls/CryptoDigestGnuTLS.cpp:31:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSA_OAEPGnuTLS.cpp:38:  CryptoAlgorithmRSA_OAEP::platformEncrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSA_OAEPGnuTLS.cpp:50:  CryptoAlgorithmRSA_OAEP::platformDecrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmHMACGnuTLS.cpp:34:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmAES_KWGnuTLS.cpp:37:  CryptoAlgorithmAES_KW::platformEncrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmAES_KWGnuTLS.cpp:48:  CryptoAlgorithmAES_KW::platformDecrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSASSA_PKCS1_v1_5GnuTLS.cpp:38:  CryptoAlgorithmRSASSA_PKCS1_v1_5::platformSign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSASSA_PKCS1_v1_5GnuTLS.cpp:51:  CryptoAlgorithmRSASSA_PKCS1_v1_5::platformVerify is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSAES_PKCS1_v1_5GnuTLS.cpp:37:  CryptoAlgorithmRSAES_PKCS1_v1_5::platformEncrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSAES_PKCS1_v1_5GnuTLS.cpp:48:  CryptoAlgorithmRSAES_PKCS1_v1_5::platformDecrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmAES_CBCGnuTLS.cpp:38:  CryptoAlgorithmAES_CBC::platformEncrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmAES_CBCGnuTLS.cpp:50:  CryptoAlgorithmAES_CBC::platformDecrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 12 in 20 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 12 Csaba Osztrogonác 2014-11-18 06:10:50 PST
any feedback from GTK side?
Comment 13 Éva Balázsfalvi 2014-12-01 03:37:09 PST
Ping?
Comment 14 Philippe Normand 2014-12-02 00:09:19 PST
Comment on attachment 241712 [details]
Patch

lgtm
Comment 15 Csaba Osztrogonác 2014-12-03 00:46:38 PST
Comment on attachment 241712 [details]
Patch

It is good for EFL and GTK, so final r+.
Comment 16 WebKit Commit Bot 2014-12-03 01:24:44 PST
Comment on attachment 241712 [details]
Patch

Clearing flags on attachment: 241712

Committed r176712: <http://trac.webkit.org/changeset/176712>
Comment 17 WebKit Commit Bot 2014-12-03 01:24:50 PST
All reviewed patches have been landed.  Closing bug.
Comment 18 Joonghun Park 2014-12-04 01:31:58 PST
It seems that there is an insignificant problem that Gtk and Efl depends on libgnutls28-dev when I see into Tools/efl/install-dependencies and Tools/gtk/install-dependencies.

So once execute build-webkit, then it leads to version check fail because system installed gnutls version is 2.12.23 which is installed by install-dependencies, and its version is lower than the version subtle_crypto feature requires(3.0).
So I think it will be needed to add gnutls(>=3.0) to install-dependencies in install-dependencies in port EFL and GTK.

Please consider  the comment I made above and if I missed something then please let me know.
Comment 19 Philippe Normand 2014-12-04 04:07:28 PST
I don't know which distro you use but on Debian at least, libgnutls28 provides gnutls 3.x, I know it's confusing...

pkg-config --modversion gnutls
3.3.8
Comment 20 Joonghun Park 2014-12-04 04:15:56 PST
I'm using Ubuntu 14.04, and even after sudo apt-get update && sudo apt-get upgrade it's not changed.
In my case, pkg-config --modversion gnutls's conclusion is 2.12.23.
I guess there's some divergence between distros when installing libgnutls28.
Don't you think we need fixed version of gnutls for webkit efl and gtk?
Maybe it might be an option to use jhbuild dependency package module, but I'm not sure it's ok because I've heard that jhbuild is for layout test originally.
Comment 21 Philippe Normand 2014-12-04 04:25:18 PST
Even since 12.04 the libgnutls28-dev package provides version 3 of gnutls :

http://packages.ubuntu.com/precise/libgnutls28-dev

Make sure to have it installed...
Comment 22 Philippe Normand 2014-12-04 04:27:05 PST
You can debug this with pkg-config --debug modversion gnutls
Comment 23 Csaba Osztrogonác 2014-12-04 04:28:59 PST
On my Ubuntu 14.04:
$ pkg-config --modversion gnutls
3.2.11

But I remember I had some issues on a 14.04 Ubuntu where another 
libgnutls was installed previously. As far as I remember removing 
the older one solved the version clashing problem.
Comment 24 Joonghun Park 2014-12-04 05:23:47 PST
I think Csaba Osztrogonác's comment is right.
As long as the older gnutls deppkg is remained even after I installed libgnutls28-dev the version of gnutls was not changed, but after remove the old one installing libgnutls28-dev changed the gnutls version to 3.2.11 at last.
It seems ubuntu's insignificant bug.

Thank you for your helps, Philippe Normand and Csaba Osztrogonác. :)