Closed
Bug 1247124
Opened 9 years ago
Closed 7 years ago
Use [SecureContext] for FIDO U2F JS API
Categories
(Core :: DOM: Device Interfaces, defect, P3)
Core
DOM: Device Interfaces
Tracking
()
RESOLVED
FIXED
mozilla59
Tracking | Status | |
---|---|---|
firefox59 | --- | fixed |
People
(Reporter: jcj, Assigned: jcj)
References
Details
Attachments
(1 file)
The U2F API should be protected by the WebIDL annotation [SecureContext] from Bug 1177957.
Assignee | ||
Updated•9 years ago
|
Updated•9 years ago
|
Component: DOM: Security → DOM: Device Interfaces
Assignee | ||
Comment 1•8 years ago
|
||
Use the [SecureContext] webidl notation to hide the powerful "window.u2f" feature when not loaded in a secure context. Review commit: https://reviewboard.mozilla.org/r/55052/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/55052/
Assignee | ||
Comment 2•8 years ago
|
||
The trivial patch for this is insufficient, as setting the [SecureContext] WebIDL feature breaks the standard mechanism for executing secure Mochitests. The core method, isSecureContext(), marks as insecure any page loaded from a context that originated from an insecure parent. Marking blocked on Bug 1274315.
Depends on: 1274315
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → jjones
Status: NEW → ASSIGNED
Assignee | ||
Comment 3•8 years ago
|
||
It looks like this is doable by setting the pref `"dom.securecontext.whitelist"` to cover the mochi.test:8888 host rather than waiting on Bug 1274315.
Updated•7 years ago
|
Priority: -- → P3
Comment 4•7 years ago
|
||
It looks like this has tests running under https now, except for the unsecured appid facet test. Is it worth adding SecureContext on this yet, or are we just gonna deprecate this anyways since WebAuthn shipped?
Flags: needinfo?(jjones)
Comment hidden (mozreview-request) |
Comment 6•7 years ago
|
||
mozreview-review |
Comment on attachment 8756254 [details] Bug 1247124 - Limit FIDO U2F to Secure Contexts https://reviewboard.mozilla.org/r/55052/#review213440 Cool.
Attachment #8756254 -
Flags: review?(ttaubert) → review+
Assignee | ||
Updated•7 years ago
|
Attachment #8756254 -
Flags: review?(bugs)
Assignee | ||
Comment 7•7 years ago
|
||
You're right, we have it basically ready now Kyle with the test refactoring, and with a minor tweak it works fine. Smaug - There's no WebIDL for this, but it's a powerful feature.
Flags: needinfo?(jjones)
Comment 8•7 years ago
|
||
mozreview-review |
Comment on attachment 8756254 [details] Bug 1247124 - Limit FIDO U2F to Secure Contexts https://reviewboard.mozilla.org/r/55052/#review213520 ::: dom/webidl/U2F.webidl:14 (Diff revision 2) > * https://www.fidoalliance.org/specs/fido-u2f-v1.1-id-20160915/fido-u2f-javascript-api-v1.1-id-20160915.html > */ > > [NoInterfaceObject] > interface GlobalU2F { > - [Throws, Pref="security.webauth.u2f"] > + [SecureContext, Throws, Pref="security.webauth.u2f"] Why is this enough?.. ::: dom/webidl/U2F.webidl:71 (Diff revision 2) > > callback U2FRegisterCallback = void(RegisterResponse response); > callback U2FSignCallback = void(SignResponse response); > > [Pref="security.webauth.u2f"] > interface U2F { ...shouldn't also this and possibly some other related interfaces, if there are such, be limited to SecureContext
Attachment #8756254 -
Flags: review?(bugs) → review-
Comment 9•7 years ago
|
||
I guess there should be a test that '"U2F" in window' is false in non-SecureContext. (I'm not sure which spec to look at here)
Assignee | ||
Comment 10•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8756254 [details] Bug 1247124 - Limit FIDO U2F to Secure Contexts https://reviewboard.mozilla.org/r/55052/#review213520 Re: the test, `dom/u2f/tests/test_appid_facet_insecure.html` does check that accessing `window.u2f` from insecure context throws, so I think that's covered. Thanks, Olli! > Why is this enough?.. Tim and I took a look at WebCrypto's SecureContext patch-in-progress and it looks like we should also do this for all Interfaces, which is just the one. Added; thanks!
Comment 11•7 years ago
|
||
(In reply to J.C. Jones [:jcj] from comment #10) > Comment on attachment 8756254 [details] > Bug 1247124 - Limit FIDO U2F to Secure Contexts > > https://reviewboard.mozilla.org/r/55052/#review213520 > > Re: the test, `dom/u2f/tests/test_appid_facet_insecure.html` does check that > accessing `window.u2f` from insecure context throws, so I think that's > covered. Testing that window.u2f throws (why should it throw?) is different from testing whether there is interface object for U2F in the scope.
Comment hidden (mozreview-request) |
Comment 13•7 years ago
|
||
mozreview-review |
Comment on attachment 8756254 [details] Bug 1247124 - Limit FIDO U2F to Secure Contexts https://reviewboard.mozilla.org/r/55052/#review213574 Of course I wish there was a spec for this stuff. ::: dom/u2f/tests/frame_appid_facet_insecure.html:20 (Diff revision 3) > var version = "U2F_V2"; > var challenge = new Uint8Array(16); > > local_is(window.location.origin, "http://test2.example.com", "Is loaded correctly"); > > - await promiseU2FRegister(null, [{ > + local_is('u2f' in window, false, "window.u2f must be undefined when accessed from an insecure origin"); FWIW, also U2F should be false. u2f refers to the getter in window object, U2F to the interface object.
Attachment #8756254 -
Flags: review?(bugs) → review+
Comment hidden (mozreview-request) |
Comment 15•7 years ago
|
||
Pushed by ttaubert@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/4c3feee4dfd2 Limit FIDO U2F to Secure Contexts r=ttaubert,smaug
Comment 16•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/4c3feee4dfd2
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox59:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
You need to log in
before you can comment on or make changes to this bug.
Description
•