927901 - Test has failed: Permission denied to access property 'encrypt'

Status: VERIFIED FIXED

(Core :: Security, defect)

Description

[Alice0775 White]

Steps To Reproduce:
1. Open http://html5test.com/
2. Click "Try the next version of HTML5test - New tests, new scores, coming soon!" link in orange bar at the top of page

Actual Results:
An alart box pops up.
Test has failed: Permission denied to access property 'encrypt'

Expected Results:
Test results should be shown on the page

Regression window(m-c)
Good:
http://hg.mozilla.org/mozilla-central/rev/54434a926c5f
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:25.0) Gecko/20100101 Firefox/25.0 ID:20130805022018
Bad:
http://hg.mozilla.org/mozilla-central/rev/ad0ae007aa9e
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:25.0) Gecko/20100101 Firefox/25.0 ID:20130805022418
Pushlog[:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=54434a926c5f&tochange=ad0ae007aa9e

In local build
Last Good: 81fada98a394
First Bad: 8aafb671f1e9
Regressed by:
	8aafb671f1e9	Yoshi Huang — Bug 883741 - Part 1: WebCrypto: Move Crypto to WebIDL. r=bz

Comment 1

[Alice0775 White]

s/An alart box/An alert box/

Comment 2

[Alex Keybl [:akeybl]]

Yoshi, any idea of the criticality of this issue for users? Difficult to say from these STR. We're less than a week from release.

Comment 3

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

I will check this bug first.
But I am not sure the criticality here.

Comment 4

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Last Friday I did see the error message "Test has failed: Permission denied to access property 'encrypt'",
but I just updated my m-c, which is 
commit a86900b72cfe2ce13c126c38c02f0e4a19a9b771
Author: Matt Brubeck <[e-mail]>
Date:   Tue Oct 22 16:35:56 2013 -0700

    Back out 1ae5d58532ad (bug 928370) for breaking Tp5 measurement

and seems it's fixed now.

Hi  Alice0775 White:
Can you check this again?

Thanks

Comment 5

[Alice0775 White]

I can still reproduce the problem

http://hg.mozilla.org/mozilla-central/rev/21d97baadc05
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:27.0) Gecko/20100101 Firefox/27.0 ID:20131022195201

Comment 6

[José Jeria]

This issue is also tracked by html5test.com here: https://github.com/NielsLeenheer/html5test/issues/293

Comment 7

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Sorry, Alice0775, I tried again and it still exists.
Not sure what went wrong yesterday.
Sorry for the noise.

After some investigation, I found something,

- In beta.html5test.com
The erroe comes from testSecurity() in beta.html5test.com/engine.js
And the test is just 

var crypto = window.crypto || ...;
... 
!!crypto && 'encrypt' in crypto

At this time,
all property access to window.crypto are failed with the errorMsg
'Permission denied to access property' ...

It starts from 

http://mxr.mozilla.org/mozilla-central/source/js/src/jsproxy.h#407

The policy has been to true, so the handler->enter will go to 
http://mxr.mozilla.org/mozilla-central/source/js/xpconnect/wrappers/FilteringWrapper.cpp#164

The check in line 179 XrayUtils::IsXrayResolving will return false
http://mxr.mozilla.org/mozilla-central/source/js/xpconnect/wrappers/FilteringWrapper.cpp#179

Now will call the Policy::check in line 183,
http://mxr.mozilla.org/mozilla-central/source/js/xpconnect/wrappers/FilteringWrapper.cpp#183

The check is in 
http://mxr.mozilla.org/mozilla-central/source/js/xpconnect/wrappers/AccessCheck.h#75
which is a CrossOriginAccessiblePropertyOnly policy

Then the check will return false in 
http://mxr.mozilla.org/mozilla-central/source/js/xpconnect/wrappers/AccessCheck.cpp#257

I am not sure what went wrong exactly.

CCing smaug and khuey here for help (bz is still in PTO)

Hi smaug and khuey
Can you kindly help to give me some information or some hints here?

Thank you :)

Comment 8

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

So, erm, nsGlobalWindow::GetCrypto should forward to inner window, as far as I see.

Comment 9

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

/me tests

Comment 10

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

Attachment: patch

Comment 11

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/692a1de53310

Comment 12

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

Yoshi, we need this at least for Aurora, but Beta too?
It is not clear to me on which branches we have bug 883741. It says mozilla26, but
the regression range in this bug talks about Firefox/25.0.

Could you please ask approval on the right branches.

Comment 13

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

I think it's on 25 and philor just set the wrong milestone there ...

Comment 14

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

It's probably too late to get this into 25 but if we can take it as a ridealong in a respin we should.

Comment 15

[Phil Ringnalda (:philor)]

You would need to ask either hg, or mcMerge since it was the one setting milestones.

Comment 16

[Boris Zbarsky [:bzbarsky]]

Is this test calling bareword "crypto" on a window that's been navigated away from (to a different origin, at that) or something?  If not, why is the forwarding direction relevant?

Or is the issue that there's a stale window.crypto on the outer from some other (different origin) page and we're getting it?

Comment 17

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

(In reply to On vacation Oct 12 - Oct 27 from comment #16)
> Or is the issue that there's a stale window.crypto on the outer from some
> other (different origin) page and we're getting it?

The latter, AIUI.

Comment 18

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 821663 [details] [diff] [review]
patch

I'm going to nominate this for both Aurora and Beta approval.  If this request is processed after the 10/28 cutover this will already be on Aurora, so only the Beta approval request will be relevant.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 883741
User impact if declined: The window.crypto property may be unavailable to a web page if a previous web page sharing the same outer window (e.g. in the same tab) also accessed it and the two pages in question are cross-origin.  This could break web sites that use window.crypto.
Testing completed (on m-c, etc.): On m-c, very simple patch.
Risk to taking this patch (and alternatives if risky): This is a simple low risk patch.  Backing out the regressing bug would be far more risky IMO.
String or IDL/UUID changes made by this patch: None.

Comment 19

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

(In reply to Olli Pettay [:smaug] from comment #12)
> Yoshi, we need this at least for Aurora, but Beta too?
> It is not clear to me on which branches we have bug 883741. It says
> mozilla26, but
> the regression range in this bug talks about Firefox/25.0.
> 
> Could you please ask approval on the right branches.

Thanks for the patch, smaug.

From mcMerge, https://tbpl.mozilla.org/mcmerge/?cset=8aafb671f1e9, the MileStone is set as Firefox26.

But from the push log, it says FIREFOX_AURORA_25_BASE
https://hg.mozilla.org/mozilla-central/pushloghtml?changeset=ad0ae007aa9e

I think you said in Comment 9 means we need another test for this, I'll file a bug for this.

Also thanks for Khuey for asking for approval here.

Comment 20

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/692a1de53310

Comment 22

[Ekanan Ketunuti]

Verified on Nightly 27.0a1 buildid 20131025030239.

Comment 23

[(no longer active)]

Also, see bug 930827 before making the approval decision please.

Comment 24

[Johnny Stenback (:jst)]

Given that this is a regression in window.crypto, which is older than dirt (almost), and it's a regression in 25 I think we have take this. To add to the reasoning, per bug 930827 this is used by some google APIs, and those APIs could be used pretty much anywhere on the web (if not today, then tomorrow).

Kyle, could you look into writing a test for this one so we don't regress this again etc?

Comment 25

[Alex Keybl [:akeybl]]

Comment on attachment 821663 [details] [diff] [review]
patch

[Triage Comment]

Comment 26

[Doug Turner (:dougt)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/0a278f7b0b34
https://hg.mozilla.org/releases/mozilla-beta/rev/aa459ad5a1ad

Comment 27

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

I'll write a testcase.

Comment 28

[Doug Turner (:dougt)]

https://hg.mozilla.org/releases/mozilla-release/rev/9a3e312d260c

Comment 29

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

Attachment: test

rs=jst

Comment 30

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

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

Comment 31

[u279076]

I tested the following try builds (as indicated by Alex in r-d email) and they do not reproduce this issue, nor bug 930828:
* Beta: https://tbpl.mozilla.org/?tree=Mozilla-Beta&rev=aa459ad5a1ad
* Release: https://tbpl.mozilla.org/?tree=Mozilla-Release&rev=9a3e312d260c

I won't explicitly marked this verified fixed until I have a chance to verify the real Beta and RC builds but I thought I'd check the try builds before we went too far down the build process. Looks promising anyways.

Comment 32

[Ioana (away)]

Verified as fixed on:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:25.0) Gecko/20100101 Firefox/25.0 - 20131025150754 (beta 12)
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:25.0) Gecko/20100101 Firefox/25.0 - 20131025151332 (RC b#3)
Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:27.0) Gecko/20100101 Firefox/27.0 - 20131025100746

Still reproducible on:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:26.0) Gecko/20100101 Firefox/26.0 - 20131025004002

Did this fix make into aurora?

Comment 33

[Ioana (away)]

(In reply to Ioana Budnar, QA [:ioana] from comment #32)
> Verified as fixed on:
> Mozilla/5.0 (Windows NT 6.1; WOW64; rv:25.0) Gecko/20100101 Firefox/25.0 -
> 20131025150754 (beta 12)
> Mozilla/5.0 (Windows NT 6.1; WOW64; rv:25.0) Gecko/20100101 Firefox/25.0 -
> 20131025151332 (RC b#3)
> Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:27.0) Gecko/20100101
> Firefox/27.0 - 20131025100746
> 
> Still reproducible on:
> Mozilla/5.0 (Windows NT 6.1; WOW64; rv:26.0) Gecko/20100101 Firefox/26.0 -
> 20131025004002
> 
> Did this fix make into aurora?

Same results on Ubuntu 13.04 and Mac OS X 10.8.4.

Comment 34

[Ekanan Ketunuti]

Verified on Aurora 26.0a2 buildid 20131026004005.

Comment 35

[Phil Ringnalda (:philor)]

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

Comment 36

[Tony Chung [:tchung]]

Also verified fixed against Fennec 25 Beta build 3.

Comment 37

[Alex Keybl [:akeybl]]

this ended up on mozilla-release as well (see FF25 status flag)

https://hg.mozilla.org/releases/mozilla-release/rev/9a3e312d260c