Closed Bug 1381645 Opened 7 years ago Closed 4 years ago

Restrict access to WebVR to HTTPS only sites.

Categories

(Core :: WebVR, task, P1)

task

Tracking

()

RESOLVED FIXED
mozilla73
Tracking Status
firefox57 --- wontfix
firefox58 --- wontfix
firefox73 --- fixed
firefox74 --- fixed
firefox75 --- fixed

People

(Reporter: kip, Assigned: kip)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-complete, site-compat)

Attachments

(5 files, 1 obsolete file)

It would be useful to have a preference that restricts WebVR usage to https-only sites.  Bug 1291405 is exploring the policy of enabling or restricting access to the WebVR API's outside of https sites.

Access to WebVR can be controlled by resolving the promise returned by Navigator.GetVRDisplays() and by suppressing the Navigator.onvrdisplayactivate event.

Perhaps such a preference could be used to identify the impact of the https-only polify on WebVR sites and later implement the policy if that decision is made.

If a site attempts to access the WebVR API without https,  console logging or another mechanism should be added to give context to the failure, and reduce confusion by the end users and developers.
Priority: -- → P1
Too late for 57, but we could still potentially take a patch in 58. Are you aiming this at 59 or 58?
Flags: needinfo?(kgilbert)
It will be much easier to just use the existing [SecureContext] tag in the WebIDL to implement this.  A side effect will be that there will be no WebVR specific pref to override this independently of the other SecureContext restricted APIs.

We will land this in FF58 and rely on the globally implemented mechanism rather than something WebVR specific.
Flags: needinfo?(kgilbert)
Summary: Add preference to enable users to restrict access to WebVR to HTTPS only sites. → Restrict access to WebVR to HTTPS only sites.
Assignee: nobody → kgilbert
Attachment #8931176 - Flags: review?(dmu)
An "Intent to remove: WebVR on insecure contexts" was posted on dev-platform:

https://groups.google.com/forum/#!topic/mozilla.dev.platform/bU2gil1SHkY
A test page confirms that WebVR is disabled on insecure contexts:

http://webvr.info/samples/insecure/test-insecure.html

A message stating that WebVR is not supported will appear on that page if viewed with this patch applied.
Comment on attachment 8931176 [details]
Bug 1381645 - Restrict access to WebVR to HTTPS only sites

https://reviewboard.mozilla.org/r/202256/#review207652

LGTM. Please let a dom peer for reviewing the webidl. Thanks!
Attachment #8931176 - Flags: review?(dmu) → review+
Looks fine to me.

:bz are you able to review this?
Flags: needinfo?(bzbarsky)
Attachment #8931176 - Flags: review?(bugs)
Comment on attachment 8931176 [details]
Bug 1381645 - Restrict access to WebVR to HTTPS only sites

https://reviewboard.mozilla.org/r/202256/#review211200

There are no tests for this?
Attachment #8931176 - Flags: review?(bugs) → review+
This was tested manually to ensure that the [SecureContext] webidl flag is working as expected.

A couple of pages that you can verify the effect:

https://webvr.info/samples/03-vr-presentation.html
http://webvr.info/samples/insecure/test-insecure.html

Before the patch, both pages allow access to the headset tracking and enable the user to enter the WebVR scene.

After the patch, the "insecure" version of the page displays a message stating that WebVR is not available.  The page is unable to enumerate devices, perform tracking, or enter the WebVR scene.
Pushed by kgilbert@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8c5d173f19b4
Restrict access to WebVR to HTTPS only sites r=daoshengmu,smaug
(In reply to Noemi Erli[:noemi_erli] from comment #13)
> Backed out changeset 8c5d173f19b4 (bug 1381645) for mochitest test failures
> e.g. dom/vr/test/mochitest/test_vrDisplay_exitPresent.html r=backout on a
> CLOSED TREE
> 
> https://treeherder.mozilla.org/logviewer.
> html#?job_id=150648166&repo=autoland&lineNumber=1811
> 
> https://hg.mozilla.org/integration/autoland/rev/
> d2dc16317572d3fc6fd7c41e0da8b533eb5e18bf
> 
> 
> https://treeherder.mozilla.org/#/
> jobs?repo=autoland&revision=8c5d173f19b41645a88cf15be539cceaf913c28b&filter-
> resultStatus=testfailed&filter-resultStatus=busted&filter-
> resultStatus=exception&filter-resultStatus=retry&filter-
> resultStatus=usercancel&filter-resultStatus=runnable&filter-
> resultStatus=success

I'll update these tests and attempt to re-land after all-hands.
Flags: needinfo?(kgilbert)
Attachment #8937075 - Flags: review?(dmu)
Comment on attachment 8937075 [details]
Bug 1381645 - Part 2: Move VR tests to secure context

https://reviewboard.mozilla.org/r/207780/#review213672

LGTM. thanks
Attachment #8937075 - Flags: review?(dmu) → review+
Sorry for the lag here; looks like smaug got this.  I don't know how it fell through the cracks.  :(
Flags: needinfo?(bzbarsky)
Can this land, looks like it has the correct approval now :)
Flags: needinfo?(kgilbert)
(In reply to Jonathan Kingston [:jkt] from comment #19)
> Can this land, looks like it has the correct approval now :)

There were still some test failures in the last try push.  I'm testing a "part 3" patch to address those and aim to land very shortly.
Flags: needinfo?(kgilbert)
I think I've fixed the last failures in the "part 3" patch.  If the try push comes out clean, I'll assign the "part 3" patch to a DOM peer and then ready to land.
Try push shows that Web-Platform-Tests will also need updating.  I'll fix this and re-run the try...
I have submitted a PR to the web-platform-tests to try and run the failing tests under https:

https://github.com/w3c/web-platform-tests/pull/9173
The last try push shows that the failing web-platform-tests were indeed due to the interfaces hidden on insecure origins.

Perhaps we can update the expectation data for now to land the part 1-3 patches and correct our wpt meta .ini file and expectation data again once the wpt fix gets landed upstream.
Attachment #8941665 - Flags: review?(bzbarsky)
Attachment #8944957 - Attachment is obsolete: true
Comment on attachment 8941665 [details]
Bug 1381645 - Part 3: Update test_interfaces.js to expect WebVR interfaces to be enabled only in secure contexts

https://reviewboard.mozilla.org/r/211904/#review221076
Attachment #8941665 - Flags: review?(bzbarsky) → review+
Attachment #8945229 - Flags: review?(james)
Comment on attachment 8945229 [details]
Bug 1381645 - Part 4: Update web-platform tests expectation data for WebVR

https://reviewboard.mozilla.org/r/215452/#review221654

Sure, but in case you're unaware it's totally acceptable to just change the wpt tests in mozilla-central and the change will be automatically upstreamed to the GH repository.
Attachment #8945229 - Flags: review?(james) → review+
Pushed by kgilbert@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2f71c8989cc0
Restrict access to WebVR to HTTPS only sites r=daoshengmu,smaug
https://hg.mozilla.org/integration/autoland/rev/7c022a5e28f2
Part 2: Move VR tests to secure context r=daoshengmu
https://hg.mozilla.org/integration/autoland/rev/ef3252b0ba21
Part 3: Update test_interfaces.js to expect WebVR interfaces to be enabled only in secure contexts r=bz
https://hg.mozilla.org/integration/autoland/rev/7b2482a5eb04
Part 4: Update web-platform tests expectation data for WebVR r=jgraham
(In reply to Noemi Erli[:noemi_erli] from comment #42)
> Backed out 4 changesets (bug 1381645) for failing in
> dom/tests/mochitest/general/test_interfaces.html on a CLOSED TREE
> 
> Push with failures:
> https://treeherder.mozilla.org/#/
> jobs?repo=autoland&revision=7b2482a5eb049a0e79824daf206c0e36a31305b0
> 
> Failure log:
> https://treeherder.mozilla.org/logviewer.
> html#?job_id=158734592&repo=autoland&lineNumber=23219
> 
> Backout:
> https://treeherder.mozilla.org/#/
> jobs?repo=autoland&revision=4bd6d0d97a7c63c3f3eae27f2baf4a914d0cb50d

Sorry to have to re-open this.

It appears that I also need to make VRDisplayEvent a SecureContext-only interface in order to satisfy test_interfaces.js.

re-submitted a try run with all tests to be sure I didn't miss anything else.
Flags: needinfo?(kgilbert)
Target Milestone: --- → mozilla60
I'd like to land Bug 1438044 first, then update the expectation data for these tests, and re-land this.

It would be nice to land this in FF60.
Bug 1438044 has been re-landed, so this can proceed once again.

This has already been fixed. WebVR now requires a secure context.

Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED

Posted site compatibility note for web developers.

Type: defect → task
Depends on: 1580567
Target Milestone: mozilla60 → mozilla73

Documentation updated:

  • Updated the main WebVR API page with a new section on availability of the API.
  • Added a note about this change to the APIs section of Firefox 73 for developers.
  • Submitted BCD PR 5838 to update data.
You need to log in before you can comment on or make changes to this bug.