Bug 138612 – [EFL] Add subtle crypto to the build system

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Éva Balázsfalvi]

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.

[Éva Balázsfalvi]

Created attachment 241352 [details]
Patch

[WebKit Commit Bot]

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.

[Gyuyoung Kim]

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/

[Gyuyoung Kim]

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 ?

[Éva Balázsfalvi]

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

[Éva Balázsfalvi]

Created attachment 241586 [details]
Patch

[WebKit Commit Bot]

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.

[Csaba Osztrogonác]

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

[Csaba Osztrogonác]

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

[Éva Balázsfalvi]

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.

[WebKit Commit Bot]

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.

[Csaba Osztrogonác]

any feedback from GTK side?

[Éva Balázsfalvi]

Ping?

[Philippe Normand]

Comment on attachment 241712 [details]
Patch

lgtm

[Csaba Osztrogonác]

Comment on attachment 241712 [details]
Patch

It is good for EFL and GTK, so final r+.

[WebKit Commit Bot]

Comment on attachment 241712 [details]
Patch

Clearing flags on attachment: 241712

Committed r176712: <http://trac.webkit.org/changeset/176712>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Joonghun Park]

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.

[Philippe Normand]

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

[Joonghun Park]

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.

[Philippe Normand]

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

[Philippe Normand]

You can debug this with pkg-config --debug modversion gnutls

[Csaba Osztrogonác]

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.

[Joonghun Park]

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