1147940 - Remove the dom.webcrypto.enabled pref

Status: RESOLVED FIXED

(Core :: DOM: Security, defect)

Description

[Mark Banner (:standard8)]

Attachment: Attempt to enable webcrypto for chrome code

In Hello, we're working on a feature that requires encryption.

For our content side parts we're using WebCrypto. We need to do some work in privileged chrome code (javascript module) as well, but unfortunately it appears like WebCrypto isn't available there.

Is it possible to have it made available?

I started a patch based on what I saw elsewhere with webidl, and although Crypto seems to be a function, it doesn't seem possible to do anything with it. Am I missing something?

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

There is Window implements GlobalCrypto; which makes .crypto available in window object.

Perhaps you could make Crypto and SubtleCrypto to have ChromeConstructor and expose those interfaces
in Window and System context as your patch already does.
Then one could access a crypto object using new Crypto() in chrome.

Comment 2

[Mark Banner (:standard8)]

Attachment: Enable WebCrypto for use in chrome; Remove old dom.webcrypto.enabled pref.

Thanks for the pointer. Adding the chrome constructor worked great.

I've also removed the dom.webcrypto.enabled pref - I'm assuming that isn't really required now. Also the webidl doesn't allow Pref to be specified if the Exposed attribute is more than just "Window".

There's a try push here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=157f241608d7

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8583944 [details] [diff] [review]
Enable WebCrypto for use in chrome; Remove old dom.webcrypto.enabled pref.

ttaubert, ok to remove the pref?

Comment 4

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 8583944 [details] [diff] [review]
Enable WebCrypto for use in chrome; Remove old dom.webcrypto.enabled pref.

Richard as the owner of WebCrypto should decide here. I think he liked to keep the pref in the beginning in case we landed something that would leave millions vulnerable to some possible security flaw. IMO we don't really need this anymore.

Comment 5

[Richard Barnes [:rbarnes]]

Comment on attachment 8583944 [details] [diff] [review]
Enable WebCrypto for use in chrome; Remove old dom.webcrypto.enabled pref.

Review of attachment 8583944 [details] [diff] [review]:
-----------------------------------------------------------------

This looks fine to me, though I don't have enough WebIDL clue to know why you need the Constructor() method as opposed to a normal ctor + Init().

I'm fine getting rid of the pref.

You'll need review from a DOM peer for the changes to test_interfaces.html.  :bz has helped out on WebCrypto stuff before, so he's probably a good candidate.

Comment 6

[Mark Banner (:standard8)]

So earlier today, I discovered that crypto is available via Cu.importGlobalProperties - it was added in bug 1116269.

Does that mean we still want to land this bug, or should we just resolve this as invalid?

Comment 7

[Richard Barnes [:rbarnes]]

Maybe we should just rename it to "remove dom.crypto.enabled", and land that part?

Comment 8

[Tim Taubert [:ttaubert] (inactive)]

(In reply to Richard Barnes [:rbarnes] from comment #7)
> Maybe we should just rename it to "remove dom.crypto.enabled", and land that
> part?

+1

Comment 9

[Mark Banner (:standard8)]

Attachment: Remove the dom.webcrypto.enabled pref as it is no longer necessary.

Updated to just remove the old pref.

Comment 10

[Richard Barnes [:rbarnes]]

Comment on attachment 8589076 [details] [diff] [review]
Remove the dom.webcrypto.enabled pref as it is no longer necessary.

Review of attachment 8589076 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM, with one nit.

::: dom/crypto/test/test-array.js
@@ +175,5 @@
>  if (MOCHITEST) {
>    SimpleTest.waitForExplicitFinish();
>    window.addEventListener("load", function() {
>      SimpleTest.waitForFocus(function() {
> +      start();

Could this just be SimpleTest.waitForFocus(start) ?

Comment 11

[Mark Banner (:standard8)]

Landed with the test simplified:

https://hg.mozilla.org/integration/mozilla-inbound/rev/dde176616801

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/dde176616801