Summary:
Ensure that transactions are cleared before U2FCallbacks are called, to allow
reentrancy from microtask checkpoints.
Move the two possible callbacks into U2FTransaction so we have nicer invariants
and know that there's a callback as long as we have a transaction.
Reviewers: jcj
Reviewed By: jcj
Bug #: 1422661
Differential Revision: https://phabricator.services.mozilla.com/D329
--HG--
extra : amend_source : 7097f38199a5bc4a215377e4f1a64079cf6d6a24
Summary: We can probably abstract more stuff in the future, but this seems like a good start.
Reviewers: jcj
Reviewed By: jcj
Bug #: 1396907
Differential Revision: https://phabricator.services.mozilla.com/D323
Summary:
We currently have a single WebAuthnManager instance per process that's shared
between all CredentialContainers. That way the nsPIDOMWindowInner parent has
to be tracked by the transaction, as multiple containers could kick off
requests simultaneously.
This patch lets us we have one WebAuthnManager instance per each
CredentialsContainer and thus each nsPIDOMWindowInner. This matches the current
U2F implementation where there is one instance per parent window too.
This somewhat simplifies the communication diagram (at least in my head), as
each U2F/WebAuthnManager instance also has their own TransactionChild/Parent
pair for IPC protocol communication. The manager and child/parent pair are
destroyed when the window is.
Reviewers: jcj
Reviewed By: jcj
Bug #: 1421616
Differential Revision: https://phabricator.services.mozilla.com/D305
Summary:
We currently have a single WebAuthnManager instance per process that's shared
between all CredentialContainers. That way the nsPIDOMWindowInner parent has
to be tracked by the transaction, as multiple containers could kick off
requests simultaneously.
This patch lets us we have one WebAuthnManager instance per each
CredentialsContainer and thus each nsPIDOMWindowInner. This matches the current
U2F implementation where there is one instance per parent window too.
This somewhat simplifies the communication diagram (at least in my head), as
each U2F/WebAuthnManager instance also has their own TransactionChild/Parent
pair for IPC protocol communication. The manager and child/parent pair are
destroyed when the window is.
Reviewers: jcj
Reviewed By: jcj
Bug #: 1421616
Differential Revision: https://phabricator.services.mozilla.com/D305
This was automatically generated by the script modeline.py.
MozReview-Commit-ID: BgulzkGteAL
--HG--
extra : rebase_source : a4b9d16a4c06c4e85d7d85f485221b1e4ebdfede
In Comment 8 of Bug 1244959 [1], Brad Hill argues that instead of leaving our
U2F Facet support completely half-way, that we could use the Public Suffix logic
introduced into HTML for W3C Web Authentication (the method named
IsRegistrableDomainSuffixOfOrEqualTo) to scope the FIDO AppID to an eTLD+1
hierarchy. This is a deviation from the FIDO specification, but doesn't break
anything that currently works with our U2F implementation, and theoretically
enables sites that otherwise need an external FacetID fetch which we aren't
implementing.
The downside to this is that it's then Firefox-specific behavior. But since this
isn't a shipped feature, we have more room to experiment. As an additional
bonus, it encourages U2F sites to use the upcoming Web Authentication security
model, which will help them prepare to adopt the newer standard.
[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1244959#c8
MozReview-Commit-ID: DzNVhHT9qRL
--HG--
extra : rebase_source : 262e2ddbec325e0391d346473f27ae2738490da1
I wasn't sure what the right behavior for the U2F API is when `.sign()`
or `.register()` is called but there's an ongoing request that wasn't fulfilled
yet.
I think it makes sense to deny the request (as we currently do) when a request
of the same type is currently active. When however sign() -> register() or
vice-versa is called then we should cancel the previous request and start
the new one. From what I understand from reading the spec we definitely should
call the callback before starting the new request.
Bug #: 1401019
Differential Revision: https://phabricator.services.mozilla.com/D70
- This patch reworks the U2F module to asynchronously call U2FManager,
which in turn handles constructing and managing the U2FTokenManager
via IPC.
- Add U2FTransaction{Parent,Child} implementations to mirror similar ones for
WebAuthn
- Rewrite all tests to compensate for U2F executing asynchronously now.
- Used async tasks, used the manifest parameters for scheme, and generally
made these cleaner.
- The mochitest "pref =" functionality from Bug 1328830 doesn't support Android
yet, causing breakage on Android. Rework the tests to go back to the old way
of using iframes to test U2F.
NOTE TO REVIEWERS:
Since this is huge, I recommend the following:
keeler - please review U2F.cpp/h, the tests, and the security-prefs.js. Most
of the U2F logic is still in U2F.cpp like before, but there's been
some reworking of how it is called.
ttaubert - please review U2FManager, the Transaction classes, build changes,
and the changes to nsGlobalWindow. All of these should be very
similar to the WebAuthn code it's patterned off.
MozReview-Commit-ID: C1ZN2ch66Rm
--HG--
extra : rebase_source : 5a2c52b0340c13f471af5040b998eb7e661b1981
- This patch reworks the U2F module to asynchronously call U2FManager,
which in turn handles constructing and managing the U2FTokenManager
via IPC.
- Add U2FTransaction{Parent,Child} implementations to mirror similar ones for
WebAuthn
- Rewrite all tests to compensate for U2F executing asynchronously now.
- Used async tasks, used the manifest parameters for prefs and scheme,
and generally made these cleaner.
NOTE TO REVIEWERS:
Since this is huge, I recommend the following:
keeler - please review U2F.cpp/h, the tests, and the security-prefs.js. Most
of the U2F logic is still in U2F.cpp like before, but there's been
some reworking of how it is called.
ttaubert - please review U2FManager, the Transaction classes, build changes,
and the changes to nsGlobalWindow. All of these should be very
similar to the WebAuthn code it's patterned off.
MozReview-Commit-ID: C1ZN2ch66Rm
--HG--
extra : transplant_source : %EA%98%D2%87C%FD%CC%A5%3D%B5%9B%1C%DA%A5J%CD%05%94%13%0D
This change includes the FIDO "App ID" as part of the function used to generate
the wrapping key used in the NSS-based U2F soft token, cryptographically binding
the "Key Handle" to the site that Key Handle is intended for.
This is a breaking change with existing registered U2F keys, but since our soft
token is hidden behind a pref, it does not attempt to be backward-compatible.
- Updated for rbarnes' and qdot's reviews comments. Thanks!
- Made more strict in size restrictions, and added a version field
to help us be this strict.
- Bugfix for an early unprotected buffer use (Thanks again rbarnes!)
- Fix a sneaky memory leak re: CryptoBuffer.ToSECItem
MozReview-Commit-ID: Jf6gNPauT4Y
--HG--
extra : rebase_source : 4ff5898e93e4a0a75576e5e54035a1cb6dd952d7
This change includes the FIDO "App ID" as part of the function used to generate
the wrapping key used in the NSS-based U2F soft token, cryptographically binding
the "Key Handle" to the site that Key Handle is intended for.
This is a breaking change with existing registered U2F keys, but since our soft
token is hidden behind a pref, it does not attempt to be backward-compatible.
- Updated for rbarnes' and qdot's reviews comments. Thanks!
- Made more strict in size restrictions, and added a version field
to help us be this strict.
- Bugfix for an early unprotected buffer use (Thanks again rbarnes!)
MozReview-Commit-ID: Jf6gNPauT4Y
--HG--
extra : rebase_source : 52d10287d10698292e1480e04f580f6f8b4847cb
Add a test that U2F supports signing with multiple keys.
Reorder the WaitGroupDone calls to ensure they always fire, even if there
are multiple actions in flight.
Also, change the status lanbda captures to capture by reference, and disable
the copy constructor that would let the by-value work. Interestingly, the
compiler is choosing by-reference by default -- at least logs show that the
behavior is correct without this change, but still - this is the right thing to
do.
Updated: Fix the unit tests and write a README that explains why they have to
use an iframe, while WebAuthn tests do not.
MozReview-Commit-ID: AqSyxU5N4yu
--HG--
extra : rebase_source : b8f18973891ba63ac364203dece65a0689f46ee5
This patch implements the W3C Web Authentication API from
https://www.w3.org/TR/webauthn/, currently the 28 September 2016
working draft.
It utilizes a tentative binding of the U2F NSS Soft Token to provide
authentication services while waiting on Bug 1245527 to support USB HID-based
U2F tokens. This binding is not in the specification yet, so it should be
considered an experiment to help the specification move fowrard.
There are also a handful of deviations from the specification's WebIDL, which
are annotated with comments in WebAuthentication.webidl.
There are no tests in this commit; they are in Part 4 of this commit series.
There is a small script online at https://webauthn.bin.coffee/ to exercise this
code, but it doesn't do any automated checks.
There are also a handful of TODOS:
1) The algorithm to relax the same-origin restriction is in Part 3.
2) The use of AlgorithmIdentifier and having a way to coerce an object to a
string is still missing.
3) Timeouts and deadlines aren't there, and are pending reworking how
the nsIU2FToken interface works.
UPDATED:
- Address qdot, keeler review comments (thanks!)
- Address more qdot, keeler review comments (thanks!)
MozReview-Commit-ID: JITapI38iOh
--HG--
extra : rebase_source : 9a09e852dd0c8dc47f42dabbcf8b845a6828b225
This patch sets up the U2F system to support multiple nsIU2FToken
"authenticators" simultaneously, such as having both a USB and a Bluetooth Smart
implementation enabled at the same time. It also paves the way to support
timeout interruptions (for Bug 1301793).
- Executes operations across a list of authenticators.
- Uses runnables, via MozPromise and SharedThreadPool.
- Remove nsNSSShutDownPreventionLock from U2F*Task and move to U2F*Runnable
- Review updates
- Some of the review updates from earlier changeset are ... painful to merge
back before this one, so I'm just tacking them on here.
It's still missing some things, though:
- It's not actually executing the operations in parallel yet, as invoking
methods on NSSU2FTokenRemote from a worker thread throws exceptions while
obtaining ContentChild::GetSingleton().
MozReview-Commit-ID: EUdZQesASo2
***
Bug 1297552 - Updates per review r?keeler
MozReview-Commit-ID: EHIWM74tfYG
--HG--
extra : transplant_source : %F9%9E%9E%5B7%19R0%7D%C1%B1%FB%BD%97%26%B2%A3%9CTg
- Breaks compatibility with non-e10s windows, as the underlying USB
implementation from Bug 1298838 won't support non-e10s either.
- Now that U2F doesn't support non-e10s, disable tests if we're not in
e10s mode.
MozReview-Commit-ID: 5F2323xtXEC
--HG--
extra : transplant_source : v%1Fl%C0%2AJ%26k4%89/%95v%89%12%87%94Y%3Cs
Create a base "nsIU2FToken" interface that all tokens must implement. This
patch does not change U2F.cpp from initializing tokens monolithically, but
if/when future tokens are added, the implementer may want to do that.
MozReview-Commit-ID: GQuu6NolF4D
--HG--
extra : transplant_source : %3Fi%8E%C4n%BF%C1%DB%DB%03HjG%B5%9Ct%9EMWH
Rework U2F.cpp to use a collection of nsINSSU2FToken for U2F/WebAuth operations.
MozReview-Commit-ID: 9qwllawzOWh
--HG--
extra : transplant_source : %E1%7B%15%AEp%8C%1A%3C%E5%9F%13%D1%B3%1D%BB%C2%88%07%0AX
- Move the AppID/FacetID algorithm into its own (potentially reentrant) method
to facilitate Bug 1244959
- Change the Register and Sign operations to be Runnables so that in the future
they can be executed after (future) remote fetches
- Clean up error handling
- Remove unnecessary remote-load Facet test files; we'll re-add some form of
them when the remote load algorithm is completed
MozReview-Commit-ID: 4K1q6ovzhgf
--HG--
extra : transplant_source : /%7F/%96o1%3E%5E%17%20%A2%D0%AA%10%21%88%19%D9%B3%C9
extra : histedit_source : 4d3c61294951920a22e1f1eb7846a2a03f7cd2f0
- Merge in test changes from Bug 1255784.
- Remove the unnecessary mutex
- Stop doing direct memory work in NSS Token
- Clean up direct memory work in ContentParent
- In order to store persistent crypto parameters, the NSSToken had to move
onto the main thread and be interfaced with via IDL/IPDL.
- Support Register/Sign via NSS using a long-lived secret key
- Rename the softtoken/usbtoken "enable" prefs, because of hierarchy issues
with the WebIDL Pref shadowing.
- Also orders the includes on nsNSSModule.cpp
- Attestation Certificates are in Part 2.
Updates per keeler review comments:
- Use //-style comments everywhere
- Refactor the PrivateKeyFromKeyHandle method
- Rename the logging and fix extraneous NS_WARN_IF/logging combinations
- Other updates from review
April 11-12:
- Correct usage of the "usageCount" flag for PK11_UnwrapPrivKey
- Rebase up to latest
April 15:
- Rebase to latest
MozReview-Commit-ID: 6T8jNmwFvHJ
--HG--
extra : transplant_source : w%26%CES%2Cu%04%3EAl%04%2Cb%E2v%C9%08%3A%CC%F4
Tim Taubert
Andrew McCreight
J.C. Jones
Phil Ringnalda
Bill McCloskey
Iris Hsiao
Bevis Tseng
Kyle Machulis
Andi-Bogdan Postelnicu
Kyle Huey
J.C. Jones