Bug 133122 – [META] [GTK] Implement WebCrypto SubtleCrypto interface

• Status: NEW
• Component: WebKitGTK

[Eduardo Lima Mitev]

http://www.w3.org/TR/2014/WD-WebCryptoAPI-20140325/

Cross platform code already exists and the Mac port has platform-specific implementation for several algorithms (bug 122679).

This bug is to track implementation for the GTK port.

[Eduardo Lima Mitev]

I have a WIP implementation at:

https://github.com/Igalia/webkit/tree/subtle-crypto

which I plan to submit for review in the coming days.

[Eduardo Lima Mitev]

Created attachment 232126 [details]
Patch

[Eduardo Lima Mitev]

Created attachment 232128 [details]
Patch

[WebKit Commit Bot]

Attachment 232126 [details] did not pass style-queue:


ERROR: Source/WebCore/CMakeLists.txt:30:  There should be exactly one empty line instead of 0 between "inspector" and "loader/appcache".  [list/emptyline] [5]
Total errors found: 1 in 17 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Eduardo Lima Mitev]

(In reply to comment #4)
> Attachment 232126 [details] did not pass style-queue:
> 
> 
> ERROR: Source/WebCore/CMakeLists.txt:30:  There should be exactly one empty line instead of 0 between "inspector" and "loader/appcache".  [list/emptyline] [5]
> Total errors found: 1 in 17 files
> 

I think this is a false positive. The patch has nothing to do with the error mentioned. The occurrence of "inspector" and "loader/appcache" happens at lines 545 and 547 (not 30), and there is an empty line between them.

[Eduardo Lima Mitev]

Created attachment 232132 [details]
Patch

[Eduardo Lima Mitev]

Comment on attachment 232128 [details]
Patch

this is now obsolete, lacks ChangeLog updates

[Eduardo Lima Mitev]

Created attachment 232135 [details]
Patch

[Eduardo Lima Mitev]

Created attachment 232138 [details]
Patch

[Eduardo Lima Mitev]

To make these changes easier to track and review (and for compliance and sanity), I will make this bug a metabug, cancel all uploaded patches, and create one new bug for each patch. This is a similar approach as in bug 122679. 

I'm really sorry for the noise and the mess.

[Eduardo Lima Mitev]

Adding dependency on bug 133316.

[Eduardo Lima Mitev]

Adding dependency on bug 133317.

[Eduardo Lima Mitev]

Adding dependency on bug 133319.

[Eduardo Lima Mitev]

Adding dependency on bug 133320.

[Eduardo Lima Mitev]

Adding dependency on bug 133344.

[Eduardo Lima Mitev]

Adding Carlos García Campos to CC.

[Michael Catanzaro]

Note that r206883 "Add a dummy SubtleCrypto interface" broke the tests under crypto/workers/subtle:

crypto/workers/subtle/aes-postMessage-worker.html
crypto/workers/subtle/hmac-postMessage-worker.html
crypto/workers/subtle/hrsa-postMessage-worker.html
crypto/workers/subtle/multiple-postMessage-worker.html
crypto/workers/subtle/rsa-postMessage-worker.html

I have skipped this entire test directory.

[Zan Dobersek]

Created attachment 305635 [details]
WIP Patch

This patch implements all the symmetric cipher and public key algorithms used in the Web Crypto standard that are currently supported and tested in WebKit, not including the webkit-prefixed interfaces. The implementations leverage libgcrypt and are in majority complete, but there's still a lot of opportunities to polish and improve the existing code.

All but two relevant tests currently pass, and the two failures that are marked in the TestExpectations file fail because more sub-tests run successfully than the text baseline expects.

This patch contains the whole changeset, but is of course inconvenient to review. I'll be splitting it up into smaller patches instead. I'll try to update this patch regularly to present the set of changes that still has to be landed.

[Jiewen Tan]

(In reply to Zan Dobersek from comment #18)
> Created attachment 305635 [details]
> WIP Patch
> 
> This patch implements all the symmetric cipher and public key algorithms
> used in the Web Crypto standard that are currently supported and tested in
> WebKit, not including the webkit-prefixed interfaces. The implementations
> leverage libgcrypt and are in majority complete, but there's still a lot of
> opportunities to polish and improve the existing code.
> 
> All but two relevant tests currently pass, and the two failures that are
> marked in the TestExpectations file fail because more sub-tests run
> successfully than the text baseline expects.
> 
> This patch contains the whole changeset, but is of course inconvenient to
> review. I'll be splitting it up into smaller patches instead. I'll try to
> update this patch regularly to present the set of changes that still has to
> be landed.

Please split it. Thanks! It will be good to split it according to specific algorithms.

[Michael Catanzaro]

WTF, you did all this over the weekend...? Wow! 

Why are the gcrypt headers missing license notices?

Can you delete FindGnutls.cmake and references to it?

[Carlos Alberto Lopez Perez]

(In reply to Michael Catanzaro from comment #20)
> WTF, you did all this over the weekend...? Wow! 
> 

I'm also shocked. This is completely amazing. \o/

> Why are the gcrypt headers missing license notices?
> 
> Can you delete FindGnutls.cmake and references to it?

Some quick comments:

I tried the patch and I'm getting several timeouts on the tests.
Also this test page https://vibornoff.github.io/webcrypto-examples/index.html that I use as reference for this WebCrypto stuff loads very slow (takes several minutes to appear some of the content, and the page doesn't finish to load). Does it work for you properly?

[Eduardo Lima Mitev]

(In reply to Zan Dobersek from comment #18)
> The implementations leverage libgcrypt and are in majority complete,
> but there's still a lot of opportunities to polish and improve the
> existing code.
> 

Did you considered using libnettle instead of gcrypt? This is the first question I faced when I started implementing the first algorithms, and libnettle was a more or less clear choice:

* gnutls moved away from gcrypt in favor of libnettle,

* libnettle API is certainly easier to work with (standard C API as opposed working with to S-expressions),

* libnettle is arguably faster, though I don't have data to back that up. libnettle includes CPU-native implementations for some algos in some archs.

* libnettle is (again arguably) better maintained, hence more future proof.

But I have not worked on this for a long time, so maybe this question was already answered before, or there are other arguments in favor of gcrypt that I'm not aware of (e.g, license issues). If that's the case, sorry for the noise.

I'm so looking forward to see good support of WebCrypto on the WebkitGTK shipped by distros. And speaking about that, Martin Robinson once suggested that we should probably audit this code before un-flagging the feature. Any plan/idea about that?

Kudos for the awesome work!

[Zan Dobersek]

(In reply to Michael Catanzaro from comment #20)
> Why are the gcrypt headers missing license notices?
> 

Forgot.

> Can you delete FindGnutls.cmake and references to it?

Yes!

[Zan Dobersek]

(In reply to Carlos Alberto Lopez Perez from comment #21)
> Some quick comments:
> 
> I tried the patch and I'm getting several timeouts on the tests.

Forgot to mention, the implementation depends on the 8-bit cihper feedback mode, represented by the GCRY_CIPHER_MODE_CFB8 constant. That was implemented in February, after the latest 1.7.6 release:
https://git.gnupg.org/cgi-bin/gitweb.cgi?p=libgcrypt.git;a=commit;h=d1ee9a660571ce4a998c9ab2299d4f2419f99127

[Zan Dobersek]

(In reply to Eduardo Lima Mitev from comment #22)
> (In reply to Zan Dobersek from comment #18)
> > The implementations leverage libgcrypt and are in majority complete,
> > but there's still a lot of opportunities to polish and improve the
> > existing code.
> > 
> 
> Did you considered using libnettle instead of gcrypt? This is the first
> question I faced when I started implementing the first algorithms, and
> libnettle was a more or less clear choice:
> 
> * gnutls moved away from gcrypt in favor of libnettle,
> 
> * libnettle API is certainly easier to work with (standard C API as opposed
> working with to S-expressions),
> 
> * libnettle is arguably faster, though I don't have data to back that up.
> libnettle includes CPU-native implementations for some algos in some archs.
> 
> * libnettle is (again arguably) better maintained, hence more future proof.
> 
> But I have not worked on this for a long time, so maybe this question was
> already answered before, or there are other arguments in favor of gcrypt
> that I'm not aware of (e.g, license issues). If that's the case, sorry for
> the noise.
> 
> I'm so looking forward to see good support of WebCrypto on the WebkitGTK
> shipped by distros. And speaking about that, Martin Robinson once suggested
> that we should probably audit this code before un-flagging the feature. Any
> plan/idea about that?
> 
> Kudos for the awesome work!

I don't remember the specifics or conclusions from any previous discussions, but I didn't consider libnettle. libgcrypt is already being used for the digest algorithms by default, and it offers coverage of all the ciphers and modes that are required in the W3C spec.

I looked at libnettle briefly now, and there appear to be missing cipher modes that are needed to implement the AES-CFB-8 and AES-KW support. (Although the AES-CFB-8 cipher was removed from the spec, so the CFB8 might not be required in the future.)

But in general, I'm OK with using libnettle if there aren't any potential licensing issues and if we're able to fill in the feature gap.

[Zan Dobersek]

(In reply to Carlos Alberto Lopez Perez from comment #21)
> Also this test page
> https://vibornoff.github.io/webcrypto-examples/index.html that I use as
> reference for this WebCrypto stuff loads very slow (takes several minutes to
> appear some of the content, and the page doesn't finish to load). Does it
> work for you properly?

It loads fine for me, using the libgcrypt 1.7.6 release or the more recent gcrypt build that includes the CFB8 support.

[Michael Catanzaro]

(In reply to Zan Dobersek from comment #25)
> But in general, I'm OK with using libnettle if there aren't any potential
> licensing issues and if we're able to fill in the feature gap.

We added libgcrypt support to allow avoiding libnettle (GPLv2+ or LGPLv3+). It's not any problem for Linux distros, but it will be a problem for customers. We should prefer libgcrypt.

(In reply to Zan Dobersek from comment #24)
> (In reply to Carlos Alberto Lopez Perez from comment #21)
> > Some quick comments:
> > 
> > I tried the patch and I'm getting several timeouts on the tests.
> 
> Forgot to mention, the implementation depends on the 8-bit cihper feedback
> mode, represented by the GCRY_CIPHER_MODE_CFB8 constant. That was
> implemented in February, after the latest 1.7.6 release:
> https://git.gnupg.org/cgi-bin/gitweb.cgi?p=libgcrypt.git;a=commit;
> h=d1ee9a660571ce4a998c9ab2299d4f2419f99127

Then we need to update OptionsGTK.cmake accordingly:

find_package(LibGcrypt 1.6.0 REQUIRED)

[Michael Catanzaro]

(In reply to Eduardo Lima Mitev from comment #22)
> I'm so looking forward to see good support of WebCrypto on the WebkitGTK
> shipped by distros. And speaking about that, Martin Robinson once suggested
> that we should probably audit this code before un-flagging the feature. Any
> plan/idea about that?

We absolutely should. Cost would be high, so I suspect it's not going to happen.

[Zan Dobersek]

Created attachment 305835 [details]
WIP patch

[Zan Dobersek]

Created attachment 306484 [details]
WIP patch

[Zan Dobersek]

Created attachment 309512 [details]
WIP Patch

Remaining changes based on top of changes in #171219, #171220, #171222, #171420.

[Zan Dobersek]

Created attachment 312638 [details]
WIP patch

Now with more libtasn1.

[Zan Dobersek]

Created attachment 314466 [details]
Remaining changes

Remaining changes that are spread across the blocker bugs.